Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-129858: Special syntax error forelif block afterelse#129902
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
Previously, having an elif block after an else block would raise a standard syntax error.
swfarnsworth commentedFeb 9, 2025
@pablogsal I see that you've been tagged for review. Please see the associated issue for discussion about whether an even more sophisticated error message is possible. |
Uh oh!
There was an error while loading.Please reload this page.
picnixz commentedFeb 9, 2025
Please add a test. There should be tests in some |
swfarnsworth commentedFeb 9, 2025
@picnixz No problem--will do. |
swfarnsworth commentedFeb 10, 2025
I've implemented a test as requested, though I'll hold off on committing or pushing it until we decide on the exact wording of the error message. |
ghost commentedFeb 11, 2025 • edited by ghost
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by ghost
Uh oh!
There was an error while loading.Please reload this page.
swfarnsworth commentedFeb 11, 2025
Those last two commits should appear as being from this GitHub account. Let me see if I can delete them and try again. |
picnixz commentedFeb 11, 2025
In general, we avoid force-pushing, but you can do it in this case and amend the commit author and put the correct email address. (Not sure if it could help in this though) |
62996a0 to883ada9Compareswfarnsworth commentedFeb 12, 2025
The force push seems to have fixed the problem without any issues. The usernames for my work and personal email differ by one letter, which is occasionally inconvenient. |
picnixz commentedFeb 12, 2025
|
swfarnsworth commentedFeb 12, 2025
We can draw inspiration fromhere, I suppose? |
swfarnsworth commentedFeb 12, 2025
What about this? I'm not sure if the blocks need something more realistic than
>>>ifword.startswith('a'):...pass...else:...pass...elifword.startswith('b'):...passFile"<stdin>",line5elifword.startswith('b'):^^^^SyntaxError:'elif'blockfollowsan'else'block |
picnixz commentedFeb 12, 2025
Yes that's what I had in mind. |
picnixz commentedFeb 12, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Another possibility is: >>>ifwho=="me":...print("This is me!")...else:...print("This is not me!")...elifwhoisNone:...print("Who is it?")File"<stdin>",line5elifwhoisNone:^^^^SyntaxError:'elif'blockfollowsan'else'block |
swfarnsworth commentedFeb 12, 2025
I like that, but I think I'll do "It's me!" and "It's not me!" in honor of Elphaba, |
picnixz commentedFeb 12, 2025
Let's go for this one :) |
swfarnsworth commentedFeb 12, 2025
We also get a subtle homage toBritney :) |
Misc/NEWS.d/next/Core_and_Builtins/2025-02-12-01-36-13.gh-issue-129858.M-f7Gb.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
hugovk left a comment
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.
I've updated the PR frommain which will fix the unrelated "computed changes" error.
Misc/NEWS.d/next/Core_and_Builtins/2025-02-12-01-36-13.gh-issue-129858.M-f7Gb.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
…e-129858.M-f7Gb.rstCo-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
picnixz left a comment
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.
Some markup nits and LGTM
Doc/whatsnew/3.14.rst Outdated
| ^^^^^^^ | ||
| ValueError: too many values to unpack (expected3, got4) | ||
| * ``elif`` statements that follow an ``else`` block now have a specific error message. |
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.
*:keyword:`elif` statements that follow an:keyword:`else` block now have a specific error message.
| @@ -0,0 +1 @@ | |||
| ``elif`` statements that follow an ``else`` block now have a specific error message. | |||
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.
:keyword:`elif` statements that follow an:keyword:`else` block now have a specific error message.
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.
You can also put the keyword syntax here.
Uh oh!
There was an error while loading.Please reload this page.
swfarnsworth commentedFeb 15, 2025
@picnixz done! |
picnixz commentedFeb 15, 2025
Yes, that's why contributors don't need to force-push unless they know what they're doing and/or it's a draft PR. Force-pushing rewrites the history and makes reviews harder if we want to compare with a previous commit (and that's the main reason why we don't want force pushes). I personally do force-push when I have no PR open or if it's a draft or if I need to amend a commit due to git issues (like you had). |
swfarnsworth commentedFeb 28, 2025
I see that there are now merge conflicts with |
picnixz commentedFeb 28, 2025
You can just regenerate the parser files indeed (I think they are automatically generated right?) |
swfarnsworth commentedFeb 28, 2025
@picnixz yeah, there's a make command to do it. |
picnixz commentedFeb 28, 2025
Just merge the grammar from main, and regenerate the Parser/parser.c file then. When there are conflicts in auto-generated files, just regenerating them is fine. |
python-cla-botbot commentedApr 18, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
hugovk commentedApr 18, 2025
Please can you fix the merge conflict? Note the 3.14 feature freeze is in just under three weeks:https://peps.python.org/pep-0745/ |
… not implement changes from this PR.
swfarnsworth commentedApr 19, 2025
hugovk commentedApr 19, 2025
Please check the CI failures. |
swfarnsworth commentedApr 24, 2025
99b71ef intopython:mainUh oh!
There was an error while loading.Please reload this page.
pablogsal commentedApr 25, 2025
LGTM! Fantastic job@swfarnsworth! Thanks for your contribution :) |
Uh oh!
There was an error while loading.Please reload this page.
Previously, having an elif block after an else block would raise a standard syntax error. This PR implements a special syntax error saying "elif not allowed after else".
I compiled cpython with this version of the parser and confirmed that it doesn't conflict with other special syntax or indentation errors.
elifafterelse#129858