Re: [lmi] Reporting errors with context (was: "Not enough room for even the first column" messagebox)

[Vadim Zeitlin]

On Sun, 2 Jun 2019 22:33:03 +0000 Greg Chicares <address@hidden> wrote:

[...we agreed on including the current page name in all exceptions thrown
    by pdf_writer_wx...]

GC> My only objection here is to the macro. If adding a pdf-specific macro
GC> here is a good idea, then other sections of lmi should have their own
GC> section-specific macros, and we'll have lots of macros. But I don't
GC> think we want macros to proliferate. Couldn't the extra information be
GC> passed by reusing the existing LMI_ASSERT_WITH_MSG macro, perhaps
GC> enhancing it in a general way to take an optional "CONTEXT" parameter?

 I'm not really sure how is this supposed to work. With the current macro
you can already do something like this:

        LMI_ASSERT_WITH_MSG
                (!save_has_been_called_
                ,"while generating the page \"" << page_name_ << "\""
                );

i.e. it can already take any number of parameters using the stream
insertion syntax and adding another parameter doesn't seem very useful,
unless I'm missing something.

 There are 2 problems with doing something like the above:

1. The "while generating the page" fragment needs to be repeated in
   each and every assert. We could add some get_context() member to
   return it, of course, but this makes the assert message less explicit.

2. Asserts from any code not directly in pdf_writer_wx.cpp still wouldn't
   include the page name (I don't think there is any such code currently,
   but I could be wrong or such code could be added in the future, even if
   I'm right).

 The straightforward way of addressing both of these concerns at once would
be by adding "the current context", i.e. some global (or a global by any
other name, i.e. a singleton, a static member variable or whatever)
containing the extra information to be used by any LMI_ASSERT() invocation.
This could be then actually done even outside of pdf_writer_wx, in
pdf_command_wx.cpp itself and in a single place inside render_all() only.

 So it looks very appealing but, as usual, convenience comes at a price as
using such global state has plenty of known bad effects too. I'm tempted to
say that as long as it's used only for error reporting, it's not too bad,
but I don't know if you share this optimistic assessment or prefer to be
more explicit and use LMI_ASSERT_WITH_MSG() directly?

 Please let me know what do you think about this, thanks in advance!
VZ

pgpizjtFO_Joa.pgp
Description: PGP signature