rebase autosquash doesn't recognise a chain of fixups

rebase autosquash doesn't recognise a chain of fixups

[Carlos Martín Nieto]

Hello,

 Say I have the following commits:

    5154127 fixup! fixup! one
    0d130d8 fixup! one
    0869d30 one

because I keep executing `git commit -a --fixup HEAD`. When I want to
squash them all into 0869d30, I do `git rebase -i --autosquash
0869d30^` and I get

    pick 0869d30 one
    fixup 0d130d8 fixup! one
    pick 5154127 fixup! fixup! one

when I was hoping for

    pick 0869d30 one
    fixup 0d130d8 fixup! one
    fixup 5154127 fixup! fixup! one

Changing the options to the latter one works (as in, the patches
apply). Is this expected? Am I just too lazy and should look up the
commit I want to fixup?

   cmn

Re: rebase autosquash doesn't recognise a chain of fixups

[Vincent van Ravesteijn]

On 5-4-2011 17:41, Carlos Martín Nieto wrote:
> Hello,
>
>   Say I have the following commits:
>
>      5154127 fixup! fixup! one
>      0d130d8 fixup! one
>      0869d30 one
>
> because I keep executing `git commit -a --fixup HEAD`.
>
> Am I just too lazy and should look up the commit I want to fixup?

I would use 'git commit --amend -a' instead if I am already sure that I 
want to squash my commit into the previous one.

Vincent

Re: rebase autosquash doesn't recognise a chain of fixups

[Carlos Martín Nieto]

On Tue, Apr 05, 2011 at 05:48:59PM +0200, Vincent van Ravesteijn wrote:
> On 5-4-2011 17:41, Carlos Martín Nieto wrote:
> >Hello,
> >
> >  Say I have the following commits:
> >
> >     5154127 fixup! fixup! one
> >     0d130d8 fixup! one
> >     0869d30 one
> >
> >because I keep executing `git commit -a --fixup HEAD`.
> >
> >Am I just too lazy and should look up the commit I want to fixup?
> 
> I would use 'git commit --amend -a' instead if I am already sure
> that I want to squash my commit into the previous one.

 That does make more sense. I guess I was just too excited about
 discovering fixups :)

   cmn

Re: rebase autosquash doesn't recognise a chain of fixups

[Junio C Hamano]

Carlos Martín Nieto <cmn@elego.de> writes:

>  Say I have the following commits:
>
>     5154127 fixup! fixup! one
>     0d130d8 fixup! one
>     0869d30 one
>
> because I keep executing `git commit -a --fixup HEAD`. When I want to
> squash them all into 0869d30, I do `git rebase -i --autosquash
> 0869d30^` and I get
>
>     pick 0869d30 one
>     fixup 0d130d8 fixup! one
>     pick 5154127 fixup! fixup! one

The way Kevin's d3d7a42 (rebase: better rearranging of fixup!/squash!
lines with --autosquash, 2010-11-04) series works is to match "fixup!"
only with "pick"; a later "fixup!" never matches an earlier "fixup!" but a
"pick" can be matched against more than one "fixup!".

I think one way to make this work is to fix what Pat did in d71b8ba
(commit: --fixup option for use with rebase --autosquash, 2010-11-02).

Perhaps like this, but I'll leave additions of test scripts to t3415 to
others.

-- >8 --
Subject: commit --fixup: do not duplicate "fixup! " at the beginning

The "rebase -i" command can match more than one "fixup!" against a single
"pick" in the right order thanks to the earlier d3d7a42 (rebase: better
rearranging of fixup!/squash! lines with --autosquash, 2010-11-04), but a
"fixup!" entry is never matched with another "fixup!" entry.

When creating a commit marked to fix up an earlier commit with --fixup, we
can mark it to look for the original and fix that one up.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/commit.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 67757e9..b3c4d63 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -668,6 +668,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		ctx.output_encoding = get_commit_output_encoding();
 		format_commit_message(commit, "fixup! %s\n\n",
 				      &sb, &ctx);
+		while (!prefixcmp(sb.buf, "fixup! fixup!"))
+			strbuf_splice(&sb, 0, 7, "", 0);
 		hook_arg1 = "message";
 	} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)

Re: rebase autosquash doesn't recognise a chain of fixups

[Kevin Ballard]

This seems unnecessary. `git commit --fixup HEAD` doesn't really make any sense at all to run when you can say `git commit --amend` instead,

-Kevin

On Apr 5, 2011, at 10:34 AM, Junio C Hamano <gitster@pobox.com> wrote:

> Carlos Martín Nieto <cmn@elego.de> writes:
> 
>> Say I have the following commits:
>> 
>>    5154127 fixup! fixup! one
>>    0d130d8 fixup! one
>>    0869d30 one
>> 
>> because I keep executing `git commit -a --fixup HEAD`. When I want to
>> squash them all into 0869d30, I do `git rebase -i --autosquash
>> 0869d30^` and I get
>> 
>>    pick 0869d30 one
>>    fixup 0d130d8 fixup! one
>>    pick 5154127 fixup! fixup! one
> 
> The way Kevin's d3d7a42 (rebase: better rearranging of fixup!/squash!
> lines with --autosquash, 2010-11-04) series works is to match "fixup!"
> only with "pick"; a later "fixup!" never matches an earlier "fixup!" but a
> "pick" can be matched against more than one "fixup!".
> 
> I think one way to make this work is to fix what Pat did in d71b8ba
> (commit: --fixup option for use with rebase --autosquash, 2010-11-02).
> 
> Perhaps like this, but I'll leave additions of test scripts to t3415 to
> others.
> 
> -- >8 --
> Subject: commit --fixup: do not duplicate "fixup! " at the beginning
> 
> The "rebase -i" command can match more than one "fixup!" against a single
> "pick" in the right order thanks to the earlier d3d7a42 (rebase: better
> rearranging of fixup!/squash! lines with --autosquash, 2010-11-04), but a
> "fixup!" entry is never matched with another "fixup!" entry.
> 
> When creating a commit marked to fix up an earlier commit with --fixup, we
> can mark it to look for the original and fix that one up.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> builtin/commit.c |    2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 67757e9..b3c4d63 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -668,6 +668,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>        ctx.output_encoding = get_commit_output_encoding();
>        format_commit_message(commit, "fixup! %s\n\n",
>                      &sb, &ctx);
> +        while (!prefixcmp(sb.buf, "fixup! fixup!"))
> +            strbuf_splice(&sb, 0, 7, "", 0);
>        hook_arg1 = "message";
>    } else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
>        if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)

Re: rebase autosquash doesn't recognise a chain of fixups

[Junio C Hamano]

Kevin Ballard <kevin@sb.org> writes:

> This seems unnecessary. `git commit --fixup HEAD` doesn't really make any sense at all to run when you can say `git commit --amend` instead,

What about this sequence?

	git commit -m 'foo'
        git commit -m 'bar'
        git commit --fixup HEAD~1
        git commit -m 'baz'
        git commit --fixup HEAD~1

Re: rebase autosquash doesn't recognise a chain of fixups

[Junio C Hamano]

Kevin Ballard <kevin@sb.org> writes:

> On Tue, Apr 5, 2011 at 11:27 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Kevin Ballard <kevin@sb.org> writes:
>>
>> > This seems unnecessary. `git commit --fixup HEAD` doesn't really make any
>> sense at all to run when you can say `git commit --amend` instead,
>>
>> What about this sequence?
>>
>>        git commit -m 'foo'
>>        git commit -m 'bar'
>>        git commit --fixup HEAD~1
>>        git commit -m 'baz'
>>        git commit --fixup HEAD~1
>>
>>
> You raise a good point. I personally think someone who types that deserves
> what he gets, but I have no objection to being intelligent about duplicate
> fixup! prefixes.

Of course an alternative solution is to fix this on the "rebase -i" side,
by building on top of your d3d7a42 (rebase: better rearranging of
fixup!/squash! lines with --autosquash, 2010-11-04).

Or perhaps we would want to do both.