Re: [lmi] [lmi-commits] master 3b168ba 2/3: Use warning() + alarum() for schema validation failure

[Vadim Zeitlin]

On Thu, 19 Jul 2018 13:07:01 -0400 (EDT) Greg Chicares <address@hidden> wrote:

GC> branch: master
GC> commit 3b168baaeb998c332ba03b0a0c682a62d9b84f85
GC> Author: Gregory W. Chicares <address@hidden>
GC> Commit: Gregory W. Chicares <address@hidden>
GC> 
GC>     Use warning() + alarum() for schema validation failure
GC>     
GC>     Combined the beneficial aspects of two different diagnostic facilities
GC>     for reporting schema validation failures.
GC>     
GC>     warning() uses wxMessageBox(), which does not necessarily work during
GC>     lmi startup or shutdown,

 wxMessageBox() is supposed to always work, including during startup and
shutdown, with the only exception of Unix when using X11 and there is no
available display. Do we have any example of a situation in which it
doesn't work?

GC>     and has been observed to segfault with
GC>     extremely long strings, such as schema validation might emit (see
GC>       https://savannah.nongnu.org/bugs/?20240
GC>     and the code removed 20180213T1240Z by commit e3c3d922).

 I thought this bug was closed because it couldn't be reproduced and so it
seems a bit strange to take it into account. I really don't think it's
acceptable for wxMessageBox() to segfault, for any input length (it might
truncate the string, in the worst case, but still not segfault), so, again,
I'd like to know if we have any way to reproduce this problem (which would
help to fix it)?

GC> diff --git a/multiple_cell_document.cpp b/multiple_cell_document.cpp
GC> index 64e68a5..c6a0bee 100644
GC> --- a/multiple_cell_document.cpp
GC> +++ b/multiple_cell_document.cpp
GC> @@ -448,13 +448,14 @@ void multiple_cell_document::validate_with_xsd_schema
GC>      xml::error_messages errors;
GC>      if(!schema.validate(cell_sorter().apply(xml), errors))
GC>          {
GC> -        alarum()
GC> +        warning()
GC>              << "Validation with schema '"
GC>              << xsd
GC>              << "' failed.\n\n"
GC>              << errors.print()
GC>              << std::flush
GC>              ;
GC> +        alarum() << "Invalid input file." << LMI_FLUSH;
GC>          }
GC>  }

 I think showing 2 message boxes in a row like this is a usability bug and
would very much prefer to avoid it. If the problem with wxMessageBox() can
be reproduced, then it can probably be fixed as well. If it can't, I think
the best would be to use wxRichMessageDialog for warning() and should just
a short excerpt of the message (until the separating blank line?) in the
main part of the dialog and the long text in the part that can be expanded
by the user to see the details. If you hadn't seen this dialog before, I
attach 2 screenshots showing how it looks.

 Please let me know if I can help with really fixing the problem, instead
of using this (IMNSHO pretty ugly) workaround with 2 dialogs.

 Thanks in advance,
VZ