Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use File.basename instead sub to correctly handle long paths on Windows #1911

Merged
merged 1 commit into from
Jan 13, 2023
Merged

Use File.basename instead sub to correctly handle long paths on Windows #1911

merged 1 commit into from
Jan 13, 2023

Conversation

CAMOBAP
Copy link
Contributor

@CAMOBAP CAMOBAP commented Jan 4, 2023

Problem

Recently I faced a crash on windows:

C:/Users/RUNNER~1/AppData/Local/Temp/ocr219E.tmp/lib/ruby/gems/2.7.0/gems/rouge-4.0.1/lib/rouge/lexer.rb:532:in `load': cannot load such file -- C:/Users/RUNNER~1/AppData/Local/Temp/ocr219E.tmp/lib/ruby/gems/2.7.0/gems/rouge-4.0.1/lib/rouge/lexers/C:/Users/runneradmin/AppData/Local/Temp/ocr219E.tmp/lib/ruby/gems/2.7.0/gems/rouge-4.0.1/lib/rouge/lexers/abap.rb (LoadError)
	from C:/Users/RUNNER~1/AppData/Local/Temp/ocr219E.tmp/lib/ruby/gems/2.7.0/gems/rouge-4.0.1/lib/rouge/lexer.rb:532:in `load_lexer'
	from C:/Users/RUNNER~1/AppData/Local/Temp/ocr219E.tmp/lib/ruby/gems/2.7.0/gems/rouge-4.0.1/lib/rouge.rb:54:in `block in load_lexers'
	from C:/Users/RUNNER~1/AppData/Local/Temp/ocr219E.tmp/lib/ruby/gems/2.7.0/gems/rouge-4.0.1/lib/rouge.rb:53:in `each'
	from C:/Users/RUNNER~1/AppData/Local/Temp/ocr219E.tmp/lib/ruby/gems/2.7.0/gems/rouge-4.0.1/lib/rouge.rb:53:in `load_lexers'
	from C:/Users/RUNNER~1/AppData/Local/Temp/ocr219E.tmp/lib/ruby/gems/2.7.0/gems/rouge-4.0.1/lib/rouge.rb:69:in `<top (required)>'

It looks like __dir__ may return a short username (or path) RUNNER~1 instead runneradmin as result this prefix not "stripped"

Proposed solution

There is more reliable way to get filename instead f.sub(...) i.e. use File.basename

@CAMOBAP CAMOBAP mentioned this pull request Jan 4, 2023
Closed
@jneen
Copy link
Member

jneen commented Jan 4, 2023

One potential issue here is that we load things that are in subfolders:

load_lexer `ocaml/common.rb`

@CAMOBAP
Copy link
Contributor Author

CAMOBAP commented Jan 4, 2023

One potential issue here is that we load things that are in subfolders:

load_lexer `ocaml/common.rb`

Good catch! I will update the PR

@CAMOBAP CAMOBAP force-pushed the bugfix/windows-long-path branch from 758a99f to 0d06755 Compare January 4, 2023 20:19
@ronaldtse ronaldtse mentioned this pull request Jan 7, 2023
Closed
@ronaldtse
Copy link

Thank you @jneen for the great work and @CAMOBAP for the PR! Any luck that this will be merged soon? Thanks!

@tancnle tancnle self-assigned this Jan 13, 2023
@tancnle tancnle added the needs-review The PR needs to be reviewed label Jan 13, 2023
tancnle
tancnle reviewed Jan 13, 2023
View reviewed changes
lib/rouge.rb Show resolved Hide resolved
@tancnle
Copy link
Collaborator

tancnle commented Jan 13, 2023

Thank you for your contribution @CAMOBAP ❤️. The changes look good to me 👍🏼 🚀

@tancnle tancnle merged commit c15c0d8 into rouge-ruby:master Jan 13, 2023
@ronaldtse
Copy link

Thank you @tancnle ! Look forward to the upcoming release 🍾😉

maxirmx added a commit to metanorma/packed-mn that referenced this pull request Feb 12, 2023
@maxirmx
Remove rouge patch after rouge-ruby/rouge#1911 release
e2fefc3
ronaldtse pushed a commit to metanorma/packed-mn that referenced this pull request Feb 15, 2023
@maxirmx @ronaldtse
Remove rouge patch after rouge-ruby/rouge#1911 release
62da2c4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tancnle tancnle tancnle left review comments

Assignees

@tancnle tancnle

Labels
needs-review The PR needs to be reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CAMOBAP @jneen @ronaldtse @tancnle