[v4] git-gui: add hotkey to toggle "Amend Last Commit"
diff mbox series

Message ID 20190913211152.8860-1-birger.sp@gmail.com
State New
Headers show
Series
  • [v4] git-gui: add hotkey to toggle "Amend Last Commit"
Related show

Commit Message

Birger Skogeng Pedersen Sept. 13, 2019, 9:11 p.m. UTC
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(+)

Comments

Birger Skogeng Pedersen Sept. 13, 2019, 9:32 p.m. UTC | #1
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
Pratyush Yadav Sept. 13, 2019, 10:11 p.m. UTC | #2
+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/
Birger Skogeng Pedersen Sept. 14, 2019, 9:07 a.m. UTC | #3
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

Patch
diff mbox series

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}