Re: [lmi] Upgrading wx: ellipsis buttons, and wxUSE_STACKWALKER

[Vadim Zeitlin]

On Thu, 9 Jun 2016 10:20:46 +0000 Greg Chicares <address@hidden> wrote:

GC> Using wx with commit c4d06e8117f8930b57bffaf6a3323007c9df8d4b
GC> I find that wx builds but lmi fails here:
GC> 
GC> /lmi/src/lmi/wx_checks.cpp:76:9: error: #error Disable wxUSE_STACKWALKER in 
wx s
GC>  #       error Disable wxUSE_STACKWALKER in wx setup.
GC>          ^

 Oops, of course it will fail when I didn't, for once, test it with the
official build system. Sorry :-(

GC> Here is the error directive in lmi's 'wx_checks.cpp':
GC> 
GC> #if defined __GNUC__ && defined LMI_MSW
GC> // Not yet implemented for gcc on the msw platform.
GC> #   if wxUSE_ON_FATAL_EXCEPTION
GC> #       error Disable wxUSE_ON_FATAL_EXCEPTION in wx setup.
GC> #   endif // wxUSE_ON_FATAL_EXCEPTION
GC> // Not yet implemented for gcc on the msw platform.
GC> #   if wxUSE_STACKWALKER
GC> #       error Disable wxUSE_STACKWALKER in wx setup.
GC> #   endif // wxUSE_STACKWALKER
GC> #endif // defined __GNUC__ && defined LMI_MSW

 Slightly off topic, but I've never really understood the purpose of
these checks. The ones for wxUSE_LIB{JPEG,TIFF,...} above have at least
some explanation (although IMNSHO are still unnecessary, if there were any
problems with using libjpeg or libtiff in GPL programs, we would have
learnt about it already, considering the existence of millions of such
programs), but why should lmi care about wxUSE_ON_FATAL_EXCEPTION and
wxUSE_STACKWALKER and what possible harm can they do? I just don't see it.

GC> I think the compiler message quoted above means that either
GC>   (1) that error directive is no longer needed and should be removed, or

 Yes (but then I think it was never needed).

GC>   (2) wx/configure is mistaken

 No, it isn't. I've implemented support for wxUSE_STACKWALKER for MinGW-w64
in ccac9d05570447204e895da0b1bdc2270ec581c9 because it's a useful class and
particularly so when debugging assert failures in programs built with MinGW
such as lmi.

[skip my ramblings about overriding wxApp::OnAssertFailure() in Skeleton
 with the version with poorer functionality which I probably repeated often
 enough already]

GC> I have two categories of questions:
GC> 
GC> (A) Between the text control and the ellipsis buttons I count one pixel
GC> of spacing for input sequences, and ten for the picker controls. To make
GC> them appear perfectly identical, could we change input sequences to use
GC> the same spacing?

 Sure, this is an even simpler change than the one removing 3 lines of code
in https://github.com/vadz/lmi/pull/41, it's enough to remove 3 characters
this time:
---------------------------------- >8 --------------------------------------
diff --git a/input_sequence_entry.cpp b/input_sequence_entry.cpp
index 91c11df..12d1324 100644
--- a/input_sequence_entry.cpp
+++ b/input_sequence_entry.cpp
@@ -1411,7 +1411,7 @@ InputSequenceEntry::InputSequenceEntry()
     button_ = new(wx) InputSequenceButton(this, wxID_ANY);

     sizer->Add(text_, wxSizerFlags(1).Expand());
-    sizer->Add(button_, wxSizerFlags().Expand().Border(wxLEFT, 1));
+    sizer->Add(button_, wxSizerFlags().Expand().Border(wxLEFT));

     SetSizer(sizer);

---------------------------------- >8 --------------------------------------

 But, as you wrote, lmi doesn't use margins anywhere else, so I'm not sure
if it's such a good idea...

GC> (B1) What's going on with wxUSE_STACKWALKER? I'm not sure we really know
GC> that it works unless we cause lmi to crash. Or does wx/configure test
GC> that so thoroughly that we can safely assume it must work?

 It's supposed to work, but lmi doesn't use it yet, so it's really rather
irrelevant to it. I'd like to use it, either implicitly, by removing the
custom OnAssertFailure() implementation and letting the base class version
show the stack trace in its own dialog, or explicitly, by using it in
Skeleton::OnAssertFailure() or at least SkeletonTest::OnAssertFailure(),
but this hasn't been done nor even discussed yet.

GC> (B2) I didn't observe this compile-time error with exactly the same
GC> compiler version and a four-month-old wx version; has wx/configure's
GC> test been improved so that it now detects a compiler feature that it
GC> previously didn't detect?

 It's not really configure, but the code that was changed to use inline
assembler with MinGW which lacks compiler support for SEH that was used to
implement an important method of this class for MSVC previously (but isn't
any longer because MinGW approach is actually better for MSVC too, and this
allows us to have only a single version of this code). Please see

https://github.com/wxWidgets/wxWidgets/commit/ccac9d05570447204e895da0b1bdc2270ec581c9

for more details. Ah, and this was done in March 2016, so less than 4
months ago.

GC> (B3) If wxUSE_STACKWALKER is now okay, then is wxUSE_ON_FATAL_EXCEPTION
GC> also now okay? I assume the answer must be "no", the reason being that
GC> these are distinct features (and wxUSE_ON_FATAL_EXCEPTION probably needs
GC> SEH, which MinGW-w64 32-bit tools don't provide).

 Yes, wxUSE_ON_FATAL_EXCEPTION is still unsupported with MinGW. But, as I
subtly hinted above, I don't see why should lmi check for it being
disabled, it doesn't care one way or the other, so I'd drop the check for
it too because it's just extra unnecessary lines of code.

 Regards,
VZ