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 ]".
> 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.
> 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
Thanks for the detailed review.
Benno
--
http://www.fastmail.fm - Or how I learned to stop worrying and
love email again
From 771f97d07444dd36ee4c9431d785a040c28cf81a Mon Sep 17 00:00:00 2001
From: Benno Schulenberg <[email protected]>
Date: Thu, 11 Sep 2014 22:04:21 +0200
Subject: [PATCH] When overriding a key, adjust also its number of levels (#57242).
Specifying an explicit key type when overriding a key should adjust
the number of levels to that of the specified type. This gets rid of
the age-old warning of the right Alt key being ONE_LEVEL but having
two symbols assigned.
Fixes bug #57242 <http://bugs.freedesktop.org/show_bug.cgi?id=57242>.
Signed-off-by: Benno Schulenberg <[email protected]>
Reviewed-by: Ran Benita <[email protected]>
---
symbols.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/symbols.c b/symbols.c
index d43ba9f..9313d3c 100644
--- a/symbols.c
+++ b/symbols.c
@@ -339,18 +339,19 @@ MergeKeyGroups(SymbolsInfo * info,
clobber = (from->defs.merge != MergeAugment);
report = (warningLevel > 9) ||
((into->defs.fileID == from->defs.fileID) && (warningLevel > 0));
- if (into->numLevels[group] >= from->numLevels[group])
- {
- resultSyms = into->syms[group];
- resultActs = into->acts[group];
- resultWidth = into->numLevels[group];
- }
- else
+ if ((from->numLevels[group] > into->numLevels[group])
+ || (clobber && (from->types[group] != None)))
{
resultSyms = from->syms[group];
resultActs = from->acts[group];
resultWidth = from->numLevels[group];
}
+ else
+ {
+ resultSyms = into->syms[group];
+ resultActs = into->acts[group];
+ resultWidth = into->numLevels[group];
+ }
if (resultSyms == NULL)
{
resultSyms = uTypedCalloc(resultWidth, KeySym);
--
1.7.0.4
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
