GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-123756: Only allow restart in command line mode by gaogaotiantian · Pull Request #123757 · python/cpython

Conversation

@gaogaotiantian

@gaogaotiantian gaogaotiantian commented Sep 6, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

An allow_restart argument is added to pdb.Pdb to enable run/restart command when needed (for stdlib, only in command line mode).

  • Issue: Only support restart command in pdb when it's a command line usage #123756

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@gaogaotiantian
Only allow restart in command line mode
8c6fb05
@bedevere-app bedevere-app Bot added the awaiting core review label Sep 6, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Sep 6, 2024
Only support restart command in pdb when it's a command line usage #123756
Closed
@blurb-it
📜🤖 Added by blurb_it.
8a46e08
@gaogaotiantian

gaogaotiantian commented Sep 6, 2024

Copy link
Copy Markdown
Member Author

Hi @iritkatriel , could you take a quick look at the proposal and the implementation and see if this makes sense? I will add the tests and the documentation after your pre-approval.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gaogaotiantian added 2 commits September 6, 2024 21:23
@gaogaotiantian
Use an invocation type enum for restart check
2342a34
@gaogaotiantian
Add PdbInvokeType to __all__
1d731c7
@gaogaotiantian

gaogaotiantian commented Sep 7, 2024

Copy link
Copy Markdown
Member Author

Okay I defined a new Enum - PdbInvokeType for pdb to check whether to enable some commands internally. I also exported the enum because the debuggers based on pdb might need it.

Do you think code wise this is the way to go? I'll work on the docs and tests if the feature is good to go.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gaogaotiantian
Fix pyclbr test
78746ba
iritkatriel
iritkatriel reviewed Sep 7, 2024
View reviewed changes
Comment thread Lib/pdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@gaogaotiantian
Add unknown for invocation type
2b9e225
iritkatriel
iritkatriel reviewed Sep 7, 2024
View reviewed changes
Comment thread Lib/pdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

iritkatriel
iritkatriel reviewed Sep 7, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2024-09-06-01-35-11.gh-issue-123756.Ozbhke.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

gaogaotiantian added 2 commits September 19, 2024 22:20
@gaogaotiantian
Change the enum name and update docs/tests
420e498
@gaogaotiantian
Update news
a49a13e
@gaogaotiantian

gaogaotiantian commented Sep 20, 2024

Copy link
Copy Markdown
Member Author

Hi @iritkatriel , I updated the docs and tests, also renamed the enum class. Could you review it? Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gaogaotiantian
Fix test_pyclbr.py
a4b995e
iritkatriel
iritkatriel reviewed Sep 20, 2024
View reviewed changes
Comment thread Doc/library/pdb.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/pdb.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/pdb.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

gaogaotiantian added 3 commits September 24, 2024 09:56
@gaogaotiantian
Use simple strings for mode
03f91dc
@gaogaotiantian
Revert changes to test_pyclbr
8284118
@gaogaotiantian
Update 2024-09-06-01-35-11.gh-issue-123756.Ozbhke.rst
162c53c
@gaogaotiantian

gaogaotiantian commented Sep 24, 2024

Copy link
Copy Markdown
Member Author

Hey @iritkatriel , I made some changes after the discussion on discord. Now the name of the argument is mode and it accepts a string 'cli' or 'inline', or a None.

A few questions - do you think we should add run-time checker for the value? If we do, will it affect devs that build their debugger based on pdb? Should we have a separate method to check it so it can be overwritten? We can also do a setting kind of thing.

The related question - should we type hint this? It's not common to type hint arguments in pdb.py - one of the reasons is of course the age of the file. However, last time I heard there's still a debate on whether we should type hint as much as possible in stdlibs.

I think one thing we need to factor in is that - this argument is basically exclusively used by debugger developers. The debugger users should never need to use this. So maybe we can cut some corners (not making it super safe to use).

For the documentation, for restart command, I did not link the availability of the command to the mode argument directly because that's not how normal user experiences the difference.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iritkatriel
iritkatriel reviewed Sep 24, 2024
View reviewed changes
Comment thread Doc/library/pdb.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Doc/library/pdb.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/pdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@gaogaotiantian @iritkatriel
Update docs
f9d01f9
Co-authored-by: Irit Katriel <[email protected]>
iritkatriel
iritkatriel reviewed Sep 24, 2024
View reviewed changes
Comment thread Lib/pdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/pdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

gaogaotiantian and others added 2 commits September 24, 2024 21:47
@gaogaotiantian @iritkatriel
Update error messages
0265d90
Co-authored-by: Irit Katriel <[email protected]>
@gaogaotiantian
Merge branch 'main' into pdb-allow-restart
78e2e66
iritkatriel
iritkatriel reviewed Sep 25, 2024
View reviewed changes
Comment thread Doc/library/pdb.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

iritkatriel
iritkatriel reviewed Sep 25, 2024
View reviewed changes
Comment thread Doc/library/pdb.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

iritkatriel
iritkatriel reviewed Sep 25, 2024
View reviewed changes
Comment thread Doc/library/pdb.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

iritkatriel
iritkatriel reviewed Sep 25, 2024
View reviewed changes
Comment thread Lib/pdb.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

iritkatriel
iritkatriel reviewed Sep 25, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2024-09-06-01-35-11.gh-issue-123756.Ozbhke.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

gaogaotiantian and others added 3 commits September 25, 2024 09:58
@gaogaotiantian @iritkatriel
Update docs
c50b039
Co-authored-by: Irit Katriel <[email protected]>
@gaogaotiantian
Update test to match the error message
27f0656
@gaogaotiantian
Add whatsnew entry to the change
7281465
@gaogaotiantian

gaogaotiantian commented Sep 25, 2024

Copy link
Copy Markdown
Member Author

The doc change makes sense. I also added the whatsnew entry.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gaogaotiantian
Remove trailing space
3ce16ec
iritkatriel
iritkatriel approved these changes Sep 25, 2024
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Sep 25, 2024
@gaogaotiantian
gaogaotiantian merged commit 28efeef into python:main Sep 25, 2024
@gaogaotiantian
gaogaotiantian deleted the pdb-allow-restart branch September 25, 2024 18:18
@bedevere-app bedevere-app Bot removed the awaiting merge label Sep 25, 2024
@ADThomas-astro ADThomas-astro mentioned this pull request Feb 14, 2025
Do not raise an Exception when exiting pdb #124703
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatriel iritkatriel iritkatriel approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@gaogaotiantian @iritkatriel
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.