Re: [lmi] Wrong wxFlexGridSizer usage in about_dialog.cpp

[Greg Chicares]

On 2010-01-15 16:25Z, Vadim Zeitlin wrote:
> 
> These lines should have been written as
> 
>       sizer_btn->Add(license_button, 0, wxALL, 2);
>       sizer_btn->Add(cancel_button,  0, wxALL, 2);

That's why we do code review. I had already tentatively rewritten them
with 1 instead of 0 as the second argument; but 0 does seem preferable
because the sizer isn't growable.

(Actually, I can't claim that I discovered the anomaly in code review.
I built the code and tested it first, and noticed that there was no
border around the two pushbuttons on the "Help | About" dialog.)

And, after delving into the wx code and documentation, I was about to
ask whether we wouldn't be better off using the typesafe alternative:

> The fact that compiler doesn't catch this kind of bugs is one of the
> reasons (the other one being that I also find it more clear to read) that I
> introduced wxSizerFlags class in wx API several years ago and practically
> never use other overloads of wxSizer::Add() any more.
> 
>  With wxSizerFlags, you'd write
> 
>       sizer_btn->Add(license_button, wxSizerFlags().Border());
>       sizer_btn->Add(cancel_button, wxSizerFlags().Border());
> 
> (well, this would use border of standard size instead of 2 pixels but you
> could use Border(wxALL, 2) as well, of course, it's just that I think that
> using borders of consistent sizes everywhere is better anyhow).

Consistency is better. In revision 4736 of 20100114T1755Z, I made
border values consistent, and I happened to choose the same value
(two) that you selected as a default--so maybe I can rewrite that
now with wxSizerFlags().Border()'s default arguments.

Or would you rather do that and send a fresh omnibus patch? I plan
to commit these changes in pieces small enough for me to understand,
so do whatever will be less work for you.