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 _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel