> Having added the Directive.system_message method, things look much
> However, I'd like to be able to throw system messages out of
> auxiliary methods. E.g., writing "self.assert_has_content()" is
> only possible if the Directives.assert_has_content() method can
> throw system messages.
We don't "throw system messages". We call the Reporter to *report* an
error/warning/etc., and it decides whether to raise an exception or
not, based on the runtime settings.
An auxiliary method can raise any exception it likes, as long as the
calling code catches it. I don't see the problem.
> So, I implemented that; see the attached patch. After patching,
> system_messages are throwable from within directive implementations.
It looks fine, except for overloading docutils.nodes.system_message as
an Exception subclass. A doctree node is not and should not be an
exception. Smells bad; don't do that. Better to create a new
exception class for this purpose, and include the system_message node
as payload to the exception. Like docutils.utils.SystemMessage (but
don't reuse that, otherwise halt levels won't work).
> However, there's one issue left; the patched directives/body.py,
> class Container reads::
> classes =3D directives.class_option(self.arguments)
> except ValueError:
> raise self.error(
> 'Invalid class attribute value for "%s" directive: "%s".=
> % (self.name, self.arguments))
> This does not exactly look bad, but I'd prefer if class_option()
> could throw that system-message exception itself so that we don't
> have to duplicate the error handling code every time we use the
> class_option function.
You must realize that the container directive is reusing the
directives.class_option function to parse the directive *argument*
(*not* an option!). So yes, a wrapper is necessary, since the context
> At the moment, the class_option function *cannot* throw the system
> message itself because it would need the directive object to
> generate the literal block and the line number. This is bad because
> that information is (logically) not necessary to throw an error.
The class_option function is doing exactly what it should be doing:
validating a value. The use above is not in directive option context,
so it needs different processing. Any exception class_option (or a
wrapper) raises can be caught by the directive handling code in the
parser, which *does* have access to everything needed to build a
system message. I don't see a problem.
> I think this is the result from not separating the Exception class
> and the system_message node (its visual representation). So,
> nodes.system_message perhaps shouldn't be made an Exception;
It should not.
> instead we should create a separate SystemMessage exception,
Call it something else, since docutils.utils.SystemMessage already
exists. Call it docutils.parsers.rst.DirectiveError or similar.
> which contains only the message and the level, and which is caught
> by the directive handler and automatically turned into a
> system_message node (with a literal block added).
> utils.SystemMessage seems like an appropriate name, but it's already
> used for other purposes -- but that's easily changeable.
Don't change utils.SystemMessage; that might cause breakage in client
David Goodger <http://python.net/~goodger>