-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-130077: Enhance GrammarVisitor to differentiate visit methods for single and multi-character node classes #130162
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
Conversation
…multi-character node classes
@lysnikolaou Can you help me understand the issue with the "NO news entry in Misc/News.d/next"? |
You need to add a blurb indicating what this changes. |
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@AA-Turner can you please help me resole the lint issue |
You need to delete the duplicate blurb entry and use double backticks for literal syntax. See the devguide for a brief overview of reStructuredText. A |
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.
Please add a test.
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.
There's two blurb files, go ahead and delete one of them.
@@ -0,0 +1,3 @@ | |||
Fix potential memory leak in c-field representation of Python objects |
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.
This doesn't seem related to the actual change. Is this from a different PR?
@@ -19,7 +19,12 @@ class GrammarError(Exception): | |||
class GrammarVisitor: | |||
def visit(self, node: Any, *args: Any, **kwargs: Any) -> Any: | |||
"""Visit a node.""" | |||
method = "visit_" + node.__class__.__name__ | |||
node_cls = node.__class__.__name__ |
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.
Use type()
, not __class__
.
I don't understand anything about this change unfortunately. Could you please explain how you approach is supposed to work? |
Yeah, I smell ChatGPT based off the PR description. @isatyamks, please be aware of our AI policy. |
I'm closing this in favour of #135317 |
gh-130077: Fix inconsistent syntax error messages for soft keyword prefixes
This PR addresses issue #130077 by ensuring consistent syntax error messages when an identifier that is a prefix of a soft keyword starts an expression. Previously, different messages were generated for cases like
(ma x)
and(match_ x)
, leading to inconsistencies.Changes:
visit_single_
andvisit_multi_
methods to improve error handling in syntax parsing.This ensures that syntax errors related to soft keyword prefixes produce uniform and informative messages.