Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
pablogsal merged 12 commits intopython:mainfromswfarnsworth:elif-error-message
Apr 25, 2025

Conversation

swfarnsworth
Copy link
Contributor

@swfarnsworthswfarnsworth commentedFeb 9, 2025
edited by bedevere-appbot
Loading

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.

Previously, having an elif block after an else block would raise a standard syntax error.
@swfarnsworth
Copy link
ContributorAuthor

@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.

@picnixz
Copy link
Member

I compiled cpython with this version of the parser and confirmed that it doesn't conflict with other special syntax or indentation errors.

Please add a test. There should be tests in sometest_syntax.py file ortest_parser.py files (I don't remember where we put them). Just Ctrl+F some error message to find the file.

@swfarnsworth
Copy link
ContributorAuthor

@picnixz No problem--will do.

picnixz reacted with rocket emoji

@swfarnsworth
Copy link
ContributorAuthor

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
Copy link

ghost commentedFeb 11, 2025
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@swfarnsworth
Copy link
ContributorAuthor

Those last two commits should appear as being from this GitHub account. Let me see if I can delete them and try again.

@picnixz
Copy link
Member

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)

@swfarnsworth
Copy link
ContributorAuthor

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 reacted with thumbs up emoji

@picnixz
Copy link
Member

  • For the detection issue, you should just merge main into that branch with theUpdate branch button!
  • A NEWS entry and a What's New entry could be added in order to indicate that we have an improved message now. I don't have a good wording here though so maybe@hugovk may have some suggestion?

@swfarnsworth
Copy link
ContributorAuthor

We can draw inspiration fromhere, I suppose?

@swfarnsworth
Copy link
ContributorAuthor

What about this? I'm not sure if the blocks need something more realistic thanpass.

elif statements that follow anelse block now have a specific error message.

>>>ifword.startswith('a'):...pass...else:...pass...elifword.startswith('b'):...passFile"<stdin>",line5elifword.startswith('b'):^^^^SyntaxError:'elif'blockfollowsan'else'block

@picnixz
Copy link
Member

We can draw inspiration fromhere, I suppose?

Yes that's what I had in mind.

@picnixz
Copy link
Member

picnixz commentedFeb 12, 2025
edited
Loading

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
Copy link
ContributorAuthor

I like that, but I think I'll do "It's me!" and "It's not me!" in honor of Elphaba,

@picnixz
Copy link
Member

I like that, but I think I'll do "It's me!" and "It's not me!" in honor of Elphaba,

Let's go for this one :)

swfarnsworth reacted with heart emoji

@swfarnsworth
Copy link
ContributorAuthor

We also get a subtle homage toBritney :)

Copy link
Member

@hugovkhugovk left a 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.

swfarnsworthand others added3 commitsFebruary 14, 2025 10:48
…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>
Copy link
Member

@picnixzpicnixz left a 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

@@ -174,6 +174,22 @@ Improved error messages
^^^^^^^
ValueError: too many values to unpack (expected 3, got 4)

* ``elif`` statements that follow an ``else`` block now have a specific error message.
Copy link
Member

@picnixzpicnixzFeb 15, 2025
edited
Loading

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.
Copy link
Member

@picnixzpicnixzFeb 15, 2025
edited
Loading

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.

Copy link
Member

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.

@swfarnsworth
Copy link
ContributorAuthor

@picnixz done!
Is this going to get squash merged?

@picnixz
Copy link
Member

Is this going to get squash merged?

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
Copy link
ContributorAuthor

I see that there are now merge conflicts withparser.c. I figure this means someone else has made changes to the grammar. Should I undo my changes to parser.c, pull the changes to the grammar, and rebuild parser.c?

@picnixz
Copy link
Member

I see that there are now merge conflicts with parser.c. I figure this means someone else has made changes to the grammar.

You can just regenerate the parser files indeed (I think they are automatically generated right?)

@swfarnsworth
Copy link
ContributorAuthor

@picnixz yeah, there's a make command to do it.

@picnixz
Copy link
Member

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-bot
Copy link

python-cla-botbot commentedApr 18, 2025
edited
Loading

All commit authors signed the Contributor License Agreement.

CLA signed

@hugovk
Copy link
Member

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/

@swfarnsworth
Copy link
ContributorAuthor

Apologies for the delay@hugovk@picnixz. I was having issues with the dev container I was using to develop this and didn't see the message from picnixz that soon followed my most recent message.

hugovk reacted with thumbs up emoji

@hugovk
Copy link
Member

Please check the CI failures.

@swfarnsworth
Copy link
ContributorAuthor

@hugovk@picnixz I think everything is fixed.

@pablogsalpablogsalenabled auto-merge (squash)April 25, 2025 01:25
@pablogsalpablogsal merged commit99b71ef intopython:mainApr 25, 2025
46 checks passed
@pablogsal
Copy link
Member

LGTM!

Fantastic job@swfarnsworth! Thanks for your contribution :)

Zoidmania reacted with rocket emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@pablogsalpablogsalpablogsal approved these changes

@lysnikolaoulysnikolaouAwaiting requested review from lysnikolaoulysnikolaou is a code owner

@hugovkhugovkAwaiting requested review from hugovk

@picnixzpicnixzAwaiting requested review from picnixz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@swfarnsworth@picnixz@hugovk@pablogsal

[8]ページ先頭

©2009-2025 Movatter.jp