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-108463: Make expressions/statements work as expected in pdb#108464

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

Conversation

gaogaotiantian
Copy link
Member

@gaogaotiantiangaogaotiantian commentedAug 25, 2023
edited by bedevere-bot
Loading

This PR makes expressions like

c.ac['a']n()j=1r"a"

work in pdb.

@gaogaotiantian
Copy link
MemberAuthor

Hi@iritkatriel , this is a very straighforward feature (implementation wise) and the code change is trivial. It's only about whether we should do it or not. Could you give it a review when you have some time? Thanks!

@iritkatriel
Copy link
Member

Hi@iritkatriel , this is a very straighforward feature (implementation wise) and the code change is trivial. It's only about whether we should do it or not. Could you give it a review when you have some time? Thanks!

Sorry only saw this now.

Can this break anything?

@gaogaotiantian
Copy link
MemberAuthor

Sorry only saw this now.

Can this break anything?

I can't think of any meaningful case that this could break. This change only affects how "cmd" parse commands - basically what characters can be considered as the command. In pdb, we expect users to use space to separate commands and args. All the supported commands only contain letters.

So in real world, this will only make a difference when the user types a string that contains characters other than letters, numbers and underscores, for example,c['a']. In such cases, the user always almost want to evaluate the expression(or execute the statement), but forget thatc is a command.

This feature basically changed the behavior when the user types such statements/expressions - it does not report "command invalid" anymore - it runs the statement/expression.

@iritkatriel
Copy link
Member

This needs to be documented quite prominently as a new feature because it changes behaviour quite visibly. Someone might be surprised that something now executes code when it was previously a NOP.

@gaogaotiantian
Copy link
MemberAuthor

gaogaotiantian commentedSep 3, 2023
edited
Loading

This needs to be documented quite prominently as a new feature because it changes behaviour quite visibly. Someone might be surprised that something now executes code when it was previously a NOP.

I think the behavior after this change actually is closer to what the current documentation claims.

Arguments to commands must be separated by whitespace (spaces or tabs)

This defines howpdb should treat commands and arguments, which is not what we are doing now. According to the current docs, if the user inputsc['a'], it should be treated as a "command" because there's no whitespace anywhere. However, the current behavior treats it as commandc + argument['a'], which is against the docs.

I don't think anyone would be surprised if they inputc['a'] and pdb evaluates the expression. If anything, this change eliminates a lot of surprises. The user would be pretty surprised if they doc['a'] andpdb complaints that it does not recognize the argument['a'].

We also claimed in the docs that

Commands that the debugger doesn’t recognize are assumed to be Python statements and are executed in the context of the program being debugged

which is exactly what this change tries to do -c['a'] is not a command the debugger recognizes and should be considered as a Python expression.

Why would the user expect different behavior when they dod['a'] (currently evaluates the expression) andc['a'] (currently reports an error)? That's not intuitive.

I honestly can not think of a single case where the user would feel surprised with this change, even if they are very used to the old pdb.

@iritkatriel
Copy link
Member

I'm not disputing the value of the change. But it's a change in behaviour which needs to be documented (a versionchanged entry, a what's new in 3.13 entry).

Maybe someone will be surprised by the 3.12 behaviour, or the change between 3.12 and 3.13. It should be clear from the docs that this is a deliberate change.

@gaogaotiantian
Copy link
MemberAuthor

I'm not disputing the value of the change. But it's a change in behaviour which needs to be documented (a versionchanged entry, a what's new in 3.13 entry).

Maybe someone will be surprised by the 3.12 behaviour, or the change between 3.12 and 3.13. It should be clear from the docs that this is a deliberate change.

Okay, I'm a bit confused about what to document, as this is more like a "bug fix" to make pdb behave like it claimed.

Maybe

pdb will execute the Python statement even if the first piece of the statement (seperated by spaces) starts with apdb command.

This is basically what this feature fixes. I'm not sure how to organize the explanation as it's doing what the docs say. Any suggestsions on the matter?

@iritkatriel
Copy link
Member

I think something like:

.. versionchanged 3.13   Expressions whose prefix is a pdb command are now correctly identified and executed.

And something similar in what's new in 3.13.

@iritkatrieliritkatrielenabled auto-merge (squash)September 4, 2023 21:26
@iritkatrieliritkatriel added the stdlibPython modules in the Lib dir labelSep 4, 2023
@iritkatrieliritkatriel merged commit6304d98 intopython:mainSep 4, 2023
@gaogaotiantiangaogaotiantian deleted the pdb-improve-commands branchSeptember 4, 2023 23:39
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@chgnrdvchgnrdvchgnrdv left review comments

@iritkatrieliritkatrieliritkatriel approved these changes

Assignees
No one assigned
Labels
stdlibPython modules in the Lib dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@gaogaotiantian@iritkatriel@chgnrdv@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp