| Message ID | pull.1551.git.1687876884.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | gitk: improve keyboard support | expand |
Am 27.06.23 um 16:41 schrieb Jens Lidestrom via GitGitGadget: > It is often convenient to use the keyboard to navigate the gitk GUI and > there are keyboard shortcut bindings for many operations such as searching > and scrolling. There is however no keyboard binding for the most common > operations on branches and commits: Check out, reset, cherry-pick, create > and delete branches. > > This PR adds keyboard bindings for these 5 commands. It also adjusts some > GUI focus defaults to simplify keyboard navigation. > > Some refactoring of the command implementation has been necessary. > Originally the commands was using the mouse context menu to get info about > the head and commit to act on. When using keyboard binds this information > isn't available so instead the row that is selected in the GUI is used. By > adding procedures for doing this the PR lays the groundwork for more similar > keyboard binds in the future. I like it when an application can be navigated with the keyboard. These changes are very much appreciated. I've left some comments on individual commits. The important one is that I think it makes the Reset dialog way too easy to destroy uncommitted work. Please note that gitk-git directory is in its own repository that is only subtree-merged into the Git repository. You should generate patches against git://git.ozlabs.org/~paulus/gitk (I don't know how difficult it would be for Paul to integrate patches that were generated by gitgitgadget). -- Hannes > > I'm including Paul Mackerras because he seems to be the maintainer of gitk. > Can you review, Paul? > > Jens Lidestrom (9): > gitk: add procedures to get commit info from selected row > gitk: use term "current branch" in gui > gitk: add keyboard bind for reset > gitk: show branch name in reset dialog > gitk: add keyboard bind for checkout > gitk: add keyboard bind for create and remove branch > gitk: add keyboard bind to cherry-pick > gitk: focus ok button in reset dialog > gitk: default select reset hard in dialog > > gitk-git/gitk | 132 ++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 96 insertions(+), 36 deletions(-) > > > base-commit: 94486b6763c29144c60932829a65fec0597e17b3 > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1551%2Fjensli%2Fkeyboard-for-gitk-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1551/jensli/keyboard-for-gitk-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1551
Thanks for your comments, Hannes! > Please note that gitk-git directory is in its own repository that is > only subtree-merged into the Git repository. You should generate > patches > against git://git.ozlabs.org/~paulus/gitk (I don't know how difficult > it > would be for Paul to integrate patches that were generated by > gitgitgadget). I'll try to figure out how to do that. I looked briefly at Pauls repo but got the impression that it was out of date. I'll have a second look. /Jens On 2023-06-28 08:09, Johannes Sixt wrote: > Am 27.06.23 um 16:41 schrieb Jens Lidestrom via GitGitGadget: >> It is often convenient to use the keyboard to navigate the gitk GUI >> and >> there are keyboard shortcut bindings for many operations such as >> searching >> and scrolling. There is however no keyboard binding for the most >> common >> operations on branches and commits: Check out, reset, cherry-pick, >> create >> and delete branches. >> >> This PR adds keyboard bindings for these 5 commands. It also adjusts >> some >> GUI focus defaults to simplify keyboard navigation. >> >> Some refactoring of the command implementation has been necessary. >> Originally the commands was using the mouse context menu to get info >> about >> the head and commit to act on. When using keyboard binds this >> information >> isn't available so instead the row that is selected in the GUI is >> used. By >> adding procedures for doing this the PR lays the groundwork for more >> similar >> keyboard binds in the future. > > I like it when an application can be navigated with the keyboard. These > changes are very much appreciated. > > I've left some comments on individual commits. The important one is > that > I think it makes the Reset dialog way too easy to destroy uncommitted > work. > > Please note that gitk-git directory is in its own repository that is > only subtree-merged into the Git repository. You should generate > patches > against git://git.ozlabs.org/~paulus/gitk (I don't know how difficult > it > would be for Paul to integrate patches that were generated by > gitgitgadget). > > -- Hannes > >> >> I'm including Paul Mackerras because he seems to be the maintainer of >> gitk. >> Can you review, Paul? >> >> Jens Lidestrom (9): >> gitk: add procedures to get commit info from selected row >> gitk: use term "current branch" in gui >> gitk: add keyboard bind for reset >> gitk: show branch name in reset dialog >> gitk: add keyboard bind for checkout >> gitk: add keyboard bind for create and remove branch >> gitk: add keyboard bind to cherry-pick >> gitk: focus ok button in reset dialog >> gitk: default select reset hard in dialog >> >> gitk-git/gitk | 132 >> ++++++++++++++++++++++++++++++++++++-------------- >> 1 file changed, 96 insertions(+), 36 deletions(-) >> >> >> base-commit: 94486b6763c29144c60932829a65fec0597e17b3 >> Published-As: >> https://github.com/gitgitgadget/git/releases/tag/pr-1551%2Fjensli%2Fkeyboard-for-gitk-v1 >> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git >> pr-1551/jensli/keyboard-for-gitk-v1 >> Pull-Request: https://github.com/gitgitgadget/git/pull/1551
@Hannes: I choose key combinations with Ctrl+<another-key>. Another possibility is to use Ctrl+Shift+<another-key>. That is more complicated to press, but it creates a nice distinction between two categories of commands: Searching and navigation command (existing): Ctrl+<another-key> Branch modification commands (new): Ctrl+Shift+<another-key> Do you have any opinion on this? Only Ctrl, or Ctrl+Shift for the new commands? /Jens On 2023-06-28 08:09, Johannes Sixt wrote: > Am 27.06.23 um 16:41 schrieb Jens Lidestrom via GitGitGadget: >> It is often convenient to use the keyboard to navigate the gitk GUI and >> there are keyboard shortcut bindings for many operations such as searching >> and scrolling. There is however no keyboard binding for the most common >> operations on branches and commits: Check out, reset, cherry-pick, create >> and delete branches. >> >> This PR adds keyboard bindings for these 5 commands. It also adjusts some >> GUI focus defaults to simplify keyboard navigation. >> >> Some refactoring of the command implementation has been necessary. >> Originally the commands was using the mouse context menu to get info about >> the head and commit to act on. When using keyboard binds this information >> isn't available so instead the row that is selected in the GUI is used. By >> adding procedures for doing this the PR lays the groundwork for more similar >> keyboard binds in the future. > I like it when an application can be navigated with the keyboard. These > changes are very much appreciated. > > I've left some comments on individual commits. The important one is that > I think it makes the Reset dialog way too easy to destroy uncommitted work. > > Please note that gitk-git directory is in its own repository that is > only subtree-merged into the Git repository. You should generate patches > against git://git.ozlabs.org/~paulus/gitk (I don't know how difficult it > would be for Paul to integrate patches that were generated by gitgitgadget). > > -- Hannes > >> I'm including Paul Mackerras because he seems to be the maintainer of gitk. >> Can you review, Paul? >> >> Jens Lidestrom (9): >> gitk: add procedures to get commit info from selected row >> gitk: use term "current branch" in gui >> gitk: add keyboard bind for reset >> gitk: show branch name in reset dialog >> gitk: add keyboard bind for checkout >> gitk: add keyboard bind for create and remove branch >> gitk: add keyboard bind to cherry-pick >> gitk: focus ok button in reset dialog >> gitk: default select reset hard in dialog >> >> gitk-git/gitk | 132 ++++++++++++++++++++++++++++++++++++-------------- >> 1 file changed, 96 insertions(+), 36 deletions(-) >> >> >> base-commit: 94486b6763c29144c60932829a65fec0597e17b3 >> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1551%2Fjensli%2Fkeyboard-for-gitk-v1 >> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1551/jensli/keyboard-for-gitk-v1 >> Pull-Request: https://github.com/gitgitgadget/git/pull/1551
Am 28.06.23 um 19:32 schrieb Jens Lideström: > @Hannes: I choose key combinations with Ctrl+<another-key>. > > Another possibility is to use Ctrl+Shift+<another-key>. That is more > complicated to press, but it creates a nice distinction between two > categories of commands: > > Searching and navigation command (existing): Ctrl+<another-key> > > Branch modification commands (new): Ctrl+Shift+<another-key> > > Do you have any opinion on this? Only Ctrl, or Ctrl+Shift for the new > commands? I have no strong opinion, with a mild preference for the versions without Shift, just because they are more ergonomic. -- Hannes
I have updated the PR after suggestions from Hannes. Mainly these changes have been made: * The reset command dialog uses "mixed" as the default, but is more convenient to navigate with the keyboard. * Remove and checkout branch commands now have branch selection dialogs if there is more than one branch head on the selected commit. * Remove and checkout branch command patches handles a few more cases regarding remote branches and detached heads that I didn't think about originally. This has made them larger. * I have split one commit, added another and moved some functionality around. Because of this the original patch number are no longer in sync with GitHub. How should I handle that? On 2023-06-28 08:09, Johannes Sixt wrote: > Please note that gitk-git directory is in its own repository that is > only subtree-merged into the Git repository. You should generate patches > against git://git.ozlabs.org/~paulus/gitk (I don't know how difficult it > would be for Paul to integrate patches that were generated by gitgitgadget). @Paul Mackerras: Paul, can you have a look at this? Can you accept a PR through GitGitGadget if I rebase it onto master for git.ozlabs.org/~paulus/gitk? Or do you have some other preferred way to receive patches? Best regards, Jens Lideström On 2023-06-28 08:09, Johannes Sixt wrote: > Am 27.06.23 um 16:41 schrieb Jens Lidestrom via GitGitGadget: >> It is often convenient to use the keyboard to navigate the gitk GUI and >> there are keyboard shortcut bindings for many operations such as searching >> and scrolling. There is however no keyboard binding for the most common >> operations on branches and commits: Check out, reset, cherry-pick, create >> and delete branches. >> >> This PR adds keyboard bindings for these 5 commands. It also adjusts some >> GUI focus defaults to simplify keyboard navigation. >> >> Some refactoring of the command implementation has been necessary. >> Originally the commands was using the mouse context menu to get info about >> the head and commit to act on. When using keyboard binds this information >> isn't available so instead the row that is selected in the GUI is used. By >> adding procedures for doing this the PR lays the groundwork for more similar >> keyboard binds in the future. > > I like it when an application can be navigated with the keyboard. These > changes are very much appreciated. > > I've left some comments on individual commits. The important one is that > I think it makes the Reset dialog way too easy to destroy uncommitted work. > > Please note that gitk-git directory is in its own repository that is > only subtree-merged into the Git repository. You should generate patches > against git://git.ozlabs.org/~paulus/gitk (I don't know how difficult it > would be for Paul to integrate patches that were generated by gitgitgadget). > > -- Hannes > >> >> I'm including Paul Mackerras because he seems to be the maintainer of gitk. >> Can you review, Paul? >> >> Jens Lidestrom (9): >> gitk: add procedures to get commit info from selected row >> gitk: use term "current branch" in gui >> gitk: add keyboard bind for reset >> gitk: show branch name in reset dialog >> gitk: add keyboard bind for checkout >> gitk: add keyboard bind for create and remove branch >> gitk: add keyboard bind to cherry-pick >> gitk: focus ok button in reset dialog >> gitk: default select reset hard in dialog >> >> gitk-git/gitk | 132 ++++++++++++++++++++++++++++++++++++-------------- >> 1 file changed, 96 insertions(+), 36 deletions(-) >> >> >> base-commit: 94486b6763c29144c60932829a65fec0597e17b3 >> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1551%2Fjensli%2Fkeyboard-for-gitk-v1 >> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1551/jensli/keyboard-for-gitk-v1 >> Pull-Request: https://github.com/gitgitgadget/git/pull/1551 >