Skip to content
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.

Sign up for GitHub

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

Jump to bottom

gh-109164: Replace getopt with argparse in pdb #109165

Merged
merged 7 commits into from
Sep 22, 2023
+27 −18
Merged

gh-109164: Replace getopt with argparse in pdb #109165

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a9d0109
Replace getopt with argparse in pdb
gaogaotiantian Sep 8, 2023
6c5bb11
📜🤖 Added by blurb_it.
blurb-it[bot] Sep 8, 2023
d51219c
Apply suggestions from code review
gaogaotiantian Sep 10, 2023
e417507
Rename & fix a bug
gaogaotiantian Sep 10, 2023
cdf3570
Add comments about why we need to check argv
gaogaotiantian Sep 16, 2023
2defa6b
Update Lib/pdb.py
gaogaotiantian Sep 18, 2023
f14bc09
Update changelog entry
AA-Turner Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 16 additions & 16 deletions Lib/pdb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2061,8 +2061,6 @@ def help():
pydoc.pager(__doc__)

_usage = """\
usage: pdb.py [-c command] ... [-m module | pyfile] [arg] ...

Debug the Python program given by pyfile. Alternatively,
an executable module or package to debug can be specified using
the -m switch.
Expand All @@ -2077,34 +2075,36 @@ def help():


def main():
import getopt
import argparse

opts, args = getopt.getopt(sys.argv[1:], 'mhc:', ['help', 'command='])
parser = argparse.ArgumentParser(prog="pdb",
description=_usage,
formatter_class=argparse.RawDescriptionHelpFormatter)
gaogaotiantian marked this conversation as resolved.
Show resolved Hide resolved

if not args:
print(_usage)
sys.exit(2)
parser.add_argument('-c', '--command', action='append', default=[])
parser.add_argument('-m', action='store_true')
parser.add_argument('pyfile', nargs=1)
parser.add_argument('args', nargs="*")
gaogaotiantian marked this conversation as resolved.
Show resolved Hide resolved

if any(opt in ['-h', '--help'] for opt, optarg in opts):
print(_usage)
sys.exit()
if len(sys.argv) == 1:
AA-Turner marked this conversation as resolved.
Show resolved Hide resolved
parser.print_help()
sys.exit(2)

commands = [optarg for opt, optarg in opts if opt in ['-c', '--command']]
opts = parser.parse_args()

module_indicated = any(opt in ['-m'] for opt, optarg in opts)
cls = _ModuleTarget if module_indicated else _ScriptTarget
target = cls(args[0])
cls = _ModuleTarget if opts.m else _ScriptTarget
target = cls(opts.pyfile[0])
gaogaotiantian marked this conversation as resolved.
Show resolved Hide resolved

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer:

if opts.module:
    args = [opts.module]
    ...
else:
    args = [opts.pyfile]
    ...

args.extend(opts.ags)

...

sys.argv = args  # Hide "pdb.py" and pdb options from argument list

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be a larger refactor, the current state reflects what's currently present.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that's why I'm asking for :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is pretty similar right? Just a slightly different implementation. I don't think this is too much if this is preferred.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just disturbed by calling "module" a "file".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, feel free to ignore my comment about this code.

target.check()

sys.argv[:] = args # Hide "pdb.py" and pdb options from argument list
sys.argv[:] = opts.pyfile + opts.args # Hide "pdb.py" and pdb options from argument list
gaogaotiantian marked this conversation as resolved.
Show resolved Hide resolved

# Note on saving/restoring sys.argv: it's a good idea when sys.argv was
# modified by the script being debugged. It's a bad idea when it was
# changed by the user from the command line. There is a "restart" command
# which allows explicit specification of command line arguments.
pdb = Pdb()
pdb.rcLines.extend(commands)
pdb.rcLines.extend(opts.command)
while True:
try:
pdb._run(target)
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2023-09-08-22-26-26.gh-issue-109164.-9BFWR.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Replace getopt with argparse for parsing arguments in :mod:`pdb`
AA-Turner marked this conversation as resolved.
Show resolved Hide resolved