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

Remove . as a operator in Python lexer #1375

Merged
merged 2 commits into from Apr 5, 2020
Merged

Remove . as a operator in Python lexer #1375

merged 2 commits into from Apr 5, 2020

Conversation

ajnisbet
Copy link
Contributor

Fixes #1374

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

pyrmont commented Dec 12, 2019

Thanks for submitting this, @ajnisbet. This does look like an error in the way Pygments and Rouge approach things. Will have a review over the next few days and see!

@pyrmont
Copy link
Contributor

pyrmont commented Dec 13, 2019

@ajnisbet I made a change as removing the line that tokenised . as an Operator caused all dot-notation syntax to break.

My suggestion is that we add . to the list of punctuation:

rule %r/[\[\]{}:(),;.]/, Punctuation

In themes that render Puncutation tokens in the same colour as Name tokens, the two will be indistinguishable, but in themes where punctuation is highlighted in a different colour, that will continue to work.

Let me know your thoughts.

@pyrmont pyrmont added author-action The PR has been reviewed but action by the author is needed and removed needs-review The PR needs to be reviewed labels Dec 13, 2019
@ajnisbet
Copy link
Contributor Author

Yeah that seems like a good solution!

@pyrmont
Copy link
Contributor

pyrmont commented Dec 13, 2019

@jneen I don't imagine you're a big Python user but do you have an opinion?

I'm hesitant to break ranks with Pygments (see the relevant line in their codebase)—especially given it is a Python project—but tokenising . as Operator does seem to me to be incorrect given it's not an operator.

@pyrmont pyrmont added maintainer-action The PR has been reviewed but action by a maintainer is required and removed author-action The PR has been reviewed but action by the author is needed labels Dec 13, 2019
@pyrmont pyrmont merged commit c1388b8 into rouge-ruby:master Apr 5, 2020
@pyrmont pyrmont removed the maintainer-action The PR has been reviewed but action by a maintainer is required label Apr 5, 2020
@pyrmont
Copy link
Contributor

pyrmont commented Apr 5, 2020

@ajnisbet Sorry it took so long to address this. This update will be part of the next version of Rouge which is scheduled for release on Tuesday 14 April. Thanks for the contribution!

mattt pushed a commit to NSHipster/rouge that referenced this pull request May 21, 2020
Rouge, consistent with Pygments, lexes `.` as an operator. Doing so
causes issues with numbers that use `.`. The Python specification does 
not identify `.` as an operator. This commit instead causes `.` to be
lexed as punctuation.
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.

. shouldn't be an operator in Python
2 participants