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

[Greg Chicares]

On 2016-06-09 14:04, Vadim Zeitlin wrote:
> On Thu, 9 Jun 2016 10:20:46 +0000 Greg Chicares <address@hidden> wrote:
[...]
> 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),

I chose those settings a decade or two ago. They reflected RMS Thought
in that era:
  // Graphics format with potential patent issues.
and I haven't bothered to reconsider because lmi has no use for those
formats anyway. I guess the GIF patents have expired, so that GIF is
now acceptable in free software, but I have been using PNG for many
years and have no reason to spend time on other formats, even to
un-forbid them. I wouldn't change this without satisfying myself that
these formats have now achieved FSF approbation anyway, no matter how
many GPL programs use them. Many GPL programs supported GIF when it
definitely was patent-encumbered.

> but why should lmi care about wxUSE_ON_FATAL_EXCEPTION and
> wxUSE_STACKWALKER

Nowadays we use 'configure', but when I started using wx, I was using
the makefiles it used to provide. I took the list of available options,
chose the ones we needed, and forbade anything that seemed unnecessary
or harmful. Ten or twenty years ago, it seemed to me, from reading the
wx documentation, that these two ought to be forbidden. In those days
we were manually editing 'config.hpp' (if I remember the file name
correctly), and such assertions were a useful safeguard against
flubbing the manual configuration.

> and what possible harm can they do? I just don't see it.

Neither of us sees it, but our reasons differ. You don't see it because
you know exactly what they do, and know that no harm can ensue. I don't
see it because I know only that 'configure' used to disallow them, so I
assumed they were dangerous, and why would I step into unknown danger?
Here, unlike in the patent-encumbered graphics case, I'll defer to your
special expertise and remove them.

> 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]

I believe I have a robust workaround for perceived shortcomings of
wx-2.5.1 built with gcc-2.95 . Maybe that's no longer needed, but I'm
not going to remove carefully written and tested code without new
careful testing, which would take time. But see below.

> 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...

Thanks, I'll try that change. BTW, it's not that lmi never uses any
margins anywhere, but rather that lmi does not use vertical margins
in many cases quite similar to this.

> 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.

So we'd get a stack trace if lmi crashes? That might be worth the
cost of testing it.