{"thread":{"id":"47883","subject":"git-gui, feature request: add hotkeys to focus different widgets","startedAt":"2018-02-20T13:32:45Z","lastAt":"2019-09-11T18:11:13Z","messageCount":46,"participants":["Birger Skogeng Pedersen","Johannes Schindelin","Junio C Hamano","Pratyush Yadav","Bert Wesarg","Philip Oakley","Johannes Sixt"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"339707","messageId":"CAGr--=LZ9TSM9v0SZOi_mj1t8se0Ck-nDHkwum3kC8uz9HKW6A@mail.gmail.com","threadId":"47883","inReplyTo":null,"subject":"git-gui, feature request: add hotkeys to focus different widgets","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2018-02-20T13:32:40Z","receivedAt":"2018-02-20T13:32:45Z","isPatch":false,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"To fully use git-gui with keyboard-only, a few more hotkeys are\nneeded. I am missing hotkeys to change focus between the \"Unstanged\nChanges\", \"Staged Changes\", diff viewer and \"Commit Message\" widgets.\nI would like to be able to stage, browse, unstage and commit in\ngit-gui, all without using the mouse.\n\nI propose that CTRL+(number) could be used as hotkeys to change the\nfocus between the four widgets I've mentioned.\n\n\nBest regards,\nBirger Skogeng Pedersen\n"},{"id":"340009","messageId":"20180223102242.5850-1-birgersp@gmail.com","threadId":"47883","inReplyTo":"CAGr--=LZ9TSM9v0SZOi_mj1t8se0Ck-nDHkwum3kC8uz9HKW6A@mail.gmail.com","subject":"[PATCH] git-gui: Add hotkeys to change focus between ui widgets","fromName":"Birger Skogeng Pedersen","fromEmail":"birgersp@gmail.com","sentAt":"2018-02-23T10:22:42Z","receivedAt":"2018-02-23T10:22:52Z","isPatch":true,"sender":{"key":"birgersp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"The user cannot change focus between the list of files, the diff view\nand the commit message widgets without using the mouse (clicking either of\nthe four widgets ).\n\nHotkeys CTRL/CMD+number (1-4) now focuses the first file of either the\n\"Unstaged Changes\" or \"Staged Changes\", the diff view or the\ncommit message dialog widgets, respectively. This enables the user to\nselect/unselect files, view the diff and create a commit in git-gui\nusing keyboard-only.\n\nSigned-off-by: Birger Skogeng Pedersen \n---\n git-gui/git-gui.sh | 34 ++++++++++++++++++++++++++++++++++\n 1 file changed, 34 insertions(+)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex 91c00e648..bdbe166f7 100755\n\n(First timere here, any feedback is highly appreciated)\n\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -2664,6 +2664,35 @@ proc show_less_context {} {\n \t}\n }\n \n+proc select_first_path {w} {\n+\tglobal file_lists last_clicked selected_paths\n+\tif {[llength $file_lists($w)] > 0} {\n+\t\tfocus $w\n+\t\tset last_clicked [list $w 1]\n+\t\tshow_diff [lindex $file_lists($w) 0] $w\n+\t}\n+}\n+\n+proc select_first_unstaged_changes_path {} {\n+\tglobal ui_workdir\n+\tselect_first_path $ui_workdir\n+}\n+\n+proc select_first_staged_changes_path {} {\n+\tglobal ui_index\n+\tselect_first_path $ui_index\n+}\n+\n+proc focus_diff {} {\n+\tglobal ui_diff\n+\tfocus $ui_diff\n+}\n+\n+proc focus_commit_message {} {\n+\tglobal ui_comm\n+\tfocus $ui_comm\n+}\n+\n ######################################################################\n ##\n ## ui construction\n@@ -3876,6 +3905,11 @@ foreach i [list $ui_index $ui_workdir] {\n }\n unset i\n \n+bind . <$M1B-Key-1> {select_first_unstaged_changes_path}\n+bind . <$M1B-Key-2> {select_first_staged_changes_path}\n+bind . <$M1B-Key-3> {focus_diff}\n+bind . <$M1B-Key-4> {focus_commit_message}\n+\n set file_lists($ui_index) [list]\n set file_lists($ui_workdir) [list]\n \n-- \n2.16.2.266.g75bb9601e\n\n"},{"id":"340041","messageId":"CAGr--=Kn5pRindZ-fu0YQ+s4uKgNz+d1rB1O4O6W+fhd_o2zTg@mail.gmail.com","threadId":"47883","inReplyTo":"20180223102242.5850-1-birgersp@gmail.com","subject":"Re: [PATCH] git-gui: Add hotkeys to change focus between ui widgets","fromName":"Birger Skogeng Pedersen","fromEmail":"birgersp@gmail.com","sentAt":"2018-02-23T16:42:22Z","receivedAt":"2018-02-23T16:42:33Z","isPatch":true,"sender":{"key":"birgersp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"Hi,\n\nI've discovered a bug, I'll be sending a new version soon.\n\nbr\nBirger\n\nOn Fri, Feb 23, 2018 at 11:22 AM, Birger Skogeng Pedersen\n wrote:\n> The user cannot change focus between the list of files, the diff view\n> and the commit message widgets without using the mouse (clicking either of\n> the four widgets ).\n>\n> Hotkeys CTRL/CMD+number (1-4) now focuses the first file of either the\n> \"Unstaged Changes\" or \"Staged Changes\", the diff view or the\n> commit message dialog widgets, respectively. This enables the user to\n> select/unselect files, view the diff and create a commit in git-gui\n> using keyboard-only.\n>\n> Signed-off-by: Birger Skogeng Pedersen \n> ---\n> git-gui/git-gui.sh | 34 ++++++++++++++++++++++++++++++++++\n> 1 file changed, 34 insertions(+)\n>\n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index 91c00e648..bdbe166f7 100755\n>\n> (First timere here, any feedback is highly appreciated)\n>\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -2664,6 +2664,35 @@ proc show_less_context {} {\n> }\n> }\n>\n> +proc select_first_path {w} {\n> + global file_lists last_clicked selected_paths\n> + if {[llength $file_lists($w)] > 0} {\n> + focus $w\n> + set last_clicked [list $w 1]\n> + show_diff [lindex $file_lists($w) 0] $w\n> + }\n> +}\n> +\n> +proc select_first_unstaged_changes_path {} {\n> + global ui_workdir\n> + select_first_path $ui_workdir\n> +}\n> +\n> +proc select_first_staged_changes_path {} {\n> + global ui_index\n> + select_first_path $ui_index\n> +}\n> +\n> +proc focus_diff {} {\n> + global ui_diff\n> + focus $ui_diff\n> +}\n> +\n> +proc focus_commit_message {} {\n> + global ui_comm\n> + focus $ui_comm\n> +}\n> +\n> ######################################################################\n> ##\n> ## ui construction\n> @@ -3876,6 +3905,11 @@ foreach i [list $ui_index $ui_workdir] {\n> }\n> unset i\n>\n> +bind . <$M1B-Key-1> {select_first_unstaged_changes_path}\n> +bind . <$M1B-Key-2> {select_first_staged_changes_path}\n> +bind . <$M1B-Key-3> {focus_diff}\n> +bind . <$M1B-Key-4> {focus_commit_message}\n> +\n> set file_lists($ui_index) [list]\n> set file_lists($ui_workdir) [list]\n>\n> --\n> 2.16.2.266.g75bb9601e\n>\n"},{"id":"340557","messageId":"20180228121052.10642-1-birgersp@gmail.com","threadId":"47883","inReplyTo":"CAGr--=LZ9TSM9v0SZOi_mj1t8se0Ck-nDHkwum3kC8uz9HKW6A@mail.gmail.com","subject":"[PATCH] git-gui: Add hotkeys to change focus between ui widgets","fromName":"Birger Skogeng Pedersen","fromEmail":"birgersp@gmail.com","sentAt":"2018-02-28T12:10:52Z","receivedAt":"2018-02-28T12:11:11Z","isPatch":true,"sender":{"key":"birgersp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"The user cannot change focus between the list of files, the diff view\nand the commit message widgets without using the mouse (clicking either of\nthe four widgets ).\n\nHotkeys CTRL/CMD+number (1-4) now focuses the first file of either the\n\"Unstaged Changes\" or \"Staged Changes\", the diff view or the\ncommit message dialog widgets, respectively. This enables the user to\nselect/unselect files, view the diff and create a commit in git-gui\nusing keyboard-only.\n\nSigned-off-by: Birger Skogeng Pedersen \n---\n git-gui/git-gui.sh | 37 +++++++++++++++++++++++++++++++++++++\n 1 file changed, 37 insertions(+)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex 91c00e648..f96c0a6b8 100755\n\n(This is my first patch ever, any feedback is highly appreciated)\n\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -2664,6 +2664,38 @@ proc show_less_context {} {\n \t}\n }\n \n+proc select_first_path {w} {\n+\tglobal file_lists last_clicked selected_paths\n+\tif {[llength $file_lists($w)] > 0} {\n+\t\tfocus $w\n+\t\tset last_clicked [list $w 1]\n+\t\tset path [lindex $file_lists($w) 0]\n+\t\tarray unset selected_paths\n+\t\tset selected_paths($path) 1\n+\t\tshow_diff $path $w\n+\t}\n+}\n+\n+proc select_first_unstaged_changes_path {} {\n+\tglobal ui_workdir\n+\tselect_first_path $ui_workdir\n+}\n+\n+proc select_first_staged_changes_path {} {\n+\tglobal ui_index\n+\tselect_first_path $ui_index\n+}\n+\n+proc focus_diff {} {\n+\tglobal ui_diff\n+\tfocus $ui_diff\n+}\n+\n+proc focus_commit_message {} {\n+\tglobal ui_comm\n+\tfocus $ui_comm\n+}\n+\n ######################################################################\n ##\n ## ui construction\n@@ -3876,6 +3908,11 @@ foreach i [list $ui_index $ui_workdir] {\n }\n unset i\n \n+bind . <$M1B-Key-1> {select_first_unstaged_changes_path}\n+bind . <$M1B-Key-2> {select_first_staged_changes_path}\n+bind . <$M1B-Key-3> {focus_diff}\n+bind . <$M1B-Key-4> {focus_commit_message}\n+\n set file_lists($ui_index) [list]\n set file_lists($ui_workdir) [list]\n \n-- \n2.16.2.268.g7f9c27f2f\n\n"},{"id":"341027","messageId":"nycvar.QRO.7.76.6.1803051749060.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"47883","inReplyTo":"20180228121052.10642-1-birgersp@gmail.com","subject":"Re: [PATCH] git-gui: Add hotkeys to change focus between ui widgets","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-05T16:55:26Z","receivedAt":"2018-03-05T16:55:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Birger,\n\nOn Wed, 28 Feb 2018, Birger Skogeng Pedersen wrote:\n\n> The user cannot change focus between the list of files, the diff view\n> and the commit message widgets without using the mouse (clicking either of\n> the four widgets ).\n> \n> Hotkeys CTRL/CMD+number (1-4) now focuses the first file of either the\n> \"Unstaged Changes\" or \"Staged Changes\", the diff view or the\n> commit message dialog widgets, respectively. This enables the user to\n> select/unselect files, view the diff and create a commit in git-gui\n> using keyboard-only.\n\nI like this!\n\n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index 91c00e648..f96c0a6b8 100755\n> \n> (This is my first patch ever, any feedback is highly appreciated)\n\nI am not an expert in Tcl/Tk, but I'll do my best to comment on this\npatch.\n\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -2664,6 +2664,38 @@ proc show_less_context {} {\n> \t}\n> }\n> \n> +proc select_first_path {w} {\n> +\tglobal file_lists last_clicked selected_paths\n> +\tif {[llength $file_lists($w)] > 0} {\n> +\t\tfocus $w\n> +\t\tset last_clicked [list $w 1]\n> +\t\tset path [lindex $file_lists($w) 0]\n> +\t\tarray unset selected_paths\n> +\t\tset selected_paths($path) 1\n> +\t\tshow_diff $path $w\n> +\t}\n> +}\n\nDo you think there is a way to focus on the last-selected path? That would\nmake this feature even more convenient, I think.\n\nI am not sure that this information is still there if switching back from\nanother component...\n\n> +proc select_first_unstaged_changes_path {} {\n> +\tglobal ui_workdir\n> +\tselect_first_path $ui_workdir\n> +}\n> +\n> +proc select_first_staged_changes_path {} {\n> +\tglobal ui_index\n> +\tselect_first_path $ui_index\n> +}\n> +\n> +proc focus_diff {} {\n> +\tglobal ui_diff\n> +\tfocus $ui_diff\n> +}\n> +\n> +proc focus_commit_message {} {\n> +\tglobal ui_comm\n> +\tfocus $ui_comm\n> +}\n> +\n> ######################################################################\n> ##\n> ## ui construction\n> @@ -3876,6 +3908,11 @@ foreach i [list $ui_index $ui_workdir] {\n> }\n> unset i\n> \n> +bind . <$M1B-Key-1> {select_first_unstaged_changes_path}\n> +bind . <$M1B-Key-2> {select_first_staged_changes_path}\n> +bind . <$M1B-Key-3> {focus_diff}\n> +bind . <$M1B-Key-4> {focus_commit_message}\n> +\n> set file_lists($ui_index) [list]\n> set file_lists($ui_workdir) [list]\n\nLooks good!\n\nWe are currently without an active Git GUI maintainer, so I hope that\nJunio (the Git maintainer) will pick this up.\n\nCiao,\nJohannes\n"},{"id":"341130","messageId":"CAGr--=+cAbd=8NDu5M8dbeWn=uoevhJjSvN+bgfDPWjA=V0KnQ@mail.gmail.com","threadId":"47883","inReplyTo":"nycvar.QRO.7.76.6.1803051749060.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [PATCH] git-gui: Add hotkeys to change focus between ui widgets","fromName":"Birger Skogeng Pedersen","fromEmail":"birgersp@gmail.com","sentAt":"2018-03-06T14:35:21Z","receivedAt":"2018-03-06T14:35:28Z","isPatch":true,"sender":{"key":"birgersp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"Thanks for the feedback.\n\nOn Mon, Mar 5, 2018 at 5:55 PM, Johannes Schindelin\n wrote:\n> Do you think there is a way to focus on the last-selected path? That would\n> make this feature even more convenient, I think.\n\nYes, good idea. I'll implement it and create a second version.\n\n> I am not sure that this information is still there if switching back from\n> another component...\n\nI don't think so. But I can add a variable to hold the last selected\n(clicked) path for both widgets.\n\nThanks (again),\nBirger\n\nOn Mon, Mar 5, 2018 at 5:55 PM, Johannes Schindelin\n wrote:\n> Hi Birger,\n>\n> On Wed, 28 Feb 2018, Birger Skogeng Pedersen wrote:\n>\n>> The user cannot change focus between the list of files, the diff view\n>> and the commit message widgets without using the mouse (clicking either of\n>> the four widgets ).\n>>\n>> Hotkeys CTRL/CMD+number (1-4) now focuses the first file of either the\n>> \"Unstaged Changes\" or \"Staged Changes\", the diff view or the\n>> commit message dialog widgets, respectively. This enables the user to\n>> select/unselect files, view the diff and create a commit in git-gui\n>> using keyboard-only.\n>\n> I like this!\n>\n>> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n>> index 91c00e648..f96c0a6b8 100755\n>>\n>> (This is my first patch ever, any feedback is highly appreciated)\n>\n> I am not an expert in Tcl/Tk, but I'll do my best to comment on this\n> patch.\n>\n>> --- a/git-gui/git-gui.sh\n>> +++ b/git-gui/git-gui.sh\n>> @@ -2664,6 +2664,38 @@ proc show_less_context {} {\n>> }\n>> }\n>>\n>> +proc select_first_path {w} {\n>> + global file_lists last_clicked selected_paths\n>> + if {[llength $file_lists($w)] > 0} {\n>> + focus $w\n>> + set last_clicked [list $w 1]\n>> + set path [lindex $file_lists($w) 0]\n>> + array unset selected_paths\n>> + set selected_paths($path) 1\n>> + show_diff $path $w\n>> + }\n>> +}\n>\n> Do you think there is a way to focus on the last-selected path? That would\n> make this feature even more convenient, I think.\n>\n> I am not sure that this information is still there if switching back from\n> another component...\n>\n>> +proc select_first_unstaged_changes_path {} {\n>> + global ui_workdir\n>> + select_first_path $ui_workdir\n>> +}\n>> +\n>> +proc select_first_staged_changes_path {} {\n>> + global ui_index\n>> + select_first_path $ui_index\n>> +}\n>> +\n>> +proc focus_diff {} {\n>> + global ui_diff\n>> + focus $ui_diff\n>> +}\n>> +\n>> +proc focus_commit_message {} {\n>> + global ui_comm\n>> + focus $ui_comm\n>> +}\n>> +\n>> ######################################################################\n>> ##\n>> ## ui construction\n>> @@ -3876,6 +3908,11 @@ foreach i [list $ui_index $ui_workdir] {\n>> }\n>> unset i\n>>\n>> +bind . <$M1B-Key-1> {select_first_unstaged_changes_path}\n>> +bind . <$M1B-Key-2> {select_first_staged_changes_path}\n>> +bind . <$M1B-Key-3> {focus_diff}\n>> +bind . <$M1B-Key-4> {focus_commit_message}\n>> +\n>> set file_lists($ui_index) [list]\n>> set file_lists($ui_workdir) [list]\n>\n> Looks good!\n>\n> We are currently without an active Git GUI maintainer, so I hope that\n> Junio (the Git maintainer) will pick this up.\n>\n> Ciao,\n> Johannes\n"},{"id":"341143","messageId":"xmqqbmg13sxq.fsf@gitster-ct.c.googlers.com","threadId":"47883","inReplyTo":"CAGr--=+cAbd=8NDu5M8dbeWn=uoevhJjSvN+bgfDPWjA=V0KnQ@mail.gmail.com","subject":"Re: [PATCH] git-gui: Add hotkeys to change focus between ui widgets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-06T19:28:01Z","receivedAt":"2018-03-06T19:28:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Birger Skogeng Pedersen writes:\n\n> Thanks for the feedback.\n>\n> On Mon, Mar 5, 2018 at 5:55 PM, Johannes Schindelin\n> wrote:\n>> Do you think there is a way to focus on the last-selected path? That would\n>> make this feature even more convenient, I think.\n>\n> Yes, good idea. I'll implement it and create a second version.\n\nPlease make it a patch against the main git-gui project, not against\nour tree. That is, your patch text would look like this:\n\n diff --git a/git-gui.sh b/git-gui.sh\n index 5bc21b878d..39e80ebafa 100755\n --- a/git-gui.sh\n +++ b/git-gui.sh\n @@ -3843,6 +3843,7 @@ bind . <$M1B-Key-equal> {show_more_context;break}\n bind . <$M1B-Key-plus> {show_more_context;break}\n bind . <$M1B-Key-KP_Add> {show_more_context;break}\n bind . <$M1B-Key-Return> do_commit\n +bind . <$M1B-Key-KP_Enter> do_commit\n foreach i [list $ui_index $ui_workdir] {\n bind $i { toggle_or_diff click %W %x %y; break }\n bind $i <$M1B-Button-1> { add_one_to_selection %W %x %y; break }\n\nWe've seen three patches to git-gui from three different people in\nthe past week. The project seems to be abandoned and we need to\nfind a volunteer (or a few) to take it over, it seems. In the\nmeantime I have blindly been picking and queuing git-gui changes\nbut because I am not even a casual user of it, I know I will not do\na good job maintaining it in the longer term.\n"},{"id":"381622","messageId":"20190831122326.9071-1-birger.sp@gmail.com","threadId":"47883","inReplyTo":"xmqqbmg13sxq.fsf@gitster-ct.c.googlers.com","subject":"[PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-08-31T12:23:26Z","receivedAt":"2019-08-31T12:23:32Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"The user cannot change focus between the list of files, the diff view and\nthe commit message widgets without using the mouse (clicking either of\nthe four widgets ).\n\nHotkeys CTRL/CMD+number (1-4) now focuses a previously selected path from\neither the \"Unstaged Changes\" or \"Staged Changes\", the diff view or the\ncommit message dialog widgets, respectively. This enables the user to\nselect/unselect files, view the diff and create a commit in git-gui\nusing keyboard-only.\n\nSigned-off-by: Birger Skogeng Pedersen \n---\n git-gui/git-gui.sh | 57 +++++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 56 insertions(+), 1 deletion(-)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex 6de74ce639..cbd0b69804 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -2494,7 +2494,7 @@ proc force_first_diff {after} {\n \n proc toggle_or_diff {mode w args} {\n \tglobal file_states file_lists current_diff_path ui_index ui_workdir\n-\tglobal last_clicked selected_paths\n+\tglobal last_clicked selected_paths file_lists_last_clicked\n \n \tif {$mode eq \"click\"} {\n \t\tforeach {x y} $args break\n@@ -2551,6 +2551,8 @@ proc toggle_or_diff {mode w args} {\n \t$ui_index tag remove in_sel 0.0 end\n \t$ui_workdir tag remove in_sel 0.0 end\n \n+\tset file_lists_last_clicked($w) $lno\n+\n \t# Determine the state of the file\n \tif {[info exists file_states($path)]} {\n \t\tset state [lindex $file_states($path) 0]\n@@ -2664,6 +2666,51 @@ proc show_less_context {} {\n \t}\n }\n \n+proc select_first_path {w} {\n+\tglobal file_lists last_clicked selected_paths ui_workdir\n+\tglobal file_lists_last_clicked\n+\n+\tset _list_length [llength $file_lists($w)]\n+\n+\tif {$_list_length > 0} {\n+\n+\t\tset _index $file_lists_last_clicked($w)\n+\n+\t\tif {$_index eq {}} {\n+\t\t\tset _index 1\n+\t\t} elseif {$_index > $_list_length} {\n+\t\t\tset _index $_list_length\n+\t\t}\n+\n+\t\tfocus $w\n+\t\tset last_clicked [list $w $_index]\n+\t\tset path [lindex $file_lists($w) [expr $_index - 1]]\n+\t\tarray unset selected_paths\n+\t\tset selected_paths($path) 1\n+\t\tshow_diff $path $w\n+\t}\n+}\n+\n+proc select_first_unstaged_changes_path {} {\n+\tglobal ui_workdir\n+\tselect_first_path $ui_workdir\n+}\n+\n+proc select_first_staged_changes_path {} {\n+\tglobal ui_index\n+\tselect_first_path $ui_index\n+}\n+\n+proc focus_diff {} {\n+\tglobal ui_diff\n+\tfocus $ui_diff\n+}\n+\n+proc focus_commit_message {} {\n+\tglobal ui_comm\n+\tfocus $ui_comm\n+}\n+\n ######################################################################\n ##\n ## ui construction\n@@ -3877,6 +3924,14 @@ foreach i [list $ui_index $ui_workdir] {\n }\n unset i\n \n+bind . <$M1B-Key-1> {select_first_unstaged_changes_path}\n+bind . <$M1B-Key-2> {select_first_staged_changes_path}\n+bind . <$M1B-Key-3> {focus_diff}\n+bind . <$M1B-Key-4> {focus_commit_message}\n+\n+set file_lists_last_clicked($ui_index) {}\n+set file_lists_last_clicked($ui_workdir) {}\n+\n set file_lists($ui_index) [list]\n set file_lists($ui_workdir) [list]\n \n-- \n2.23.0.37.g745f681289\n\n"},{"id":"381623","messageId":"CAGr--=JbyagKMFMjZ2P0er8wvNYte5evMh8xn411z6s-b2z_FQ@mail.gmail.com","threadId":"47883","inReplyTo":"20190831122326.9071-1-birger.sp@gmail.com","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-08-31T12:27:00Z","receivedAt":"2019-08-31T12:27:14Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"(Finally picking this up again)\n\nI've been using this feature for about one year now. If I may say so,\nI think it is actually really great. It's really helpful for me to be\nable to use git-gui with just the keyboard, and I'm hoping others will\nfind it useful, too.\n\nLet me know what you guys think.\n"},{"id":"381630","messageId":"20190901113218.3lfu4ifsxhzrsw4g@yadavpratyush.com","threadId":"47883","inReplyTo":"20190831122326.9071-1-birger.sp@gmail.com","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-01T11:32:18Z","receivedAt":"2019-09-01T11:32:24Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi Birger,\n\nIn case you haven't been following the list, Pat has been inactive \nrecently, so I am acting as the interim maintainer of git-gui for now, \nbecause no one else stepped up and Junio would rather not maintain it.\n\nYou can find my fork over at https://github.com/prati0100/git-gui. I \nmunged your patches to apply on my tree (which is separate from the \ngit.git tree), but it would be great if you base them on my tree next \ntime around.\n\nOn 31/08/19 02:23PM, Birger Skogeng Pedersen wrote:\n> The user cannot change focus between the list of files, the diff view and\n> the commit message widgets without using the mouse (clicking either of\n> the four widgets ).\n\nNit: s/widgets )/widgets)/\n\n> \n> Hotkeys CTRL/CMD+number (1-4) now focuses a previously selected path from\n> either the \"Unstaged Changes\" or \"Staged Changes\", the diff view or the\n> commit message dialog widgets, respectively. This enables the user to\n> select/unselect files, view the diff and create a commit in git-gui\n> using keyboard-only.\n> \n> Signed-off-by: Birger Skogeng Pedersen \n> ---\n> git-gui/git-gui.sh | 57 +++++++++++++++++++++++++++++++++++++++++++++-\n> 1 file changed, 56 insertions(+), 1 deletion(-)\n> \n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index 6de74ce639..cbd0b69804 100755\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -2494,7 +2494,7 @@ proc force_first_diff {after} {\n> \n> proc toggle_or_diff {mode w args} {\n> \tglobal file_states file_lists current_diff_path ui_index ui_workdir\n> -\tglobal last_clicked selected_paths\n> +\tglobal last_clicked selected_paths file_lists_last_clicked\n> \n> \tif {$mode eq \"click\"} {\n> \t\tforeach {x y} $args break\n> @@ -2551,6 +2551,8 @@ proc toggle_or_diff {mode w args} {\n> \t$ui_index tag remove in_sel 0.0 end\n> \t$ui_workdir tag remove in_sel 0.0 end\n> \n> +\tset file_lists_last_clicked($w) $lno\n> +\n> \t# Determine the state of the file\n> \tif {[info exists file_states($path)]} {\n> \t\tset state [lindex $file_states($path) 0]\n> @@ -2664,6 +2666,51 @@ proc show_less_context {} {\n> \t}\n> }\n> \n> +proc select_first_path {w} {\n\nI have recently been going through the git-gui code, and my biggest \ngripe was non-descriptive single letter variable names. So maybe \ns/w/window/\n\n> +\tglobal file_lists last_clicked selected_paths ui_workdir\n> +\tglobal file_lists_last_clicked\n\nAside: this almost made me ask why you declared file_lists_last_clicked \ntwice :D\n\n> +\n> +\tset _list_length [llength $file_lists($w)]\n> +\n> +\tif {$_list_length > 0} {\n> +\n\nNit: Drop the blank line.\n\n> +\t\tset _index $file_lists_last_clicked($w)\n\nIf some files are added/removed via an external command, that means the \nindex we choose won't be the file the user last looked at, correct? What \nabout using path names instead, so we know exactly which file to \ndisplay, even though its index might have changed?\n\nBut if it is not a trivial change, and needs a lot of work, I'm fine \nwith the way things are. If the user changes stuff outside of git-gui, \nsome side effects are to be expected.\n\n> +\n> +\t\tif {$_index eq {}} {\n> +\t\t\tset _index 1\n> +\t\t} elseif {$_index > $_list_length} {\n> +\t\t\tset _index $_list_length\n\nJust to be sure: _index should start at 1 right, and not 0?\n\n> +\t\t}\n> +\n> +\t\tfocus $w\n> +\t\tset last_clicked [list $w $_index]\n> +\t\tset path [lindex $file_lists($w) [expr $_index - 1]]\n> +\t\tarray unset selected_paths\n> +\t\tset selected_paths($path) 1\n> +\t\tshow_diff $path $w\n> +\t}\n\nIf _list_length is 0 (iow, no files are staged/unstaged), this won't \nchange the focus at all. Are you sure this is the desired behaviour? \nWould it make no sense if we switch to an empty pane? The user did \nexplicitly hit the button combo to go there. Yes, they won't be able to \nactually do anything, but what is the harm in switching focus if the \nuser explicitly requests it?\n\n> +}\n> +\n> +proc select_first_unstaged_changes_path {} {\n> +\tglobal ui_workdir\n> +\tselect_first_path $ui_workdir\n> +}\n> +\n> +proc select_first_staged_changes_path {} {\n> +\tglobal ui_index\n> +\tselect_first_path $ui_index\n> +}\n> +\n> +proc focus_diff {} {\n> +\tglobal ui_diff\n> +\tfocus $ui_diff\n> +}\n> +\n> +proc focus_commit_message {} {\n> +\tglobal ui_comm\n> +\tfocus $ui_comm\n> +}\n> +\n\nDo you expect these functions to be re-used somewhere in the near \nfuture? Otherwise...\n\n> ######################################################################\n> ##\n> ## ui construction\n> @@ -3877,6 +3924,14 @@ foreach i [list $ui_index $ui_workdir] {\n> }\n> unset i\n> \n> +bind . <$M1B-Key-1> {select_first_unstaged_changes_path}\n> +bind . <$M1B-Key-2> {select_first_staged_changes_path}\n> +bind . <$M1B-Key-3> {focus_diff}\n> +bind . <$M1B-Key-4> {focus_commit_message}\n\n... why not just put their bodies directly in here? Something like:\n\n bind . <$M1B-Key-1> {\n\tglobal $ui_workdir\n\tselect_first_path $ui_workdir\n }\n\n> +\n> +set file_lists_last_clicked($ui_index) {}\n> +set file_lists_last_clicked($ui_workdir) {}\n> +\n> set file_lists($ui_index) [list]\n> set file_lists($ui_workdir) [list]\n\nOverall, IMO it is a great idea. I tested it, and it works fine on my \nsetup. Thanks.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"381632","messageId":"xmqqmufougvc.fsf@gitster-ct.c.googlers.com","threadId":"47883","inReplyTo":"20190901113218.3lfu4ifsxhzrsw4g@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-01T16:21:43Z","receivedAt":"2019-09-01T16:21:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pratyush Yadav writes:\n\n> ... I am acting as the interim maintainer of git-gui for now, \n> ...\n> You can find my fork over at https://github.com/prati0100/git-gui.\n\nI am aware of three topics (call-do-quit-before-exit, reload-config\nand revert-hunks-lines) of your own, plus your 'master' over there.\nI'd expect that you'd also queue others' topics in the repository as\nyou act as the (interim) maintainer, updating the changes as they\nget reviews, and then eventually when a topic matures enough [*1*],\nyou'd merge it to your 'master' and tell me to pull from there.\n\n\tside note *1*. Decidinging when a topic gets mature enough\n\tis at your discretion as a subsystem maintainer. With your\n\treputation on line, I think everybody on the list would\n\ttrust that you'd make a good judgment ;-).\n\nThanks again for volunteering.\n"},{"id":"381634","messageId":"CAGr--=+x6rdb9wexJ3bo+yZu3_zgNB48Ku-jDbcAnQ2We77cSQ@mail.gmail.com","threadId":"47883","inReplyTo":"20190901113218.3lfu4ifsxhzrsw4g@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-01T18:24:47Z","receivedAt":"2019-09-01T18:25:01Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"Hello Pratyush,\n\n\n(New patch according to our discussion coming up)\n\n\nOn Sun, Sep 1, 2019 at 1:32 PM Pratyush Yadav wrote:\n> In case you haven't been following the list, Pat has been inactive\n> recently, so I am acting as the interim maintainer of git-gui for now,\n> because no one else stepped up and Junio would rather not maintain it.\n\nI saw a discussion about it some time ago. But I didn't catch that you\nare the maintainer. That's great! I use git-gui all the time, so I'm\nhappy to hear someone is going to maintain it.\n\n\n> ... it would be great if you base them on my tree next\n> time around.\n\nOkay, I will.\n\n\n> I have recently been going through the git-gui code, and my biggest\n> gripe was non-descriptive single letter variable names. So maybe\n> s/w/window/\n\nI agree about non-descriptive variables. I was following the style of\nthe rest of git-gui.\n\n\n> If some files are added/removed via an external command, that means the\n> index we choose won't be the file the user last looked at, correct? What\n> about using path names instead, so we know exactly which file to\n> display, even though its index might have changed?\n>\n> But if it is not a trivial change, and needs a lot of work, I'm fine\n> with the way things are. If the user changes stuff outside of git-gui,\n> some side effects are to be expected.\n\nA somewhat non-trivial change, for me at least. To implement what\nyou're suggesting, I'm gonna need some help or the patch will be\ndelayed quite a lot...\n\nSo honestly, I'd appreciate it if we could leave it like this (for\nnow, at least).\n\n\n> > +\n> > + if {$_index eq {}} {\n> > + set _index 1\n> > + } elseif {$_index > $_list_length} {\n> > + set _index $_list_length\n>\n> Just to be sure: _index should start at 1 right, and not 0?\n\nI'm quite sure this is correct. Setting the index to 0 throws an error.\n\n\n> If _list_length is 0 (iow, no files are staged/unstaged), this won't\n> change the focus at all. Are you sure this is the desired behaviour?\n> Would it make no sense if we switch to an empty pane? The user did\n> explicitly hit the button combo to go there. Yes, they won't be able to\n> actually do anything, but what is the harm in switching focus if the\n> user explicitly requests it?\n\nAn error is thrown if we force focus to the \"Unstaged Changes\" widget\nwhen it has no files listed. The same goes for the \"Staged Changes\"\nwidget. That's why I put the condition there.\n\n\n> Do you expect these functions to be re-used somewhere in the near\n> future?\n\nNot really, but I feel the \"key bindings\" section of the script should\nhave as little logic as possible (and just be a bunch of key bindings\ninvoking functions).\nAlso I think function names are a good way to describe what the code\nis doing, so personally I actually think it's better like this.\nBut if you feel strongly that it should be like you suggested, I'm open to it.\n\n\nBest regards,\nBirger\n"},{"id":"381635","messageId":"CAKPyHN0+KQTNe=Wc_6KEpbtd-KS1Y6a9ad=dpq9QFR-F5tvpEA@mail.gmail.com","threadId":"47883","inReplyTo":"20190831122326.9071-1-birger.sp@gmail.com","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-09-01T18:58:15Z","receivedAt":"2019-09-01T18:58:32Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Hi Birger,\n\nOn Sat, Aug 31, 2019 at 2:23 PM Birger Skogeng Pedersen\n wrote:\n>\n> The user cannot change focus between the list of files, the diff view and\n> the commit message widgets without using the mouse (clicking either of\n> the four widgets ).\n\nthat bugged me two, but never come up with a good idea.\n\n>\n> Hotkeys CTRL/CMD+number (1-4) now focuses a previously selected path from\n> either the \"Unstaged Changes\" or \"Staged Changes\", the diff view or the\n> commit message dialog widgets, respectively. This enables the user to\n> select/unselect files, view the diff and create a commit in git-gui\n> using keyboard-only.\n\nBut I don't understand this in full. Does this mean pressing CTRL+1 or\n+2 does also changes the file selection? Why isn't it sufficient to\njust focus the respective file list widget? And than have bindings to\nchange the selection?\n\nBert\n\n>\n> Signed-off-by: Birger Skogeng Pedersen \n> ---\n> git-gui/git-gui.sh | 57 +++++++++++++++++++++++++++++++++++++++++++++-\n> 1 file changed, 56 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index 6de74ce639..cbd0b69804 100755\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -2494,7 +2494,7 @@ proc force_first_diff {after} {\n>\n> proc toggle_or_diff {mode w args} {\n> global file_states file_lists current_diff_path ui_index ui_workdir\n> - global last_clicked selected_paths\n> + global last_clicked selected_paths file_lists_last_clicked\n>\n> if {$mode eq \"click\"} {\n> foreach {x y} $args break\n> @@ -2551,6 +2551,8 @@ proc toggle_or_diff {mode w args} {\n> $ui_index tag remove in_sel 0.0 end\n> $ui_workdir tag remove in_sel 0.0 end\n>\n> + set file_lists_last_clicked($w) $lno\n> +\n> # Determine the state of the file\n> if {[info exists file_states($path)]} {\n> set state [lindex $file_states($path) 0]\n> @@ -2664,6 +2666,51 @@ proc show_less_context {} {\n> }\n> }\n>\n> +proc select_first_path {w} {\n> + global file_lists last_clicked selected_paths ui_workdir\n> + global file_lists_last_clicked\n> +\n> + set _list_length [llength $file_lists($w)]\n> +\n> + if {$_list_length > 0} {\n> +\n> + set _index $file_lists_last_clicked($w)\n> +\n> + if {$_index eq {}} {\n> + set _index 1\n> + } elseif {$_index > $_list_length} {\n> + set _index $_list_length\n> + }\n> +\n> + focus $w\n> + set last_clicked [list $w $_index]\n> + set path [lindex $file_lists($w) [expr $_index - 1]]\n> + array unset selected_paths\n> + set selected_paths($path) 1\n> + show_diff $path $w\n> + }\n> +}\n> +\n> +proc select_first_unstaged_changes_path {} {\n> + global ui_workdir\n> + select_first_path $ui_workdir\n> +}\n> +\n> +proc select_first_staged_changes_path {} {\n> + global ui_index\n> + select_first_path $ui_index\n> +}\n> +\n> +proc focus_diff {} {\n> + global ui_diff\n> + focus $ui_diff\n> +}\n> +\n> +proc focus_commit_message {} {\n> + global ui_comm\n> + focus $ui_comm\n> +}\n> +\n> ######################################################################\n> ##\n> ## ui construction\n> @@ -3877,6 +3924,14 @@ foreach i [list $ui_index $ui_workdir] {\n> }\n> unset i\n>\n> +bind . <$M1B-Key-1> {select_first_unstaged_changes_path}\n> +bind . <$M1B-Key-2> {select_first_staged_changes_path}\n> +bind . <$M1B-Key-3> {focus_diff}\n> +bind . <$M1B-Key-4> {focus_commit_message}\n> +\n> +set file_lists_last_clicked($ui_index) {}\n> +set file_lists_last_clicked($ui_workdir) {}\n> +\n> set file_lists($ui_index) [list]\n> set file_lists($ui_workdir) [list]\n>\n> --\n> 2.23.0.37.g745f681289\n>\n"},{"id":"381636","messageId":"CAKPyHN2_CpaJahKo2Ju2SWvU=Y79k2ArKVkkxPTbeW1t2TYRZQ@mail.gmail.com","threadId":"47883","inReplyTo":"20190901113218.3lfu4ifsxhzrsw4g@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-09-01T19:01:53Z","receivedAt":"2019-09-01T19:02:06Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Sun, Sep 1, 2019 at 1:32 PM Pratyush Yadav wrote:\n> > +\n> > +proc select_first_unstaged_changes_path {} {\n> > + global ui_workdir\n> > + select_first_path $ui_workdir\n> > +}\n> > +\n> > +proc select_first_staged_changes_path {} {\n> > + global ui_index\n> > + select_first_path $ui_index\n> > +}\n> > +\n> > +proc focus_diff {} {\n> > + global ui_diff\n> > + focus $ui_diff\n> > +}\n> > +\n> > +proc focus_commit_message {} {\n> > + global ui_comm\n> > + focus $ui_comm\n> > +}\n> > +\n>\n> Do you expect these functions to be re-used somewhere in the near\n> future? Otherwise...\n>\n> > ######################################################################\n> > ##\n> > ## ui construction\n> > @@ -3877,6 +3924,14 @@ foreach i [list $ui_index $ui_workdir] {\n> > }\n> > unset i\n> >\n> > +bind . <$M1B-Key-1> {select_first_unstaged_changes_path}\n> > +bind . <$M1B-Key-2> {select_first_staged_changes_path}\n> > +bind . <$M1B-Key-3> {focus_diff}\n> > +bind . <$M1B-Key-4> {focus_commit_message}\n>\n> ... why not just put their bodies directly in here? Something like:\n>\n> bind . <$M1B-Key-1> {\n> global $ui_workdir\n> select_first_path $ui_workdir\n> }\n\nthat should also work:\n\nbind . <$M1B-Key-1> {select_first_path $::ui_workdir}\n"},{"id":"381637","messageId":"CAGr--=+H+kFVRDA+aJve1CjvQMg-uVArKOVCrndc=rJcmfvBeA@mail.gmail.com","threadId":"47883","inReplyTo":"CAKPyHN0+KQTNe=Wc_6KEpbtd-KS1Y6a9ad=dpq9QFR-F5tvpEA@mail.gmail.com","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-01T19:25:19Z","receivedAt":"2019-09-01T19:26:00Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"Hello Bert,\n\n\n> But I don't understand this in full. Does this mean pressing CTRL+1 or\n> +2 does also changes the file selection? Why isn't it sufficient to\n> just focus the respective file list widget? And than have bindings to\n> change the selection?\n>\n> Bert\n\nI don't think it's feasible to focus either of those two widgets\nwithout explicitly selecting a file. I tried it at first, but I\ncouldn't get it working. I'm not going to claim it's impossible (it's\nprobably possible), but I gave it up.\nConsider this: when you click either of those two widgets with your\nmouse, you're not really clicking the widget. You're clicking a file\nlisted in that widget. Clicking just the widget (in the blank space\nbeneath the files) does nothing in git-gui. You have to actually click\na file for the focus to change.\n\n(To be precise, pressing CTRL+1/2 selects the same file that was\nalready selected in that widget)\n\nReading your question, I realised my wording in the commit message\nwasn't very good. I'll try to improve it.\n\n\nBest regards,\nBirger\n"},{"id":"381638","messageId":"20190901193652.21086-1-birger.sp@gmail.com","threadId":"47883","inReplyTo":"CAKPyHN2_CpaJahKo2Ju2SWvU=Y79k2ArKVkkxPTbeW1t2TYRZQ@mail.gmail.com","subject":"[PATCH] [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-01T19:36:52Z","receivedAt":"2019-09-01T19:36:59Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"The user cannot change focus between the list of files, the diff view and\nthe commit message widgets without using the mouse (clicking either of\nthe four widgets).\n\nWith this patch, the user may set ui focus to the previously selected path\nin either the \"Unstaged Changes\" or \"Staged Changes\" widgets, using\nCTRL/CMD+1 or CTRL/CMD+2.\n\nThe user may also set the ui focus to the diff view widget with\nCTRL/CMD+3, or to the commit message widget with CTRL/CMD+4.\n\nThis enables the user to select/unselect files, view the diff and create a\ncommit in git-gui using keyboard-only.\n\nSigned-off-by: Birger Skogeng Pedersen \n---\n git-gui.sh | 35 ++++++++++++++++++++++++++++++++++-\n 1 file changed, 34 insertions(+), 1 deletion(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 5bc21b8..ce620f1 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -2495,7 +2495,7 @@ proc force_first_diff {after} {\n \n proc toggle_or_diff {mode w args} {\n \tglobal file_states file_lists current_diff_path ui_index ui_workdir\n-\tglobal last_clicked selected_paths\n+\tglobal last_clicked selected_paths file_lists_last_clicked\n \n \tif {$mode eq \"click\"} {\n \t\tforeach {x y} $args break\n@@ -2527,6 +2527,8 @@ proc toggle_or_diff {mode w args} {\n \t$ui_index tag remove in_sel 0.0 end\n \t$ui_workdir tag remove in_sel 0.0 end\n \n+\tset file_lists_last_clicked($w) $lno\n+\n \t# Determine the state of the file\n \tif {[info exists file_states($path)]} {\n \t\tset state [lindex $file_states($path) 0]\n@@ -2640,6 +2642,29 @@ proc show_less_context {} {\n \t}\n }\n \n+proc select_path_in {widget} {\n+\tglobal file_lists last_clicked selected_paths ui_workdir\n+\tglobal file_lists_last_clicked\n+\n+\tset _list_length [llength $file_lists($widget)]\n+\tif {$_list_length > 0} {\n+\n+\t\tset _index $file_lists_last_clicked($widget)\n+\t\tif {$_index eq {}} {\n+\t\t\tset _index 1\n+\t\t} elseif {$_index > $_list_length} {\n+\t\t\tset _index $_list_length\n+\t\t}\n+\n+\t\tfocus $widget\n+\t\tset last_clicked [list $widget $_index]\n+\t\tset path [lindex $file_lists($widget) [expr $_index - 1]]\n+\t\tarray unset selected_paths\n+\t\tset selected_paths($path) 1\n+\t\tshow_diff $path $widget\n+\t}\n+}\n+\n ######################################################################\n ##\n ## ui construction\n@@ -3852,6 +3877,14 @@ foreach i [list $ui_index $ui_workdir] {\n }\n unset i\n \n+bind . <$M1B-Key-1> {select_path_in $::ui_workdir}\n+bind . <$M1B-Key-2> {select_path_in $::ui_index}\n+bind . <$M1B-Key-3> {focus $::ui_diff}\n+bind . <$M1B-Key-4> {focus $::ui_comm}\n+\n+set file_lists_last_clicked($ui_index) {}\n+set file_lists_last_clicked($ui_workdir) {}\n+\n set file_lists($ui_index) [list]\n set file_lists($ui_workdir) [list]\n \n-- \n2.23.0.37.g745f681289\n\n"},{"id":"381639","messageId":"e2b35f49-5578-c58f-326d-3111333737a0@iee.email","threadId":"47883","inReplyTo":"20190901113218.3lfu4ifsxhzrsw4g@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-09-01T22:27:48Z","receivedAt":"2019-09-01T22:27:50Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Pratyus,\nOn 01/09/2019 12:32, Pratyush Yadav wrote:\n> Hi Birger,\n>\n> In case you haven't been following the list, Pat has been inactive\n> recently, so I am acting as the interim maintainer of git-gui for now,\n> because no one else stepped up and Junio would rather not maintain it.\n>\n> You can find my fork over athttps://github.com/prati0100/git-gui. I\n> munged your patches to apply on my tree (which is separate from the\n> git.git tree), but it would be great if you base them on my tree next\n> time around.\n\nAre there any plans or thoughts about creating a more inclusive man page \nfor the git-gui?\n\nSuch things as the Options dialog linkages [1], and how to drive the \ncommand line options are areas I've wondered about over the years.\n\nNot exactly sure how our plain text man pages and formatted HTML would \nfare for describing the gui layout and where to click. One thing I am \nnoting is that these hotkey nicely have numbers so can easily be used \nfor reference..\n\nPhilip\n\n[1] \nhttps://stackoverflow.com/questions/6007823/is-there-a-help-page-for-the-git-gui-options-dialog\n"},{"id":"381645","messageId":"20190902122527.6cbcizo5dsewrl57@yadavpratyush.com","threadId":"47883","inReplyTo":"e2b35f49-5578-c58f-326d-3111333737a0@iee.email","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-02T12:25:27Z","receivedAt":"2019-09-02T12:25:59Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 01/09/19 11:27PM, Philip Oakley wrote:\n> Hi Pratyus,\n> On 01/09/2019 12:32, Pratyush Yadav wrote:\n> > Hi Birger,\n> > \n> > In case you haven't been following the list, Pat has been inactive\n> > recently, so I am acting as the interim maintainer of git-gui for now,\n> > because no one else stepped up and Junio would rather not maintain it.\n> > \n> > You can find my fork over athttps://github.com/prati0100/git-gui. I\n> > munged your patches to apply on my tree (which is separate from the\n> > git.git tree), but it would be great if you base them on my tree next\n> > time around.\n> \n> Are there any plans or thoughts about creating a more inclusive man page for\n> the git-gui?\n \nHaving better documentation has been one of the things I have in my \nfuture plans, but I can't really say when I can get to it depending on \nmy schedule and time available. I have a couple other topics active \nwhich I'd like to get resolved first.\n\nOf course, if someone else is willing to take the initiative, I'm happy \nto help :)\n\n> Such things as the Options dialog linkages [1], and how to drive the command\n> line options are areas I've wondered about over the years.\n> \n> Not exactly sure how our plain text man pages and formatted HTML would fare\n> for describing the gui layout and where to click. One thing I am noting is\n> that these hotkey nicely have numbers so can easily be used for reference..\n \nFor the options dialog, I think a \"tooltip\" (something like what you get \nwhen you hover over a image in a browser) that describes the option is a \nbetter idea than having a separate man page. I don't expect the option \ndescriptions to be too long or complicated. This approach has the added \nbenefit of not having to maintain a separate man page. Whenever someone \nadds a new options, they have to add its description as well.\n\nI also think the \"tools\" feature needs some documentation, especially \nabout what environment variables we export.\n\nOther than these two, I don't see many places that need too much \ndocumentation. Rest of the UI is pretty self-intuitive, at least to me.\n\n> Philip\n> \n> [1] https://stackoverflow.com/questions/6007823/is-there-a-help-page-for-the-git-gui-options-dialog\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"381660","messageId":"ca5052b8-32ea-5d38-76ba-2389b5f95e45@iee.email","threadId":"47883","inReplyTo":"20190902122527.6cbcizo5dsewrl57@yadavpratyush.com","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-09-02T17:23:07Z","receivedAt":"2019-09-02T17:23:12Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 02/09/2019 13:25, Pratyush Yadav wrote:\n> On 01/09/19 11:27PM, Philip Oakley wrote:\n>> Hi Pratyus,\n>> On 01/09/2019 12:32, Pratyush Yadav wrote:\n>>> Hi Birger,\n>>>\n>>> In case you haven't been following the list, Pat has been inactive\n>>> recently, so I am acting as the interim maintainer of git-gui for now,\n>>> because no one else stepped up and Junio would rather not maintain it.\n>>>\n>>> You can find my fork over athttps://github.com/prati0100/git-gui. I\n>>> munged your patches to apply on my tree (which is separate from the\n>>> git.git tree), but it would be great if you base them on my tree next\n>>> time around.\n>> Are there any plans or thoughts about creating a more inclusive man page for\n>> the git-gui?\n> \n> Having better documentation has been one of the things I have in my\n> future plans, but I can't really say when I can get to it depending on\n> my schedule and time available. I have a couple other topics active\n> which I'd like to get resolved first.\n>\n> Of course, if someone else is willing to take the initiative, I'm happy\n> to help :)\n\nThe main aspect that would help for providing a contribution would be to \nat least decide the (rough) framework/format for a full Gui 'man page'. \nThe existing one \nhttps://github.com/git/git/blob/master/Documentation/git-gui.txt is \nrather short. (would also need the sub-tree integration to be finessed)\n\ne.g.\n1. how much should it be done via 'include' files (like the git-config \nman page now does include::config.txt[] and onwards).\n\n2. Does it use the doc-book man-page format, or something akin to the \nformer tutorial format? (everything appears to have shifted to the man \npage format, so looks like man format is the one.. [1,2,3,4]\n\nI'm thinking that, as it is a big job, it will need the documentation to \nbe split over a number of small include files so that more folk can be \ncontributors.\n\n>> Such things as the Options dialog linkages [1], and how to drive the command\n>> line options are areas I've wondered about over the years.\n>>\n>> Not exactly sure how our plain text man pages and formatted HTML would fare\n>> for describing the gui layout and where to click. One thing I am noting is\n>> that these hotkey nicely have numbers so can easily be used for reference..\n> \n> For the options dialog, I think a \"tooltip\" (something like what you get\n> when you hover over a image in a browser) that describes the option is a\n> better idea than having a separate man page. I don't expect the option\n> descriptions to be too long or complicated. This approach has the added\n> benefit of not having to maintain a separate man page. Whenever someone\n> adds a new options, they have to add its description as well.\nA tool tip that says 'see git help config.. ' could be done. Any \npointers to an existing one for trying a cookie cutter approach getting \nstarted on those ones?\n>\n> I also think the \"tools\" feature needs some documentation, especially\n> about what environment variables we export.\n>\n> Other than these two, I don't see many places that need too much\n> documentation. Rest of the UI is pretty self-intuitive, at least to me.\n>\n>> Philip\n>>\n>> [1] https://stackoverflow.com/questions/6007823/is-there-a-help-page-for-the-git-gui-options-dialog\n[1,2,3,4] \nhttps://github.com/git/git/blob/master/Documentation/giteveryday.txt\nhttps://github.com/git/git/blob/master/Documentation/gittutorial.txt\nhttps://github.com/git/git/blob/master/Documentation/gitcore-tutorial.txt\nhttps://github.com/git/git/blob/master/Documentation/everyday.txto\n\n"},{"id":"381670","messageId":"20190902181954.zcxuyqrdzs3dzjtq@yadavpratyush.com","threadId":"47883","inReplyTo":"20190901193652.21086-1-birger.sp@gmail.com","subject":"Re: [PATCH] [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-02T18:19:54Z","receivedAt":"2019-09-02T18:19:59Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi Birger,\n\nOther than a minor change that I can do locally, this looks good. I'll \ntest it a bit and then merge it. Thanks.\n\nOn 01/09/19 09:36PM, Birger Skogeng Pedersen wrote:\n[snip]\n> +proc select_path_in {widget} {\n> +\tglobal file_lists last_clicked selected_paths ui_workdir\n> +\tglobal file_lists_last_clicked\n> +\n> +\tset _list_length [llength $file_lists($widget)]\n> +\tif {$_list_length > 0} {\n> +\n\nYou missed removing this extra blank line. Will fix it up locally.\n\n> +\t\tset _index $file_lists_last_clicked($widget)\n[snip]\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"381673","messageId":"CAGr--=L64jiE=x_McdfWJuQTz1Qo3wp0nc8xaeKBGe0GgW5Kxg@mail.gmail.com","threadId":"47883","inReplyTo":"20190902181954.zcxuyqrdzs3dzjtq@yadavpratyush.com","subject":"Re: [PATCH] [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-02T18:35:07Z","receivedAt":"2019-09-02T18:35:21Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"On Mon, Sep 2, 2019 at 8:19 PM Pratyush Yadav wrote:\n> You missed removing this extra blank line. Will fix it up locally.\n>\n> > + set _index $file_lists_last_clicked($widget)\n> [snip]\n\n\nTo be honest I felt the blank line there was appropriate, in an\nattempt to follow the same code style as the rest of the git-gui.sh.\nFor instance, the \"add_range_to_selection\" function\n(https://github.com/prati0100/git-gui/blob/master/git-gui.sh#L2601-L2625)\nhas blank lines like this;\n\n\nproc add_range_to_selection {w x y} {\n global file_lists last_clicked selected_paths\n\n if {[lindex $last_clicked 0] ne $w} {\n toggle_or_diff click $w $x $y\n return\n }\n\n set lno [lindex [split [$w index @$x,$y] .] 0]\n set lc [lindex $last_clicked 1]\n if {$lc < $lno} {\n set begin $lc\n set end $lno\n } else {\n set begin $lno\n set end $lc\n }\n\n foreach path [lrange $file_lists($w) \\\n [expr {$begin - 1}] \\\n [expr {$end - 1}]] {\n set selected_paths($path) 1\n }\n $w tag add in_sel $begin.0 [expr {$end + 1}].0\n}\n\n\nNot saying the blank line should defintely be there, I'm just saying\nmy reasoning for keeping it there in the first place :-)\n\n\nBirger\n"},{"id":"381676","messageId":"20190902185321.m76dffzfc76astar@yadavpratyush.com","threadId":"47883","inReplyTo":"CAGr--=L64jiE=x_McdfWJuQTz1Qo3wp0nc8xaeKBGe0GgW5Kxg@mail.gmail.com","subject":"Re: [PATCH] [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-02T18:53:21Z","receivedAt":"2019-09-02T18:53:26Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 02/09/19 08:35PM, Birger Skogeng Pedersen wrote:\n> On Mon, Sep 2, 2019 at 8:19 PM Pratyush Yadav wrote:\n> > You missed removing this extra blank line. Will fix it up locally.\n> >\n> > > + set _index $file_lists_last_clicked($widget)\n> > [snip]\n> \n> \n> To be honest I felt the blank line there was appropriate, in an\n> attempt to follow the same code style as the rest of the git-gui.sh.\n> For instance, the \"add_range_to_selection\" function\n> (https://github.com/prati0100/git-gui/blob/master/git-gui.sh#L2601-L2625)\n> has blank lines like this;\n \nOh yes, these types of blank lines are perfectly fine in my opinion too. \nI actually prefer this alternative over not having any spacing at all. \nThis way you divide the function in logical sections/steps.\n\nBut that was not what I pointed out. The blank line I pointed out was \nlike this:\n\n if {condition} {\n \t\n\tfoo\n\tbar\n }\n\nMy nitpick was with the blank line _just after_ the if.\n\n> \n> proc add_range_to_selection {w x y} {\n> global file_lists last_clicked selected_paths\n> \n\nSo in this example, the above blank line is fine...\n\n> if {[lindex $last_clicked 0] ne $w} {\n\n...but if I add a blank line here like:\n\n if {[lindex $last_clicked 0] ne $w} {\n\n\ttoggle_or_diff click $w $x $y\n\treturn\n }\n\ninstead of like:\n\n if {[lindex $last_clicked 0] ne $w} {\n\ttoggle_or_diff click $w $x $y\n\treturn\n }\n\nIt is the blank line just after the start of your if statement that I \nwas pointing out. I did leave other blank lines untouched in your patch.\n\n[snip]\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"381679","messageId":"CAGr--=L3eL3L1456uQo=poVqXfxO7PxRpgEA8kvp7DY8KGs2Zw@mail.gmail.com","threadId":"47883","inReplyTo":"20190902185321.m76dffzfc76astar@yadavpratyush.com","subject":"Re: [PATCH] [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-02T19:05:19Z","receivedAt":"2019-09-02T19:05:32Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"Ah, sorry I misunderstood you.\n\nThanks,\nBirger\n"},{"id":"381683","messageId":"CAKPyHN0LtOG9mFQ6qxc-w-EdpJ3HGJmHAGdrGp2FKkq4uCf55A@mail.gmail.com","threadId":"47883","inReplyTo":"20190901193652.21086-1-birger.sp@gmail.com","subject":"Re: [PATCH] [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-09-02T19:42:12Z","receivedAt":"2019-09-02T19:42:27Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Sun, Sep 1, 2019 at 9:37 PM Birger Skogeng Pedersen\n wrote:\n>\n> The user cannot change focus between the list of files, the diff view and\n> the commit message widgets without using the mouse (clicking either of\n> the four widgets).\n>\n> With this patch, the user may set ui focus to the previously selected path\n> in either the \"Unstaged Changes\" or \"Staged Changes\" widgets, using\n> CTRL/CMD+1 or CTRL/CMD+2.\n>\n> The user may also set the ui focus to the diff view widget with\n> CTRL/CMD+3, or to the commit message widget with CTRL/CMD+4.\n>\n> This enables the user to select/unselect files, view the diff and create a\n> commit in git-gui using keyboard-only.\n>\n> Signed-off-by: Birger Skogeng Pedersen \n> ---\n> git-gui.sh | 35 ++++++++++++++++++++++++++++++++++-\n> 1 file changed, 34 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-gui.sh b/git-gui.sh\n> index 5bc21b8..ce620f1 100755\n> --- a/git-gui.sh\n> +++ b/git-gui.sh\n> @@ -2495,7 +2495,7 @@ proc force_first_diff {after} {\n>\n> proc toggle_or_diff {mode w args} {\n> global file_states file_lists current_diff_path ui_index ui_workdir\n> - global last_clicked selected_paths\n> + global last_clicked selected_paths file_lists_last_clicked\n>\n> if {$mode eq \"click\"} {\n> foreach {x y} $args break\n> @@ -2527,6 +2527,8 @@ proc toggle_or_diff {mode w args} {\n> $ui_index tag remove in_sel 0.0 end\n> $ui_workdir tag remove in_sel 0.0 end\n>\n> + set file_lists_last_clicked($w) $lno\n\nSo we only remember the lno in the widget, that could mean, that we\nselect the wrong file after a rescan, which shifted the previous path\none down. Can we remember the pathname instead, and try to find this\nagain in the file list?\n\n> +\n> # Determine the state of the file\n> if {[info exists file_states($path)]} {\n> set state [lindex $file_states($path) 0]\n> @@ -2640,6 +2642,29 @@ proc show_less_context {} {\n> }\n> }\n>\n> +proc select_path_in {widget} {\n\ncan we name it 'focus_and_select_path_in', as the main job ob this\nfunction is to focus the widget. It makes also the 'bind' command\nbelow more readily, because than all bind commands start with 'focus'.\n\n> + global file_lists last_clicked selected_paths ui_workdir\n\nui_workdir not referenced in this function\n\n> + global file_lists_last_clicked\n> +\n> + set _list_length [llength $file_lists($widget)]\n> + if {$_list_length > 0} {\n> +\n> + set _index $file_lists_last_clicked($widget)\n\nI have the impression that variables starting with '_' are mainly used\nas read-only global variables, see the list at line 158, and not that\noften as temporal local variables.\n\n> + if {$_index eq {}} {\n> + set _index 1\n> + } elseif {$_index > $_list_length} {\n> + set _index $_list_length\n> + }\n> +\n> + focus $widget\n> + set last_clicked [list $widget $_index]\n> + set path [lindex $file_lists($widget) [expr $_index - 1]]\n> + array unset selected_paths\n> + set selected_paths($path) 1\n> + show_diff $path $widget\n> + }\n> +}\n> +\n> ######################################################################\n> ##\n> ## ui construction\n> @@ -3852,6 +3877,14 @@ foreach i [list $ui_index $ui_workdir] {\n> }\n> unset i\n>\n> +bind . <$M1B-Key-1> {select_path_in $::ui_workdir}\n> +bind . <$M1B-Key-2> {select_path_in $::ui_index}\n> +bind . <$M1B-Key-3> {focus $::ui_diff}\n> +bind . <$M1B-Key-4> {focus $::ui_comm}\n\nI would like to bring up a proposal: AFAICS, more or less all CTRL\nbindings have a menu entry. But it does not make sense to have a menu\nentry for these bindings. And I think we could add more bindings for\nkeyboard-afine users. Thus I would like to propose to use ALT as the\nmodifier for these bindings, which would give us a nice binding\nclassification.\n\nHow about that?\n\nBert\n\n> +\n> +set file_lists_last_clicked($ui_index) {}\n> +set file_lists_last_clicked($ui_workdir) {}\n> +\n> set file_lists($ui_index) [list]\n> set file_lists($ui_workdir) [list]\n>\n> --\n> 2.23.0.37.g745f681289\n>\n"},{"id":"381725","messageId":"CAGr--=L=iUxAOGwwbQwVB5OwmNoXorBXgehMfiC5gvfgUQEa9g@mail.gmail.com","threadId":"47883","inReplyTo":"CAKPyHN0LtOG9mFQ6qxc-w-EdpJ3HGJmHAGdrGp2FKkq4uCf55A@mail.gmail.com","subject":"Re: [PATCH] [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-03T14:21:42Z","receivedAt":"2019-09-03T14:21:57Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"Hi Bert,\n\n\nOn Mon, Sep 2, 2019 at 9:42 PM Bert Wesarg wrote:\n> So we only remember the lno in the widget, that could mean, that we\n> select the wrong file after a rescan, which shifted the previous path\n> one down. Can we remember the pathname instead, and try to find this\n> again in the file list?\n\nSeems to me like a rescan makes git-gui select the first file in the\nlist, regardless of this feature (patch) or not. Ideally, git-gui\nwould not do this. It should select the same file that was selected\nbefore (if it can). But honestly it seems like a separate (perhaps\nbroader) issue. My biggest issue is not having the hotkeys, and that's\nwhat I'm trying to mend :-)\n\n\n> can we name it 'focus_and_select_path_in', as the main job ob this\n> function is to focus the widget. It makes also the 'bind' command\n> below more readily, because than all bind commands start with 'focus'.\n\nAgreed.\n\n\n> > + global file_lists_last_clicked\n> > +\n> > + set _list_length [llength $file_lists($widget)]\n> > + if {$_list_length > 0} {\n> > +\n> > + set _index $file_lists_last_clicked($widget)\n>\n> I have the impression that variables starting with '_' are mainly used\n> as read-only global variables, see the list at line 158, and not that\n> often as temporal local variables.\n\nAgreed, I'll rename it. It's been almost a year since I wrote this\npatch so I can't really remember my reasoning for putting the\nunderscore there.\n\n\n> I would like to bring up a proposal: AFAICS, more or less all CTRL\n> bindings have a menu entry. But it does not make sense to have a menu\n> entry for these bindings. And I think we could add more bindings for\n> keyboard-afine users. Thus I would like to propose to use ALT as the\n> modifier for these bindings, which would give us a nice binding\n> classification.\n>\n> How about that?\n\nMakes sense. And the ALT+1/2/3/4 seems to be unused for anything else.\nAt least on Windows, maybe on other systems those keystrokes are\nalready in use?\n\n\nBirger\n"},{"id":"381726","messageId":"20190903142252.oovvxslvvzteaqlc@yadavpratyush.com","threadId":"47883","inReplyTo":"CAKPyHN0LtOG9mFQ6qxc-w-EdpJ3HGJmHAGdrGp2FKkq4uCf55A@mail.gmail.com","subject":"Re: [PATCH] [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-03T14:22:52Z","receivedAt":"2019-09-03T14:23:02Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 02/09/19 09:42PM, Bert Wesarg wrote:\n> On Sun, Sep 1, 2019 at 9:37 PM Birger Skogeng Pedersen\n> wrote:\n> >\n> > The user cannot change focus between the list of files, the diff view and\n> > the commit message widgets without using the mouse (clicking either of\n> > the four widgets).\n> >\n> > With this patch, the user may set ui focus to the previously selected path\n> > in either the \"Unstaged Changes\" or \"Staged Changes\" widgets, using\n> > CTRL/CMD+1 or CTRL/CMD+2.\n> >\n> > The user may also set the ui focus to the diff view widget with\n> > CTRL/CMD+3, or to the commit message widget with CTRL/CMD+4.\n> >\n> > This enables the user to select/unselect files, view the diff and create a\n> > commit in git-gui using keyboard-only.\n> >\n> > Signed-off-by: Birger Skogeng Pedersen \n> > ---\n> > git-gui.sh | 35 ++++++++++++++++++++++++++++++++++-\n> > 1 file changed, 34 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/git-gui.sh b/git-gui.sh\n> > index 5bc21b8..ce620f1 100755\n> > --- a/git-gui.sh\n> > +++ b/git-gui.sh\n> > @@ -2495,7 +2495,7 @@ proc force_first_diff {after} {\n> >\n> > proc toggle_or_diff {mode w args} {\n> > global file_states file_lists current_diff_path ui_index ui_workdir\n> > - global last_clicked selected_paths\n> > + global last_clicked selected_paths file_lists_last_clicked\n> >\n> > if {$mode eq \"click\"} {\n> > foreach {x y} $args break\n> > @@ -2527,6 +2527,8 @@ proc toggle_or_diff {mode w args} {\n> > $ui_index tag remove in_sel 0.0 end\n> > $ui_workdir tag remove in_sel 0.0 end\n> >\n> > + set file_lists_last_clicked($w) $lno\n> \n> So we only remember the lno in the widget, that could mean, that we\n> select the wrong file after a rescan, which shifted the previous path\n> one down. Can we remember the pathname instead, and try to find this\n> again in the file list?\n \nI raised a similar concern. Birger's response was that it is not a \ntrivial change for him, and he needs help with it. So I decided to keep \nit like it is.\n\nBut now I thought about it a bit more, and I don't think it should be \ntoo difficult. A quick look tells me that $file_lists should be a sorted \nlist with unique entries (git-gui.sh::display_file_helper{}), so it \nshouldn't be too difficult to find a given path. display_file_helper \ndoes:\n\n set lno [lsearch -sorted -exact $file_lists($w) $path]\n\nwhich should also be what we want.\n\nI have a quick-and-dirty fix for it. Haven't tested it too well, but it \nseems to work for some basic things like removing a file and refreshing, \nand then selecting focus again. Do give it a spin. I'll send the patch \nin reply to this email.\n\n> > +\n> > # Determine the state of the file\n> > if {[info exists file_states($path)]} {\n> > set state [lindex $file_states($path) 0]\n> > @@ -2640,6 +2642,29 @@ proc show_less_context {} {\n> > }\n> > }\n> >\n> > +proc select_path_in {widget} {\n> \n> can we name it 'focus_and_select_path_in', as the main job ob this\n> function is to focus the widget. It makes also the 'bind' command\n> below more readily, because than all bind commands start with 'focus'.\n\nAh, I was kind of uneasy with the function name, but couldn't come up \nwith a good one. This sounds all right. Another suggestion that I'd put \nout: \"focus_widget\". As you said, focusing a widget is the main job of \nthis function. Selecting paths is secondary. IMO it should be fine to \nnot have \"select_path_in\" in the function name because you select the \npath in the process of focusing on widget.\n \n> > + global file_lists last_clicked selected_paths ui_workdir\n> \n> ui_workdir not referenced in this function\n\nNice catch.\n\n> > + global file_lists_last_clicked\n> > +\n> > + set _list_length [llength $file_lists($widget)]\n> > + if {$_list_length > 0} {\n> > +\n> > + set _index $file_lists_last_clicked($widget)\n> \n> I have the impression that variables starting with '_' are mainly used\n> as read-only global variables, see the list at line 158, and not that\n> often as temporal local variables.\n> \n> > + if {$_index eq {}} {\n> > + set _index 1\n> > + } elseif {$_index > $_list_length} {\n> > + set _index $_list_length\n> > + }\n> > +\n> > + focus $widget\n> > + set last_clicked [list $widget $_index]\n> > + set path [lindex $file_lists($widget) [expr $_index - 1]]\n> > + array unset selected_paths\n> > + set selected_paths($path) 1\n> > + show_diff $path $widget\n> > + }\n> > +}\n> > +\n> > ######################################################################\n> > ##\n> > ## ui construction\n> > @@ -3852,6 +3877,14 @@ foreach i [list $ui_index $ui_workdir] {\n> > }\n> > unset i\n> >\n> > +bind . <$M1B-Key-1> {select_path_in $::ui_workdir}\n> > +bind . <$M1B-Key-2> {select_path_in $::ui_index}\n> > +bind . <$M1B-Key-3> {focus $::ui_diff}\n> > +bind . <$M1B-Key-4> {focus $::ui_comm}\n> \n> I would like to bring up a proposal: AFAICS, more or less all CTRL\n> bindings have a menu entry. But it does not make sense to have a menu\n> entry for these bindings. And I think we could add more bindings for\n> keyboard-afine users. Thus I would like to propose to use ALT as the\n> modifier for these bindings, which would give us a nice binding\n> classification.\n> \n> How about that?\n \nFWIW, I agree with this. To add to this, it would also somewhat match \nwith bindings in other programs. For example, in Firefox you can choose \ntabs 0-9 with Alt+0-9. In gnome-terminal, you can also choose tabs with \nAlt+0-9. AFAIK Chrome does this too.\n\nMy one concern is, does an Alt key exist on Mac (AFAIK it does, but I \nwant to be sure)? I don't think we have any existing bindings with Alt, \nso will they work well with Macs?\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"381727","messageId":"20190903144536.8935-1-me@yadavpratyush.com","threadId":"47883","inReplyTo":"20190903142252.oovvxslvvzteaqlc@yadavpratyush.com","subject":"[PATCH] git-gui: use path name instead of list index to track last clicked file","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-03T14:45:36Z","receivedAt":"2019-09-03T14:45:45Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Birger,\n\nYou would probably want to squash this patch with yours when you send a\nre-roll. Of course, I'd like some comments and tests on the patch before\nconsidering it \"done\". Just letting you know that I'd like to have this\nchange in your original patch/commit, not as a separate commit. I put it\nin a separate patch for now for easier readability.\n\nAlso, FYI, pass '--scissors' to git-am when applying this to not get the\nabove text in the commit message.\n\n-- >8 --\nFor using the hotkeys CTRL/CMD+1/2/3/4, we save the index of the last\nclicked file. This index may change if some external command changes the\nrepo state. So use the path name of the file instead.\n\nSigned-off-by: Pratyush Yadav \n---\n git-gui.sh | 16 +++++++---------\n 1 file changed, 7 insertions(+), 9 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex ce620f1..9be1b6a 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -2527,7 +2527,7 @@ proc toggle_or_diff {mode w args} {\n \t$ui_index tag remove in_sel 0.0 end\n \t$ui_workdir tag remove in_sel 0.0 end\n\n-\tset file_lists_last_clicked($w) $lno\n+\tset file_lists_last_clicked($w) $path\n\n \t# Determine the state of the file\n \tif {[info exists file_states($path)]} {\n@@ -2648,17 +2648,15 @@ proc select_path_in {widget} {\n\n \tset _list_length [llength $file_lists($widget)]\n \tif {$_list_length > 0} {\n-\n-\t\tset _index $file_lists_last_clicked($widget)\n-\t\tif {$_index eq {}} {\n-\t\t\tset _index 1\n-\t\t} elseif {$_index > $_list_length} {\n-\t\t\tset _index $_list_length\n+\t\tset path $file_lists_last_clicked($widget)\n+\t\tset index [lsearch -sorted -exact $file_lists($widget) $path]\n+\t\tif {$index < 0} {\n+\t\t\tset index 0\n+\t\t\tset path [lindex $file_lists($widget) $index]\n \t\t}\n\n \t\tfocus $widget\n-\t\tset last_clicked [list $widget $_index]\n-\t\tset path [lindex $file_lists($widget) [expr $_index - 1]]\n+\t\tset last_clicked [list $widget [expr $index + 1]]\n \t\tarray unset selected_paths\n \t\tset selected_paths($path) 1\n \t\tshow_diff $path $widget\n--\n2.21.0\n\n"},{"id":"381730","messageId":"CAKPyHN3LFheRGR1Hdh7rzbEsuOdumzphaZB2e1_gPshgsk18Tg@mail.gmail.com","threadId":"47883","inReplyTo":"20190903142252.oovvxslvvzteaqlc@yadavpratyush.com","subject":"Re: [PATCH] [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-09-03T16:06:56Z","receivedAt":"2019-09-03T16:07:11Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Tue, Sep 3, 2019 at 4:22 PM Pratyush Yadav wrote:\n>\n> On 02/09/19 09:42PM, Bert Wesarg wrote:\n> > On Sun, Sep 1, 2019 at 9:37 PM Birger Skogeng Pedersen\n> > wrote:\n> > >\n> > > The user cannot change focus between the list of files, the diff view and\n> > > the commit message widgets without using the mouse (clicking either of\n> > > the four widgets).\n> > >\n> > > With this patch, the user may set ui focus to the previously selected path\n> > > in either the \"Unstaged Changes\" or \"Staged Changes\" widgets, using\n> > > CTRL/CMD+1 or CTRL/CMD+2.\n> > >\n> > > The user may also set the ui focus to the diff view widget with\n> > > CTRL/CMD+3, or to the commit message widget with CTRL/CMD+4.\n> > >\n> > > This enables the user to select/unselect files, view the diff and create a\n> > > commit in git-gui using keyboard-only.\n> > >\n> > > Signed-off-by: Birger Skogeng Pedersen \n> > > ---\n> > > git-gui.sh | 35 ++++++++++++++++++++++++++++++++++-\n> > > 1 file changed, 34 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/git-gui.sh b/git-gui.sh\n> > > index 5bc21b8..ce620f1 100755\n> > > --- a/git-gui.sh\n> > > +++ b/git-gui.sh\n> > > @@ -2495,7 +2495,7 @@ proc force_first_diff {after} {\n> > >\n> > > proc toggle_or_diff {mode w args} {\n> > > global file_states file_lists current_diff_path ui_index ui_workdir\n> > > - global last_clicked selected_paths\n> > > + global last_clicked selected_paths file_lists_last_clicked\n> > >\n> > > if {$mode eq \"click\"} {\n> > > foreach {x y} $args break\n> > > @@ -2527,6 +2527,8 @@ proc toggle_or_diff {mode w args} {\n> > > $ui_index tag remove in_sel 0.0 end\n> > > $ui_workdir tag remove in_sel 0.0 end\n> > >\n> > > + set file_lists_last_clicked($w) $lno\n> >\n> > So we only remember the lno in the widget, that could mean, that we\n> > select the wrong file after a rescan, which shifted the previous path\n> > one down. Can we remember the pathname instead, and try to find this\n> > again in the file list?\n>\n> I raised a similar concern. Birger's response was that it is not a\n> trivial change for him, and he needs help with it. So I decided to keep\n> it like it is.\n>\n> But now I thought about it a bit more, and I don't think it should be\n> too difficult. A quick look tells me that $file_lists should be a sorted\n> list with unique entries (git-gui.sh::display_file_helper{}), so it\n> shouldn't be too difficult to find a given path. display_file_helper\n> does:\n>\n> set lno [lsearch -sorted -exact $file_lists($w) $path]\n>\n> which should also be what we want.\n>\n> I have a quick-and-dirty fix for it. Haven't tested it too well, but it\n> seems to work for some basic things like removing a file and refreshing,\n> and then selecting focus again. Do give it a spin. I'll send the patch\n> in reply to this email.\n>\n> > > +\n> > > # Determine the state of the file\n> > > if {[info exists file_states($path)]} {\n> > > set state [lindex $file_states($path) 0]\n> > > @@ -2640,6 +2642,29 @@ proc show_less_context {} {\n> > > }\n> > > }\n> > >\n> > > +proc select_path_in {widget} {\n> >\n> > can we name it 'focus_and_select_path_in', as the main job ob this\n> > function is to focus the widget. It makes also the 'bind' command\n> > below more readily, because than all bind commands start with 'focus'.\n>\n> Ah, I was kind of uneasy with the function name, but couldn't come up\n> with a good one. This sounds all right. Another suggestion that I'd put\n> out: \"focus_widget\". As you said, focusing a widget is the main job of\n> this function. Selecting paths is secondary. IMO it should be fine to\n> not have \"select_path_in\" in the function name because you select the\n> path in the process of focusing on widget.\n>\n> > > + global file_lists last_clicked selected_paths ui_workdir\n> >\n> > ui_workdir not referenced in this function\n>\n> Nice catch.\n>\n> > > + global file_lists_last_clicked\n> > > +\n> > > + set _list_length [llength $file_lists($widget)]\n> > > + if {$_list_length > 0} {\n> > > +\n> > > + set _index $file_lists_last_clicked($widget)\n> >\n> > I have the impression that variables starting with '_' are mainly used\n> > as read-only global variables, see the list at line 158, and not that\n> > often as temporal local variables.\n> >\n> > > + if {$_index eq {}} {\n> > > + set _index 1\n> > > + } elseif {$_index > $_list_length} {\n> > > + set _index $_list_length\n> > > + }\n> > > +\n> > > + focus $widget\n> > > + set last_clicked [list $widget $_index]\n> > > + set path [lindex $file_lists($widget) [expr $_index - 1]]\n> > > + array unset selected_paths\n> > > + set selected_paths($path) 1\n> > > + show_diff $path $widget\n> > > + }\n> > > +}\n> > > +\n> > > ######################################################################\n> > > ##\n> > > ## ui construction\n> > > @@ -3852,6 +3877,14 @@ foreach i [list $ui_index $ui_workdir] {\n> > > }\n> > > unset i\n> > >\n> > > +bind . <$M1B-Key-1> {select_path_in $::ui_workdir}\n> > > +bind . <$M1B-Key-2> {select_path_in $::ui_index}\n> > > +bind . <$M1B-Key-3> {focus $::ui_diff}\n> > > +bind . <$M1B-Key-4> {focus $::ui_comm}\n> >\n> > I would like to bring up a proposal: AFAICS, more or less all CTRL\n> > bindings have a menu entry. But it does not make sense to have a menu\n> > entry for these bindings. And I think we could add more bindings for\n> > keyboard-afine users. Thus I would like to propose to use ALT as the\n> > modifier for these bindings, which would give us a nice binding\n> > classification.\n> >\n> > How about that?\n>\n> FWIW, I agree with this. To add to this, it would also somewhat match\n> with bindings in other programs. For example, in Firefox you can choose\n> tabs 0-9 with Alt+0-9. In gnome-terminal, you can also choose tabs with\n> Alt+0-9. AFAIK Chrome does this too.\n\nttk::notebook only provides Ctrl+Tab, Ctrl+Shift+Tab, and\nAlt+ keybindings as default tab traversal, thus this should\nnot conflict with Alt+1-4 to begin with.\n\n>\n> My one concern is, does an Alt key exist on Mac (AFAIK it does, but I\n> want to be sure)? I don't think we have any existing bindings with Alt,\n> so will they work well with Macs?\n\nthey should ;-)\n\nhttps://www.macworld.co.uk/how-to/mac/mac-keyboard-shortcuts-3504584/\n\nBert\n\n>\n> --\n> Regards,\n> Pratyush Yadav\n"},{"id":"381744","messageId":"20190903180704.11812-1-birger.sp@gmail.com","threadId":"47883","inReplyTo":"20190903144536.8935-1-me@yadavpratyush.com","subject":"[PATCH v4] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-03T18:07:04Z","receivedAt":"2019-09-03T18:07:39Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"The user cannot change focus between the list of files, the diff view and\nthe commit message widgets without using the mouse (clicking either of\nthe four widgets).\n\nWith this patch, the user may set ui focus to the previously selected path\nin either the \"Unstaged Changes\" or \"Staged Changes\" widgets, using\nALT+1 or ALT+2.\n\nThe user may also set the ui focus to the diff view widget with\nALT+3, or to the commit message widget with ALT+4.\n\nThis enables the user to select/unselect files, view the diff and create a\ncommit in git-gui using keyboard-only.\n\nSigned-off-by: Birger Skogeng Pedersen \n---\n git-gui.sh | 33 ++++++++++++++++++++++++++++++++-\n 1 file changed, 32 insertions(+), 1 deletion(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 5bc21b8..3a2df2f 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -2495,7 +2495,7 @@ proc force_first_diff {after} {\n \n proc toggle_or_diff {mode w args} {\n \tglobal file_states file_lists current_diff_path ui_index ui_workdir\n-\tglobal last_clicked selected_paths\n+\tglobal last_clicked selected_paths file_lists_last_clicked\n \n \tif {$mode eq \"click\"} {\n \t\tforeach {x y} $args break\n@@ -2527,6 +2527,8 @@ proc toggle_or_diff {mode w args} {\n \t$ui_index tag remove in_sel 0.0 end\n \t$ui_workdir tag remove in_sel 0.0 end\n \n+\tset file_lists_last_clicked($w) $path\n+\n \t# Determine the state of the file\n \tif {[info exists file_states($path)]} {\n \t\tset state [lindex $file_states($path) 0]\n@@ -2640,6 +2642,27 @@ proc show_less_context {} {\n \t}\n }\n \n+proc select_path_in {widget} {\n+\tglobal file_lists last_clicked selected_paths\n+\tglobal file_lists_last_clicked\n+\n+\tset _list_length [llength $file_lists($widget)]\n+\tif {$_list_length > 0} {\n+\t\tset path $file_lists_last_clicked($widget)\n+\t\tset index [lsearch -sorted -exact $file_lists($widget) $path]\n+\t\tif {$index < 0} {\n+\t\t\tset index 0\n+\t\t\tset path [lindex $file_lists($widget) $index]\n+\t\t}\n+\n+\t\tfocus $widget\n+\t\tset last_clicked [list $widget [expr $index + 1]]\n+\t\tarray unset selected_paths\n+\t\tset selected_paths($path) 1\n+\t\tshow_diff $path $widget\n+\t}\n+}\n+\n ######################################################################\n ##\n ## ui construction\n@@ -3852,6 +3875,14 @@ foreach i [list $ui_index $ui_workdir] {\n }\n unset i\n \n+bind . {select_path_in $::ui_workdir}\n+bind . {select_path_in $::ui_index}\n+bind . {focus $::ui_diff}\n+bind . {focus $::ui_comm}\n+\n+set file_lists_last_clicked($ui_index) {}\n+set file_lists_last_clicked($ui_workdir) {}\n+\n set file_lists($ui_index) [list]\n set file_lists($ui_workdir) [list]\n \n-- \n2.21.0.windows.1\n\n"},{"id":"381746","messageId":"CAGr--=K4cEjWD4q14M63Muta3X=YSKff4sebH64AKGKWT7+--A@mail.gmail.com","threadId":"47883","inReplyTo":"20190903180704.11812-1-birger.sp@gmail.com","subject":"Re: [PATCH v4] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-03T18:13:02Z","receivedAt":"2019-09-03T18:13:18Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"Hi,\n\n(Sorry that I hadn't used the proper version in the subject before,\nI'm new (as you could probably tell already))\nIn addition to your changes, I removed the unused ui_workdir variable\nand modified the bindings to be ALT+1/2/3/4.\nShoud I have listed you in the commit? Or did I do it according to\nyour suggestion?\n\nThanks,\nBirger\n"},{"id":"381763","messageId":"CAGr--=KOJjQ2YkovVaqkxi_vFqS2AQ+DXzLhPnG6KipYdVp47w@mail.gmail.com","threadId":"47883","inReplyTo":"CAGr--=K4cEjWD4q14M63Muta3X=YSKff4sebH64AKGKWT7+--A@mail.gmail.com","subject":"Re: [PATCH v4] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-03T19:30:27Z","receivedAt":"2019-09-03T19:30:43Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"Hi Pratyush,\n\nI just realised I had forgotten about the local variable prefixed with\nan underscore. So v5 of the patch will be coming up.\nAlso I got quite uncertain, should I have added you in the commit msg\nsomehow? I've seen elsewhere that people add the \"Signed-off-by\" line\nwith the name of the other developer.\n\nBirger\n"},{"id":"381772","messageId":"20190903214942.ubcbdgtphpwnaxs6@yadavpratyush.com","threadId":"47883","inReplyTo":"20190903180704.11812-1-birger.sp@gmail.com","subject":"Re: [PATCH v4] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-03T21:49:42Z","receivedAt":"2019-09-03T21:49:47Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Thanks for the re-roll. Before you send out the v5, a couple more things \nyou missed. Other than those, LGTM.\n\nOn 03/09/19 08:07PM, Birger Skogeng Pedersen wrote:\n[snip]\n> @@ -2640,6 +2642,27 @@ proc show_less_context {} {\n> \t}\n> }\n> \n> +proc select_path_in {widget} {\n\nThere was a suggestion from me and Bert about changing the function \nname. I suggested \"focus_widget\", and Bert suggested \n\"focus_and_select_path_in\". Choose whichever you feel is better.\n\n> +\tglobal file_lists last_clicked selected_paths\n> +\tglobal file_lists_last_clicked\n> +\n> +\tset _list_length [llength $file_lists($widget)]\n> +\tif {$_list_length > 0} {\n\nSince we only use $_list_length in one place after my suggestion, why \nnot just remove it entirely?\n\nThat makes your if statement:\n\nif {[llength $file_lists($widget)] > 0} {\n\t...\n\n[snip]\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"381774","messageId":"20190903221851.gkbbvnrl72szwydx@yadavpratyush.com","threadId":"47883","inReplyTo":"ca5052b8-32ea-5d38-76ba-2389b5f95e45@iee.email","subject":"Re: [PATCH] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-03T22:18:51Z","receivedAt":"2019-09-03T22:18:57Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 02/09/19 06:23PM, Philip Oakley wrote:\n> On 02/09/2019 13:25, Pratyush Yadav wrote:\n> > On 01/09/19 11:27PM, Philip Oakley wrote:\n> > > Hi Pratyus,\n[snip]\n> > > Are there any plans or thoughts about creating a more inclusive \n> > > man page for\n> > > the git-gui?\n> > Having better documentation has been one of the things I have in my\n> > future plans, but I can't really say when I can get to it depending on\n> > my schedule and time available. I have a couple other topics active\n> > which I'd like to get resolved first.\n> > \n> > Of course, if someone else is willing to take the initiative, I'm happy\n> > to help :)\n> \n> The main aspect that would help for providing a contribution would be to at\n> least decide the (rough) framework/format for a full Gui 'man page'. The\n> existing one\n> https://github.com/git/git/blob/master/Documentation/git-gui.txt is rather\n> short. (would also need the sub-tree integration to be finessed)\n> \n> e.g.\n> 1. how much should it be done via 'include' files (like the git-config man\n> page now does include::config.txt[] and onwards).\n> \n> 2. Does it use the doc-book man-page format, or something akin to the former\n> tutorial format? (everything appears to have shifted to the man page format,\n> so looks like man format is the one.. [1,2,3,4]\n> \n> I'm thinking that, as it is a big job, it will need the documentation to be\n> split over a number of small include files so that more folk can be\n> contributors.\n\nWhat exactly do you think we should document? From what I can see, the \nmajor topics are \"options\", \"key bindings\", and \"tools\". Maybe also the \nblame viewer.\n\nIf we do options inside the dialog with tooltips, that leaves key \nbindings. If there is not much else, we might as well do it in the one \nsingle man page we have, and worry about splitting later when it grows \nin size.\n\nIf you intend to have a more comprehensive documentation where we \ndemonstrate the UI stuff, then using a man page will handicap us. In \nthat case a HTML page is a better idea. Although I'm not too sure what \nwarrants documentation is the general UI. It all seems pretty intuitive \nto me, but them I am a \"power user\" so maybe I'm assuming too much.\n\n> > For the options dialog, I think a \"tooltip\" (something like what you \n> > get\n> > when you hover over a image in a browser) that describes the option is a\n> > better idea than having a separate man page. I don't expect the option\n> > descriptions to be too long or complicated. This approach has the added\n> > benefit of not having to maintain a separate man page. Whenever someone\n> > adds a new options, they have to add its description as well.\n> A tool tip that says 'see git help config.. ' could be done. Any pointers to\n> an existing one for trying a cookie cutter approach getting started on those\n> ones?\n\nThe \"Choose a revision\" dialog shows a tooltip. You can get it by \ncreating a tool with the \"Ask the user to select a revision\" option \nselected. Look in lib/choose_rev.tcl.\n\nThe blame viewer also uses tooltips. When you hover over a line, it \nshows the commit message of the last commit that touched it. Look in \nlib/blame.tcl.\n\nThen there is Tk's tooltip package [0]. I haven't used it, so can't \nreally say what the differences are, and which is better.\n\nIf you do end up using the tooltip implemented in choose_rev.tcl and \nblame.tcl, I think it is a good idea to move the common tooltip code in \na \"tooltip framework\".\n\nAlthough I understand that it is a lot of work (especially if you decide \nto refactor the existing tooltip code), and not exactly documentation, \nso it might not be what you want to do.\n\n[0] https://core.tcl-lang.org/tklib/doc/trunk/embedded/www/tklib/files/modules/tooltip/tooltip.html\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"381802","messageId":"20190904143055.11400-1-birger.sp@gmail.com","threadId":"47883","inReplyTo":"20190903214942.ubcbdgtphpwnaxs6@yadavpratyush.com","subject":"[PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-04T14:30:55Z","receivedAt":"2019-09-04T14:31:41Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"The user cannot change focus between the list of files, the diff view and\nthe commit message widgets without using the mouse (clicking either of\nthe four widgets).\n\nWith this patch, the user may set ui focus to the previously selected path\nin either the \"Unstaged Changes\" or \"Staged Changes\" widgets, using\nALT+1 or ALT+2.\n\nThe user may also set the ui focus to the diff view widget with\nALT+3, or to the commit message widget with ALT+4.\n\nThis enables the user to select/unselect files, view the diff and create a\ncommit in git-gui using keyboard-only.\n\nSigned-off-by: Birger Skogeng Pedersen \n---\n git-gui.sh | 32 +++++++++++++++++++++++++++++++-\n 1 file changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 5bc21b8..5dae8da 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -2495,7 +2495,7 @@ proc force_first_diff {after} {\n \n proc toggle_or_diff {mode w args} {\n \tglobal file_states file_lists current_diff_path ui_index ui_workdir\n-\tglobal last_clicked selected_paths\n+\tglobal last_clicked selected_paths file_lists_last_clicked\n \n \tif {$mode eq \"click\"} {\n \t\tforeach {x y} $args break\n@@ -2527,6 +2527,8 @@ proc toggle_or_diff {mode w args} {\n \t$ui_index tag remove in_sel 0.0 end\n \t$ui_workdir tag remove in_sel 0.0 end\n \n+\tset file_lists_last_clicked($w) $path\n+\n \t# Determine the state of the file\n \tif {[info exists file_states($path)]} {\n \t\tset state [lindex $file_states($path) 0]\n@@ -2640,6 +2642,26 @@ proc show_less_context {} {\n \t}\n }\n \n+proc focus_widget {widget} {\n+\tglobal file_lists last_clicked selected_paths\n+\tglobal file_lists_last_clicked\n+\n+\tif {[llength $file_lists($widget)] > 0} {\n+\t\tset path $file_lists_last_clicked($widget)\n+\t\tset index [lsearch -sorted -exact $file_lists($widget) $path]\n+\t\tif {$index < 0} {\n+\t\t\tset index 0\n+\t\t\tset path [lindex $file_lists($widget) $index]\n+\t\t}\n+\n+\t\tfocus $widget\n+\t\tset last_clicked [list $widget [expr $index + 1]]\n+\t\tarray unset selected_paths\n+\t\tset selected_paths($path) 1\n+\t\tshow_diff $path $widget\n+\t}\n+}\n+\n ######################################################################\n ##\n ## ui construction\n@@ -3852,6 +3874,14 @@ foreach i [list $ui_index $ui_workdir] {\n }\n unset i\n \n+bind . {focus_widget $::ui_workdir}\n+bind . {focus_widget $::ui_index}\n+bind . {focus $::ui_diff}\n+bind . {focus $::ui_comm}\n+\n+set file_lists_last_clicked($ui_index) {}\n+set file_lists_last_clicked($ui_workdir) {}\n+\n set file_lists($ui_index) [list]\n set file_lists($ui_workdir) [list]\n \n-- \n2.21.0.windows.1\n\n"},{"id":"381812","messageId":"510e80f4-a8e1-329d-2395-b1268bf7c1f8@kdbg.org","threadId":"47883","inReplyTo":"20190904143055.11400-1-birger.sp@gmail.com","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-09-04T18:59:46Z","receivedAt":"2019-09-04T18:59:50Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 04.09.19 um 16:30 schrieb Birger Skogeng Pedersen:\n> The user cannot change focus between the list of files, the diff view and\n> the commit message widgets without using the mouse (clicking either of\n> the four widgets).\n> \n> With this patch, the user may set ui focus to the previously selected path\n> in either the \"Unstaged Changes\" or \"Staged Changes\" widgets, using\n> ALT+1 or ALT+2.\n> \n> The user may also set the ui focus to the diff view widget with\n> ALT+3, or to the commit message widget with ALT+4.\n\nMany keyboards do not have a right Alt-key. That means that Alt+1 to\nAlt+4 combinations must be typed single-handed with the left hand. This\nis mildly awkward for Alt+4. Can we please have the very important\ncommit widget *not* at Alt+4? I could live with Alt+3.\n\n-- Hannes\n"},{"id":"381814","messageId":"CAGr--=LY4JWdms3GtnKAtN6z-2c-jpXE2HJ5_dMM5gEFayrj_g@mail.gmail.com","threadId":"47883","inReplyTo":"510e80f4-a8e1-329d-2395-b1268bf7c1f8@kdbg.org","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-04T19:20:38Z","receivedAt":"2019-09-04T19:20:52Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"Hi Johannes,\n\nOn Wed, Sep 4, 2019 at 8:59 PM Johannes Sixt wrote:\n> Many keyboards do not have a right Alt-key. That means that Alt+1 to\n> Alt+4 combinations must be typed single-handed with the left hand. This\n> is mildly awkward for Alt+4. Can we please have the very important\n> commit widget *not* at Alt+4? I could live with Alt+3.\n\n(RightAlt wouldn't be used by Europeans, anyways)\nAre you suggesting to keep Alt+1/2/3 for the files/staged/diff\nwidgets, but use something other than Alt+4 for the commit dialog? If\nso, which one would you prefer?\nThe initial propsal from me was to use CTRL/CMD+1/2/3/4. What do you\nthink of using the CTRL/CMD key instead of ALT?\n\nBirger\n"},{"id":"381818","messageId":"CAKPyHN3WNPbJUX-xhMPVg-WigJkMg=NU9HHm98Z6wn8xeHKv7g@mail.gmail.com","threadId":"47883","inReplyTo":"510e80f4-a8e1-329d-2395-b1268bf7c1f8@kdbg.org","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2019-09-04T19:55:56Z","receivedAt":"2019-09-04T19:56:10Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Dear Hannes,\n\nOn Wed, Sep 4, 2019 at 8:59 PM Johannes Sixt wrote:\n>\n> Am 04.09.19 um 16:30 schrieb Birger Skogeng Pedersen:\n> > The user cannot change focus between the list of files, the diff view and\n> > the commit message widgets without using the mouse (clicking either of\n> > the four widgets).\n> >\n> > With this patch, the user may set ui focus to the previously selected path\n> > in either the \"Unstaged Changes\" or \"Staged Changes\" widgets, using\n> > ALT+1 or ALT+2.\n> >\n> > The user may also set the ui focus to the diff view widget with\n> > ALT+3, or to the commit message widget with ALT+4.\n>\n> Many keyboards do not have a right Alt-key. That means that Alt+1 to\n> Alt+4 combinations must be typed single-handed with the left hand. This\n> is mildly awkward for Alt+4. Can we please have the very important\n> commit widget *not* at Alt+4? I could live with Alt+3.\n>\n\nI use my left thumb to press the left Alt key and it does not feel\nmildly awkward. As Alt is also used for the mnemonics, there will\nprobably more of mildly awkward key combinations, wont there?\n\nBert\n\n> -- Hannes\n"},{"id":"381835","messageId":"fcf52af1-dc57-fb58-f52c-7581910a28d6@kdbg.org","threadId":"47883","inReplyTo":"CAGr--=LY4JWdms3GtnKAtN6z-2c-jpXE2HJ5_dMM5gEFayrj_g@mail.gmail.com","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-09-04T21:39:14Z","receivedAt":"2019-09-04T21:39:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 04.09.19 um 21:20 schrieb Birger Skogeng Pedersen:\n> On Wed, Sep 4, 2019 at 8:59 PM Johannes Sixt wrote:\n>> Many keyboards do not have a right Alt-key. That means that Alt+1 to\n>> Alt+4 combinations must be typed single-handed with the left hand. This\n>> is mildly awkward for Alt+4. Can we please have the very important\n>> commit widget *not* at Alt+4? I could live with Alt+3.\n> \n> (RightAlt wouldn't be used by Europeans, anyways)\n> Are you suggesting to keep Alt+1/2/3 for the files/staged/diff\n> widgets, but use something other than Alt+4 for the commit dialog? If\n> so, which one would you prefer?\n\nI was suggesting Alt+3 for the commit message widget, but my preferences\nare actually Alt+1, Alt+2, Alt+3, in this order. My preference for the\ndiff widget would be Alt+4 (the awkward one) because I do not foresee\nthat I would use it a lot. Use what remains for the two file lists.\n\n> The initial propsal from me was to use CTRL/CMD+1/2/3/4. What do you\n> think of using the CTRL/CMD key instead of ALT?\n\nI would not mind Ctrl instead of Alt. Take your pick.\n\n-- Hannes\n"},{"id":"381839","messageId":"05475361-88c2-6698-9a1a-3f237561ee75@kdbg.org","threadId":"47883","inReplyTo":"CAKPyHN3WNPbJUX-xhMPVg-WigJkMg=NU9HHm98Z6wn8xeHKv7g@mail.gmail.com","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-09-04T21:45:38Z","receivedAt":"2019-09-04T21:45:41Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 04.09.19 um 21:55 schrieb Bert Wesarg:\n> I use my left thumb to press the left Alt key and it does not feel\n> mildly awkward. As Alt is also used for the mnemonics, there will\n> probably more of mildly awkward key combinations, wont there?\n\nThat may well be the case. However, it is the commit message widget that\nI would want to access with a hotkey most of the time. It would be nice\nto have one that is the least stressfull for the hand. See my message\nnearby for my preferences.\n\n-- Hannes\n"},{"id":"381841","messageId":"20190904223102.webntwh5awsl2m3i@yadavpratyush.com","threadId":"47883","inReplyTo":"fcf52af1-dc57-fb58-f52c-7581910a28d6@kdbg.org","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-04T22:31:03Z","receivedAt":"2019-09-04T22:31:09Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 04/09/19 11:39PM, Johannes Sixt wrote:\n> Am 04.09.19 um 21:20 schrieb Birger Skogeng Pedersen:\n> > On Wed, Sep 4, 2019 at 8:59 PM Johannes Sixt wrote:\n> >> Many keyboards do not have a right Alt-key. That means that Alt+1 to\n> >> Alt+4 combinations must be typed single-handed with the left hand. This\n> >> is mildly awkward for Alt+4. Can we please have the very important\n> >> commit widget *not* at Alt+4? I could live with Alt+3.\n> > \n> > (RightAlt wouldn't be used by Europeans, anyways)\n> > Are you suggesting to keep Alt+1/2/3 for the files/staged/diff\n> > widgets, but use something other than Alt+4 for the commit dialog? If\n> > so, which one would you prefer?\n> \n> I was suggesting Alt+3 for the commit message widget, but my preferences\n> are actually Alt+1, Alt+2, Alt+3, in this order. My preference for the\n> diff widget would be Alt+4 (the awkward one) because I do not foresee\n> that I would use it a lot. Use what remains for the two file lists.\n\nI wonder if that binding is very intuitive. If we do 1/2 for the top \nand bottom panes on the left side, and 3/4 for the top and bottom panes \non the right side, that makes some sense. Doing it your way makes it a \ncounter-clockwise motion.\n\nI am not arguing for or against this proposal, just pointing something \nworth thinking about. Either way, I suppose after a while it becomes \nmuscle memory so I'm not sure how much difference this subtle thing will \nmake.\n\n> \n> > The initial propsal from me was to use CTRL/CMD+1/2/3/4. What do you\n> > think of using the CTRL/CMD key instead of ALT?\n> \n> I would not mind Ctrl instead of Alt. Take your pick.\n\nFWIW, I vote for sticking with Alt.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"381852","messageId":"xmqqy2z3r5sx.fsf@gitster-ct.c.googlers.com","threadId":"47883","inReplyTo":"20190904223102.webntwh5awsl2m3i@yadavpratyush.com","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-04T23:38:06Z","receivedAt":"2019-09-04T23:38:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pratyush Yadav writes:\n\n> On 04/09/19 11:39PM, Johannes Sixt wrote:\n>> Am 04.09.19 um 21:20 schrieb Birger Skogeng Pedersen:\n>> > On Wed, Sep 4, 2019 at 8:59 PM Johannes Sixt wrote:\n>> >> Many keyboards do not have a right Alt-key. That means that Alt+1 to\n>> >> Alt+4 combinations must be typed single-handed with the left hand. This\n>> >> is mildly awkward for Alt+4. Can we please have the very important\n>> >> commit widget *not* at Alt+4? I could live with Alt+3.\n>> > \n>> > (RightAlt wouldn't be used by Europeans, anyways)\n>> > Are you suggesting to keep Alt+1/2/3 for the files/staged/diff\n>> > widgets, but use something other than Alt+4 for the commit dialog? If\n>> > so, which one would you prefer?\n>> \n>> I was suggesting Alt+3 for the commit message widget, but my preferences\n>> are actually Alt+1, Alt+2, Alt+3, in this order. My preference for the\n>> diff widget would be Alt+4 (the awkward one) because I do not foresee\n>> that I would use it a lot. Use what remains for the two file lists.\n>\n> I wonder if that binding is very intuitive. If we do 1/2 for the top \n> and bottom panes on the left side, and 3/4 for the top and bottom panes \n> on the right side, that makes some sense. Doing it your way makes it a \n> counter-clockwise motion.\n>\n> I am not arguing for or against this proposal, just pointing something \n> worth thinking about. Either way, I suppose after a while it becomes \n> muscle memory so I'm not sure how much difference this subtle thing will \n> make.\n>\n>> \n>> > The initial propsal from me was to use CTRL/CMD+1/2/3/4. What do you\n>> > think of using the CTRL/CMD key instead of ALT?\n>> \n>> I would not mind Ctrl instead of Alt. Take your pick.\n>\n> FWIW, I vote for sticking with Alt.\n\nCan't these differences in personal preference be solved by\nconfigurable key binding?\n"},{"id":"381865","messageId":"20190905123319.c53x34yxlt2abiac@yadavpratyush.com","threadId":"47883","inReplyTo":"xmqqy2z3r5sx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-05T12:33:19Z","receivedAt":"2019-09-05T12:33:25Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 04/09/19 04:38PM, Junio C Hamano wrote:\n> Pratyush Yadav writes:\n> \n> > On 04/09/19 11:39PM, Johannes Sixt wrote:\n> >> Am 04.09.19 um 21:20 schrieb Birger Skogeng Pedersen:\n> >> > On Wed, Sep 4, 2019 at 8:59 PM Johannes Sixt wrote:\n> >> >> Many keyboards do not have a right Alt-key. That means that Alt+1 to\n> >> >> Alt+4 combinations must be typed single-handed with the left hand. This\n> >> >> is mildly awkward for Alt+4. Can we please have the very important\n> >> >> commit widget *not* at Alt+4? I could live with Alt+3.\n> >> > \n> >> > (RightAlt wouldn't be used by Europeans, anyways)\n> >> > Are you suggesting to keep Alt+1/2/3 for the files/staged/diff\n> >> > widgets, but use something other than Alt+4 for the commit dialog? If\n> >> > so, which one would you prefer?\n> >> \n> >> I was suggesting Alt+3 for the commit message widget, but my preferences\n> >> are actually Alt+1, Alt+2, Alt+3, in this order. My preference for the\n> >> diff widget would be Alt+4 (the awkward one) because I do not foresee\n> >> that I would use it a lot. Use what remains for the two file lists.\n> >\n> > I wonder if that binding is very intuitive. If we do 1/2 for the top \n> > and bottom panes on the left side, and 3/4 for the top and bottom panes \n> > on the right side, that makes some sense. Doing it your way makes it a \n> > counter-clockwise motion.\n> >\n> > I am not arguing for or against this proposal, just pointing something \n> > worth thinking about. Either way, I suppose after a while it becomes \n> > muscle memory so I'm not sure how much difference this subtle thing will \n> > make.\n> >\n> >> \n> >> > The initial propsal from me was to use CTRL/CMD+1/2/3/4. What do you\n> >> > think of using the CTRL/CMD key instead of ALT?\n> >> \n> >> I would not mind Ctrl instead of Alt. Take your pick.\n> >\n> > FWIW, I vote for sticking with Alt.\n> \n> Can't these differences in personal preference be solved by\n> configurable key binding?\n\nThey can be. But as of now, there is no existing mechanism to specify \nkeybindings in git-gui that I know of, so that is not a trivial task. \nAnd if we do go with configurable keybindings, that raises the question \nof whether we should allow all existing keybindings to be changed. \nAgain, this is a significant refactor.\n\nAnd in the end we still have to come up with a reasonable default, so \nthis discussion is still worth having IMO. We can keep configurable key \nkeybindings as a future feature. Of course, if the contributors are \nwilling to implement this feature along with this patch it would be \ngreat, but I don't think that is reason enough to block this patch for \ntoo long.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"382145","messageId":"20190910191217.qgb23x5tvaajppfh@yadavpratyush.com","threadId":"47883","inReplyTo":"20190904143055.11400-1-birger.sp@gmail.com","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-10T19:12:18Z","receivedAt":"2019-09-10T19:12:24Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"+Cc j6t\n\nThis patch LGTM, but I'm not sure how to resolve the keybindings \nproblem. Junio suggested we have configurable keybindings, and I agree \nwith him, but until we do, something has to be agreed upon. And we also \nneed to come up with a reasonable default.\n\nSo, I don't have any preferences for either using Alt+3 for the commit \nmessage buffer, or Alt+4. Unless someone has objections, I'll go with \nAlt+3 for the commit message buffer, and Alt+4 for the diff.\n\nThanks for the patch Birger. I'll change the keybindings locally before \npushing out, no need to send a re-roll for something so trivial.\n\nOn 04/09/19 04:30PM, Birger Skogeng Pedersen wrote:\n> The user cannot change focus between the list of files, the diff view and\n> the commit message widgets without using the mouse (clicking either of\n> the four widgets).\n> \n> With this patch, the user may set ui focus to the previously selected path\n> in either the \"Unstaged Changes\" or \"Staged Changes\" widgets, using\n> ALT+1 or ALT+2.\n> \n> The user may also set the ui focus to the diff view widget with\n> ALT+3, or to the commit message widget with ALT+4.\n> \n> This enables the user to select/unselect files, view the diff and create a\n> commit in git-gui using keyboard-only.\n> \n> Signed-off-by: Birger Skogeng Pedersen \n> ---\n> git-gui.sh | 32 +++++++++++++++++++++++++++++++-\n> 1 file changed, 31 insertions(+), 1 deletion(-)\n> \n> diff --git a/git-gui.sh b/git-gui.sh\n> index 5bc21b8..5dae8da 100755\n> --- a/git-gui.sh\n> +++ b/git-gui.sh\n> @@ -2495,7 +2495,7 @@ proc force_first_diff {after} {\n> \n> proc toggle_or_diff {mode w args} {\n> \tglobal file_states file_lists current_diff_path ui_index ui_workdir\n> -\tglobal last_clicked selected_paths\n> +\tglobal last_clicked selected_paths file_lists_last_clicked\n> \n> \tif {$mode eq \"click\"} {\n> \t\tforeach {x y} $args break\n> @@ -2527,6 +2527,8 @@ proc toggle_or_diff {mode w args} {\n> \t$ui_index tag remove in_sel 0.0 end\n> \t$ui_workdir tag remove in_sel 0.0 end\n> \n> +\tset file_lists_last_clicked($w) $path\n> +\n> \t# Determine the state of the file\n> \tif {[info exists file_states($path)]} {\n> \t\tset state [lindex $file_states($path) 0]\n> @@ -2640,6 +2642,26 @@ proc show_less_context {} {\n> \t}\n> }\n> \n> +proc focus_widget {widget} {\n> +\tglobal file_lists last_clicked selected_paths\n> +\tglobal file_lists_last_clicked\n> +\n> +\tif {[llength $file_lists($widget)] > 0} {\n> +\t\tset path $file_lists_last_clicked($widget)\n> +\t\tset index [lsearch -sorted -exact $file_lists($widget) $path]\n> +\t\tif {$index < 0} {\n> +\t\t\tset index 0\n> +\t\t\tset path [lindex $file_lists($widget) $index]\n> +\t\t}\n> +\n> +\t\tfocus $widget\n> +\t\tset last_clicked [list $widget [expr $index + 1]]\n> +\t\tarray unset selected_paths\n> +\t\tset selected_paths($path) 1\n> +\t\tshow_diff $path $widget\n> +\t}\n> +}\n> +\n> ######################################################################\n> ##\n> ## ui construction\n> @@ -3852,6 +3874,14 @@ foreach i [list $ui_index $ui_workdir] {\n> }\n> unset i\n> \n> +bind . {focus_widget $::ui_workdir}\n> +bind . {focus_widget $::ui_index}\n> +bind . {focus $::ui_diff}\n> +bind . {focus $::ui_comm}\n> +\n> +set file_lists_last_clicked($ui_index) {}\n> +set file_lists_last_clicked($ui_workdir) {}\n> +\n> set file_lists($ui_index) [list]\n> set file_lists($ui_workdir) [list]\n> \n> -- \n> 2.21.0.windows.1\n> \n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"382153","messageId":"CAGr--=+9t1m-8LdVnnhkUFS0LMss0WecUjjtR=s_b98qB9qKeA@mail.gmail.com","threadId":"47883","inReplyTo":"20190910191217.qgb23x5tvaajppfh@yadavpratyush.com","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Birger Skogeng Pedersen","fromEmail":"birger.sp@gmail.com","sentAt":"2019-09-11T06:49:08Z","receivedAt":"2019-09-11T06:51:42Z","isPatch":true,"sender":{"key":"birger.sp@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5260237?v=4"},"body":"Hi Pratyush,\n\nOn Tue, Sep 10, 2019 at 9:12 PM Pratyush Yadav wrote:\n> This patch LGTM, but I'm not sure how to resolve the keybindings\n> problem. Junio suggested we have configurable keybindings, and I agree\n> with him, but until we do, something has to be agreed upon. And we also\n> need to come up with a reasonable default.\n>\n> So, I don't have any preferences for either using Alt+3 for the commit\n> message buffer, or Alt+4. Unless someone has objections, I'll go with\n> Alt+3 for the commit message buffer, and Alt+4 for the diff.\n\nI honestly don't quite follow the argumentation to use Alt+3 for the\ncommit message widget. Is Alt+4 (really) too awkward? And if it is,\nhow is Alt+3 better?\nIf you want to see it merged now (which I do, too), I propose we leave\nit at Alt+3 for the diff, and Alt+4 for the commit message buffer.\n\nAs David A. mentioned in his email[1], git-cola utilizes CTRL+J/K/L\nfor navigation, maybe we should consider(?):\nAlt+i: focus unstaged\nAlt+j: focus staged\nAlt+k: focus commit widget\nAlt+l: focus diff view\n\n[1] https://public-inbox.org/git/20190910085446.GB32239@gmail.com/#t\n\nBest regards,\nBirger\n"},{"id":"382169","messageId":"20190911174813.3v2f4s7wxolkkyef@yadavpratyush.com","threadId":"47883","inReplyTo":"CAGr--=+9t1m-8LdVnnhkUFS0LMss0WecUjjtR=s_b98qB9qKeA@mail.gmail.com","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-09-11T17:48:13Z","receivedAt":"2019-09-11T17:48:25Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 11/09/19 08:49AM, Birger Skogeng Pedersen wrote:\n> Hi Pratyush,\n> \n> On Tue, Sep 10, 2019 at 9:12 PM Pratyush Yadav wrote:\n> > This patch LGTM, but I'm not sure how to resolve the keybindings\n> > problem. Junio suggested we have configurable keybindings, and I agree\n> > with him, but until we do, something has to be agreed upon. And we also\n> > need to come up with a reasonable default.\n> >\n> > So, I don't have any preferences for either using Alt+3 for the commit\n> > message buffer, or Alt+4. Unless someone has objections, I'll go with\n> > Alt+3 for the commit message buffer, and Alt+4 for the diff.\n> \n> I honestly don't quite follow the argumentation to use Alt+3 for the\n> commit message widget. Is Alt+4 (really) too awkward? And if it is,\n> how is Alt+3 better?\n\nIt isn't really much more than personal preference.\n\n> If you want to see it merged now (which I do, too), I propose we leave\n> it at Alt+3 for the diff, and Alt+4 for the commit message buffer.\n\nSince this entire debate essentially boils down to personal preference, \nthere is no clear answer. So I'll just go with the author's \nimplementation.\n\nDo note that I fixed a small nitpick locally. Changed the subject from \n\"git-gui: Add...\" to \"git-gui: add...\".\n \n> As David A. mentioned in his email[1], git-cola utilizes CTRL+J/K/L\n> for navigation, maybe we should consider(?):\n> Alt+i: focus unstaged\n> Alt+j: focus staged\n> Alt+k: focus commit widget\n> Alt+l: focus diff view\n\nDavid's suggestion was to try to use the vim keys, which are h,j,k,l, \nand not i,j,k,l. Also, in git-cola (I learned this from reading his \nemail, I don't use git-cola myself), Ctrl+j is for focussing on the \nlower part of the UI, which is their diff viewer. j is used for moving \ndown in vim. Ctrl+k focusses on the status widget, which is in the upper \npart of the UI. k is used for moving up in vim.\n\nBut we can't do something similar with git-gui. Why would Alt+k make \nmore sense when bound to diff instead of the unstaged changes widget? \nBoth are in the upper part of the UI. Similar argument for why Alt+j \nwould make more sense bound to commit message buffer instead of the \nstaged changes widget? Both are in the lower part of the UI. And you \ncan't fit h or l in this similar analogy, because they don't make sense \nto me at all.\n\nI realise that the bindings I mentioned are not what you proposed, but \nthey are more in line with the vim keys.\n\nMy point being, it is not simple to bind our 4 widgets to 4 directional \nkeys, unlike in git-cola where you only have an up and down.\n\nSo I don't see any reasonable bindings what would work with the 4 vim \ndirectional keys: h,j,k,l. Maybe I'm missing something?\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"382172","messageId":"1b330dd0-9daa-b178-4725-2b0257090dc2@kdbg.org","threadId":"47883","inReplyTo":"20190911174813.3v2f4s7wxolkkyef@yadavpratyush.com","subject":"Re: [PATCH v5] git-gui: Add hotkeys to set widget focus","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2019-09-11T18:11:09Z","receivedAt":"2019-09-11T18:11:13Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 11.09.19 um 19:48 schrieb Pratyush Yadav:\n> Since this entire debate essentially boils down to personal preference, \n> there is no clear answer. So I'll just go with the author's \n> implementation.\n\nFair enough.\n\n-- Hannes\n"}]}