{"thread":{"id":"41857","subject":"[PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","startedAt":"2016-03-29T11:38:10Z","lastAt":"2019-11-20T14:22:47Z","messageCount":27,"participants":["Harish K","David Aguilar","harish k","Pratyush Yadav","Johannes Schindelin","Harish Karumuthil","Philip Oakley","Birger Skogeng Pedersen","Alban Gruin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"282041","messageId":"01020153c22ab06b-e195b148-37cc-4f89-92f3-f4bed1915eb9-000000@eu-west-1.amazonses.com","threadId":"41857","inReplyTo":null,"subject":"[PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Harish K","fromEmail":"harish2704@gmail.com","sentAt":"2016-03-29T11:38:10Z","receivedAt":"2016-03-29T11:38:10Z","isPatch":true,"sender":{"key":"harish2704@gmail.com","avatar":"https://gravatar.com/avatar/774092af5a4df0d7d135b48fcbe03df5b331c434506ea37eb31b3cddfb17458f?d=mp&s=160"},"body":"---\n git-gui/lib/tools.tcl | 16 +++++++++++++---\n 1 file changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl\nindex 6ec9411..749bc67 100644\n--- a/git-gui/lib/tools.tcl\n+++ b/git-gui/lib/tools.tcl\n@@ -38,7 +38,7 @@ proc tools_create_item {parent args} {\n }\n \n proc tools_populate_one {fullname} {\n-\tglobal tools_menubar tools_menutbl tools_id\n+\tglobal tools_menubar tools_menutbl tools_id repo_config\n \n \tif {![info exists tools_id]} {\n \t\tset tools_id 0\n@@ -61,9 +61,19 @@ proc tools_populate_one {fullname} {\n \t\t}\n \t}\n \n-\ttools_create_item $parent command \\\n+\tif {[info exists repo_config(guitool.$fullname.accelerator)] && [info exists repo_config(guitool.$fullname.accelerator-label)]} {\n+\t\tset accele_key $repo_config(guitool.$fullname.accelerator)\n+\t\tset accel_label $repo_config(guitool.$fullname.accelerator-label)\n+\t\ttools_create_item $parent command \\\n \t\t-label [lindex $names end] \\\n-\t\t-command [list tools_exec $fullname]\n+\t\t-command [list tools_exec $fullname] \\\n+\t\t-accelerator $accel_label\n+\t\tbind . $accele_key [list tools_exec $fullname]\n+\t} else {\n+\t\ttools_create_item $parent command \\\n+\t\t\t-label [lindex $names end] \\\n+\t\t\t-command [list tools_exec $fullname]\n+\t}\n }\n \n proc tools_exec {fullname} {\n\n--\nhttps://github.com/git/git/pull/220\n"},{"id":"282338","messageId":"20160331164137.GA11150@gmail.com","threadId":"41857","inReplyTo":"01020153c22ab06b-e195b148-37cc-4f89-92f3-f4bed1915eb9-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2016-03-31T16:41:37Z","receivedAt":"2016-03-31T16:41:37Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Hello,\n\nOn Tue, Mar 29, 2016 at 11:38:10AM +0000, Harish K wrote:\n> ---\n>  git-gui/lib/tools.tcl | 16 +++++++++++++---\n>  1 file changed, 13 insertions(+), 3 deletions(-)\n> \n> diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl\n> index 6ec9411..749bc67 100644\n> --- a/git-gui/lib/tools.tcl\n> +++ b/git-gui/lib/tools.tcl\n> @@ -38,7 +38,7 @@ proc tools_create_item {parent args} {\n>  }\n>  \n>  proc tools_populate_one {fullname} {\n> -\tglobal tools_menubar tools_menutbl tools_id\n> +\tglobal tools_menubar tools_menutbl tools_id repo_config\n>  \n>  \tif {![info exists tools_id]} {\n>  \t\tset tools_id 0\n> @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} {\n>  \t\t}\n>  \t}\n>  \n> -\ttools_create_item $parent command \\\n> +\tif {[info exists repo_config(guitool.$fullname.accelerator)] && [info exists repo_config(guitool.$fullname.accelerator-label)]} {\n> +\t\tset accele_key $repo_config(guitool.$fullname.accelerator)\n> +\t\tset accel_label $repo_config(guitool.$fullname.accelerator-label)\n> +\t\ttools_create_item $parent command \\\n>  \t\t-label [lindex $names end] \\\n> -\t\t-command [list tools_exec $fullname]\n> +\t\t-command [list tools_exec $fullname] \\\n> +\t\t-accelerator $accel_label\n> +\t\tbind . $accele_key [list tools_exec $fullname]\n> +\t} else {\n> +\t\ttools_create_item $parent command \\\n> +\t\t\t-label [lindex $names end] \\\n> +\t\t\t-command [list tools_exec $fullname]\n> +\t}\n>  }\n>  \n>  proc tools_exec {fullname} {\n> \n> --\n> https://github.com/git/git/pull/220\n\nWe also support \"custom guitools\" in git-cola using this same\nmechanism.  If this gets accepted then we'll want to make\nsimilar change there.\n\nThere's always a small risk that user-defined tools can conflict\nwith builtin shortcuts, but otherwise this seems like a pretty\nnice feature.  Curious, what is the behavior in the event of a\nconflict?  Do the builtins win?  IIRC, Qt handles this by\ndisabling the shortcut and warning that it's ambiguous.\n\nPlease documentation guitool.<name>.accellerator[-label] in\nDocumentation/config.txt otherwise users will not know that it\nexists.\n\nIt would also be good for the docs to clarify what the\naccelerators look like in case we need to munge them when making\nit work in cola via Qt, which has its own mechanism for\nassociating actions with shortcuts.  Documented examples with\none and two modifier keys would be helpful.\n\n\ncheers,\n-- \nDavid\n"},{"id":"282432","messageId":"CACV9s2MFiikZWq=s8kYQ+qwidQ=oO-SHyKWAs4MUkNcgDhJzeg@mail.gmail.com","threadId":"41857","inReplyTo":"20160331164137.GA11150@gmail.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"harish k","fromEmail":"harish2704@gmail.com","sentAt":"2016-04-01T06:32:02Z","receivedAt":"2016-04-01T06:32:02Z","isPatch":true,"sender":{"key":"harish2704@gmail.com","avatar":"https://gravatar.com/avatar/774092af5a4df0d7d135b48fcbe03df5b331c434506ea37eb31b3cddfb17458f?d=mp&s=160"},"body":"Hi David,\n\nActually Im a TCL primer.  This is the first time Im dealing with.\nThat is why I kept it simple ( ie both accel-key and accel-label need\nto be defined in config ).\n\nI think, git-cola is using Qt style representation accel-key in the config file.\n\nIs git-cola is an official tool from git ? like git-gui?\nif not ,\nMy suggesion is, it is better to seperate config fields according to\napplication-domain\nlike, \"git-gui-accel = <Ctrl-l>\" etc..\nOther vise there is good chance for conflicts. ( Eg: consider the case\n that, <Ctrl-p> was assined to a custom tool by git-cola )\n\nCurrently this patch will not handle any conflicting shortcuts. I\nthink custom shortcuts will overwrite the other.\n\n\nOn Thu, Mar 31, 2016 at 10:11 PM, David Aguilar <davvid@gmail.com> wrote:\n> Hello,\n>\n> On Tue, Mar 29, 2016 at 11:38:10AM +0000, Harish K wrote:\n>> ---\n>>  git-gui/lib/tools.tcl | 16 +++++++++++++---\n>>  1 file changed, 13 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl\n>> index 6ec9411..749bc67 100644\n>> --- a/git-gui/lib/tools.tcl\n>> +++ b/git-gui/lib/tools.tcl\n>> @@ -38,7 +38,7 @@ proc tools_create_item {parent args} {\n>>  }\n>>\n>>  proc tools_populate_one {fullname} {\n>> -     global tools_menubar tools_menutbl tools_id\n>> +     global tools_menubar tools_menutbl tools_id repo_config\n>>\n>>       if {![info exists tools_id]} {\n>>               set tools_id 0\n>> @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} {\n>>               }\n>>       }\n>>\n>> -     tools_create_item $parent command \\\n>> +     if {[info exists repo_config(guitool.$fullname.accelerator)] && [info exists repo_config(guitool.$fullname.accelerator-label)]} {\n>> +             set accele_key $repo_config(guitool.$fullname.accelerator)\n>> +             set accel_label $repo_config(guitool.$fullname.accelerator-label)\n>> +             tools_create_item $parent command \\\n>>               -label [lindex $names end] \\\n>> -             -command [list tools_exec $fullname]\n>> +             -command [list tools_exec $fullname] \\\n>> +             -accelerator $accel_label\n>> +             bind . $accele_key [list tools_exec $fullname]\n>> +     } else {\n>> +             tools_create_item $parent command \\\n>> +                     -label [lindex $names end] \\\n>> +                     -command [list tools_exec $fullname]\n>> +     }\n>>  }\n>>\n>>  proc tools_exec {fullname} {\n>>\n>> --\n>> https://github.com/git/git/pull/220\n>\n> We also support \"custom guitools\" in git-cola using this same\n> mechanism.  If this gets accepted then we'll want to make\n> similar change there.\n>\n> There's always a small risk that user-defined tools can conflict\n> with builtin shortcuts, but otherwise this seems like a pretty\n> nice feature.  Curious, what is the behavior in the event of a\n> conflict?  Do the builtins win?  IIRC, Qt handles this by\n> disabling the shortcut and warning that it's ambiguous.\n>\n> Please documentation guitool.<name>.accellerator[-label] in\n> Documentation/config.txt otherwise users will not know that it\n> exists.\n>\n> It would also be good for the docs to clarify what the\n> accelerators look like in case we need to munge them when making\n> it work in cola via Qt, which has its own mechanism for\n> associating actions with shortcuts.  Documented examples with\n> one and two modifier keys would be helpful.\n>\n>\n> cheers,\n> --\n> David\n\n\n\n-- \n\n-Regards\nHarish.K\n"},{"id":"383332","messageId":"CACV9s2MQCP04QASgt0xhi3cSNPSKjwXTufxmZQXAUNvnWD9DSw@mail.gmail.com","threadId":"41857","inReplyTo":"CACV9s2MFiikZWq=s8kYQ+qwidQ=oO-SHyKWAs4MUkNcgDhJzeg@mail.gmail.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"harish k","fromEmail":"harish2704@gmail.com","sentAt":"2019-10-03T14:48:06Z","receivedAt":"2019-10-03T14:48:22Z","isPatch":true,"sender":{"key":"harish2704@gmail.com","avatar":"https://gravatar.com/avatar/774092af5a4df0d7d135b48fcbe03df5b331c434506ea37eb31b3cddfb17458f?d=mp&s=160"},"body":"Hi All,\nI', Just reopening this feature request.\nA quick summary of my proposal is given below.\n\n1. This PR will allow an additional configuration option\n\"guitool.<name>.gitgui-shortcut\" which will allow us to specify\nkeyboard shortcut  for custom commands in git-gui\n\n2. Even there exists a parameter called \"guitool.<name>.shortcut\"\nwhich is used by git-cola, I suggest to keep this new additional\nconfig parameter as an independent config parameter, which will not\ninterfere with git-cola in any way, because, both are different\napplications and it may have different \"built-in\" shortcuts already\nassigned. So, sharing shortcut scheme between two apps is not a good\nidea.\n\n3. New parameter will expect shortcut combinations specified in TCL/TK\n's format and we will not be doing any processing on it. Will keep it\nsimple.\n\n---\n Documentation/config/guitool.txt | 15 +++++++++++++++\n git-gui/lib/tools.tcl            | 15 ++++++++++++---\n 2 files changed, 27 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config/guitool.txt b/Documentation/config/guitool.txt\nindex 43fb9466ff..79dac23ca3 100644\n--- a/Documentation/config/guitool.txt\n+++ b/Documentation/config/guitool.txt\n@@ -48,3 +48,18 @@ guitool.<name>.prompt::\n  Specifies the general prompt string to display at the top of\n  the dialog, before subsections for 'argPrompt' and 'revPrompt'.\n  The default value includes the actual command.\n+\n+guitool.<name>.gitgui-shortcut\n+ Specifies a keyboard shortcut for the custom tool in the git-gui\n+ application. The value must be a valid string ( without \"<\" , \">\" wrapper )\n+ understood by the TCL/TK 's bind command.See\nhttps://www.tcl.tk/man/tcl8.4/TkCmd/bind.htm\n+ for more details about the supported values. Avoid creating shortcuts that\n+ conflict with existing built-in `git gui` shortcuts.\n+ Example:\n+ [guitool \"Terminal\"]\n+ cmd = gnome-terminal -e zsh\n+ noconsole = yes\n+ gitgui-shortcut = \"Control-y\"\n+ [guitool \"Sync\"]\n+ cmd = \"git pull; git push\"\n+ gitgui-shortcut = \"Alt-s\"\ndiff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl\nindex 413f1a1700..40db3f6395 100644\n--- a/git-gui/lib/tools.tcl\n+++ b/git-gui/lib/tools.tcl\n@@ -38,7 +38,7 @@ proc tools_create_item {parent args} {\n }\n\n proc tools_populate_one {fullname} {\n- global tools_menubar tools_menutbl tools_id\n+ global tools_menubar tools_menutbl tools_id repo_config\n\n  if {![info exists tools_id]} {\n  set tools_id 0\n@@ -61,9 +61,18 @@ proc tools_populate_one {fullname} {\n  }\n  }\n\n- tools_create_item $parent command \\\n+ if {[info exists repo_config(guitool.$fullname.gitgui-shortcut)]} {\n+ set gitgui_shortcut $repo_config(guitool.$fullname.gitgui-shortcut)\n+ tools_create_item $parent command \\\n  -label [lindex $names end] \\\n- -command [list tools_exec $fullname]\n+ -command [list tools_exec $fullname] \\\n+ -accelerator $gitgui_shortcut\n+ bind . <$gitgui_shortcut> [list tools_exec $fullname]\n+ } else {\n+ tools_create_item $parent command \\\n+ -label [lindex $names end] \\\n+ -command [list tools_exec $fullname]\n+ }\n }\n\n proc tools_exec {fullname} {\n--\nhttps://github.com/git/git/pull/220\n--\n\n\nOn Fri, Apr 1, 2016 at 12:02 PM harish k <harish2704@gmail.com> wrote:\n>\n> Hi David,\n>\n> Actually Im a TCL primer.  This is the first time Im dealing with.\n> That is why I kept it simple ( ie both accel-key and accel-label need\n> to be defined in config ).\n>\n> I think, git-cola is using Qt style representation accel-key in the config file.\n>\n> Is git-cola is an official tool from git ? like git-gui?\n> if not ,\n> My suggesion is, it is better to seperate config fields according to\n> application-domain\n> like, \"git-gui-accel = <Ctrl-l>\" etc..\n> Other vise there is good chance for conflicts. ( Eg: consider the case\n>  that, <Ctrl-p> was assined to a custom tool by git-cola )\n>\n> Currently this patch will not handle any conflicting shortcuts. I\n> think custom shortcuts will overwrite the other.\n>\n>\n> On Thu, Mar 31, 2016 at 10:11 PM, David Aguilar <davvid@gmail.com> wrote:\n> > Hello,\n> >\n> > On Tue, Mar 29, 2016 at 11:38:10AM +0000, Harish K wrote:\n> >> ---\n> >>  git-gui/lib/tools.tcl | 16 +++++++++++++---\n> >>  1 file changed, 13 insertions(+), 3 deletions(-)\n> >>\n> >> diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl\n> >> index 6ec9411..749bc67 100644\n> >> --- a/git-gui/lib/tools.tcl\n> >> +++ b/git-gui/lib/tools.tcl\n> >> @@ -38,7 +38,7 @@ proc tools_create_item {parent args} {\n> >>  }\n> >>\n> >>  proc tools_populate_one {fullname} {\n> >> -     global tools_menubar tools_menutbl tools_id\n> >> +     global tools_menubar tools_menutbl tools_id repo_config\n> >>\n> >>       if {![info exists tools_id]} {\n> >>               set tools_id 0\n> >> @@ -61,9 +61,19 @@ proc tools_populate_one {fullname} {\n> >>               }\n> >>       }\n> >>\n> >> -     tools_create_item $parent command \\\n> >> +     if {[info exists repo_config(guitool.$fullname.accelerator)] && [info exists repo_config(guitool.$fullname.accelerator-label)]} {\n> >> +             set accele_key $repo_config(guitool.$fullname.accelerator)\n> >> +             set accel_label $repo_config(guitool.$fullname.accelerator-label)\n> >> +             tools_create_item $parent command \\\n> >>               -label [lindex $names end] \\\n> >> -             -command [list tools_exec $fullname]\n> >> +             -command [list tools_exec $fullname] \\\n> >> +             -accelerator $accel_label\n> >> +             bind . $accele_key [list tools_exec $fullname]\n> >> +     } else {\n> >> +             tools_create_item $parent command \\\n> >> +                     -label [lindex $names end] \\\n> >> +                     -command [list tools_exec $fullname]\n> >> +     }\n> >>  }\n> >>\n> >>  proc tools_exec {fullname} {\n> >>\n> >> --\n> >> https://github.com/git/git/pull/220\n> >\n> > We also support \"custom guitools\" in git-cola using this same\n> > mechanism.  If this gets accepted then we'll want to make\n> > similar change there.\n> >\n> > There's always a small risk that user-defined tools can conflict\n> > with builtin shortcuts, but otherwise this seems like a pretty\n> > nice feature.  Curious, what is the behavior in the event of a\n> > conflict?  Do the builtins win?  IIRC, Qt handles this by\n> > disabling the shortcut and warning that it's ambiguous.\n> >\n> > Please documentation guitool.<name>.accellerator[-label] in\n> > Documentation/config.txt otherwise users will not know that it\n> > exists.\n> >\n> > It would also be good for the docs to clarify what the\n> > accelerators look like in case we need to munge them when making\n> > it work in cola via Qt, which has its own mechanism for\n> > associating actions with shortcuts.  Documented examples with\n> > one and two modifier keys would be helpful.\n> >\n> >\n> > cheers,\n> > --\n> > David\n>\n>\n>\n> --\n>\n> -Regards\n> Harish.K\n\n\n\n-- \n\n-Thanks\nHarish.K\n"},{"id":"383365","messageId":"20191003214422.d4nocrxadxt47smg@yadavpratyush.com","threadId":"41857","inReplyTo":"CACV9s2MQCP04QASgt0xhi3cSNPSKjwXTufxmZQXAUNvnWD9DSw@mail.gmail.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-10-03T21:44:22Z","receivedAt":"2019-10-03T21:44:27Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi Harish,\n\nThanks for the patch. Unfortunately, it seems your mail client messed up \nthe formatting, and the patch won't apply. I'm guessing it is because \nyour mail client broke long lines into two, messing up the diff.\n\nWe use an email-based workflow, so please either configure your mail \nclient so it doesn't munge patches, or use `git send-email`. You can \nfind a pretty good tutorial on sending patches via email over at [0]. \nThe tutorial is for git.git, but works for git-gui.git as well.\n\nIf you feel more comfortable with GitHub pull requests, please take a \nlook at Gitgitgadget [1]. Johannes (in Cc) has used it recently to send \npatches based on the git-gui repo (AFAIK, it was originally designed \nonly for the git.git repo). Maybe ask the folks over there how they do \nit.\n\nOne more thing: your patch is based on the main Git repo. That repo is \nnot where git-gui development takes place. The current \"official\" repo \nfor git-gui is over at [2]. Please base your patches on top of that \nrepo.\n\n[0] https://matheustavares.gitlab.io/posts/first-steps-contributing-to-git#submitting-patches\n[1] https://gitgitgadget.github.io/\n[2] https://github.com/prati0100/git-gui\n\nNow on to the nitty gritty details.\n\nI like the idea. In fact, there were some discussions recently about \nhaving configurable key bindings for _all_ shortcuts in git-gui. Nothing \nconcrete has been done in that direction yet though. But I feel like \nthis is a pretty good first step.\n\nOn 03/10/19 08:18PM, harish k wrote:\n> Hi All,\n> I', Just reopening this feature request.\n> A quick summary of my proposal is given below.\n> \n> 1. This PR will allow an additional configuration option\n> \"guitool.<name>.gitgui-shortcut\" which will allow us to specify\n> keyboard shortcut  for custom commands in git-gui\n\nA pretty nice way of doing it. But I would _really_ like it if there was \nan option in the \"create tool\" dialog to specify the shortcut. People of \na gui tool shouldn't have to mess around with config files as much as \npossible.\n \n> 2. Even there exists a parameter called \"guitool.<name>.shortcut\"\n> which is used by git-cola, I suggest to keep this new additional\n> config parameter as an independent config parameter, which will not\n> interfere with git-cola in any way, because, both are different\n> applications and it may have different \"built-in\" shortcuts already\n> assigned. So, sharing shortcut scheme between two apps is not a good\n> idea.\n\nDavid has advocated inter-operability between git-gui and git-cola. \nWhile I personally don't know how many people actually use both the \ntools at the same time, it doesn't sound like a bad idea either.\n\nSo, sharing shortcuts with git-cola would be nice. Of course, it would \nthen mean that we would have to parse the config parameter before \nfeeding them to `bind`. I don't suppose that should be something too \ncomplicated to do, but I admit I haven't looked too deeply into it.\n\nI'd like to hear what other people think about whether it is worth the \neffort to inter-operate with git-cola.\n \n> 3. New parameter will expect shortcut combinations specified in TCL/TK\n> 's format and we will not be doing any processing on it. Will keep it\n> simple.\n\nAre you sure that is a good idea? I think we should at least make sure \nwe are not binding some illegal sequence, and if we are, we should warn \nthe user about it. And a much more important case would be when a user \nover-writes a pre-existing shortcut for other commands like \"commit\", \n\"reset\", etc. In that case, the menu entires of those commands would \nstill be labelled with the shortcut, but it won't actually work.\n\nYes, your current implementation keeps things simple, but I think some \nlight processing would be beneficial. And if we do decide to go the \ninter-operability with git-cola route, then processing would be needed \nanyway, and we can validate there.\n \n> ---\n>  Documentation/config/guitool.txt | 15 +++++++++++++++\n>  git-gui/lib/tools.tcl            | 15 ++++++++++++---\n>  2 files changed, 27 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/config/guitool.txt b/Documentation/config/guitool.txt\n> index 43fb9466ff..79dac23ca3 100644\n> --- a/Documentation/config/guitool.txt\n> +++ b/Documentation/config/guitool.txt\n> @@ -48,3 +48,18 @@ guitool.<name>.prompt::\n>   Specifies the general prompt string to display at the top of\n>   the dialog, before subsections for 'argPrompt' and 'revPrompt'.\n>   The default value includes the actual command.\n> +\n> +guitool.<name>.gitgui-shortcut\n> + Specifies a keyboard shortcut for the custom tool in the git-gui\n> + application. The value must be a valid string ( without \"<\" , \">\" wrapper )\n> + understood by the TCL/TK 's bind command.See\n> https://www.tcl.tk/man/tcl8.4/TkCmd/bind.htm\n> + for more details about the supported values. Avoid creating shortcuts that\n> + conflict with existing built-in `git gui` shortcuts.\n> + Example:\n> + [guitool \"Terminal\"]\n> + cmd = gnome-terminal -e zsh\n> + noconsole = yes\n> + gitgui-shortcut = \"Control-y\"\n> + [guitool \"Sync\"]\n> + cmd = \"git pull; git push\"\n> + gitgui-shortcut = \"Alt-s\"\n\nThe \"Documentation/\" subdirectory belongs to the Git project, and not to \ngit-gui, so if you want to see this change, you'd have to submit a \nseparate patch for it.\n\nAs far as git-gui's documentation is concerned, unfortunately there is \nnone yet. I have been meaning to start working towards it, but just \nhaven't found the time or motivation to do it yet.\n\n> diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl\n\nLike I mentioned before, please base your patches on the git-gui.git \nrepo, and not git.git. So, this should read \"a/lib/tools.tcl\" instead of \n\"a/git-gui/lib/tools.tcl\".\n\nI haven't looked at the contents of the patch because I can't apply it, \nand I'd prefer to tinker around with it before commenting. So please \nre-send the patch in the proper format and we can discuss the \nimplementation :).\n\n> index 413f1a1700..40db3f6395 100644\n> --- a/git-gui/lib/tools.tcl\n> +++ b/git-gui/lib/tools.tcl\n[snip]\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"383401","messageId":"nycvar.QRO.7.76.6.1910041046000.46@tvgsbejvaqbjf.bet","threadId":"41857","inReplyTo":"20191003214422.d4nocrxadxt47smg@yadavpratyush.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-04T08:49:52Z","receivedAt":"2019-10-04T08:50:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Pratyush,\n\nplease don't top-post on this list (yet another of these things\nrequiring extra brain cycles in the mailing list workflow).\n\nOn Fri, 4 Oct 2019, Pratyush Yadav wrote:\n\n> Hi Harish,\n>\n> Thanks for the patch. Unfortunately, it seems your mail client messed up\n> the formatting, and the patch won't apply. I'm guessing it is because\n> your mail client broke long lines into two, messing up the diff.\n>\n> We use an email-based workflow, so please either configure your mail\n> client so it doesn't munge patches, or use `git send-email`. You can\n> find a pretty good tutorial on sending patches via email over at [0].\n> The tutorial is for git.git, but works for git-gui.git as well.\n\nAh, well. Mailing list-based workflows are so easy, amirite? They are so\nwelcoming and inclusive, yes?\n</sarcasm>\n\n> If you feel more comfortable with GitHub pull requests, please take a\n> look at Gitgitgadget [1]. Johannes (in Cc) has used it recently to send\n> patches based on the git-gui repo (AFAIK, it was originally designed\n> only for the git.git repo). Maybe ask the folks over there how they do\n> it.\n\nHarish, it is actually relatively easy to use GitGitGadget: just add a\nremote like this:\n\n\tgit remote add gitgitgadget https://github.com/gitgitgadget/git\n\tgit fetch gitgitgadget git-gui/master\n\nand then rebase your patch on top of that branch:\n\n\tgit rebase -i --onto git-gui/master HEAD~1\n\nThen force-push your branch to your GitHub fork of git.git and open a\nPull Request at https://github.com/gitgitgadget/git/pulls, targeting\ngit-gui/master.\n\nGitGitGadget will welcome you with a (hopefully) helpful message ;-)\n\nCiao,\nJohannes\n\n>\n> One more thing: your patch is based on the main Git repo. That repo is\n> not where git-gui development takes place. The current \"official\" repo\n> for git-gui is over at [2]. Please base your patches on top of that\n> repo.\n>\n> [0] https://matheustavares.gitlab.io/posts/first-steps-contributing-to-git#submitting-patches\n> [1] https://gitgitgadget.github.io/\n> [2] https://github.com/prati0100/git-gui\n>\n> Now on to the nitty gritty details.\n>\n> I like the idea. In fact, there were some discussions recently about\n> having configurable key bindings for _all_ shortcuts in git-gui. Nothing\n> concrete has been done in that direction yet though. But I feel like\n> this is a pretty good first step.\n>\n> On 03/10/19 08:18PM, harish k wrote:\n> > Hi All,\n> > I', Just reopening this feature request.\n> > A quick summary of my proposal is given below.\n> >\n> > 1. This PR will allow an additional configuration option\n> > \"guitool.<name>.gitgui-shortcut\" which will allow us to specify\n> > keyboard shortcut  for custom commands in git-gui\n>\n> A pretty nice way of doing it. But I would _really_ like it if there was\n> an option in the \"create tool\" dialog to specify the shortcut. People of\n> a gui tool shouldn't have to mess around with config files as much as\n> possible.\n>\n> > 2. Even there exists a parameter called \"guitool.<name>.shortcut\"\n> > which is used by git-cola, I suggest to keep this new additional\n> > config parameter as an independent config parameter, which will not\n> > interfere with git-cola in any way, because, both are different\n> > applications and it may have different \"built-in\" shortcuts already\n> > assigned. So, sharing shortcut scheme between two apps is not a good\n> > idea.\n>\n> David has advocated inter-operability between git-gui and git-cola.\n> While I personally don't know how many people actually use both the\n> tools at the same time, it doesn't sound like a bad idea either.\n>\n> So, sharing shortcuts with git-cola would be nice. Of course, it would\n> then mean that we would have to parse the config parameter before\n> feeding them to `bind`. I don't suppose that should be something too\n> complicated to do, but I admit I haven't looked too deeply into it.\n>\n> I'd like to hear what other people think about whether it is worth the\n> effort to inter-operate with git-cola.\n>\n> > 3. New parameter will expect shortcut combinations specified in TCL/TK\n> > 's format and we will not be doing any processing on it. Will keep it\n> > simple.\n>\n> Are you sure that is a good idea? I think we should at least make sure\n> we are not binding some illegal sequence, and if we are, we should warn\n> the user about it. And a much more important case would be when a user\n> over-writes a pre-existing shortcut for other commands like \"commit\",\n> \"reset\", etc. In that case, the menu entires of those commands would\n> still be labelled with the shortcut, but it won't actually work.\n>\n> Yes, your current implementation keeps things simple, but I think some\n> light processing would be beneficial. And if we do decide to go the\n> inter-operability with git-cola route, then processing would be needed\n> anyway, and we can validate there.\n>\n> > ---\n> >  Documentation/config/guitool.txt | 15 +++++++++++++++\n> >  git-gui/lib/tools.tcl            | 15 ++++++++++++---\n> >  2 files changed, 27 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/Documentation/config/guitool.txt b/Documentation/config/guitool.txt\n> > index 43fb9466ff..79dac23ca3 100644\n> > --- a/Documentation/config/guitool.txt\n> > +++ b/Documentation/config/guitool.txt\n> > @@ -48,3 +48,18 @@ guitool.<name>.prompt::\n> >   Specifies the general prompt string to display at the top of\n> >   the dialog, before subsections for 'argPrompt' and 'revPrompt'.\n> >   The default value includes the actual command.\n> > +\n> > +guitool.<name>.gitgui-shortcut\n> > + Specifies a keyboard shortcut for the custom tool in the git-gui\n> > + application. The value must be a valid string ( without \"<\" , \">\" wrapper )\n> > + understood by the TCL/TK 's bind command.See\n> > https://www.tcl.tk/man/tcl8.4/TkCmd/bind.htm\n> > + for more details about the supported values. Avoid creating shortcuts that\n> > + conflict with existing built-in `git gui` shortcuts.\n> > + Example:\n> > + [guitool \"Terminal\"]\n> > + cmd = gnome-terminal -e zsh\n> > + noconsole = yes\n> > + gitgui-shortcut = \"Control-y\"\n> > + [guitool \"Sync\"]\n> > + cmd = \"git pull; git push\"\n> > + gitgui-shortcut = \"Alt-s\"\n>\n> The \"Documentation/\" subdirectory belongs to the Git project, and not to\n> git-gui, so if you want to see this change, you'd have to submit a\n> separate patch for it.\n>\n> As far as git-gui's documentation is concerned, unfortunately there is\n> none yet. I have been meaning to start working towards it, but just\n> haven't found the time or motivation to do it yet.\n>\n> > diff --git a/git-gui/lib/tools.tcl b/git-gui/lib/tools.tcl\n>\n> Like I mentioned before, please base your patches on the git-gui.git\n> repo, and not git.git. So, this should read \"a/lib/tools.tcl\" instead of\n> \"a/git-gui/lib/tools.tcl\".\n>\n> I haven't looked at the contents of the patch because I can't apply it,\n> and I'd prefer to tinker around with it before commenting. So please\n> re-send the patch in the proper format and we can discuss the\n> implementation :).\n>\n> > index 413f1a1700..40db3f6395 100644\n> > --- a/git-gui/lib/tools.tcl\n> > +++ b/git-gui/lib/tools.tcl\n> [snip]\n>\n> --\n> Regards,\n> Pratyush Yadav\n>\n"},{"id":"383415","messageId":"20191004120107.kpskplwhflnsamwu@yadavpratyush.com","threadId":"41857","inReplyTo":"nycvar.QRO.7.76.6.1910041046000.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-10-04T12:01:07Z","receivedAt":"2019-10-04T12:01:19Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 04/10/19 10:49AM, Johannes Schindelin wrote:\n> Hi Pratyush,\n> \n> please don't top-post on this list (yet another of these things\n> requiring extra brain cycles in the mailing list workflow).\n\nI didn't top-post, or at least it wasn't the intention. The text above \nthe quoted text is the \"preface\" (just like this line I'm replying to is \na \"preface\"). It was a comment on the patch formatting, and it wouldn't \nmake sense (at least to me) to have it _below_ the quoted part, \nespecially since I did have comments in-line (IOW, bottom-posted).\n\nMaybe because I included the references in the middle of the text you \nthought the message was over (understandably so. I probably shouldn't do \nthat), and didn't scroll till the end to read the rest of the message.\n\nOr maybe I don't know something about email etiquette that I should.\n \n> On Fri, 4 Oct 2019, Pratyush Yadav wrote:\n> \n> > Hi Harish,\n> >\n> > Thanks for the patch. Unfortunately, it seems your mail client messed up\n> > the formatting, and the patch won't apply. I'm guessing it is because\n> > your mail client broke long lines into two, messing up the diff.\n> >\n> > We use an email-based workflow, so please either configure your mail\n> > client so it doesn't munge patches, or use `git send-email`. You can\n> > find a pretty good tutorial on sending patches via email over at [0].\n> > The tutorial is for git.git, but works for git-gui.git as well.\n> \n> Ah, well. Mailing list-based workflows are so easy, amirite? They are so\n> welcoming and inclusive, yes?\n> </sarcasm>\n> \n> > If you feel more comfortable with GitHub pull requests, please take a\n> > look at Gitgitgadget [1]. Johannes (in Cc) has used it recently to send\n> > patches based on the git-gui repo (AFAIK, it was originally designed\n> > only for the git.git repo). Maybe ask the folks over there how they do\n> > it.\n> \n> Harish, it is actually relatively easy to use GitGitGadget: just add a\n> remote like this:\n> \n> \tgit remote add gitgitgadget https://github.com/gitgitgadget/git\n> \tgit fetch gitgitgadget git-gui/master\n> \n> and then rebase your patch on top of that branch:\n> \n> \tgit rebase -i --onto git-gui/master HEAD~1\n> \n> Then force-push your branch to your GitHub fork of git.git and open a\n> Pull Request at https://github.com/gitgitgadget/git/pulls, targeting\n> git-gui/master.\n> \n> GitGitGadget will welcome you with a (hopefully) helpful message ;-)\n\nThanks for these instructions. I will include them in a \"Contributing\" \ndocument I'm writing for git-gui.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"383478","messageId":"149a83fd40b71896b134b16c2b499ff472c6234e.camel@gmail.com","threadId":"41857","inReplyTo":"20191004120107.kpskplwhflnsamwu@yadavpratyush.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Harish Karumuthil","fromEmail":"harish2704@gmail.com","sentAt":"2019-10-05T20:16:41Z","receivedAt":"2019-10-05T20:16:51Z","isPatch":true,"sender":{"key":"harish2704@gmail.com","avatar":"https://gravatar.com/avatar/774092af5a4df0d7d135b48fcbe03df5b331c434506ea37eb31b3cddfb17458f?d=mp&s=160"},"body":"Hi All,\n\nFrom https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I\nunderstood that, my current email client ( that is gmail web ) is not good\nfor submitting patches. So I was tying to setup a mail client which is\ncompatible with `git send-mail`. But I was not able to get a satisfactory\nresult in that.\n\nFor now, I followed the instruction of Johannes Schindelin and submitted a\npull request . Please see https://github.com/gitgitgadget/git/pull/376\n\n---------\n@ Pratyush: Regarding your comments,\n\n\n> A pretty nice way of doing it. But I would _really_ like it if there was\n> an option in the \"create tool\" dialog to specify the shortcut. People of\n> a gui tool shouldn't have to mess around with config files as much as\n> possible.\n\nI agree with this, But that may require some more profficiency in TCL/TK\nprogramming which I don't have. This is the first time I am looking into a\nTCL/TK source code.\nAny way I will try to integrate the gui gradually in feature. But\nunfortunatly, I may not be able to do that now.\n\n\n\n> David has advocated inter-operability between git-gui and git-cola.\n> While I personally don't know how many people actually use both the\n> tools at the same time, it doesn't sound like a bad idea either.\n>\n> So, sharing shortcuts with git-cola would be nice. Of course, it would\n> then mean that we would have to parse the config parameter before\n> feeding them to `bind`. I don't suppose that should be something too\n> complicated to do, but I admit I haven't looked too deeply into it.\n\nIMHO, Using a uniform shortcut-key code/foramat for both application can be\nconsidered as nice feature.\nBut, whether we should share common shortcut-scheme with both application is\na different question.\nCurrently, both apps don't have a common shortcut-scheme. So in this\nsituation, only sharing custom-tool's shortcut-scheme with both applications\ndoesn't look like a good  idea to me \n\n\n> Are you sure that is a good idea? I think we should at least make sure\n> we are not binding some illegal sequence, and if we are, we should warn\n> the user about it. And a much more important case would be when a user\n> over-writes a pre-existing shortcut for other commands like \"commit\",\n> \"reset\", etc. In that case, the menu entires of those commands would\n> still be labelled with the shortcut, but it won't actually work.\n\nI agree with you. It is an important point. After reading this, I checked\ncurrent status of these issues. What I found is given below.\n\n1. When user provides an invalid sequence for the shortcut, it will cuase the\nentire gitgui application to crash at the startup\n\n2. When user tries to overwrite existing shortcut, it will not have any\neffect. Because, built in shortcuts will overwrite user provided one. But\nstill, wrong menu accelerator label will persist for custom tools\n\nSince #1 is a serious issue, I tried to find out the function which does the\nkeycode validation, but I haven't succeded till now. ( I found the C function\nname  which is \"TkStringToKeysym\" from TK source, but I couldn't find its TCL\nbinding ). It will be helpful if any one can help me on this.\n\n"},{"id":"383479","messageId":"20191005210127.uinrgazj5ezyqftj@yadavpratyush.com","threadId":"41857","inReplyTo":"149a83fd40b71896b134b16c2b499ff472c6234e.camel@gmail.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-10-05T21:01:27Z","receivedAt":"2019-10-05T21:01:44Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 06/10/19 01:46AM, Harish Karumuthil wrote:\n> Hi All,\n> \n> From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I\n> understood that, my current email client ( that is gmail web ) is not good\n> for submitting patches. So I was tying to setup a mail client which is\n> compatible with `git send-mail`. But I was not able to get a satisfactory\n> result in that.\n\nYou don't need to \"set up\" an email client with git-send-email. \ngit-send-email is an email client itself. Well, one which can only send \nemails.\n\nSo what you should do is run `git format-patch -o feature master..HEAD`, \nassuming your feature branch is checked out. This will give you a set of \n'.patch' files depending on how many commits you made in your branch in \nthe folder feature/. Then, you can run \n\n  git send-email --to='Pratyush Yadav <me@yadavpratyush.com>' --cc='<git@vger.kernel.org>' feature/*.patch\n\nThis will send all your patch files via email to me with the git list in \nCc. You can add multiple '--to' and '--cc' options to send it to \nmultiple people.\n\nTry sending the patches to yourself to experiment around with it.\n\nA pretty good tutorial to configuring and using git-send-email can be \nfound at [0]. And of course, read the man page.\n\nThese instructions are for Linux, but you can probably do something \nsimilar in Windows too (if you're using Windows that is).\n \n> For now, I followed the instruction of Johannes Schindelin and submitted a\n> pull request . Please see https://github.com/gitgitgadget/git/pull/376\n\nYou haven't sent '/submit' over there, so those emails aren't in the \nlist (and my inbox) yet. You need to comment with '/submit' (without the \nquotes) to tell GitGitGadget to send your PR as email.\n\nBut I see that Dscho has left a comment over there, so you should \nprobably address that first. You probably need to amend the commit, \nforce push, and then comment with '/submit'. But I'm not a 100% sure \nbecause I haven't used GitHub PRs a lot.\n \n> ---------\n> @ Pratyush: Regarding your comments,\n> \n> \n> > A pretty nice way of doing it. But I would _really_ like it if there was\n> > an option in the \"create tool\" dialog to specify the shortcut. People of\n> > a gui tool shouldn't have to mess around with config files as much as\n> > possible.\n> \n> I agree with this, But that may require some more profficiency in TCL/TK\n> programming which I don't have. This is the first time I am looking into a\n> TCL/TK source code.\n> Any way I will try to integrate the gui gradually in feature. But\n> unfortunatly, I may not be able to do that now.\n\nPlease do whatever you can. I will try to add a patch on top of yours to \nadd the GUI option.\n \n> > David has advocated inter-operability between git-gui and git-cola.\n> > While I personally don't know how many people actually use both the\n> > tools at the same time, it doesn't sound like a bad idea either.\n> >\n> > So, sharing shortcuts with git-cola would be nice. Of course, it would\n> > then mean that we would have to parse the config parameter before\n> > feeding them to `bind`. I don't suppose that should be something too\n> > complicated to do, but I admit I haven't looked too deeply into it.\n> \n> IMHO, Using a uniform shortcut-key code/foramat for both application can be\n> considered as nice feature.\n> But, whether we should share common shortcut-scheme with both application is\n> a different question.\n> Currently, both apps don't have a common shortcut-scheme. So in this\n> situation, only sharing custom-tool's shortcut-scheme with both applications\n> doesn't look like a good  idea to me \n\nMakes sense.\n \n> > Are you sure that is a good idea? I think we should at least make \n> > sure\n> > we are not binding some illegal sequence, and if we are, we should warn\n> > the user about it. And a much more important case would be when a user\n> > over-writes a pre-existing shortcut for other commands like \"commit\",\n> > \"reset\", etc. In that case, the menu entires of those commands would\n> > still be labelled with the shortcut, but it won't actually work.\n> \n> I agree with you. It is an important point. After reading this, I checked\n> current status of these issues. What I found is given below.\n> \n> 1. When user provides an invalid sequence for the shortcut, it will cuase the\n> entire gitgui application to crash at the startup\n> \n> 2. When user tries to overwrite existing shortcut, it will not have any\n> effect. Because, built in shortcuts will overwrite user provided one. But\n> still, wrong menu accelerator label will persist for custom tools\n\nOne point I forgot to mention earlier was that I'm honestly not a big \nfan of separating the binding and accelerator label. I understand that \nyou might not have the time to do this, but I think it is still worth \nmentioning. Maybe I will implement something like that over your patch. \nBut it would certainly be nice if you can figure it out :).\n\nEither ways, detecting an existing shortcut is pretty easy. The `bind` \nman page [1] says:\n\n  If sequence is specified without a script, then the script currently \n  bound to sequence is returned, or an empty string is returned if there \n  is no binding for sequence.\n\nSo you can use this to find out if there is a binding conflict, and warn \nthe user.\n\n> Since #1 is a serious issue, I tried to find out the function which does the\n> keycode validation, but I haven't succeded till now. ( I found the C function\n> name  which is \"TkStringToKeysym\" from TK source, but I couldn't find its TCL\n> binding ). It will be helpful if any one can help me on this.\n\nI really think you shouldn't dive around in the C parts of Tcl. I \nhaven't looked too deeply into this, but you can probably wrap your bind \ncalls in `catch` [2] and handle errors from there. Again, I haven't \ntried actually doing this, so you do need to check first.\n\nYou can find examples of how to use `catch` in our codebase. Just search \nfor it.\n\n[0] https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/\n[1] https://www.tcl.tk/man/tcl8.4/TkCmd/bind.htm\n[2] https://www.tcl.tk/man/tcl8.4/TclCmd/catch.htm\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"383513","messageId":"nycvar.QRO.7.76.6.1910061054470.46@tvgsbejvaqbjf.bet","threadId":"41857","inReplyTo":"20191005210127.uinrgazj5ezyqftj@yadavpratyush.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-06T09:49:55Z","receivedAt":"2019-10-06T09:50:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Pratyush,\n\nOn Sun, 6 Oct 2019, Pratyush Yadav wrote:\n\n> On 06/10/19 01:46AM, Harish Karumuthil wrote:\n> >\n> > From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I\n> > understood that, my current email client ( that is gmail web ) is not good\n> > for submitting patches. So I was tying to setup a mail client which is\n> > compatible with `git send-mail`. But I was not able to get a satisfactory\n> > result in that.\n>\n> You don't need to \"set up\" an email client with git-send-email.\n> git-send-email is an email client itself. Well, one which can only send\n> emails.\n\nIt also cannot reply to mails on the mailing list.\n\nIt cannot even notify you when anybody replied to your patch.\n\nTwo rather problematic aspects when it comes to patch contributions: how\nare you supposed to work with the reviewers when you lack all the tools\nto _interact_ with them? All `git send-email` provides is a \"fire and\nforget\" way to send patches, i.e. it encourages a monologue, when you\nwant to start a dialogue instead.\n\n> So what you should do is run `git format-patch -o feature master..HEAD`,\n> assuming your feature branch is checked out. This will give you a set of\n> '.patch' files depending on how many commits you made in your branch in\n> the folder feature/. Then, you can run\n>\n>   git send-email --to='Pratyush Yadav <me@yadavpratyush.com>' --cc='<git@vger.kernel.org>' feature/*.patch\n>\n> This will send all your patch files via email to me with the git list in\n> Cc. You can add multiple '--to' and '--cc' options to send it to\n> multiple people.\n>\n> Try sending the patches to yourself to experiment around with it.\n>\n> A pretty good tutorial to configuring and using git-send-email can be\n> found at [0]. And of course, read the man page.\n>\n> These instructions are for Linux, but you can probably do something\n> similar in Windows too (if you're using Windows that is).\n\nLast I checked, `git send-email` worked in Git for Windows.\n\nBut of course, it does not only not address the problem it tries to\nsolve fully (to provide a way to interact with a mailing list when\nsubmitting patches for review), not even close, to add insult to injury,\nit now adds an additional burden to contributors (who might already have\nstruggled to learn themselves enough Tcl/Tk to fix the problem) to\nconfigure `git send-email` correctly.\n\n> > For now, I followed the instruction of Johannes Schindelin and submitted a\n> > pull request . Please see https://github.com/gitgitgadget/git/pull/376\n>\n> You haven't sent '/submit' over there, so those emails aren't in the\n> list (and my inbox) yet. You need to comment with '/submit' (without the\n> quotes) to tell GitGitGadget to send your PR as email.\n\nThey probably did not hit `/submit` because the initial hurdle is to be\n`/allow`ed (a very, very simplistic attempt at trying to prevent\nspamming the mailing list by jokesters, of which there are unfortunately\nquite a number).\n\nThis `/allow` command, BTW, can be issued by anybody who has been\n`/allow`ed before, it does not always have to be me.\n\nFWIW you should probably be in that list of `/allow`ed people so that\nyou can `/allow` new contributors to use GitGitGadget, too.\n\n> [...]\n>\n> > Since #1 is a serious issue, I tried to find out the function which does the\n> > keycode validation, but I haven't succeded till now. ( I found the C function\n> > name  which is \"TkStringToKeysym\" from TK source, but I couldn't find its TCL\n> > binding ). It will be helpful if any one can help me on this.\n>\n> I really think you shouldn't dive around in the C parts of Tcl. I\n> haven't looked too deeply into this, but you can probably wrap your bind\n> calls in `catch` [2] and handle errors from there. Again, I haven't\n> tried actually doing this, so you do need to check first.\n>\n> You can find examples of how to use `catch` in our codebase. Just search\n> for it.\n\nFWIW in addition to the `catch` method, I would also recommend looking\ninto a minimal (not even necessarily complete) way to translate the Qt\nway to specify the keyboard shortcuts (as used by `git-cola`) to Tk\nones.\n\nAs indicated in\nhttps://github.com/git/git/pull/220#issuecomment-536045075, the Qt style\n`CTRL+,` should be translated to `Control-comma`, for example. In\nparticular, keystrokes specified in the format indicated at\nhttps://doc.qt.io/archives/qt-4.8/qkeysequence.html#QKeySequence-2 to\nthe format indicated at https://www.tcl.tk/man/tcl8.4/TkCmd/keysyms.htm.\n\nHowever, it might not even need to put in _such_ a lot of work: in my\ntests, `Control-,` worked just as well as `Control-comma`. To test this\nfor yourself, use this snippet (that is slightly modified from the\nexample at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so\nthat it reacts _only_ to Control+comma instead of all keys):\n\n-- snip --\nset keysym \"Press any key\"\npack [label .l -textvariable keysym -padx 2m -pady 1m]\n#bind . <Key> {\nbind . <Control-,> {\n    set keysym \"You pressed %K\"\n}\n-- snap --\n\nSo I could imagine that something like this could serve as an initial\ndraft for a function that you can turn into a \"good enough\" version:\n\n-- snip --\nproc QKeySequence2keysym {keystroke} {\n\tregsub -all {(?i)Ctrl\\+} $keystroke \"Control-\" keystroke\n\tregsub -all {(?i)Alt\\+} $keystroke \"Alt-\" keystroke\n\tregsub -all {(?i)Shift\\+} $keystroke \"Shift-\" keystroke\n\treturn $keystroke\n}\n-- snap --\n\nThat way, you don't have to introduce settings separate from\n`git-cola`'s, and you can reuse the short-and-sweet variable name.\n\nCiao,\nJohannes\n"},{"id":"383523","messageId":"20191006183948.5n23sdy2l4uwl6kb@yadavpratyush.com","threadId":"41857","inReplyTo":"nycvar.QRO.7.76.6.1910061054470.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-10-06T18:39:49Z","receivedAt":"2019-10-06T18:40:12Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 06/10/19 11:49AM, Johannes Schindelin wrote:\n> Hi Pratyush,\n> \n> On Sun, 6 Oct 2019, Pratyush Yadav wrote:\n> \n> > On 06/10/19 01:46AM, Harish Karumuthil wrote:\n> > >\n> > > From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I\n> > > understood that, my current email client ( that is gmail web ) is not good\n> > > for submitting patches. So I was tying to setup a mail client which is\n> > > compatible with `git send-mail`. But I was not able to get a satisfactory\n> > > result in that.\n> >\n> > You don't need to \"set up\" an email client with git-send-email.\n> > git-send-email is an email client itself. Well, one which can only send\n> > emails.\n> \n> It also cannot reply to mails on the mailing list.\n> \n> It cannot even notify you when anybody replied to your patch.\n> \n> Two rather problematic aspects when it comes to patch contributions: how\n> are you supposed to work with the reviewers when you lack all the tools\n> to _interact_ with them? All `git send-email` provides is a \"fire and\n> forget\" way to send patches, i.e. it encourages a monologue, when you\n> want to start a dialogue instead.\n\nWell, I started with email based patch contribution when I was first got \nstarted with open source, so I might be a bit biased, but in my \nexperience, it is not that difficult to set all these things up. Most of \nthe time, all you need to tell git-send-email is your SMTP server, \nusername, and password. All pretty easy things to do.\n\nAnd you add in your email client (which pretty much everyone should \nhave), and it is a complete setup. I personally use neomutt as my email \nclient, but I have used Thunderbird before, and it is really easy to set \nit up to send plain text emails. All you need to do is hold Shift before \nhitting reply, and you're in plain text mode. And you can even make it \nuse plain text by default by flipping a switch in the settings.\n\nSo while I agree with you that there is certainly a learning curve \ninvolved, I don't think it is all too bad. But again, that is all my \npersonal opinion, and nothing based on facts or data.\n \n> > So what you should do is run `git format-patch -o feature master..HEAD`,\n> > assuming your feature branch is checked out. This will give you a set of\n> > '.patch' files depending on how many commits you made in your branch in\n> > the folder feature/. Then, you can run\n> >\n> >   git send-email --to='Pratyush Yadav <me@yadavpratyush.com>' --cc='<git@vger.kernel.org>' feature/*.patch\n> >\n> > This will send all your patch files via email to me with the git list in\n> > Cc. You can add multiple '--to' and '--cc' options to send it to\n> > multiple people.\n> >\n> > Try sending the patches to yourself to experiment around with it.\n> >\n> > A pretty good tutorial to configuring and using git-send-email can be\n> > found at [0]. And of course, read the man page.\n> >\n> > These instructions are for Linux, but you can probably do something\n> > similar in Windows too (if you're using Windows that is).\n> \n> Last I checked, `git send-email` worked in Git for Windows.\n> \n> But of course, it does not only not address the problem it tries to\n> solve fully (to provide a way to interact with a mailing list when\n> submitting patches for review), not even close, to add insult to injury,\n> it now adds an additional burden to contributors (who might already have\n> struggled to learn themselves enough Tcl/Tk to fix the problem) to\n> configure `git send-email` correctly.\n\nThe way I see it, git-send-email does not need to solve the problem of \ninteracting with a mailing list. That problem is already solved by a \nhoard of MUAs. All git-send-email should do is, you guessed it, send \nemails (or patches to be specific).\n\nAnyway, GitGitGadget solves a large part of the problem. It eliminates \nthe need for using git-send-email, and it even shows you the replies \nreceived on the list. I honestly think it is a great tool, and it gives \npeople a very good alternative to using git-send-email.\n\nOne feature that would make it complete would be the ability to reply to \nreview comments. This would remove the need for an email client (almost) \ncompletely. I have never written Typescript or used Azure pipelines \never, but I can try tinkering around to see if I can figure out how to \ndo something like that. Unless, of course, you or someone else is \nalready doing it. If not, some pointers would be appreciated.\n \n> > > For now, I followed the instruction of Johannes Schindelin and submitted a\n> > > pull request . Please see https://github.com/gitgitgadget/git/pull/376\n> >\n> > You haven't sent '/submit' over there, so those emails aren't in the\n> > list (and my inbox) yet. You need to comment with '/submit' (without the\n> > quotes) to tell GitGitGadget to send your PR as email.\n> \n> They probably did not hit `/submit` because the initial hurdle is to be\n> `/allow`ed (a very, very simplistic attempt at trying to prevent\n> spamming the mailing list by jokesters, of which there are unfortunately\n> quite a number).\n> \n> This `/allow` command, BTW, can be issued by anybody who has been\n> `/allow`ed before, it does not always have to be me.\n> \n> FWIW you should probably be in that list of `/allow`ed people so that\n> you can `/allow` new contributors to use GitGitGadget, too.\n\nThat would be great! How do I get '/allow'ed? Do I have to open a PR \nthere for you to '/allow' me?\n \n> > [...]\n> >\n> > > Since #1 is a serious issue, I tried to find out the function which does the\n> > > keycode validation, but I haven't succeded till now. ( I found the C function\n> > > name  which is \"TkStringToKeysym\" from TK source, but I couldn't find its TCL\n> > > binding ). It will be helpful if any one can help me on this.\n> >\n> > I really think you shouldn't dive around in the C parts of Tcl. I\n> > haven't looked too deeply into this, but you can probably wrap your bind\n> > calls in `catch` [2] and handle errors from there. Again, I haven't\n> > tried actually doing this, so you do need to check first.\n> >\n> > You can find examples of how to use `catch` in our codebase. Just search\n> > for it.\n> \n> FWIW in addition to the `catch` method, I would also recommend looking\n> into a minimal (not even necessarily complete) way to translate the Qt\n> way to specify the keyboard shortcuts (as used by `git-cola`) to Tk\n> ones.\n> \n> As indicated in\n> https://github.com/git/git/pull/220#issuecomment-536045075, the Qt style\n> `CTRL+,` should be translated to `Control-comma`, for example. In\n> particular, keystrokes specified in the format indicated at\n> https://doc.qt.io/archives/qt-4.8/qkeysequence.html#QKeySequence-2 to\n> the format indicated at https://www.tcl.tk/man/tcl8.4/TkCmd/keysyms.htm.\n> \n> However, it might not even need to put in _such_ a lot of work: in my\n> tests, `Control-,` worked just as well as `Control-comma`. To test this\n> for yourself, use this snippet (that is slightly modified from the\n> example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so\n> that it reacts _only_ to Control+comma instead of all keys):\n\nAnother benefit to the translation framework would be that we could also \ngenerate the menu labels (aka \"accelerator\") for the tools, instead of \nmaking the user specify both the shortcut and the label.\n \n> -- snip --\n> set keysym \"Press any key\"\n> pack [label .l -textvariable keysym -padx 2m -pady 1m]\n> #bind . <Key> {\n> bind . <Control-,> {\n>     set keysym \"You pressed %K\"\n> }\n> -- snap --\n> \n> So I could imagine that something like this could serve as an initial\n> draft for a function that you can turn into a \"good enough\" version:\n> \n> -- snip --\n> proc QKeySequence2keysym {keystroke} {\n> \tregsub -all {(?i)Ctrl\\+} $keystroke \"Control-\" keystroke\n> \tregsub -all {(?i)Alt\\+} $keystroke \"Alt-\" keystroke\n> \tregsub -all {(?i)Shift\\+} $keystroke \"Shift-\" keystroke\n> \treturn $keystroke\n> }\n> -- snap --\n> \n> That way, you don't have to introduce settings separate from\n> `git-cola`'s, and you can reuse the short-and-sweet variable name.\n\nI think a more important question is whether we _really_ need to have \ncompatibility with git-cola. Most of our shortcuts don't match with \nthem, so is it really worth the effort to try to keep compatibility?\n\nI'm not against something like this, but just want to be sure we \nevaluate whether the effort is worth it.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"383524","messageId":"293e0738-85dd-14eb-20b9-a837aa88c0cc@iee.email","threadId":"41857","inReplyTo":"20191006183948.5n23sdy2l4uwl6kb@yadavpratyush.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-10-06T19:37:14Z","receivedAt":"2019-10-06T19:37:18Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 06/10/2019 19:39, Pratyush Yadav wrote:\n>> That way, you don't have to introduce settings separate from\n>> `git-cola`'s, and you can reuse the short-and-sweet variable name.\n> I think a more important question is whether we_really_  need to have\n> compatibility with git-cola. Most of our shortcuts don't match with\n> them, so is it really worth the effort to try to keep compatibility?\n>\n> I'm not against something like this, but just want to be sure we\n> evaluate whether the effort is worth it.\nI just wondered if the custom list would/could be split between a \n\"Common-shortcuts\" list, and \"GUI-local\" list (and hence, elsewhere, a \n\"Cola-local\" list)\n\nI don't use Cola at all, so this is just a bikeshed comment...\n-- \nPhilip\n"},{"id":"383525","messageId":"nycvar.QRO.7.76.6.1910062208460.46@tvgsbejvaqbjf.bet","threadId":"41857","inReplyTo":"20191006183948.5n23sdy2l4uwl6kb@yadavpratyush.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-06T20:27:38Z","receivedAt":"2019-10-06T20:28:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Pratyush,\n\nOn Mon, 7 Oct 2019, Pratyush Yadav wrote:\n\n> On 06/10/19 11:49AM, Johannes Schindelin wrote:\n> > Hi Pratyush,\n> >\n> > On Sun, 6 Oct 2019, Pratyush Yadav wrote:\n> >\n> > > On 06/10/19 01:46AM, Harish Karumuthil wrote:\n> > > >\n> > > > From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I\n> > > > understood that, my current email client ( that is gmail web ) is not good\n> > > > for submitting patches. So I was tying to setup a mail client which is\n> > > > compatible with `git send-mail`. But I was not able to get a satisfactory\n> > > > result in that.\n> > >\n> > > You don't need to \"set up\" an email client with git-send-email.\n> > > git-send-email is an email client itself. Well, one which can only send\n> > > emails.\n> >\n> > It also cannot reply to mails on the mailing list.\n> >\n> > It cannot even notify you when anybody replied to your patch.\n> >\n> > Two rather problematic aspects when it comes to patch contributions: how\n> > are you supposed to work with the reviewers when you lack all the tools\n> > to _interact_ with them? All `git send-email` provides is a \"fire and\n> > forget\" way to send patches, i.e. it encourages a monologue, when you\n> > want to start a dialogue instead.\n>\n> Well, I started with email based patch contribution when I was first got\n> started with open source, so I might be a bit biased, but in my\n> experience, it is not that difficult to set all these things up. Most of\n> the time, all you need to tell git-send-email is your SMTP server,\n> username, and password. All pretty easy things to do.\n\nOkay, set it up with a corporate Exchange server.\n\nI'll be waiting right here.\n\n> And you add in your email client (which pretty much everyone should\n> have), and it is a complete setup. I personally use neomutt as my email\n> client, but I have used Thunderbird before, and it is really easy to set\n> it up to send plain text emails. All you need to do is hold Shift before\n> hitting reply, and you're in plain text mode. And you can even make it\n> use plain text by default by flipping a switch in the settings.\n\nHow intuitive. And of course Thunderbird still messes up the patches so\nthat they won't apply, unless you *checks notes* do things that are\nquite involved or *checks notes* do other things that are quite\ninvolved.\n\nBut then, everybody reads their mails on their command-line anyway.\nRight?\n\n> So while I agree with you that there is certainly a learning curve\n> involved, I don't think it is all too bad. But again, that is all my\n> personal opinion, and nothing based on facts or data.\n\nLet me provide you with some data, then. Granted, it's not necessarily\nall Git GUI, but it includes Git GUI patches, too: Git for Windows'\ncontributions.\n\nAs should be well-known, I try to follow Postel's Law when it comes to\nGit for Windows' patches: be lenient in the input, strict in the output.\nAs such, I don't force contributors to use GitHub PRs (although that is\ncertainly encouraged by virtue of Git for Windows' source code being\nhosted on GitHub), or send patches, or send pull requests to their own\npublic repositories or bundles sent to the mailing list. I accept them\nall. At least that is the idea.\n\nI cannot tell you how many contributions came in via GitHub PRs. I can\ntell precisely you how many contributions were made _not_ using GitHub\nPRs. One one hand. Actually, on zero hands.\n\nSo clearly, at least Git for Windows' contributors (including some who\nprovided Git GUI patches) are much more comfortable with the PR workflow\nthan with the mailing list-based workflow.\n\nJust so you can't say you don't have data.\n\n> > > So what you should do is run `git format-patch -o feature master..HEAD`,\n> > > assuming your feature branch is checked out. This will give you a set of\n> > > '.patch' files depending on how many commits you made in your branch in\n> > > the folder feature/. Then, you can run\n> > >\n> > >   git send-email --to='Pratyush Yadav <me@yadavpratyush.com>' --cc='<git@vger.kernel.org>' feature/*.patch\n> > >\n> > > This will send all your patch files via email to me with the git list in\n> > > Cc. You can add multiple '--to' and '--cc' options to send it to\n> > > multiple people.\n> > >\n> > > Try sending the patches to yourself to experiment around with it.\n> > >\n> > > A pretty good tutorial to configuring and using git-send-email can be\n> > > found at [0]. And of course, read the man page.\n> > >\n> > > These instructions are for Linux, but you can probably do something\n> > > similar in Windows too (if you're using Windows that is).\n> >\n> > Last I checked, `git send-email` worked in Git for Windows.\n> >\n> > But of course, it does not only not address the problem it tries to\n> > solve fully (to provide a way to interact with a mailing list when\n> > submitting patches for review), not even close, to add insult to injury,\n> > it now adds an additional burden to contributors (who might already have\n> > struggled to learn themselves enough Tcl/Tk to fix the problem) to\n> > configure `git send-email` correctly.\n>\n> The way I see it, git-send-email does not need to solve the problem of\n> interacting with a mailing list. That problem is already solved by a\n> hoard of MUAs. All git-send-email should do is, you guessed it, send\n> emails (or patches to be specific).\n\nBut of course! And it is natural that you should use two separate MUAs.\n\n> Anyway, GitGitGadget solves a large part of the problem. It eliminates\n> the need for using git-send-email, and it even shows you the replies\n> received on the list. I honestly think it is a great tool, and it gives\n> people a very good alternative to using git-send-email.\n\nGitGitGadget is just a workaround. Not even complete. Can't be complete,\nreally. Because problems. It has much of the same problems of `git\nsend-email`: it's a one-way conversation. Code is not discussed in the\nright context (which would be a worktree with the correct commit checked\nout). The transfer is lossy (email is designed for human-readable\nmessages, not for transferring machine-readable serialized objects).\nMatching original commits and/or branches to the ones on the other side\nis tedious. Any interaction requires switching between many tools. Etc\n\n> One feature that would make it complete would be the ability to reply to\n> review comments.\n\nAnd how would that work, exactly? How to determine *which* email to\nrespond to? *Which* person to reply to? *What* to quote?\n\n> This would remove the need for an email client (almost) completely. I\n> have never written Typescript or used Azure pipelines ever, but I can\n> try tinkering around to see if I can figure out how to do something\n> like that. Unless, of course, you or someone else is already doing it.\n> If not, some pointers would be appreciated.\n\nFeel free to give this challenge a try.\n\n> > > > For now, I followed the instruction of Johannes Schindelin and submitted a\n> > > > pull request . Please see https://github.com/gitgitgadget/git/pull/376\n> > >\n> > > You haven't sent '/submit' over there, so those emails aren't in the\n> > > list (and my inbox) yet. You need to comment with '/submit' (without the\n> > > quotes) to tell GitGitGadget to send your PR as email.\n> >\n> > They probably did not hit `/submit` because the initial hurdle is to be\n> > `/allow`ed (a very, very simplistic attempt at trying to prevent\n> > spamming the mailing list by jokesters, of which there are unfortunately\n> > quite a number).\n> >\n> > This `/allow` command, BTW, can be issued by anybody who has been\n> > `/allow`ed before, it does not always have to be me.\n> >\n> > FWIW you should probably be in that list of `/allow`ed people so that\n> > you can `/allow` new contributors to use GitGitGadget, too.\n>\n> That would be great! How do I get '/allow'ed? Do I have to open a PR\n> there for you to '/allow' me?\n\nhttps://github.com/gitgitgadget/git/pull/376#issuecomment-538784646\n\n> > > [...]\n> > >\n> > > > Since #1 is a serious issue, I tried to find out the function which does the\n> > > > keycode validation, but I haven't succeded till now. ( I found the C function\n> > > > name  which is \"TkStringToKeysym\" from TK source, but I couldn't find its TCL\n> > > > binding ). It will be helpful if any one can help me on this.\n> > >\n> > > I really think you shouldn't dive around in the C parts of Tcl. I\n> > > haven't looked too deeply into this, but you can probably wrap your bind\n> > > calls in `catch` [2] and handle errors from there. Again, I haven't\n> > > tried actually doing this, so you do need to check first.\n> > >\n> > > You can find examples of how to use `catch` in our codebase. Just search\n> > > for it.\n> >\n> > FWIW in addition to the `catch` method, I would also recommend looking\n> > into a minimal (not even necessarily complete) way to translate the Qt\n> > way to specify the keyboard shortcuts (as used by `git-cola`) to Tk\n> > ones.\n> >\n> > As indicated in\n> > https://github.com/git/git/pull/220#issuecomment-536045075, the Qt style\n> > `CTRL+,` should be translated to `Control-comma`, for example. In\n> > particular, keystrokes specified in the format indicated at\n> > https://doc.qt.io/archives/qt-4.8/qkeysequence.html#QKeySequence-2 to\n> > the format indicated at https://www.tcl.tk/man/tcl8.4/TkCmd/keysyms.htm.\n> >\n> > However, it might not even need to put in _such_ a lot of work: in my\n> > tests, `Control-,` worked just as well as `Control-comma`. To test this\n> > for yourself, use this snippet (that is slightly modified from the\n> > example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so\n> > that it reacts _only_ to Control+comma instead of all keys):\n>\n> Another benefit to the translation framework would be that we could also\n> generate the menu labels (aka \"accelerator\") for the tools, instead of\n> making the user specify both the shortcut and the label.\n>\n> > -- snip --\n> > set keysym \"Press any key\"\n> > pack [label .l -textvariable keysym -padx 2m -pady 1m]\n> > #bind . <Key> {\n> > bind . <Control-,> {\n> >     set keysym \"You pressed %K\"\n> > }\n> > -- snap --\n> >\n> > So I could imagine that something like this could serve as an initial\n> > draft for a function that you can turn into a \"good enough\" version:\n> >\n> > -- snip --\n> > proc QKeySequence2keysym {keystroke} {\n> > \tregsub -all {(?i)Ctrl\\+} $keystroke \"Control-\" keystroke\n> > \tregsub -all {(?i)Alt\\+} $keystroke \"Alt-\" keystroke\n> > \tregsub -all {(?i)Shift\\+} $keystroke \"Shift-\" keystroke\n> > \treturn $keystroke\n> > }\n> > -- snap --\n> >\n> > That way, you don't have to introduce settings separate from\n> > `git-cola`'s, and you can reuse the short-and-sweet variable name.\n>\n> I think a more important question is whether we _really_ need to have\n> compatibility with git-cola. Most of our shortcuts don't match with\n> them, so is it really worth the effort to try to keep compatibility?\n>\n> I'm not against something like this, but just want to be sure we\n> evaluate whether the effort is worth it.\n\n`git-cola`, by virtue of being there first, squats on the neat config\nsetting name `shortcut`.\n\nI expect users to be utterly surprised when that name does not work for\nthem.\n\nPlease note that I intended `QKeySequence2keysym` to leave parameters\nunchanged that already refer to keysyms. So this is purely a way to\nreuse the same name as `git-cola` while still playing nice with existing\nconfigurations targeting `git-cola`.\n\nCiao,\nJohannes\n"},{"id":"383527","messageId":"20191006210647.wfjr7lhw5fxs4bin@yadavpratyush.com","threadId":"41857","inReplyTo":"nycvar.QRO.7.76.6.1910062208460.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-10-06T21:06:47Z","receivedAt":"2019-10-06T21:06:58Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 06/10/19 10:27PM, Johannes Schindelin wrote:\n> Hi Pratyush,\n> \n> On Mon, 7 Oct 2019, Pratyush Yadav wrote:\n> \n> > On 06/10/19 11:49AM, Johannes Schindelin wrote:\n> > > Hi Pratyush,\n> > >\n> > > On Sun, 6 Oct 2019, Pratyush Yadav wrote:\n> > >\n> > > > On 06/10/19 01:46AM, Harish Karumuthil wrote:\n> > > > >\n> > > > > From https://www.kernel.org/doc/html/v4.10/process/email-clients.html, I\n> > > > > understood that, my current email client ( that is gmail web ) is not good\n> > > > > for submitting patches. So I was tying to setup a mail client which is\n> > > > > compatible with `git send-mail`. But I was not able to get a satisfactory\n> > > > > result in that.\n> > > >\n> > > > You don't need to \"set up\" an email client with git-send-email.\n> > > > git-send-email is an email client itself. Well, one which can only send\n> > > > emails.\n> > >\n> > > It also cannot reply to mails on the mailing list.\n> > >\n> > > It cannot even notify you when anybody replied to your patch.\n> > >\n> > > Two rather problematic aspects when it comes to patch contributions: how\n> > > are you supposed to work with the reviewers when you lack all the tools\n> > > to _interact_ with them? All `git send-email` provides is a \"fire and\n> > > forget\" way to send patches, i.e. it encourages a monologue, when you\n> > > want to start a dialogue instead.\n> >\n> > Well, I started with email based patch contribution when I was first got\n> > started with open source, so I might be a bit biased, but in my\n> > experience, it is not that difficult to set all these things up. Most of\n> > the time, all you need to tell git-send-email is your SMTP server,\n> > username, and password. All pretty easy things to do.\n> \n> Okay, set it up with a corporate Exchange server.\n> \n> I'll be waiting right here.\n\nI admit, I've never had to do that. And by how you word it, hope I never \nhave to do it in the future either.\n \n> > And you add in your email client (which pretty much everyone should\n> > have), and it is a complete setup. I personally use neomutt as my email\n> > client, but I have used Thunderbird before, and it is really easy to set\n> > it up to send plain text emails. All you need to do is hold Shift before\n> > hitting reply, and you're in plain text mode. And you can even make it\n> > use plain text by default by flipping a switch in the settings.\n> \n> How intuitive. And of course Thunderbird still messes up the patches so\n> that they won't apply, unless you *checks notes* do things that are\n> quite involved or *checks notes* do other things that are quite\n> involved.\n\nHa! Made me chuckle. You got me there :). I suppose it isn't _that_ \nsimple and I'm just biased because I am so used to it.\n \n> But then, everybody reads their mails on their command-line anyway.\n> Right?\n\nWell, they should. It is objectively superior ;) </sarcasm>\n \n> > So while I agree with you that there is certainly a learning curve\n> > involved, I don't think it is all too bad. But again, that is all my\n> > personal opinion, and nothing based on facts or data.\n> \n> Let me provide you with some data, then. Granted, it's not necessarily\n> all Git GUI, but it includes Git GUI patches, too: Git for Windows'\n> contributions.\n> \n> As should be well-known, I try to follow Postel's Law when it comes to\n> Git for Windows' patches: be lenient in the input, strict in the output.\n> As such, I don't force contributors to use GitHub PRs (although that is\n> certainly encouraged by virtue of Git for Windows' source code being\n> hosted on GitHub), or send patches, or send pull requests to their own\n> public repositories or bundles sent to the mailing list. I accept them\n> all. At least that is the idea.\n> \n> I cannot tell you how many contributions came in via GitHub PRs. I can\n> tell precisely you how many contributions were made _not_ using GitHub\n> PRs. One one hand. Actually, on zero hands.\n> \n> So clearly, at least Git for Windows' contributors (including some who\n> provided Git GUI patches) are much more comfortable with the PR workflow\n> than with the mailing list-based workflow.\n\nI never said email is better that GitHub PRs. It isn't. My point was \nthat using email isn't _that_ hard. When I first did it, it maybe took \nme 3-4 hours to figure everything out, and then I was set forever. I \ncarry around the same '.gitconfig' file to all my setups, and everything \n\"just works\".\n\nSo yes, GitHub PRs are certainly easier, but email wasn't too difficult \nin my experience. But then I'm a kernel developer, so I'm a minority to \nbegin with.\n\nI suspect you've had this debate more than once, because you come in \nguns blazing ;)\n \n> Just so you can't say you don't have data.\n> \n> > > > So what you should do is run `git format-patch -o feature master..HEAD`,\n> > > > assuming your feature branch is checked out. This will give you a set of\n> > > > '.patch' files depending on how many commits you made in your branch in\n> > > > the folder feature/. Then, you can run\n> > > >\n> > > >   git send-email --to='Pratyush Yadav <me@yadavpratyush.com>' --cc='<git@vger.kernel.org>' feature/*.patch\n> > > >\n> > > > This will send all your patch files via email to me with the git list in\n> > > > Cc. You can add multiple '--to' and '--cc' options to send it to\n> > > > multiple people.\n> > > >\n> > > > Try sending the patches to yourself to experiment around with it.\n> > > >\n> > > > A pretty good tutorial to configuring and using git-send-email can be\n> > > > found at [0]. And of course, read the man page.\n> > > >\n> > > > These instructions are for Linux, but you can probably do something\n> > > > similar in Windows too (if you're using Windows that is).\n> > >\n> > > Last I checked, `git send-email` worked in Git for Windows.\n> > >\n> > > But of course, it does not only not address the problem it tries to\n> > > solve fully (to provide a way to interact with a mailing list when\n> > > submitting patches for review), not even close, to add insult to injury,\n> > > it now adds an additional burden to contributors (who might already have\n> > > struggled to learn themselves enough Tcl/Tk to fix the problem) to\n> > > configure `git send-email` correctly.\n> >\n> > The way I see it, git-send-email does not need to solve the problem of\n> > interacting with a mailing list. That problem is already solved by a\n> > hoard of MUAs. All git-send-email should do is, you guessed it, send\n> > emails (or patches to be specific).\n> \n> But of course! And it is natural that you should use two separate MUAs.\n> \n> > Anyway, GitGitGadget solves a large part of the problem. It eliminates\n> > the need for using git-send-email, and it even shows you the replies\n> > received on the list. I honestly think it is a great tool, and it gives\n> > people a very good alternative to using git-send-email.\n> \n> GitGitGadget is just a workaround. Not even complete. Can't be complete,\n> really. Because problems. It has much of the same problems of `git\n> send-email`: it's a one-way conversation. Code is not discussed in the\n> right context (which would be a worktree with the correct commit checked\n> out). The transfer is lossy (email is designed for human-readable\n> messages, not for transferring machine-readable serialized objects).\n> Matching original commits and/or branches to the ones on the other side\n> is tedious. Any interaction requires switching between many tools. Etc\n> \n> > One feature that would make it complete would be the ability to reply to\n> > review comments.\n> \n> And how would that work, exactly? How to determine *which* email to\n> respond to? *Which* person to reply to? *What* to quote?\n\nGGG already shows replies to the patches as a comment. On GitHub you can \n\"Quote reply\" a comment, which quotes the entire comment just like your \nMUA would. The option can be found by clicking the 3 dots on the top \nright of a comment.\n\nThen you can write your reply there, and the last line would be \n'/reply', which would make GGG send that email as a reply. You would \nneed to strip the first line from the reply because GGG starts the reply \nwith something like:\n\n  > [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)):\n \nGGG also adds 3 backticks before and after the reply content, so those \nwould need to be removed too.\n\nDoes this sound like a sane solution?\n\n> > This would remove the need for an email client (almost) completely. I\n> > have never written Typescript or used Azure pipelines ever, but I can\n> > try tinkering around to see if I can figure out how to do something\n> > like that. Unless, of course, you or someone else is already doing it.\n> > If not, some pointers would be appreciated.\n> \n> Feel free to give this challenge a try.\n\nThe first challenge is learning Typescript :)\n \n> > > > > For now, I followed the instruction of Johannes Schindelin and submitted a\n> > > > > pull request . Please see https://github.com/gitgitgadget/git/pull/376\n> > > >\n> > > > You haven't sent '/submit' over there, so those emails aren't in the\n> > > > list (and my inbox) yet. You need to comment with '/submit' (without the\n> > > > quotes) to tell GitGitGadget to send your PR as email.\n> > >\n> > > They probably did not hit `/submit` because the initial hurdle is to be\n> > > `/allow`ed (a very, very simplistic attempt at trying to prevent\n> > > spamming the mailing list by jokesters, of which there are unfortunately\n> > > quite a number).\n> > >\n> > > This `/allow` command, BTW, can be issued by anybody who has been\n> > > `/allow`ed before, it does not always have to be me.\n> > >\n> > > FWIW you should probably be in that list of `/allow`ed people so that\n> > > you can `/allow` new contributors to use GitGitGadget, too.\n> >\n> > That would be great! How do I get '/allow'ed? Do I have to open a PR\n> > there for you to '/allow' me?\n> \n> https://github.com/gitgitgadget/git/pull/376#issuecomment-538784646\n> \n> > > > [...]\n> > > >\n> > > > > Since #1 is a serious issue, I tried to find out the function which does the\n> > > > > keycode validation, but I haven't succeded till now. ( I found the C function\n> > > > > name  which is \"TkStringToKeysym\" from TK source, but I couldn't find its TCL\n> > > > > binding ). It will be helpful if any one can help me on this.\n> > > >\n> > > > I really think you shouldn't dive around in the C parts of Tcl. I\n> > > > haven't looked too deeply into this, but you can probably wrap your bind\n> > > > calls in `catch` [2] and handle errors from there. Again, I haven't\n> > > > tried actually doing this, so you do need to check first.\n> > > >\n> > > > You can find examples of how to use `catch` in our codebase. Just search\n> > > > for it.\n> > >\n> > > FWIW in addition to the `catch` method, I would also recommend looking\n> > > into a minimal (not even necessarily complete) way to translate the Qt\n> > > way to specify the keyboard shortcuts (as used by `git-cola`) to Tk\n> > > ones.\n> > >\n> > > As indicated in\n> > > https://github.com/git/git/pull/220#issuecomment-536045075, the Qt style\n> > > `CTRL+,` should be translated to `Control-comma`, for example. In\n> > > particular, keystrokes specified in the format indicated at\n> > > https://doc.qt.io/archives/qt-4.8/qkeysequence.html#QKeySequence-2 to\n> > > the format indicated at https://www.tcl.tk/man/tcl8.4/TkCmd/keysyms.htm.\n> > >\n> > > However, it might not even need to put in _such_ a lot of work: in my\n> > > tests, `Control-,` worked just as well as `Control-comma`. To test this\n> > > for yourself, use this snippet (that is slightly modified from the\n> > > example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so\n> > > that it reacts _only_ to Control+comma instead of all keys):\n> >\n> > Another benefit to the translation framework would be that we could also\n> > generate the menu labels (aka \"accelerator\") for the tools, instead of\n> > making the user specify both the shortcut and the label.\n> >\n> > > -- snip --\n> > > set keysym \"Press any key\"\n> > > pack [label .l -textvariable keysym -padx 2m -pady 1m]\n> > > #bind . <Key> {\n> > > bind . <Control-,> {\n> > >     set keysym \"You pressed %K\"\n> > > }\n> > > -- snap --\n> > >\n> > > So I could imagine that something like this could serve as an initial\n> > > draft for a function that you can turn into a \"good enough\" version:\n> > >\n> > > -- snip --\n> > > proc QKeySequence2keysym {keystroke} {\n> > > \tregsub -all {(?i)Ctrl\\+} $keystroke \"Control-\" keystroke\n> > > \tregsub -all {(?i)Alt\\+} $keystroke \"Alt-\" keystroke\n> > > \tregsub -all {(?i)Shift\\+} $keystroke \"Shift-\" keystroke\n> > > \treturn $keystroke\n> > > }\n> > > -- snap --\n> > >\n> > > That way, you don't have to introduce settings separate from\n> > > `git-cola`'s, and you can reuse the short-and-sweet variable name.\n> >\n> > I think a more important question is whether we _really_ need to have\n> > compatibility with git-cola. Most of our shortcuts don't match with\n> > them, so is it really worth the effort to try to keep compatibility?\n> >\n> > I'm not against something like this, but just want to be sure we\n> > evaluate whether the effort is worth it.\n> \n> `git-cola`, by virtue of being there first, squats on the neat config\n> setting name `shortcut`.\n> \n> I expect users to be utterly surprised when that name does not work for\n> them.\n\nCorrect. Makes sense.\n \n> Please note that I intended `QKeySequence2keysym` to leave parameters\n> unchanged that already refer to keysyms. So this is purely a way to\n> reuse the same name as `git-cola` while still playing nice with existing\n> configurations targeting `git-cola`.\n\nSince you already gave a draft of the function, it shouldn't be too \ndifficult to pick up from there. I'm just waiting for Harish to send in \nhis first iteration so we can first discuss that.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"383529","messageId":"f0c10e8e-75e3-bebd-4912-d2b5ecd3fa5c@iee.email","threadId":"41857","inReplyTo":"nycvar.QRO.7.76.6.1910062208460.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-10-06T22:40:23Z","receivedAt":"2019-10-06T22:40:27Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Dscho,\n\nOn 06/10/2019 21:27, Johannes Schindelin wrote:\n> Let me provide you with some data, then. Granted, it's not necessarily\n> all Git GUI, but it includes Git GUI patches, too: Git for Windows'\n> contributions.\n>\n> As should be well-known, I try to follow Postel's Law when it comes to\n> Git for Windows' patches: be lenient in the input, strict in the output.\n> As such, I don't force contributors to use GitHub PRs (although that is\n> certainly encouraged by virtue of Git for Windows' source code being\n> hosted on GitHub), or send patches, or send pull requests to their own\n> public repositories or bundles sent to the mailing list. I accept them\n> all. At least that is the idea.\n>\n> I cannot tell you how many contributions came in via GitHub PRs. I can\n> tell precisely you how many contributions were made_not_  using GitHub\n> PRs. One one hand. Actually, on zero hands.\n>\n> So clearly, at least Git for Windows' contributors (including some who\n> provided Git GUI patches) are much more comfortable with the PR workflow\n> than with the mailing list-based workflow.\nJust to say that most of the numbers are governed by the strength and \nexperience with the particular infrastructures.\n\nTonight I had wanted to send in patches for G-f-W because of branch \nplacement confusion. Eventually I had to throw in an extra rebase to a \nfresh branch, just so I could create a PR, all because of the zero \nexperience you mentioned with using a G-f-W 2-patch series.\n\nThe Git list is strongly patch based and it's infrastructure works \nadequately, even if it is 'antiquated' by millennial standards.\n\nThe G-f-W interaction is almost totally via Github, and a few \nnotification emails and occasional google-groups interactions. It's \nstill imperfect, just like the Git list emails, but with it's own, \ndifferent issues for trying to build its community.\n\nMost community bondings actually build through their common adversity, \nrather than the apparent ease of joining/leaving.\n\nThe main bit I wanted to say (I think), was that having a maintainer who \naccepts input is probably the most important aspect, no matter the \nparticular route used for the input. So thanks to both of you (Dscho, \nPratyush) for *facilitating* the contribution flow.\n-- \nPhilip\n"},{"id":"383560","messageId":"1dbb69d96229fa9400d7eae0b4fd467ab9706815.camel@gmail.com","threadId":"41857","inReplyTo":"20191005210127.uinrgazj5ezyqftj@yadavpratyush.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Harish Karumuthil","fromEmail":"harish2704@gmail.com","sentAt":"2019-10-07T06:13:06Z","receivedAt":"2019-10-07T06:13:13Z","isPatch":true,"sender":{"key":"harish2704@gmail.com","avatar":"https://gravatar.com/avatar/774092af5a4df0d7d135b48fcbe03df5b331c434506ea37eb31b3cddfb17458f?d=mp&s=160"},"body":"Hi Pratyush, Regarding your messages,\n\n>On Sun, 2019-10-06 at 02:31 +0530, Pratyush Yadav wrote:\n> You don't need to \"set up\" an email client with git-send-email. \n> git-send-email is an email client itself. Well, one which can only send \n> emails.\n\nFor now, I am sticking with a mail client ( evolution ) which does minimal\n( or atleast transparent ) preprocessing  ( Tab => space conversion , line\nwrapping  etc ).\nNow I can send patches using the output of `git diff --patch-with-stat`\ncommand and I hope is it enough for now.\nPersonaly I dont' like any solution which requires storing our mail password\nas a plain text file.\n\n\n> You haven't sent '/submit' over there, so those emails aren't in the \n> list (and my inbox) yet. You need to comment with '/submit' (without the \n> quotes) to tell GitGitGadget to send your PR as email.\n\nI thought, lets finalize discussion about all the changes here in mail\nthread   it self before submitting the patch. Otherwise, That is why I didn't\nsubmitted the patch.\n\n\n> One point I forgot to mention earlier was that I'm honestly not a big \n> fan of separating the binding and accelerator label. I understand that \n> you might not have the time to do this, but I think it is still worth \n> mentioning. Maybe I will implement something like that over your patch. \n> But it would certainly be nice if you can figure it out :).\n\nI think there is a small missunderstanind in that point.\n\nI agree that, in the initial implementation ( which I did @ 2016 ) menu\nlabels were separated from binding keys. But in the last update, it is not\nlike that.\n\nCurrently, user only need to specify single config value which is\n`guitool.<name>.gitgui-shortcut` and don't have to specify accel-lable\nseparatly.\nLabel is generated from the shortcut.\n\n\n> Either ways, detecting an existing shortcut is pretty easy. The `bind` \n> man page [1] says:\n> \n>   If sequence is specified without a script, then the script currently \n>   bound to sequence is returned, or an empty string is returned if there \n>   is no binding for sequence.\n> \n> So you can use this to find out if there is a binding conflict, and warn \n> the user.\n\nWill try this. Thanks!\n\n\n\n"},{"id":"383561","messageId":"e71835129c0628ff3b9a0653febc3737128fa23c.camel@gmail.com","threadId":"41857","inReplyTo":"nycvar.QRO.7.76.6.1910061054470.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Harish Karumuthil","fromEmail":"harish2704@gmail.com","sentAt":"2019-10-07T06:22:14Z","receivedAt":"2019-10-07T06:22:21Z","isPatch":true,"sender":{"key":"harish2704@gmail.com","avatar":"https://gravatar.com/avatar/774092af5a4df0d7d135b48fcbe03df5b331c434506ea37eb31b3cddfb17458f?d=mp&s=160"},"body":"Hi Johannes Schindelin, Regarding your messages,\n\n\n> However, it might not even need to put in _such_ a lot of work: in my\n> tests, `Control-,` worked just as well as `Control-comma`. To test this\n> for yourself, use this snippet (that is slightly modified from the\n> example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so\n> that it reacts _only_ to Control+comma instead of all keys):\n> \n> -- snip --\n> set keysym \"Press any key\"\n> pack [label .l -textvariable keysym -padx 2m -pady 1m]\n> #bind . <Key> {\n> bind . <Control-,> {\n>     set keysym \"You pressed %K\"\n> }\n> -- snap --\n\nI tried this, but unfortunatly, it didn't worked for me. My tclsh version is\n\"8.6\".  The script crashed with following error message\n\n---\nbad event type or keysym \",\"\n    while executing\n\"bind . <Control-,> {\n    set keysym \"You pressed %K\"\n}\"\n    (file \"./test.tcl\" line 6)\n---\n\n\nThe complete ( or modified ) script which I used is given below\n\n---\npackage require Tk\n\nset keysym \"Press any key\"\npack [label .l -textvariable keysym -padx 2m -pady 1m]\n#bind . <Key> {\nbind . <Control-,> {\n    set keysym \"You pressed %K\"\n}\n\n---\n\nFrom the error messages, I understand that, \"<Control-,>\" will not work\ninstead of \"<Control-comma>\" .\n\n> \n> So I could imagine that something like this could serve as an initial\n> draft for a function that you can turn into a \"good enough\" version:\n> \n> -- snip --\n> proc QKeySequence2keysym {keystroke} {\n> \tregsub -all {(?i)Ctrl\\+} $keystroke \"Control-\" keystroke\n> \tregsub -all {(?i)Alt\\+} $keystroke \"Alt-\" keystroke\n> \tregsub -all {(?i)Shift\\+} $keystroke \"Shift-\" keystroke\n> \treturn $keystroke\n> }\n> -- snap --\n> \n> That way, you don't have to introduce settings separate from\n> `git-cola`'s, and you can reuse the short-and-sweet variable name.\n\nIf my previous observation is correct, then we may have to translate a list\nof key names ( in addition to atl,ctrl & shirt ) to get it working .\n\n\n> Ciao,\n> Johannes\n\n"},{"id":"383563","messageId":"nycvar.QRO.7.76.6.1910071101590.46@tvgsbejvaqbjf.bet","threadId":"41857","inReplyTo":"20191006210647.wfjr7lhw5fxs4bin@yadavpratyush.com","subject":"GitGUIGadget, was Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-07T09:20:11Z","receivedAt":"2019-10-07T09:20:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Pratyush,\n\nOn Mon, 7 Oct 2019, Pratyush Yadav wrote:\n\n> On 06/10/19 10:27PM, Johannes Schindelin wrote:\n> >\n> > On Mon, 7 Oct 2019, Pratyush Yadav wrote:\n> >\n> > > On 06/10/19 11:49AM, Johannes Schindelin wrote:\n> > > >\n> > > > On Sun, 6 Oct 2019, Pratyush Yadav wrote:\n> > > >\n> > > > > On 06/10/19 01:46AM, Harish Karumuthil wrote:\n> > > > > >\n> > > > > > From\n> > > > > > https://www.kernel.org/doc/html/v4.10/process/email-clients.html,\n> > > > > > I understood that, my current email client ( that is gmail\n> > > > > > web ) is not good for submitting patches. So I was tying to\n> > > > > > setup a mail client which is compatible with `git\n> > > > > > send-mail`. But I was not able to get a satisfactory result\n> > > > > > in that.\n> > > > >\n> > > > > You don't need to \"set up\" an email client with\n> > > > > git-send-email.  git-send-email is an email client itself.\n> > > > > Well, one which can only send emails.\n> > > >\n> > > > It also cannot reply to mails on the mailing list.\n> > > >\n> > > > It cannot even notify you when anybody replied to your patch.\n> > > >\n> > > > Two rather problematic aspects when it comes to patch\n> > > > contributions: how are you supposed to work with the reviewers\n> > > > when you lack all the tools to _interact_ with them? All `git\n> > > > send-email` provides is a \"fire and forget\" way to send patches,\n> > > > i.e. it encourages a monologue, when you want to start a\n> > > > dialogue instead.\n> > >\n> > > Well, I started with email based patch contribution when I was\n> > > first got started with open source, so I might be a bit biased,\n> > > but in my experience, it is not that difficult to set all these\n> > > things up. Most of the time, all you need to tell git-send-email\n> > > is your SMTP server, username, and password. All pretty easy\n> > > things to do.\n> >\n> > Okay, set it up with a corporate Exchange server.\n> >\n> > I'll be waiting right here.\n>\n> I admit, I've never had to do that. And by how you word it, hope I\n> never have to do it in the future either.\n\nAnd I hope that you make peace with the fact that you prevent any\ncorporate developer from contributing easily.\n\n> > > And you add in your email client (which pretty much everyone\n> > > should have), and it is a complete setup. I personally use neomutt\n> > > as my email client, but I have used Thunderbird before, and it is\n> > > really easy to set it up to send plain text emails. All you need\n> > > to do is hold Shift before hitting reply, and you're in plain text\n> > > mode. And you can even make it use plain text by default by\n> > > flipping a switch in the settings.\n> >\n> > How intuitive. And of course Thunderbird still messes up the patches\n> > so that they won't apply, unless you *checks notes* do things that\n> > are quite involved or *checks notes* do other things that are quite\n> > involved.\n>\n> Ha! Made me chuckle. You got me there :). I suppose it isn't _that_\n> simple and I'm just biased because I am so used to it.\n\nI assumed that you were comfortable with it, and a bit oblivious about\nthe hurdle that this represents to the majority of potential\ncontributors.\n\nRemember, one of the beautiful things GitHub has given the world _on\ntop_ of Git is how easy one-off contributions are.\n\n> > > So while I agree with you that there is certainly a learning curve\n> > > involved, I don't think it is all too bad. But again, that is all my\n> > > personal opinion, and nothing based on facts or data.\n> >\n> > Let me provide you with some data, then. Granted, it's not necessarily\n> > all Git GUI, but it includes Git GUI patches, too: Git for Windows'\n> > contributions.\n> >\n> > As should be well-known, I try to follow Postel's Law when it comes to\n> > Git for Windows' patches: be lenient in the input, strict in the output.\n> > As such, I don't force contributors to use GitHub PRs (although that is\n> > certainly encouraged by virtue of Git for Windows' source code being\n> > hosted on GitHub), or send patches, or send pull requests to their own\n> > public repositories or bundles sent to the mailing list. I accept them\n> > all. At least that is the idea.\n> >\n> > I cannot tell you how many contributions came in via GitHub PRs. I can\n> > tell precisely you how many contributions were made _not_ using GitHub\n> > PRs. One one hand. Actually, on zero hands.\n> >\n> > So clearly, at least Git for Windows' contributors (including some who\n> > provided Git GUI patches) are much more comfortable with the PR workflow\n> > than with the mailing list-based workflow.\n>\n> I never said email is better that GitHub PRs. It isn't. My point was\n> that using email isn't _that_ hard. When I first did it, it maybe took\n> me 3-4 hours to figure everything out, and then I was set forever. I\n> carry around the same '.gitconfig' file to all my setups, and everything\n> \"just works\".\n>\n> So yes, GitHub PRs are certainly easier, but email wasn't too difficult\n> in my experience. But then I'm a kernel developer, so I'm a minority to\n> begin with.\n>\n> I suspect you've had this debate more than once, because you come in\n> guns blazing ;)\n\nI come in guns blazing because these obstacles that are put in the way\nof contributors are the opposite of what I consider inclusive and\nwelcoming.\n\nThe fact that I am blessed with a lot of privilege (which, let's face\nit, I did nothing to earn) does not mean that I want to discount those\nwho do not have that privilege. I have the time to contribute to Open\nSource, which is a privilege. I have the education to do so, with is a\nprivilege. I even have the time to struggle with a mailing list-based\ncode contribution process, which is a privilege I imagine only\npreciously few people enjoy.\n\nSo I work as hard as I can against obstacles that are essentially big\n\"Keep Out\" signs (or, if you will, a big middle finger) to contributors\nwithout these privileges.\n\n> > > [... talking about GitGitGadget...]\n> > >\n> > > One  feature that would make it complete would be the ability to\n> > > reply to review comments.\n> >\n> > And how would that work, exactly? How to determine *which* email to\n> > respond to? *Which* person to reply to? *What* to quote?\n>\n> GGG already shows replies to the patches as a comment.\n\nYes.\n\n(I know, I implemented this.)\n\n> On GitHub you can \"Quote reply\" a comment, which quotes the entire\n> comment just like your MUA would.\n\nOn GitHub, you can also select part of the comment and press the `r`\nkey, which results in the equivalent of what I am doing right here:\nquoting part of your mail and responding to just that part.\n\nYou can also just reply without quoting.\n\nThese are three ways to reply to comments on GitHub, and in my\nexperience the rarest form is the full quote, the most common form is\nthe \"no quote\" form. (Which makes sense because you already have\neverything in that UI, you don't need to quote unless you need to make a\npoint about only a certain part of what you are replying to, and only if\nthat point might be otherwise missed.)\n\nSomething I also saw often enough is to accumulate quotes from multiple\ncomments in the same reply.\n\n> Then you can write your reply there, and the last line would be\n> '/reply', which would make GGG send that email as a reply. You would\n> need to strip the first line from the reply because GGG starts the\n> reply with something like:\n>\n>   > [On the Git mailing\n>   > list](https://public-inbox.org/git/xmqq7e5l9zb1.fsf@gitster-ct.c.googlers.com),\n>   > Junio C Hamano wrote ([reply to\n>   > this](https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis)):\n>\n> GGG also adds 3 backticks before and after the reply content, so those\n> would need to be removed too.\n\nApart from the problems to identify the correct mail to reply to\n(unless, as you suggested, the Message-ID is part of the quoted part by\nvirtue of including that public-inbox URL), I think it would make it\ncumbersome to require the `/reply` command. Quite honestly, I would\nprefer it if GitGitGadget would simply send replies whenever it can\nfigure out to who to send, and which Message-ID to reply to.\n\n> > > This would remove the need for an email client (almost)\n> > > completely. I have never written Typescript or used Azure\n> > > pipelines ever, but I can try tinkering around to see if I can\n> > > figure out how to do something like that. Unless, of course, you\n> > > or someone else is already doing it.  If not, some pointers would\n> > > be appreciated.\n> >\n> > Feel free to give this challenge a try.\n>\n> The first challenge is learning Typescript :)\n\nI learned Typescript to implement GitGitGadget. It maybe took me 3-4\nhours to figure everything out, and then I was set forever. :-P\n\nCiao,\nJohannes\n"},{"id":"383572","messageId":"nycvar.QRO.7.76.6.1910071159530.46@tvgsbejvaqbjf.bet","threadId":"41857","inReplyTo":"e71835129c0628ff3b9a0653febc3737128fa23c.camel@gmail.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-07T10:01:47Z","receivedAt":"2019-10-07T10:02:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Harish,\n\nOn Mon, 7 Oct 2019, Harish Karumuthil wrote:\n\n> > However, it might not even need to put in _such_ a lot of work: in my\n> > tests, `Control-,` worked just as well as `Control-comma`. To test this\n> > for yourself, use this snippet (that is slightly modified from the\n> > example at the bottom of https://www.tcl.tk/man/tcl/TkCmd/bind.htm so\n> > that it reacts _only_ to Control+comma instead of all keys):\n> >\n> > -- snip --\n> > set keysym \"Press any key\"\n> > pack [label .l -textvariable keysym -padx 2m -pady 1m]\n> > #bind . <Key> {\n> > bind . <Control-,> {\n> >     set keysym \"You pressed %K\"\n> > }\n> > -- snap --\n>\n> I tried this, but unfortunatly, it didn't worked for me. My tclsh version is\n> \"8.6\".  The script crashed with following error message\n>\n> ---\n> bad event type or keysym \",\"\n>     while executing\n> \"bind . <Control-,> {\n>     set keysym \"You pressed %K\"\n> }\"\n>     (file \"./test.tcl\" line 6)\n> ---\n\nThat's too bad! I tested this on Windows, and I imagine it just does not\nwork on Linux/macOS...\n\n> The complete ( or modified ) script which I used is given below\n>\n> ---\n> package require Tk\n>\n> set keysym \"Press any key\"\n> pack [label .l -textvariable keysym -padx 2m -pady 1m]\n> #bind . <Key> {\n> bind . <Control-,> {\n>     set keysym \"You pressed %K\"\n> }\n>\n> ---\n>\n> From the error messages, I understand that, \"<Control-,>\" will not work\n> instead of \"<Control-comma>\" .\n>\n> >\n> > So I could imagine that something like this could serve as an initial\n> > draft for a function that you can turn into a \"good enough\" version:\n> >\n> > -- snip --\n> > proc QKeySequence2keysym {keystroke} {\n> > \tregsub -all {(?i)Ctrl\\+} $keystroke \"Control-\" keystroke\n> > \tregsub -all {(?i)Alt\\+} $keystroke \"Alt-\" keystroke\n> > \tregsub -all {(?i)Shift\\+} $keystroke \"Shift-\" keystroke\n> > \treturn $keystroke\n> > }\n> > -- snap --\n> >\n> > That way, you don't have to introduce settings separate from\n> > `git-cola`'s, and you can reuse the short-and-sweet variable name.\n>\n> If my previous observation is correct, then we may have to translate a list\n> of key names ( in addition to atl,ctrl & shirt ) to get it working .\n\nI fear you're right. Hopefully we can get away with a relatively short\nlist...\n\nCiao,\nJohannes\n"},{"id":"383575","messageId":"CAGr--=JpC88+Bnc233aJRe1XDgPzZxThxHQShoDobBA6hcYChQ@mail.gmail.com","threadId":"41857","inReplyTo":"nycvar.QRO.7.76.6.1910071101590.46@tvgsbejvaqbjf.bet","subject":"Re: GitGUIGadget, was Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Birger Skogeng Pedersen","fromEmail":"birgersp@gmail.com","sentAt":"2019-10-07T10:43:30Z","receivedAt":"2019-10-07T10:46:32Z","isPatch":true,"sender":{"key":"birgersp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"It seems this topic has kindof derailed(?). But I feel like voicing my\nopinion nonetheless.\n\nI have to say I absolutely agree with Johannes Schindelin here. I\nwould really prefer a more modern way of contributing to this project\n(any software project, really) than using emails. Be that Github,\nGitlab, bitbucket or whatever else.\n\nBut no it's not *impossible* to learn how to send patches by email.\nBut IMO it is a less efficient way to develop and maintain an open\nsource project. If getting more people interested in contributing,\nthere is no doubt in my mind that using Github would be better.\n\nMy biggest gripe with not using Github is how to keep track of replies\n(comments) to a topic. I have to navigate through multiple emails to\nget and overview of what everyone has been saying, when it should just\nbe a single collection of replies from everyone (like in a Github\nissue). Where each issue has their own thread, and they can be linked\nto each other or to PRs.\n\n\nJust felt like chiming in,\nBirger\n\nOn Mon, Oct 7, 2019 at 11:21 AM Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>\n> Hi Pratyush,\n>\n> On Mon, 7 Oct 2019, Pratyush Yadav wrote:\n>\n> > On 06/10/19 10:27PM, Johannes Schindelin wrote:\n> > >\n> > > On Mon, 7 Oct 2019, Pratyush Yadav wrote:\n> > >\n> > > > On 06/10/19 11:49AM, Johannes Schindelin wrote:\n> > > > >\n> > > > > On Sun, 6 Oct 2019, Pratyush Yadav wrote:\n> > > > >\n> > > > > > On 06/10/19 01:46AM, Harish Karumuthil wrote:\n> > > > > > >\n> > > > > > > From\n> > > > > > > https://www.kernel.org/doc/html/v4.10/process/email-clients.html,\n> > > > > > > I understood that, my current email client ( that is gmail\n> > > > > > > web ) is not good for submitting patches. So I was tying to\n> > > > > > > setup a mail client which is compatible with `git\n> > > > > > > send-mail`. But I was not able to get a satisfactory result\n> > > > > > > in that.\n> > > > > >\n> > > > > > You don't need to \"set up\" an email client with\n> > > > > > git-send-email.  git-send-email is an email client itself.\n> > > > > > Well, one which can only send emails.\n> > > > >\n> > > > > It also cannot reply to mails on the mailing list.\n> > > > >\n> > > > > It cannot even notify you when anybody replied to your patch.\n> > > > >\n> > > > > Two rather problematic aspects when it comes to patch\n> > > > > contributions: how are you supposed to work with the reviewers\n> > > > > when you lack all the tools to _interact_ with them? All `git\n> > > > > send-email` provides is a \"fire and forget\" way to send patches,\n> > > > > i.e. it encourages a monologue, when you want to start a\n> > > > > dialogue instead.\n> > > >\n> > > > Well, I started with email based patch contribution when I was\n> > > > first got started with open source, so I might be a bit biased,\n> > > > but in my experience, it is not that difficult to set all these\n> > > > things up. Most of the time, all you need to tell git-send-email\n> > > > is your SMTP server, username, and password. All pretty easy\n> > > > things to do.\n> > >\n> > > Okay, set it up with a corporate Exchange server.\n> > >\n> > > I'll be waiting right here.\n> >\n> > I admit, I've never had to do that. And by how you word it, hope I\n> > never have to do it in the future either.\n>\n> And I hope that you make peace with the fact that you prevent any\n> corporate developer from contributing easily.\n>\n> > > > And you add in your email client (which pretty much everyone\n> > > > should have), and it is a complete setup. I personally use neomutt\n> > > > as my email client, but I have used Thunderbird before, and it is\n> > > > really easy to set it up to send plain text emails. All you need\n> > > > to do is hold Shift before hitting reply, and you're in plain text\n> > > > mode. And you can even make it use plain text by default by\n> > > > flipping a switch in the settings.\n> > >\n> > > How intuitive. And of course Thunderbird still messes up the patches\n> > > so that they won't apply, unless you *checks notes* do things that\n> > > are quite involved or *checks notes* do other things that are quite\n> > > involved.\n> >\n> > Ha! Made me chuckle. You got me there :). I suppose it isn't _that_\n> > simple and I'm just biased because I am so used to it.\n>\n> I assumed that you were comfortable with it, and a bit oblivious about\n> the hurdle that this represents to the majority of potential\n> contributors.\n>\n> Remember, one of the beautiful things GitHub has given the world _on\n> top_ of Git is how easy one-off contributions are.\n>\n> > > > So while I agree with you that there is certainly a learning curve\n> > > > involved, I don't think it is all too bad. But again, that is all my\n> > > > personal opinion, and nothing based on facts or data.\n> > >\n> > > Let me provide you with some data, then. Granted, it's not necessarily\n> > > all Git GUI, but it includes Git GUI patches, too: Git for Windows'\n> > > contributions.\n> > >\n> > > As should be well-known, I try to follow Postel's Law when it comes to\n> > > Git for Windows' patches: be lenient in the input, strict in the output.\n> > > As such, I don't force contributors to use GitHub PRs (although that is\n> > > certainly encouraged by virtue of Git for Windows' source code being\n> > > hosted on GitHub), or send patches, or send pull requests to their own\n> > > public repositories or bundles sent to the mailing list. I accept them\n> > > all. At least that is the idea.\n> > >\n> > > I cannot tell you how many contributions came in via GitHub PRs. I can\n> > > tell precisely you how many contributions were made _not_ using GitHub\n> > > PRs. One one hand. Actually, on zero hands.\n> > >\n> > > So clearly, at least Git for Windows' contributors (including some who\n> > > provided Git GUI patches) are much more comfortable with the PR workflow\n> > > than with the mailing list-based workflow.\n> >\n> > I never said email is better that GitHub PRs. It isn't. My point was\n> > that using email isn't _that_ hard. When I first did it, it maybe took\n> > me 3-4 hours to figure everything out, and then I was set forever. I\n> > carry around the same '.gitconfig' file to all my setups, and everything\n> > \"just works\".\n> >\n> > So yes, GitHub PRs are certainly easier, but email wasn't too difficult\n> > in my experience. But then I'm a kernel developer, so I'm a minority to\n> > begin with.\n> >\n> > I suspect you've had this debate more than once, because you come in\n> > guns blazing ;)\n>\n> I come in guns blazing because these obstacles that are put in the way\n> of contributors are the opposite of what I consider inclusive and\n> welcoming.\n>\n> The fact that I am blessed with a lot of privilege (which, let's face\n> it, I did nothing to earn) does not mean that I want to discount those\n> who do not have that privilege. I have the time to contribute to Open\n> Source, which is a privilege. I have the education to do so, with is a\n> privilege. I even have the time to struggle with a mailing list-based\n> code contribution process, which is a privilege I imagine only\n> preciously few people enjoy.\n>\n> So I work as hard as I can against obstacles that are essentially big\n> \"Keep Out\" signs (or, if you will, a big middle finger) to contributors\n> without these privileges.\n>\n> > > > [... talking about GitGitGadget...]\n> > > >\n> > > > One  feature that would make it complete would be the ability to\n> > > > reply to review comments.\n> > >\n> > > And how would that work, exactly? How to determine *which* email to\n> > > respond to? *Which* person to reply to? *What* to quote?\n> >\n> > GGG already shows replies to the patches as a comment.\n>\n> Yes.\n>\n> (I know, I implemented this.)\n>\n> > On GitHub you can \"Quote reply\" a comment, which quotes the entire\n> > comment just like your MUA would.\n>\n> On GitHub, you can also select part of the comment and press the `r`\n> key, which results in the equivalent of what I am doing right here:\n> quoting part of your mail and responding to just that part.\n>\n> You can also just reply without quoting.\n>\n> These are three ways to reply to comments on GitHub, and in my\n> experience the rarest form is the full quote, the most common form is\n> the \"no quote\" form. (Which makes sense because you already have\n> everything in that UI, you don't need to quote unless you need to make a\n> point about only a certain part of what you are replying to, and only if\n> that point might be otherwise missed.)\n>\n> Something I also saw often enough is to accumulate quotes from multiple\n> comments in the same reply.\n>\n> > Then you can write your reply there, and the last line would be\n> > '/reply', which would make GGG send that email as a reply. You would\n> > need to strip the first line from the reply because GGG starts the\n> > reply with something like:\n> >\n> >   > [On the Git mailing\n> >   > list](https://public-inbox.org/git/xmqq7e5l9zb1.fsf@gitster-ct.c.googlers.com),\n> >   > Junio C Hamano wrote ([reply to\n> >   > this](https://github.com/gitgitgadget/gitgitgadget/wiki/ReplyToThis)):\n> >\n> > GGG also adds 3 backticks before and after the reply content, so those\n> > would need to be removed too.\n>\n> Apart from the problems to identify the correct mail to reply to\n> (unless, as you suggested, the Message-ID is part of the quoted part by\n> virtue of including that public-inbox URL), I think it would make it\n> cumbersome to require the `/reply` command. Quite honestly, I would\n> prefer it if GitGitGadget would simply send replies whenever it can\n> figure out to who to send, and which Message-ID to reply to.\n>\n> > > > This would remove the need for an email client (almost)\n> > > > completely. I have never written Typescript or used Azure\n> > > > pipelines ever, but I can try tinkering around to see if I can\n> > > > figure out how to do something like that. Unless, of course, you\n> > > > or someone else is already doing it.  If not, some pointers would\n> > > > be appreciated.\n> > >\n> > > Feel free to give this challenge a try.\n> >\n> > The first challenge is learning Typescript :)\n>\n> I learned Typescript to implement GitGitGadget. It maybe took me 3-4\n> hours to figure everything out, and then I was set forever. :-P\n>\n> Ciao,\n> Johannes\n"},{"id":"383603","messageId":"86599b38-fe3c-903e-2f52-22cacea06e8d@gmail.com","threadId":"41857","inReplyTo":"CAGr--=JpC88+Bnc233aJRe1XDgPzZxThxHQShoDobBA6hcYChQ@mail.gmail.com","subject":"Re: GitGUIGadget, was Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Alban Gruin","fromEmail":"alban.gruin@gmail.com","sentAt":"2019-10-07T19:16:43Z","receivedAt":"2019-10-07T19:16:55Z","isPatch":true,"sender":{"key":"alban.gruin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/6310153?v=4"},"body":"Hi Birger,\n\nLe 07/10/2019 à 12:43, Birger Skogeng Pedersen a écrit :\n> It seems this topic has kindof derailed(?). But I feel like voicing my\n> opinion nonetheless.\n> \n> -%<-\n> \n> My biggest gripe with not using Github is how to keep track of replies\n> (comments) to a topic. I have to navigate through multiple emails to\n> get and overview of what everyone has been saying, when it should just\n> be a single collection of replies from everyone (like in a Github\n> issue). Where each issue has their own thread, and they can be linked\n> to each other or to PRs.\n> \n\nI don’t know which email client you use, but they usually have an option\nto sort messages by thread -- this is how I handle my git folder with\nThunderbird; it takes one click to switch from plain view to threaded\nview, and two to isolate a specific thread.  public-inbox.org also does\nthis.  Some don’t though, and gmail specifically has weird opinions on\nwhich message is part of which thread.\n\nOn the other hand, I don’t really like non-threaded means of\ncommunication, and I am very confused as to why modern services neglect\nthis feature -- there may be a good reason, I’m not an expert in\nhuman-computer interaction :).  But personally, I find it very hard to\nfollow issues on github (or even IRC logs, for that matter) when there\nis more than 2 people involved.\n\n> \n> Just felt like chiming in,\n> Birger\n> \nCheers,\nAlban\n\n"},{"id":"383704","messageId":"f751705949a7fd23c77cbbf839c081b95b12394b.camel@gmail.com","threadId":"41857","inReplyTo":"nycvar.QRO.7.76.6.1910071159530.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Harish Karumuthil","fromEmail":"harish2704@gmail.com","sentAt":"2019-10-08T19:31:59Z","receivedAt":"2019-10-08T19:32:06Z","isPatch":true,"sender":{"key":"harish2704@gmail.com","avatar":"https://gravatar.com/avatar/774092af5a4df0d7d135b48fcbe03df5b331c434506ea37eb31b3cddfb17458f?d=mp&s=160"},"body":"Hi all, there is an update:\n\nI added necessary error catching code so that, script will not crash if the\nkeybinding code is worng. Instead of crashing it will print error message.\nThe final patch will look something like this.\n\n---\n lib/tools.tcl | 24 ++++++++++++++++++++----\n 1 file changed, 20 insertions(+), 4 deletions(-)\n\ndiff --git a/lib/tools.tcl b/lib/tools.tcl\nindex 413f1a1700..3135e19131 100644\n--- a/lib/tools.tcl\n+++ b/lib/tools.tcl\n@@ -38,7 +38,7 @@ proc tools_create_item {parent args} {\n }\n \n proc tools_populate_one {fullname} {\n-\tglobal tools_menubar tools_menutbl tools_id\n+\tglobal tools_menubar tools_menutbl tools_id repo_config\n \n \tif {![info exists tools_id]} {\n \t\tset tools_id 0\n@@ -61,9 +61,25 @@ proc tools_populate_one {fullname} {\n \t\t}\n \t}\n \n-\ttools_create_item $parent command \\\n-\t\t-label [lindex $names end] \\\n-\t\t-command [list tools_exec $fullname]\n+\tset accel_key_bound 0\n+\tif {[info exists repo_config(guitool.$fullname.gitgui-shortcut)]} {\n+\t\tset accel_key $repo_config(guitool.$fullname.gitgui-shortcut)\n+\t\tif { [ catch { bind . <$accel_key> [list tools_exec $fullname] } msg ] } {\n+\t\t\tputs stderr \"Failed to bind keyboard shortcut '$accel_key' for custom tool '$fullname'. Error: $msg\"\n+\t\t} else {\n+\t\t\ttools_create_item $parent command \\\n+\t\t\t-label [lindex $names end] \\\n+\t\t\t-command [list tools_exec $fullname] \\\n+\t\t\t-accelerator $accel_key\n+\t\t\tset accel_key_bound true\n+\t\t}\n+\t}\n+\n+\tif { ! $accel_key_bound } {\n+\t\ttools_create_item $parent command \\\n+\t\t\t-label [lindex $names end] \\\n+\t\t\t-command [list tools_exec $fullname]\n+\t}\n }\n \n proc tools_exec {fullname} {\n---\n\n@Johannes Schindelin: In short, from your previous message I understand point.\n\n1. shortcut codes like \"<Control-,>\" will only in Windows platform. It may not work in Linux / Mac.\n2. We need do translate shortcut codes somehow ( using one-to-one maping ).\n\nIf this is correct, do you have any example on how to do one-to-one maping of a list of string on TCL ?\n\n"},{"id":"383763","messageId":"nycvar.QRO.7.76.6.1910092240190.46@tvgsbejvaqbjf.bet","threadId":"41857","inReplyTo":"f751705949a7fd23c77cbbf839c081b95b12394b.camel@gmail.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-09T20:42:56Z","receivedAt":"2019-10-09T20:43:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 9 Oct 2019, Harish Karumuthil wrote:\n\n> @Johannes Schindelin: In short, from your previous message I understand point.\n>\n> 1. shortcut codes like \"<Control-,>\" will only in Windows platform. It may not work in Linux / Mac.\n> 2. We need do translate shortcut codes somehow ( using one-to-one maping ).\n>\n> If this is correct, do you have any example on how to do one-to-one maping of a list of string on TCL ?\n\nThis took much longer to find than I expected, probably my web search fu\nis deserting me. But I found something: `string map`, see\nhttps://tcl.tk/man/tcl8.6/TclCmd/string.htm#M34 for details.\n\nCiao,\nJohannes\n"},{"id":"383997","messageId":"20191013191710.535nho3pec2c5wlk@yadavpratyush.com","threadId":"41857","inReplyTo":"1dbb69d96229fa9400d7eae0b4fd467ab9706815.camel@gmail.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-10-13T19:17:10Z","receivedAt":"2019-10-13T19:17:17Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi Harish,\n\nSorry for the late reply. Couldn't find much time last few days.\n\nOn 07/10/19 11:43AM, Harish Karumuthil wrote:\n> Hi Pratyush, Regarding your messages,\n> \n> >On Sun, 2019-10-06 at 02:31 +0530, Pratyush Yadav wrote:\n> > You don't need to \"set up\" an email client with git-send-email. \n> > git-send-email is an email client itself. Well, one which can only send \n> > emails.\n> \n> For now, I am sticking with a mail client ( evolution ) which does minimal\n> ( or atleast transparent ) preprocessing  ( Tab => space conversion , line\n> wrapping  etc ).\n> Now I can send patches using the output of `git diff --patch-with-stat`\n> command and I hope is it enough for now.\n> Personaly I dont' like any solution which requires storing our mail password\n> as a plain text file.\n\nI'm afraid this won't work. The '.patch' file that `git-format-patch` \ngenerates also contains your commit message and the author information. \nAll those are needed to properly convert your patch to a commit in my \nrepo. The output of `git diff --patch-with-stat` won't be enough.\n\nAs for not wanting to store your mail password in a plain text file, \ncheck out [0].\n\nAnd then there is GitGitGadget too, which I'd recommend since you seem \nto be having trouble sending patches directly :).\n \n> > You haven't sent '/submit' over there, so those emails aren't in the \n> > list (and my inbox) yet. You need to comment with '/submit' (without \n> > the quotes) to tell GitGitGadget to send your PR as email.\n> \n> I thought, lets finalize discussion about all the changes here in mail\n> thread   it self before submitting the patch. Otherwise, That is why I didn't\n> submitted the patch.\n\nMakes sense.\n \n> > One point I forgot to mention earlier was that I'm honestly not a big \n> > fan of separating the binding and accelerator label. I understand that \n> > you might not have the time to do this, but I think it is still worth \n> > mentioning. Maybe I will implement something like that over your patch. \n> > But it would certainly be nice if you can figure it out :).\n> \n> I think there is a small missunderstanind in that point.\n> \n> I agree that, in the initial implementation ( which I did @ 2016 ) menu\n> labels were separated from binding keys. But in the last update, it is not\n> like that.\n> \n> Currently, user only need to specify single config value which is\n> `guitool.<name>.gitgui-shortcut` and don't have to specify accel-lable\n> separatly.\n> Label is generated from the shortcut.\n\nThanks for clarifying. It indeed was a misunderstanding.\n \n> > Either ways, detecting an existing shortcut is pretty easy. The `bind` \n> > man page [1] says:\n> > \n> >   If sequence is specified without a script, then the script currently \n> >   bound to sequence is returned, or an empty string is returned if there \n> >   is no binding for sequence.\n> > \n> > So you can use this to find out if there is a binding conflict, and warn \n> > the user.\n> \n> Will try this. Thanks!\n\n[0] https://www.softwaredeveloper.blog/git-credential-storage-libsecret\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"383998","messageId":"20191013200929.72giwtxlt6ivitfr@yadavpratyush.com","threadId":"41857","inReplyTo":"f751705949a7fd23c77cbbf839c081b95b12394b.camel@gmail.com","subject":"Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-10-13T20:09:29Z","receivedAt":"2019-10-13T20:09:36Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 09/10/19 01:01AM, Harish Karumuthil wrote:\n> Hi all, there is an update:\n> \n> I added necessary error catching code so that, script will not crash if the\n> keybinding code is worng. Instead of crashing it will print error message.\n> The final patch will look something like this.\n\nLike I mentioned another reply I wrote just now, this unfortunately is \nnot a \"proper\" patch because it does not contain the subject and commit \nmessage of your commit. Just the diff is not enough, and needs the \ncommit subject and message too.\n\nAnd even for just \"preview\" patches, I would still recommend sending a \nproper patch so people can also peek at the commit message too. You can \npass '-rfc' to `git-format-patch` to have something like '[RFC PATCH]' \nin your email subject so people know it is a preview. \"RFC\" stands for \n\"Request For Comments\".\n\nSome comments below.\n \n> ---\n>  lib/tools.tcl | 24 ++++++++++++++++++++----\n>  1 file changed, 20 insertions(+), 4 deletions(-)\n> \n> diff --git a/lib/tools.tcl b/lib/tools.tcl\n> index 413f1a1700..3135e19131 100644\n> --- a/lib/tools.tcl\n> +++ b/lib/tools.tcl\n> @@ -38,7 +38,7 @@ proc tools_create_item {parent args} {\n>  }\n>  \n>  proc tools_populate_one {fullname} {\n> -\tglobal tools_menubar tools_menutbl tools_id\n> +\tglobal tools_menubar tools_menutbl tools_id repo_config\n>  \n>  \tif {![info exists tools_id]} {\n>  \t\tset tools_id 0\n> @@ -61,9 +61,25 @@ proc tools_populate_one {fullname} {\n>  \t\t}\n>  \t}\n>  \n> -\ttools_create_item $parent command \\\n> -\t\t-label [lindex $names end] \\\n> -\t\t-command [list tools_exec $fullname]\n> +\tset accel_key_bound 0\n> +\tif {[info exists repo_config(guitool.$fullname.gitgui-shortcut)]} {\n> +\t\tset accel_key $repo_config(guitool.$fullname.gitgui-shortcut)\n> +\t\tif { [ catch { bind . <$accel_key> [list tools_exec $fullname] } msg ] } {\n\nThis has inconsistent style. There should not be any spaces between '{' \nand '['. So this line should look something like:\n\n  if {[catch {bind . <$accel_key> [list tools_exec $fullname]} msg]} {\n\nYou can look at the code around yours to pick up on the general style. \n\n> +\t\t\tputs stderr \"Failed to bind keyboard shortcut '$accel_key' for custom tool '$fullname'. Error: $msg\"\n\nPutting the error message of a GUI application on stderr is probably not \na good idea. Firstly, since it is a GUI application, we should show \nerror messages in the GUI. And secondly, a lot of the time, people \nprobably don't even launch git-gui from a terminal command, and do it \nvia some application launcher. In that case, there is no stderr that the \nuser can easily read from.\n\nSo please use a popup dialog instead. Functions to easily create them \ncan be found in lib/error.tcl. I'd recommend either `warn_popup` or \n`error_popup`.\n\nAs an example, this is what I got when I added a bad shortcut:\n\n  Failed to bind keyboard shortcut 'Ctrl-Z' for custom tool 'Foo'. Error: bad event type or keysym \"Ctrl\"\n\nShowing the error message from `bind` is pretty neat! The user can know \n_exactly_ what's wrong. One problem is that this might not make that \nmuch sense to a non-Tcler. But I still think giving a hint of the why it \nfailed is a good idea.\n\nI'd like to hear other people's thoughts about it though.\n\n> +\t\t} else {\n> +\t\t\ttools_create_item $parent command \\\n> +\t\t\t-label [lindex $names end] \\\n> +\t\t\t-command [list tools_exec $fullname] \\\n> +\t\t\t-accelerator $accel_key\n> +\t\t\tset accel_key_bound true\n\nAbove you set `accel_key_bound` to '0', and here you set it to 'true'. \nPlease use consistent forms of a boolean. Either use 'true' and 'false', \nor use '0' and '1'.\n\n> +\t\t}\n> +\t}\n> +\n> +\tif { ! $accel_key_bound } {\n\nSame style nitpick about the spaces as above.\n\n> +\t\ttools_create_item $parent command \\\n> +\t\t\t-label [lindex $names end] \\\n> +\t\t\t-command [list tools_exec $fullname]\n> +\t}\n>  }\n\nCan your whole logic of setting an accelerator in case a shortcut exists \nbe simplified a bit? Right now, the tools creation command is executed \nin two places, and it is not obvious at first sight that only one of \nthem will ever be executed. So maybe something like:\n\n  ...\n  if {[catch {bind . <$accel_key> ...} {\n  \tputs stderr ...\n  \tset accel_key_bound false\n  } else {\n  \tset accel_key_bound true\n  }\n  \n  if {accel_key_bound} {\n  \t# Create tool with accelerator\n  \t...\n  } else {\n  \t# Create tool without accelerator\n  \t...\n  }\n\nI hope you get what this means, but if you don't, please let me know, \nand I'll clarify.\n\nOverall, I like the idea of the patch. This would move us one step in \nthe direction of customizable keybindings for _all_ shortcuts. Thanks.\n\n>  \n>  proc tools_exec {fullname} {\n> ---\n> \n> @Johannes Schindelin: In short, from your previous message I understand point.\n> \n> 1. shortcut codes like \"<Control-,>\" will only in Windows platform. It may not work in Linux / Mac.\n> 2. We need do translate shortcut codes somehow ( using one-to-one maping ).\n> \n> If this is correct, do you have any example on how to do one-to-one maping of a list of string on TCL ?\n \n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"386560","messageId":"nycvar.QRO.7.76.6.1911192305410.15956@tvgsbejvaqbjf.bet","threadId":"41857","inReplyTo":"20191006210647.wfjr7lhw5fxs4bin@yadavpratyush.com","subject":"Making GitGitGadget's list -> PR comment mirroring bidirectional, was Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-19T22:09:10Z","receivedAt":"2019-11-19T22:09:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Pratyush,\n\nOn Mon, 7 Oct 2019, Pratyush Yadav wrote:\n\n> On 06/10/19 10:27PM, Johannes Schindelin wrote:\n> > Hi Pratyush,\n> >\n> > On Mon, 7 Oct 2019, Pratyush Yadav wrote:\n> >\n> > > Anyway, GitGitGadget solves a large part of the problem. It\n> > > eliminates the need for using git-send-email, and it even shows you\n> > > the replies received on the list. I honestly think it is a great\n> > > tool, and it gives people a very good alternative to using\n> > > git-send-email.\n> >\n> > GitGitGadget is just a workaround. Not even complete. Can't be\n> > complete, really. Because problems. It has much of the same problems\n> > of `git send-email`: it's a one-way conversation. Code is not\n> > discussed in the right context (which would be a worktree with the\n> > correct commit checked out). The transfer is lossy (email is designed\n> > for human-readable messages, not for transferring machine-readable\n> > serialized objects). Matching original commits and/or branches to the\n> > ones on the other side is tedious. Any interaction requires switching\n> > between many tools. Etc\n> >\n> > > One feature that would make it complete would be the ability to\n> > > reply to review comments.\n> >\n> > And how would that work, exactly? How to determine *which* email to\n> > respond to? *Which* person to reply to? *What* to quote?\n>\n> GGG already shows replies to the patches as a comment. On GitHub you can\n> \"Quote reply\" a comment, which quotes the entire comment just like your\n> MUA would. The option can be found by clicking the 3 dots on the top\n> right of a comment.\n>\n> Then you can write your reply there, and the last line would be\n> '/reply', which would make GGG send that email as a reply. You would\n> need to strip the first line from the reply because GGG starts the reply\n> with something like:\n>\n>   > [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)):\n>\n> GGG also adds 3 backticks before and after the reply content, so those\n> would need to be removed too.\n>\n> Does this sound like a sane solution?\n\nHere are two real life examples where an unsuspecting GitGitGadget user\nexpected GitGitGadget to mirror replies _to_ the Git mailing list:\n\nhttps://github.com/gitgitgadget/git/pull/451#issuecomment-555044068 and\nhttps://github.com/gitgitgadget/git/pull/451#issuecomment-555077933\n\nNeither of them include the line with the link.\n\nJust to throw a bit of real life into the discussion...\n\nCiao,\nDscho\n"},{"id":"386641","messageId":"20191120142240.pc5kfpj4eflu7dub@yadavpratyush.com","threadId":"41857","inReplyTo":"nycvar.QRO.7.76.6.1911192305410.15956@tvgsbejvaqbjf.bet","subject":"Re: Making GitGitGadget's list -> PR comment mirroring bidirectional, was Re: [PATCH] Feature: custom guitool commands can now have custom keyboard shortcuts","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-11-20T14:22:40Z","receivedAt":"2019-11-20T14:22:47Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 19/11/19 11:09PM, Johannes Schindelin wrote:\n> Hi Pratyush,\n> \n> On Mon, 7 Oct 2019, Pratyush Yadav wrote:\n> \n> > On 06/10/19 10:27PM, Johannes Schindelin wrote:\n> > > Hi Pratyush,\n> > >\n> > > On Mon, 7 Oct 2019, Pratyush Yadav wrote:\n> > >\n> > > > Anyway, GitGitGadget solves a large part of the problem. It\n> > > > eliminates the need for using git-send-email, and it even shows you\n> > > > the replies received on the list. I honestly think it is a great\n> > > > tool, and it gives people a very good alternative to using\n> > > > git-send-email.\n> > >\n> > > GitGitGadget is just a workaround. Not even complete. Can't be\n> > > complete, really. Because problems. It has much of the same problems\n> > > of `git send-email`: it's a one-way conversation. Code is not\n> > > discussed in the right context (which would be a worktree with the\n> > > correct commit checked out). The transfer is lossy (email is designed\n> > > for human-readable messages, not for transferring machine-readable\n> > > serialized objects). Matching original commits and/or branches to the\n> > > ones on the other side is tedious. Any interaction requires switching\n> > > between many tools. Etc\n> > >\n> > > > One feature that would make it complete would be the ability to\n> > > > reply to review comments.\n> > >\n> > > And how would that work, exactly? How to determine *which* email to\n> > > respond to? *Which* person to reply to? *What* to quote?\n> >\n> > GGG already shows replies to the patches as a comment. On GitHub you can\n> > \"Quote reply\" a comment, which quotes the entire comment just like your\n> > MUA would. The option can be found by clicking the 3 dots on the top\n> > right of a comment.\n> >\n> > Then you can write your reply there, and the last line would be\n> > '/reply', which would make GGG send that email as a reply. You would\n> > need to strip the first line from the reply because GGG starts the reply\n> > with something like:\n> >\n> >   > [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)):\n> >\n> > GGG also adds 3 backticks before and after the reply content, so those\n> > would need to be removed too.\n> >\n> > Does this sound like a sane solution?\n> \n> Here are two real life examples where an unsuspecting GitGitGadget user\n> expected GitGitGadget to mirror replies _to_ the Git mailing list:\n> \n> https://github.com/gitgitgadget/git/pull/451#issuecomment-555044068 and\n> https://github.com/gitgitgadget/git/pull/451#issuecomment-555077933\n> \n> Neither of them include the line with the link.\n\nCorrect.\n\nThe fundamental problem we have is that GitHub's threads are \n\"shallow\"/\"linear\". You don't reply to a reply, you reply to the main \nthread, and your comment gets appended to the end of that list. In \ncontrast, email based threads are \"deep\"/\"tree-like\". Here you can reply \nto a reply.\n\nSo the comment model of GitHub is less information-rich than the email \nmodel. The piece of information missing is \"which comment does this \ncomment reply to\". That information has to be obtained somehow, and the \nbest bet are the users themselves (by not deleting the line with the \nlink in their replies). We can give the instructions in the GGG welcome \nmessage, and hope the users read it.\n\nFrequent users and anyone who properly reads the instructions will \nprobably manage to use this just fine. Those who don't, well they \nweren't sending replies to the list to begin with. So this will help \npeople who don't want to open their email clients to send replies to the \nlist, but won't help the uninformed ones. Certainly not ideal, but I \nthink this might be the best we can do given the constraints.\n\nIn the meantime, I notice that GGG does not advertise the fact that \nreplies don't go on the list directly very well. Yes, its mentioned in \nthe welcome message, but its not instantly obvious. So maybe making it \nclearer/more noticeable will help the issue.\n\nAnother alternative might be to rely on heuristics like seeing how \nsimilar the quoted text in a reply is to the replies in the list. But I \nthink this will cause more problems than help because users can cut \nun-necessary quoting and even edit them sometimes.\n\nIf you can think of something clever that I can't, suggestions are \nwelcome :)\n\nAnyway, I probably won't have much time to work on this feature for at \nleast a couple more weeks. Maybe we'll learn more after the feature goes \nlive.\n\n> Just to throw a bit of real life into the discussion...\n> \n> Ciao,\n> Dscho\n\n-- \nRegards,\nPratyush Yadav\n"}]}