From: Guenter M. <mi...@us...> - 2012-01-22 20:45:37
|
On 2012-01-20, David Goodger wrote: > On Fri, Jan 20, 2012 at 05:01, Guenter Milde <mi...@us...> wrote: >> On 2010-12-28, Eli Bendersky wrote: >>> I'm using docutils programmatically for converting ReST to HTML >>> (specifically docutils.core.publish_from_doctree). Since the usage is >>> programmatic (controlled by my application) I must have full control of >>> exceptions for logging. However, I noticed that the FileInput class >>> used by docutils for various purposes exits the application (sys.exit) >>> when it can't find a file. ... >> The easiest fix (with the above side-effect of a distracting >> error message) would be to change the default behaviour: ... >> - autoclose=True, handle_io_errors=True, mode='rU'): >> + autoclose=True, handle_io_errors=False, mode='rU'): ... > -1 on this change. If we only do this, we'll be bombarded by error > reports from users who mis-typed their command lines. This is a > *feature*. I agree that this should be only part of the solution. > The issue seems to be that the handle_io_errors parameter is not > accessible from client code. Turning it off doesn't make it > accessible. Either we should make it accessible, or remove it > completely and handle errors elsewhere. >> For a nice user experience this should be combined with a better response >> to missing "main" input files (catching the error and reporting as >> "severe" system message at an more appropriate place). > I think the current error handling *is* at the most appropriate place. > If you disagree, please suggest a place to do the error handling. The > problem is, removing error handling from there would require > duplicating code in multiple places elsewhere. IMV, the right place is core.Publisher.report_Exceptions. Please see my patch below for a possible implementation. > Instead, how about adding a command-line/config option to control > this? Then programmatic users can handle any exceptions themselves, > and regular users won't see any change. I don't think that an additional config setting is necessary, as the "traceback" setting already switches between passing the exception to the caller or catching and reporting exceptions by core.Publisher.report_Exceptions. > Problem: only docutils.io.FileInput & FileOutput have handle_io_errors > parameters, none of the other docutils.io classes do (they don't need > them, but would were we to generalize). We don't currently pass > settings objects to the docutils.io classes, so we have two choices. > Either add settings parameters to all appropriate docutils.io classes, > or add handle_io_errors parameters to the StringInput & Output > classes. I vote for the latter. Either way, we'll have to change > calling code to pass in appropriate values depending on the settings > value. With "handle_io_errors" for all docutils.io classes, we could pass :: handle_io_errors=not(settings.traceback) to the io.FileInput/Output classes in core.Publisher.set_source() and core.Publisher.set_destination. This also allows to change the default value to ``False``, as throughout docutils FileInput is called 5x with handle_io_errors=False and specific error handling but only one time with handle_io_errors=True. However, I prefer to (first deprecate and later) remove the handle_io_errors argument from io.File(In|Out)put and raise specific exceptions instead. This enables core.Publisher.report_Exceptions() to clear these errors. This way, error handling is done in one place and it is not necessary to pass arguments around with every instantiation of FileInput or FileOutput. Instead, using one of the io.File* classes for file input/output will "automacigally" do the right thing for both, programmatic and command line use. The following patch does not (yet) remove the error-handling code in docutils.io, nor the "handle_io_errors" argument from the calls elsewhere. The 10 lines of additional code are more than offset by 15 lines of obsoleted code. --- io.py (Revision 7307) +++ io.py (Arbeitskopie) @@ -17,6 +17,11 @@ from docutils._compat import b from docutils.error_reporting import locale_encoding, ErrorString, ErrorOutput + +class InputError(IOError): pass +class OutputError(IOError): pass + + class Input(TransformSpec): """ @@ -184,7 +189,7 @@ """ def __init__(self, source=None, source_path=None, encoding=None, error_handler='strict', - autoclose=True, handle_io_errors=True, mode='rU'): + autoclose=True, handle_io_errors=False, mode='rU'): """ :Parameters: - `source`: either a file-like object (which is read directly), or @@ -217,7 +222,8 @@ self.source = open(source_path, mode, **kwargs) except IOError, error: if not handle_io_errors: - raise + raise InputError(error.errno, error.strerror, + source_path) print >>self._stderr, ErrorString(error) print >>self._stderr, (u'Unable to open source' u" file for reading ('%s'). Exiting." % source_path) @@ -286,7 +292,7 @@ def __init__(self, destination=None, destination_path=None, encoding=None, error_handler='strict', autoclose=True, - handle_io_errors=True): + handle_io_errors=False): """ :Parameters: - `destination`: either a file-like object (which is written @@ -327,7 +333,8 @@ self.destination = open(self.destination_path, 'w', **kwargs) except IOError, error: if not self.handle_io_errors: - raise + raise OutputError(error.errno, error.strerror, + self.destination_path) print >>self._stderr, ErrorString(error) print >>self._stderr, (u'Unable to open destination file' u" for writing ('%s'). Exiting." % self.destination_path) --- core.py (Revision 7320) +++ core.py (Arbeitskopie) @@ -260,6 +261,10 @@ self.report_SystemMessage(error) elif isinstance(error, UnicodeEncodeError): self.report_UnicodeError(error) + elif isinstance(error, io.InputError): + self.report_InputError(error) + elif isinstance(error, io.OutputError): + self.report_OutputError(error) else: print >>self._stderr, u'%s' % ErrorString(error) print >>self._stderr, ("""\ @@ -275,6 +280,14 @@ % (error.level, utils.Reporter.levels[error.level])) + def report_InputError(self, error): + self._stderr.write(u'Unable to open source file for reading:\n' + u' %s\n' % ErrorString(error)) + + def report_OutputError(self, error): + self._stderr.write(u'Unable to open destination file for writing:\n' + u' %s\n' % ErrorString(error)) + def report_UnicodeError(self, error): data = error.object[error.start:error.end] self._stderr.write( Thanks, Günter |