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

enh(python) Match numeric literals per the language reference #2780

Merged

Conversation

gibson042
Copy link
Contributor

I started to work on #2766, but realized that the Python foundation I planned to use was itself incomplete, so I'm testing the waters with this and can follow up if it is accepted.

Changes

Replaced generic numeric regular expressions with specific ones to match the Python language reference.

Checklist

  • Added markup tests, or they don't apply here because...
  • Updated the changelog at CHANGES.md
  • Added myself to AUTHORS.txt, under Contributors

src/languages/python.js Outdated Show resolved Hide resolved
@joshgoebel
Copy link
Member

Great start! Looking good.

@gibson042
Copy link
Contributor Author

@joshgoebel I see that a WIP 10.4.0 has been added to CHANGES.md... should this PR be part of that version, or a new WIP 10.3.2?

@joshgoebel
Copy link
Member

10.4, x.x.Y is only for patch releases.

Are you wanting to solve the issue here on top of this work or make another PR after this one is merged?

src/languages/python.js Outdated Show resolved Hide resolved
@joshgoebel
Copy link
Member

Looking good other than the notes I added. Almost there.

@gibson042 gibson042 force-pushed the 2020-10-python-numeric-literals branch from 68c500d to 2c59cf0 Compare October 24, 2020 20:28
@gibson042
Copy link
Contributor Author

Are you wanting to solve the issue here on top of this work or make another PR after this one is merged?

I'd prefer to handle the Ruby updates in a separate PR, which I can start on shortly (it will look very similar to this one).

@joshgoebel
Copy link
Member

I'd prefer to handle the Ruby updates in a separate PR, which I can start on shortly (it will look very similar to this one).

Oh sure, I thought there was more python stuffs. :)

src/languages/python.js Outdated Show resolved Hide resolved
src/languages/python.js Outdated Show resolved Hide resolved
@joshgoebel joshgoebel added the hacktoberfest-accepted Hacktoberfest label Oct 25, 2020
@joshgoebel joshgoebel merged commit 21b1466 into highlightjs:master Oct 25, 2020
@joshgoebel
Copy link
Member

@gibson042 Thanks so much!

@joshgoebel
Copy link
Member

@gibson042 Can you please have a look at #2985

Is there a reason you went with \b for terminators for many numbers? IE, a specific problem you were trying to solve?

@joshgoebel
Copy link
Member

Ping @gibson042

@joshgoebel
Copy link
Member

@gibson042 Ping.

1 similar comment
@joshgoebel
Copy link
Member

@gibson042 Ping.

@gibson042
Copy link
Contributor Author

Whoops, I'm not sure how I missed all those (but it has been a heck of a year). The primary reason for leading \bs is avoiding erroneous recognition of numbers inside identifiers like var0, and the primary reason for trailing \bs is avoiding tokenization like that of #2985 (which are invalid in many languages, but apparently not in Python). Let me see what I can do with it.

@joshgoebel
Copy link
Member

Let me see what I can do with it.

Thanks - we're looking for an easy/simple fix though. I don't want to just keep adding tons of edge cases/complexity only to deal with code golf. I'd rather just say we don't support it.

So if nothing is obviously don't throw a bunch of time into coming up with anything crazy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants