Re: [lmi] [PATCH] More XRC fixes, including wxDatePickerCtrl ones

[Greg Chicares]

On 2015-04-10 18:26, Vadim Zeitlin wrote:
> 
>  I wanted to wait until the XRC validation patch from
> http://lists.nongnu.org/archive/html/lmi/2015-04/msg00002.html was applied

Now it has been applied (20150417T2049Z, revision 6167).

> before submitting the subsequent ones, but maybe it's better to not wait
> any longer, so here are 8 more XRC-related patches.
[...]
>  Now let me describe the attached patches. As usual, their commit messages,
> included in each patch, provide more details.
> 
> 1. Consistently use wxGROW and not wxEXPAND in XRC files.

Committed 20150417T2124Z, revision 6168.

>    This patch is trivial and just homogenizes the use of these flags.
>    wxGROW was used in the vast majority of cases and so, even though I
>    prefer wxEXPAND personally, I changed the few occurrences of the latter
>    to wxGROW as well.

Different authors had different preferences, and I had to look this up:
  https://groups.google.com/forum/#!topic/wxpython-users/URM9u8xzQ8A
to be sure they're exactly equivalent. Now I don't have to wonder:
concinnity is a Good Thing.

>    This patch doesn't change any behaviour with any
>    version of wxWidgets as wxGROW and wxEXPAND are exactly the same thing.

Doesn't the following excerpt from the patch actually change behavior?

diff --git a/rounding_view.xrc b/rounding_view.xrc
index c8168ee..39bba29 100644
--- a/rounding_view.xrc
+++ b/rounding_view.xrc
@@ -187,7 +187,7 @@
                     </object>
                 </object>
                 <object class="sizeritem">
-                    
<flag>wxALIGN_LEFT|wxALIGN_TOP|wxBOTTOM|wxLEFT|wxRIGHT|wxEXPAND</flag>
+                    
<flag>wxALIGN_LEFT|wxALIGN_TOP|wxBOTTOM|wxLEFT|wxRIGHT</flag>

wxEXPAND was removed altogether--not replaced by wxGROW. But I'm not
going to change anything until all patches have been applied.

> 2. Remove invalid alignment flags from box sizers in XRC files.

Committed 20150417T2143Z, revision 6169.

> 3. Remove "option" element from wxFlexGridSizer items in XRC.

Committed 20150417T2155Z, revision 6170.

> 4. Remove all alignment flags combined with wxGROW from XRC files.

Committed 20150417T2235Z, revision 6171.

>    All these patches don't change anything with the version of wxWidgets
>    currently used by lmi as the XRC elements removed by them were ignored
>    anyhow, but they would be needed to use the resources without errors
>    with the latest wxWidgets version. They also fix many validation errors
>    with the latest version of the XRC schema (see my earlier message at
>    http://lists.nongnu.org/archive/html/lmi/2015-04/msg00002.html for how
>    to validate them on your own).

Now the schema finds only two errors, as I mentioned in an earlier message.

This part of the commentary in the patch gave me pause at first:

| Combining alignment with wxGROW in 2D grid sizers can, on the contrary, be
| useful and is supported by the latest wxWidgets, but it used to be just
| ignored as well, so removing any already present alignment flags ensures that
| the current lmi appearance is exactly preserved even with the latest wxWidgets
| version where it would start behave differently otherwise.

but after inspecting the differences I really think the alignment flags
(removed here) were introduced somewhat haphazardly, and then propagated
widely without another thought.

> 5. Add missing border flags to XRC files.

Committed 20150417T2245Z, revision 6172.

> 6. Fix incorrect uses of wxGROW in XRC.

Committed 20150417T2252Z, revision 6173.

>    [...] So it would be great if you could apply
>    this patch, even though I realize that it requires somewhat of a leap of
>    faith as you probably won't have time to review all of the changes it
>    makes. But, again, all the changes (325 of them AFAIR) were done
>    manually and not by automatic search-and-replace and were tested
>    carefully, so I really think they should be fine.

Okay. Thanks for taking extra care here. As I said in an earlier message,
I found it hard to discern any change resulting from the entire patchset.

> 7. Remove explicit sizes from wxDatePickerCtrl in the resources.

Committed 20150417T2304Z, revision 6174.

>    This is the patch previously posted here which removes the hard coded
>    sizes from wxDatePickerCtrl. It is completely trivial and intentionally
>    does nothing but what it says on the tin, and it does fix the truncation
>    of wxDatePickerCtrl contents. However it also breaks visual alignment of
>    the controls occupying the same column as wxDatePickerCtrl, so you might
>    prefer to combine it with the next patch when applying it, I separated
>    them mostly for ease of reviewing.

Thanks. Another reason to keep these separate is that it makes it easier
to track down any possible regression.

> 8. Fix controls alignment in XRC after wxDatePickerCtrl size change.

Committed 20150417T2315Z, revision 6175.

>    This part finishes the job of the last patch and re-fixes the controls
>    alignment broken by it. As it uses wxGROW|wxALIGN_CENTER_VERTICAL with
>    2D sizer elements, it works best with the latest wxWidgets version, but
>    in practice the difference is unnoticeable under MSW, so you can apply
>    it even if you don't upgrade your wxWidgets version.
> 
>    This patch is not as bad as (6), but I still recommend using word diff
>    for viewing it as it shows that it mostly replaces wxALIGN_LEFT with
>    wxGROW, to ensure that all controls have the same size (with wxGROW they
>    expand to the width of the widest one).

Agreed: this is one situation where GUI tools were helpful.

>  As always, please let me know if you have any questions about these
> patches and thanks in advance for applying them!

And thank you for filtering out a decade's accumulation of recrement.