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.

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

sys.exit() called from optparse - bad, bad, bad #47329

Closed
smontanaro opened this issue Jun 11, 2008 · 7 comments
Closed

sys.exit() called from optparse - bad, bad, bad #47329

smontanaro opened this issue Jun 11, 2008 · 7 comments
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@smontanaro
Copy link
Contributor

BPO 3079
Nosy @smontanaro, @birkenfeld, @pitrou, @bitdancer, @Phillip_M_Feldman

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2009-10-16.00:36:50.116>
created_at = <Date 2008-06-11.17:23:26.915>
labels = ['easy', 'type-bug', 'library']
title = 'sys.exit() called from optparse - bad, bad, bad'
updated_at = <Date 2009-10-16.03:33:12.963>
user = 'https://github.com/smontanaro'

bugs.python.org fields:

activity = <Date 2009-10-16.03:33:12.963>
actor = 'Phillip.M.Feldman@gmail.com'
assignee = 'none'
closed = True
closed_date = <Date 2009-10-16.00:36:50.116>
closer = 'r.david.murray'
components = ['Library (Lib)']
creation = <Date 2008-06-11.17:23:26.915>
creator = 'skip.montanaro'
dependencies = []
files = []
hgrepos = []
issue_num = 3079
keywords = ['easy']
message_count = 7.0
messages = ['67999', '68149', '68156', '68171', '94114', '94116', '94122']
nosy_count = 6.0
nosy_names = ['skip.montanaro', 'georg.brandl', 'ggenellina', 'pitrou', 'r.david.murray', 'Phillip.M.Feldman@gmail.com']
pr_nums = []
priority = 'normal'
resolution = 'wont fix'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue3079'
versions = ['Python 2.6', 'Python 2.5', 'Python 3.0']

@smontanaro
Copy link
Contributor Author

This seems like a bug in optparse.OptionParser:

    def exit(self, status=0, msg=None):
        if msg:
            sys.stderr.write(msg)
        sys.exit(status)

    def error(self, msg):
        """error(msg : string)
    Print a usage message incorporating 'msg' to stderr and exit.
    If you override this in a subclass, it should not return -- it
    should either exit or raise an exception.
    """
    self.print_usage(sys.stderr)
    self.exit(2, "%s: error: %s\n" % (self.get_prog_name(), msg))

By default I think it should raise an exception when it encounters an
error, not exit. Programmers shouldn't be forced to subclass code in
the standard library to get recommended practice.

If you feel this behavior can't be changed in 2.6 it should at least
be corrected in 3.0.

The cruel irony is that inside OptionParser.parse_args it actually
catches both BadOptionError and OptionValueError but suppresses them
by calling self.error() within the except block... *arrgggghhh*...
The correct behavior there is (in my opinion) to get rid of the
try/except statement altogether and just let the exceptions propagate.
Other calls to self.error() should be replaced with suitable raise
statements.

Skip

@smontanaro smontanaro added stdlib Python modules in the Lib dir easy type-bug An unexpected behavior, bug, or error labels Jun 11, 2008
@pitrou
Copy link
Member

pitrou commented Jun 13, 2008

The current behaviour is useful in that most of time, it is convenient
to let OptionParser display a standard error message and bail out.
However, having an attribute on the OptionParser object (e.g.
exit_on_errors) to be able to change this behaviour is a reasonable
proposition.

@birkenfeld
Copy link
Member

I agree with Antoine that the standard behavior is what you want in most
simple command-line scripts.

It's easy enough to replace the parser's exit function to just print the
message, or raise an exception.

@smontanaro
Copy link
Contributor Author

Georg> I agree with Antoine that the standard behavior is what you want in most
Georg> simple command-line scripts.

Georg> It's easy enough to replace the parser's exit function to just print the
Georg> message, or raise an exception.

Check the code. Most of the time error is called without an exception
having been raised. In one case it actually is called and swallows an
exception that the code did raise. It would be preferable in my mind to
always raise an exception. If you want, the default can be to catch them
and ignore them as error() does now, but as it stands you can't raise
anything more specific than OptParseError, and that's just a punt even
though specific problems were detected by the code.

Skip

@PhillipMFeldman
Copy link
Mannequin

PhillipMFeldman mannequin commented Oct 16, 2009

The current behavior of optparse is contrary to how most of Python
works. optparse should throw a named exception that can be trapped and
identified by the calling program. Doing a SystemExit is unacceptable.
I can't believe that this is such a hard thing to fix.

@bitdancer
Copy link
Member

There was recently a long discussion of this on python-dev (in the
context of a proposal to add argparse to the stdlib; argparse does the
same thing). The conclusion was that the current behavior is the most
useful behavior, and that if you don't want to exit you can either
subclass or catch SystemExit.

@PhillipMFeldman
Copy link
Mannequin

PhillipMFeldman mannequin commented Oct 16, 2009

Thanks for the response!

I can indeed catch SystemExit, but I would like to be able to take one
action (terminate the program) if the user supplied an unknown option,
and another action (prompt for a new value) if the user supplied a bad
value for an option. I suspect that I can achieve this by subclassing,
but I'm not yet at that level of Python sophistication.

Yours,

Phillip

R. David Murray wrote:

R. David Murray <rdmurray@bitdance.com> added the comment:

There was recently a long discussion of this on python-dev (in the
context of a proposal to add argparse to the stdlib; argparse does the
same thing). The conclusion was that the current behavior is the most
useful behavior, and that if you don't want to exit you can either
subclass or catch SystemExit.

----------
nosy: +r.david.murray
resolution: -> wont fix
stage: -> committed/rejected
status: open -> closed


Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue3079\>


@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants