threads / patch / 36104

patchrebase: new option to post edit a squashed or fixed up commit

Subject: [PATCH] rebase: new option to post edit a squashed or fixed up commit

## tl;dr

3 messages between Mar 9, 2014 and Mar 11, 2014. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Nguyễn Thái Ngọc Duy· Mar 9, 2014, 03:12 UTC · lore

After squashing or fixing up, you may want to have a final look at the commit, edit some more if needed or even do some testing. --postedit enables that. This is (to me) a paranoid mode so either I enable it for all squashes and fixups, or none. Hence a new option, not new todo commands that give finer selection.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-rebase.txt | 5 +++++
 git-rebase--interactive.sh   | 5 +++++
 git-rebase.sh                | 7 +++++++
 3 files changed, 17 insertions(+)
Show changes to 3 files +17 −0

Documentation/git-rebase.txt, git-rebase--interactive.sh, git-rebase.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 2a93c64..dacb163 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -432,6 +432,11 @@ recreates the topic branch with fresh commits so it can be remerged
 successfully without needing to "revert the reversion" (see the
 link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for details).
 
+--[no-]postedit::
+	With --interactive, stop after performing the command "squash"
+	or "fixup". This gives the user a chance to look at the final
+	commit and update or test it if needed.
+
 include::merge-strategies.txt[]
 
 NOTES
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a1adae8..42061fc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -571,6 +571,11 @@ do_next () {
 			;;
 		esac
 		record_in_rewritten $sha1
+		if test -n "$postedit"
+		then
+			warn "Stopped at $sha1... $rest"
+			exit_with_patch $sha1 0
+		fi
 		;;
 	x|"exec")
 		read -r command rest < "$todo"
diff --git a/git-rebase.sh b/git-rebase.sh
index 5f6732b..2d4beb7 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -32,6 +32,7 @@ verify             allow pre-rebase hook to run
 rerere-autoupdate  allow rerere to update index with resolved conflicts
 root!              rebase all reachable commits up to the root(s)
 autosquash         move commits that begin with squash!/fixup! under -i
+postedit           stop after squash or fixup commands
 committer-date-is-author-date! passed to 'git am'
 ignore-date!       passed to 'git am'
 whitespace=!       passed to 'git apply'
@@ -264,6 +265,12 @@ do
 	--no-autosquash)
 		autosquash=
 		;;
+	--postedit)
+		postedit=t
+		;;
+	--no-postedit)
+		postedit=
+		;;
 	--fork-point)
 		fork_point=t
 		;;

-- 
1.9.0.40.gaa8c3ea
Junio C Hamano· Mar 10, 2014, 19:47 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] rebase: new option to post edit a squashed or fixed up commit

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 5 quoted lines
> After squashing or fixing up, you may want to have a final look at the
> commit, edit some more if needed or even do some testing. --postedit
> enables that. This is (to me) a paranoid mode so either I enable it
> for all squashes and fixups, or none. Hence a new option, not new todo
> commands that give finer selection.

If we were to adopt Michael's (?) idea of allowing flags to each insn in the insn sheet, would this restriction be easily lifted?

That is, instead of saying "squash", you say "squash --stop" or something.

Show 14 quoted lines
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index a1adae8..42061fc 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -571,6 +571,11 @@ do_next () {
>  			;;
>  		esac
>  		record_in_rewritten $sha1
> +		if test -n "$postedit"
> +		then
> +			warn "Stopped at $sha1... $rest"
> +			exit_with_patch $sha1 0
> +		fi
>  		;;

I would have expected that any new code would stop only at the last squash (or fixup) in a series of squashes, but this appears to stop even at an intermediate squashed result, which will not appear in the final history. Am I misreading the patch (or misunderstanding the intent of the patch)?

Duy Nguyen· Mar 11, 2014, 10:01 UTC · re: Junio C Hamano · lore

Re: [PATCH] rebase: new option to post edit a squashed or fixed up commit

On Tue, Mar 11, 2014 at 2:47 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 13 quoted lines
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> After squashing or fixing up, you may want to have a final look at the
>> commit, edit some more if needed or even do some testing. --postedit
>> enables that. This is (to me) a paranoid mode so either I enable it
>> for all squashes and fixups, or none. Hence a new option, not new todo
>> commands that give finer selection.
>
> If we were to adopt Michael's (?) idea of allowing flags to each
> insn in the insn sheet, would this restriction be easily lifted?
>
> That is, instead of saying "squash", you say "squash --stop" or
> something.

I think I still need something similar, otherwise I would need to s/squash/squash --stop/ after "rebase -i --autosquash". --postedit code could be simplified by generating "squash --stop" though.

Show 20 quoted lines
>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>> index a1adae8..42061fc 100644
>> --- a/git-rebase--interactive.sh
>> +++ b/git-rebase--interactive.sh
>> @@ -571,6 +571,11 @@ do_next () {
>>                       ;;
>>               esac
>>               record_in_rewritten $sha1
>> +             if test -n "$postedit"
>> +             then
>> +                     warn "Stopped at $sha1... $rest"
>> +                     exit_with_patch $sha1 0
>> +             fi
>>               ;;
>
> I would have expected that any new code would stop only at the last
> squash (or fixup) in a series of squashes, but this appears to stop
> even at an intermediate squashed result, which will not appear in
> the final history.  Am I misreading the patch (or misunderstanding
> the intent of the patch)?
Never thought of that case. Yes it should only stop at the last squash/fixup.
-- 
Duy

← back to recent threads