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.

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

Fix symbol pattern in Scala lexer #1438

Merged
merged 1 commit into from Feb 16, 2020

Conversation

pyrmont
Copy link
Contributor

@pyrmont pyrmont commented Feb 12, 2020

The Scala lexer currently matches 'symbols' by ensuring that the last character in the match is not a '.

The problem with this approach is that if the symbol occurs within a lexical structure (e.g. parentheses), the first character after the symbol (e.g. the closing parenthesis) would be matched as part of the symbol.

This PR uses the approach of a negative lookahead to avoid this problem.

This fixes #374.

@pyrmont pyrmont self-assigned this Feb 12, 2020
@pyrmont pyrmont added the needs-review The PR needs to be reviewed label Feb 12, 2020
@pyrmont
Copy link
Contributor Author

pyrmont commented Feb 12, 2020

@anthony-cros How's this look to you?

@anthony-cros
Copy link

Conceptually the negative lookahead approach sounds reasonnable.

I'm not very familiar with Ruby in general, I tried TEST=spec/lexers/scala_spec.rb rake from the cloned repo but got a:

rake aborted!
LoadError: cannot load such file -- bundler/setup

Can you maybe just show a screenshot?

@pyrmont
Copy link
Contributor Author

pyrmont commented Feb 12, 2020

@anthony-cros See below:

08162B73-44DF-482A-BB98-43E351C8F1B1

@anthony-cros
Copy link

Looks good :)

@pyrmont pyrmont merged commit fcddd15 into rouge-ruby:master Feb 16, 2020
@pyrmont pyrmont deleted the bugfix.scala-symbols branch February 16, 2020 22:33
@pyrmont
Copy link
Contributor Author

pyrmont commented Feb 16, 2020

@anthony-cros Thanks! This fix will be out as part of v3.17.0. That's scheduled for release on Tuesday 17 March 🎉

@pyrmont pyrmont removed the needs-review The PR needs to be reviewed label Feb 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tiny bug in scala lexer
2 participants