(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

Reply via email to