* [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts @ 2016-03-29 11:38 Harish K 2016-03-31 16:41 ` David Aguilar 0 siblings, 1 reply; 29+ messages in thread From: Harish K @ 2016-03-29 11:38 UTC (permalink / raw) To: git --- git-gui/lib/tools.tcl | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl index 6ec9411..749bc67 100644 --- a/git-gui/lib/tools.tcl +++ b/git-gui/lib/tools.tcl @@ -38,7 +38,7 @@ proc tools_create_item {parent args} { } proc tools_populate_one {fullname} { - global tools_menubar tools_menutbl tools_id + global tools_menubar tools_menutbl tools_id repo_config if {![info exists tools_id]} { set tools_id 0 @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} { } } - tools_create_item $parent command \ + if {[info exists repo_config(guitool.$fullname.accelerator)] && [info exists repo_config(guitool.$fullname.accelerator-label)]} { + set accele_key $repo_config(guitool.$fullname.accelerator) + set accel_label $repo_config(guitool.$fullname.accelerator-label) + tools_create_item $parent command \ -label [lindex $names end] \ - -command [list tools_exec $fullname] + -command [list tools_exec $fullname] \ + -accelerator $accel_label + bind . $accele_key [list tools_exec $fullname] + } else { + tools_create_item $parent command \ + -label [lindex $names end] \ + -command [list tools_exec $fullname] + } } proc tools_exec {fullname} { -- https://github.com/git/git/pull/220 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2016-03-29 11:38 [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts Harish K @ 2016-03-31 16:41 ` David Aguilar 2016-04-01 6:32 ` harish k 0 siblings, 1 reply; 29+ messages in thread From: David Aguilar @ 2016-03-31 16:41 UTC (permalink / raw) To: Harish K; +Cc: git Hello, On Tue, Mar 29, 2016 at 11:38:10AM +0000, Harish K wrote: > --- > git-gui/lib/tools.tcl | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl > index 6ec9411..749bc67 100644 > --- a/git-gui/lib/tools.tcl > +++ b/git-gui/lib/tools.tcl > @@ -38,7 +38,7 @@ proc tools_create_item {parent args} { > } > > proc tools_populate_one {fullname} { > - global tools_menubar tools_menutbl tools_id > + global tools_menubar tools_menutbl tools_id repo_config > > if {![info exists tools_id]} { > set tools_id 0 > @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} { > } > } > > - tools_create_item $parent command \ > + if {[info exists repo_config(guitool.$fullname.accelerator)] && [info exists repo_config(guitool.$fullname.accelerator-label)]} { > + set accele_key $repo_config(guitool.$fullname.accelerator) > + set accel_label $repo_config(guitool.$fullname.accelerator-label) > + tools_create_item $parent command \ > -label [lindex $names end] \ > - -command [list tools_exec $fullname] > + -command [list tools_exec $fullname] \ > + -accelerator $accel_label > + bind . $accele_key [list tools_exec $fullname] > + } else { > + tools_create_item $parent command \ > + -label [lindex $names end] \ > + -command [list tools_exec $fullname] > + } > } > > proc tools_exec {fullname} { > > -- > https://github.com/git/git/pull/220 We also support "custom guitools" in git-cola using this same mechanism. If this gets accepted then we'll want to make similar change there. There's always a small risk that user-defined tools can conflict with builtin shortcuts, but otherwise this seems like a pretty nice feature. Curious, what is the behavior in the event of a conflict? Do the builtins win? IIRC, Qt handles this by disabling the shortcut and warning that it's ambiguous. Please documentation guitool.<name>.accellerator[-label] in Documentation/config.txt otherwise users will not know that it exists. It would also be good for the docs to clarify what the accelerators look like in case we need to munge them when making it work in cola via Qt, which has its own mechanism for associating actions with shortcuts. Documented examples with one and two modifier keys would be helpful. cheers, -- David ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2016-03-31 16:41 ` David Aguilar @ 2016-04-01 6:32 ` harish k 2019-10-03 14:48 ` harish k 0 siblings, 1 reply; 29+ messages in thread From: harish k @ 2016-04-01 6:32 UTC (permalink / raw) To: David Aguilar; +Cc: git Hi David, Actually Im a TCL primer. This is the first time Im dealing with. That is why I kept it simple ( ie both accel-key and accel-label need to be defined in config ). I think, git-cola is using Qt style representation accel-key in the config file. Is git-cola is an official tool from git ? like git-gui? if not , My suggesion is, it is better to seperate config fields according to application-domain like, "git-gui-accel = <Ctrl-l>" etc.. Other vise there is good chance for conflicts. ( Eg: consider the case that, <Ctrl-p> was assined to a custom tool by git-cola ) Currently this patch will not handle any conflicting shortcuts. I think custom shortcuts will overwrite the other. On Thu, Mar 31, 2016 at 10:11 PM, David Aguilar <davvid@gmail.com> wrote: > Hello, > > On Tue, Mar 29, 2016 at 11:38:10AM +0000, Harish K wrote: >> --- >> git-gui/lib/tools.tcl | 16 +++++++++++++--- >> 1 file changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl >> index 6ec9411..749bc67 100644 >> --- a/git-gui/lib/tools.tcl >> +++ b/git-gui/lib/tools.tcl >> @@ -38,7 +38,7 @@ proc tools_create_item {parent args} { >> } >> >> proc tools_populate_one {fullname} { >> - global tools_menubar tools_menutbl tools_id >> + global tools_menubar tools_menutbl tools_id repo_config >> >> if {![info exists tools_id]} { >> set tools_id 0 >> @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} { >> } >> } >> >> - tools_create_item $parent command \ >> + if {[info exists repo_config(guitool.$fullname.accelerator)] && [info exists repo_config(guitool.$fullname.accelerator-label)]} { >> + set accele_key $repo_config(guitool.$fullname.accelerator) >> + set accel_label $repo_config(guitool.$fullname.accelerator-label) >> + tools_create_item $parent command \ >> -label [lindex $names end] \ >> - -command [list tools_exec $fullname] >> + -command [list tools_exec $fullname] \ >> + -accelerator $accel_label >> + bind . $accele_key [list tools_exec $fullname] >> + } else { >> + tools_create_item $parent command \ >> + -label [lindex $names end] \ >> + -command [list tools_exec $fullname] >> + } >> } >> >> proc tools_exec {fullname} { >> >> -- >> https://github.com/git/git/pull/220 > > We also support "custom guitools" in git-cola using this same > mechanism. If this gets accepted then we'll want to make > similar change there. > > There's always a small risk that user-defined tools can conflict > with builtin shortcuts, but otherwise this seems like a pretty > nice feature. Curious, what is the behavior in the event of a > conflict? Do the builtins win? IIRC, Qt handles this by > disabling the shortcut and warning that it's ambiguous. > > Please documentation guitool.<name>.accellerator[-label] in > Documentation/config.txt otherwise users will not know that it > exists. > > It would also be good for the docs to clarify what the > accelerators look like in case we need to munge them when making > it work in cola via Qt, which has its own mechanism for > associating actions with shortcuts. Documented examples with > one and two modifier keys would be helpful. > > > cheers, > -- > David -- -Regards Harish.K ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2016-04-01 6:32 ` harish k @ 2019-10-03 14:48 ` harish k 2019-10-03 21:44 ` Pratyush Yadav 0 siblings, 1 reply; 29+ messages in thread From: harish k @ 2019-10-03 14:48 UTC (permalink / raw) To: git; +Cc: David Aguilar, Pratyush Yadav Hi All, I', Just reopening this feature request. A quick summary of my proposal is given below. 1. This PR will allow an additional configuration option "guitool.<name>.gitgui-shortcut" which will allow us to specify keyboard shortcut for custom commands in git-gui 2. Even there exists a parameter called "guitool.<name>.shortcut" which is used by git-cola, I suggest to keep this new 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. 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. --- Documentation/config/guitool.txt | 15 +++++++++++++++ git-gui/lib/tools.tcl | 15 ++++++++++++--- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/Documentation/config/guitool.txt b/Documentation/config/guitool.txt index 43fb9466ff..79dac23ca3 100644 --- a/Documentation/config/guitool.txt +++ b/Documentation/config/guitool.txt @@ -48,3 +48,18 @@ guitool.<name>.prompt:: Specifies the general prompt string to display at the top of the dialog, before subsections for 'argPrompt' and 'revPrompt'. The default value includes the actual command. + +guitool.<name>.gitgui-shortcut + Specifies a keyboard shortcut for the custom tool in the git-gui + application. The value must be a valid string ( without "<" , ">" wrapper ) + understood by the TCL/TK 's bind command.See https://www.tcl.tk/man/tcl8.4/TkCmd/bind.htm + for more details about the supported values. Avoid creating shortcuts that + conflict with existing built-in `git gui` shortcuts. + Example: + [guitool "Terminal"] + cmd = gnome-terminal -e zsh + noconsole = yes + gitgui-shortcut = "Control-y" + [guitool "Sync"] + cmd = "git pull; git push" + gitgui-shortcut = "Alt-s" diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl index 413f1a1700..40db3f6395 100644 --- a/git-gui/lib/tools.tcl +++ b/git-gui/lib/tools.tcl @@ -38,7 +38,7 @@ proc tools_create_item {parent args} { } proc tools_populate_one {fullname} { - global tools_menubar tools_menutbl tools_id + global tools_menubar tools_menutbl tools_id repo_config if {![info exists tools_id]} { set tools_id 0 @@ -61,9 +61,18 @@ proc tools_populate_one {fullname} { } } - tools_create_item $parent command \ + if {[info exists repo_config(guitool.$fullname.gitgui-shortcut)]} { + set gitgui_shortcut $repo_config(guitool.$fullname.gitgui-shortcut) + tools_create_item $parent command \ -label [lindex $names end] \ - -command [list tools_exec $fullname] + -command [list tools_exec $fullname] \ + -accelerator $gitgui_shortcut + bind . <$gitgui_shortcut> [list tools_exec $fullname] + } else { + tools_create_item $parent command \ + -label [lindex $names end] \ + -command [list tools_exec $fullname] + } } proc tools_exec {fullname} { -- https://github.com/git/git/pull/220 -- On Fri, Apr 1, 2016 at 12:02 PM harish k <harish2704@gmail.com> wrote: > > Hi David, > > Actually Im a TCL primer. This is the first time Im dealing with. > That is why I kept it simple ( ie both accel-key and accel-label need > to be defined in config ). > > I think, git-cola is using Qt style representation accel-key in the config file. > > Is git-cola is an official tool from git ? like git-gui? > if not , > My suggesion is, it is better to seperate config fields according to > application-domain > like, "git-gui-accel = <Ctrl-l>" etc.. > Other vise there is good chance for conflicts. ( Eg: consider the case > that, <Ctrl-p> was assined to a custom tool by git-cola ) > > Currently this patch will not handle any conflicting shortcuts. I > think custom shortcuts will overwrite the other. > > > On Thu, Mar 31, 2016 at 10:11 PM, David Aguilar <davvid@gmail.com> wrote: > > Hello, > > > > On Tue, Mar 29, 2016 at 11:38:10AM +0000, Harish K wrote: > >> --- > >> git-gui/lib/tools.tcl | 16 +++++++++++++--- > >> 1 file changed, 13 insertions(+), 3 deletions(-) > >> > >> diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl > >> index 6ec9411..749bc67 100644 > >> --- a/git-gui/lib/tools.tcl > >> +++ b/git-gui/lib/tools.tcl > >> @@ -38,7 +38,7 @@ proc tools_create_item {parent args} { > >> } > >> > >> proc tools_populate_one {fullname} { > >> - global tools_menubar tools_menutbl tools_id > >> + global tools_menubar tools_menutbl tools_id repo_config > >> > >> if {![info exists tools_id]} { > >> set tools_id 0 > >> @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} { > >> } > >> } > >> > >> - tools_create_item $parent command \ > >> + if {[info exists repo_config(guitool.$fullname.accelerator)] && [info exists repo_config(guitool.$fullname.accelerator-label)]} { > >> + set accele_key $repo_config(guitool.$fullname.accelerator) > >> + set accel_label $repo_config(guitool.$fullname.accelerator-label) > >> + tools_create_item $parent command \ > >> -label [lindex $names end] \ > >> - -command [list tools_exec $fullname] > >> + -command [list tools_exec $fullname] \ > >> + -accelerator $accel_label > >> + bind . $accele_key [list tools_exec $fullname] > >> + } else { > >> + tools_create_item $parent command \ > >> + -label [lindex $names end] \ > >> + -command [list tools_exec $fullname] > >> + } > >> } > >> > >> proc tools_exec {fullname} { > >> > >> -- > >> https://github.com/git/git/pull/220 > > > > We also support "custom guitools" in git-cola using this same > > mechanism. If this gets accepted then we'll want to make > > similar change there. > > > > There's always a small risk that user-defined tools can conflict > > with builtin shortcuts, but otherwise this seems like a pretty > > nice feature. Curious, what is the behavior in the event of a > > conflict? Do the builtins win? IIRC, Qt handles this by > > disabling the shortcut and warning that it's ambiguous. > > > > Please documentation guitool.<name>.accellerator[-label] in > > Documentation/config.txt otherwise users will not know that it > > exists. > > > > It would also be good for the docs to clarify what the > > accelerators look like in case we need to munge them when making > > it work in cola via Qt, which has its own mechanism for > > associating actions with shortcuts. Documented examples with > > one and two modifier keys would be helpful. > > > > > > cheers, > > -- > > David > > > > -- > > -Regards > Harish.K -- -Thanks Harish.K ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-03 14:48 ` harish k @ 2019-10-03 21:44 ` Pratyush Yadav 2019-10-04 8:49 ` Johannes Schindelin 0 siblings, 1 reply; 29+ messages in thread From: Pratyush Yadav @ 2019-10-03 21:44 UTC (permalink / raw) To: harish k; +Cc: git, David Aguilar, Johannes Schindelin Hi Harish, Thanks for the patch. Unfortunately, it seems your mail client messed up the formatting, and the patch won't apply. I'm guessing it is because your mail client broke long lines into two, messing up the diff. We use an email-based workflow, so please either configure your mail client so it doesn't munge patches, or use `git send-email`. You can find a pretty good tutorial on sending patches via email over at [0]. The tutorial is for git.git, but works for git-gui.git as well. If you feel more comfortable with GitHub pull requests, please take a look at Gitgitgadget [1]. Johannes (in Cc) has used it recently to send patches based on the git-gui repo (AFAIK, it was originally designed only for the git.git repo). Maybe ask the folks over there how they do it. One more thing: your patch is based on the main Git repo. That repo is not where git-gui development takes place. The current "official" repo for git-gui is over at [2]. Please base your patches on top of that repo. [0] https://matheustavares.gitlab.io/posts/first-steps-contributing-to-git#submitting-patches [1] https://gitgitgadget.github.io/ [2] https://github.com/prati0100/git-gui Now on to the nitty gritty details. I like the idea. In fact, there were some discussions recently about having configurable key bindings for _all_ shortcuts in git-gui. Nothing concrete has been done in that direction yet though. But I feel like this is a pretty good first step. On 03/10/19 08:18PM, harish k wrote: > Hi All, > I', Just reopening this feature request. > A quick summary of my proposal is given below. > > 1. This PR will allow an additional configuration option > "guitool.<name>.gitgui-shortcut" which will allow us to specify > keyboard shortcut for custom commands in git-gui A pretty nice way of doing it. But I would _really_ like it if there was an option in the "create tool" dialog to specify the shortcut. People of a gui tool shouldn't have to mess around with config files as much as possible. > 2. Even there exists a parameter called "guitool.<name>.shortcut" > which is used by git-cola, I suggest to keep this new 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. David has advocated inter-operability between git-gui and git-cola. While I personally don't know how many people actually use both the tools at the same time, it doesn't sound like a bad idea either. So, sharing shortcuts with git-cola would be nice. Of course, it would then mean that we would have to parse the config parameter before feeding them to `bind`. I don't suppose that should be something too complicated to do, but I admit I haven't looked too deeply into it. I'd like to hear what other people think about whether it is worth the effort to inter-operate with git-cola. > 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. Are you sure that is a good idea? I think we should at least make sure we are not binding some illegal sequence, and if we are, we should warn the user about it. And a much more important case would be when a user over-writes a pre-existing shortcut for other commands like "commit", "reset", etc. In that case, the menu entires of those commands would still be labelled with the shortcut, but it won't actually work. Yes, your current implementation keeps things simple, but I think some light processing would be beneficial. And if we do decide to go the inter-operability with git-cola route, then processing would be needed anyway, and we can validate there. > --- > Documentation/config/guitool.txt | 15 +++++++++++++++ > git-gui/lib/tools.tcl | 15 ++++++++++++--- > 2 files changed, 27 insertions(+), 3 deletions(-) > > diff --git a/Documentation/config/guitool.txt b/Documentation/config/guitool.txt > index 43fb9466ff..79dac23ca3 100644 > --- a/Documentation/config/guitool.txt > +++ b/Documentation/config/guitool.txt > @@ -48,3 +48,18 @@ guitool.<name>.prompt:: > Specifies the general prompt string to display at the top of > the dialog, before subsections for 'argPrompt' and 'revPrompt'. > The default value includes the actual command. > + > +guitool.<name>.gitgui-shortcut > + Specifies a keyboard shortcut for the custom tool in the git-gui > + application. The value must be a valid string ( without "<" , ">" wrapper ) > + understood by the TCL/TK 's bind command.See > https://www.tcl.tk/man/tcl8.4/TkCmd/bind.htm > + for more details about the supported values. Avoid creating shortcuts that > + conflict with existing built-in `git gui` shortcuts. > + Example: > + [guitool "Terminal"] > + cmd = gnome-terminal -e zsh > + noconsole = yes > + gitgui-shortcut = "Control-y" > + [guitool "Sync"] > + cmd = "git pull; git push" > + gitgui-shortcut = "Alt-s" The "Documentation/" subdirectory belongs to the Git project, and not to git-gui, so if you want to see this change, you'd have to submit a separate patch for it. As far as git-gui's documentation is concerned, unfortunately there is none yet. I have been meaning to start working towards it, but just haven't found the time or motivation to do it yet. > diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl Like I mentioned before, please base your patches on the git-gui.git repo, and not git.git. So, this should read "a/lib/tools.tcl" instead of "a/git-gui/lib/tools.tcl". I haven't looked at the contents of the patch because I can't apply it, and I'd prefer to tinker around with it before commenting. So please re-send the patch in the proper format and we can discuss the implementation :). > index 413f1a1700..40db3f6395 100644 > --- a/git-gui/lib/tools.tcl > +++ b/git-gui/lib/tools.tcl [snip] -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-03 21:44 ` Pratyush Yadav @ 2019-10-04 8:49 ` Johannes Schindelin 2019-10-04 12:01 ` Pratyush Yadav 0 siblings, 1 reply; 29+ messages in thread From: Johannes Schindelin @ 2019-10-04 8:49 UTC (permalink / raw) To: Pratyush Yadav; +Cc: harish k, git, David Aguilar Hi Pratyush, please don't top-post on this list (yet another of these things requiring extra brain cycles in the mailing list workflow). On Fri, 4 Oct 2019, Pratyush Yadav wrote: > Hi Harish, > > Thanks for the patch. Unfortunately, it seems your mail client messed up > the formatting, and the patch won't apply. I'm guessing it is because > your mail client broke long lines into two, messing up the diff. > > We use an email-based workflow, so please either configure your mail > client so it doesn't munge patches, or use `git send-email`. You can > find a pretty good tutorial on sending patches via email over at [0]. > The tutorial is for git.git, but works for git-gui.git as well. Ah, well. Mailing list-based workflows are so easy, amirite? They are so welcoming and inclusive, yes? </sarcasm> > If you feel more comfortable with GitHub pull requests, please take a > look at Gitgitgadget [1]. Johannes (in Cc) has used it recently to send > patches based on the git-gui repo (AFAIK, it was originally designed > only for the git.git repo). Maybe ask the folks over there how they do > it. Harish, it is actually relatively easy to use GitGitGadget: just add a remote like this: git remote add gitgitgadget https://github.com/gitgitgadget/git git fetch gitgitgadget git-gui/master and then rebase your patch on top of that branch: git rebase -i --onto git-gui/master HEAD~1 Then force-push your branch to your GitHub fork of git.git and open a Pull Request at https://github.com/gitgitgadget/git/pulls, targeting git-gui/master. GitGitGadget will welcome you with a (hopefully) helpful message ;-) Ciao, Johannes > > One more thing: your patch is based on the main Git repo. That repo is > not where git-gui development takes place. The current "official" repo > for git-gui is over at [2]. Please base your patches on top of that > repo. > > [0] https://matheustavares.gitlab.io/posts/first-steps-contributing-to-git#submitting-patches > [1] https://gitgitgadget.github.io/ > [2] https://github.com/prati0100/git-gui > > Now on to the nitty gritty details. > > I like the idea. In fact, there were some discussions recently about > having configurable key bindings for _all_ shortcuts in git-gui. Nothing > concrete has been done in that direction yet though. But I feel like > this is a pretty good first step. > > On 03/10/19 08:18PM, harish k wrote: > > Hi All, > > I', Just reopening this feature request. > > A quick summary of my proposal is given below. > > > > 1. This PR will allow an additional configuration option > > "guitool.<name>.gitgui-shortcut" which will allow us to specify > > keyboard shortcut for custom commands in git-gui > > A pretty nice way of doing it. But I would _really_ like it if there was > an option in the "create tool" dialog to specify the shortcut. People of > a gui tool shouldn't have to mess around with config files as much as > possible. > > > 2. Even there exists a parameter called "guitool.<name>.shortcut" > > which is used by git-cola, I suggest to keep this new 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. > > David has advocated inter-operability between git-gui and git-cola. > While I personally don't know how many people actually use both the > tools at the same time, it doesn't sound like a bad idea either. > > So, sharing shortcuts with git-cola would be nice. Of course, it would > then mean that we would have to parse the config parameter before > feeding them to `bind`. I don't suppose that should be something too > complicated to do, but I admit I haven't looked too deeply into it. > > I'd like to hear what other people think about whether it is worth the > effort to inter-operate with git-cola. > > > 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. > > Are you sure that is a good idea? I think we should at least make sure > we are not binding some illegal sequence, and if we are, we should warn > the user about it. And a much more important case would be when a user > over-writes a pre-existing shortcut for other commands like "commit", > "reset", etc. In that case, the menu entires of those commands would > still be labelled with the shortcut, but it won't actually work. > > Yes, your current implementation keeps things simple, but I think some > light processing would be beneficial. And if we do decide to go the > inter-operability with git-cola route, then processing would be needed > anyway, and we can validate there. > > > --- > > Documentation/config/guitool.txt | 15 +++++++++++++++ > > git-gui/lib/tools.tcl | 15 ++++++++++++--- > > 2 files changed, 27 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/config/guitool.txt b/Documentation/config/guitool.txt > > index 43fb9466ff..79dac23ca3 100644 > > --- a/Documentation/config/guitool.txt > > +++ b/Documentation/config/guitool.txt > > @@ -48,3 +48,18 @@ guitool.<name>.prompt:: > > Specifies the general prompt string to display at the top of > > the dialog, before subsections for 'argPrompt' and 'revPrompt'. > > The default value includes the actual command. > > + > > +guitool.<name>.gitgui-shortcut > > + Specifies a keyboard shortcut for the custom tool in the git-gui > > + application. The value must be a valid string ( without "<" , ">" wrapper ) > > + understood by the TCL/TK 's bind command.See > > https://www.tcl.tk/man/tcl8.4/TkCmd/bind.htm > > + for more details about the supported values. Avoid creating shortcuts that > > + conflict with existing built-in `git gui` shortcuts. > > + Example: > > + [guitool "Terminal"] > > + cmd = gnome-terminal -e zsh > > + noconsole = yes > > + gitgui-shortcut = "Control-y" > > + [guitool "Sync"] > > + cmd = "git pull; git push" > > + gitgui-shortcut = "Alt-s" > > The "Documentation/" subdirectory belongs to the Git project, and not to > git-gui, so if you want to see this change, you'd have to submit a > separate patch for it. > > As far as git-gui's documentation is concerned, unfortunately there is > none yet. I have been meaning to start working towards it, but just > haven't found the time or motivation to do it yet. > > > diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl > > Like I mentioned before, please base your patches on the git-gui.git > repo, and not git.git. So, this should read "a/lib/tools.tcl" instead of > "a/git-gui/lib/tools.tcl". > > I haven't looked at the contents of the patch because I can't apply it, > and I'd prefer to tinker around with it before commenting. So please > re-send the patch in the proper format and we can discuss the > implementation :). > > > index 413f1a1700..40db3f6395 100644 > > --- a/git-gui/lib/tools.tcl > > +++ b/git-gui/lib/tools.tcl > [snip] > > -- > Regards, > Pratyush Yadav > ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-04 8:49 ` Johannes Schindelin @ 2019-10-04 12:01 ` Pratyush Yadav 2019-10-05 20:16 ` Harish Karumuthil 0 siblings, 1 reply; 29+ messages in thread From: Pratyush Yadav @ 2019-10-04 12:01 UTC (permalink / raw) To: Johannes Schindelin; +Cc: harish k, git, David Aguilar On 04/10/19 10:49AM, Johannes Schindelin wrote: > Hi Pratyush, > > please don't top-post on this list (yet another of these things > requiring extra brain cycles in the mailing list workflow). I didn't top-post, or at least it wasn't the intention. The text above the quoted text is the "preface" (just like this line I'm replying to is a "preface"). It was a comment on the patch formatting, and it wouldn't make sense (at least to me) to have it _below_ the quoted part, especially since I did have comments in-line (IOW, bottom-posted). Maybe because I included the references in the middle of the text you thought the message was over (understandably so. I probably shouldn't do that), and didn't scroll till the end to read the rest of the message. Or maybe I don't know something about email etiquette that I should. > On Fri, 4 Oct 2019, Pratyush Yadav wrote: > > > Hi Harish, > > > > Thanks for the patch. Unfortunately, it seems your mail client messed up > > the formatting, and the patch won't apply. I'm guessing it is because > > your mail client broke long lines into two, messing up the diff. > > > > We use an email-based workflow, so please either configure your mail > > client so it doesn't munge patches, or use `git send-email`. You can > > find a pretty good tutorial on sending patches via email over at [0]. > > The tutorial is for git.git, but works for git-gui.git as well. > > Ah, well. Mailing list-based workflows are so easy, amirite? They are so > welcoming and inclusive, yes? > </sarcasm> > > > If you feel more comfortable with GitHub pull requests, please take a > > look at Gitgitgadget [1]. Johannes (in Cc) has used it recently to send > > patches based on the git-gui repo (AFAIK, it was originally designed > > only for the git.git repo). Maybe ask the folks over there how they do > > it. > > Harish, it is actually relatively easy to use GitGitGadget: just add a > remote like this: > > git remote add gitgitgadget https://github.com/gitgitgadget/git > git fetch gitgitgadget git-gui/master > > and then rebase your patch on top of that branch: > > git rebase -i --onto git-gui/master HEAD~1 > > Then force-push your branch to your GitHub fork of git.git and open a > Pull Request at https://github.com/gitgitgadget/git/pulls, targeting > git-gui/master. > > GitGitGadget will welcome you with a (hopefully) helpful message ;-) Thanks for these instructions. I will include them in a "Contributing" document I'm writing for git-gui. -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-04 12:01 ` Pratyush Yadav @ 2019-10-05 20:16 ` Harish Karumuthil 2019-10-05 21:01 ` Pratyush Yadav 0 siblings, 1 reply; 29+ messages in thread From: Harish Karumuthil @ 2019-10-05 20:16 UTC (permalink / raw) To: Pratyush Yadav, Johannes Schindelin; +Cc: git, David Aguilar Hi All, From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I understood that, my current email client ( that is gmail web ) is not good for submitting patches. So I was tying to setup a mail client which is compatible with `git send-mail`. But I was not able to get a satisfactory result in that. For now, I followed the instruction of Johannes Schindelin and submitted a pull request . Please see https://github.com/gitgitgadget/git/pull/376 --------- @ Pratyush: Regarding your comments, > A pretty nice way of doing it. But I would _really_ like it if there was > an option in the "create tool" dialog to specify the shortcut. People of > a gui tool shouldn't have to mess around with config files as much as > possible. I agree with this, But that may require some more profficiency in TCL/TK programming which I don't have. This is the first time I am looking into a TCL/TK source code. Any way I will try to integrate the gui gradually in feature. But unfortunatly, I may not be able to do that now. > David has advocated inter-operability between git-gui and git-cola. > While I personally don't know how many people actually use both the > tools at the same time, it doesn't sound like a bad idea either. > > So, sharing shortcuts with git-cola would be nice. Of course, it would > then mean that we would have to parse the config parameter before > feeding them to `bind`. I don't suppose that should be something too > complicated to do, but I admit I haven't looked too deeply into it. IMHO, Using a uniform shortcut-key code/foramat for both application can be considered as nice feature. But, whether we should share common shortcut-scheme with both application is a different question. Currently, both apps don't have a common shortcut-scheme. So in this situation, only sharing custom-tool's shortcut-scheme with both applications doesn't look like a good idea to me > Are you sure that is a good idea? I think we should at least make sure > we are not binding some illegal sequence, and if we are, we should warn > the user about it. And a much more important case would be when a user > over-writes a pre-existing shortcut for other commands like "commit", > "reset", etc. In that case, the menu entires of those commands would > still be labelled with the shortcut, but it won't actually work. I agree with you. It is an important point. After reading this, I checked current status of these issues. What I found is given below. 1. When user provides an invalid sequence for the shortcut, it will cuase the entire gitgui application to crash at the startup 2. When user tries to overwrite existing shortcut, it will not have any effect. Because, built in shortcuts will overwrite user provided one. But still, wrong menu accelerator label will persist for custom tools Since #1 is a serious issue, I tried to find out the function which does the keycode validation, but I haven't succeded till now. ( I found the C function name which is "TkStringToKeysym" from TK source, but I couldn't find its TCL binding ). It will be helpful if any one can help me on this. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-05 20:16 ` Harish Karumuthil @ 2019-10-05 21:01 ` Pratyush Yadav 2019-10-06 9:49 ` Johannes Schindelin 2019-10-07 6:13 ` Harish Karumuthil 0 siblings, 2 replies; 29+ messages in thread From: Pratyush Yadav @ 2019-10-05 21:01 UTC (permalink / raw) To: Harish Karumuthil; +Cc: Johannes Schindelin, git, David Aguilar On 06/10/19 01:46AM, Harish Karumuthil wrote: > Hi All, > > From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I > understood that, my current email client ( that is gmail web ) is not good > for submitting patches. So I was tying to setup a mail client which is > compatible with `git send-mail`. But I was not able to get a satisfactory > result in that. You don't need to "set up" an email client with git-send-email. git-send-email is an email client itself. Well, one which can only send emails. So what you should do is run `git format-patch -o feature master..HEAD`, assuming your feature branch is checked out. This will give you a set of '.patch' files depending on how many commits you made in your branch in the folder feature/. Then, you can run git send-email --to='Pratyush Yadav <me@yadavpratyush.com>' --cc='<git@vger.kernel.org>' feature/*.patch This will send all your patch files via email to me with the git list in Cc. You can add multiple '--to' and '--cc' options to send it to multiple people. Try sending the patches to yourself to experiment around with it. A pretty good tutorial to configuring and using git-send-email can be found at [0]. And of course, read the man page. These instructions are for Linux, but you can probably do something similar in Windows too (if you're using Windows that is). > For now, I followed the instruction of Johannes Schindelin and submitted a > pull request . Please see https://github.com/gitgitgadget/git/pull/376 You haven't sent '/submit' over there, so those emails aren't in the list (and my inbox) yet. You need to comment with '/submit' (without the quotes) to tell GitGitGadget to send your PR as email. But I see that Dscho has left a comment over there, so you should probably address that first. You probably need to amend the commit, force push, and then comment with '/submit'. But I'm not a 100% sure because I haven't used GitHub PRs a lot. > --------- > @ Pratyush: Regarding your comments, > > > > A pretty nice way of doing it. But I would _really_ like it if there was > > an option in the "create tool" dialog to specify the shortcut. People of > > a gui tool shouldn't have to mess around with config files as much as > > possible. > > I agree with this, But that may require some more profficiency in TCL/TK > programming which I don't have. This is the first time I am looking into a > TCL/TK source code. > Any way I will try to integrate the gui gradually in feature. But > unfortunatly, I may not be able to do that now. Please do whatever you can. I will try to add a patch on top of yours to add the GUI option. > > David has advocated inter-operability between git-gui and git-cola. > > While I personally don't know how many people actually use both the > > tools at the same time, it doesn't sound like a bad idea either. > > > > So, sharing shortcuts with git-cola would be nice. Of course, it would > > then mean that we would have to parse the config parameter before > > feeding them to `bind`. I don't suppose that should be something too > > complicated to do, but I admit I haven't looked too deeply into it. > > IMHO, Using a uniform shortcut-key code/foramat for both application can be > considered as nice feature. > But, whether we should share common shortcut-scheme with both application is > a different question. > Currently, both apps don't have a common shortcut-scheme. So in this > situation, only sharing custom-tool's shortcut-scheme with both applications > doesn't look like a good idea to me Makes sense. > > Are you sure that is a good idea? I think we should at least make > > sure > > we are not binding some illegal sequence, and if we are, we should warn > > the user about it. And a much more important case would be when a user > > over-writes a pre-existing shortcut for other commands like "commit", > > "reset", etc. In that case, the menu entires of those commands would > > still be labelled with the shortcut, but it won't actually work. > > I agree with you. It is an important point. After reading this, I checked > current status of these issues. What I found is given below. > > 1. When user provides an invalid sequence for the shortcut, it will cuase the > entire gitgui application to crash at the startup > > 2. When user tries to overwrite existing shortcut, it will not have any > effect. Because, built in shortcuts will overwrite user provided one. But > still, wrong menu accelerator label will persist for custom tools One point I forgot to mention earlier was that I'm honestly not a big fan of separating the binding and accelerator label. I understand that you might not have the time to do this, but I think it is still worth mentioning. Maybe I will implement something like that over your patch. But it would certainly be nice if you can figure it out :). Either ways, detecting an existing shortcut is pretty easy. The `bind` man page [1] says: If sequence is specified without a script, then the script currently bound to sequence is returned, or an empty string is returned if there is no binding for sequence. So you can use this to find out if there is a binding conflict, and warn the user. > Since #1 is a serious issue, I tried to find out the function which does the > keycode validation, but I haven't succeded till now. ( I found the C function > name which is "TkStringToKeysym" from TK source, but I couldn't find its TCL > binding ). It will be helpful if any one can help me on this. I really think you shouldn't dive around in the C parts of Tcl. I haven't looked too deeply into this, but you can probably wrap your bind calls in `catch` [2] and handle errors from there. Again, I haven't tried actually doing this, so you do need to check first. You can find examples of how to use `catch` in our codebase. Just search for it. [0] https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/ [1] https://www.tcl.tk/man/tcl8.4/TkCmd/bind.htm [2] https://www.tcl.tk/man/tcl8.4/TclCmd/catch.htm -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-05 21:01 ` Pratyush Yadav @ 2019-10-06 9:49 ` Johannes Schindelin 2019-10-06 18:39 ` Pratyush Yadav 2019-10-07 6:22 ` Harish Karumuthil 2019-10-07 6:13 ` Harish Karumuthil 1 sibling, 2 replies; 29+ messages in thread From: Johannes Schindelin @ 2019-10-06 9:49 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Harish Karumuthil, git, David Aguilar Hi Pratyush, On Sun, 6 Oct 2019, Pratyush Yadav wrote: > On 06/10/19 01:46AM, Harish Karumuthil wrote: > > > > From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I > > understood that, my current email client ( that is gmail web ) is not good > > for submitting patches. So I was tying to setup a mail client which is > > compatible with `git send-mail`. But I was not able to get a satisfactory > > result in that. > > You don't need to "set up" an email client with git-send-email. > git-send-email is an email client itself. Well, one which can only send > emails. It also cannot reply to mails on the mailing list. It cannot even notify you when anybody replied to your patch. Two rather problematic aspects when it comes to patch contributions: how are you supposed to work with the reviewers when you lack all the tools to _interact_ with them? All `git send-email` provides is a "fire and forget" way to send patches, i.e. it encourages a monologue, when you want to start a dialogue instead. > So what you should do is run `git format-patch -o feature master..HEAD`, > assuming your feature branch is checked out. This will give you a set of > '.patch' files depending on how many commits you made in your branch in > the folder feature/. Then, you can run > > git send-email --to='Pratyush Yadav <me@yadavpratyush.com>' --cc='<git@vger.kernel.org>' feature/*.patch > > This will send all your patch files via email to me with the git list in > Cc. You can add multiple '--to' and '--cc' options to send it to > multiple people. > > Try sending the patches to yourself to experiment around with it. > > A pretty good tutorial to configuring and using git-send-email can be > found at [0]. And of course, read the man page. > > These instructions are for Linux, but you can probably do something > similar in Windows too (if you're using Windows that is). Last I checked, `git send-email` worked in Git for Windows. But of course, it does not only not address the problem it tries to solve fully (to provide a way to interact with a mailing list when submitting patches for review), not even close, to add insult to injury, it now adds an additional burden to contributors (who might already have struggled to learn themselves enough Tcl/Tk to fix the problem) to configure `git send-email` correctly. > > For now, I followed the instruction of Johannes Schindelin and submitted a > > pull request . Please see https://github.com/gitgitgadget/git/pull/376 > > You haven't sent '/submit' over there, so those emails aren't in the > list (and my inbox) yet. You need to comment with '/submit' (without the > quotes) to tell GitGitGadget to send your PR as email. They probably did not hit `/submit` because the initial hurdle is to be `/allow`ed (a very, very simplistic attempt at trying to prevent spamming the mailing list by jokesters, of which there are unfortunately quite a number). This `/allow` command, BTW, can be issued by anybody who has been `/allow`ed before, it does not always have to be me. FWIW you should probably be in that list of `/allow`ed people so that you can `/allow` new contributors to use GitGitGadget, too. > [...] > > > Since #1 is a serious issue, I tried to find out the function which does the > > keycode validation, but I haven't succeded till now. ( I found the C function > > name which is "TkStringToKeysym" from TK source, but I couldn't find its TCL > > binding ). It will be helpful if any one can help me on this. > > I really think you shouldn't dive around in the C parts of Tcl. I > haven't looked too deeply into this, but you can probably wrap your bind > calls in `catch` [2] and handle errors from there. Again, I haven't > tried actually doing this, so you do need to check first. > > You can find examples of how to use `catch` in our codebase. Just search > for it. FWIW in addition to the `catch` method, I would also recommend looking into a minimal (not even necessarily complete) way to translate the Qt way to specify the keyboard shortcuts (as used by `git-cola`) to Tk ones. As indicated in https://github.com/git/git/pull/220#issuecomment-536045075, the Qt style `CTRL+,` should be translated to `Control-comma`, for example. In particular, keystrokes specified in the format indicated at https://doc.qt.io/archives/qt-4.8/qkeysequence.html#QKeySequence-2 to the format indicated at https://www.tcl.tk/man/tcl8.4/TkCmd/keysyms.htm. However, it might not even need to put in _such_ a lot of work: in my tests, `Control-,` worked just as well as `Control-comma`. To test this for yourself, use this snippet (that is slightly modified from the example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so that it reacts _only_ to Control+comma instead of all keys): -- snip -- set keysym "Press any key" pack [label .l -textvariable keysym -padx 2m -pady 1m] #bind . <Key> { bind . <Control-,> { set keysym "You pressed %K" } -- snap -- So I could imagine that something like this could serve as an initial draft for a function that you can turn into a "good enough" version: -- snip -- proc QKeySequence2keysym {keystroke} { regsub -all {(?i)Ctrl\+} $keystroke "Control-" keystroke regsub -all {(?i)Alt\+} $keystroke "Alt-" keystroke regsub -all {(?i)Shift\+} $keystroke "Shift-" keystroke return $keystroke } -- snap -- That way, you don't have to introduce settings separate from `git-cola`'s, and you can reuse the short-and-sweet variable name. Ciao, Johannes ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-06 9:49 ` Johannes Schindelin @ 2019-10-06 18:39 ` Pratyush Yadav 2019-10-06 19:37 ` Philip Oakley 2019-10-06 20:27 ` Johannes Schindelin 2019-10-07 6:22 ` Harish Karumuthil 1 sibling, 2 replies; 29+ messages in thread From: Pratyush Yadav @ 2019-10-06 18:39 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Harish Karumuthil, git, David Aguilar On 06/10/19 11:49AM, Johannes Schindelin wrote: > Hi Pratyush, > > On Sun, 6 Oct 2019, Pratyush Yadav wrote: > > > On 06/10/19 01:46AM, Harish Karumuthil wrote: > > > > > > From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I > > > understood that, my current email client ( that is gmail web ) is not good > > > for submitting patches. So I was tying to setup a mail client which is > > > compatible with `git send-mail`. But I was not able to get a satisfactory > > > result in that. > > > > You don't need to "set up" an email client with git-send-email. > > git-send-email is an email client itself. Well, one which can only send > > emails. > > It also cannot reply to mails on the mailing list. > > It cannot even notify you when anybody replied to your patch. > > Two rather problematic aspects when it comes to patch contributions: how > are you supposed to work with the reviewers when you lack all the tools > to _interact_ with them? All `git send-email` provides is a "fire and > forget" way to send patches, i.e. it encourages a monologue, when you > want to start a dialogue instead. Well, I started with email based patch contribution when I was first got started with open source, so I might be a bit biased, but in my experience, it is not that difficult to set all these things up. Most of the time, all you need to tell git-send-email is your SMTP server, username, and password. All pretty easy things to do. And you add in your email client (which pretty much everyone should have), and it is a complete setup. I personally use neomutt as my email client, but I have used Thunderbird before, and it is really easy to set it up to send plain text emails. All you need to do is hold Shift before hitting reply, and you're in plain text mode. And you can even make it use plain text by default by flipping a switch in the settings. So while I agree with you that there is certainly a learning curve involved, I don't think it is all too bad. But again, that is all my personal opinion, and nothing based on facts or data. > > So what you should do is run `git format-patch -o feature master..HEAD`, > > assuming your feature branch is checked out. This will give you a set of > > '.patch' files depending on how many commits you made in your branch in > > the folder feature/. Then, you can run > > > > git send-email --to='Pratyush Yadav <me@yadavpratyush.com>' --cc='<git@vger.kernel.org>' feature/*.patch > > > > This will send all your patch files via email to me with the git list in > > Cc. You can add multiple '--to' and '--cc' options to send it to > > multiple people. > > > > Try sending the patches to yourself to experiment around with it. > > > > A pretty good tutorial to configuring and using git-send-email can be > > found at [0]. And of course, read the man page. > > > > These instructions are for Linux, but you can probably do something > > similar in Windows too (if you're using Windows that is). > > Last I checked, `git send-email` worked in Git for Windows. > > But of course, it does not only not address the problem it tries to > solve fully (to provide a way to interact with a mailing list when > submitting patches for review), not even close, to add insult to injury, > it now adds an additional burden to contributors (who might already have > struggled to learn themselves enough Tcl/Tk to fix the problem) to > configure `git send-email` correctly. The way I see it, git-send-email does not need to solve the problem of interacting with a mailing list. That problem is already solved by a hoard of MUAs. All git-send-email should do is, you guessed it, send emails (or patches to be specific). Anyway, GitGitGadget solves a large part of the problem. It eliminates the need for using git-send-email, and it even shows you the replies received on the list. I honestly think it is a great tool, and it gives people a very good alternative to using git-send-email. One feature that would make it complete would be the ability to reply to review comments. This would remove the need for an email client (almost) completely. I have never written Typescript or used Azure pipelines ever, but I can try tinkering around to see if I can figure out how to do something like that. Unless, of course, you or someone else is already doing it. If not, some pointers would be appreciated. > > > For now, I followed the instruction of Johannes Schindelin and submitted a > > > pull request . Please see https://github.com/gitgitgadget/git/pull/376 > > > > You haven't sent '/submit' over there, so those emails aren't in the > > list (and my inbox) yet. You need to comment with '/submit' (without the > > quotes) to tell GitGitGadget to send your PR as email. > > They probably did not hit `/submit` because the initial hurdle is to be > `/allow`ed (a very, very simplistic attempt at trying to prevent > spamming the mailing list by jokesters, of which there are unfortunately > quite a number). > > This `/allow` command, BTW, can be issued by anybody who has been > `/allow`ed before, it does not always have to be me. > > FWIW you should probably be in that list of `/allow`ed people so that > you can `/allow` new contributors to use GitGitGadget, too. That would be great! How do I get '/allow'ed? Do I have to open a PR there for you to '/allow' me? > > [...] > > > > > Since #1 is a serious issue, I tried to find out the function which does the > > > keycode validation, but I haven't succeded till now. ( I found the C function > > > name which is "TkStringToKeysym" from TK source, but I couldn't find its TCL > > > binding ). It will be helpful if any one can help me on this. > > > > I really think you shouldn't dive around in the C parts of Tcl. I > > haven't looked too deeply into this, but you can probably wrap your bind > > calls in `catch` [2] and handle errors from there. Again, I haven't > > tried actually doing this, so you do need to check first. > > > > You can find examples of how to use `catch` in our codebase. Just search > > for it. > > FWIW in addition to the `catch` method, I would also recommend looking > into a minimal (not even necessarily complete) way to translate the Qt > way to specify the keyboard shortcuts (as used by `git-cola`) to Tk > ones. > > As indicated in > https://github.com/git/git/pull/220#issuecomment-536045075, the Qt style > `CTRL+,` should be translated to `Control-comma`, for example. In > particular, keystrokes specified in the format indicated at > https://doc.qt.io/archives/qt-4.8/qkeysequence.html#QKeySequence-2 to > the format indicated at https://www.tcl.tk/man/tcl8.4/TkCmd/keysyms.htm. > > However, it might not even need to put in _such_ a lot of work: in my > tests, `Control-,` worked just as well as `Control-comma`. To test this > for yourself, use this snippet (that is slightly modified from the > example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so > that it reacts _only_ to Control+comma instead of all keys): Another benefit to the translation framework would be that we could also generate the menu labels (aka "accelerator") for the tools, instead of making the user specify both the shortcut and the label. > -- snip -- > set keysym "Press any key" > pack [label .l -textvariable keysym -padx 2m -pady 1m] > #bind . <Key> { > bind . <Control-,> { > set keysym "You pressed %K" > } > -- snap -- > > So I could imagine that something like this could serve as an initial > draft for a function that you can turn into a "good enough" version: > > -- snip -- > proc QKeySequence2keysym {keystroke} { > regsub -all {(?i)Ctrl\+} $keystroke "Control-" keystroke > regsub -all {(?i)Alt\+} $keystroke "Alt-" keystroke > regsub -all {(?i)Shift\+} $keystroke "Shift-" keystroke > return $keystroke > } > -- snap -- > > That way, you don't have to introduce settings separate from > `git-cola`'s, and you can reuse the short-and-sweet variable name. I think a more important question is whether we _really_ need to have compatibility with git-cola. Most of our shortcuts don't match with them, so is it really worth the effort to try to keep compatibility? I'm not against something like this, but just want to be sure we evaluate whether the effort is worth it. -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-06 18:39 ` Pratyush Yadav @ 2019-10-06 19:37 ` Philip Oakley 2019-10-06 20:27 ` Johannes Schindelin 1 sibling, 0 replies; 29+ messages in thread From: Philip Oakley @ 2019-10-06 19:37 UTC (permalink / raw) To: Pratyush Yadav, Johannes Schindelin; +Cc: Harish Karumuthil, git, David Aguilar On 06/10/2019 19:39, Pratyush Yadav wrote: >> That way, you don't have to introduce settings separate from >> `git-cola`'s, and you can reuse the short-and-sweet variable name. > I think a more important question is whether we_really_ need to have > compatibility with git-cola. Most of our shortcuts don't match with > them, so is it really worth the effort to try to keep compatibility? > > I'm not against something like this, but just want to be sure we > evaluate whether the effort is worth it. I just wondered if the custom list would/could be split between a "Common-shortcuts" list, and "GUI-local" list (and hence, elsewhere, a "Cola-local" list) I don't use Cola at all, so this is just a bikeshed comment... -- Philip ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-06 18:39 ` Pratyush Yadav 2019-10-06 19:37 ` Philip Oakley @ 2019-10-06 20:27 ` Johannes Schindelin 2019-10-06 21:06 ` Pratyush Yadav 2019-10-06 22:40 ` Philip Oakley 1 sibling, 2 replies; 29+ messages in thread From: Johannes Schindelin @ 2019-10-06 20:27 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Harish Karumuthil, git, David Aguilar Hi Pratyush, On Mon, 7 Oct 2019, Pratyush Yadav wrote: > On 06/10/19 11:49AM, Johannes Schindelin wrote: > > Hi Pratyush, > > > > On Sun, 6 Oct 2019, Pratyush Yadav wrote: > > > > > On 06/10/19 01:46AM, Harish Karumuthil wrote: > > > > > > > > From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I > > > > understood that, my current email client ( that is gmail web ) is not good > > > > for submitting patches. So I was tying to setup a mail client which is > > > > compatible with `git send-mail`. But I was not able to get a satisfactory > > > > result in that. > > > > > > You don't need to "set up" an email client with git-send-email. > > > git-send-email is an email client itself. Well, one which can only send > > > emails. > > > > It also cannot reply to mails on the mailing list. > > > > It cannot even notify you when anybody replied to your patch. > > > > Two rather problematic aspects when it comes to patch contributions: how > > are you supposed to work with the reviewers when you lack all the tools > > to _interact_ with them? All `git send-email` provides is a "fire and > > forget" way to send patches, i.e. it encourages a monologue, when you > > want to start a dialogue instead. > > Well, I started with email based patch contribution when I was first got > started with open source, so I might be a bit biased, but in my > experience, it is not that difficult to set all these things up. Most of > the time, all you need to tell git-send-email is your SMTP server, > username, and password. All pretty easy things to do. Okay, set it up with a corporate Exchange server. I'll be waiting right here. > And you add in your email client (which pretty much everyone should > have), and it is a complete setup. I personally use neomutt as my email > client, but I have used Thunderbird before, and it is really easy to set > it up to send plain text emails. All you need to do is hold Shift before > hitting reply, and you're in plain text mode. And you can even make it > use plain text by default by flipping a switch in the settings. How intuitive. And of course Thunderbird still messes up the patches so that they won't apply, unless you *checks notes* do things that are quite involved or *checks notes* do other things that are quite involved. But then, everybody reads their mails on their command-line anyway. Right? > So while I agree with you that there is certainly a learning curve > involved, I don't think it is all too bad. But again, that is all my > personal opinion, and nothing based on facts or data. Let me provide you with some data, then. Granted, it's not necessarily all Git GUI, but it includes Git GUI patches, too: Git for Windows' contributions. As should be well-known, I try to follow Postel's Law when it comes to Git for Windows' patches: be lenient in the input, strict in the output. As such, I don't force contributors to use GitHub PRs (although that is certainly encouraged by virtue of Git for Windows' source code being hosted on GitHub), or send patches, or send pull requests to their own public repositories or bundles sent to the mailing list. I accept them all. At least that is the idea. I cannot tell you how many contributions came in via GitHub PRs. I can tell precisely you how many contributions were made _not_ using GitHub PRs. One one hand. Actually, on zero hands. So clearly, at least Git for Windows' contributors (including some who provided Git GUI patches) are much more comfortable with the PR workflow than with the mailing list-based workflow. Just so you can't say you don't have data. > > > So what you should do is run `git format-patch -o feature master..HEAD`, > > > assuming your feature branch is checked out. This will give you a set of > > > '.patch' files depending on how many commits you made in your branch in > > > the folder feature/. Then, you can run > > > > > > git send-email --to='Pratyush Yadav <me@yadavpratyush.com>' --cc='<git@vger.kernel.org>' feature/*.patch > > > > > > This will send all your patch files via email to me with the git list in > > > Cc. You can add multiple '--to' and '--cc' options to send it to > > > multiple people. > > > > > > Try sending the patches to yourself to experiment around with it. > > > > > > A pretty good tutorial to configuring and using git-send-email can be > > > found at [0]. And of course, read the man page. > > > > > > These instructions are for Linux, but you can probably do something > > > similar in Windows too (if you're using Windows that is). > > > > Last I checked, `git send-email` worked in Git for Windows. > > > > But of course, it does not only not address the problem it tries to > > solve fully (to provide a way to interact with a mailing list when > > submitting patches for review), not even close, to add insult to injury, > > it now adds an additional burden to contributors (who might already have > > struggled to learn themselves enough Tcl/Tk to fix the problem) to > > configure `git send-email` correctly. > > The way I see it, git-send-email does not need to solve the problem of > interacting with a mailing list. That problem is already solved by a > hoard of MUAs. All git-send-email should do is, you guessed it, send > emails (or patches to be specific). But of course! And it is natural that you should use two separate MUAs. > Anyway, GitGitGadget solves a large part of the problem. It eliminates > the need for using git-send-email, and it even shows you the replies > received on the list. I honestly think it is a great tool, and it gives > people a very good alternative to using git-send-email. GitGitGadget is just a workaround. Not even complete. Can't be complete, really. Because problems. It has much of the same problems of `git send-email`: it's a one-way conversation. Code is not discussed in the right context (which would be a worktree with the correct commit checked out). The transfer is lossy (email is designed for human-readable messages, not for transferring machine-readable serialized objects). Matching original commits and/or branches to the ones on the other side is tedious. Any interaction requires switching between many tools. Etc > One feature that would make it complete would be the ability to reply to > review comments. And how would that work, exactly? How to determine *which* email to respond to? *Which* person to reply to? *What* to quote? > This would remove the need for an email client (almost) completely. I > have never written Typescript or used Azure pipelines ever, but I can > try tinkering around to see if I can figure out how to do something > like that. Unless, of course, you or someone else is already doing it. > If not, some pointers would be appreciated. Feel free to give this challenge a try. > > > > For now, I followed the instruction of Johannes Schindelin and submitted a > > > > pull request . Please see https://github.com/gitgitgadget/git/pull/376 > > > > > > You haven't sent '/submit' over there, so those emails aren't in the > > > list (and my inbox) yet. You need to comment with '/submit' (without the > > > quotes) to tell GitGitGadget to send your PR as email. > > > > They probably did not hit `/submit` because the initial hurdle is to be > > `/allow`ed (a very, very simplistic attempt at trying to prevent > > spamming the mailing list by jokesters, of which there are unfortunately > > quite a number). > > > > This `/allow` command, BTW, can be issued by anybody who has been > > `/allow`ed before, it does not always have to be me. > > > > FWIW you should probably be in that list of `/allow`ed people so that > > you can `/allow` new contributors to use GitGitGadget, too. > > That would be great! How do I get '/allow'ed? Do I have to open a PR > there for you to '/allow' me? https://github.com/gitgitgadget/git/pull/376#issuecomment-538784646 > > > [...] > > > > > > > Since #1 is a serious issue, I tried to find out the function which does the > > > > keycode validation, but I haven't succeded till now. ( I found the C function > > > > name which is "TkStringToKeysym" from TK source, but I couldn't find its TCL > > > > binding ). It will be helpful if any one can help me on this. > > > > > > I really think you shouldn't dive around in the C parts of Tcl. I > > > haven't looked too deeply into this, but you can probably wrap your bind > > > calls in `catch` [2] and handle errors from there. Again, I haven't > > > tried actually doing this, so you do need to check first. > > > > > > You can find examples of how to use `catch` in our codebase. Just search > > > for it. > > > > FWIW in addition to the `catch` method, I would also recommend looking > > into a minimal (not even necessarily complete) way to translate the Qt > > way to specify the keyboard shortcuts (as used by `git-cola`) to Tk > > ones. > > > > As indicated in > > https://github.com/git/git/pull/220#issuecomment-536045075, the Qt style > > `CTRL+,` should be translated to `Control-comma`, for example. In > > particular, keystrokes specified in the format indicated at > > https://doc.qt.io/archives/qt-4.8/qkeysequence.html#QKeySequence-2 to > > the format indicated at https://www.tcl.tk/man/tcl8.4/TkCmd/keysyms.htm. > > > > However, it might not even need to put in _such_ a lot of work: in my > > tests, `Control-,` worked just as well as `Control-comma`. To test this > > for yourself, use this snippet (that is slightly modified from the > > example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so > > that it reacts _only_ to Control+comma instead of all keys): > > Another benefit to the translation framework would be that we could also > generate the menu labels (aka "accelerator") for the tools, instead of > making the user specify both the shortcut and the label. > > > -- snip -- > > set keysym "Press any key" > > pack [label .l -textvariable keysym -padx 2m -pady 1m] > > #bind . <Key> { > > bind . <Control-,> { > > set keysym "You pressed %K" > > } > > -- snap -- > > > > So I could imagine that something like this could serve as an initial > > draft for a function that you can turn into a "good enough" version: > > > > -- snip -- > > proc QKeySequence2keysym {keystroke} { > > regsub -all {(?i)Ctrl\+} $keystroke "Control-" keystroke > > regsub -all {(?i)Alt\+} $keystroke "Alt-" keystroke > > regsub -all {(?i)Shift\+} $keystroke "Shift-" keystroke > > return $keystroke > > } > > -- snap -- > > > > That way, you don't have to introduce settings separate from > > `git-cola`'s, and you can reuse the short-and-sweet variable name. > > I think a more important question is whether we _really_ need to have > compatibility with git-cola. Most of our shortcuts don't match with > them, so is it really worth the effort to try to keep compatibility? > > I'm not against something like this, but just want to be sure we > evaluate whether the effort is worth it. `git-cola`, by virtue of being there first, squats on the neat config setting name `shortcut`. I expect users to be utterly surprised when that name does not work for them. Please note that I intended `QKeySequence2keysym` to leave parameters unchanged that already refer to keysyms. So this is purely a way to reuse the same name as `git-cola` while still playing nice with existing configurations targeting `git-cola`. Ciao, Johannes ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-06 20:27 ` Johannes Schindelin @ 2019-10-06 21:06 ` Pratyush Yadav 2019-10-07 9:20 ` GitGUIGadget, was " Johannes Schindelin 2019-11-19 22:09 ` Making GitGitGadget's list -> PR comment mirroring bidirectional, " Johannes Schindelin 2019-10-06 22:40 ` Philip Oakley 1 sibling, 2 replies; 29+ messages in thread From: Pratyush Yadav @ 2019-10-06 21:06 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Harish Karumuthil, git, David Aguilar On 06/10/19 10:27PM, Johannes Schindelin wrote: > Hi Pratyush, > > On Mon, 7 Oct 2019, Pratyush Yadav wrote: > > > On 06/10/19 11:49AM, Johannes Schindelin wrote: > > > Hi Pratyush, > > > > > > On Sun, 6 Oct 2019, Pratyush Yadav wrote: > > > > > > > On 06/10/19 01:46AM, Harish Karumuthil wrote: > > > > > > > > > > From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I > > > > > understood that, my current email client ( that is gmail web ) is not good > > > > > for submitting patches. So I was tying to setup a mail client which is > > > > > compatible with `git send-mail`. But I was not able to get a satisfactory > > > > > result in that. > > > > > > > > You don't need to "set up" an email client with git-send-email. > > > > git-send-email is an email client itself. Well, one which can only send > > > > emails. > > > > > > It also cannot reply to mails on the mailing list. > > > > > > It cannot even notify you when anybody replied to your patch. > > > > > > Two rather problematic aspects when it comes to patch contributions: how > > > are you supposed to work with the reviewers when you lack all the tools > > > to _interact_ with them? All `git send-email` provides is a "fire and > > > forget" way to send patches, i.e. it encourages a monologue, when you > > > want to start a dialogue instead. > > > > Well, I started with email based patch contribution when I was first got > > started with open source, so I might be a bit biased, but in my > > experience, it is not that difficult to set all these things up. Most of > > the time, all you need to tell git-send-email is your SMTP server, > > username, and password. All pretty easy things to do. > > Okay, set it up with a corporate Exchange server. > > I'll be waiting right here. I admit, I've never had to do that. And by how you word it, hope I never have to do it in the future either. > > And you add in your email client (which pretty much everyone should > > have), and it is a complete setup. I personally use neomutt as my email > > client, but I have used Thunderbird before, and it is really easy to set > > it up to send plain text emails. All you need to do is hold Shift before > > hitting reply, and you're in plain text mode. And you can even make it > > use plain text by default by flipping a switch in the settings. > > How intuitive. And of course Thunderbird still messes up the patches so > that they won't apply, unless you *checks notes* do things that are > quite involved or *checks notes* do other things that are quite > involved. Ha! Made me chuckle. You got me there :). I suppose it isn't _that_ simple and I'm just biased because I am so used to it. > But then, everybody reads their mails on their command-line anyway. > Right? Well, they should. It is objectively superior ;) </sarcasm> > > So while I agree with you that there is certainly a learning curve > > involved, I don't think it is all too bad. But again, that is all my > > personal opinion, and nothing based on facts or data. > > Let me provide you with some data, then. Granted, it's not necessarily > all Git GUI, but it includes Git GUI patches, too: Git for Windows' > contributions. > > As should be well-known, I try to follow Postel's Law when it comes to > Git for Windows' patches: be lenient in the input, strict in the output. > As such, I don't force contributors to use GitHub PRs (although that is > certainly encouraged by virtue of Git for Windows' source code being > hosted on GitHub), or send patches, or send pull requests to their own > public repositories or bundles sent to the mailing list. I accept them > all. At least that is the idea. > > I cannot tell you how many contributions came in via GitHub PRs. I can > tell precisely you how many contributions were made _not_ using GitHub > PRs. One one hand. Actually, on zero hands. > > So clearly, at least Git for Windows' contributors (including some who > provided Git GUI patches) are much more comfortable with the PR workflow > than with the mailing list-based workflow. I never said email is better that GitHub PRs. It isn't. My point was that using email isn't _that_ hard. When I first did it, it maybe took me 3-4 hours to figure everything out, and then I was set forever. I carry around the same '.gitconfig' file to all my setups, and everything "just works". So yes, GitHub PRs are certainly easier, but email wasn't too difficult in my experience. But then I'm a kernel developer, so I'm a minority to begin with. I suspect you've had this debate more than once, because you come in guns blazing ;) > Just so you can't say you don't have data. > > > > > So what you should do is run `git format-patch -o feature master..HEAD`, > > > > assuming your feature branch is checked out. This will give you a set of > > > > '.patch' files depending on how many commits you made in your branch in > > > > the folder feature/. Then, you can run > > > > > > > > git send-email --to='Pratyush Yadav <me@yadavpratyush.com>' --cc='<git@vger.kernel.org>' feature/*.patch > > > > > > > > This will send all your patch files via email to me with the git list in > > > > Cc. You can add multiple '--to' and '--cc' options to send it to > > > > multiple people. > > > > > > > > Try sending the patches to yourself to experiment around with it. > > > > > > > > A pretty good tutorial to configuring and using git-send-email can be > > > > found at [0]. And of course, read the man page. > > > > > > > > These instructions are for Linux, but you can probably do something > > > > similar in Windows too (if you're using Windows that is). > > > > > > Last I checked, `git send-email` worked in Git for Windows. > > > > > > But of course, it does not only not address the problem it tries to > > > solve fully (to provide a way to interact with a mailing list when > > > submitting patches for review), not even close, to add insult to injury, > > > it now adds an additional burden to contributors (who might already have > > > struggled to learn themselves enough Tcl/Tk to fix the problem) to > > > configure `git send-email` correctly. > > > > The way I see it, git-send-email does not need to solve the problem of > > interacting with a mailing list. That problem is already solved by a > > hoard of MUAs. All git-send-email should do is, you guessed it, send > > emails (or patches to be specific). > > But of course! And it is natural that you should use two separate MUAs. > > > Anyway, GitGitGadget solves a large part of the problem. It eliminates > > the need for using git-send-email, and it even shows you the replies > > received on the list. I honestly think it is a great tool, and it gives > > people a very good alternative to using git-send-email. > > GitGitGadget is just a workaround. Not even complete. Can't be complete, > really. Because problems. It has much of the same problems of `git > send-email`: it's a one-way conversation. Code is not discussed in the > right context (which would be a worktree with the correct commit checked > out). The transfer is lossy (email is designed for human-readable > messages, not for transferring machine-readable serialized objects). > Matching original commits and/or branches to the ones on the other side > is tedious. Any interaction requires switching between many tools. Etc > > > One feature that would make it complete would be the ability to reply to > > review comments. > > And how would that work, exactly? How to determine *which* email to > respond to? *Which* person to reply to? *What* to quote? GGG already shows replies to the patches as a comment. On GitHub you can "Quote reply" a comment, which quotes the entire comment just like your MUA would. The option can be found by clicking the 3 dots on the top right of a comment. Then you can write your reply there, and the last line would be '/reply', which would make GGG send that email as a reply. You would need to strip the first line from the reply because GGG starts the reply with something like: > [On the Git mailing list](https://public-inbox.org/git/xmqq7e5l9zb1.fsf@gitster-ct.c.googlers.com), Junio C Hamano wrote ([reply to this](https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis)): GGG also adds 3 backticks before and after the reply content, so those would need to be removed too. Does this sound like a sane solution? > > This would remove the need for an email client (almost) completely. I > > have never written Typescript or used Azure pipelines ever, but I can > > try tinkering around to see if I can figure out how to do something > > like that. Unless, of course, you or someone else is already doing it. > > If not, some pointers would be appreciated. > > Feel free to give this challenge a try. The first challenge is learning Typescript :) > > > > > For now, I followed the instruction of Johannes Schindelin and submitted a > > > > > pull request . Please see https://github.com/gitgitgadget/git/pull/376 > > > > > > > > You haven't sent '/submit' over there, so those emails aren't in the > > > > list (and my inbox) yet. You need to comment with '/submit' (without the > > > > quotes) to tell GitGitGadget to send your PR as email. > > > > > > They probably did not hit `/submit` because the initial hurdle is to be > > > `/allow`ed (a very, very simplistic attempt at trying to prevent > > > spamming the mailing list by jokesters, of which there are unfortunately > > > quite a number). > > > > > > This `/allow` command, BTW, can be issued by anybody who has been > > > `/allow`ed before, it does not always have to be me. > > > > > > FWIW you should probably be in that list of `/allow`ed people so that > > > you can `/allow` new contributors to use GitGitGadget, too. > > > > That would be great! How do I get '/allow'ed? Do I have to open a PR > > there for you to '/allow' me? > > https://github.com/gitgitgadget/git/pull/376#issuecomment-538784646 > > > > > [...] > > > > > > > > > Since #1 is a serious issue, I tried to find out the function which does the > > > > > keycode validation, but I haven't succeded till now. ( I found the C function > > > > > name which is "TkStringToKeysym" from TK source, but I couldn't find its TCL > > > > > binding ). It will be helpful if any one can help me on this. > > > > > > > > I really think you shouldn't dive around in the C parts of Tcl. I > > > > haven't looked too deeply into this, but you can probably wrap your bind > > > > calls in `catch` [2] and handle errors from there. Again, I haven't > > > > tried actually doing this, so you do need to check first. > > > > > > > > You can find examples of how to use `catch` in our codebase. Just search > > > > for it. > > > > > > FWIW in addition to the `catch` method, I would also recommend looking > > > into a minimal (not even necessarily complete) way to translate the Qt > > > way to specify the keyboard shortcuts (as used by `git-cola`) to Tk > > > ones. > > > > > > As indicated in > > > https://github.com/git/git/pull/220#issuecomment-536045075, the Qt style > > > `CTRL+,` should be translated to `Control-comma`, for example. In > > > particular, keystrokes specified in the format indicated at > > > https://doc.qt.io/archives/qt-4.8/qkeysequence.html#QKeySequence-2 to > > > the format indicated at https://www.tcl.tk/man/tcl8.4/TkCmd/keysyms.htm. > > > > > > However, it might not even need to put in _such_ a lot of work: in my > > > tests, `Control-,` worked just as well as `Control-comma`. To test this > > > for yourself, use this snippet (that is slightly modified from the > > > example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so > > > that it reacts _only_ to Control+comma instead of all keys): > > > > Another benefit to the translation framework would be that we could also > > generate the menu labels (aka "accelerator") for the tools, instead of > > making the user specify both the shortcut and the label. > > > > > -- snip -- > > > set keysym "Press any key" > > > pack [label .l -textvariable keysym -padx 2m -pady 1m] > > > #bind . <Key> { > > > bind . <Control-,> { > > > set keysym "You pressed %K" > > > } > > > -- snap -- > > > > > > So I could imagine that something like this could serve as an initial > > > draft for a function that you can turn into a "good enough" version: > > > > > > -- snip -- > > > proc QKeySequence2keysym {keystroke} { > > > regsub -all {(?i)Ctrl\+} $keystroke "Control-" keystroke > > > regsub -all {(?i)Alt\+} $keystroke "Alt-" keystroke > > > regsub -all {(?i)Shift\+} $keystroke "Shift-" keystroke > > > return $keystroke > > > } > > > -- snap -- > > > > > > That way, you don't have to introduce settings separate from > > > `git-cola`'s, and you can reuse the short-and-sweet variable name. > > > > I think a more important question is whether we _really_ need to have > > compatibility with git-cola. Most of our shortcuts don't match with > > them, so is it really worth the effort to try to keep compatibility? > > > > I'm not against something like this, but just want to be sure we > > evaluate whether the effort is worth it. > > `git-cola`, by virtue of being there first, squats on the neat config > setting name `shortcut`. > > I expect users to be utterly surprised when that name does not work for > them. Correct. Makes sense. > Please note that I intended `QKeySequence2keysym` to leave parameters > unchanged that already refer to keysyms. So this is purely a way to > reuse the same name as `git-cola` while still playing nice with existing > configurations targeting `git-cola`. Since you already gave a draft of the function, it shouldn't be too difficult to pick up from there. I'm just waiting for Harish to send in his first iteration so we can first discuss that. -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 29+ messages in thread
* GitGUIGadget, was Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-06 21:06 ` Pratyush Yadav @ 2019-10-07 9:20 ` Johannes Schindelin 2019-10-07 10:43 ` Birger Skogeng Pedersen 2019-11-19 22:09 ` Making GitGitGadget's list -> PR comment mirroring bidirectional, " Johannes Schindelin 1 sibling, 1 reply; 29+ messages in thread From: Johannes Schindelin @ 2019-10-07 9:20 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Harish Karumuthil, git, David Aguilar Hi Pratyush, On Mon, 7 Oct 2019, Pratyush Yadav wrote: > On 06/10/19 10:27PM, Johannes Schindelin wrote: > > > > On Mon, 7 Oct 2019, Pratyush Yadav wrote: > > > > > On 06/10/19 11:49AM, Johannes Schindelin wrote: > > > > > > > > On Sun, 6 Oct 2019, Pratyush Yadav wrote: > > > > > > > > > On 06/10/19 01:46AM, Harish Karumuthil wrote: > > > > > > > > > > > > From > > > > > > https://www.kernel.org/doc/html/v4.10/process/email-clients.html, > > > > > > I understood that, my current email client ( that is gmail > > > > > > web ) is not good for submitting patches. So I was tying to > > > > > > setup a mail client which is compatible with `git > > > > > > send-mail`. But I was not able to get a satisfactory result > > > > > > in that. > > > > > > > > > > You don't need to "set up" an email client with > > > > > git-send-email. git-send-email is an email client itself. > > > > > Well, one which can only send emails. > > > > > > > > It also cannot reply to mails on the mailing list. > > > > > > > > It cannot even notify you when anybody replied to your patch. > > > > > > > > Two rather problematic aspects when it comes to patch > > > > contributions: how are you supposed to work with the reviewers > > > > when you lack all the tools to _interact_ with them? All `git > > > > send-email` provides is a "fire and forget" way to send patches, > > > > i.e. it encourages a monologue, when you want to start a > > > > dialogue instead. > > > > > > Well, I started with email based patch contribution when I was > > > first got started with open source, so I might be a bit biased, > > > but in my experience, it is not that difficult to set all these > > > things up. Most of the time, all you need to tell git-send-email > > > is your SMTP server, username, and password. All pretty easy > > > things to do. > > > > Okay, set it up with a corporate Exchange server. > > > > I'll be waiting right here. > > I admit, I've never had to do that. And by how you word it, hope I > never have to do it in the future either. And I hope that you make peace with the fact that you prevent any corporate developer from contributing easily. > > > And you add in your email client (which pretty much everyone > > > should have), and it is a complete setup. I personally use neomutt > > > as my email client, but I have used Thunderbird before, and it is > > > really easy to set it up to send plain text emails. All you need > > > to do is hold Shift before hitting reply, and you're in plain text > > > mode. And you can even make it use plain text by default by > > > flipping a switch in the settings. > > > > How intuitive. And of course Thunderbird still messes up the patches > > so that they won't apply, unless you *checks notes* do things that > > are quite involved or *checks notes* do other things that are quite > > involved. > > Ha! Made me chuckle. You got me there :). I suppose it isn't _that_ > simple and I'm just biased because I am so used to it. I assumed that you were comfortable with it, and a bit oblivious about the hurdle that this represents to the majority of potential contributors. Remember, one of the beautiful things GitHub has given the world _on top_ of Git is how easy one-off contributions are. > > > So while I agree with you that there is certainly a learning curve > > > involved, I don't think it is all too bad. But again, that is all my > > > personal opinion, and nothing based on facts or data. > > > > Let me provide you with some data, then. Granted, it's not necessarily > > all Git GUI, but it includes Git GUI patches, too: Git for Windows' > > contributions. > > > > As should be well-known, I try to follow Postel's Law when it comes to > > Git for Windows' patches: be lenient in the input, strict in the output. > > As such, I don't force contributors to use GitHub PRs (although that is > > certainly encouraged by virtue of Git for Windows' source code being > > hosted on GitHub), or send patches, or send pull requests to their own > > public repositories or bundles sent to the mailing list. I accept them > > all. At least that is the idea. > > > > I cannot tell you how many contributions came in via GitHub PRs. I can > > tell precisely you how many contributions were made _not_ using GitHub > > PRs. One one hand. Actually, on zero hands. > > > > So clearly, at least Git for Windows' contributors (including some who > > provided Git GUI patches) are much more comfortable with the PR workflow > > than with the mailing list-based workflow. > > I never said email is better that GitHub PRs. It isn't. My point was > that using email isn't _that_ hard. When I first did it, it maybe took > me 3-4 hours to figure everything out, and then I was set forever. I > carry around the same '.gitconfig' file to all my setups, and everything > "just works". > > So yes, GitHub PRs are certainly easier, but email wasn't too difficult > in my experience. But then I'm a kernel developer, so I'm a minority to > begin with. > > I suspect you've had this debate more than once, because you come in > guns blazing ;) I come in guns blazing because these obstacles that are put in the way of contributors are the opposite of what I consider inclusive and welcoming. The fact that I am blessed with a lot of privilege (which, let's face it, I did nothing to earn) does not mean that I want to discount those who do not have that privilege. I have the time to contribute to Open Source, which is a privilege. I have the education to do so, with is a privilege. I even have the time to struggle with a mailing list-based code contribution process, which is a privilege I imagine only preciously few people enjoy. So I work as hard as I can against obstacles that are essentially big "Keep Out" signs (or, if you will, a big middle finger) to contributors without these privileges. > > > [... talking about GitGitGadget...] > > > > > > One feature that would make it complete would be the ability to > > > reply to review comments. > > > > And how would that work, exactly? How to determine *which* email to > > respond to? *Which* person to reply to? *What* to quote? > > GGG already shows replies to the patches as a comment. Yes. (I know, I implemented this.) > On GitHub you can "Quote reply" a comment, which quotes the entire > comment just like your MUA would. On GitHub, you can also select part of the comment and press the `r` key, which results in the equivalent of what I am doing right here: quoting part of your mail and responding to just that part. You can also just reply without quoting. These are three ways to reply to comments on GitHub, and in my experience the rarest form is the full quote, the most common form is the "no quote" form. (Which makes sense because you already have everything in that UI, you don't need to quote unless you need to make a point about only a certain part of what you are replying to, and only if that point might be otherwise missed.) Something I also saw often enough is to accumulate quotes from multiple comments in the same reply. > Then you can write your reply there, and the last line would be > '/reply', which would make GGG send that email as a reply. You would > need to strip the first line from the reply because GGG starts the > reply with something like: > > > [On the Git mailing > > list](https://public-inbox.org/git/xmqq7e5l9zb1.fsf@gitster-ct.c.googlers.com), > > Junio C Hamano wrote ([reply to > > this](https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis)): > > GGG also adds 3 backticks before and after the reply content, so those > would need to be removed too. Apart from the problems to identify the correct mail to reply to (unless, as you suggested, the Message-ID is part of the quoted part by virtue of including that public-inbox URL), I think it would make it cumbersome to require the `/reply` command. Quite honestly, I would prefer it if GitGitGadget would simply send replies whenever it can figure out to who to send, and which Message-ID to reply to. > > > This would remove the need for an email client (almost) > > > completely. I have never written Typescript or used Azure > > > pipelines ever, but I can try tinkering around to see if I can > > > figure out how to do something like that. Unless, of course, you > > > or someone else is already doing it. If not, some pointers would > > > be appreciated. > > > > Feel free to give this challenge a try. > > The first challenge is learning Typescript :) I learned Typescript to implement GitGitGadget. It maybe took me 3-4 hours to figure everything out, and then I was set forever. :-P Ciao, Johannes ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: GitGUIGadget, was Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-07 9:20 ` GitGUIGadget, was " Johannes Schindelin @ 2019-10-07 10:43 ` Birger Skogeng Pedersen 2019-10-07 19:16 ` Alban Gruin 0 siblings, 1 reply; 29+ messages in thread From: Birger Skogeng Pedersen @ 2019-10-07 10:43 UTC (permalink / raw) To: Johannes Schindelin Cc: Pratyush Yadav, Harish Karumuthil, Git List, David Aguilar It seems this topic has kindof derailed(?). But I feel like voicing my opinion nonetheless. I have to say I absolutely agree with Johannes Schindelin here. I would really prefer a more modern way of contributing to this project (any software project, really) than using emails. Be that Github, Gitlab, bitbucket or whatever else. But no it's not *impossible* to learn how to send patches by email. But IMO it is a less efficient way to develop and maintain an open source project. If getting more people interested in contributing, there is no doubt in my mind that using Github would be better. My biggest gripe with not using Github is how to keep track of replies (comments) to a topic. I have to navigate through multiple emails to get and overview of what everyone has been saying, when it should just be a single collection of replies from everyone (like in a Github issue). Where each issue has their own thread, and they can be linked to each other or to PRs. Just felt like chiming in, Birger On Mon, Oct 7, 2019 at 11:21 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > > Hi Pratyush, > > On Mon, 7 Oct 2019, Pratyush Yadav wrote: > > > On 06/10/19 10:27PM, Johannes Schindelin wrote: > > > > > > On Mon, 7 Oct 2019, Pratyush Yadav wrote: > > > > > > > On 06/10/19 11:49AM, Johannes Schindelin wrote: > > > > > > > > > > On Sun, 6 Oct 2019, Pratyush Yadav wrote: > > > > > > > > > > > On 06/10/19 01:46AM, Harish Karumuthil wrote: > > > > > > > > > > > > > > From > > > > > > > https://www.kernel.org/doc/html/v4.10/process/email-clients.html, > > > > > > > I understood that, my current email client ( that is gmail > > > > > > > web ) is not good for submitting patches. So I was tying to > > > > > > > setup a mail client which is compatible with `git > > > > > > > send-mail`. But I was not able to get a satisfactory result > > > > > > > in that. > > > > > > > > > > > > You don't need to "set up" an email client with > > > > > > git-send-email. git-send-email is an email client itself. > > > > > > Well, one which can only send emails. > > > > > > > > > > It also cannot reply to mails on the mailing list. > > > > > > > > > > It cannot even notify you when anybody replied to your patch. > > > > > > > > > > Two rather problematic aspects when it comes to patch > > > > > contributions: how are you supposed to work with the reviewers > > > > > when you lack all the tools to _interact_ with them? All `git > > > > > send-email` provides is a "fire and forget" way to send patches, > > > > > i.e. it encourages a monologue, when you want to start a > > > > > dialogue instead. > > > > > > > > Well, I started with email based patch contribution when I was > > > > first got started with open source, so I might be a bit biased, > > > > but in my experience, it is not that difficult to set all these > > > > things up. Most of the time, all you need to tell git-send-email > > > > is your SMTP server, username, and password. All pretty easy > > > > things to do. > > > > > > Okay, set it up with a corporate Exchange server. > > > > > > I'll be waiting right here. > > > > I admit, I've never had to do that. And by how you word it, hope I > > never have to do it in the future either. > > And I hope that you make peace with the fact that you prevent any > corporate developer from contributing easily. > > > > > And you add in your email client (which pretty much everyone > > > > should have), and it is a complete setup. I personally use neomutt > > > > as my email client, but I have used Thunderbird before, and it is > > > > really easy to set it up to send plain text emails. All you need > > > > to do is hold Shift before hitting reply, and you're in plain text > > > > mode. And you can even make it use plain text by default by > > > > flipping a switch in the settings. > > > > > > How intuitive. And of course Thunderbird still messes up the patches > > > so that they won't apply, unless you *checks notes* do things that > > > are quite involved or *checks notes* do other things that are quite > > > involved. > > > > Ha! Made me chuckle. You got me there :). I suppose it isn't _that_ > > simple and I'm just biased because I am so used to it. > > I assumed that you were comfortable with it, and a bit oblivious about > the hurdle that this represents to the majority of potential > contributors. > > Remember, one of the beautiful things GitHub has given the world _on > top_ of Git is how easy one-off contributions are. > > > > > So while I agree with you that there is certainly a learning curve > > > > involved, I don't think it is all too bad. But again, that is all my > > > > personal opinion, and nothing based on facts or data. > > > > > > Let me provide you with some data, then. Granted, it's not necessarily > > > all Git GUI, but it includes Git GUI patches, too: Git for Windows' > > > contributions. > > > > > > As should be well-known, I try to follow Postel's Law when it comes to > > > Git for Windows' patches: be lenient in the input, strict in the output. > > > As such, I don't force contributors to use GitHub PRs (although that is > > > certainly encouraged by virtue of Git for Windows' source code being > > > hosted on GitHub), or send patches, or send pull requests to their own > > > public repositories or bundles sent to the mailing list. I accept them > > > all. At least that is the idea. > > > > > > I cannot tell you how many contributions came in via GitHub PRs. I can > > > tell precisely you how many contributions were made _not_ using GitHub > > > PRs. One one hand. Actually, on zero hands. > > > > > > So clearly, at least Git for Windows' contributors (including some who > > > provided Git GUI patches) are much more comfortable with the PR workflow > > > than with the mailing list-based workflow. > > > > I never said email is better that GitHub PRs. It isn't. My point was > > that using email isn't _that_ hard. When I first did it, it maybe took > > me 3-4 hours to figure everything out, and then I was set forever. I > > carry around the same '.gitconfig' file to all my setups, and everything > > "just works". > > > > So yes, GitHub PRs are certainly easier, but email wasn't too difficult > > in my experience. But then I'm a kernel developer, so I'm a minority to > > begin with. > > > > I suspect you've had this debate more than once, because you come in > > guns blazing ;) > > I come in guns blazing because these obstacles that are put in the way > of contributors are the opposite of what I consider inclusive and > welcoming. > > The fact that I am blessed with a lot of privilege (which, let's face > it, I did nothing to earn) does not mean that I want to discount those > who do not have that privilege. I have the time to contribute to Open > Source, which is a privilege. I have the education to do so, with is a > privilege. I even have the time to struggle with a mailing list-based > code contribution process, which is a privilege I imagine only > preciously few people enjoy. > > So I work as hard as I can against obstacles that are essentially big > "Keep Out" signs (or, if you will, a big middle finger) to contributors > without these privileges. > > > > > [... talking about GitGitGadget...] > > > > > > > > One feature that would make it complete would be the ability to > > > > reply to review comments. > > > > > > And how would that work, exactly? How to determine *which* email to > > > respond to? *Which* person to reply to? *What* to quote? > > > > GGG already shows replies to the patches as a comment. > > Yes. > > (I know, I implemented this.) > > > On GitHub you can "Quote reply" a comment, which quotes the entire > > comment just like your MUA would. > > On GitHub, you can also select part of the comment and press the `r` > key, which results in the equivalent of what I am doing right here: > quoting part of your mail and responding to just that part. > > You can also just reply without quoting. > > These are three ways to reply to comments on GitHub, and in my > experience the rarest form is the full quote, the most common form is > the "no quote" form. (Which makes sense because you already have > everything in that UI, you don't need to quote unless you need to make a > point about only a certain part of what you are replying to, and only if > that point might be otherwise missed.) > > Something I also saw often enough is to accumulate quotes from multiple > comments in the same reply. > > > Then you can write your reply there, and the last line would be > > '/reply', which would make GGG send that email as a reply. You would > > need to strip the first line from the reply because GGG starts the > > reply with something like: > > > > > [On the Git mailing > > > list](https://public-inbox.org/git/xmqq7e5l9zb1.fsf@gitster-ct.c.googlers.com), > > > Junio C Hamano wrote ([reply to > > > this](https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis)): > > > > GGG also adds 3 backticks before and after the reply content, so those > > would need to be removed too. > > Apart from the problems to identify the correct mail to reply to > (unless, as you suggested, the Message-ID is part of the quoted part by > virtue of including that public-inbox URL), I think it would make it > cumbersome to require the `/reply` command. Quite honestly, I would > prefer it if GitGitGadget would simply send replies whenever it can > figure out to who to send, and which Message-ID to reply to. > > > > > This would remove the need for an email client (almost) > > > > completely. I have never written Typescript or used Azure > > > > pipelines ever, but I can try tinkering around to see if I can > > > > figure out how to do something like that. Unless, of course, you > > > > or someone else is already doing it. If not, some pointers would > > > > be appreciated. > > > > > > Feel free to give this challenge a try. > > > > The first challenge is learning Typescript :) > > I learned Typescript to implement GitGitGadget. It maybe took me 3-4 > hours to figure everything out, and then I was set forever. :-P > > Ciao, > Johannes ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: GitGUIGadget, was Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-07 10:43 ` Birger Skogeng Pedersen @ 2019-10-07 19:16 ` Alban Gruin 0 siblings, 0 replies; 29+ messages in thread From: Alban Gruin @ 2019-10-07 19:16 UTC (permalink / raw) To: Birger Skogeng Pedersen, Johannes Schindelin Cc: Pratyush Yadav, Harish Karumuthil, Git List, David Aguilar Hi Birger, Le 07/10/2019 à 12:43, Birger Skogeng Pedersen a écrit : > It seems this topic has kindof derailed(?). But I feel like voicing my > opinion nonetheless. > > -%<- > > My biggest gripe with not using Github is how to keep track of replies > (comments) to a topic. I have to navigate through multiple emails to > get and overview of what everyone has been saying, when it should just > be a single collection of replies from everyone (like in a Github > issue). Where each issue has their own thread, and they can be linked > to each other or to PRs. > I don’t know which email client you use, but they usually have an option to sort messages by thread -- this is how I handle my git folder with Thunderbird; it takes one click to switch from plain view to threaded view, and two to isolate a specific thread. public-inbox.org also does this. Some don’t though, and gmail specifically has weird opinions on which message is part of which thread. On the other hand, I don’t really like non-threaded means of communication, and I am very confused as to why modern services neglect this feature -- there may be a good reason, I’m not an expert in human-computer interaction :). But personally, I find it very hard to follow issues on github (or even IRC logs, for that matter) when there is more than 2 people involved. > > Just felt like chiming in, > Birger > Cheers, Alban ^ permalink raw reply [flat|nested] 29+ messages in thread
* Making GitGitGadget's list -> PR comment mirroring bidirectional, was Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-06 21:06 ` Pratyush Yadav 2019-10-07 9:20 ` GitGUIGadget, was " Johannes Schindelin @ 2019-11-19 22:09 ` Johannes Schindelin 2019-11-20 14:22 ` Pratyush Yadav 1 sibling, 1 reply; 29+ messages in thread From: Johannes Schindelin @ 2019-11-19 22:09 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Harish Karumuthil, git, David Aguilar Hi Pratyush, On Mon, 7 Oct 2019, Pratyush Yadav wrote: > On 06/10/19 10:27PM, Johannes Schindelin wrote: > > Hi Pratyush, > > > > On Mon, 7 Oct 2019, Pratyush Yadav wrote: > > > > > Anyway, GitGitGadget solves a large part of the problem. It > > > eliminates the need for using git-send-email, and it even shows you > > > the replies received on the list. I honestly think it is a great > > > tool, and it gives people a very good alternative to using > > > git-send-email. > > > > GitGitGadget is just a workaround. Not even complete. Can't be > > complete, really. Because problems. It has much of the same problems > > of `git send-email`: it's a one-way conversation. Code is not > > discussed in the right context (which would be a worktree with the > > correct commit checked out). The transfer is lossy (email is designed > > for human-readable messages, not for transferring machine-readable > > serialized objects). Matching original commits and/or branches to the > > ones on the other side is tedious. Any interaction requires switching > > between many tools. Etc > > > > > One feature that would make it complete would be the ability to > > > reply to review comments. > > > > And how would that work, exactly? How to determine *which* email to > > respond to? *Which* person to reply to? *What* to quote? > > GGG already shows replies to the patches as a comment. On GitHub you can > "Quote reply" a comment, which quotes the entire comment just like your > MUA would. The option can be found by clicking the 3 dots on the top > right of a comment. > > Then you can write your reply there, and the last line would be > '/reply', which would make GGG send that email as a reply. You would > need to strip the first line from the reply because GGG starts the reply > with something like: > > > [On the Git mailing list](https://public-inbox.org/git/xmqq7e5l9zb1.fsf@gitster-ct.c.googlers.com), Junio C Hamano wrote ([reply to this](https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis)): > > GGG also adds 3 backticks before and after the reply content, so those > would need to be removed too. > > Does this sound like a sane solution? Here are two real life examples where an unsuspecting GitGitGadget user expected GitGitGadget to mirror replies _to_ the Git mailing list: https://github.com/gitgitgadget/git/pull/451#issuecomment-555044068 and https://github.com/gitgitgadget/git/pull/451#issuecomment-555077933 Neither of them include the line with the link. Just to throw a bit of real life into the discussion... Ciao, Dscho ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: Making GitGitGadget's list -> PR comment mirroring bidirectional, was Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-11-19 22:09 ` Making GitGitGadget's list -> PR comment mirroring bidirectional, " Johannes Schindelin @ 2019-11-20 14:22 ` Pratyush Yadav 0 siblings, 0 replies; 29+ messages in thread From: Pratyush Yadav @ 2019-11-20 14:22 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Harish Karumuthil, git, David Aguilar On 19/11/19 11:09PM, Johannes Schindelin wrote: > Hi Pratyush, > > On Mon, 7 Oct 2019, Pratyush Yadav wrote: > > > On 06/10/19 10:27PM, Johannes Schindelin wrote: > > > Hi Pratyush, > > > > > > On Mon, 7 Oct 2019, Pratyush Yadav wrote: > > > > > > > Anyway, GitGitGadget solves a large part of the problem. It > > > > eliminates the need for using git-send-email, and it even shows you > > > > the replies received on the list. I honestly think it is a great > > > > tool, and it gives people a very good alternative to using > > > > git-send-email. > > > > > > GitGitGadget is just a workaround. Not even complete. Can't be > > > complete, really. Because problems. It has much of the same problems > > > of `git send-email`: it's a one-way conversation. Code is not > > > discussed in the right context (which would be a worktree with the > > > correct commit checked out). The transfer is lossy (email is designed > > > for human-readable messages, not for transferring machine-readable > > > serialized objects). Matching original commits and/or branches to the > > > ones on the other side is tedious. Any interaction requires switching > > > between many tools. Etc > > > > > > > One feature that would make it complete would be the ability to > > > > reply to review comments. > > > > > > And how would that work, exactly? How to determine *which* email to > > > respond to? *Which* person to reply to? *What* to quote? > > > > GGG already shows replies to the patches as a comment. On GitHub you can > > "Quote reply" a comment, which quotes the entire comment just like your > > MUA would. The option can be found by clicking the 3 dots on the top > > right of a comment. > > > > Then you can write your reply there, and the last line would be > > '/reply', which would make GGG send that email as a reply. You would > > need to strip the first line from the reply because GGG starts the reply > > with something like: > > > > > [On the Git mailing list](https://public-inbox.org/git/xmqq7e5l9zb1.fsf@gitster-ct.c.googlers.com), Junio C Hamano wrote ([reply to this](https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis)): > > > > GGG also adds 3 backticks before and after the reply content, so those > > would need to be removed too. > > > > Does this sound like a sane solution? > > Here are two real life examples where an unsuspecting GitGitGadget user > expected GitGitGadget to mirror replies _to_ the Git mailing list: > > https://github.com/gitgitgadget/git/pull/451#issuecomment-555044068 and > https://github.com/gitgitgadget/git/pull/451#issuecomment-555077933 > > Neither of them include the line with the link. Correct. The fundamental problem we have is that GitHub's threads are "shallow"/"linear". You don't reply to a reply, you reply to the main thread, and your comment gets appended to the end of that list. In contrast, email based threads are "deep"/"tree-like". Here you can reply to a reply. So the comment model of GitHub is less information-rich than the email model. The piece of information missing is "which comment does this comment reply to". That information has to be obtained somehow, and the best bet are the users themselves (by not deleting the line with the link in their replies). We can give the instructions in the GGG welcome message, and hope the users read it. Frequent users and anyone who properly reads the instructions will probably manage to use this just fine. Those who don't, well they weren't sending replies to the list to begin with. So this will help people who don't want to open their email clients to send replies to the list, but won't help the uninformed ones. Certainly not ideal, but I think this might be the best we can do given the constraints. In the meantime, I notice that GGG does not advertise the fact that replies don't go on the list directly very well. Yes, its mentioned in the welcome message, but its not instantly obvious. So maybe making it clearer/more noticeable will help the issue. Another alternative might be to rely on heuristics like seeing how similar the quoted text in a reply is to the replies in the list. But I think this will cause more problems than help because users can cut un-necessary quoting and even edit them sometimes. If you can think of something clever that I can't, suggestions are welcome :) Anyway, I probably won't have much time to work on this feature for at least a couple more weeks. Maybe we'll learn more after the feature goes live. > Just to throw a bit of real life into the discussion... > > Ciao, > Dscho -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-06 20:27 ` Johannes Schindelin 2019-10-06 21:06 ` Pratyush Yadav @ 2019-10-06 22:40 ` Philip Oakley 1 sibling, 0 replies; 29+ messages in thread From: Philip Oakley @ 2019-10-06 22:40 UTC (permalink / raw) To: Johannes Schindelin, Pratyush Yadav; +Cc: Harish Karumuthil, git, David Aguilar Hi Dscho, On 06/10/2019 21:27, Johannes Schindelin wrote: > Let me provide you with some data, then. Granted, it's not necessarily > all Git GUI, but it includes Git GUI patches, too: Git for Windows' > contributions. > > As should be well-known, I try to follow Postel's Law when it comes to > Git for Windows' patches: be lenient in the input, strict in the output. > As such, I don't force contributors to use GitHub PRs (although that is > certainly encouraged by virtue of Git for Windows' source code being > hosted on GitHub), or send patches, or send pull requests to their own > public repositories or bundles sent to the mailing list. I accept them > all. At least that is the idea. > > I cannot tell you how many contributions came in via GitHub PRs. I can > tell precisely you how many contributions were made_not_ using GitHub > PRs. One one hand. Actually, on zero hands. > > So clearly, at least Git for Windows' contributors (including some who > provided Git GUI patches) are much more comfortable with the PR workflow > than with the mailing list-based workflow. Just to say that most of the numbers are governed by the strength and experience with the particular infrastructures. Tonight I had wanted to send in patches for G-f-W because of branch placement confusion. Eventually I had to throw in an extra rebase to a fresh branch, just so I could create a PR, all because of the zero experience you mentioned with using a G-f-W 2-patch series. The Git list is strongly patch based and it's infrastructure works adequately, even if it is 'antiquated' by millennial standards. The G-f-W interaction is almost totally via Github, and a few notification emails and occasional google-groups interactions. It's still imperfect, just like the Git list emails, but with it's own, different issues for trying to build its community. Most community bondings actually build through their common adversity, rather than the apparent ease of joining/leaving. The main bit I wanted to say (I think), was that having a maintainer who accepts input is probably the most important aspect, no matter the particular route used for the input. So thanks to both of you (Dscho, Pratyush) for *facilitating* the contribution flow. -- Philip ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-06 9:49 ` Johannes Schindelin 2019-10-06 18:39 ` Pratyush Yadav @ 2019-10-07 6:22 ` Harish Karumuthil 2019-10-07 10:01 ` Johannes Schindelin 1 sibling, 1 reply; 29+ messages in thread From: Harish Karumuthil @ 2019-10-07 6:22 UTC (permalink / raw) To: Johannes Schindelin, Pratyush Yadav; +Cc: git, David Aguilar Hi Johannes Schindelin, Regarding your messages, > However, it might not even need to put in _such_ a lot of work: in my > tests, `Control-,` worked just as well as `Control-comma`. To test this > for yourself, use this snippet (that is slightly modified from the > example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so > that it reacts _only_ to Control+comma instead of all keys): > > -- snip -- > set keysym "Press any key" > pack [label .l -textvariable keysym -padx 2m -pady 1m] > #bind . <Key> { > bind . <Control-,> { > set keysym "You pressed %K" > } > -- snap -- I tried this, but unfortunatly, it didn't worked for me. My tclsh version is "8.6". The script crashed with following error message --- bad event type or keysym "," while executing "bind . <Control-,> { set keysym "You pressed %K" }" (file "./test.tcl" line 6) --- The complete ( or modified ) script which I used is given below --- package require Tk set keysym "Press any key" pack [label .l -textvariable keysym -padx 2m -pady 1m] #bind . <Key> { bind . <Control-,> { set keysym "You pressed %K" } --- From the error messages, I understand that, "<Control-,>" will not work instead of "<Control-comma>" . > > So I could imagine that something like this could serve as an initial > draft for a function that you can turn into a "good enough" version: > > -- snip -- > proc QKeySequence2keysym {keystroke} { > regsub -all {(?i)Ctrl\+} $keystroke "Control-" keystroke > regsub -all {(?i)Alt\+} $keystroke "Alt-" keystroke > regsub -all {(?i)Shift\+} $keystroke "Shift-" keystroke > return $keystroke > } > -- snap -- > > That way, you don't have to introduce settings separate from > `git-cola`'s, and you can reuse the short-and-sweet variable name. If my previous observation is correct, then we may have to translate a list of key names ( in addition to atl,ctrl & shirt ) to get it working . > Ciao, > Johannes ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-07 6:22 ` Harish Karumuthil @ 2019-10-07 10:01 ` Johannes Schindelin 2019-10-08 19:31 ` Harish Karumuthil 0 siblings, 1 reply; 29+ messages in thread From: Johannes Schindelin @ 2019-10-07 10:01 UTC (permalink / raw) To: Harish Karumuthil; +Cc: Pratyush Yadav, git, David Aguilar Hi Harish, On Mon, 7 Oct 2019, Harish Karumuthil wrote: > > However, it might not even need to put in _such_ a lot of work: in my > > tests, `Control-,` worked just as well as `Control-comma`. To test this > > for yourself, use this snippet (that is slightly modified from the > > example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so > > that it reacts _only_ to Control+comma instead of all keys): > > > > -- snip -- > > set keysym "Press any key" > > pack [label .l -textvariable keysym -padx 2m -pady 1m] > > #bind . <Key> { > > bind . <Control-,> { > > set keysym "You pressed %K" > > } > > -- snap -- > > I tried this, but unfortunatly, it didn't worked for me. My tclsh version is > "8.6". The script crashed with following error message > > --- > bad event type or keysym "," > while executing > "bind . <Control-,> { > set keysym "You pressed %K" > }" > (file "./test.tcl" line 6) > --- That's too bad! I tested this on Windows, and I imagine it just does not work on Linux/macOS... > The complete ( or modified ) script which I used is given below > > --- > package require Tk > > set keysym "Press any key" > pack [label .l -textvariable keysym -padx 2m -pady 1m] > #bind . <Key> { > bind . <Control-,> { > set keysym "You pressed %K" > } > > --- > > From the error messages, I understand that, "<Control-,>" will not work > instead of "<Control-comma>" . > > > > > So I could imagine that something like this could serve as an initial > > draft for a function that you can turn into a "good enough" version: > > > > -- snip -- > > proc QKeySequence2keysym {keystroke} { > > regsub -all {(?i)Ctrl\+} $keystroke "Control-" keystroke > > regsub -all {(?i)Alt\+} $keystroke "Alt-" keystroke > > regsub -all {(?i)Shift\+} $keystroke "Shift-" keystroke > > return $keystroke > > } > > -- snap -- > > > > That way, you don't have to introduce settings separate from > > `git-cola`'s, and you can reuse the short-and-sweet variable name. > > If my previous observation is correct, then we may have to translate a list > of key names ( in addition to atl,ctrl & shirt ) to get it working . I fear you're right. Hopefully we can get away with a relatively short list... Ciao, Johannes ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-07 10:01 ` Johannes Schindelin @ 2019-10-08 19:31 ` Harish Karumuthil 2019-10-09 20:42 ` Johannes Schindelin 2019-10-13 20:09 ` Pratyush Yadav 0 siblings, 2 replies; 29+ messages in thread From: Harish Karumuthil @ 2019-10-08 19:31 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Pratyush Yadav, git, David Aguilar Hi all, there is an update: I added necessary error catching code so that, script will not crash if the keybinding code is worng. Instead of crashing it will print error message. The final patch will look something like this. --- lib/tools.tcl | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/tools.tcl b/lib/tools.tcl index 413f1a1700..3135e19131 100644 --- a/lib/tools.tcl +++ b/lib/tools.tcl @@ -38,7 +38,7 @@ proc tools_create_item {parent args} { } proc tools_populate_one {fullname} { - global tools_menubar tools_menutbl tools_id + global tools_menubar tools_menutbl tools_id repo_config if {![info exists tools_id]} { set tools_id 0 @@ -61,9 +61,25 @@ proc tools_populate_one {fullname} { } } - tools_create_item $parent command \ - -label [lindex $names end] \ - -command [list tools_exec $fullname] + set accel_key_bound 0 + if {[info exists repo_config(guitool.$fullname.gitgui-shortcut)]} { + set accel_key $repo_config(guitool.$fullname.gitgui-shortcut) + if { [ catch { bind . <$accel_key> [list tools_exec $fullname] } msg ] } { + puts stderr "Failed to bind keyboard shortcut '$accel_key' for custom tool '$fullname'. Error: $msg" + } else { + tools_create_item $parent command \ + -label [lindex $names end] \ + -command [list tools_exec $fullname] \ + -accelerator $accel_key + set accel_key_bound true + } + } + + if { ! $accel_key_bound } { + tools_create_item $parent command \ + -label [lindex $names end] \ + -command [list tools_exec $fullname] + } } proc tools_exec {fullname} { --- @Johannes Schindelin: In short, from your previous message I understand point. 1. shortcut codes like "<Control-,>" will only in Windows platform. It may not work in Linux / Mac. 2. We need do translate shortcut codes somehow ( using one-to-one maping ). If this is correct, do you have any example on how to do one-to-one maping of a list of string on TCL ? ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-08 19:31 ` Harish Karumuthil @ 2019-10-09 20:42 ` Johannes Schindelin 2019-10-13 20:09 ` Pratyush Yadav 1 sibling, 0 replies; 29+ messages in thread From: Johannes Schindelin @ 2019-10-09 20:42 UTC (permalink / raw) To: Harish Karumuthil; +Cc: Pratyush Yadav, git, David Aguilar Hi, On Wed, 9 Oct 2019, Harish Karumuthil wrote: > @Johannes Schindelin: In short, from your previous message I understand point. > > 1. shortcut codes like "<Control-,>" will only in Windows platform. It may not work in Linux / Mac. > 2. We need do translate shortcut codes somehow ( using one-to-one maping ). > > If this is correct, do you have any example on how to do one-to-one maping of a list of string on TCL ? This took much longer to find than I expected, probably my web search fu is deserting me. But I found something: `string map`, see https://tcl.tk/man/tcl8.6/TclCmd/string.htm#M34 for details. Ciao, Johannes ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-08 19:31 ` Harish Karumuthil 2019-10-09 20:42 ` Johannes Schindelin @ 2019-10-13 20:09 ` Pratyush Yadav 1 sibling, 0 replies; 29+ messages in thread From: Pratyush Yadav @ 2019-10-13 20:09 UTC (permalink / raw) To: Harish Karumuthil; +Cc: Johannes Schindelin, git, David Aguilar On 09/10/19 01:01AM, Harish Karumuthil wrote: > Hi all, there is an update: > > I added necessary error catching code so that, script will not crash if the > keybinding code is worng. Instead of crashing it will print error message. > The final patch will look something like this. Like I mentioned another reply I wrote just now, this unfortunately is not a "proper" patch because it does not contain the subject and commit message of your commit. Just the diff is not enough, and needs the commit subject and message too. And even for just "preview" patches, I would still recommend sending a proper patch so people can also peek at the commit message too. You can pass '-rfc' to `git-format-patch` to have something like '[RFC PATCH]' in your email subject so people know it is a preview. "RFC" stands for "Request For Comments". Some comments below. > --- > lib/tools.tcl | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/lib/tools.tcl b/lib/tools.tcl > index 413f1a1700..3135e19131 100644 > --- a/lib/tools.tcl > +++ b/lib/tools.tcl > @@ -38,7 +38,7 @@ proc tools_create_item {parent args} { > } > > proc tools_populate_one {fullname} { > - global tools_menubar tools_menutbl tools_id > + global tools_menubar tools_menutbl tools_id repo_config > > if {![info exists tools_id]} { > set tools_id 0 > @@ -61,9 +61,25 @@ proc tools_populate_one {fullname} { > } > } > > - tools_create_item $parent command \ > - -label [lindex $names end] \ > - -command [list tools_exec $fullname] > + set accel_key_bound 0 > + if {[info exists repo_config(guitool.$fullname.gitgui-shortcut)]} { > + set accel_key $repo_config(guitool.$fullname.gitgui-shortcut) > + if { [ catch { bind . <$accel_key> [list tools_exec $fullname] } msg ] } { This has inconsistent style. There should not be any spaces between '{' and '['. So this line should look something like: if {[catch {bind . <$accel_key> [list tools_exec $fullname]} msg]} { You can look at the code around yours to pick up on the general style. > + puts stderr "Failed to bind keyboard shortcut '$accel_key' for custom tool '$fullname'. Error: $msg" Putting the error message of a GUI application on stderr is probably not a good idea. Firstly, since it is a GUI application, we should show error messages in the GUI. And secondly, a lot of the time, people probably don't even launch git-gui from a terminal command, and do it via some application launcher. In that case, there is no stderr that the user can easily read from. So please use a popup dialog instead. Functions to easily create them can be found in lib/error.tcl. I'd recommend either `warn_popup` or `error_popup`. As an example, this is what I got when I added a bad shortcut: Failed to bind keyboard shortcut 'Ctrl-Z' for custom tool 'Foo'. Error: bad event type or keysym "Ctrl" Showing the error message from `bind` is pretty neat! The user can know _exactly_ what's wrong. One problem is that this might not make that much sense to a non-Tcler. But I still think giving a hint of the why it failed is a good idea. I'd like to hear other people's thoughts about it though. > + } else { > + tools_create_item $parent command \ > + -label [lindex $names end] \ > + -command [list tools_exec $fullname] \ > + -accelerator $accel_key > + set accel_key_bound true Above you set `accel_key_bound` to '0', and here you set it to 'true'. Please use consistent forms of a boolean. Either use 'true' and 'false', or use '0' and '1'. > + } > + } > + > + if { ! $accel_key_bound } { Same style nitpick about the spaces as above. > + tools_create_item $parent command \ > + -label [lindex $names end] \ > + -command [list tools_exec $fullname] > + } > } Can your whole logic of setting an accelerator in case a shortcut exists be simplified a bit? Right now, the tools creation command is executed in two places, and it is not obvious at first sight that only one of them will ever be executed. So maybe something like: ... if {[catch {bind . <$accel_key> ...} { puts stderr ... set accel_key_bound false } else { set accel_key_bound true } if {accel_key_bound} { # Create tool with accelerator ... } else { # Create tool without accelerator ... } I hope you get what this means, but if you don't, please let me know, and I'll clarify. Overall, I like the idea of the patch. This would move us one step in the direction of customizable keybindings for _all_ shortcuts. Thanks. > > proc tools_exec {fullname} { > --- > > @Johannes Schindelin: In short, from your previous message I understand point. > > 1. shortcut codes like "<Control-,>" will only in Windows platform. It may not work in Linux / Mac. > 2. We need do translate shortcut codes somehow ( using one-to-one maping ). > > If this is correct, do you have any example on how to do one-to-one maping of a list of string on TCL ? -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-05 21:01 ` Pratyush Yadav 2019-10-06 9:49 ` Johannes Schindelin @ 2019-10-07 6:13 ` Harish Karumuthil 2019-10-13 19:17 ` Pratyush Yadav 1 sibling, 1 reply; 29+ messages in thread From: Harish Karumuthil @ 2019-10-07 6:13 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Johannes Schindelin, git, David Aguilar Hi Pratyush, Regarding your messages, >On Sun, 2019-10-06 at 02:31 +0530, Pratyush Yadav wrote: > You don't need to "set up" an email client with git-send-email. > git-send-email is an email client itself. Well, one which can only send > emails. For now, I am sticking with a mail client ( evolution ) which does minimal ( or atleast transparent ) preprocessing ( Tab => space conversion , line wrapping etc ). Now I can send patches using the output of `git diff --patch-with-stat` command and I hope is it enough for now. Personaly I dont' like any solution which requires storing our mail password as a plain text file. > You haven't sent '/submit' over there, so those emails aren't in the > list (and my inbox) yet. You need to comment with '/submit' (without the > quotes) to tell GitGitGadget to send your PR as email. I thought, lets finalize discussion about all the changes here in mail thread it self before submitting the patch. Otherwise, That is why I didn't submitted the patch. > One point I forgot to mention earlier was that I'm honestly not a big > fan of separating the binding and accelerator label. I understand that > you might not have the time to do this, but I think it is still worth > mentioning. Maybe I will implement something like that over your patch. > But it would certainly be nice if you can figure it out :). I think there is a small missunderstanind in that point. I agree that, in the initial implementation ( which I did @ 2016 ) menu labels were separated from binding keys. But in the last update, it is not like that. Currently, user only need to specify single config value which is `guitool.<name>.gitgui-shortcut` and don't have to specify accel-lable separatly. Label is generated from the shortcut. > Either ways, detecting an existing shortcut is pretty easy. The `bind` > man page [1] says: > > If sequence is specified without a script, then the script currently > bound to sequence is returned, or an empty string is returned if there > is no binding for sequence. > > So you can use this to find out if there is a binding conflict, and warn > the user. Will try this. Thanks! ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2019-10-07 6:13 ` Harish Karumuthil @ 2019-10-13 19:17 ` Pratyush Yadav 0 siblings, 0 replies; 29+ messages in thread From: Pratyush Yadav @ 2019-10-13 19:17 UTC (permalink / raw) To: Harish Karumuthil; +Cc: Johannes Schindelin, git, David Aguilar Hi Harish, Sorry for the late reply. Couldn't find much time last few days. On 07/10/19 11:43AM, Harish Karumuthil wrote: > Hi Pratyush, Regarding your messages, > > >On Sun, 2019-10-06 at 02:31 +0530, Pratyush Yadav wrote: > > You don't need to "set up" an email client with git-send-email. > > git-send-email is an email client itself. Well, one which can only send > > emails. > > For now, I am sticking with a mail client ( evolution ) which does minimal > ( or atleast transparent ) preprocessing ( Tab => space conversion , line > wrapping etc ). > Now I can send patches using the output of `git diff --patch-with-stat` > command and I hope is it enough for now. > Personaly I dont' like any solution which requires storing our mail password > as a plain text file. I'm afraid this won't work. The '.patch' file that `git-format-patch` generates also contains your commit message and the author information. All those are needed to properly convert your patch to a commit in my repo. The output of `git diff --patch-with-stat` won't be enough. As for not wanting to store your mail password in a plain text file, check out [0]. And then there is GitGitGadget too, which I'd recommend since you seem to be having trouble sending patches directly :). > > You haven't sent '/submit' over there, so those emails aren't in the > > list (and my inbox) yet. You need to comment with '/submit' (without > > the quotes) to tell GitGitGadget to send your PR as email. > > I thought, lets finalize discussion about all the changes here in mail > thread it self before submitting the patch. Otherwise, That is why I didn't > submitted the patch. Makes sense. > > One point I forgot to mention earlier was that I'm honestly not a big > > fan of separating the binding and accelerator label. I understand that > > you might not have the time to do this, but I think it is still worth > > mentioning. Maybe I will implement something like that over your patch. > > But it would certainly be nice if you can figure it out :). > > I think there is a small missunderstanind in that point. > > I agree that, in the initial implementation ( which I did @ 2016 ) menu > labels were separated from binding keys. But in the last update, it is not > like that. > > Currently, user only need to specify single config value which is > `guitool.<name>.gitgui-shortcut` and don't have to specify accel-lable > separatly. > Label is generated from the shortcut. Thanks for clarifying. It indeed was a misunderstanding. > > Either ways, detecting an existing shortcut is pretty easy. The `bind` > > man page [1] says: > > > > If sequence is specified without a script, then the script currently > > bound to sequence is returned, or an empty string is returned if there > > is no binding for sequence. > > > > So you can use this to find out if there is a binding conflict, and warn > > the user. > > Will try this. Thanks! [0] https://www.softwaredeveloper.blog/git-credential-storage-libsecret -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts @ 2016-03-29 11:29 Harish K 2016-03-31 16:49 ` David Aguilar 0 siblings, 1 reply; 29+ messages in thread From: Harish K @ 2016-03-29 11:29 UTC (permalink / raw) To: git --- git-gui/lib/tools.tcl | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl index 6ec9411..749bc67 100644 --- a/git-gui/lib/tools.tcl +++ b/git-gui/lib/tools.tcl @@ -38,7 +38,7 @@ proc tools_create_item {parent args} { } proc tools_populate_one {fullname} { - global tools_menubar tools_menutbl tools_id + global tools_menubar tools_menutbl tools_id repo_config if {![info exists tools_id]} { set tools_id 0 @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} { } } - tools_create_item $parent command \ + if {[info exists repo_config(guitool.$fullname.accelerator)] && [info exists repo_config(guitool.$fullname.accelerator-label)]} { + set accele_key $repo_config(guitool.$fullname.accelerator) + set accel_label $repo_config(guitool.$fullname.accelerator-label) + tools_create_item $parent command \ -label [lindex $names end] \ - -command [list tools_exec $fullname] + -command [list tools_exec $fullname] \ + -accelerator $accel_label + bind . $accele_key [list tools_exec $fullname] + } else { + tools_create_item $parent command \ + -label [lindex $names end] \ + -command [list tools_exec $fullname] + } } proc tools_exec {fullname} { -- https://github.com/git/git/pull/220 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts 2016-03-29 11:29 Harish K @ 2016-03-31 16:49 ` David Aguilar 0 siblings, 0 replies; 29+ messages in thread From: David Aguilar @ 2016-03-31 16:49 UTC (permalink / raw) To: Harish K; +Cc: git On Tue, Mar 29, 2016 at 11:29:41AM +0000, Harish K wrote: > --- > git-gui/lib/tools.tcl | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) I forgot to mention that git-gui has its own repository. The git project merges the upstream repo as a subtree into its git-gui directory. You should probably clone the git-gui repository and create a patch there so that it can be applied to the upstream repo: http://repo.or.cz/w/git-gui.git -- David ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2019-11-20 14:22 UTC | newest] Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-29 11:38 [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts Harish K 2016-03-31 16:41 ` David Aguilar 2016-04-01 6:32 ` harish k 2019-10-03 14:48 ` harish k 2019-10-03 21:44 ` Pratyush Yadav 2019-10-04 8:49 ` Johannes Schindelin 2019-10-04 12:01 ` Pratyush Yadav 2019-10-05 20:16 ` Harish Karumuthil 2019-10-05 21:01 ` Pratyush Yadav 2019-10-06 9:49 ` Johannes Schindelin 2019-10-06 18:39 ` Pratyush Yadav 2019-10-06 19:37 ` Philip Oakley 2019-10-06 20:27 ` Johannes Schindelin 2019-10-06 21:06 ` Pratyush Yadav 2019-10-07 9:20 ` GitGUIGadget, was " Johannes Schindelin 2019-10-07 10:43 ` Birger Skogeng Pedersen 2019-10-07 19:16 ` Alban Gruin 2019-11-19 22:09 ` Making GitGitGadget's list -> PR comment mirroring bidirectional, " Johannes Schindelin 2019-11-20 14:22 ` Pratyush Yadav 2019-10-06 22:40 ` Philip Oakley 2019-10-07 6:22 ` Harish Karumuthil 2019-10-07 10:01 ` Johannes Schindelin 2019-10-08 19:31 ` Harish Karumuthil 2019-10-09 20:42 ` Johannes Schindelin 2019-10-13 20:09 ` Pratyush Yadav 2019-10-07 6:13 ` Harish Karumuthil 2019-10-13 19:17 ` Pratyush Yadav -- strict thread matches above, loose matches on Subject: below -- 2016-03-29 11:29 Harish K 2016-03-31 16:49 ` David Aguilar
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).