Toggle dropper shortcut overrides keyboard definitions

[Johncoswell]

The toggle dropper shortcut (mapped to the "d" key) in event-context.cpp is overriding any keyboard verb definitions in use. This toggle call should instead be moved to verbs.cpp and mapped within the keyboard shortcut definitions files.

[Johncoswell (johncoswell)]

If no one's going to complain, I'm going to go ahead and remove this feature. It's really getting on my nerves. I don't have commit access to the main branch, so I'll publish the change on a personal branch and refer to it here.

[ScislaC (scislac)]

Johncoswell,

You have commit access now. I just needed you LP username. :)

[su_v (suv-lp)]

'd' as keyboard shortcut is still hard-coded in current trunk (r12170):
<http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/view/12170/src/event-context.cpp#L589>

[Christoffer Holmstedt (christoffer-holmstedt)]

I have looked at the code and it seems like a bug I can fix. If I understood it correctly it's not just about removing hard-coding of "d" key but also adding it to verbs.cpp. I've found "ContextVerb" for the Dropper Tool already in verbs.cpp so maybe parts of this has been solved before.

[su_v (suv-lp)]

Finally getting around to dig out some additional context (sorry for being late with this):

Christoffer Holmstedt wrote on 2013-04-23:
> (…) I've found "ContextVerb" for the Dropper Tool already in verbs.cpp
> so maybe parts of this has been solved before.

1) Based on the old SVN history still available at sf.net, it appears that the verb for the dropper tool context was already defined in the initial commit for 'src/verbs.c':
<http://inkscape.svn.sourceforge.net/viewvc/inkscape/trunk/inkscape/src/verbs.c?view=log&pathrev=868>
and kept after partial C++ification in rev 869:
<http://inkscape.svn.sourceforge.net/viewvc/inkscape/trunk/inkscape/src/verbs.cpp?view=log&pathrev=10642>
<http://inkscape.svn.sourceforge.net/viewvc/inkscape/inkscape/trunk/src/verbs.cpp?view=log&pathrev=19921>
<http://inkscape.svn.sourceforge.net/viewvc/inkscape/inkscape/trunk/src/verbs.cpp?view=log&pathrev=20227>
<http://inkscape.svn.sourceforge.net/viewvc/inkscape/inkscape/trunk/src/verbs.cpp?view=log&pathrev=20240>
<http://inkscape.svn.sourceforge.net/viewvc/inkscape/inkscape/trunk/src/verbs.cpp?view=log&pathrev=22882>

2) The hard-coded 'D|d' keyboard shortcut appears to originate from this revision:
<http://inkscape.svn.sourceforge.net/viewvc/inkscape?view=revision&revision=16742>
as fix for
- Bug #170590 (sf1123529) “toggle key for dropper”
  <https://bugs.launchpad.net/inkscape/+bug/170590>

The proposed fix in the branch removes the special toggle feature of the dropper tool (toggles between any prior tool and the dropper tool) - the <space> toggle for tool switching still works, but always switches back to the select tool, not to the tool used before switching to the dropper.

<opinion>
Personally, I'm not sure which feature is to rate higher: having a configurable keyboard shortcut, or a special toggle feature for the dropper tool.
</opinion>

[Christoffer Holmstedt (christoffer-holmstedt)]

"<opinion>
Personally, I'm not sure which feature is to rate higher: having a configurable keyboard shortcut, or a special toggle feature for the dropper tool.
</opinion>"

@suv yea, was thinking about that as well. Really hard to tell I personally would like to have a configurable keyboard shortcut.

Is it perhaps possible to implement some kind of verb that remembers what was previously selected?

[su_v (suv-lp)]

Diff from new branch <lp:~christoffer-holmstedt/inkscape/lp180912-remove-hard-coded-toggle-dropper-keybinding> tested successfully on OS X 10.7.5 with Inkscape 0.48+devel r12379 (GTK+/X11 as well as GTK+/Quartz):

- the dropper keyboard shortcut still acts a special toggle between dropper and last used tool
- the toggle feature also works after assigning a custom keyboard shortcut to the dropper tool via 'Preferences > Interface': the newly defined custom keyboard shortcut also acts as toggle between dropper and last used tool.
- the original keyboard shortcut for the dropper tool (mapped to the "d" key) can be assigned to a different command, and is no longer overridden by a hard-coded definition.

[Christoffer Holmstedt (christoffer-holmstedt)]

Commit merged into trunk, revision 12388.

http://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/revision/12388?start_revid=12388