git interactive rebase 'consume' command

git interactive rebase 'consume' command

[Stephen Kelly]

Hi there,

I find the fixup command during an interactive rebase useful.

Sometimes when cleaning up a branch, I end up in a situation like this:

 pick 07bc3c9 Good commit.
 pick 1313a5e Commit to fixup into c2f62a3.
 pick c2f62a3 Another commit.


So, I have to reorder the commits, and change 1313a5e to 'f'. An alternative 
would be to squash 's' c2f62a3 into 1313a5e and clean up the commit message. 
The problem with that is it ends up with the wrong author time information.

So, I usually reorder and then fixup, but that can also be problematic if I 
get a conflict during the re-order (which is quite likely).

I would prefer to be able to mark a commit as 'should be consumed', so that:

 pick 07bc3c9 Good commit.
 consume 1313a5e Commit to fixup into c2f62a3.
 pick c2f62a3 Another commit.

will result in 

 pick 07bc3c9 Good commit.
 pick 62a3c2f Another commit.

directly.

Any thoughts on that? 

Thanks,

Steve.

Re: git interactive rebase 'consume' command

[John Keeping]

On Sun, Jan 20, 2013 at 03:05:18PM +0100, Stephen Kelly wrote:
> I find the fixup command during an interactive rebase useful.
> 
> Sometimes when cleaning up a branch, I end up in a situation like this:
> 
>  pick 07bc3c9 Good commit.
>  pick 1313a5e Commit to fixup into c2f62a3.
>  pick c2f62a3 Another commit.
> 
> So, I have to reorder the commits, and change 1313a5e to 'f'. An alternative 
> would be to squash 's' c2f62a3 into 1313a5e and clean up the commit message. 
> The problem with that is it ends up with the wrong author time information.
> 
> So, I usually reorder and then fixup, but that can also be problematic if I 
> get a conflict during the re-order (which is quite likely).
> 
> I would prefer to be able to mark a commit as 'should be consumed', so that:
> 
>  pick 07bc3c9 Good commit.
>  consume 1313a5e Commit to fixup into c2f62a3.
>  pick c2f62a3 Another commit.
> 
> will result in 
> 
>  pick 07bc3c9 Good commit.
>  pick 62a3c2f Another commit.
> 
> directly.
> 
> Any thoughts on that? 

Are you aware of the "--autosqush" option to git-rebase (and the
"rebase.autosquash" config setting)?  I find that using that combined
with the "--fixup" option to git-commit makes this workflow a lot more
intuitive.

(Which is not to say that I wouldn't find an option like 'consume'
useful but I find myself reordering the list very rarely since I started
using "git commit --fixup=...".)


John

Re: git interactive rebase 'consume' command

[Stephen Kelly]

John Keeping wrote:
>> Any thoughts on that?
> 
> Are you aware of the "--autosqush" option to git-rebase (and the
> "rebase.autosquash" config setting)?  I find that using that combined
> with the "--fixup" option to git-commit makes this workflow a lot more
> intuitive.

Yes, I'm aware of it, but I think it's not related to the proposal I made. 

Mostly my proposal is about avoiding unnecessary conflict resolution.

Thanks,

Steve.

Re: git interactive rebase 'consume' command

[Junio C Hamano]

Stephen Kelly <steveire@gmail.com> writes:

> Hi there,
>
> I find the fixup command during an interactive rebase useful.
>
> Sometimes when cleaning up a branch, I end up in a situation like this:
>
>  pick 07bc3c9 Good commit.
>  pick 1313a5e Commit to fixup into c2f62a3.
>  pick c2f62a3 Another commit.
>
>
> So, I have to reorder the commits, and change 1313a5e to 'f'. An alternative 
> would be to squash 's' c2f62a3 into 1313a5e and clean up the commit message. 
> The problem with that is it ends up with the wrong author time information.
>
> So, I usually reorder and then fixup, but that can also be problematic if I 
> get a conflict during the re-order (which is quite likely).
>
> I would prefer to be able to mark a commit as 'should be consumed', so that:
>
>  pick 07bc3c9 Good commit.
>  consume 1313a5e Commit to fixup into c2f62a3.
>  pick c2f62a3 Another commit.
>
> will result in 
>
>  pick 07bc3c9 Good commit.
>  pick 62a3c2f Another commit.
>
> directly.
>
> Any thoughts on that? 

Sorry, but I do not understand what you are trying to solve.

How can 1313a5e, which fixes misakes made in c2f62a3, come before
that commit in the first place?

Re: git interactive rebase 'consume' command

[Stephen Kelly]

Junio C Hamano wrote:
> Sorry, but I do not understand what you are trying to solve.
> 
> How can 1313a5e, which fixes misakes made in c2f62a3, come before
> that commit in the first place?

One scenario is something like this:

 Start with a clean HEAD (always a good idea :) )
 hack hack hack
 make multiple commits
 realize that a hunk you committed in an early patch belongs in a later one.
 use git rebase -i to fix it.


Is that more clear?

Thanks,

Steve.

Re: git interactive rebase 'consume' command

[Junio C Hamano]

Stephen Kelly <steveire@gmail.com> writes:

> Junio C Hamano wrote:
>> Sorry, but I do not understand what you are trying to solve.
>> 
>> How can 1313a5e, which fixes misakes made in c2f62a3, come before
>> that commit in the first place?
>
> One scenario is something like this:
>
>  Start with a clean HEAD (always a good idea :) )
>  hack hack hack
>  make multiple commits
>  realize that a hunk you committed in an early patch belongs in a later one.
>  use git rebase -i to fix it.
>
> Is that more clear?

Not really.

If you think that the author timestamp is the time the author
finished working on the commit, shouldn't the squashed result get
the timestamp when you finished squashing, not the timestamp of
either of the commits that were squashed?  Unlike "fixup" and
"reword", the change you are making is very different from any of
the original constituent commmits, and you finished working on that
change when you squashed these commits into one.  Propagating the
timestamp from the later ones sounds equally wrong for that purpose.

In any case, the intent of the author timestamp is to record the
time the author _started_ working on the change and came up with an
initial, possibly a partial, draft.  It does not record the time
when the commit was finalized.  "git commit --amend" preserves the
original timestamp, doesn't it?

In your example:

>  pick 07bc3c9 Good commit.
>  pick 1313a5e Commit to fixup into c2f62a3.
>  pick c2f62a3 Another commit.

you can view 1313a5e as a "preparatory clean-up for the real change
in c2f62a3", which could be a separate commit in the final history.
If you choose to squash them together into one, the time you
recorded 1313a5e was when you started working on the combined
change, so it does not sound so wrong to take that author timestamp
for the result.

Re: git interactive rebase 'consume' command

[Jeff King]

On Sun, Jan 20, 2013 at 12:23:41PM -0800, Junio C Hamano wrote:

> In any case, the intent of the author timestamp is to record the
> time the author _started_ working on the change and came up with an
> initial, possibly a partial, draft.  It does not record the time
> when the commit was finalized.  "git commit --amend" preserves the
> original timestamp, doesn't it?

And we have "--reset-author" if you want to do that. It seems like just
doing "git commit --amend --reset-author" at the end[1] would solve the
original problem.  Perhaps that is something that we could better
support directly from the instruction sheet.

-Peff

[1] or after an "edit" break in the instruction sheet, if it is in the
    middle of a set of commits

Re: git interactive rebase 'consume' command

[Stephen Kelly]

Junio C Hamano wrote:

> Stephen Kelly <steveire@gmail.com> writes:

>> One scenario is something like this:
>>
>>  Start with a clean HEAD (always a good idea :) )
>>  hack hack hack
>>  make multiple commits
>>  realize that a hunk you committed in an early patch belongs in a later
>>  one. use git rebase -i to fix it.
>>
>> Is that more clear?
> 
> Not really.

I think there are other scenarios, but I guess this won't happen anyway.

Thanks,

Steve.

Re: git interactive rebase 'consume' command

[Michael Haggerty]

On 01/20/2013 03:05 PM, Stephen Kelly wrote:
> I find the fixup command during an interactive rebase useful.
> 
> Sometimes when cleaning up a branch, I end up in a situation like this:
> 
>  pick 07bc3c9 Good commit.
>  pick 1313a5e Commit to fixup into c2f62a3.
>  pick c2f62a3 Another commit.
> 
> 
> So, I have to reorder the commits, and change 1313a5e to 'f'. An alternative 
> would be to squash 's' c2f62a3 into 1313a5e and clean up the commit message. 
> The problem with that is it ends up with the wrong author time information.

I do "squash with successor then clean up commit message" all the time.
 I had never worried (or even thought much) about the author time of the
resulting commit.  I think I will continue not worrying about it :-)

I think it would be great to have a shorthand for this operation in "git
rebase --interactive" and I probably would have implemented it when I
added "fixup" if I had been able to think of a good name for it.  Even
though I do this sort of thing less frequently than "fixup", it still
comes up often enough that a special command for it would be useful.

> So, I usually reorder and then fixup, but that can also be problematic if I 
> get a conflict during the re-order (which is quite likely).

It is perverse to have to turn a well-defined and manifestly
conflict-free wish into one that has a good chance of conflicting, just
because of a limitation of the tool.

> I would prefer to be able to mark a commit as 'should be consumed', so that:
> 
>  pick 07bc3c9 Good commit.
>  consume 1313a5e Commit to fixup into c2f62a3.
>  pick c2f62a3 Another commit.
> 
> will result in 
> 
>  pick 07bc3c9 Good commit.
>  pick 62a3c2f Another commit.
> 
> directly.

Excellent.  But the name is not self-explanatory.  And there is
something different about your "consume" command:

Normally, "pick" means that the commit on that line is the start of a
new commit unrelated to its predecessors.  And in general, the command
on one line only affects the lines that come before it, not the lines
that come after it.  Under your proposal "consume" would change the
meaning of the following line, namely by changing what its "pick" means.
 It might be more consistent to require the following line to be changed
to "squash":

    pick 07bc3c9 Good commit.
    consume 1313a5e Commit to fixup into c2f62a3.
    squash c2f62a3 Another commit.

in which case the meaning of "consume" would be something like "pick
this commit but not its commit message.  There would have to be a
prohibition against generating commits with *no* commit messages, to
prevent series like [consume, pick] or [consume, fix, pick] while
allowing series like [consume, consume, squash, fix, fix].

If this is the interpretation, the name "quiet/q" might make things clearer.

Yet another approach would be to allow options on the commands.  For
example,

    pick 07bc3c9 Good commit.
    pick --quiet 1313a5e Commit to fixup into c2f62a3.
    squash c2f62a3 Another commit.

In fact if options were implemented, then "fixup" would mean the same as
"squash --quiet", "reword" could be written "pick --edit", and I'm sure
the new flexibility would make it easier to add other features (e.g.,
"pick --reset-author").

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: git interactive rebase 'consume' command

[Stephen Kelly]

On 01/21/2013 12:05 PM, Michael Haggerty wrote:
> It is perverse to have to turn a well-defined and manifestly
> conflict-free wish into one that has a good chance of conflicting, just
> because of a limitation of the tool.

Yes, I agree.

>> I would prefer to be able to mark a commit as 'should be consumed', so that:
>>
>>   pick 07bc3c9 Good commit.
>>   consume 1313a5e Commit to fixup into c2f62a3.
>>   pick c2f62a3 Another commit.
>>
>> will result in
>>
>>   pick 07bc3c9 Good commit.
>>   pick 62a3c2f Another commit.
>>
>> directly.
> Excellent.  But the name is not self-explanatory.  And there is
> something different about your "consume" command:
>
> Normally, "pick" means that the commit on that line is the start of a
> new commit unrelated to its predecessors.  And in general, the command
> on one line only affects the lines that come before it, not the lines
> that come after it.  Under your proposal "consume" would change the
> meaning of the following line, namely by changing what its "pick" means.

>   It might be more consistent to require the following line to be changed
> to "squash":

I'm -1 on that. I value the simple format of the todo file. If I want to 
edit a commit, I type deif, reword - deir, fixup - deif. I'd like 
something equally simple like deic for this operation. There's also a 
'consistency' argument there, and one I prefer to your consistency 
interpretation.

The same simplicity request applies to what you write below.

Thanks,

Steve.

>      pick 07bc3c9 Good commit.
>      consume 1313a5e Commit to fixup into c2f62a3.
>      squash c2f62a3 Another commit.
>
> in which case the meaning of "consume" would be something like "pick
> this commit but not its commit message.  There would have to be a
> prohibition against generating commits with *no* commit messages, to
> prevent series like [consume, pick] or [consume, fix, pick] while
> allowing series like [consume, consume, squash, fix, fix].
>
> If this is the interpretation, the name "quiet/q" might make things clearer.
>
> Yet another approach would be to allow options on the commands.  For
> example,
>
>      pick 07bc3c9 Good commit.
>      pick --quiet 1313a5e Commit to fixup into c2f62a3.
>      squash c2f62a3 Another commit.
>
> In fact if options were implemented, then "fixup" would mean the same as
> "squash --quiet", "reword" could be written "pick --edit", and I'm sure
> the new flexibility would make it easier to add other features (e.g.,
> "pick --reset-author").
>
> Michael
>