From: Teruaki Kawashima <kamekun_at_r6.dion.ne.jp>
Date: Tue, 30 Nov 2010 20:42:54 +0900

(2010/11/30 1:09), Marianne Arnold wrote:
> Am 29.11.2010 14:11, schrieb Jeff Goode:
>> Good luck with that. Have you tested this change on all these targets?
>
> I'm also curious (and admittedly doubtful) about this commit. The
> keymaps are a bit of a fragile construct, especially when one context
> links to another like it is the case here (which on the other hands
> saves duplication). I am afraid that this change broke something on at
> least some targets and I'm actually not too keen on trying.
>
> There is a possibility that the action with release event was used in
> the list context and in the linked one(s) - before, the list context one
> would take precedence over the other in lists, now it is possible that
> the list context one (without the release event) hits first and then the
> other when the button is released, maybe getting you into trouble.
>
> At least this change should have been put on flyspray first with a call
> for testers and probably a discussion on the mailing list before. I am
> also interested to know what the change should achieve as the commit
> message doesn't tell much. Could you explain?
>
> Kind regards, Marianne

Let me explain about the reason of commit first.
BUTTON_XXX and BUTTON_XXX|BUTTON_REL is mapped for same ACTION_XXX,
ACTION_XXX is generated twice with single "press then release" action.
It annoyed me and I made up my mind to change this.

Below is part of change to keymap-gigabeat.c (ex.1).
> { ACTION_TREE_PGLEFT, BUTTON_LEFT|BUTTON_REPEAT,
BUTTON_NONE },
> - { ACTION_TREE_PGLEFT, BUTTON_LEFT|BUTTON_REL,
BUTTON_LEFT|BUTTON_REPEAT },
ACTION_TREE_PGLEFT scrolls liens (or screen) moves horizontally.
As you can see, both long left and releasing left after long left
generate ACTION_TREE_PGLEFT.
So, it always moves lines by at least 2 units.

Below is another part of change to keymap-gigabeat.c (ex.2).
> - { ACTION_LISTTREE_PGUP, BUTTON_A|BUTTON_UP,
  BUTTON_A },
> - { ACTION_LISTTREE_PGUP, BUTTON_UP|BUTTON_REL,
  BUTTON_A|BUTTON_UP },
> + { ACTION_LISTTREE_PGUP, BUTTON_A|BUTTON_UP,
  BUTTON_NONE },
> { ACTION_LISTTREE_PGUP, BUTTON_A|BUTTON_UP|BUTTON_REPEAT,
  BUTTON_NONE },
ACTION_LISTTREE_PGUP moves selection by one screen up.
In the previous code, selection can move by two screen depending on the
timing.

Changes to other keymaps are almost same except name of the buttons.
e.g. BUTTON_ON is used in keymap-recorder.c where BUTTON_A is used in
keymap-gigabeat.c.


What is concerned about is the change broke something, right?
I didn't test all related targets, but i think it is unlikely to broke
something.
basically, the case which breaks thing is, button conbination is removed
or added, and same conbination is defined in the linked one(s). This can
be checked manually. for example, in (ex.2), BUTTON_UP|BUTTON_REL is
removed from the list context, so search for BUTTON_UP|BUTTON_REL in
keymap-gigabeat.c, and check if it's context is linked to the list
context and affect the behaviour.

I re-checked all keymaps and all changes except in keymap-iaudio67.c
doesn't affect other mapping.

note:
In (ex.2), previous button for ACTION_LISTTREE_PGUP is changed to
BUTTON_NONE from BUTTON_A, so now pressing BUTTON_UP then BUTTON_A does
generate ACTION_LISTTREE_PGUP as well as pressing BUTTON_A then
BUTTON_UP. I don't think this is a problem.


Best regards,
teru
Received on 2010-11-30