Re: [lmi] Allow switching skin while lmi is running

[Greg Chicares]

Let me first say that this is a very welcome change that makes life
easier for Kim and me especially, and probably for some of our more
experienced end users too.

On 2016-05-31 19:34, Vadim Zeitlin wrote:
> On Tue, 31 May 2016 01:50:13 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2016-05-30 17:24, Vadim Zeitlin wrote:
> GC> > On Mon, 30 May 2016 14:04:03 +0000 Greg Chicares <address@hidden> wrote:
[...]
> GC> I plan to make a few more small changes. First of all, I'm thinking
> GC> that the fetch_*_names() code can be simpler and more similar in both
> GC> 'ce_*.cpp' files.
> 
>  I did try to make the new version as similar as possible to the existing
> code,

Yes; I noticed that and very much appreciate it. The crucial similarity to
product files is that we don't necessarily distribute every skin (or product)
to every end user, so both classes have to be dynamic--I don't remember
mentioning that, but you got it exactly right.

> but there is a real difference between them: we can't select all .xrc
> files in the skins case, but need to also check their name. Except for
> this, they're pretty similar already I'd say.

Oh yes, of course. They were so similar that the only thing to review was
a very small set of dissimilarities, which led me to remove some complexity
from both, in the commits I just pushed a few minutes ago.

> GC> Showing basenames instead of full filenames in the GUI seems friendlier.

I've convinced myself that the way you implemented it is actually better,
and gave my reasons in a commit recently pushed.

> GC> don't think we really need to distinguish between regexes
> GC>   /skin\.xrc/ and /skin_.*\.xrc/
> GC> because the single regex /skin.*\.xrc/ is good enough.
> 
>  I preferred to be more precise to avoid possible errors and the extra
> complexity of checking for both versions is not big. Initially I also
> wanted to discard the initial and not really significant "skin_" prefix and
> show just the rest of the file name in the GUI, but I discarded this idea
> because I wasn't sure what to do with "skin.xrc" itself (and there was also
> a degenerate case of "skin_.xrc"). But I wanted to mention this idea just
> in case you see any merit in doing it like this.

I also thought of renaming the skin files, using names that might be
even easier for end users to understand; but that would break the
entire universe. In this case, though, a simpler implementation seems
better.

> GC> And I wonder why I ever transformed '.product' filenames to lowercase, 
> when
> GC> we never distribute them with any uppercase letters; the reason is lost in
> GC> the mists of time, but presumably had something to do with DOS 
> compatibility.
> GC> I think it's time to get rid of this.
> 
>  Very well, I don't see any point in doing this neither.

Expunged with pleasure.

> GC> Is there any particular reason for the following implementation?
> GC>   std::string const& default_skin_name()
> GC>   {
> GC>       static std::string const default_name("default");
> GC>       return default_name;
> GC>   }
> 
>  No, it looks pretty useless to me. I'm afraid this was accidentally left
> over from the older version of the patch which used "default" for
> "skin.xrc" (and "foo" for "skin_foo.xrc"). I think leaving "value_" empty
> by default would work just as well, but I didn't test it yet -- should I do
> this?

No. I think I'll just do something like 's/default/skin.xrc'. But I
might want to adapt the 'ce_product_names' logic, and default to
'skin.xrc' if it exists.

A while ago, you proposed some sweeping changes to these 'skin*.xrc'
files, and I thought we adopted them all--but did we miss any? With
the "single premium" skin, I get these messages from wx on the console
(even though they validate with 'xrc.rnc'):
  wxSpinCtrl "SurviveToAge": initial width 20 is too small, at least 34 pixels 
needed.

  wxSpinCtrl "SurviveToYear": initial width 20 is too small, at least 34 pixels 
needed.

Would you mind fixing those apparent errors? And if you can spare the
time...
  "single premium": "Policy" listbox should be expanded vertically
     (there might be one hundred items in this listbox)
  "coli boli": inconsistent borders between "Corp" and "Agent" tabs
  "group carveout" skins: "Names" tab labels misaligned with fields
Those are the glaring UI sins that jump out at me; you may find others.