* [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton @ 2019-09-05 20:09 Bert Wesarg 2019-09-05 20:09 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Bert Wesarg ` (2 more replies) 0 siblings, 3 replies; 24+ messages in thread From: Bert Wesarg @ 2019-09-05 20:09 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Bert Wesarg Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com> --- git-gui.sh | 36 +++++++++--------------------------- lib/checkout_op.tcl | 6 +++--- lib/commit.tcl | 4 ++-- lib/index.tcl | 8 ++++---- 4 files changed, 18 insertions(+), 36 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 5bc21b8..80a07d5 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -1341,6 +1341,7 @@ set HEAD {} set PARENT {} set MERGE_HEAD [list] set commit_type {} +set commit_type_is_amend 0 set empty_tree {} set current_branch {} set is_detached 0 @@ -1348,7 +1349,6 @@ set current_diff_path {} set is_3way_diff 0 set is_submodule_diff 0 set is_conflict_diff 0 -set selected_commit_type new set diff_empty_count 0 set nullid "0000000000000000000000000000000000000000" @@ -1435,7 +1435,7 @@ proc PARENT {} { } proc force_amend {} { - global selected_commit_type + global commit_type_is_amend global HEAD PARENT MERGE_HEAD commit_type repository_state newType newHEAD newMERGE_HEAD @@ -1444,7 +1444,7 @@ proc force_amend {} { set MERGE_HEAD $newMERGE_HEAD set commit_type $newType - set selected_commit_type amend + set commit_type_is_amend 1 do_select_commit_type } @@ -2828,19 +2828,10 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} { menu .mbar.commit if {![is_enabled nocommit]} { - .mbar.commit add radiobutton \ - -label [mc "New Commit"] \ - -command do_select_commit_type \ - -variable selected_commit_type \ - -value new - lappend disable_on_lock \ - [list .mbar.commit entryconf [.mbar.commit index last] -state] - - .mbar.commit add radiobutton \ + .mbar.commit add checkbutton \ -label [mc "Amend Last Commit"] \ - -command do_select_commit_type \ - -variable selected_commit_type \ - -value amend + -variable commit_type_is_amend \ + -command do_select_commit_type lappend disable_on_lock \ [list .mbar.commit entryconf [.mbar.commit index last] -state] @@ -3313,18 +3304,10 @@ set ui_comm .vpane.lower.commarea.buffer.frame.t set ui_coml .vpane.lower.commarea.buffer.header.l if {![is_enabled nocommit]} { - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \ - -text [mc "New Commit"] \ - -command do_select_commit_type \ - -variable selected_commit_type \ - -value new - lappend disable_on_lock \ - [list .vpane.lower.commarea.buffer.header.new conf -state] - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \ + ${NS}::checkbutton .vpane.lower.commarea.buffer.header.amend \ -text [mc "Amend Last Commit"] \ - -command do_select_commit_type \ - -variable selected_commit_type \ - -value amend + -variable commit_type_is_amend \ + -command do_select_commit_type lappend disable_on_lock \ [list .vpane.lower.commarea.buffer.header.amend conf -state] } @@ -3349,7 +3332,6 @@ pack $ui_coml -side left -fill x if {![is_enabled nocommit]} { pack .vpane.lower.commarea.buffer.header.amend -side right - pack .vpane.lower.commarea.buffer.header.new -side right } textframe .vpane.lower.commarea.buffer.frame diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl index 9e7412c..a522829 100644 --- a/lib/checkout_op.tcl +++ b/lib/checkout_op.tcl @@ -389,7 +389,7 @@ $err } method _after_readtree {} { - global selected_commit_type commit_type HEAD MERGE_HEAD PARENT + global commit_type HEAD MERGE_HEAD PARENT global current_branch is_detached global ui_comm @@ -490,12 +490,12 @@ method _update_repo_state {} { # amend mode our file lists are accurate and we can avoid # the rescan. # - global selected_commit_type commit_type HEAD MERGE_HEAD PARENT + global commit_type_is_amend commit_type HEAD MERGE_HEAD PARENT global ui_comm unlock_index set name [_name $this] - set selected_commit_type new + set commit_type_is_amend 0 if {[string match amend* $commit_type]} { $ui_comm delete 0.0 end $ui_comm edit reset diff --git a/lib/commit.tcl b/lib/commit.tcl index 83620b7..384f18f 100644 --- a/lib/commit.tcl +++ b/lib/commit.tcl @@ -327,7 +327,7 @@ proc commit_writetree {curHEAD msg_p} { proc commit_committree {fd_wt curHEAD msg_p} { global HEAD PARENT MERGE_HEAD commit_type commit_author global current_branch - global ui_comm selected_commit_type + global ui_comm commit_type_is_amend global file_states selected_paths rescan_active global repo_config global env @@ -461,8 +461,8 @@ A rescan will be automatically started now. # -- Update in memory status # - set selected_commit_type new set commit_type normal + set commit_type_is_amend 0 set HEAD $cmt_id set PARENT $cmt_id set MERGE_HEAD [list] diff --git a/lib/index.tcl b/lib/index.tcl index b588db1..e07b7a3 100644 --- a/lib/index.tcl +++ b/lib/index.tcl @@ -466,19 +466,19 @@ proc do_revert_selection {} { } proc do_select_commit_type {} { - global commit_type selected_commit_type + global commit_type commit_type_is_amend - if {$selected_commit_type eq {new} + if {$commit_type_is_amend == 0 && [string match amend* $commit_type]} { create_new_commit - } elseif {$selected_commit_type eq {amend} + } elseif {$commit_type_is_amend == 1 && ![string match amend* $commit_type]} { load_last_commit # The amend request was rejected... # if {![string match amend* $commit_type]} { - set selected_commit_type new + set commit_type_is_amend 0 } } } -- 2.21.0.789.ga095d9d866 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu 2019-09-05 20:09 [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton Bert Wesarg @ 2019-09-05 20:09 ` Bert Wesarg 2019-09-11 20:55 ` Pratyush Yadav 2019-09-05 20:33 ` [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton Pratyush Yadav 2019-09-11 20:15 ` Pratyush Yadav 2 siblings, 1 reply; 24+ messages in thread From: Bert Wesarg @ 2019-09-05 20:09 UTC (permalink / raw) To: git; +Cc: Pratyush Yadav, Birger Skogeng Pedersen, Bert Wesarg From: Birger Skogeng Pedersen <birger.sp@gmail.com> Selecting whether to "Amend Last Commit" or not does not have a hotkey. With this patch, the user may toggle between the two options with CTRL/CMD+e. Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com> Rebased-by: Bert Wesarg <bert.wesarg@googlemail.com> --- git-gui.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/git-gui.sh b/git-gui.sh index 80a07d5..8b776dd 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -2640,6 +2640,12 @@ proc show_less_context {} { } } +proc toggle_commit_type {} { + global commit_type_is_amend + set commit_type_is_amend [expr !$commit_type_is_amend] + do_select_commit_type +} + ###################################################################### ## ## ui construction @@ -2830,6 +2836,7 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} { if {![is_enabled nocommit]} { .mbar.commit add checkbutton \ -label [mc "Amend Last Commit"] \ + -accelerator $M1T-E \ -variable commit_type_is_amend \ -command do_select_commit_type lappend disable_on_lock \ @@ -3825,6 +3832,7 @@ bind . <$M1B-Key-equal> {show_more_context;break} bind . <$M1B-Key-plus> {show_more_context;break} bind . <$M1B-Key-KP_Add> {show_more_context;break} bind . <$M1B-Key-Return> do_commit +bind . <$M1B-Key-e> toggle_commit_type foreach i [list $ui_index $ui_workdir] { bind $i <Button-1> { toggle_or_diff click %W %x %y; break } bind $i <$M1B-Button-1> { add_one_to_selection %W %x %y; break } -- 2.21.0.789.ga095d9d866 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu 2019-09-05 20:09 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Bert Wesarg @ 2019-09-11 20:55 ` Pratyush Yadav 2019-09-12 6:05 ` Birger Skogeng Pedersen 0 siblings, 1 reply; 24+ messages in thread From: Pratyush Yadav @ 2019-09-11 20:55 UTC (permalink / raw) To: Bert Wesarg; +Cc: git, Birger Skogeng Pedersen Birger, Please ignore the two emails I sent about basing your work on top of Bert's. I see that you have already done so (or maybe Bert did it, I don't know), and I was reading an older version of the patch. On 05/09/19 10:09PM, Bert Wesarg wrote: > From: Birger Skogeng Pedersen <birger.sp@gmail.com> > > Selecting whether to "Amend Last Commit" or not does not have a hotkey. > > With this patch, the user may toggle between the two options with > CTRL/CMD+e. > > Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com> > Rebased-by: Bert Wesarg <bert.wesarg@googlemail.com> > --- > git-gui.sh | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/git-gui.sh b/git-gui.sh > index 80a07d5..8b776dd 100755 > --- a/git-gui.sh > +++ b/git-gui.sh > @@ -2640,6 +2640,12 @@ proc show_less_context {} { > } > } > > +proc toggle_commit_type {} { > + global commit_type_is_amend > + set commit_type_is_amend [expr !$commit_type_is_amend] > + do_select_commit_type > +} Ah! So we had almost exactly the same thought process. This is pretty much what I suggested in my other email ;) > + > ###################################################################### > ## > ## ui construction > @@ -2830,6 +2836,7 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} { > if {![is_enabled nocommit]} { > .mbar.commit add checkbutton \ > -label [mc "Amend Last Commit"] \ > + -accelerator $M1T-E \ > -variable commit_type_is_amend \ > -command do_select_commit_type > lappend disable_on_lock \ > @@ -3825,6 +3832,7 @@ bind . <$M1B-Key-equal> {show_more_context;break} > bind . <$M1B-Key-plus> {show_more_context;break} > bind . <$M1B-Key-KP_Add> {show_more_context;break} > bind . <$M1B-Key-Return> do_commit > +bind . <$M1B-Key-e> toggle_commit_type Nitpick: Please move this up with the binds for other letters (u, j, etc) Also, I notice that the bindings for other letters have the same function bound for both small and capital letters (IOW, same behavior with shift held and released). I don't necessarily think that is a great idea. It is a pretty common pattern to have, say Ctrl+a, do something, and Ctrl+Shift+a, do something else. Just want to pick your brain on whether you think we should do the same thing for both Ctrl+e and for Ctrl+E (aka Ctrl+Shift+e), or just bind it to Ctrl+e, and leave Ctrl+E for something else. > foreach i [list $ui_index $ui_workdir] { > bind $i <Button-1> { toggle_or_diff click %W %x %y; break } > bind $i <$M1B-Button-1> { add_one_to_selection %W %x %y; break } Overall, the patch LGTM apart from a couple of nitpicks above. Thanks. -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu 2019-09-11 20:55 ` Pratyush Yadav @ 2019-09-12 6:05 ` Birger Skogeng Pedersen 2019-09-12 16:29 ` Pratyush Yadav 0 siblings, 1 reply; 24+ messages in thread From: Birger Skogeng Pedersen @ 2019-09-12 6:05 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Bert Wesarg, Git List Hi Pratyush, On Wed, Sep 11, 2019 at 10:55 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > Also, I notice that the bindings for other letters have the same > function bound for both small and capital letters (IOW, same behavior > with shift held and released). > > I don't necessarily think that is a great idea. It is a pretty common > pattern to have, say Ctrl+a, do something, and Ctrl+Shift+a, do > something else. Just want to pick your brain on whether you think we > should do the same thing for both Ctrl+e and for Ctrl+E (aka > Ctrl+Shift+e), or just bind it to Ctrl+e, and leave Ctrl+E for something > else. I just tested what happens when you press Ctrl+e while Caps Lock is enabled; the Ctrl+e binding is not invoked. That's probably why other key bindings have the same function bound for both lower- and upper-case letters, to have the same behaviour with/without Caps Lock enabled. With that in mind, we should probably bind Ctrl+E aswell. Should I create and send a new patch? Birger ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu 2019-09-12 6:05 ` Birger Skogeng Pedersen @ 2019-09-12 16:29 ` Pratyush Yadav 2019-09-12 18:41 ` [PATCH v3] git-gui: add hotkey to toggle "Amend Last Commit" Birger Skogeng Pedersen 2019-09-12 21:34 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Marc Branchaud 0 siblings, 2 replies; 24+ messages in thread From: Pratyush Yadav @ 2019-09-12 16:29 UTC (permalink / raw) To: Birger Skogeng Pedersen; +Cc: Bert Wesarg, Git List On 12/09/19 08:05AM, Birger Skogeng Pedersen wrote: > Hi Pratyush, > > On Wed, Sep 11, 2019 at 10:55 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > > Also, I notice that the bindings for other letters have the same > > function bound for both small and capital letters (IOW, same behavior > > with shift held and released). > > > > I don't necessarily think that is a great idea. It is a pretty common > > pattern to have, say Ctrl+a, do something, and Ctrl+Shift+a, do > > something else. Just want to pick your brain on whether you think we > > should do the same thing for both Ctrl+e and for Ctrl+E (aka > > Ctrl+Shift+e), or just bind it to Ctrl+e, and leave Ctrl+E for something > > else. > > I just tested what happens when you press Ctrl+e while Caps Lock is > enabled; the Ctrl+e binding is not invoked. That's probably why other > key bindings have the same function bound for both lower- and > upper-case letters, to have the same behaviour with/without Caps Lock > enabled. With that in mind, we should probably bind Ctrl+E aswell. Nice catch! Makes sense to have the same behaviour for both caps lock enabled and disabled. > > Should I create and send a new patch? Yes, please do. -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3] git-gui: add hotkey to toggle "Amend Last Commit" 2019-09-12 16:29 ` Pratyush Yadav @ 2019-09-12 18:41 ` Birger Skogeng Pedersen 2019-09-13 14:37 ` Pratyush Yadav 2019-09-12 21:34 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Marc Branchaud 1 sibling, 1 reply; 24+ messages in thread From: Birger Skogeng Pedersen @ 2019-09-12 18:41 UTC (permalink / raw) To: me; +Cc: git, Birger Skogeng Pedersen, Bert Wesarg Selecting whether to do a "New Commit" or "Amend Last Commit" does not have a hotkey. With this patch, the user may toggle between the two options with CTRL/CMD+e. Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com> --- git-gui.sh | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 5bc21b8..ebe267f 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -1445,7 +1445,7 @@ proc force_amend {} { set commit_type $newType set selected_commit_type amend - do_select_commit_type + ui_select_commit_type } proc rescan {after {honor_trustmtime 1}} { @@ -2640,6 +2640,16 @@ proc show_less_context {} { } } +proc toggle_commit_type {} { + global selected_commit_type + if {[string match amend* $selected_commit_type]} { + set selected_commit_type new + } else { + set selected_commit_type amend + } + ui_select_commit_type +} + ###################################################################### ## ## ui construction @@ -2824,13 +2834,31 @@ proc commit_btn_caption {} { } } +proc ui_select_commit_type {} { + global selected_commit_type + global ui_commit_type_commit ui_commit_type_amend + + do_select_commit_type + if {$selected_commit_type eq {new}} { + .mbar.commit entryconf [mc "New Commit"] \ + -accelerator {} + .mbar.commit entryconf [mc "Amend Last Commit"] \ + -accelerator $::M1T-E + } elseif {$selected_commit_type eq {amend}} { + .mbar.commit entryconf [mc "New Commit"] \ + -accelerator $::M1T-E + .mbar.commit entryconf [mc "Amend Last Commit"] \ + -accelerator {} + } +} + if {[is_enabled multicommit] || [is_enabled singlecommit]} { menu .mbar.commit if {![is_enabled nocommit]} { .mbar.commit add radiobutton \ -label [mc "New Commit"] \ - -command do_select_commit_type \ + -command ui_select_commit_type \ -variable selected_commit_type \ -value new lappend disable_on_lock \ @@ -2838,7 +2866,8 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} { .mbar.commit add radiobutton \ -label [mc "Amend Last Commit"] \ - -command do_select_commit_type \ + -accelerator $M1T-E \ + -command ui_select_commit_type \ -variable selected_commit_type \ -value amend lappend disable_on_lock \ @@ -3315,14 +3344,14 @@ set ui_coml .vpane.lower.commarea.buffer.header.l if {![is_enabled nocommit]} { ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \ -text [mc "New Commit"] \ - -command do_select_commit_type \ + -command ui_select_commit_type \ -variable selected_commit_type \ -value new lappend disable_on_lock \ [list .vpane.lower.commarea.buffer.header.new conf -state] ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \ -text [mc "Amend Last Commit"] \ - -command do_select_commit_type \ + -command ui_select_commit_type \ -variable selected_commit_type \ -value amend lappend disable_on_lock \ @@ -3837,6 +3866,8 @@ bind . <$M1B-Key-j> do_revert_selection bind . <$M1B-Key-J> do_revert_selection bind . <$M1B-Key-i> do_add_all bind . <$M1B-Key-I> do_add_all +bind . <$M1B-Key-e> toggle_commit_type +bind . <$M1B-Key-E> toggle_commit_type bind . <$M1B-Key-minus> {show_less_context;break} bind . <$M1B-Key-KP_Subtract> {show_less_context;break} bind . <$M1B-Key-equal> {show_more_context;break} -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3] git-gui: add hotkey to toggle "Amend Last Commit" 2019-09-12 18:41 ` [PATCH v3] git-gui: add hotkey to toggle "Amend Last Commit" Birger Skogeng Pedersen @ 2019-09-13 14:37 ` Pratyush Yadav 2019-09-13 21:11 ` [PATCH v4] " Birger Skogeng Pedersen 0 siblings, 1 reply; 24+ messages in thread From: Pratyush Yadav @ 2019-09-13 14:37 UTC (permalink / raw) To: Birger Skogeng Pedersen; +Cc: git, Bert Wesarg Hi Birger, I'm afraid you are working on an older version of this patch. You should be re-rolling [0], which works well with Bert's "amend check button" change. [0] https://public-inbox.org/git/b82a00441ff1a6a9cea3fd235c1c33729ec31b71.1567713659.git.bert.wesarg@googlemail.com/ On 12/09/19 08:41PM, Birger Skogeng Pedersen wrote: > Selecting whether to do a "New Commit" or "Amend Last Commit" does not have > a hotkey. > > With this patch, the user may toggle between the two options with > CTRL/CMD+e. > > Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com> > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com> > --- > git-gui.sh | 41 ++++++++++++++++++++++++++++++++++++----- > 1 file changed, 36 insertions(+), 5 deletions(-) > > diff --git a/git-gui.sh b/git-gui.sh > index 5bc21b8..ebe267f 100755 > --- a/git-gui.sh > +++ b/git-gui.sh > @@ -1445,7 +1445,7 @@ proc force_amend {} { > set commit_type $newType > > set selected_commit_type amend > - do_select_commit_type > + ui_select_commit_type > } > > proc rescan {after {honor_trustmtime 1}} { > @@ -2640,6 +2640,16 @@ proc show_less_context {} { > } > } > > +proc toggle_commit_type {} { > + global selected_commit_type > + if {[string match amend* $selected_commit_type]} { > + set selected_commit_type new > + } else { > + set selected_commit_type amend > + } > + ui_select_commit_type > +} > + > ###################################################################### > ## > ## ui construction > @@ -2824,13 +2834,31 @@ proc commit_btn_caption {} { > } > } > > +proc ui_select_commit_type {} { > + global selected_commit_type > + global ui_commit_type_commit ui_commit_type_amend > + > + do_select_commit_type > + if {$selected_commit_type eq {new}} { > + .mbar.commit entryconf [mc "New Commit"] \ > + -accelerator {} > + .mbar.commit entryconf [mc "Amend Last Commit"] \ > + -accelerator $::M1T-E > + } elseif {$selected_commit_type eq {amend}} { > + .mbar.commit entryconf [mc "New Commit"] \ > + -accelerator $::M1T-E > + .mbar.commit entryconf [mc "Amend Last Commit"] \ > + -accelerator {} > + } > +} > + > if {[is_enabled multicommit] || [is_enabled singlecommit]} { > menu .mbar.commit > > if {![is_enabled nocommit]} { > .mbar.commit add radiobutton \ > -label [mc "New Commit"] \ > - -command do_select_commit_type \ > + -command ui_select_commit_type \ > -variable selected_commit_type \ > -value new > lappend disable_on_lock \ > @@ -2838,7 +2866,8 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} { > > .mbar.commit add radiobutton \ > -label [mc "Amend Last Commit"] \ > - -command do_select_commit_type \ > + -accelerator $M1T-E \ > + -command ui_select_commit_type \ > -variable selected_commit_type \ > -value amend > lappend disable_on_lock \ > @@ -3315,14 +3344,14 @@ set ui_coml .vpane.lower.commarea.buffer.header.l > if {![is_enabled nocommit]} { > ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \ > -text [mc "New Commit"] \ > - -command do_select_commit_type \ > + -command ui_select_commit_type \ > -variable selected_commit_type \ > -value new > lappend disable_on_lock \ > [list .vpane.lower.commarea.buffer.header.new conf -state] > ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \ > -text [mc "Amend Last Commit"] \ > - -command do_select_commit_type \ > + -command ui_select_commit_type \ > -variable selected_commit_type \ > -value amend > lappend disable_on_lock \ > @@ -3837,6 +3866,8 @@ bind . <$M1B-Key-j> do_revert_selection > bind . <$M1B-Key-J> do_revert_selection > bind . <$M1B-Key-i> do_add_all > bind . <$M1B-Key-I> do_add_all > +bind . <$M1B-Key-e> toggle_commit_type > +bind . <$M1B-Key-E> toggle_commit_type > bind . <$M1B-Key-minus> {show_less_context;break} > bind . <$M1B-Key-KP_Subtract> {show_less_context;break} > bind . <$M1B-Key-equal> {show_more_context;break} > -- > 2.21.0.windows.1 > -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v4] git-gui: add hotkey to toggle "Amend Last Commit" 2019-09-13 14:37 ` Pratyush Yadav @ 2019-09-13 21:11 ` Birger Skogeng Pedersen 2019-09-13 21:32 ` Birger Skogeng Pedersen 0 siblings, 1 reply; 24+ messages in thread From: Birger Skogeng Pedersen @ 2019-09-13 21:11 UTC (permalink / raw) To: me; +Cc: git, Birger Skogeng Pedersen, Bert Wesarg Selecting whether to "Amend Last Commit" or not does not have a hotkey. With this patch, the user may toggle between the two options with CTRL/CMD+e. Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com> Rebased-by: Bert Wesarg <bert.wesarg@googlemail.com> --- git-gui.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/git-gui.sh b/git-gui.sh index 5bc21b8..d6e4631 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -2640,6 +2640,12 @@ proc show_less_context {} { } } +proc toggle_commit_type {} { + global commit_type_is_amend + set commit_type_is_amend [expr !$commit_type_is_amend] + do_select_commit_type +} + ###################################################################### ## ## ui construction @@ -2830,6 +2836,7 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} { if {![is_enabled nocommit]} { .mbar.commit add radiobutton \ -label [mc "New Commit"] \ + -accelerator $M1T-E \ -command do_select_commit_type \ -variable selected_commit_type \ -value new @@ -3837,6 +3844,8 @@ bind . <$M1B-Key-j> do_revert_selection bind . <$M1B-Key-J> do_revert_selection bind . <$M1B-Key-i> do_add_all bind . <$M1B-Key-I> do_add_all +bind . <$M1B-Key-e> toggle_commit_type +bind . <$M1B-Key-E> toggle_commit_type bind . <$M1B-Key-minus> {show_less_context;break} bind . <$M1B-Key-KP_Subtract> {show_less_context;break} bind . <$M1B-Key-equal> {show_more_context;break} -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v4] git-gui: add hotkey to toggle "Amend Last Commit" 2019-09-13 21:11 ` [PATCH v4] " Birger Skogeng Pedersen @ 2019-09-13 21:32 ` Birger Skogeng Pedersen 2019-09-13 22:11 ` Pratyush Yadav 0 siblings, 1 reply; 24+ messages in thread From: Birger Skogeng Pedersen @ 2019-09-13 21:32 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Git List, Bert Wesarg On Fri, Sep 13, 2019 at 4:37 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > Hi Birger, > > I'm afraid you are working on an older version of this patch. You should > be re-rolling [0], which works well with Bert's "amend check button" > change. > > [0] https://public-inbox.org/git/b82a00441ff1a6a9cea3fd235c1c33729ec31b71.1567713659.git.bert.wesarg@googlemail.com/ Forgive me, I get a little bit confused. Should my patch be based on "next" or "master" branch? Also, is it an issue that this patch won't work unless you merge Bert's 1/2 patch[0]? Your feedback cannot be too specific, I want to learn how to do this properly :-) [0] https://public-inbox.org/git/ab1f68cc8552e405c9d04622be1e728ab81bda17.1567713659.git.bert.wesarg@googlemail.com/ Birger ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] git-gui: add hotkey to toggle "Amend Last Commit" 2019-09-13 21:32 ` Birger Skogeng Pedersen @ 2019-09-13 22:11 ` Pratyush Yadav 2019-09-14 9:07 ` Birger Skogeng Pedersen 0 siblings, 1 reply; 24+ messages in thread From: Pratyush Yadav @ 2019-09-13 22:11 UTC (permalink / raw) To: Birger Skogeng Pedersen; +Cc: Git List, Bert Wesarg, Junio C Hamano +Cc Junio so you know what development model I'm using, and comment with your thoughts (if you want to). On 13/09/19 11:32PM, Birger Skogeng Pedersen wrote: > On Fri, Sep 13, 2019 at 4:37 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > > Hi Birger, > > > > I'm afraid you are working on an older version of this patch. You should > > be re-rolling [0], which works well with Bert's "amend check button" > > change. > > > > [0] https://public-inbox.org/git/b82a00441ff1a6a9cea3fd235c1c33729ec31b71.1567713659.git.bert.wesarg@googlemail.com/ > > Forgive me, I get a little bit confused. Should my patch be based on > "next" or "master" branch? For git-gui, ignore "next" for now. I considered using a model similar to Git where patches first get queued to "next" (or "pu" depending on how finished they are). And then after some time letting people use them, they are merged to "master" which eventually goes in the release. But this seems to be too complicated to me without any clear benefit. I think for now using just "master" for git-gui should be fine, since I won't directly release git-gui. Instead I'll periodically ask Junio to pull changes from my master. This will be our "release". So essentially my "master" for now acts as a place for people involved in development to test out all the changes, and then the rest of the world gets the new version along with Git's release. (Junio, you have done this for much longer than I have, so is there a major problem with my workflow?) If all this seems too complicated, just work on top of my "master". The rest of it is mostly my problem. > Also, is it an issue that this patch won't work unless you merge > Bert's 1/2 patch[0]? Your patch is dependent on Bert's patch. This means I will have to merge his patch first, and then yours. And that makes complete sense. That's how dependent changes should work. So no, it is not an issue that this patch won't work unless I merge Bert's patch first. So while my advice above was to work on top of "master", that does not apply in this case since your patch is dependent on someone's patch which isn't in master yet. So in this specific case, you should base your patch on top of Bert's. Otherwise there are two problems: - Your patch in and of itself makes little sense, it probably would crash or do some unexpected stuff. This hurts people doing a bisect, where if they land on your patch stuff is broken, and they have to manually move a commit up or down to continue. - Your patch will textually conflict with Bert's. That means I'd first merge Bert's patch since yours doesn't work without his. But then when I merge yours, I'd get a nasty merge conflict that is not easy to resolve and leaves chances for a subtle bug. > Your feedback cannot be too specific, I want to learn how to do this > properly :-) I'm not sure how well I explained this, so feel free to ask more questions if I didn't explain something properly :). > [0] https://public-inbox.org/git/ab1f68cc8552e405c9d04622be1e728ab81bda17.1567713659.git.bert.wesarg@googlemail.com/ -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v4] git-gui: add hotkey to toggle "Amend Last Commit" 2019-09-13 22:11 ` Pratyush Yadav @ 2019-09-14 9:07 ` Birger Skogeng Pedersen 2019-09-14 9:18 ` [PATCH v5] " Birger Skogeng Pedersen 0 siblings, 1 reply; 24+ messages in thread From: Birger Skogeng Pedersen @ 2019-09-14 9:07 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Git List, Bert Wesarg, Junio C Hamano Thanks, I really appreciate you taking time to explain it thoroughly. On Sat, Sep 14, 2019 at 12:11 AM Pratyush Yadav <me@yadavpratyush.com> wrote: > So while my advice above was to work on top of "master", that does not > apply in this case since your patch is dependent on someone's patch > which isn't in master yet. So in this specific case, you should base > your patch on top of Bert's. Okay, I might have to make another re-roll then. I based v4 on master. Birger ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v5] git-gui: add hotkey to toggle "Amend Last Commit" 2019-09-14 9:07 ` Birger Skogeng Pedersen @ 2019-09-14 9:18 ` Birger Skogeng Pedersen 2019-09-14 17:48 ` Pratyush Yadav 0 siblings, 1 reply; 24+ messages in thread From: Birger Skogeng Pedersen @ 2019-09-14 9:18 UTC (permalink / raw) To: me; +Cc: git, Birger Skogeng Pedersen, Bert Wesarg Selecting whether to "Amend Last Commit" or not does not have a hotkey. With this patch, the user may toggle between the two options with CTRL/CMD+e. Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com> Rebased-by: Bert Wesarg <bert.wesarg@googlemail.com> --- git-gui.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/git-gui.sh b/git-gui.sh index c7d9103..790adf1 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -2664,6 +2664,12 @@ proc focus_widget {widget} { } } +proc toggle_commit_type {} { + global commit_type_is_amend + set commit_type_is_amend [expr !$commit_type_is_amend] + do_select_commit_type +} + ###################################################################### ## ## ui construction @@ -3892,6 +3898,8 @@ bind . <$M1B-Key-j> do_revert_selection bind . <$M1B-Key-J> do_revert_selection bind . <$M1B-Key-i> do_add_all bind . <$M1B-Key-I> do_add_all +bind . <$M1B-Key-e> toggle_commit_type +bind . <$M1B-Key-E> toggle_commit_type bind . <$M1B-Key-minus> {show_less_context;break} bind . <$M1B-Key-KP_Subtract> {show_less_context;break} bind . <$M1B-Key-equal> {show_more_context;break} -- 2.21.0.windows.1 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v5] git-gui: add hotkey to toggle "Amend Last Commit" 2019-09-14 9:18 ` [PATCH v5] " Birger Skogeng Pedersen @ 2019-09-14 17:48 ` Pratyush Yadav 2019-09-16 12:23 ` Birger Skogeng Pedersen 0 siblings, 1 reply; 24+ messages in thread From: Pratyush Yadav @ 2019-09-14 17:48 UTC (permalink / raw) To: Birger Skogeng Pedersen; +Cc: git, Bert Wesarg You missed labelling the menu item of "Amend Last Commit" with the shortcut, like we do for other menu items bound to a hotkey like F5 for rescan, Ctrl-T for stage, etc. I added that locally. Thanks for the re-roll. Will queue. On 14/09/19 11:18AM, Birger Skogeng Pedersen wrote: > Selecting whether to "Amend Last Commit" or not does not have a hotkey. > > With this patch, the user may toggle between the two options with > CTRL/CMD+e. > > Signed-off-by: Birger Skogeng Pedersen <birger.sp@gmail.com> > Rebased-by: Bert Wesarg <bert.wesarg@googlemail.com> > --- > git-gui.sh | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/git-gui.sh b/git-gui.sh > index c7d9103..790adf1 100755 > --- a/git-gui.sh > +++ b/git-gui.sh > @@ -2664,6 +2664,12 @@ proc focus_widget {widget} { > } > } > > +proc toggle_commit_type {} { > + global commit_type_is_amend > + set commit_type_is_amend [expr !$commit_type_is_amend] > + do_select_commit_type > +} > + > ###################################################################### > ## > ## ui construction > @@ -3892,6 +3898,8 @@ bind . <$M1B-Key-j> do_revert_selection > bind . <$M1B-Key-J> do_revert_selection > bind . <$M1B-Key-i> do_add_all > bind . <$M1B-Key-I> do_add_all > +bind . <$M1B-Key-e> toggle_commit_type > +bind . <$M1B-Key-E> toggle_commit_type > bind . <$M1B-Key-minus> {show_less_context;break} > bind . <$M1B-Key-KP_Subtract> {show_less_context;break} > bind . <$M1B-Key-equal> {show_more_context;break} > -- > 2.21.0.windows.1 > -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v5] git-gui: add hotkey to toggle "Amend Last Commit" 2019-09-14 17:48 ` Pratyush Yadav @ 2019-09-16 12:23 ` Birger Skogeng Pedersen 0 siblings, 0 replies; 24+ messages in thread From: Birger Skogeng Pedersen @ 2019-09-16 12:23 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Git List Hi, On Sat, Sep 14, 2019 at 7:48 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > You missed labelling the menu item of "Amend Last Commit" with the > shortcut, like we do for other menu items bound to a hotkey like F5 for > rescan, Ctrl-T for stage, etc. I added that locally. Sorry about that. Thanks! Birger ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu 2019-09-12 16:29 ` Pratyush Yadav 2019-09-12 18:41 ` [PATCH v3] git-gui: add hotkey to toggle "Amend Last Commit" Birger Skogeng Pedersen @ 2019-09-12 21:34 ` Marc Branchaud 2019-09-12 22:23 ` Philip Oakley 1 sibling, 1 reply; 24+ messages in thread From: Marc Branchaud @ 2019-09-12 21:34 UTC (permalink / raw) To: Pratyush Yadav, Birger Skogeng Pedersen; +Cc: Bert Wesarg, Git List On 2019-09-12 12:29 p.m., Pratyush Yadav wrote: > On 12/09/19 08:05AM, Birger Skogeng Pedersen wrote: >> Hi Pratyush, >> >> On Wed, Sep 11, 2019 at 10:55 PM Pratyush Yadav <me@yadavpratyush.com> wrote: >>> Also, I notice that the bindings for other letters have the same >>> function bound for both small and capital letters (IOW, same behavior >>> with shift held and released). >>> >>> I don't necessarily think that is a great idea. It is a pretty common >>> pattern to have, say Ctrl+a, do something, and Ctrl+Shift+a, do >>> something else. Just want to pick your brain on whether you think we >>> should do the same thing for both Ctrl+e and for Ctrl+E (aka >>> Ctrl+Shift+e), or just bind it to Ctrl+e, and leave Ctrl+E for something >>> else. >> >> I just tested what happens when you press Ctrl+e while Caps Lock is >> enabled; the Ctrl+e binding is not invoked. That's probably why other >> key bindings have the same function bound for both lower- and >> upper-case letters, to have the same behaviour with/without Caps Lock >> enabled. With that in mind, we should probably bind Ctrl+E aswell. > > Nice catch! Makes sense to have the same behaviour for both caps lock > enabled and disabled. (I've been a git-gui user for many years...) I disagree! Who expects anything to work properly when capslock is on? M. >> >> Should I create and send a new patch? > > Yes, please do. > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu 2019-09-12 21:34 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Marc Branchaud @ 2019-09-12 22:23 ` Philip Oakley 2019-09-13 7:50 ` Birger Skogeng Pedersen 0 siblings, 1 reply; 24+ messages in thread From: Philip Oakley @ 2019-09-12 22:23 UTC (permalink / raw) To: Marc Branchaud, Pratyush Yadav, Birger Skogeng Pedersen Cc: Bert Wesarg, Git List On 12/09/2019 22:34, Marc Branchaud wrote: >>> I just tested what happens when you press Ctrl+e while Caps Lock is >>> enabled; the Ctrl+e binding is not invoked. That's probably why other >>> key bindings have the same function bound for both lower- and >>> upper-case letters, to have the same behaviour with/without Caps Lock >>> enabled. With that in mind, we should probably bind Ctrl+E aswell. >> >> Nice catch! Makes sense to have the same behaviour for both caps lock >> enabled and disabled. > > (I've been a git-gui user for many years...) > > I disagree! Who expects anything to work properly when capslock is on? > > M. > I'd tend to agree. In other areas the use of shift is often used as the complement of the unshifted action, so it does feel 'odd'. Thus it could be used directly as the bool for amend or direct commit. This all assumes that Caps Lock is equivalent to having the shift on, rather than being a special extra key. Philip ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu 2019-09-12 22:23 ` Philip Oakley @ 2019-09-13 7:50 ` Birger Skogeng Pedersen 2019-09-13 13:55 ` Marc Branchaud 2019-09-13 17:47 ` Pratyush Yadav 0 siblings, 2 replies; 24+ messages in thread From: Birger Skogeng Pedersen @ 2019-09-13 7:50 UTC (permalink / raw) To: Philip Oakley, Marc Branchaud; +Cc: Pratyush Yadav, Bert Wesarg, Git List Hi Marc and Philip, On 12/09/2019 22:34, Marc Branchaud wrote: > I disagree! Who expects anything to work properly when capslock is on? Me :-) On Fri, Sep 13, 2019 at 12:23 AM Philip Oakley <philipoakley@iee.email> wrote: > I'd tend to agree. In other areas the use of shift is often used as the > complement of the unshifted action, so it does feel 'odd'. Thus it could > be used directly as the bool for amend or direct commit. > > This all assumes that Caps Lock is equivalent to having the shift on, > rather than being a special extra key. It seems all the Ctrl+(lowercase character) hotkeys in git-gui have an equivalent Ctrl+(uppercase character). So for this feature, we should keep the Ctrl+E bind aswell as the Ctrl+e bind. If nothing else, to keep it consistent with the rest of the hotkey bindings. But honestly, (as Marc pointed out) it is a quite weird that Ctrl+Shift+(character) has the excact same function as Ctrl+(character). Perhaps we should find another way to bind the hotkeys, where the state of Caps Lock doesn't matter? If possible. Birger ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu 2019-09-13 7:50 ` Birger Skogeng Pedersen @ 2019-09-13 13:55 ` Marc Branchaud 2019-09-13 17:47 ` Pratyush Yadav 1 sibling, 0 replies; 24+ messages in thread From: Marc Branchaud @ 2019-09-13 13:55 UTC (permalink / raw) To: Birger Skogeng Pedersen, Philip Oakley Cc: Pratyush Yadav, Bert Wesarg, Git List On 2019-09-13 3:50 a.m., Birger Skogeng Pedersen wrote: > Hi Marc and Philip, > > > On 12/09/2019 22:34, Marc Branchaud wrote: >> I disagree! Who expects anything to work properly when capslock is on? > > Me :-) Fair enough, though I imagine you have a pretty narrow definition of "anything". :) > On Fri, Sep 13, 2019 at 12:23 AM Philip Oakley <philipoakley@iee.email> wrote: >> I'd tend to agree. In other areas the use of shift is often used as the >> complement of the unshifted action, so it does feel 'odd'. Thus it could >> be used directly as the bool for amend or direct commit. >> >> This all assumes that Caps Lock is equivalent to having the shift on, >> rather than being a special extra key. > > It seems all the Ctrl+(lowercase character) hotkeys in git-gui have an > equivalent Ctrl+(uppercase character). > So for this feature, we should keep the Ctrl+E bind aswell as the > Ctrl+e bind. If nothing else, to keep it consistent with the rest of > the hotkey bindings. Ah, OK. I agree that keeping git-gui internally consistent trumps the other considerations. M. > But honestly, (as Marc pointed out) it is a quite weird that > Ctrl+Shift+(character) has the excact same function as > Ctrl+(character). Perhaps we should find another way to bind the > hotkeys, where the state of Caps Lock doesn't matter? If possible. > > > Birger > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu 2019-09-13 7:50 ` Birger Skogeng Pedersen 2019-09-13 13:55 ` Marc Branchaud @ 2019-09-13 17:47 ` Pratyush Yadav 1 sibling, 0 replies; 24+ messages in thread From: Pratyush Yadav @ 2019-09-13 17:47 UTC (permalink / raw) To: Birger Skogeng Pedersen Cc: Philip Oakley, Marc Branchaud, Bert Wesarg, Git List On 13/09/19 09:50AM, Birger Skogeng Pedersen wrote: > Hi Marc and Philip, > > > On 12/09/2019 22:34, Marc Branchaud wrote: > > I disagree! Who expects anything to work properly when capslock is on? > > Me :-) > > > On Fri, Sep 13, 2019 at 12:23 AM Philip Oakley <philipoakley@iee.email> wrote: > > I'd tend to agree. In other areas the use of shift is often used as the > > complement of the unshifted action, so it does feel 'odd'. Thus it could > > be used directly as the bool for amend or direct commit. > > > > This all assumes that Caps Lock is equivalent to having the shift on, > > rather than being a special extra key. > > It seems all the Ctrl+(lowercase character) hotkeys in git-gui have an > equivalent Ctrl+(uppercase character). > So for this feature, we should keep the Ctrl+E bind aswell as the > Ctrl+e bind. If nothing else, to keep it consistent with the rest of > the hotkey bindings. I agree with this that we should keep it consistent with the rest of the bindings for now... > But honestly, (as Marc pointed out) it is a quite weird that > Ctrl+Shift+(character) has the excact same function as > Ctrl+(character). Perhaps we should find another way to bind the > hotkeys, where the state of Caps Lock doesn't matter? If possible. ...but I'd love to see this happen. To me shift is a modifier. No matter whether Caps Lock is pressed or not, it should not do the shift-modified behavior (that's just me, maybe other people think differently). AFAIK, Tk does not provide any direct way to find out whether shift is pressed (correct me if I'm wrong). What you instead have to do is some bit arithmetic on the number passed to the "Key" event via the "%s" substitution. Source: [0]. We can probably have a bind_alpha procedure that takes two arguments: what to run when shift is pressed and what to run when it isn't. This, of course, would be incompatible with the current behavior, but do people even keep the Caps Lock on? I personally use it so rarely I have my Caps Lock bound to Escape because I might as well use that key for something I use more often. [0] https://blog.tcl.tk/4238 -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton 2019-09-05 20:09 [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton Bert Wesarg 2019-09-05 20:09 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Bert Wesarg @ 2019-09-05 20:33 ` Pratyush Yadav 2019-09-11 20:15 ` Pratyush Yadav 2 siblings, 0 replies; 24+ messages in thread From: Pratyush Yadav @ 2019-09-05 20:33 UTC (permalink / raw) To: Bert Wesarg; +Cc: git Hi Bert, I have some _very_ busy few days coming up, so I won't be able to devote much time to this right now. I will take a look at the series and your other patches by the coming Wednesday. Thanks. [snip] -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton 2019-09-05 20:09 [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton Bert Wesarg 2019-09-05 20:09 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Bert Wesarg 2019-09-05 20:33 ` [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton Pratyush Yadav @ 2019-09-11 20:15 ` Pratyush Yadav 2019-09-12 19:35 ` Bert Wesarg 2019-09-12 19:39 ` Bert Wesarg 2 siblings, 2 replies; 24+ messages in thread From: Pratyush Yadav @ 2019-09-11 20:15 UTC (permalink / raw) To: Bert Wesarg; +Cc: git Typo in the subject. s/checketton/checkbutton/ On 05/09/19 10:09PM, Bert Wesarg wrote: > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com> > --- > git-gui.sh | 36 +++++++++--------------------------- > lib/checkout_op.tcl | 6 +++--- > lib/commit.tcl | 4 ++-- > lib/index.tcl | 8 ++++---- > 4 files changed, 18 insertions(+), 36 deletions(-) > > diff --git a/git-gui.sh b/git-gui.sh > index 5bc21b8..80a07d5 100755 > --- a/git-gui.sh > +++ b/git-gui.sh > @@ -1341,6 +1341,7 @@ set HEAD {} > set PARENT {} > set MERGE_HEAD [list] > set commit_type {} > +set commit_type_is_amend 0 > set empty_tree {} > set current_branch {} > set is_detached 0 > @@ -1348,7 +1349,6 @@ set current_diff_path {} > set is_3way_diff 0 > set is_submodule_diff 0 > set is_conflict_diff 0 > -set selected_commit_type new > set diff_empty_count 0 > > set nullid "0000000000000000000000000000000000000000" > @@ -1435,7 +1435,7 @@ proc PARENT {} { > } > > proc force_amend {} { > - global selected_commit_type > + global commit_type_is_amend > global HEAD PARENT MERGE_HEAD commit_type > > repository_state newType newHEAD newMERGE_HEAD > @@ -1444,7 +1444,7 @@ proc force_amend {} { > set MERGE_HEAD $newMERGE_HEAD > set commit_type $newType > > - set selected_commit_type amend > + set commit_type_is_amend 1 Why do we need a separate variable for this? Why not just check commit_type to know whether it is an amend or not? My guess after reading the patch is that we need to associate a variable with the checkbutton to decide its state, and since there are multiple types of amend that commit_type can be (amend, amend-initial, amend-merge), it is not easy to use it directly. Am I guessing correctly? > do_select_commit_type > } > > @@ -2828,19 +2828,10 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} { > menu .mbar.commit > > if {![is_enabled nocommit]} { > - .mbar.commit add radiobutton \ > - -label [mc "New Commit"] \ > - -command do_select_commit_type \ > - -variable selected_commit_type \ > - -value new > - lappend disable_on_lock \ > - [list .mbar.commit entryconf [.mbar.commit index last] -state] > - > - .mbar.commit add radiobutton \ > + .mbar.commit add checkbutton \ > -label [mc "Amend Last Commit"] \ > - -command do_select_commit_type \ > - -variable selected_commit_type \ > - -value amend > + -variable commit_type_is_amend \ > + -command do_select_commit_type > lappend disable_on_lock \ > [list .mbar.commit entryconf [.mbar.commit index last] -state] > > @@ -3313,18 +3304,10 @@ set ui_comm .vpane.lower.commarea.buffer.frame.t > set ui_coml .vpane.lower.commarea.buffer.header.l > > if {![is_enabled nocommit]} { > - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \ > - -text [mc "New Commit"] \ > - -command do_select_commit_type \ > - -variable selected_commit_type \ > - -value new > - lappend disable_on_lock \ > - [list .vpane.lower.commarea.buffer.header.new conf -state] > - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \ > + ${NS}::checkbutton .vpane.lower.commarea.buffer.header.amend \ > -text [mc "Amend Last Commit"] \ > - -command do_select_commit_type \ > - -variable selected_commit_type \ > - -value amend > + -variable commit_type_is_amend \ > + -command do_select_commit_type > lappend disable_on_lock \ > [list .vpane.lower.commarea.buffer.header.amend conf -state] > } > @@ -3349,7 +3332,6 @@ pack $ui_coml -side left -fill x > > if {![is_enabled nocommit]} { > pack .vpane.lower.commarea.buffer.header.amend -side right > - pack .vpane.lower.commarea.buffer.header.new -side right > } > > textframe .vpane.lower.commarea.buffer.frame > diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl > index 9e7412c..a522829 100644 > --- a/lib/checkout_op.tcl > +++ b/lib/checkout_op.tcl > @@ -389,7 +389,7 @@ $err > } > > method _after_readtree {} { > - global selected_commit_type commit_type HEAD MERGE_HEAD PARENT > + global commit_type HEAD MERGE_HEAD PARENT This cleans up an unused variable declaration. Nice. > global current_branch is_detached > global ui_comm > > @@ -490,12 +490,12 @@ method _update_repo_state {} { > # amend mode our file lists are accurate and we can avoid > # the rescan. > # > - global selected_commit_type commit_type HEAD MERGE_HEAD PARENT > + global commit_type_is_amend commit_type HEAD MERGE_HEAD PARENT > global ui_comm > > unlock_index > set name [_name $this] > - set selected_commit_type new > + set commit_type_is_amend 0 > if {[string match amend* $commit_type]} { > $ui_comm delete 0.0 end > $ui_comm edit reset > diff --git a/lib/commit.tcl b/lib/commit.tcl > index 83620b7..384f18f 100644 > --- a/lib/commit.tcl > +++ b/lib/commit.tcl > @@ -327,7 +327,7 @@ proc commit_writetree {curHEAD msg_p} { > proc commit_committree {fd_wt curHEAD msg_p} { > global HEAD PARENT MERGE_HEAD commit_type commit_author > global current_branch > - global ui_comm selected_commit_type > + global ui_comm commit_type_is_amend > global file_states selected_paths rescan_active > global repo_config > global env > @@ -461,8 +461,8 @@ A rescan will be automatically started now. > > # -- Update in memory status > # > - set selected_commit_type new > set commit_type normal > + set commit_type_is_amend 0 > set HEAD $cmt_id > set PARENT $cmt_id > set MERGE_HEAD [list] > diff --git a/lib/index.tcl b/lib/index.tcl > index b588db1..e07b7a3 100644 > --- a/lib/index.tcl > +++ b/lib/index.tcl > @@ -466,19 +466,19 @@ proc do_revert_selection {} { > } > > proc do_select_commit_type {} { > - global commit_type selected_commit_type > + global commit_type commit_type_is_amend > > - if {$selected_commit_type eq {new} > + if {$commit_type_is_amend == 0 > && [string match amend* $commit_type]} { > create_new_commit > - } elseif {$selected_commit_type eq {amend} > + } elseif {$commit_type_is_amend == 1 > && ![string match amend* $commit_type]} { Not exactly related to your change, but shouldn't these "string match amend*" in the two ifs be assertions instead of checks? If $commit_type_is_amend == 0, then $commit_type should _always_ be amend*, and if $commit_type_is_amend == 1, then $commit_type should _never_ be amend*. I don't see assertions being used anywhere, but I suppose we should look into them in the future. It would be great if you can start using something like that here, but I'm fine with keeping this like it is right now too. > load_last_commit > > # The amend request was rejected... > # > if {![string match amend* $commit_type]} { > - set selected_commit_type new > + set commit_type_is_amend 0 > } > } > } I tested it on my setup and it works fine. Thanks. -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton 2019-09-11 20:15 ` Pratyush Yadav @ 2019-09-12 19:35 ` Bert Wesarg 2019-09-13 17:14 ` Pratyush Yadav 2019-09-12 19:39 ` Bert Wesarg 1 sibling, 1 reply; 24+ messages in thread From: Bert Wesarg @ 2019-09-12 19:35 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Git Mailing List On Wed, Sep 11, 2019 at 10:15 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > > Typo in the subject. s/checketton/checkbutton/ > > On 05/09/19 10:09PM, Bert Wesarg wrote: > > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com> > > --- > > git-gui.sh | 36 +++++++++--------------------------- > > lib/checkout_op.tcl | 6 +++--- > > lib/commit.tcl | 4 ++-- > > lib/index.tcl | 8 ++++---- > > 4 files changed, 18 insertions(+), 36 deletions(-) > > > > diff --git a/git-gui.sh b/git-gui.sh > > index 5bc21b8..80a07d5 100755 > > --- a/git-gui.sh > > +++ b/git-gui.sh > > @@ -1341,6 +1341,7 @@ set HEAD {} > > set PARENT {} > > set MERGE_HEAD [list] > > set commit_type {} > > +set commit_type_is_amend 0 > > set empty_tree {} > > set current_branch {} > > set is_detached 0 > > @@ -1348,7 +1349,6 @@ set current_diff_path {} > > set is_3way_diff 0 > > set is_submodule_diff 0 > > set is_conflict_diff 0 > > -set selected_commit_type new > > set diff_empty_count 0 > > > > set nullid "0000000000000000000000000000000000000000" > > @@ -1435,7 +1435,7 @@ proc PARENT {} { > > } > > > > proc force_amend {} { > > - global selected_commit_type > > + global commit_type_is_amend > > global HEAD PARENT MERGE_HEAD commit_type > > > > repository_state newType newHEAD newMERGE_HEAD > > @@ -1444,7 +1444,7 @@ proc force_amend {} { > > set MERGE_HEAD $newMERGE_HEAD > > set commit_type $newType > > > > - set selected_commit_type amend > > + set commit_type_is_amend 1 > > Why do we need a separate variable for this? Why not just check > commit_type to know whether it is an amend or not? My guess after > reading the patch is that we need to associate a variable with the > checkbutton to decide its state, and since there are multiple types of > amend that commit_type can be (amend, amend-initial, amend-merge), it is > not easy to use it directly. Am I guessing correctly? > > > do_select_commit_type > > } > > > > @@ -2828,19 +2828,10 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} { > > menu .mbar.commit > > > > if {![is_enabled nocommit]} { > > - .mbar.commit add radiobutton \ > > - -label [mc "New Commit"] \ > > - -command do_select_commit_type \ > > - -variable selected_commit_type \ > > - -value new > > - lappend disable_on_lock \ > > - [list .mbar.commit entryconf [.mbar.commit index last] -state] > > - > > - .mbar.commit add radiobutton \ > > + .mbar.commit add checkbutton \ > > -label [mc "Amend Last Commit"] \ > > - -command do_select_commit_type \ > > - -variable selected_commit_type \ > > - -value amend > > + -variable commit_type_is_amend \ > > + -command do_select_commit_type > > lappend disable_on_lock \ > > [list .mbar.commit entryconf [.mbar.commit index last] -state] > > > > @@ -3313,18 +3304,10 @@ set ui_comm .vpane.lower.commarea.buffer.frame.t > > set ui_coml .vpane.lower.commarea.buffer.header.l > > > > if {![is_enabled nocommit]} { > > - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \ > > - -text [mc "New Commit"] \ > > - -command do_select_commit_type \ > > - -variable selected_commit_type \ > > - -value new > > - lappend disable_on_lock \ > > - [list .vpane.lower.commarea.buffer.header.new conf -state] > > - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \ > > + ${NS}::checkbutton .vpane.lower.commarea.buffer.header.amend \ > > -text [mc "Amend Last Commit"] \ > > - -command do_select_commit_type \ > > - -variable selected_commit_type \ > > - -value amend > > + -variable commit_type_is_amend \ > > + -command do_select_commit_type > > lappend disable_on_lock \ > > [list .vpane.lower.commarea.buffer.header.amend conf -state] > > } > > @@ -3349,7 +3332,6 @@ pack $ui_coml -side left -fill x > > > > if {![is_enabled nocommit]} { > > pack .vpane.lower.commarea.buffer.header.amend -side right > > - pack .vpane.lower.commarea.buffer.header.new -side right > > } > > > > textframe .vpane.lower.commarea.buffer.frame > > diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl > > index 9e7412c..a522829 100644 > > --- a/lib/checkout_op.tcl > > +++ b/lib/checkout_op.tcl > > @@ -389,7 +389,7 @@ $err > > } > > > > method _after_readtree {} { > > - global selected_commit_type commit_type HEAD MERGE_HEAD PARENT > > + global commit_type HEAD MERGE_HEAD PARENT > > This cleans up an unused variable declaration. Nice. > > > global current_branch is_detached > > global ui_comm > > > > @@ -490,12 +490,12 @@ method _update_repo_state {} { > > # amend mode our file lists are accurate and we can avoid > > # the rescan. > > # > > - global selected_commit_type commit_type HEAD MERGE_HEAD PARENT > > + global commit_type_is_amend commit_type HEAD MERGE_HEAD PARENT > > global ui_comm > > > > unlock_index > > set name [_name $this] > > - set selected_commit_type new > > + set commit_type_is_amend 0 > > if {[string match amend* $commit_type]} { > > $ui_comm delete 0.0 end > > $ui_comm edit reset > > diff --git a/lib/commit.tcl b/lib/commit.tcl > > index 83620b7..384f18f 100644 > > --- a/lib/commit.tcl > > +++ b/lib/commit.tcl > > @@ -327,7 +327,7 @@ proc commit_writetree {curHEAD msg_p} { > > proc commit_committree {fd_wt curHEAD msg_p} { > > global HEAD PARENT MERGE_HEAD commit_type commit_author > > global current_branch > > - global ui_comm selected_commit_type > > + global ui_comm commit_type_is_amend > > global file_states selected_paths rescan_active > > global repo_config > > global env > > @@ -461,8 +461,8 @@ A rescan will be automatically started now. > > > > # -- Update in memory status > > # > > - set selected_commit_type new > > set commit_type normal > > + set commit_type_is_amend 0 > > set HEAD $cmt_id > > set PARENT $cmt_id > > set MERGE_HEAD [list] > > diff --git a/lib/index.tcl b/lib/index.tcl > > index b588db1..e07b7a3 100644 > > --- a/lib/index.tcl > > +++ b/lib/index.tcl > > @@ -466,19 +466,19 @@ proc do_revert_selection {} { > > } > > > > proc do_select_commit_type {} { > > - global commit_type selected_commit_type > > + global commit_type commit_type_is_amend > > > > - if {$selected_commit_type eq {new} > > + if {$commit_type_is_amend == 0 > > && [string match amend* $commit_type]} { > > create_new_commit > > - } elseif {$selected_commit_type eq {amend} > > + } elseif {$commit_type_is_amend == 1 > > && ![string match amend* $commit_type]} { > > Not exactly related to your change, but shouldn't these "string match > amend*" in the two ifs be assertions instead of checks? If > $commit_type_is_amend == 0, then $commit_type should _always_ be amend*, > and if $commit_type_is_amend == 1, then $commit_type should _never_ be > amend*. > AFAIU this now is, that the former 'selected_commit_type' was also used as a request for the commit type, you set it to the desired one, than call do_select_commit_type, it will than check if a state change actually happen, and if it was to request an amend, which may also fail, it goes back to !amend. Thus its not an assert. > I don't see assertions being used anywhere, but I suppose we should look > into them in the future. It would be great if you can start using > something like that here, but I'm fine with keeping this like it is > right now too. > > > load_last_commit > > > > # The amend request was rejected... > > # > > if {![string match amend* $commit_type]} { > > - set selected_commit_type new > > + set commit_type_is_amend 0 > > } > > } > > } > > I tested it on my setup and it works fine. Thanks. > > -- > Regards, > Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton 2019-09-12 19:35 ` Bert Wesarg @ 2019-09-13 17:14 ` Pratyush Yadav 0 siblings, 0 replies; 24+ messages in thread From: Pratyush Yadav @ 2019-09-13 17:14 UTC (permalink / raw) To: Bert Wesarg; +Cc: Git Mailing List On 12/09/19 09:35PM, Bert Wesarg wrote: > On Wed, Sep 11, 2019 at 10:15 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > > > > Typo in the subject. s/checketton/checkbutton/ > > > > On 05/09/19 10:09PM, Bert Wesarg wrote: > > > diff --git a/lib/index.tcl b/lib/index.tcl > > > index b588db1..e07b7a3 100644 > > > --- a/lib/index.tcl > > > +++ b/lib/index.tcl > > > @@ -466,19 +466,19 @@ proc do_revert_selection {} { > > > } > > > > > > proc do_select_commit_type {} { > > > - global commit_type selected_commit_type > > > + global commit_type commit_type_is_amend > > > > > > - if {$selected_commit_type eq {new} > > > + if {$commit_type_is_amend == 0 > > > && [string match amend* $commit_type]} { > > > create_new_commit > > > - } elseif {$selected_commit_type eq {amend} > > > + } elseif {$commit_type_is_amend == 1 > > > && ![string match amend* $commit_type]} { > > > > Not exactly related to your change, but shouldn't these "string match > > amend*" in the two ifs be assertions instead of checks? If > > $commit_type_is_amend == 0, then $commit_type should _always_ be amend*, > > and if $commit_type_is_amend == 1, then $commit_type should _never_ be > > amend*. > > > > AFAIU this now is, that the former 'selected_commit_type' was also > used as a request for the commit type, you set it to the desired one, > than call do_select_commit_type, it will than check if a state change > actually happen, and if it was to request an amend, which may also > fail, it goes back to !amend. > > Thus its not an assert. Thanks for explaining. While I'm not the biggest fan of this design, let's just keep it this way for now. -- Regards, Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton 2019-09-11 20:15 ` Pratyush Yadav 2019-09-12 19:35 ` Bert Wesarg @ 2019-09-12 19:39 ` Bert Wesarg 1 sibling, 0 replies; 24+ messages in thread From: Bert Wesarg @ 2019-09-12 19:39 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Git Mailing List, Birger Skogeng Pedersen On Wed, Sep 11, 2019 at 10:15 PM Pratyush Yadav <me@yadavpratyush.com> wrote: > > Typo in the subject. s/checketton/checkbutton/\ Will re-roll and drop the actual keybinding patch, so that Birger can resend his part, Bert > > On 05/09/19 10:09PM, Bert Wesarg wrote: > > Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com> > > --- > > git-gui.sh | 36 +++++++++--------------------------- > > lib/checkout_op.tcl | 6 +++--- > > lib/commit.tcl | 4 ++-- > > lib/index.tcl | 8 ++++---- > > 4 files changed, 18 insertions(+), 36 deletions(-) > > > > diff --git a/git-gui.sh b/git-gui.sh > > index 5bc21b8..80a07d5 100755 > > --- a/git-gui.sh > > +++ b/git-gui.sh > > @@ -1341,6 +1341,7 @@ set HEAD {} > > set PARENT {} > > set MERGE_HEAD [list] > > set commit_type {} > > +set commit_type_is_amend 0 > > set empty_tree {} > > set current_branch {} > > set is_detached 0 > > @@ -1348,7 +1349,6 @@ set current_diff_path {} > > set is_3way_diff 0 > > set is_submodule_diff 0 > > set is_conflict_diff 0 > > -set selected_commit_type new > > set diff_empty_count 0 > > > > set nullid "0000000000000000000000000000000000000000" > > @@ -1435,7 +1435,7 @@ proc PARENT {} { > > } > > > > proc force_amend {} { > > - global selected_commit_type > > + global commit_type_is_amend > > global HEAD PARENT MERGE_HEAD commit_type > > > > repository_state newType newHEAD newMERGE_HEAD > > @@ -1444,7 +1444,7 @@ proc force_amend {} { > > set MERGE_HEAD $newMERGE_HEAD > > set commit_type $newType > > > > - set selected_commit_type amend > > + set commit_type_is_amend 1 > > Why do we need a separate variable for this? Why not just check > commit_type to know whether it is an amend or not? My guess after > reading the patch is that we need to associate a variable with the > checkbutton to decide its state, and since there are multiple types of > amend that commit_type can be (amend, amend-initial, amend-merge), it is > not easy to use it directly. Am I guessing correctly? > > > do_select_commit_type > > } > > > > @@ -2828,19 +2828,10 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} { > > menu .mbar.commit > > > > if {![is_enabled nocommit]} { > > - .mbar.commit add radiobutton \ > > - -label [mc "New Commit"] \ > > - -command do_select_commit_type \ > > - -variable selected_commit_type \ > > - -value new > > - lappend disable_on_lock \ > > - [list .mbar.commit entryconf [.mbar.commit index last] -state] > > - > > - .mbar.commit add radiobutton \ > > + .mbar.commit add checkbutton \ > > -label [mc "Amend Last Commit"] \ > > - -command do_select_commit_type \ > > - -variable selected_commit_type \ > > - -value amend > > + -variable commit_type_is_amend \ > > + -command do_select_commit_type > > lappend disable_on_lock \ > > [list .mbar.commit entryconf [.mbar.commit index last] -state] > > > > @@ -3313,18 +3304,10 @@ set ui_comm .vpane.lower.commarea.buffer.frame.t > > set ui_coml .vpane.lower.commarea.buffer.header.l > > > > if {![is_enabled nocommit]} { > > - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.new \ > > - -text [mc "New Commit"] \ > > - -command do_select_commit_type \ > > - -variable selected_commit_type \ > > - -value new > > - lappend disable_on_lock \ > > - [list .vpane.lower.commarea.buffer.header.new conf -state] > > - ${NS}::radiobutton .vpane.lower.commarea.buffer.header.amend \ > > + ${NS}::checkbutton .vpane.lower.commarea.buffer.header.amend \ > > -text [mc "Amend Last Commit"] \ > > - -command do_select_commit_type \ > > - -variable selected_commit_type \ > > - -value amend > > + -variable commit_type_is_amend \ > > + -command do_select_commit_type > > lappend disable_on_lock \ > > [list .vpane.lower.commarea.buffer.header.amend conf -state] > > } > > @@ -3349,7 +3332,6 @@ pack $ui_coml -side left -fill x > > > > if {![is_enabled nocommit]} { > > pack .vpane.lower.commarea.buffer.header.amend -side right > > - pack .vpane.lower.commarea.buffer.header.new -side right > > } > > > > textframe .vpane.lower.commarea.buffer.frame > > diff --git a/lib/checkout_op.tcl b/lib/checkout_op.tcl > > index 9e7412c..a522829 100644 > > --- a/lib/checkout_op.tcl > > +++ b/lib/checkout_op.tcl > > @@ -389,7 +389,7 @@ $err > > } > > > > method _after_readtree {} { > > - global selected_commit_type commit_type HEAD MERGE_HEAD PARENT > > + global commit_type HEAD MERGE_HEAD PARENT > > This cleans up an unused variable declaration. Nice. > > > global current_branch is_detached > > global ui_comm > > > > @@ -490,12 +490,12 @@ method _update_repo_state {} { > > # amend mode our file lists are accurate and we can avoid > > # the rescan. > > # > > - global selected_commit_type commit_type HEAD MERGE_HEAD PARENT > > + global commit_type_is_amend commit_type HEAD MERGE_HEAD PARENT > > global ui_comm > > > > unlock_index > > set name [_name $this] > > - set selected_commit_type new > > + set commit_type_is_amend 0 > > if {[string match amend* $commit_type]} { > > $ui_comm delete 0.0 end > > $ui_comm edit reset > > diff --git a/lib/commit.tcl b/lib/commit.tcl > > index 83620b7..384f18f 100644 > > --- a/lib/commit.tcl > > +++ b/lib/commit.tcl > > @@ -327,7 +327,7 @@ proc commit_writetree {curHEAD msg_p} { > > proc commit_committree {fd_wt curHEAD msg_p} { > > global HEAD PARENT MERGE_HEAD commit_type commit_author > > global current_branch > > - global ui_comm selected_commit_type > > + global ui_comm commit_type_is_amend > > global file_states selected_paths rescan_active > > global repo_config > > global env > > @@ -461,8 +461,8 @@ A rescan will be automatically started now. > > > > # -- Update in memory status > > # > > - set selected_commit_type new > > set commit_type normal > > + set commit_type_is_amend 0 > > set HEAD $cmt_id > > set PARENT $cmt_id > > set MERGE_HEAD [list] > > diff --git a/lib/index.tcl b/lib/index.tcl > > index b588db1..e07b7a3 100644 > > --- a/lib/index.tcl > > +++ b/lib/index.tcl > > @@ -466,19 +466,19 @@ proc do_revert_selection {} { > > } > > > > proc do_select_commit_type {} { > > - global commit_type selected_commit_type > > + global commit_type commit_type_is_amend > > > > - if {$selected_commit_type eq {new} > > + if {$commit_type_is_amend == 0 > > && [string match amend* $commit_type]} { > > create_new_commit > > - } elseif {$selected_commit_type eq {amend} > > + } elseif {$commit_type_is_amend == 1 > > && ![string match amend* $commit_type]} { > > Not exactly related to your change, but shouldn't these "string match > amend*" in the two ifs be assertions instead of checks? If > $commit_type_is_amend == 0, then $commit_type should _always_ be amend*, > and if $commit_type_is_amend == 1, then $commit_type should _never_ be > amend*. > > I don't see assertions being used anywhere, but I suppose we should look > into them in the future. It would be great if you can start using > something like that here, but I'm fine with keeping this like it is > right now too. > > > load_last_commit > > > > # The amend request was rejected... > > # > > if {![string match amend* $commit_type]} { > > - set selected_commit_type new > > + set commit_type_is_amend 0 > > } > > } > > } > > I tested it on my setup and it works fine. Thanks. > > -- > Regards, > Pratyush Yadav ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-09-16 12:26 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-05 20:09 [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton Bert Wesarg 2019-09-05 20:09 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Bert Wesarg 2019-09-11 20:55 ` Pratyush Yadav 2019-09-12 6:05 ` Birger Skogeng Pedersen 2019-09-12 16:29 ` Pratyush Yadav 2019-09-12 18:41 ` [PATCH v3] git-gui: add hotkey to toggle "Amend Last Commit" Birger Skogeng Pedersen 2019-09-13 14:37 ` Pratyush Yadav 2019-09-13 21:11 ` [PATCH v4] " Birger Skogeng Pedersen 2019-09-13 21:32 ` Birger Skogeng Pedersen 2019-09-13 22:11 ` Pratyush Yadav 2019-09-14 9:07 ` Birger Skogeng Pedersen 2019-09-14 9:18 ` [PATCH v5] " Birger Skogeng Pedersen 2019-09-14 17:48 ` Pratyush Yadav 2019-09-16 12:23 ` Birger Skogeng Pedersen 2019-09-12 21:34 ` [PATCH 2/2] git-gui: add hotkey to toggle "Amend Last Commit" check button/menu Marc Branchaud 2019-09-12 22:23 ` Philip Oakley 2019-09-13 7:50 ` Birger Skogeng Pedersen 2019-09-13 13:55 ` Marc Branchaud 2019-09-13 17:47 ` Pratyush Yadav 2019-09-05 20:33 ` [PATCH 1/2] git-gui: convert new/amend commit radiobutton to checketton Pratyush Yadav 2019-09-11 20:15 ` Pratyush Yadav 2019-09-12 19:35 ` Bert Wesarg 2019-09-13 17:14 ` Pratyush Yadav 2019-09-12 19:39 ` Bert Wesarg
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).