Feature: custom guitool commands can now have custom keyboard shortcuts - #220

@harish2704 sent this commit (90f7b16...7d2bbab) as a patch to the mailing list with submitGit - here on Gmane, MARC

@harish2704 sent this commit (90f7b16...7d2bbab) as a patch to the mailing list with submitGit - here on Gmane, MARC

@harish2704 seems that this patch has stalled in https://marc.info/?l=git&m=145949232928597&w=2...

@dscho : You are right. Do you have any suggestion to make this feature live ?

Do you have any suggestion to make this feature live ?

Well, David seemed to have a few good points:

• this should be documented,
• it should be designed in a way that is compatible (or at least not incompatible) with git-cola.

From looking at https://git-cola.readthedocs.io/en/latest/git-cola.html#configuration-variables, it does not appear to me that git-cola developed anything in that direction yet. And I am not necessarily certain that it would even be possible, given that you use Tk's built-in hotkey parsing and git-cola uses Qt for that (via Python), and they might very possibly have different naming conventions.

In any case, the naming convention of Git GUI's command names (the fullname in your patch) will probably not match how git-cola names things internally.

So I would recommend replying to David with considerations based on what I just wrote, to the extent that it is probably not possible to keep these configurable hotkeys in sync.

Other than that, I would try to make the diff a bit nicer to read, if only to make review easier.

In particular, I think I would rather try to use $path configure so that all of the added code could come after the tools_create_item call.

Also, my preference would be to use the convention guitool.<fullname>.hotkey and guitool.<fullname>.label, as the accelerator and accelerator-label names are not terribly self-explanatory to me.

And yes, it would definitely benefit from a description and at least one example in the documentation, although, admittedly, there seems not to be any good documentation for Git GUI (https://github.com/git/git/blob/v2.22.0/Documentation/git-gui.txt is really too short). But at least the commit message would benefit from such an example.

Speaking of commit message: it should start with a line that has the prefix guitool:, not Feature:. And it needs a good description what motivated the change (see other commits in git.git for inspiration, they are a lot more verbose, and focus a lot more on answering the "why?" than the "what?"). And it needs your sign off.

@harish2704 gentle ping?

@dscho I am sorry for the delayed response.
Yes. we can discuss. My email ID is [email protected].

@harish2704 I am not interested in private discussions about open source projects 😃 Let's keep this conversation in this here PR.

Sure. We can discuss here it self. I will put an eye on my github update emails

I will put an eye on my github update emails

Don't forget to look at the comments that are already there, in particular the comment I left on June 19th.

@harish2704 gentle ping?

@dscho : Hi,
I was following your suggestions . I renamed accelerator into hotkey and tried to document it.

Then , I noticed that, git-cola already has a guitool.<name>.shortcut config variable which serves the same purpose for git-cola tool.
But, there is a problem. the guitool.<name>.shortcut variable should follow the format specified by QKeySequence but, our new hotkey variable is using the format specified by tcl/tk 's bind command

For example, Control-comma in TCL/TK is equivalent to CTRL+, in QKeySequence code

In short, both will conflict.

git-cola is already using a config guitool.<name>.shortcut .
what should we do now? can we proceed with an alternate , non-conflicting config variable name like guitool.<name>.hotkey ?

Will it meet the code quality ? Let me know your thoughts

Will it meet the code quality ? Let me know your thoughts

I don't know whether it will meet the code quality, but you can ask the Git GUI maintainer (we now have an active one again! Yay!). How about mentioning your concern to the Git mailing list ([email protected]) Cc:ing Pratyush Yadav ([email protected])?

Maybe somebody on the list already has a cunning idea how to maybe reuse the QKeySequences and turn them into their Tcl/Tk equivalent programmatically. Then you could reuse the .shortcut name.

@dscho , Yes. will do that.

A quick summary of my proposal is given below

1. I suggest to keep this additional config parameter as an independent config parameter ( which will not interfere with git-cola in any way ), because, both are different applications, and it may have different "built-in" shortcuts already assigned. So, sharing shortcut scheme between two apps is not a good idea.
2. suggested name of new config parameter is guitool.<name>.gitgui_shortcut
3. New parameter will expect shortcut combinations specified in TCL/TK 's format and we will not be doing any processing on it. Will keep it simple

-Will update you once I send the mail

@harish2704 looks sensible to me, perfect as a discussion starter.

@dscho : I had sent a mail as you described , but even after 24 hours, I couldn't find that mail in any of the git mailing list archives. So I think it lost due to some reason.

Now I sent it again in plain text mode ( previous mail was sent in normal mode which is html mode )

Update: It became available in mailing list . Please see https://marc.info/?l=git&m=157011410330286&w=2

Closing this PR since , this has to split into two separate PRs.

1. agains git-gui branch for updating git-gui, which is already made and in progress @ [guitool] feature to specify keyboard shortcuts for custom tools gitgitgadget/git#376 (review)
2. Against main git repo for updating documentation of Documentation/config/guitool.txt which is not yet made.