# [PATCH] git-rebase--interactive.sh: Add new command "shell" 39 messages from 2010-11-04 to 2010-12-03. Participants: Kevin Ballard, Matthieu Moy, Ævar Arnfjörð Bjarmason, Erik Faye-Lund, Johannes Sixt, Yann Dirson, Eric Raible, Jonathan Nieder, Junio C Hamano. Thread: https://gitlist.dev/t/25638 ## Kevin Ballard, 2010-11-04 05:17 Subject: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <1288847836-84882-1-git-send-email-kevin@sb.org> URL: https://gitlist.dev/e/1288847836-84882-1-git-send-email-kevin%40sb.org ``` Add a new command "shell", which takes an option commit. It simply exits to the shell with the commit (if given) and a message telling the user how to resume the rebase. This is effectively the same thing as "x false" but much friendlier to the user. Signed-off-by: Kevin Ballard --- I discovered the need for this when I wanted to edit a commit, but apply a fixup first. The only way with the existing tools was an exec command that fails (e.g. "x false"). git-rebase--interactive.sh | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 9121bb6..3501757 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -566,6 +566,26 @@ do_next () { exit 1 fi ;; + !|"shell") + read -r command comment < "$TODO" + mark_action_done + # can't use $sha1 here for same reason as "exec" + line=$(git rev-list --pretty=oneline -1 --abbrev-commit --abbrev=7 HEAD) + sha1="${line%% *}" + rest="${line#* }" + echo "$sha1" > "$DOTEST"/stopped-sha + warn "Stopped at $sha1... $rest" + if test -n "$comment"; then + warn + warn " $comment" + fi + warn + warn "Once you are ready to continue, run" + warn + warn " git rebase --continue" + warn + exit 0 + ;; *) warn "Unknown command: $command $sha1 $rest" if git rev-parse --verify -q "$sha1" >/dev/null @@ -1007,6 +1027,7 @@ first and then run 'git rebase --continue' again." # s, squash = use commit, but meld into previous commit # f, fixup = like "squash", but discard this commit's log message # x , exec = Run a shell command , and stop if it fails +# !, shell = Exit to the shell # # If you remove a line here THAT COMMIT WILL BE LOST. # However, if you remove everything, the rebase will be aborted. -- 1.7.3.2.202.g3b863.dirty ``` ## Kevin Ballard, 2010-11-04 05:22 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <3014427A-06FC-4EEB-B823-F3716E1DA4E5@sb.org> URL: https://gitlist.dev/e/3014427A-06FC-4EEB-B823-F3716E1DA4E5%40sb.org In-Reply-To: <1288847836-84882-1-git-send-email-kevin@sb.org> ``` On Nov 3, 2010, at 10:17 PM, Kevin Ballard wrote: > Add a new command "shell", which takes an option commit. It simply exits > to the shell with the commit (if given) and a message telling the user how > to resume the rebase. This is effectively the same thing as "x false" but > much friendlier to the user. That was supposed to say "optional comment", not "option commit". And again below, "comment" not "commit". -Kevin Ballard ``` ## Matthieu Moy, 2010-11-04 08:42 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: URL: https://gitlist.dev/e/vpq39rhzdht.fsf%40bauges.imag.fr In-Reply-To: <1288847836-84882-1-git-send-email-kevin@sb.org> ``` Kevin Ballard writes: > Add a new command "shell", which takes an option commit. It simply exits > to the shell with the commit (if given) and a message telling the user how > to resume the rebase. "shell" sounds like you're going to execute something in a shell, not that you're going back to the shell. Looking at the commit message, I thought you had missed the "exec" command and re-implemented it. What about "pause", abbreviated as "p" for the command name? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ``` ## Kevin Ballard, 2010-11-04 08:53 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <914D7AE3-22D5-4069-B815-2B11A2897BE9@sb.org> URL: https://gitlist.dev/e/914D7AE3-22D5-4069-B815-2B11A2897BE9%40sb.org In-Reply-To: ``` On Nov 4, 2010, at 1:42 AM, Matthieu Moy wrote: > Kevin Ballard writes: > >> Add a new command "shell", which takes an option commit. It simply exits >> to the shell with the commit (if given) and a message telling the user how >> to resume the rebase. > > "shell" sounds like you're going to execute something in a shell, not > that you're going back to the shell. Looking at the commit message, I > thought you had missed the "exec" command and re-implemented it. > > What about "pause", abbreviated as "p" for the command name? That sounds like a reasonable suggestion, except "p" is already taken by "pick". I suppose this command could simply omit the short version. ---8<--- Subject: git-rebase--interactive.sh: Add new command "pause" Add a new command "pause", which takes an optional comment. It simply exits to the shell with the comment (if given) and a message telling the user how to resume the rebase. This is effectively the same thing as "x false" but much friendlier to the user. Signed-off-by: Kevin Ballard --- git-rebase--interactive.sh | 21 +++++++++++++++++++++ 1 files changed, 21 insertions(+), 0 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index a27952d..e29fd91 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -566,6 +566,26 @@ do_next () { exit 1 fi ;; + pause) + read -r command comment < "$TODO" + mark_action_done + # can't use $sha1 here for same reason as "exec" + line=$(git rev-list --pretty=oneline -1 --abbrev-commit --abbrev=7 HEAD) + sha1="${line%% *}" + rest="${line#* }" + echo "$sha1" > "$DOTEST"/stopped-sha + warn "Stopped at $sha1... $rest" + if test -n "$comment"; then + warn + warn " $comment" + fi + warn + warn "Once you are ready to continue, run" + warn + warn " git rebase --continue" + warn + exit 0 + ;; *) warn "Unknown command: $command $sha1 $rest" if git rev-parse --verify -q "$sha1" >/dev/null @@ -998,6 +1018,7 @@ first and then run 'git rebase --continue' again." # s, squash = use commit, but meld into previous commit # f, fixup = like "squash", but discard this commit's log message # x , exec = Run a shell command , and stop if it fails +# pause = exit to the shell # # If you remove a line here THAT COMMIT WILL BE LOST. # However, if you remove everything, the rebase will be aborted. -- 1.7.3.2.195.gc69dde ``` ## Ævar Arnfjörð Bjarmason, 2010-11-04 09:23 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: URL: https://gitlist.dev/e/AANLkTimzTzUvoHT9bHve-qvt8V_mvJHmQtgpqY6f_H3u%40mail.gmail.com In-Reply-To: <914D7AE3-22D5-4069-B815-2B11A2897BE9@sb.org> ``` On Thu, Nov 4, 2010 at 09:53, Kevin Ballard wrote: > On Nov 4, 2010, at 1:42 AM, Matthieu Moy wrote: > >> Kevin Ballard writes: >> >>> Add a new command "shell", which takes an option commit. It simply exits >>> to the shell with the commit (if given) and a message telling the user how >>> to resume the rebase. >> >> "shell" sounds like you're going to execute something in a shell, not >> that you're going back to the shell. Looking at the commit message, I >> thought you had missed the "exec" command and re-implemented it. >> >> What about "pause", abbreviated as "p" for the command name? > > That sounds like a reasonable suggestion, except "p" is already taken by "pick". > I suppose this command could simply omit the short version. I thought "shell" would do exactly what your patch does. And it has the "s" short version. So +1 for "shell" from me and -1 for "pause", which *does* confuse me. I'd expect that to just sleep for a few seconds. ``` ## Kevin Ballard, 2010-11-04 09:25 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: URL: https://gitlist.dev/e/EE792829-6A68-44FB-8C8D-2365DB4E5A5D%40sb.org In-Reply-To: ``` On Nov 4, 2010, at 2:23 AM, Ævar Arnfjörð Bjarmason wrote: > On Thu, Nov 4, 2010 at 09:53, Kevin Ballard wrote: >> On Nov 4, 2010, at 1:42 AM, Matthieu Moy wrote: >> >>> Kevin Ballard writes: >>> >>>> Add a new command "shell", which takes an option commit. It simply exits >>>> to the shell with the commit (if given) and a message telling the user how >>>> to resume the rebase. >>> >>> "shell" sounds like you're going to execute something in a shell, not >>> that you're going back to the shell. Looking at the commit message, I >>> thought you had missed the "exec" command and re-implemented it. >>> >>> What about "pause", abbreviated as "p" for the command name? >> >> That sounds like a reasonable suggestion, except "p" is already taken by "pick". >> I suppose this command could simply omit the short version. > > I thought "shell" would do exactly what your patch does. And it has > the "s" short version. > > So +1 for "shell" from me and -1 for "pause", which *does* confuse me. > I'd expect that > to just sleep for a few seconds. "s" is actually taken by "squash". That's why my original patch used "!", though a user might actually expect "!" to do what "x" does. -Kevin Ballard ``` ## Ævar Arnfjörð Bjarmason, 2010-11-04 09:27 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: URL: https://gitlist.dev/e/AANLkTinmPF-Q9hy%2Bs5qe_66hLaF%3DmsTh_cFc5uZZQxs-%40mail.gmail.com In-Reply-To: ``` On Thu, Nov 4, 2010 at 10:25, Kevin Ballard wrote: >> I thought "shell" would do exactly what your patch does. And it has >> the "s" short version. >> >> So +1 for "shell" from me and -1 for "pause", which *does* confuse me. >> I'd expect that >> to just sleep for a few seconds. > > "s" is actually taken by "squash". That's why my original patch used "!", > though a user might actually expect "!" to do what "x" does. Indeed, eek! ``` ## Erik Faye-Lund, 2010-11-04 09:36 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: URL: https://gitlist.dev/e/AANLkTin7d-RJcy4CHmd5A6LaiphAvHEdbsxJExHt317_%40mail.gmail.com In-Reply-To: <1288847836-84882-1-git-send-email-kevin@sb.org> ``` On Thu, Nov 4, 2010 at 6:17 AM, Kevin Ballard wrote: > Add a new command "shell", which takes an option commit. It simply exits > to the shell with the commit (if given) and a message telling the user how > to resume the rebase. This is effectively the same thing as "x false" but > much friendlier to the user. > I'm sorry if I'm missing something, but how is this different from "edit"? ``` ## Kevin Ballard, 2010-11-04 09:43 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <9C0BAFB4-299E-459B-A64A-54D480C5445D@sb.org> URL: https://gitlist.dev/e/9C0BAFB4-299E-459B-A64A-54D480C5445D%40sb.org In-Reply-To: ``` On Nov 4, 2010, at 2:36 AM, Erik Faye-Lund wrote: > On Thu, Nov 4, 2010 at 6:17 AM, Kevin Ballard wrote: >> Add a new command "shell", which takes an option commit. It simply exits >> to the shell with the commit (if given) and a message telling the user how >> to resume the rebase. This is effectively the same thing as "x false" but >> much friendlier to the user. >> > > I'm sorry if I'm missing something, but how is this different from "edit"? Edit cherry-picks a commit, then exits to the shell. I needed to exit to the shell without cherry-picking a commit. As stated in the comments above the diffstat on the patch, the original use case here was something along the lines of edit 12345 some commit fixup 23456 another commit shell I want to amend the commit after the fixup -Kevin Ballard ``` ## Johannes Sixt, 2010-11-04 10:24 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <4CD289EA.7050800@viscovery.net> URL: https://gitlist.dev/e/4CD289EA.7050800%40viscovery.net In-Reply-To: <914D7AE3-22D5-4069-B815-2B11A2897BE9@sb.org> ``` Am 11/4/2010 9:53, schrieb Kevin Ballard: > +# pause = exit to the shell The short form could be just the dash -. I'd describe the command as # pause,- = interrupt automatic processing of commits or similar to avoid the term "shell". -- Hannes ``` ## Yann Dirson, 2010-11-04 10:25 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <20101104112530.5c0e444a@chalon.bertin.fr> URL: https://gitlist.dev/e/20101104112530.5c0e444a%40chalon.bertin.fr In-Reply-To: <9C0BAFB4-299E-459B-A64A-54D480C5445D@sb.org> ``` >> I'm sorry if I'm missing something, but how is this different from >> "edit"? > >Edit cherry-picks a commit, then exits to the shell. I needed to exit >to the shell without cherry-picking a commit. Indeed, before "x false" was available, I had found out that "edit" without an argument fails with a harmless error and indeed achieves that "pause" mechanism which was really missing. What about just fixing this so we can use "edit" ? Do we really need another command here ? -- Yann Dirson - Bertin Technologies ``` ## Erik Faye-Lund, 2010-11-04 10:40 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: URL: https://gitlist.dev/e/AANLkTikbrab3kmDqTCLo_tPeZWm_c-5Yux0FnVmwQE85%40mail.gmail.com In-Reply-To: <20101104112530.5c0e444a@chalon.bertin.fr> ``` On Thu, Nov 4, 2010 at 11:25 AM, Yann Dirson wrote: >>> I'm sorry if I'm missing something, but how is this different from >>> "edit"? >> >>Edit cherry-picks a commit, then exits to the shell. I needed to exit >>to the shell without cherry-picking a commit. > Then you do "edit" on the preceding commit instead, no? > Indeed, before "x false" was available, I had found out that "edit" > without an argument fails with a harmless error and indeed achieves that > "pause" mechanism which was really missing. > > What about just fixing this so we can use "edit" ?  Do we really need > another command here ? > Having an parameter-less "edit" would indeed be a bit more convenient. ``` ## Eric Raible, 2010-11-04 17:04 Subject: Re: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <4CD2E7B4.3000908@nextest.com> URL: https://gitlist.dev/e/4CD2E7B4.3000908%40nextest.com In-Reply-To: <20101104112530.5c0e444a@chalon.bertin.fr> ``` On 11:59 AM, Yann Dirson wrote: >>> I'm sorry if I'm missing something, but how is this different from >>> "edit"? >> >> Edit cherry-picks a commit, then exits to the shell. I needed to exit >> to the shell without cherry-picking a commit. > > Indeed, before "x false" was available, I had found out that "edit" > without an argument fails with a harmless error and indeed achieves that > "pause" mechanism which was really missing. > > What about just fixing this so we can use "edit" ? Do we really need > another command here ? FWIW: +1 for edit. ``` ## Matthieu Moy, 2010-11-04 17:34 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: URL: https://gitlist.dev/e/vpq62wddmc0.fsf%40bauges.imag.fr In-Reply-To: <4CD2E7B4.3000908@nextest.com> ``` Eric Raible writes: > On 11:59 AM, Yann Dirson wrote: >>>> I'm sorry if I'm missing something, but how is this different from >>>> "edit"? >>> >>> Edit cherry-picks a commit, then exits to the shell. I needed to exit >>> to the shell without cherry-picking a commit. >> >> Indeed, before "x false" was available, I had found out that "edit" >> without an argument fails with a harmless error and indeed achieves that >> "pause" mechanism which was really missing. >> >> What about just fixing this so we can use "edit" ? Do we really need >> another command here ? > > FWIW: +1 for edit. I like the idea (and I won't fight for my "pause" proposal if others don't find it intuitive), but I'm wondering how to write the quick documentation (in the todo-list). And if we don't find a concise way to document it, it may reveal that it's a bad idea ... Maybe: # e , edit = use commit, but stop for amending # e, edit = stop for amending but I find this rather ugly. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ``` ## Eric Raible, 2010-11-04 17:43 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <4CD2F0B0.5060501@nextest.com> URL: https://gitlist.dev/e/4CD2F0B0.5060501%40nextest.com In-Reply-To: ``` On 11/4/2010 10:34 AM, Matthieu Moy wrote: > ... And if we don't find a concise way > to document it, it may reveal that it's a bad idea ... > > Maybe: > > # e , edit = use commit, but stop for amending > # e, edit = stop for amending > > but I find this rather ugly. How about: # e [], edit [] = use commit (if present) but pause to amend ``` ## Jonathan Nieder, 2010-11-04 18:10 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <20101104181020.GB16431@burratino> URL: https://gitlist.dev/e/20101104181020.GB16431%40burratino In-Reply-To: ``` Matthieu Moy wrote: > # e , edit = use commit, but stop for amending > # e, edit = stop for amending Before it said: # Commands: # p, pick = use commit # r, reword = use commit, but edit the commit message # e, edit = use commit, but stop for amending # s, squash = use commit, but meld into previous commit # f, fixup = like "squash", but discard this commit's log message # x , exec = Run a shell command , and stop if it fails # # If you remove a line here THAT COMMIT WILL BE LOST. # However, if you remove everything, the rebase will be aborted. How about: # Commands: # p, pick = use commit # r, reword = use commit, but edit the commit message # e, edit = use commit, but stop for amending # s, squash = use commit, but meld into previous commit # f, fixup = like "squash", but discard this commit's log message # x, exec = run command using shell, and stop if it fails # # The argument to edit is optional; if left out, it means to # stop to examine or amend the previous commit. # # If you remove a line here, THAT COMMIT WILL BE LOST. # However, if you remove everything, the rebase will be aborted. # Use the noop command if you really want to remove all commits. Ciao, Jonathan who is happy to help paint today ``` ## Yann Dirson, 2010-11-04 20:53 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <20101104205307.GA8911@home.lan> URL: https://gitlist.dev/e/20101104205307.GA8911%40home.lan In-Reply-To: <20101104181020.GB16431@burratino> ``` On Thu, Nov 04, 2010 at 01:10:20PM -0500, Jonathan Nieder wrote: > How about: > > # Commands: > # p, pick = use commit > # r, reword = use commit, but edit the commit message > # e, edit = use commit, but stop for amending > # s, squash = use commit, but meld into previous commit > # f, fixup = like "squash", but discard this commit's log message > # x, exec = run command using shell, and stop if it fails > # > # The argument to edit is optional; if left out, it means to > # stop to examine or amend the previous commit. > # > # If you remove a line here, THAT COMMIT WILL BE LOST. > # However, if you remove everything, the rebase will be aborted. > # Use the noop command if you really want to remove all commits. That may be too far from the "edit" line, although I do like the idea of mentionning other uses than "amend". Eric Raible suggested: > How about: > > # e [], edit [] = use commit (if present) but pause to amend Other commands do not mention commit (or other things) as a synopsis would. What about: # e, edit = use commit (if specified) but pause to amend/examine/test ``` ## Eric Raible, 2010-11-04 21:05 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <4CD32034.4030104@nextest.com> URL: https://gitlist.dev/e/4CD32034.4030104%40nextest.com In-Reply-To: <20101104205307.GA8911@home.lan> ``` On 11/4/2010 1:53 PM, Yann Dirson wrote: > Eric Raible suggested: >> How about: >> >> # e [], edit [] = use commit (if present) but pause to amend > > Other commands do not mention commit (or other things) as a synopsis would. > What about: > > # e, edit = use commit (if specified) but pause to amend/examine/test > . I like that color better. ``` ## Kevin Ballard, 2010-11-04 21:33 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <3B610A5B-DE74-4DB1-A61D-13AAF167E36C@sb.org> URL: https://gitlist.dev/e/3B610A5B-DE74-4DB1-A61D-13AAF167E36C%40sb.org In-Reply-To: <20101104205307.GA8911@home.lan> ``` On Nov 4, 2010, at 1:53 PM, Yann Dirson wrote: > Eric Raible suggested: >> How about: >> >> # e [], edit [] = use commit (if present) but pause to amend > > Other commands do not mention commit (or other things) as a synopsis would. > What about: > > # e, edit = use commit (if specified) but pause to amend/examine/test I like this. My only remaining concern is the original "shell" version let you put in a comment (though this was not yet documented) that would be printed when you were sent back to the shell. This was a useful reminder as to what step you were on. But when we overload "edit", this functionality is lost. I won't fight for it if nobody else here thinks it's worthwhile, but I did want to point that out. -Kevin Ballard ``` ## Kevin Ballard, 2010-11-04 22:01 Subject: [PATCHv2] git-rebase--interactive.sh: extend "edit" command to be more useful Message-ID: <1288908086-91520-1-git-send-email-kevin@sb.org> URL: https://gitlist.dev/e/1288908086-91520-1-git-send-email-kevin%40sb.org In-Reply-To: <4CD32034.4030104@nextest.com> ``` Extend the "edit" command to simply stop for editing if no sha1 is given. This behaves the same as "x false" but is a bit friendlier for the user. Signed-off-by: Kevin Ballard --- git-rebase--interactive.sh | 19 ++++++++++++++----- 1 files changed, 14 insertions(+), 5 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 9121bb6..a8e00a2 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -477,10 +477,19 @@ do_next () { comment_for_reflog edit mark_action_done - pick_one $sha1 || - die_with_patch $sha1 "Could not apply $sha1... $rest" - echo "$sha1" > "$DOTEST"/stopped-sha - make_patch $sha1 + if test -n "$sha1"; then + pick_one $sha1 || + die_with_patch $sha1 "Could not apply $sha1... $rest" + echo "$sha1" > "$DOTEST"/stopped-sha + make_patch $sha1 + else + # we just want to exit to the shell + # we don't have a $sha1 or $rest, so recreate that + line=$(git rev-list --pretty=oneline -1 --abbrev-commit --abbrev=7 HEAD) + sha1="${line%% *}" + rest="${line#* }" + echo "$sha1" > "$DOTEST"/stopped-sha + fi git rev-parse --verify HEAD > "$AMEND" warn "Stopped at $sha1... $rest" warn "You can amend the commit now, with" @@ -1003,7 +1012,7 @@ first and then run 'git rebase --continue' again." # Commands: # p, pick = use commit # r, reword = use commit, but edit the commit message -# e, edit = use commit, but stop for amending +# e, edit = use commit (if specified), but pause to amend/examine/test # s, squash = use commit, but meld into previous commit # f, fixup = like "squash", but discard this commit's log message # x , exec = Run a shell command , and stop if it fails -- 1.7.3.2.203.gd142e ``` ## Johannes Sixt, 2010-11-05 07:33 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <4CD3B35B.3010404@viscovery.net> URL: https://gitlist.dev/e/4CD3B35B.3010404%40viscovery.net In-Reply-To: <20101104205307.GA8911@home.lan> ``` Am 11/4/2010 21:53, schrieb Yann Dirson: > # e, edit = use commit (if specified) but pause to amend/examine/test That's fine. But how would you determine the "if specified"? In particular, I like to replace the commit subject by instructions that remember me what I intended to do after rebase stopped, and I would like to do that in either of these two forms: e merge foo-topic! or e - merge foo-topic! -- Hannes ``` ## Kevin Ballard, 2010-11-05 08:39 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <9290474C-CD01-4C28-8B3B-3A577D569FC7@sb.org> URL: https://gitlist.dev/e/9290474C-CD01-4C28-8B3B-3A577D569FC7%40sb.org In-Reply-To: <4CD3B35B.3010404@viscovery.net> ``` On Nov 5, 2010, at 12:33 AM, Johannes Sixt wrote: > Am 11/4/2010 21:53, schrieb Yann Dirson: >> # e, edit = use commit (if specified) but pause to amend/examine/test > > That's fine. But how would you determine the "if specified"? In > particular, I like to replace the commit subject by instructions that > remember me what I intended to do after rebase stopped, and I would like > to do that in either of these two forms: > > e merge foo-topic! > > or > > e - merge foo-topic! This was my complaint about overriding "edit" as well, but I kind of like your second example. Can you come up with a simple way to explain it in the instructions? -Kevin Ballard ``` ## Junio C Hamano, 2010-11-08 18:31 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <7vd3qfr7ki.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd3qfr7ki.fsf%40alter.siamese.dyndns.org In-Reply-To: <20101104205307.GA8911@home.lan> ``` Yann Dirson writes: > # e, edit = use commit (if specified) but pause to amend/examine/test When an end user is given pick one pick two pick three ... and told the above, would it be crystal clear that, if he changed the insn sheet to pick one edit pick three ... then he will _lose_ the change made by foo, or will the user come back here and complain that a precious change "two" is lost and it is git's fault? ``` ## Kevin Ballard, 2010-11-08 21:49 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <663A3F43-5F64-41F0-B272-64EEE9775250@sb.org> URL: https://gitlist.dev/e/663A3F43-5F64-41F0-B272-64EEE9775250%40sb.org In-Reply-To: <7vd3qfr7ki.fsf@alter.siamese.dyndns.org> ``` On Nov 8, 2010, at 10:31 AM, Junio C Hamano wrote: > Yann Dirson writes: > >> # e, edit = use commit (if specified) but pause to amend/examine/test > > When an end user is given > > pick one > pick two > pick three > ... > > and told the above, would it be crystal clear that, if he changed the insn > sheet to > > pick one > edit > pick three > ... > > then he will _lose_ the change made by foo, or will the user come back > here and complain that a precious change "two" is lost and it is git's > fault? On the one hand, once someone understands what the todo list is actually doing, then it should be instantly obvious that removing the reference to a commit will remove that commit entirely. On the other hand, I agree it may be confusing to new git users (or new rebase users). Do you have an alternative solution in mind? -Kevin Ballard ``` ## Yann Dirson, 2010-11-08 22:29 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <20101108222937.GH3167@home.lan> URL: https://gitlist.dev/e/20101108222937.GH3167%40home.lan In-Reply-To: <663A3F43-5F64-41F0-B272-64EEE9775250@sb.org> ``` On Mon, Nov 08, 2010 at 01:49:44PM -0800, Kevin Ballard wrote: > On Nov 8, 2010, at 10:31 AM, Junio C Hamano wrote: > > > Yann Dirson writes: > > > >> # e, edit = use commit (if specified) but pause to amend/examine/test > > > > When an end user is given > > > > pick one > > pick two > > pick three > > ... > > > > and told the above, would it be crystal clear that, if he changed the insn > > sheet to > > > > pick one > > edit > > pick three > > ... > > > > then he will _lose_ the change made by foo, or will the user come back > > here and complain that a precious change "two" is lost and it is git's > > fault? > > On the one hand, once someone understands what the todo list is actually > doing, then it should be instantly obvious that removing the reference to > a commit will remove that commit entirely. On the other hand, I agree it > may be confusing to new git users (or new rebase users). Do you have an > alternative solution in mind? Maybe restating in an explanatory paragraph something like: |Keep in mind that any commit in the original todo list, that would |not be there after your edits, would not be included in the resulting |rebased branch. In case you realize afterwards that you need such a |commit, you can still access it as an ancestor of @{1}, see |git-reflog(1) for details. Maybe we could list a copy of the todo list in the comments, as a reference for double-checking. Such a list could even be used for a final check before applying, that would ask confirmation if the set of patches has changed, and offer to edit again. The same config item (eg. advice.interactiveRebase ?) could be used to hide the note and the check. Now making "rebase -i" possibly interactive may cause problems, for any porcelain scripts above it. Not sure it'd be the way to do it. Maybe add a "check" command to be inserted at bottom of todo list to activate it, that would be here by default but commented out ? ``` ## Jonathan Nieder, 2010-11-10 01:42 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <20101110014215.GA1503@burratino> URL: https://gitlist.dev/e/20101110014215.GA1503%40burratino In-Reply-To: <20101108222937.GH3167@home.lan> ``` Yann Dirson wrote: > |Keep in mind that any commit in the original todo list, that would > |not be there after your edits, would not be included in the resulting > |rebased branch. In case you realize afterwards that you need such a > |commit, you can still access it as an ancestor of @{1}, see > |git-reflog(1) for details. Do you mean @{-1}? > Maybe we could list a copy of the todo list in the comments, as a > reference for double-checking. Such a list could even be used for a > final check before applying, that would ask confirmation if the set of > patches has changed, and offer to edit again. The same config item > (eg. advice.interactiveRebase ?) could be used to hide the note and > the check. Mm, but intentionally dropping commits is common, no? What would be nice is to be able to do git rebase --change-of-plans and somehow get my editor of choice to open with the original todo list (read-only) and the current todo list (read/write). Well, a person can dream. :) ``` ## Kevin Ballard, 2010-11-10 01:46 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <833D47AD-041C-47BF-9AF3-69FD97F42712@sb.org> URL: https://gitlist.dev/e/833D47AD-041C-47BF-9AF3-69FD97F42712%40sb.org In-Reply-To: <20101110014215.GA1503@burratino> ``` On Nov 9, 2010, at 5:42 PM, Jonathan Nieder wrote: > Yann Dirson wrote: > >> |Keep in mind that any commit in the original todo list, that would >> |not be there after your edits, would not be included in the resulting >> |rebased branch. In case you realize afterwards that you need such a >> |commit, you can still access it as an ancestor of @{1}, see >> |git-reflog(1) for details. > > Do you mean @{-1}? @{-1} is the previously-checked-out branch. @{1} is the previous commit that the current branch was pointing to. I believe @{1} is correct here. >> Maybe we could list a copy of the todo list in the comments, as a >> reference for double-checking. Such a list could even be used for a >> final check before applying, that would ask confirmation if the set of >> patches has changed, and offer to edit again. The same config item >> (eg. advice.interactiveRebase ?) could be used to hide the note and >> the check. > > Mm, but intentionally dropping commits is common, no? > > What would be nice is to be able to do > > git rebase --change-of-plans > > and somehow get my editor of choice to open with the original todo > list (read-only) and the current todo list (read/write). > > Well, a person can dream. :) Not a bad idea. It would be especially nice if you could then selectively roll back to the state after previous entries in your todo list so you could change something you've done without having to start all over again. -Kevin Ballard ``` ## Jonathan Nieder, 2010-11-10 01:53 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <20101110015327.GB1503@burratino> URL: https://gitlist.dev/e/20101110015327.GB1503%40burratino In-Reply-To: <7vd3qfr7ki.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano wrote: > Yann Dirson writes: >> # e, edit = use commit (if specified) but pause to amend/examine/test [...] > would it be crystal clear that, if he changed the insn > sheet to > > pick one > edit > pick three > ... > > then he will _lose_ the change made by foo, or will the user come back > here and complain that a precious change "two" is lost and it is git's > fault? If we explain it clearly then I think yes, the end user would not be confused. The above description (that starts with "e, edit") looks more like a reminder than a full explanation. Can we rely on the perplexed operator to read the text after the command list? If so, some trailing explanation[1] might help. # Commands: # p, pick = use commit # r, reword = use commit, but edit the commit message # e, edit = use commit (if specified), but stop to amend/examine/test # s, squash = use commit, but meld into previous commit # f, fixup = like "squash", but discard this commit's log message # x, exec = run command using shell, and stop if it fails # # The argument to edit is optional; if left out or equal to "-", # it means to stop to examine or amend the previous commit. # # If you remove a line here, THAT COMMIT WILL BE LOST. # However, if you remove everything, the rebase will be aborted. # Use the noop command if you really want to remove all commits. [1] http://thread.gmane.org/gmane.comp.version-control.git/160691/focus=160742 ``` ## Jonathan Nieder, 2010-11-10 01:56 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <20101110015623.GC1503@burratino> URL: https://gitlist.dev/e/20101110015623.GC1503%40burratino In-Reply-To: <833D47AD-041C-47BF-9AF3-69FD97F42712@sb.org> ``` Kevin Ballard wrote: > On Nov 9, 2010, at 5:42 PM, Jonathan Nieder wrote: >> Yann Dirson wrote: >>> |Keep in mind that any commit in the original todo list, that would >>> |not be there after your edits, would not be included in the resulting >>> |rebased branch. In case you realize afterwards that you need such a >>> |commit, you can still access it as an ancestor of @{1}, see >>> |git-reflog(1) for details. >> >> Do you mean @{-1}? > > @{-1} is the previously-checked-out branch. @{1} is the previous commit > that the current branch was pointing to. I believe @{1} is correct here. Ah, this is after a successful rebase, so @{1} is a synonym for ORIG_HEAD. Sorry for the noise. ``` ## Kevin Ballard, 2010-11-10 02:14 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <6F2D0BEA-187E-4683-826C-D8582AC16D8F@sb.org> URL: https://gitlist.dev/e/6F2D0BEA-187E-4683-826C-D8582AC16D8F%40sb.org In-Reply-To: <20101110015327.GB1503@burratino> ``` On Nov 9, 2010, at 5:53 PM, Jonathan Nieder wrote: > Junio C Hamano wrote: >> Yann Dirson writes: > >>> # e, edit = use commit (if specified) but pause to amend/examine/test > [...] >> would it be crystal clear that, if he changed the insn >> sheet to >> >> pick one >> edit >> pick three >> ... >> >> then he will _lose_ the change made by foo, or will the user come back >> here and complain that a precious change "two" is lost and it is git's >> fault? > > If we explain it clearly then I think yes, the end user would not > be confused. > > The above description (that starts with "e, edit") looks more like a > reminder than a full explanation. Can we rely on the perplexed > operator to read the text after the command list? > > If so, some trailing explanation[1] might help. > > # Commands: > # p, pick = use commit > # r, reword = use commit, but edit the commit message > # e, edit = use commit (if specified), but stop to amend/examine/test > # s, squash = use commit, but meld into previous commit > # f, fixup = like "squash", but discard this commit's log message > # x, exec = run command using shell, and stop if it fails > # > # The argument to edit is optional; if left out or equal to "-", > # it means to stop to examine or amend the previous commit. > # > # If you remove a line here, THAT COMMIT WILL BE LOST. > # However, if you remove everything, the rebase will be aborted. > # Use the noop command if you really want to remove all commits. I like it. Especially because if we support "-" in place of a sha1, then we can treat the rest of the line like a comment and display it when stopped, as the old "shell" version did. -Kevin Ballard ``` ## Yann Dirson, 2010-11-10 07:43 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: <20101110084343.0c519764@chalon.bertin.fr> URL: https://gitlist.dev/e/20101110084343.0c519764%40chalon.bertin.fr In-Reply-To: <20101110014215.GA1503@burratino> ``` On Tue, 09 Nov 2010 19:42:15 -0600 Jonathan Nieder wrote: > Yann Dirson wrote: > > > |Keep in mind that any commit in the original todo list, that would > > |not be there after your edits, would not be included in the > > resulting |rebased branch. In case you realize afterwards that you > > need such a |commit, you can still access it as an ancestor of > > @{1}, see |git-reflog(1) for details. > > Do you mean @{-1}? > > > Maybe we could list a copy of the todo list in the comments, as a > > reference for double-checking. Such a list could even be used for a > > final check before applying, that would ask confirmation if the set > > of patches has changed, and offer to edit again. The same config > > item (eg. advice.interactiveRebase ?) could be used to hide the > > note and the check. > > Mm, but intentionally dropping commits is common, no? Yes, but for people new to the feature, who may not feel at ease right away with it, it may make sense to get warned when some change will get lost. BTW, about people feeling at ease with "rebase -i", I often feel not quite comfortable to explain why to reorder commits you have to use this "rebase" feature which sounds so strange in itself to people used to centralized VCS. Would that make sense to have a standard command to reduce some confusion, like (untested): alias.reroll = rebase -i $(git merge-base HEAD @{upstream}) > What would be nice is to be able to do > > git rebase --change-of-plans > > and somehow get my editor of choice to open with the original todo > list (read-only) and the current todo list (read/write). > > Well, a person can dream. :) Well, that's not far from my own dreams of --back, --next and the like :) -- Yann Dirson - Bertin Technologies ``` ## Matthieu Moy, 2010-11-10 16:00 Subject: Re: [PATCH] git-rebase--interactive.sh: Add new command "shell" Message-ID: URL: https://gitlist.dev/e/vpqtyjpw4m9.fsf%40bauges.imag.fr In-Reply-To: <20101110084343.0c519764@chalon.bertin.fr> ``` Yann Dirson writes: > BTW, about people feeling at ease with "rebase -i", I often feel not > quite comfortable to explain why to reorder commits you have to use > this "rebase" feature I feel a bit the same. Actually, I don't think I ever used "rebase -i" to actually perform a rebase. I usually "git pull --rebase" to rebase, and "rebase -i" to rewrite history without changing the origin of the branch. > alias.reroll = rebase -i $(git merge-base HEAD @{upstream}) Mercurial calls this "histedit" for example. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ``` ## Kevin Ballard, 2010-11-24 20:19 Subject: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful Message-ID: <1290629960-60917-1-git-send-email-kevin@sb.org> URL: https://gitlist.dev/e/1290629960-60917-1-git-send-email-kevin%40sb.org In-Reply-To: <20101110015327.GB1503@burratino> ``` Extend the "edit" command to simply stop for editing if no sha1 is given or if the sha1 is equal to "-". This behaves the same as "x false" but is a bit friendlier for the user. Signed-off-by: Kevin Ballard --- Two changes since the last patch: * Picked up the extended explanation suggested by Jonathan Nieder. I left off the last line about "noop" as that doesn't seem related. * If the line given is "edit - some comments", emit "some comments" when stopped. This is undocumented, so if anyone has any suggestions for how it should be documented I'm all ears. I'm also not sure if it should use the output format I selected now, or if it should just emit the comment in place of the commit summary (e.g. Stopped at $sha1... $comment). git-rebase--interactive.sh | 30 +++++++++++++++++++++++++----- 1 files changed, 25 insertions(+), 5 deletions(-) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 5934b97..176f735 100755 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -469,12 +469,29 @@ do_next () { comment_for_reflog edit mark_action_done - pick_one $sha1 || - die_with_patch $sha1 "Could not apply $sha1... $rest" - echo "$sha1" > "$DOTEST"/stopped-sha - make_patch $sha1 + comment='' + if test -n "$sha1" -a "$sha1" != "-"; then + pick_one $sha1 || + die_with_patch $sha1 "Could not apply $sha1... $rest" + echo "$sha1" > "$DOTEST"/stopped-sha + make_patch $sha1 + else + # we just want to exit to the shell + # we don't have a valid $sha1 or $rest, so recreate that + # save the original $rest to a comment for later + comment="$rest" + line=$(git rev-list --pretty=oneline -1 --abbrev-commit --abbrev=7 HEAD) + sha1="${line%% *}" + rest="${line#* }" + echo "$sha1" > "$DOTEST"/stopped-sha + fi git rev-parse --verify HEAD > "$AMEND" warn "Stopped at $sha1... $rest" + if test -n "$comment"; then + warn + warn " $comment" + warn + fi warn "You can amend the commit now, with" warn warn " git commit --amend" @@ -1016,11 +1033,14 @@ first and then run 'git rebase --continue' again." # Commands: # p, pick = use commit # r, reword = use commit, but edit the commit message -# e, edit = use commit, but stop for amending +# e, edit = use commit (if specified), but stop to amend/examine/test # s, squash = use commit, but meld into previous commit # f, fixup = like "squash", but discard this commit's log message # x , exec = Run a shell command , and stop if it fails # +# The argument to edit is optional; if left out or equal to "-", +# it means to stop to examine or amend the previous commit. +# # If you remove a line here THAT COMMIT WILL BE LOST. # However, if you remove everything, the rebase will be aborted. # -- 1.7.3.2.488.gc5e8 ``` ## Jonathan Nieder, 2010-12-03 08:06 Subject: Re: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful Message-ID: <20101203080603.GC18202@burratino> URL: https://gitlist.dev/e/20101203080603.GC18202%40burratino In-Reply-To: <1290629960-60917-1-git-send-email-kevin@sb.org> ``` Hi, Kevin Ballard wrote: > [Subject: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful Maybe something like rebase-i: treat "edit" without sha1 as a request to amend previous commit would make the meaning more obvious in a shortlog. > Extend the "edit" command to simply stop for editing if no sha1 is > given or if the sha1 is equal to "-". This behaves the same as "x false" > but is a bit friendlier for the user. Nice. I like the semantics. > * Picked up the extended explanation suggested by Jonathan Nieder. > I left off the last line about "noop" as that doesn't seem related. Right, please feel free to remind me if I forget to pick that up again. > * If the line given is "edit - some comments", emit "some comments" when > stopped. This is undocumented I think that's okay for now (though of course it would be best to explain some example uses in Documentation/git-rebase.txt in the form of examples). > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -469,12 +469,29 @@ do_next () { > + comment="$rest" > + line=$(git rev-list --pretty=oneline -1 --abbrev-commit --abbrev=7 HEAD) Hmm, the script seems to assume rev-list will not fail throughout. :/ Ok. > + sha1="${line%% *}" > + rest="${line#* }" > + echo "$sha1" > "$DOTEST"/stopped-sha Maybe this can be done without relying on details of --pretty=oneline format? sha1=$(git rev-parse --short HEAD) rest=$(git show -s --format=%s HEAD) (Yes, elsewhere the script uses git rev-list --no-merges --pretty=oneline --abbrev-commit \ --abbrev=7 --reverse --left-right --topo-order "$@" | sed -n "s/^>//p" | while read -r shortsha1 rest but in that loop, avoiding an extra exec seems more important.) > + fi > git rev-parse --verify HEAD > "$AMEND" > warn "Stopped at $sha1... $rest" > + if test -n "$comment"; then > + warn > + warn " $comment" > + warn Thanks, looks good to me. Ideas for tests? (see t3404 for inspiration) ``` ## Kevin Ballard, 2010-12-03 08:16 Subject: Re: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful Message-ID: <048EACFB-2038-4D49-B6C3-7E7354F62171@sb.org> URL: https://gitlist.dev/e/048EACFB-2038-4D49-B6C3-7E7354F62171%40sb.org In-Reply-To: <20101203080603.GC18202@burratino> ``` On Dec 3, 2010, at 12:06 AM, Jonathan Nieder wrote: > Hi, > > Kevin Ballard wrote: > >> [Subject: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful > > Maybe something like > > rebase-i: treat "edit" without sha1 as a request to amend previous commit > > would make the meaning more obvious in a shortlog. That seems a bit misleading, though. This command really has nothing to do with amending the previous commit. You can do anything you want once you break back to the shell. I personally used it to run git-merge at that point in the history. For this reason I'm a bit uneasy about overloading "edit", but it does have the benefit that people already know "edit" brings them to the shell. >> Extend the "edit" command to simply stop for editing if no sha1 is >> given or if the sha1 is equal to "-". This behaves the same as "x false" >> but is a bit friendlier for the user. > > Nice. I like the semantics. > >> * Picked up the extended explanation suggested by Jonathan Nieder. >> I left off the last line about "noop" as that doesn't seem related. > > Right, please feel free to remind me if I forget to pick that up again. > >> * If the line given is "edit - some comments", emit "some comments" when >> stopped. This is undocumented > > I think that's okay for now (though of course it would be best to explain > some example uses in Documentation/git-rebase.txt in the form of examples). Yep, I definitely need to add documentation. >> --- a/git-rebase--interactive.sh >> +++ b/git-rebase--interactive.sh >> @@ -469,12 +469,29 @@ do_next () { >> + comment="$rest" >> + line=$(git rev-list --pretty=oneline -1 --abbrev-commit --abbrev=7 HEAD) > > Hmm, the script seems to assume rev-list will not fail throughout. :/ > Ok. > >> + sha1="${line%% *}" >> + rest="${line#* }" >> + echo "$sha1" > "$DOTEST"/stopped-sha > > Maybe this can be done without relying on details of --pretty=oneline > format? > > sha1=$(git rev-parse --short HEAD) > rest=$(git show -s --format=%s HEAD) Does this not similarly assume that rev-parse and show will not fail? Or was the above comment only meant to point out this potential issue without suggesting that it needed to be fixed? > (Yes, elsewhere the script uses > > git rev-list --no-merges --pretty=oneline --abbrev-commit \ > --abbrev=7 --reverse --left-right --topo-order "$@" | > sed -n "s/^>//p" | > while read -r shortsha1 rest > > but in that loop, avoiding an extra exec seems more important.) > >> + fi >> git rev-parse --verify HEAD > "$AMEND" >> warn "Stopped at $sha1... $rest" >> + if test -n "$comment"; then >> + warn >> + warn " $comment" >> + warn > > Thanks, looks good to me. > > Ideas for tests? (see t3404 for inspiration) I'll look into that. I wasn't really sure how to test this before, but t3404 does have some examples of testing the edit command already. -Kevin Ballard ``` ## Jonathan Nieder, 2010-12-03 08:55 Subject: Re: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful Message-ID: <20101203085528.GE18202@burratino> URL: https://gitlist.dev/e/20101203085528.GE18202%40burratino In-Reply-To: <048EACFB-2038-4D49-B6C3-7E7354F62171@sb.org> ``` Kevin Ballard wrote: > On Dec 3, 2010, at 12:06 AM, Jonathan Nieder wrote: >> Maybe something like >> >> rebase-i: treat "edit" without sha1 as a request to amend previous commit >> >> would make the meaning more obvious in a shortlog. > > That seems a bit misleading, though. This command really has nothing to do with > amending the previous commit. Okay, maybe rebase-i: extend "edit" to allow stopping without a commit to amend Or something else entirely; I only meant that "to be more useful" is a bit vague (it could be cut out without loss of meaning). >> Maybe this can be done without relying on details of --pretty=oneline >> format? >> >> sha1=$(git rev-parse --short HEAD) >> rest=$(git show -s --format=%s HEAD) > > Does this not similarly assume that rev-parse and show will not fail? Or was > the above comment only meant to point out this potential issue without > suggesting that it needed to be fixed? Yes, that's right. The exit status from rev-list is ignored throughout the script; making that more robust is a separate topic. BTW this suggestion about avoiding --pretty=oneline was nonsense --- the output format from git rev-list --pretty=oneline is guaranteed to stay the same because rev-list is plumbing. Sorry for the noise. Good night, Jonathan ``` ## Johannes Sixt, 2010-12-03 09:55 Subject: Re: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful Message-ID: <4CF8BE8E.4090100@viscovery.net> URL: https://gitlist.dev/e/4CF8BE8E.4090100%40viscovery.net In-Reply-To: <20101203080603.GC18202@burratino> ``` Am 12/3/2010 9:06, schrieb Jonathan Nieder: > Kevin Ballard wrote: >> + sha1="${line%% *}" >> + rest="${line#* }" >> + echo "$sha1" > "$DOTEST"/stopped-sha > > Maybe this can be done without relying on details of --pretty=oneline > format? No. This is a matter of the syntax of the recipe file. If the details of --pretty=oneline ever changed, then the way how the boilerplate recipe file is generated would have to be changed accordingly. > > sha1=$(git rev-parse --short HEAD) > rest=$(git show -s --format=%s HEAD) Shouldn't $sha1 be the one given in the recipe rather than current HEAD? But most importantly, since $rest is echoed on the terminal, it MUST be derived from the recipe ($line). Rationale: I replace the commit subject in the recipe by a reminder what I intend to do when the "edit" command stops---I don't care so much what the commit subject is. -- Hannes ``` ## Jonathan Nieder, 2010-12-03 10:00 Subject: Re: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful Message-ID: <20101203100059.GA12043@burratino> URL: https://gitlist.dev/e/20101203100059.GA12043%40burratino In-Reply-To: <4CF8BE8E.4090100@viscovery.net> ``` Johannes Sixt wrote: > Am 12/3/2010 9:06, schrieb Jonathan Nieder: >> Maybe this can be done without relying on details of --pretty=oneline >> format? > > No. This is a matter of the syntax of the recipe file. My suggestion was nonsense for other reasons, too. >> >> sha1=$(git rev-parse --short HEAD) >> rest=$(git show -s --format=%s HEAD) > > Shouldn't $sha1 be the one given in the recipe rather than current HEAD? This code branch is about mentally rewriting pick 87a78c fixup 987ca edit - time to test to pick 87a78c fixup 987ca edit and printing "time to test" as a reminder to the user. > But most importantly, since $rest is echoed on the terminal, it MUST be > derived from the recipe ($line). Rationale: I replace the commit subject > in the recipe by a reminder what I intend to do when the "edit" command > stops---I don't care so much what the commit subject is. Kevin, this sounds like a vote for the "replace commit message" output format. Thanks, that was useful. Jonathan ``` ## Kevin Ballard, 2010-12-03 10:14 Subject: Re: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful Message-ID: <85DF30E1-E823-41D9-BAD7-4A11BD0D03C7@sb.org> URL: https://gitlist.dev/e/85DF30E1-E823-41D9-BAD7-4A11BD0D03C7%40sb.org In-Reply-To: <20101203100059.GA12043@burratino> ``` On Dec 3, 2010, at 2:00 AM, Jonathan Nieder wrote: >> But most importantly, since $rest is echoed on the terminal, it MUST be >> derived from the recipe ($line). Rationale: I replace the commit subject >> in the recipe by a reminder what I intend to do when the "edit" command >> stops---I don't care so much what the commit subject is. > > Kevin, this sounds like a vote for the "replace commit message" output > format. The v3 patch will emit both a description of the commit it stopped on, as well as the comment. The rationale for extracting the first line of HEAD is for when the user doesn't provide any comment - e.g. they just add "edit". It may be worth doing this only in that case, and if the user did provide a comment, emit it in place of the first line of HEAD. Given the recipe pick bc17bb7 git-rebase--interactive.sh: extend "edit" command to be more useful edit - foo the edit command would print Stopped at bc17bb7... git-rebase--interactive.sh: extend "edit" command to be more useful foo You can amend the commit now... The alternative is to make that same recipe emit Stopped at bc17bb7... foo You can amend the commit now... I'm leaning towards making that change right now, but I'm not certain. Do either of you have a preference? -Kevin Ballard ```