Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Ready colorize bug #192

Closed
wants to merge 1 commit into from

3 participants

@tomspur

When calling pycolor on a non-existent file, ipython crashes.
This commit tells the user, that the file doesn't exist and exists properly.

Reproducer, e.g. pycolor "HI" from the RH bug:
https://bugzilla.redhat.com/show_bug.cgi?id=628742

@ellisonbg
Owner

Hmm, I don't see my comment here. Basically, I think I said that I don't like the idea of using print for error messages. Can we just raise an appropriate exception that has a useful error message?

@fperez
Owner

Well, since this is in the main(argv) part of the code, not in the library part, I do think that a print (though it should go to sys.stderr and not to stdout) makes sense: it is shown at the user's system command-line, where I think that human error messages are better than tracebacks.

My thought on this is in general:

  • code called inside libraries always raises, never prints errors.

  • code meant to be called as a system command (possibly by a user who doesn't even know python or what a traceback is) should turn the exception into a more human-friendly, language-indepenedent error message.

This means that the application's main() is about the only part that should be doing this, but in this case that's what is happening so I think that's OK.

How does that sound, Brian?

In summary, I'm OK with the approach, but it should:

  • print to stderr, not stdout.

  • exit via sys.exit(1) which appropriately sets the exit status as a failure, not return.

If those changes are made I have no problem; Brian how does that sound?

@ellisonbg
Owner

This sounds fine. Thanks for clarifying.

@tomspur tomspur pycolor: Wrong filename given -> print error
When a user wanted to colorize a file, which doesn't exist, IPython
would crash. This commit changes this, so the user gets a usefull
message about the wrong filename.

This fixes RH bug #628742.

Signed-off-by: Thomas Spura <tomspur@fedoraproject.org>
5c4429a
@tomspur

It's changed now to print to stderr and sys.exit(1) is used
(Don't know if there is a better approach for python3 and python2...)

Thanks.

@fperez
Owner

I just merged it, but note that I rebased it to avoid a whole branch/merge commit for a single commit. Done in e5effe0.

Thanks a lot!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 2, 2010
  1. @tomspur

    pycolor: Wrong filename given -> print error

    tomspur authored
    When a user wanted to colorize a file, which doesn't exist, IPython
    would crash. This commit changes this, so the user gets a usefull
    message about the wrong filename.
    
    This fixes RH bug #628742.
    
    Signed-off-by: Thomas Spura <tomspur@fedoraproject.org>
This page is out of date. Refresh to see the latest.
Showing with 5 additions and 1 deletion.
  1. +5 −1 IPython/utils/PyColorize.py
View
6 IPython/utils/PyColorize.py
@@ -277,7 +277,11 @@ def main(argv=None):
if fname == '-':
stream = sys.stdin
else:
- stream = file(fname)
+ try:
+ stream = file(fname)
+ except IOError,msg:
+ print >> sys.stderr, msg
+ sys.exit(1)
parser = Parser()
Something went wrong with that request. Please try again.