Re: [PATCH 6/9] gitk: add keyboard bind for create and remove branch
- From
- Jens Lideström <jens@lidestrom.se>
- Date
- Jun 28, 2023, 07:12 UTC
- Message-ID
- <36b3ce90d4ecc9eb9fa5174aba0cf9d4@lidestrom.se>
- In-Reply-To
- <7c73cc47-302d-8706-dd7f-fd034ef8d945@kdbg.org>
>> +[mc "<%s-C> Create branch on selected commit" $M1T] > > ... "C"? Which one is it?
"C" is a mistake. Good catch, thanks!
I choose Ctrl-B to avoid a conflict with Ctrl-C for copying text.
Show 5 quoted lines
> The key binding to remove a branch does not make sense to me. It does > happen that I have more than one branch on a commit, but there is no > way > to select which one to remove via the keyboard. I have to use the > context menu. This needs more thought IMHO.
My intention is to always remove the first branch head that is displayed for a single commit in the GUI. This caters to the common use case, with only one branch for a single commit. If there are multiple branch heads on a commit and the users don't want to remove the first one then they need to use the mouse context menu to choose which one to delete.
I could change the implementation to display a dialog that lets the user choose in case of multiple branch heads.
In that case, should I do that as part of this PR, or as a follow up? I would prefer to finish this one first.
> At a minimum, separate it out into its own commit.
I'll do so.
/Jens
On 2023-06-28 07:59, Johannes Sixt wrote:
Show 110 quoted lines
> Am 27.06.23 um 16:41 schrieb Jens Lidestrom via GitGitGadget:
>> From: Jens Lidestrom <jens@lidestrom.se>
>>
>> Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
>> ---
>> gitk-git/gitk | 25 ++++++++++++++++---------
>> 1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/gitk-git/gitk b/gitk-git/gitk
>> index 596977abe89..0d83a72a424 100755
>> --- a/gitk-git/gitk
>> +++ b/gitk-git/gitk
>> @@ -2692,6 +2692,8 @@ proc makewindow {} {
>> bind $ctext <<Selection>> rehighlight_search_results
>> bind . <$M1B-t> {resethead [selected_line_id]}
>> bind . <$M1B-o> {checkout [selected_line_head]
>> [selected_line_id]}
>> + bind . <$M1B-m> {rmbranch [selected_line_head] [selected_line_id]
>> 1}
>> + bind . <$M1B-b> {mkbranch [selected_line_id]}
>
> "b" vs...
>
>> for {set i 1} {$i < 10} {incr i} {
>> bind . <$M1B-Key-$i> [list go_to_parent $i]
>> }
>> @@ -2735,7 +2737,7 @@ proc makewindow {} {
>> makemenu $headctxmenu {
>> {mc "Check out this branch" command {checkout $headmenuhead
>> $headmenuid}}
>> {mc "Rename this branch" command mvbranch}
>> - {mc "Remove this branch" command rmbranch}
>> + {mc "Remove this branch" command {rmbranch $headmenuhead
>> $headmenuid 0}}
>> {mc "Copy branch name" command {clipboard clear; clipboard
>> append $headmenuhead}}
>> }
>> $headctxmenu configure -tearoff 0
>> @@ -3185,6 +3187,8 @@ proc keys {} {
>> [mc "<F5> Update"]
>> [mc "<%s-T> Reset current branch to selected commit" $M1T]
>> [mc "<%s-O> Check out selected commit" $M1T]
>> +[mc "<%s-C> Create branch on selected commit" $M1T]
>
> ... "C"? Which one is it?
>
>> +[mc "<%s-M> Remove selected branch" $M1T]
>> " \
>> -justify left -bg $bgcolor -border 2 -relief groove
>> pack $w.m -side top -fill both -padx 2 -pady 2
>> @@ -9576,13 +9580,13 @@ proc wrcomcan {} {
>> unset wrcomtop
>> }
>>
>> -proc mkbranch {} {
>> - global NS rowmenuid
>> +proc mkbranch {id} {
>> + global NS
>>
>> set top .branchdialog
>>
>> set val(name) ""
>> - set val(id) $rowmenuid
>> + set val(id) $id
>> set val(command) [list mkbrgo $top]
>>
>> set ui(title) [mc "Create branch"]
>> @@ -10054,13 +10058,14 @@ proc readcheckoutstat {fd newhead newheadref
>> newheadid} {
>> }
>> }
>>
>> -proc rmbranch {} {
>> - global headmenuid headmenuhead mainhead
>> +proc rmbranch {head id shouldComfirm} {
>> + global mainhead
>> global idheads
>> -
>> - set head $headmenuhead
>> - set id $headmenuid
>> # this check shouldn't be needed any more...
>> + if {$head eq ""} {
>> + error_popup [mc "Cannot delete a detached head"]
>> + return
>> + }
>> if {$head eq $mainhead} {
>> error_popup [mc "Cannot delete the currently checked-out
>> branch"]
>> return
>> @@ -10070,6 +10075,8 @@ proc rmbranch {} {
>> # the stuff on this branch isn't on any other branch
>> if {![confirm_popup [mc "The commits on branch %s aren't on
>> any other\
>> branch.\nReally delete branch %s?" $head
>> $head]]} return
>> + } elseif {$shouldComfirm} {
>> + if {![confirm_popup [mc "Really delete branch %s?" $head]]}
>> return
>> }
>> nowbusy rmbranch
>> update
>
> The key binding to remove a branch does not make sense to me. It does
> happen that I have more than one branch on a commit, but there is no
> way
> to select which one to remove via the keyboard. I have to use the
> context menu. This needs more thought IMHO. At a minimum, separate it
> out into its own commit.
>
> -- Hannes