Skip to content

CLI option parser passes empty option strings, causing startup failure#428

Open
huyhoang171106 wants to merge 1 commit intojoerick:mainfrom
huyhoang171106:fix/cli-option-parser-passes-empty-option-st
Open

CLI option parser passes empty option strings, causing startup failure#428
huyhoang171106 wants to merge 1 commit intojoerick:mainfrom
huyhoang171106:fix/cli-option-parser-passes-empty-option-st

Conversation

@huyhoang171106
Copy link
Copy Markdown

Summary

Several optparse.OptionParser.add_option(...) calls include "" as an option string (e.g. parser.add_option("", "--load-prev", ...), parser.add_option("-m", "", ...)). optparse validates every option token and rejects empty strings with OptionError during parser setup, which can crash the CLI before argument parsing begins.

Files changed

  • pyinstrument/__main__.py (modified)

Testing

  • Not run in this environment.

…tup failure

Signed-off-by: Nguyen Huy Hoang <181364121+huyhoang171106@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 26, 2026 16:23
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a CLI startup crash caused by invalid optparse option definitions, where empty option strings ("") are passed to OptionParser.add_option(...) and trigger OptionError during parser setup.

Changes:

  • Removed empty option-string arguments from several parser.add_option(...) calls in pyinstrument/__main__.py.
Comments suppressed due to low confidence (2)

pyinstrument/main.py:187

  • This fix is incomplete: there are still several later parser.add_option("", ...) calls (e.g. for --show-regex, --show-all, --unicode/--no-unicode, --color/--no-color, --use-timing-thread). optparse will still raise OptionError during parser setup as long as any empty option string remains. Please remove the empty-string option token from those remaining calls as well so main() can start up reliably.
    parser.add_option(
        "--show",
        dest="show_fnmatch",
        action="store",
        metavar="EXPR",

pyinstrument/main.py:72

  • Please add a regression test that invokes pyinstrument.__main__.main() (or at least builds the OptionParser) to assert it doesn't raise optparse.OptionError during option registration. There are existing CLI tests under test/test_cmdline_main.py, so a small test covering parser construction would prevent this startup-crash regression from returning.
    parser.add_option(
        "--load-prev",
        dest="load_prev",
        action="store",
        metavar="IDENTIFIER",
        help="instead of running a script, load a previous profile session as specified by an identifier",
    )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants