-
Notifications
You must be signed in to change notification settings - Fork 251
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
Marker name is a string, not a lexer token #286
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add some regression coverage for this one?
I must admit that I'm struggling now to recall why this was a problem. Per https://lark-parser.readthedocs.io/en/latest/classes.html#lark.Token
which seems like it should be fine, and my attempts to demonstrate otherwise in unit test are failing. I'd be inclined to merge this anyway - it can't possibly be wrong, can it? But I am unable to prove that it is necessary, so if you're feeling super-cautious I guess don't. Maybe we sometimes end up with different tokens for the same thing? This looks like an accident, I wonder whether that could confuse things sometimes?!? |
I'm back on the trail... depending on where the parser is called from, you can end up with (eg) either |
testcase added |
ea59115
to
f2382c9
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
* Marker name is a string, not a lexer token * add unit test
* Marker name is a string, not a lexer token * add unit test
Discovered incidentally while working on python-poetry/poetry#5156 - sometimes a marker name was set to a lexer token rather than a string which, for instance, defeats comparisons for equality.