[patch] xkbcomp: fixing an age-old warning
Ran Benita
ran234 at gmail.com
Sat Sep 13 13:13:25 PDT 2014
On Thu, Sep 11, 2014 at 10:18:52PM +0200, Benno Schulenberg wrote:
> On Thu, Sep 11, 2014, at 13:01, Ran Benita wrote:
> > Actually I'm not so sure. The current behavior of a key-group override
> > is per-symbol, e.g.
> >
> > override key <FOO> {
> > [ NoSymbol, B, C ];
> > };
> >
> > Means: replace whatever is in the 2nd and 3rd levels with the symbols B
> > and C (create them if they do not exist). "override" is almost always
> > the default. Often, the key type for both `from` and `into` is not
> > specified, so an automatic type is assigned (FindAutomaticType) at a
> > later point.
> >
> > Now, if we have this:
> >
> > key <FOO> {
> > [1, 2, 3, 4];
> > }
> > [...]
> > override key <FOO> {
> > [ NoSymbol, B, C ];
> > };
> >
> > With current code you get: [1, B, C, 4], with your patch: [1, B, C].
> > I think the current behavior is a bit more intuitive,
>
> Hmm. I wouldn't call that more intuitive, I would call it plain wrong. :)
> If I wanted an overridden key to retain a possible fourth symbol, I would
> write "[ any, B, C, any ]" instead of "[ any, B, C ]".
If you want to outright replace the key, you should use Replace (see the
beginning of MergeKeys() - haven't tried it though). Override in this
case means to do the merge per-level. So if I put [ NoSymbol, B, C ] it
shouldn't touch the other levels IMO.
But if you override the *type* then it obviously affects the levels.
> > and we should not
> > change it. [...]
>
> Fair enough... let's assume that some definitions depend on the current
> behaviour.
>
> > So if you want to get rid of the warning, I'd suggest either:
> >
> > 1. Change the test to
> >
> > if ((from->numLevels[group] > into->numLevels[group])
> > || (override && from->types[group] != None))
>
> Implemented in the revised patch, attached.
Looks good.
> > 2. Just make the warning require a verbosity>0. It is not necessarily
> > a problem if the truncation kicks in, so it should not be displayed
> > to the user by default. However a keymap author would probably like
> > to see it.
>
> I would like to propose that solution for this warning:
> Warning: Symbol map for key <RALT> redefined
> Using last definition for conflicting fields
If you send a patch I will review it :) (but you will have to get
someone to apply it).
Ran
> Thanks for the detailed review.
>
> Benno
More information about the xorg-devel
mailing list