Re: [lmi] [lmi-commits] master 5bb0b55 2/3: Update statusbar smoothly (VZ)

[Vadim Zeitlin]

On Tue, 26 Jun 2018 18:36:54 -0400 (EDT) Greg Chicares <address@hidden> wrote:

GC> branch: master
GC> commit 5bb0b550b96b1543bbabf43a0c75d167af4756ad
GC> Author: Gregory W. Chicares <address@hidden>
GC> Commit: Gregory W. Chicares <address@hidden>
GC> 
GC>     Update statusbar smoothly (VZ)
GC>     
GC>     See:
GC>       https://lists.nongnu.org/archive/html/lmi/2018-06/msg00025.html
GC>     | it is indeed possible with wxStatusBar -- wxWindow::Update() should
GC>     | be called instead, to update the window appearance immediately.
GC>     
GC>     On the removal of wxSafeYield(), which caused "choppy" updates to the
GC>     statusbar even with an immediate Update() call, see other messages in
GC>     that thread.
GC> ---
GC>  alert_wx.cpp    | 9 +++++++--
GC>  census_view.cpp | 1 -
GC>  2 files changed, 7 insertions(+), 3 deletions(-)
GC> 
GC> diff --git a/alert_wx.cpp b/alert_wx.cpp
GC> index 681239b..f865745 100644
GC> --- a/alert_wx.cpp
GC> +++ b/alert_wx.cpp
GC> @@ -59,9 +59,14 @@ void status_alert(std::string const& s)
GC>      if(wxTheApp && wxTheApp->GetTopWindow())
GC>          {
GC>          wxFrame* f = dynamic_cast<wxFrame*>(wxTheApp->GetTopWindow());
GC> -        if(f && f->GetStatusBar())
GC> +        if(f)
GC>              {
GC> -            f->SetStatusText(s);
GC> +            wxStatusBar* b = f->GetStatusBar();
GC> +            if(b)
GC> +                {
GC> +                f->SetStatusText(s);
GC> +                f->GetStatusBar()->Update();
GC> +                }
GC>              }
GC>          }
GC>  }

 This is very minor, of course, but why call Update() on f->GetStatusBar()
instead of calling it on "b" that we already have? And, if we use "b" on
that line, why not use it on the one above too, for consistency?

 FWIW I'd also put this variable in the scope of the if statement itself,
even if, again, it hardly matters here as the enclosing scope ends just
after it anyhow -- but in general I think it's better hygiene to make the
scope as small as possible, i.e. my version would be

        if(auto b = f->GetStatusBar())
            {
            b->SetStatusText(s);
            b->Update();
            }

 What do you think?
VZ