All of lore.kernel.org
 help / color / mirror / Atom feed
* git rebase --interactive squash/squish/fold/rollup
@ 2009-06-17 12:06 Minty
  2009-06-17 12:55 ` John Tapsell
  0 siblings, 1 reply; 40+ messages in thread
From: Minty @ 2009-06-17 12:06 UTC (permalink / raw)
  To: git

I was wondering if there was a git rebase --interactive "squash"
alternative, that instead of letting me edit and manually combine the
commit messages from multiple commits that I want squashed together,
instead simply used the first and folded the other commits into that?

I often find myself in the following pattern:

branch, hack, commit.
hack, commit, hack, commit
git rebase --interactive master
squash later commits into an earlier one
repeated [hack, commit]+, rebase, squash
merge

At which point, 99% of the time I want to fold the later commits into
an earlier commit, keep the commit message of the first and throw the
remaining commit messages into /dev/null.

I understand this is just a matter of editting the commit messages,
but I'm lazy and I find myself repeatadly dumbly deleting the latter
commit messages again and again.

A $EDITOR macro/extension might address this, but it seems (to me)
cleaner to extend the rebase command set:

< pick, edit, squash
> pick, edit, squash, fold

"fold" is perhaps the wrong word.  "squish" is perhaps too similar.
"rollup" maybe?

In any event, functionally it would do exactly the same as "squash",
except rather than let you edit the commit messages it would instead
simply use the commit message of the first commit.  And throw the
other commit messages away.

I'm not adverse at having a go at putting a patch together (although
this is not my forte), but I thought I'd check there wasn't prior art
or a good reason why this would be a "bad thing" to have?

Murray.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: git rebase --interactive squash/squish/fold/rollup
  2009-06-17 12:06 git rebase --interactive squash/squish/fold/rollup Minty
@ 2009-06-17 12:55 ` John Tapsell
  2009-06-17 13:45   ` Minty
  2009-06-17 16:33   ` Junio C Hamano
  0 siblings, 2 replies; 40+ messages in thread
From: John Tapsell @ 2009-06-17 12:55 UTC (permalink / raw)
  To: git

> branch, hack, commit.
> hack, commit, hack, commit

What if you used  commit --append  instead?

The trouble though of squashing all the commits into one is that it
makes it impossible to bisect later.  Are you really sure that your
final commit cannot be broken into small commits?  Ideally each commit
is small but self contained.  Squashing should be done only to fix
cases where you introduced a bug then fixed it, or to fix a partial
implementation etc.

John

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: git rebase --interactive squash/squish/fold/rollup
  2009-06-17 12:55 ` John Tapsell
@ 2009-06-17 13:45   ` Minty
  2009-06-17 16:33   ` Junio C Hamano
  1 sibling, 0 replies; 40+ messages in thread
From: Minty @ 2009-06-17 13:45 UTC (permalink / raw)
  To: git

On Wed, Jun 17, 2009 at 1:55 PM, John Tapsell <johnflux@gmail.com> wrote:
>
> > branch, hack, commit.
> > hack, commit, hack, commit
>
> What if you used  commit --append  instead?

That appears to be a switch I don't have, nor is documented

http://www.kernel.org/pub/software/scm/git/docs/git-commit.html

Did you perhaps mean --amend?  Or have I missed something?

--amend is not really a solution for me - it is perhaps a quirk of my
working pattern, but I typically (on the branch) commit tiny tiny bits
of a (possibly incomplete) feature, then want to merge them back into
a single "feature commit" to merge with trunk.  It's a case of
building up a feature commit one step at a time.

Perhaps I'm not normal or going about it wrong, in that I'm happy to
commit (on a branch) an incomplete bit of code ... pop off to do
something else, come back, hack a little more ... go off, come back
... eventually ending up with a bunch of commits I want to merge down
into a smaller set of (combined) commits which to then merge with
master/trunk.

fwiw, I didn't set out with this pattern in mind, it's rather one I
have noticed myself being in frequently.  It seems quite natural to
me, except for this repeated squashing mini commits down.  I'm not
squashing ALL commits down into one single commit.  Rather many
commits down into a few commits, which then get merged with
master/trunk.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: git rebase --interactive squash/squish/fold/rollup
  2009-06-17 12:55 ` John Tapsell
  2009-06-17 13:45   ` Minty
@ 2009-06-17 16:33   ` Junio C Hamano
  2009-06-17 16:40     ` John Tapsell
                       ` (4 more replies)
  1 sibling, 5 replies; 40+ messages in thread
From: Junio C Hamano @ 2009-06-17 16:33 UTC (permalink / raw)
  To: John Tapsell; +Cc: git

John Tapsell <johnflux@gmail.com> writes:

>> branch, hack, commit.
>> hack, commit, hack, commit
>
> What if you used  commit --append  instead?
>
> The trouble though of squashing all the commits into one is that it
> makes it impossible to bisect later.  Are you really sure that your
> final commit cannot be broken into small commits?  Ideally each commit
> is small but self contained.  Squashing should be done only to fix
> cases where you introduced a bug then fixed it, or to fix a partial
> implementation etc.

I think you meant --amend, but it often happens to me that after preparing
a three-patch series:

	[1/3] Clean up the surrounding code I will touch
        [2/3] Lay the groundwork
        [3/3] Implement a cool new feature

I find that there are more clean-up that should have been done in [1/3].
The way "rebase -i" expects me to work is:

	$ edit ;# more clean-ups
	$ git commit -a -m 'squash to "clean up"'
        $ git rebase -i HEAD~5

which will give me
        
        pick 1/3 Clean up ...
        pick 2/3 Lay the groundwork
        pick 3/3 Implement
        pick 4/3 squash to "clean up"

that I'll change to 

        pick 1/3 Clean up ...
        squash 4/3 squash to "clean up"
        pick 2/3 Lay the groundwork
        pick 3/3 Implement

and then I'll need to edit the commit message for the first two combined.
More than half of the time (but not necessarily all the time), the edit
involves just removing the single-liner 'squash to "clean up"',

You _could_ work this way instead using "amend".  Immediately after
finishing the three-patch series:

	$ git rebase -i HEAD~4

which gives me

        pick 1/3 Clean up ...
        pick 2/3 Lay the groundwork
        pick 3/3 Implement

that I'll change to

        edit 1/3 Clean up ...
        pick 2/3 Lay the groundwork
        pick 3/3 Implement

and then perform extra clean-up when "rebase -i" let's me amend the first
one.

But this is much less convenient than being able to accumulate fix-ups as
separate commits on top of the mostly finished main series, and then being
able to later insert these fix-ups into the main series to be squashed
using "rebase -i", if (and only if) what you need to do are many small
fixups (imagine there are not just a single '[4/3] squash to "clean up"'
but a lot more fix-up commits in the above example).  Depending on the
style you work, "go back to amend" is Ok, or you may even prefer to.  But
some people do not switch context as rapidly as others.  After finding a
small "missed piece", having to go back to edit and come back is much more
heavyweight operation than being able to make a small "fix-up" commit on
top and keep going.  The latter keeps your thought process less disrupted.

And it is very likely that the "small fixups" won't change what the
original commit log message of the commit in the main needs to say
(otherwise they won't be "small").

So I can see why a variant of "squash" that does not change (nor even ask
for a replacement of) the commit log message from the one that is being
amended could be useful.  I am tempted to suggest calling that a "fixup"
operation, but some people may expect "fixup" to mean a variant of "edit"
that does not bother you by dropping you back to the shell to touch the
tree that is recorded (i.e. "fixing up the commit log message only"), so
it is not a very good word.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: git rebase --interactive squash/squish/fold/rollup
  2009-06-17 16:33   ` Junio C Hamano
@ 2009-06-17 16:40     ` John Tapsell
  2009-06-17 16:48     ` Paolo Bonzini
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 40+ messages in thread
From: John Tapsell @ 2009-06-17 16:40 UTC (permalink / raw)
  To: git

2009/6/17 Junio C Hamano <gitster@pobox.com>:
> John Tapsell <johnflux@gmail.com> writes:
>
>>> branch, hack, commit.
>>> hack, commit, hack, commit
>>
>> What if you used  commit --append  instead?
>>
>> The trouble though of squashing all the commits into one is that it
>> makes it impossible to bisect later.  Are you really sure that your
>> final commit cannot be broken into small commits?  Ideally each commit
>> is small but self contained.  Squashing should be done only to fix
>> cases where you introduced a bug then fixed it, or to fix a partial
>> implementation etc.
>
> I think you meant --amend, but it often happens to me that after preparing
> a three-patch series:
>
>        [1/3] Clean up the surrounding code I will touch
>        [2/3] Lay the groundwork
>        [3/3] Implement a cool new feature
>
> I find that there are more clean-up that should have been done in [1/3].
> The way "rebase -i" expects me to work is:
>
>        $ edit ;# more clean-ups
>        $ git commit -a -m 'squash to "clean up"'
>        $ git rebase -i HEAD~5
>
> which will give me
>
>        pick 1/3 Clean up ...
>        pick 2/3 Lay the groundwork
>        pick 3/3 Implement
>        pick 4/3 squash to "clean up"
>
> that I'll change to
>
>        pick 1/3 Clean up ...
>        squash 4/3 squash to "clean up"
>        pick 2/3 Lay the groundwork
>        pick 3/3 Implement

Yeah.  It would be nice to have a 'crush' or something here.
It's similar to the arguments to have a command to just edit the
commit message in a single go, rather than the rather long way of
using edit.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: git rebase --interactive squash/squish/fold/rollup
  2009-06-17 16:33   ` Junio C Hamano
  2009-06-17 16:40     ` John Tapsell
@ 2009-06-17 16:48     ` Paolo Bonzini
  2009-06-17 17:05     ` John Koleszar
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 40+ messages in thread
From: Paolo Bonzini @ 2009-06-17 16:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: John Tapsell, git


> So I can see why a variant of "squash" that does not change (nor even ask
> for a replacement of) the commit log message from the one that is being
> amended could be useful.

One way could be to have arguments to squash in a way that was proposed 
for the sequencer GSOC project last year.  For example,

commit 123456
squash -C HEAD abcdef

would just proceed with the commit of HEAD, and since squash is 
basically apply-to-index + commit, the HEAD would still be 123456.

Paolo

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: git rebase --interactive squash/squish/fold/rollup
  2009-06-17 16:33   ` Junio C Hamano
  2009-06-17 16:40     ` John Tapsell
  2009-06-17 16:48     ` Paolo Bonzini
@ 2009-06-17 17:05     ` John Koleszar
  2009-06-17 20:50       ` John Tapsell
  2009-06-17 18:20     ` Clemens Buchacher
  2009-06-17 21:33     ` [PATCH] rebase -i: auto-squash commits Nanako Shiraishi
  4 siblings, 1 reply; 40+ messages in thread
From: John Koleszar @ 2009-06-17 17:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: John Tapsell, git

On Wed, 2009-06-17 at 12:33 -0400, Junio C Hamano wrote:
> So I can see why a variant of "squash" that does not change (nor even ask
> for a replacement of) the commit log message from the one that is being
> amended could be useful.  I am tempted to suggest calling that a "fixup"
> operation, but some people may expect "fixup" to mean a variant of "edit"
> that does not bother you by dropping you back to the shell to touch the
> tree that is recorded (i.e. "fixing up the commit log message only"), so
> it is not a very good word.

I wonder if a better approach might be to add an operator to squash
rather than another verb. "squash!" maybe? This has the nice property
that future verbs that have both interactive and non-interactive modes
could be made consistent with squash easily, rather than having to think
of another synonym.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: git rebase --interactive squash/squish/fold/rollup
  2009-06-17 16:33   ` Junio C Hamano
                       ` (2 preceding siblings ...)
  2009-06-17 17:05     ` John Koleszar
@ 2009-06-17 18:20     ` Clemens Buchacher
  2009-06-18 22:31       ` Minty
  2009-06-17 21:33     ` [PATCH] rebase -i: auto-squash commits Nanako Shiraishi
  4 siblings, 1 reply; 40+ messages in thread
From: Clemens Buchacher @ 2009-06-17 18:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: John Tapsell, git

On Wed, Jun 17, 2009 at 09:33:19AM -0700, Junio C Hamano wrote:
> So I can see why a variant of "squash" that does not change (nor even ask
> for a replacement of) the commit log message from the one that is being
> amended could be useful.

How about deleting the commit message header?

pick c0ffee not to be modified commit message
pick 012345 squashme
...

=>

pick c0ffee not to be modified commit message
squash 012345
...

It requires explicit removal the unwanted commit message, avoiding any
accidents due to ambiguous keywords.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: git rebase --interactive squash/squish/fold/rollup
  2009-06-17 17:05     ` John Koleszar
@ 2009-06-17 20:50       ` John Tapsell
  0 siblings, 0 replies; 40+ messages in thread
From: John Tapsell @ 2009-06-17 20:50 UTC (permalink / raw)
  To: john.koleszar; +Cc: Junio C Hamano, git

> I wonder if a better approach might be to add an operator to squash
> rather than another verb. "squash!" maybe? This has the nice property
> that future verbs that have both interactive and non-interactive modes
> could be made consistent with squash easily, rather than having to think
> of another synonym.


We could also have   edit!  to just straight the commit message stage,
and then automatically continue.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH] rebase -i: auto-squash commits
  2009-06-17 16:33   ` Junio C Hamano
                       ` (3 preceding siblings ...)
  2009-06-17 18:20     ` Clemens Buchacher
@ 2009-06-17 21:33     ` Nanako Shiraishi
  2009-06-17 22:08       ` Johannes Schindelin
                         ` (2 more replies)
  4 siblings, 3 replies; 40+ messages in thread
From: Nanako Shiraishi @ 2009-06-17 21:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: John Tapsell, git

When the commit log message begins with "squash to ...", and there
is a commit whose title begins with the same ..., automatically
modify the todo list of rebase -i so that the commit marked for
squashing come right after the commit to be modified, and change
the action of the moved commit from pick to squash.

Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---

  Quoting Junio C Hamano <gitster@pobox.com>:

  I think you meant --amend, but it often happens to me that after preparing
  a three-patch series:
  
      [1/3] Clean up the surrounding code I will touch
      [2/3] Lay the groundwork
      [3/3] Implement a cool new feature
  
  I find that there are more clean-up that should have been done in [1/3].
  The way "rebase -i" expects me to work is:
  
      $ edit ;# more clean-ups
      $ git commit -a -m 'squash to "clean up"'
      $ git rebase -i HEAD~5
  
  which will give me
          
      pick 1/3 Clean up ...
      pick 2/3 Lay the groundwork
      pick 3/3 Implement
      pick 4/3 squash to "clean up"
  
  that I'll change to 
  
      pick 1/3 Clean up ...
      squash 4/3 squash to "clean up"
      pick 2/3 Lay the groundwork
      pick 3/3 Implement
  
  and then I'll need to edit the commit message for the first two combined.

How about this patch?  It does not let you say 'squash to "clean up"'
but other people who are more skillfull than me can enhance such details.

 git-rebase--interactive.sh   |   31 +++++++++++++++++++++++++++++++
 t/t3414-rebase-autosquash.sh |   36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 0 deletions(-)
 create mode 100755 t/t3414-rebase-autosquash.sh

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f96d887..0832164 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -482,6 +482,35 @@ get_saved_options () {
 	test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
 }
 
+# Rearrange the todo list that has both "pick sha1 msg" and
+# "pick sha1 squash to msg" in it, so that the latter comes
+# immediately after the former, and change "pick" to "squash".
+rearrange_squash () {
+	sed -n -e 's/^pick \([0-9a-f]*\) squash to /\1 /p' "$1" >"$1.sq"
+	test -s "$1.sq" || return
+
+	used=
+	while read pick sha1 message
+	do
+		case " $used" in
+		*" $sha1 "*) continue ;;
+		esac
+		echo "$pick $sha1 $message"
+		while read squash msg
+		do
+			case "$message" in
+			"$msg"*)
+				echo "squash $squash to $msg"
+				used="$used$squash "
+				break
+				;;
+			esac
+		done <"$1.sq"
+	done <"$1" >"$1.rearranged"
+
+	cat "$1.rearranged" >"$1"
+}
+
 while test $# != 0
 do
 	case "$1" in
@@ -746,6 +776,7 @@ first and then run 'git rebase --continue' again."
 		fi
 
 		test -s "$TODO" || echo noop >> "$TODO"
+		rearrange_squash "$TODO"
 		cat >> "$TODO" << EOF
 
 # Rebase $SHORTREVISIONS onto $SHORTONTO
diff --git a/t/t3414-rebase-autosquash.sh b/t/t3414-rebase-autosquash.sh
new file mode 100755
index 0000000..ddb0daf
--- /dev/null
+++ b/t/t3414-rebase-autosquash.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+
+test_description='auto squash'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo 0 > file0
+	git add .
+	test_tick
+	git commit -m "initial commit"
+	echo 0 > file1
+	echo 2 > file2
+	git add .
+	test_tick
+	git commit -m "first commit"
+	echo 3 > file3
+	git add .
+	test_tick
+	git commit -m "second commit"
+'
+
+test_expect_success 'auto squash' '
+	echo 1 > file1
+	git add -u
+	test_tick
+	git commit -m "squash to first"
+	git tag final
+	test_tick
+	git rebase -i HEAD^^^
+	git log --oneline >actual
+	test 3 = $(wc -l <actual) &&
+	git diff --exit-code final
+'
+
+test_done
-- 
1.6.2.GIT

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH] rebase -i: auto-squash commits
  2009-06-17 21:33     ` [PATCH] rebase -i: auto-squash commits Nanako Shiraishi
@ 2009-06-17 22:08       ` Johannes Schindelin
  2009-06-18  0:11         ` [PATCH] " Nicolas Sebrecht
  2009-06-18  5:21       ` [PATCH] " Junio C Hamano
  2009-06-18  7:20       ` [PATCH] rebase -i: " Michael Haggerty
  2 siblings, 1 reply; 40+ messages in thread
From: Johannes Schindelin @ 2009-06-17 22:08 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, John Tapsell, git

Hi,

On Thu, 18 Jun 2009, Nanako Shiraishi wrote:

> When the commit log message begins with "squash to ...", and there

I do not like this at all.  It assumes that you never have valid commit 
messages starting with "squash to".

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH] Re: rebase -i: auto-squash commits
  2009-06-17 22:08       ` Johannes Schindelin
@ 2009-06-18  0:11         ` Nicolas Sebrecht
  2009-06-18  5:07           ` Junio C Hamano
  0 siblings, 1 reply; 40+ messages in thread
From: Nicolas Sebrecht @ 2009-06-18  0:11 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Nanako Shiraishi, Junio C Hamano, John Tapsell, git

The 18/06/09, Johannes Schindelin wrote:

> > When the commit log message begins with "squash to ...", and there
> 
> I do not like this at all.  It assumes that you never have valid commit 
> messages starting with "squash to".

Plus, a commit message should not be anything else that a message about
a commit. Please, don't make the Git's behavior depends on the commit
message itself.

If we need a program to have various behaviours, we have:
- the compilation options;
- the command line options;
- the configuration files.


-- 
Nicolas Sebrecht

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  0:11         ` [PATCH] " Nicolas Sebrecht
@ 2009-06-18  5:07           ` Junio C Hamano
  2009-06-18  8:06             ` Johannes Schindelin
  2009-06-18 10:59             ` Nicolas Sebrecht
  0 siblings, 2 replies; 40+ messages in thread
From: Junio C Hamano @ 2009-06-18  5:07 UTC (permalink / raw)
  To: Nicolas Sebrecht; +Cc: Johannes Schindelin, Nanako Shiraishi, John Tapsell, git

Nicolas Sebrecht <nicolas.s.dev@gmx.fr> writes:

> The 18/06/09, Johannes Schindelin wrote:
>
>> > When the commit log message begins with "squash to ...", and there
>> 
>> I do not like this at all.  It assumes that you never have valid commit 
>> messages starting with "squash to".
>
> Plus, a commit message should not be anything else that a message about
> a commit. Please, don't make the Git's behavior depends on the commit
> message itself.
>
> If we need a program to have various behaviours, we have:
> - the compilation options;
> - the command line options;
> - the configuration files.

Sorry, but I have to disagree to such a dogmatic statement.

We do want our commands to be able to act intelligently and/or differently
depending on what commit says in some cases.  It is does not make sense to
insist that the command line or configuration mechanism must be used.

A really trivial example.  "git log -p" shows the patch text for non-merge
commits but not for merge commits.  "git log --grep=foo" shows only
commits that says "foo" and "git log --author=Nicolas" shows only commits
written by you.  We used to leave an explicit note in the message part of
cherry-picked commits where they were cherry-picked from; "git merge"
and/or "git rebase" could have paid attention to it to act differently
(i.e. "ah, even though that commit is not in the ancestry, the moral
equivalent patch is already applied").

Besides, if you as the end user want to tell this and that commit are
special among other commits that are being rebased to the command, which
is the scenario Nana's patch is about, how would you do that from the
command line option?  "rebase -i --move=4-to-2 --squash=2"?

I do not necessarily think the behaviour suggested by the patch should be
the default, but as an optional feature, it makes perfect sense for a
command to pay attention to commit messages when deciding what to do.

IOW, I understand Dscho's objection that there is a risk that this feature
may trigger when not wanted (but more on this later), and I'd be fine if
it can fire only with an extra option, e.g. "git rebase -i --autosquash".

But from the workflow point of view, I think what the patch tries to do (I
haven't studied the actual implementation carefully, so it may not be what
it actually _does_) makes perfect sense, and it matches what I often do
very well.  Accumulate changes as a series of basically sound commits,
queue some small "fix this breakage in that commit" commits on top of them
while proofreading, and finish the series with "rebase -i" to reorder,
squash and typofix.

Now, I initially had the same reaction as Dscho.  What happens if I really
want to write a commit message that begins with "squash to "?

But after thinking about it a bit more, I do not think it is as bad as it
sounds anymore.

The commit not only must begin with "squash to " but also there has to be
a matching commit whose message begins with the remainder of the title of
the "squash to" commit _in the range you are rebasing INTERACTIVELY_.

In addition, the resulting rebase insn is presented in the editor, and in
a rare case where you do have such a commit, you can rearrange it back.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] rebase -i: auto-squash commits
  2009-06-17 21:33     ` [PATCH] rebase -i: auto-squash commits Nanako Shiraishi
  2009-06-17 22:08       ` Johannes Schindelin
@ 2009-06-18  5:21       ` Junio C Hamano
  2009-06-18 21:55         ` [PATCH v2] rebase -i --autosquash: " Nanako Shiraishi
  2009-06-18  7:20       ` [PATCH] rebase -i: " Michael Haggerty
  2 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2009-06-18  5:21 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: John Tapsell, git

Nanako Shiraishi <nanako3@lavabit.com> writes:

>       pick 1/3 Clean up ...
>       pick 2/3 Lay the groundwork
>       pick 3/3 Implement
>       pick 4/3 squash to "clean up"
>   
>   that I'll change to 
>   
>       pick 1/3 Clean up ...
>       squash 4/3 squash to "clean up"
>       pick 2/3 Lay the groundwork
>       pick 3/3 Implement
>   
>   and then I'll need to edit the commit message for the first two combined.
>
> How about this patch?  It does not let you say 'squash to "clean up"'
> but other people who are more skillfull than me can enhance such details.

I have to admit that I wished to see something like this for more than
once.  It would have been nicer if the patch went one step further and did
"squash the patch, but use the log message from the commit that is
squashed into, without even asking for a consolidated message", but I
think it is a reasonable start.

But as Dscho already objected to, this is a new feature that is
potentially dangerous --- there is a risk of matching a commit that was
not intended for squashing, albeit small.  We may want an explicit option
to enable it.  On the other hand, you may be able to argue that use of
"interactive" rebase is already a sign that the user is likely to want
such a convenience, though.

>  git-rebase--interactive.sh   |   31 +++++++++++++++++++++++++++++++
>  t/t3414-rebase-autosquash.sh |   36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+), 0 deletions(-)
>  create mode 100755 t/t3414-rebase-autosquash.sh
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index f96d887..0832164 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -482,6 +482,35 @@ get_saved_options () {
>  	test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
>  }
>  
> +# Rearrange the todo list that has both "pick sha1 msg" and
> +# "pick sha1 squash to msg" in it, so that the latter comes
> +# immediately after the former, and change "pick" to "squash".
> +rearrange_squash () {
> +	sed -n -e 's/^pick \([0-9a-f]*\) squash to /\1 /p' "$1" >"$1.sq"
> +	test -s "$1.sq" || return
> +
> +	used=
> +	while read pick sha1 message
> +	do
> +		case " $used" in
> +		*" $sha1 "*) continue ;;
> +		esac
> +		echo "$pick $sha1 $message"
> +		while read squash msg
> +		do
> +			case "$message" in
> +			"$msg"*)

I guess we could even loosen this "must match the leading substring
exactly" restriction if we can expose Dscho's Levenstein to Porcelain
writers.

> +				echo "squash $squash to $msg"
> +				used="$used$squash "
> +				break
> +				;;

Do you really want to break here?  What happens if I have more than one
fixup patches to the same commit?

> +			esac
> +		done <"$1.sq"
> +	done <"$1" >"$1.rearranged"
> +
> +	cat "$1.rearranged" >"$1"
> +}
> +
>  while test $# != 0
>  do
>  	case "$1" in
> @@ -746,6 +776,7 @@ first and then run 'git rebase --continue' again."
>  		fi
>  
>  		test -s "$TODO" || echo noop >> "$TODO"
> +		rearrange_squash "$TODO"
>  		cat >> "$TODO" << EOF
>  
>  # Rebase $SHORTREVISIONS onto $SHORTONTO
> diff --git a/t/t3414-rebase-autosquash.sh b/t/t3414-rebase-autosquash.sh
> new file mode 100755
> index 0000000..ddb0daf
> --- /dev/null
> +++ b/t/t3414-rebase-autosquash.sh
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +
> +test_description='auto squash'
> +
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +	echo 0 > file0
> +	git add .
> +	test_tick
> +	git commit -m "initial commit"
> +	echo 0 > file1
> +	echo 2 > file2
> +	git add .
> +	test_tick
> +	git commit -m "first commit"
> +	echo 3 > file3
> +	git add .
> +	test_tick
> +	git commit -m "second commit"
> +'

These tests want to be stringed together with && to catch possible
breakages during the setup.  The same for the real test below.

> +test_expect_success 'auto squash' '
> +	echo 1 > file1
> +	git add -u
> +	test_tick
> +	git commit -m "squash to first"
> +	git tag final
> +	test_tick
> +	git rebase -i HEAD^^^
> +	git log --oneline >actual
> +	test 3 = $(wc -l <actual) &&

Not just count, but you would want to make sure that the rewritten "first
commit" now has the desired tree ("1" instead of "0" in file1, if I am
reading the test correctly).

> +	git diff --exit-code final
> +'
> +
> +test_done

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] rebase -i: auto-squash commits
  2009-06-17 21:33     ` [PATCH] rebase -i: auto-squash commits Nanako Shiraishi
  2009-06-17 22:08       ` Johannes Schindelin
  2009-06-18  5:21       ` [PATCH] " Junio C Hamano
@ 2009-06-18  7:20       ` Michael Haggerty
  2009-06-18  7:54         ` Junio C Hamano
  2 siblings, 1 reply; 40+ messages in thread
From: Michael Haggerty @ 2009-06-18  7:20 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, John Tapsell, git

Nanako Shiraishi wrote:
> When the commit log message begins with "squash to ...", and there
> is a commit whose title begins with the same ..., automatically
> modify the todo list of rebase -i so that the commit marked for
> squashing come right after the commit to be modified, and change
> the action of the moved commit from pick to squash.

It seems to me that even this requires more steps than strictly
necessary, namely a commit then a rebase, and conveying the information
from the commit step to the rebase step is somewhat awkward.  Since I
have to specify a magic commit message to trigger this behavior, I
obviously know at the time of the commit that I want to squash the new
changes onto an older commit.  So why not implement this functionality
as a variant of "commit"?  Something like:

git commit --fix=old-commit

which would commit the changes in index as an amendment to the specified
old-commit (requiring no new log message) and then rebase later commits
on top of the new (combined) commit.

If a conflict arises while applying the changes in index to old-commit,
then probably the whole process should be undone and aborted.  If a
conflict arises while rebasing later commits on top of the combined
commit, then the usual rebase conflict-handling machinery would be invoked.

Michael

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] rebase -i: auto-squash commits
  2009-06-18  7:20       ` [PATCH] rebase -i: " Michael Haggerty
@ 2009-06-18  7:54         ` Junio C Hamano
  0 siblings, 0 replies; 40+ messages in thread
From: Junio C Hamano @ 2009-06-18  7:54 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: Nanako Shiraishi, John Tapsell, git

Michael Haggerty <mhagger@alum.mit.edu> writes:

> It seems to me that even this requires more steps than strictly
> necessary, namely a commit then a rebase, and conveying the information
> from the commit step to the rebase step is somewhat awkward.  Since I
> have to specify a magic commit message to trigger this behavior, I
> obviously know at the time of the commit that I want to squash the new
> changes onto an older commit.  So why not implement this functionality
> as a variant of "commit"?

That may be a good feature, but that won't work as well as the patch being
discussed for _me_.

IOW, I think what you are suggesting is a different feature.

It largely depends on how you work.  I do not function well when I get
interrupted and/or disrupted often, and I would prefer the convenience of
being able to simply queue a trivial patch with a minimum amount of fuss
(e.g. just leave a note that says "to be squashed to that other one" and
nothing else) when I find a trivial breakage that is unrelated to what I
am concentrating on.

Imagine the "Clean up the surrounding code" then "Lay the groundwork" and
finally "Implement a cool new feature" sequence I outlined in the message
the patch was response to.  When I thought I am finished cleaning up the
surrounding code and laid the groundwork, and finally concentrating on
implementing the new feature (which is the fun part), I may notice small
breakages and untidiness I could squash into earlier commits.

It is very distracting, however, if I have to go back to the state _before
I wrote all the fun code for the new feature_ to fix the breakage right
there.  Once I go back, the surrounding code would look all different, and
I may even be tempted to do the full test cycle before finishing your
"amend in the past" operation.  The distraction will destroy my momentum
and concentration.

It's much more easier on my brain to commit the fix-up to be later
squashed (use "add -p then commit" for that) and continue.  I can keep the
momentum going that way.

But that is how _I_ work.  You may well work differently, and for you
"stop, switch brain back to the state before all these fun work and amend,
then finally come back" workflow may work better.

What I am saying is that "a variant of commit" you talk may be good but it
won't be a _replacement_ for the effort to make squash easier to do while
running "rebase -i".

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  5:07           ` Junio C Hamano
@ 2009-06-18  8:06             ` Johannes Schindelin
  2009-06-18  8:11               ` Jakub Narebski
                                 ` (3 more replies)
  2009-06-18 10:59             ` Nicolas Sebrecht
  1 sibling, 4 replies; 40+ messages in thread
From: Johannes Schindelin @ 2009-06-18  8:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Hi,

On Wed, 17 Jun 2009, Junio C Hamano wrote:

> Now, I initially had the same reaction as Dscho.  What happens if I 
> really want to write a commit message that begins with "squash to "?
> 
> But after thinking about it a bit more, I do not think it is as bad as 
> it sounds anymore.
> 
> The commit not only must begin with "squash to " but also there has to 
> be a matching commit whose message begins with the remainder of the 
> title of the "squash to" commit _in the range you are rebasing 
> INTERACTIVELY_.
> 
> In addition, the resulting rebase insn is presented in the editor, and 
> in a rare case where you do have such a commit, you can rearrange it 
> back.

Well, that really sounds pretty awkward to me.  I regularly call such 
commits "amend".  If there is a risk I confuse myself as to which commit 
needs to be amended, I use "amend.<short-hint>".

I'd really rather stay with "fixup".  And as I use single-letter commands 
quite often, I'd also rather stay away from that magic "!".  And by 
"magic" I really mean that: people will not find that magic intuitive at 
all.

My vote is for "fixup".

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:06             ` Johannes Schindelin
@ 2009-06-18  8:11               ` Jakub Narebski
  2009-06-18  8:21                 ` Junio C Hamano
  2009-06-18  8:17               ` Teemu Likonen
                                 ` (2 subsequent siblings)
  3 siblings, 1 reply; 40+ messages in thread
From: Jakub Narebski @ 2009-06-18  8:11 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

A bit off-topic: I wonder if there is an easy way to make rebase run
testsuite for the each commit it rebases, or even simple compile test,
to not introduce untestable commits when rebasing by mistake...

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:06             ` Johannes Schindelin
  2009-06-18  8:11               ` Jakub Narebski
@ 2009-06-18  8:17               ` Teemu Likonen
  2009-06-18  8:29                 ` Johannes Schindelin
  2009-06-18  8:20               ` Junio C Hamano
  2009-06-18  8:34               ` Matthieu Moy
  3 siblings, 1 reply; 40+ messages in thread
From: Teemu Likonen @ 2009-06-18  8:17 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

On 2009-06-18 10:06 (+0200), Johannes Schindelin wrote:

> I'd really rather stay with "fixup". And as I use single-letter
> commands quite often, I'd also rather stay away from that magic "!".
> And by "magic" I really mean that: people will not find that magic
> intuitive at all.

I don't know about people but I do find "!" intuitive. It is squash
after all so I like the idea of using small modifier character.

> My vote is for "fixup".

Mine is for "squash!".

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:06             ` Johannes Schindelin
  2009-06-18  8:11               ` Jakub Narebski
  2009-06-18  8:17               ` Teemu Likonen
@ 2009-06-18  8:20               ` Junio C Hamano
  2009-06-18  8:33                 ` Johannes Schindelin
  2009-06-18 11:18                 ` Nicolas Sebrecht
  2009-06-18  8:34               ` Matthieu Moy
  3 siblings, 2 replies; 40+ messages in thread
From: Junio C Hamano @ 2009-06-18  8:20 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Wed, 17 Jun 2009, Junio C Hamano wrote:
> ...
>> The commit not only must begin with "squash to " but also there has to 
>> be a matching commit whose message begins with the remainder of the 
>> title of the "squash to" commit _in the range you are rebasing 
>> INTERACTIVELY_.
>> 
>> In addition, the resulting rebase insn is presented in the editor, and 
>> in a rare case where you do have such a commit, you can rearrange it 
>> back.
>
> Well, that really sounds pretty awkward to me.  I regularly call such 
> commits "amend".  If there is a risk I confuse myself as to which commit 
> needs to be amended, I use "amend.<short-hint>".
>
> I'd really rather stay with "fixup".  And as I use single-letter commands 
> quite often, I'd also rather stay away from that magic "!".  And by 
> "magic" I really mean that: people will not find that magic intuitive at 
> all.
>
> My vote is for "fixup".

I am too tired to either make the final judgement nor proposal on this
topic now, but before I forget here is one tangent.

I also often use "magic" commit log message in other occasions.  The most
important is "[DONTMERGE]" prefix to somebody else's commit I queue to
'pu' (or leave unmerged even to 'pu'---just keeping on a topic branch).  I
accept a patch with "am" and then "amend" after review when I find that it
needs more work.  One day I am hoping to write a pre-merge hook that
forbids commits marked with such magic to come into 'next' and down.

The point?

Earlier somebody objected to a command that changes behaviour based on
what is in the commit log message, but for the private commits the patch
under discussion deals with and the ones I mark with "[DONTMERGE]", the
commit log message _is_ the right place to leave a mark for commands to
take notice and act differently.

Of course we _could_ use notes for that, but that won't play well with
rebasing I suppose ...

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:11               ` Jakub Narebski
@ 2009-06-18  8:21                 ` Junio C Hamano
  2009-06-18  8:26                   ` Johannes Schindelin
  0 siblings, 1 reply; 40+ messages in thread
From: Junio C Hamano @ 2009-06-18  8:21 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: Johannes Schindelin, Nicolas Sebrecht, Nanako Shiraishi,
	John Tapsell, git

Jakub Narebski <jnareb@gmail.com> writes:

> A bit off-topic: I wonder if there is an easy way to make rebase run
> testsuite for the each commit it rebases, or even simple compile test,
> to not introduce untestable commits when rebasing by mistake...

I used to do that manually, i.e. s/^pick /edit /;

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:21                 ` Junio C Hamano
@ 2009-06-18  8:26                   ` Johannes Schindelin
  0 siblings, 0 replies; 40+ messages in thread
From: Johannes Schindelin @ 2009-06-18  8:26 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jakub Narebski, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Hi,

On Thu, 18 Jun 2009, Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > A bit off-topic: I wonder if there is an easy way to make rebase run 
> > testsuite for the each commit it rebases, or even simple compile test, 
> > to not introduce untestable commits when rebasing by mistake...
> 
> I used to do that manually, i.e. s/^pick /edit /;

This could be a command

	run-foreach (cd t && make)

Hmm?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:17               ` Teemu Likonen
@ 2009-06-18  8:29                 ` Johannes Schindelin
  2009-06-18  8:44                   ` Teemu Likonen
  2009-06-18 14:04                   ` John Koleszar
  0 siblings, 2 replies; 40+ messages in thread
From: Johannes Schindelin @ 2009-06-18  8:29 UTC (permalink / raw)
  To: Teemu Likonen
  Cc: Junio C Hamano, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Hi,

On Thu, 18 Jun 2009, Teemu Likonen wrote:

> On 2009-06-18 10:06 (+0200), Johannes Schindelin wrote:
> 
> > I'd really rather stay with "fixup". And as I use single-letter
> > commands quite often, I'd also rather stay away from that magic "!".
> > And by "magic" I really mean that: people will not find that magic
> > intuitive at all.
> 
> I don't know about people but I do find "!" intuitive. It is squash
> after all so I like the idea of using small modifier character.

Mhm.

So let's just interpret the "!" in the most common meaning, namely to add 
an imperative.  Then it means "yes, I do want to squash".  Not 
"squash, but oh, BTW, I want to lose the second commit message 
completely, and I do not want to edit the commit message either".

Really, I do not see how anybody could find this intuitive at all.  Maybe 
after reading the manual, but kinda defeats the meaning of the word 
"intuitive".

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:20               ` Junio C Hamano
@ 2009-06-18  8:33                 ` Johannes Schindelin
  2009-06-18  8:44                   ` Michael J Gruber
  2009-06-19  7:18                   ` Miles Bader
  2009-06-18 11:18                 ` Nicolas Sebrecht
  1 sibling, 2 replies; 40+ messages in thread
From: Johannes Schindelin @ 2009-06-18  8:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Hi,

On Thu, 18 Jun 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Wed, 17 Jun 2009, Junio C Hamano wrote:
> > ...
> >> The commit not only must begin with "squash to " but also there has to 
> >> be a matching commit whose message begins with the remainder of the 
> >> title of the "squash to" commit _in the range you are rebasing 
> >> INTERACTIVELY_.
> >> 
> >> In addition, the resulting rebase insn is presented in the editor, and 
> >> in a rare case where you do have such a commit, you can rearrange it 
> >> back.
> >
> > Well, that really sounds pretty awkward to me.  I regularly call such 
> > commits "amend".  If there is a risk I confuse myself as to which commit 
> > needs to be amended, I use "amend.<short-hint>".
> >
> > I'd really rather stay with "fixup".  And as I use single-letter commands 
> > quite often, I'd also rather stay away from that magic "!".  And by 
> > "magic" I really mean that: people will not find that magic intuitive at 
> > all.
> >
> > My vote is for "fixup".
> 
> I am too tired to either make the final judgement nor proposal on this 
> topic now,

Okay, I'll add another point that should convince you that the commit 
message is not the good place to trigger that behavior:

Interactive rebasing is about having made a quite messy patch series, 
maybe having a few fixup commits, and then deciding how to clean it up.

The decision how to clean it up is very much a rebase-time decision, not a 
commit-time decision.

For example, it is very easy to decide that you want to squash one fixup 
after all instead of leaving it stand-alone.

> Of course we _could_ use notes for that, but that won't play well with
> rebasing I suppose ...

Reminds me.  Nothing has happened on that front, right?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:06             ` Johannes Schindelin
                                 ` (2 preceding siblings ...)
  2009-06-18  8:20               ` Junio C Hamano
@ 2009-06-18  8:34               ` Matthieu Moy
  2009-06-18  8:44                 ` Johannes Schindelin
  3 siblings, 1 reply; 40+ messages in thread
From: Matthieu Moy @ 2009-06-18  8:34 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I'd really rather stay with "fixup".

I like fixup. I'd say "fixup: <message>" so that the thing actually
looks like a program directive rather than natural language.

(I disliked this at first, but I may actually like it if it gets into
Git!)

-- 
Matthieu

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:33                 ` Johannes Schindelin
@ 2009-06-18  8:44                   ` Michael J Gruber
  2009-06-19  7:18                   ` Miles Bader
  1 sibling, 0 replies; 40+ messages in thread
From: Michael J Gruber @ 2009-06-18  8:44 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Johannes Schindelin venit, vidit, dixit 18.06.2009 10:33:
> Hi,
> 
> On Thu, 18 Jun 2009, Junio C Hamano wrote:
> 
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>> On Wed, 17 Jun 2009, Junio C Hamano wrote:
>>> ...
>>>> The commit not only must begin with "squash to " but also there has to 
>>>> be a matching commit whose message begins with the remainder of the 
>>>> title of the "squash to" commit _in the range you are rebasing 
>>>> INTERACTIVELY_.
>>>>
>>>> In addition, the resulting rebase insn is presented in the editor, and 
>>>> in a rare case where you do have such a commit, you can rearrange it 
>>>> back.
>>>
>>> Well, that really sounds pretty awkward to me.  I regularly call such 
>>> commits "amend".  If there is a risk I confuse myself as to which commit 
>>> needs to be amended, I use "amend.<short-hint>".
>>>
>>> I'd really rather stay with "fixup".  And as I use single-letter commands 
>>> quite often, I'd also rather stay away from that magic "!".  And by 
>>> "magic" I really mean that: people will not find that magic intuitive at 
>>> all.
>>>
>>> My vote is for "fixup".
>>
>> I am too tired to either make the final judgement nor proposal on this 
>> topic now,
> 
> Okay, I'll add another point that should convince you that the commit 
> message is not the good place to trigger that behavior:
> 
> Interactive rebasing is about having made a quite messy patch series, 
> maybe having a few fixup commits, and then deciding how to clean it up.
> 
> The decision how to clean it up is very much a rebase-time decision, not a 
> commit-time decision.
> 
> For example, it is very easy to decide that you want to squash one fixup 
> after all instead of leaving it stand-alone.
> 
>> Of course we _could_ use notes for that, but that won't play well with
>> rebasing I suppose ...
> 
> Reminds me.  Nothing has happened on that front, right?

<!--#if expr="$SARCASM_ON" -->
No, but isn't that the true purpose of out-sourcing? You've got someone
else to blame now!
<!--#endif -->

Cheers,
Michael

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:29                 ` Johannes Schindelin
@ 2009-06-18  8:44                   ` Teemu Likonen
  2009-06-18 12:16                     ` Johannes Schindelin
  2009-06-18 14:04                   ` John Koleszar
  1 sibling, 1 reply; 40+ messages in thread
From: Teemu Likonen @ 2009-06-18  8:44 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

On 2009-06-18 10:29 (+0200), Johannes Schindelin wrote:

> So let's just interpret the "!" in the most common meaning, namely to
> add an imperative. Then it means "yes, I do want to squash". Not
> "squash, but oh, BTW, I want to lose the second commit message
> completely, and I do not want to edit the commit message either".

My main point is the "small modifier character" for squash. Perhaps
"squash*" is better? I'll repeat that it is still doing very much the
same thing as "squash" expect for one little thing. Hence it would be
nice to use only small modifier character, not totally new word with
possibly different connotations.

    pick aaaa ...
    squash* bbbb Small fix to be squashed
    pick cccc ...
    pick dddd ...

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:34               ` Matthieu Moy
@ 2009-06-18  8:44                 ` Johannes Schindelin
  2009-06-18  8:59                   ` Matthieu Moy
  0 siblings, 1 reply; 40+ messages in thread
From: Johannes Schindelin @ 2009-06-18  8:44 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Junio C Hamano, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Hi,

On Thu, 18 Jun 2009, Matthieu Moy wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > I'd really rather stay with "fixup".
> 
> I like fixup. I'd say "fixup: <message>" so that the thing actually
> looks like a program directive rather than natural language.

Umm...  The thing would be used like this:

original edit list:

	pick b1ab1ab First commit
	pick deafbee Second commit
	pick 0123456 This is a fixup for the first commit

edited edit list:

	pick b1ab1ab First commit
	fixup 0123456 This is a fixup for the first commit
	pick deafbee Second commit

It would squash the first two commits, forget about the second commit 
message and continue with the last commit.  No user interaction unless 
there are merge conflicts.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:44                 ` Johannes Schindelin
@ 2009-06-18  8:59                   ` Matthieu Moy
  0 siblings, 0 replies; 40+ messages in thread
From: Matthieu Moy @ 2009-06-18  8:59 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Thu, 18 Jun 2009, Matthieu Moy wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > I'd really rather stay with "fixup".
>> 
>> I like fixup. I'd say "fixup: <message>" so that the thing actually
>> looks like a program directive rather than natural language.

[...]

> edited edit list:
>
> 	pick b1ab1ab First commit
> 	fixup 0123456 This is a fixup for the first commit
> 	pick deafbee Second commit

Sorry, we were not talking about the same thing: I was still talking
about the dwimery in the commit message. So, yes, your "fixup" (that
could be abbreviated by "f") sounds good to me.

But some (optional) magic to get the edited list by default could be
nice in addition, and that could be triggered by "fixup: ..." in the
commit message.

I do often find myself commiting something knowing that the commit is
meant for rebase+squash-ing (i.e. I know that at commit time more
often than at rebase time).

(not yet 100% convinced myself, and I can sure do without)

-- 
Matthieu

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  5:07           ` Junio C Hamano
  2009-06-18  8:06             ` Johannes Schindelin
@ 2009-06-18 10:59             ` Nicolas Sebrecht
  1 sibling, 0 replies; 40+ messages in thread
From: Nicolas Sebrecht @ 2009-06-18 10:59 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Nicolas Sebrecht, Johannes Schindelin, Nanako Shiraishi,
	John Tapsell, git

The 17/06/09, Junio C Hamano wrote:

> We do want our commands to be able to act intelligently and/or differently
> depending on what commit says in some cases.  It is does not make sense to
> insist that the command line or configuration mechanism must be used.
> 
> A really trivial example.  "git log -p" shows the patch text for non-merge
> commits but not for merge commits.  "git log --grep=foo" shows only
> commits that says "foo" and "git log --author=Nicolas" shows only commits
> written by you.  We used to leave an explicit note in the message part of
> cherry-picked commits where they were cherry-picked from; "git merge"
> and/or "git rebase" could have paid attention to it to act differently
> (i.e. "ah, even though that commit is not in the ancestry, the moral
> equivalent patch is already applied").

But I see a huge difference between a message added by the program
itself to act well on possible comming cases and a message added by the
user to act differently at the commit time.

The latter case is exposed to the user mistakes (wrong typo,
unintentional matching pattern, etc) which could leave the repository in
unexpected states.

Git is enough hard to learn. Please, don't make the learning curve even
worse. Having the commit message possibly making git acts differently is
not usual or expected by most users.

> Besides, if you as the end user want to tell this and that commit are
> special among other commits that are being rebased to the command, which
> is the scenario Nana's patch is about, how would you do that from the
> command line option?  "rebase -i --move=4-to-2 --squash=2"?

Well, as we always squash to one of the first direct ancestor and as
squashing to a merge is not usual (here at least), in most cases it just
gives "rebase -i --move=4-to-2" wich sounds reasonable enough to me.


-- 
Nicolas Sebrecht

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:20               ` Junio C Hamano
  2009-06-18  8:33                 ` Johannes Schindelin
@ 2009-06-18 11:18                 ` Nicolas Sebrecht
  1 sibling, 0 replies; 40+ messages in thread
From: Nicolas Sebrecht @ 2009-06-18 11:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Johannes Schindelin, Nicolas Sebrecht, Nanako Shiraishi,
	John Tapsell, git

The 18/06/09, Junio C Hamano wrote:

> I also often use "magic" commit log message in other occasions.  The most
> important is "[DONTMERGE]" prefix to somebody else's commit I queue to
> 'pu' (or leave unmerged even to 'pu'---just keeping on a topic branch).  I
> accept a patch with "am" and then "amend" after review when I find that it
> needs more work.  One day I am hoping to write a pre-merge hook that
> forbids commits marked with such magic to come into 'next' and down.
> 
> The point?
> 
> Earlier somebody objected to a command that changes behaviour based on
> what is in the commit log message, but for the private commits the patch
> under discussion deals with and the ones I mark with "[DONTMERGE]", the
> commit log message _is_ the right place to leave a mark for commands to
> take notice and act differently.
> 
> Of course we _could_ use notes for that, but that won't play well with
> rebasing I suppose ...

Not for now; you're right. But what I see here is all about
commit/branch metadata to make our like with workflows easier.

What about implementing a true metadata feature into Git? There are a
lot of nice possible functionalities around metadata.

Fast, stupid and superficial thoughts on that:
- have metadata to make git to act differently and/or for information
  purpose;
- let the user create its own metadata for his own purpose;
- let the user have hooks script where appropriate.


-- 
Nicolas Sebrecht

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:44                   ` Teemu Likonen
@ 2009-06-18 12:16                     ` Johannes Schindelin
  2009-06-18 13:10                       ` Jakub Narebski
  0 siblings, 1 reply; 40+ messages in thread
From: Johannes Schindelin @ 2009-06-18 12:16 UTC (permalink / raw)
  To: Teemu Likonen
  Cc: Junio C Hamano, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Hi,

On Thu, 18 Jun 2009, Teemu Likonen wrote:

> On 2009-06-18 10:29 (+0200), Johannes Schindelin wrote:
> 
> > So let's just interpret the "!" in the most common meaning, namely to 
> > add an imperative. Then it means "yes, I do want to squash". Not 
> > "squash, but oh, BTW, I want to lose the second commit message 
> > completely, and I do not want to edit the commit message either".
> 
> My main point is the "small modifier character" for squash. Perhaps
> "squash*" is better?

If you think that putting a special meaning to a special character is 
intuitive, I have to inform you that you are mistaken.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18 12:16                     ` Johannes Schindelin
@ 2009-06-18 13:10                       ` Jakub Narebski
  0 siblings, 0 replies; 40+ messages in thread
From: Jakub Narebski @ 2009-06-18 13:10 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Teemu Likonen, Junio C Hamano, Nicolas Sebrecht,
	Nanako Shiraishi, John Tapsell, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> On Thu, 18 Jun 2009, Teemu Likonen wrote:
> 
> > On 2009-06-18 10:29 (+0200), Johannes Schindelin wrote:
> > 
> > > So let's just interpret the "!" in the most common meaning, namely to 
> > > add an imperative. Then it means "yes, I do want to squash". Not 
> > > "squash, but oh, BTW, I want to lose the second commit message 
> > > completely, and I do not want to edit the commit message either".
> > 
> > My main point is the "small modifier character" for squash. Perhaps
> > "squash*" is better?
> 
> If you think that putting a special meaning to a special character is 
> intuitive, I have to inform you that you are mistaken.

Nice bike-shedding... But UI is hard to change later, usually.

Yet another proposition would be to simply remove subject to mark
commit to be squashed without adding commit message to squashed result
commit message

    pick aaaa ...
    squash bbbb
    pick cccc ...
    pick dddd ...

 Just my 2 eurocents.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:29                 ` Johannes Schindelin
  2009-06-18  8:44                   ` Teemu Likonen
@ 2009-06-18 14:04                   ` John Koleszar
  1 sibling, 0 replies; 40+ messages in thread
From: John Koleszar @ 2009-06-18 14:04 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Teemu Likonen, Junio C Hamano, Nicolas Sebrecht,
	Nanako Shiraishi, John Tapsell, git

On Thu, 2009-06-18 at 04:29 -0400, Johannes Schindelin wrote:
> Hi,
> 
> On Thu, 18 Jun 2009, Teemu Likonen wrote:
> 
> > On 2009-06-18 10:06 (+0200), Johannes Schindelin wrote:
> > 
> > > I'd really rather stay with "fixup". And as I use single-letter
> > > commands quite often, I'd also rather stay away from that magic "!".
> > > And by "magic" I really mean that: people will not find that magic
> > > intuitive at all.
> > 
> > I don't know about people but I do find "!" intuitive. It is squash
> > after all so I like the idea of using small modifier character.
> 
> Mhm.
> 
> So let's just interpret the "!" in the most common meaning, namely to add 
> an imperative.  Then it means "yes, I do want to squash".  Not 
> "squash, but oh, BTW, I want to lose the second commit message 
> completely, and I do not want to edit the commit message either".
> 
> Really, I do not see how anybody could find this intuitive at all.  Maybe 
> after reading the manual, but kinda defeats the meaning of the word 
> "intuitive".

The imperative is actually the reason I picked that modifier, as in
"yes, I /really/ do want to squash. Don't ask me, just do it!" Something
akin to -f. I think it makes sense here, but not in the case someone
else mentioned of a commit message only edit. ("recommit" for that
case?) 

In any case, I think this non-interactive squash is orthagonal to being
able to automatically rearrange the commits by "squash to ...". I think
that's a cool idea, but I know that I often don't remember the text of
the commit I want to squash into. So in my case I prefer rearranging
manually and squashing non-interactively. If I planned ahead, I could
pick a prefix for each "class" of commit, and then "squash to prefix",
but I'd want to be able to edit the original commit to remove the
prefix. Sure, I could look at the log, but if I'm just writing a
nonsense message to remind myself where to squash to, I think it would
get in the way of my flow.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v2] rebase -i --autosquash: auto-squash commits
  2009-06-18  5:21       ` [PATCH] " Junio C Hamano
@ 2009-06-18 21:55         ` Nanako Shiraishi
  2009-06-18 22:35           ` Alex Riesen
  2009-06-19 23:07           ` Wincent Colaiuta
  0 siblings, 2 replies; 40+ messages in thread
From: Nanako Shiraishi @ 2009-06-18 21:55 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Johannes Schindelin, Nicolas Sebrecht,
	Jakub Narebski, Teemu Likonen, Matthieu Moy, Sverre Rabbelier,
	John Tapsell

Teach a new option, --autosquash, to the interactive rebase.
When the commit log message begins with "!fixup ...", and there
is a commit whose title begins with the same ..., automatically
modify the todo list of rebase -i so that the commit marked for
squashing come right after the commit to be modified, and change
the action of the moved commit from pick to squash.

This will help the use case outlined in

    From: Junio C Hamano <gitster@pobox.com>
    Date: Wed, 17 Jun 2009 09:33:19 -0700
    Subject: Re: git rebase --interactive squash/squish/fold/rollup
    Message-ID: <7vvdmurfao.fsf@alter.siamese.dyndns.org>

and further explained in

    From: Junio C Hamano <gitster@pobox.com>
    Date: Thu, 18 Jun 2009 00:54:47 -0700
    Subject: Re: [PATCH] rebase -i: auto-squash commits
    Message-ID: <7vws7ayo1k.fsf@alter.siamese.dyndns.org>

Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
---

Changes from my yesterday's patch are as follows.

 * The feature is disabled by default; the user needs to explicitly ask for it with --autosquash option.
 * Squashing more than one commits to the same commit should work.
 * The commit message must begin with a more magic string "!fixup" instead of "squash to".
 * Commands in the test script are joined with &&.
 * The test examines the content of the file to verify that the commit was correctly squashed.
 * Add documentation.

 Documentation/git-rebase.txt |    9 +++++++++
 git-rebase--interactive.sh   |   35 +++++++++++++++++++++++++++++++++++
 t/t3414-rebase-autosquash.sh |   37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 0 deletions(-)
 create mode 100755 t/t3414-rebase-autosquash.sh

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 26f3b7b..0c2f99e 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -293,6 +293,15 @@ OPTIONS
 	root commits will be rewritten to have <newbase> as parent
 	instead.
 
+--autosquash::
+	When the commit log message begins with "!fixup ...", and there
+	is a commit whose title begins with the same ..., automatically
+	modify the todo list of rebase -i so that the commit marked for
+	squashing come right after the commit to be modified, and change
+	the action of the moved commit from pick to squash.
++
+This option is only valid when '--interactive' option is used.
+
 include::merge-strategies.txt[]
 
 NOTES
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f96d887..6e223d5 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -28,6 +28,7 @@ abort              abort rebasing process and restore original branch
 skip               skip current patch and continue rebasing process
 no-verify          override pre-rebase hook from stopping the operation
 root               rebase all reachable commmits up to the root(s)
+autosquash         automatically squash commits that begin with !fixup
 "
 
 . git-sh-setup
@@ -46,6 +47,7 @@ ONTO=
 VERBOSE=
 OK_TO_SKIP_PRE_REBASE=
 REBASE_ROOT=
+AUTOSQUASH=
 
 GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
 mark the corrected paths with 'git add <paths>', and
@@ -482,6 +484,35 @@ get_saved_options () {
 	test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
 }
 
+# Rearrange the todo list that has both "pick sha1 msg" and
+# "pick sha1 !fixup msg" appears in it so that the latter
+# comes immediately after the former, and change "pick" to
+# "squash".
+rearrange_squash () {
+	sed -n -e 's/^pick \([0-9a-f]*\) !fixup /\1 /p' "$1" >"$1.sq"
+	test -s "$1.sq" || return
+
+	used=
+	while read pick sha1 message
+	do
+		case " $used" in
+		*" $sha1 "*) continue ;;
+		esac
+		echo "$pick $sha1 $message"
+		while read squash msg
+		do
+			case "$message" in
+			"$msg"*)
+				echo "squash $squash !fixup $msg"
+				used="$used$squash "
+				;;
+			esac
+		done <"$1.sq"
+	done <"$1" >"$1.rearranged"
+
+	cat "$1.rearranged" >"$1"
+}
+
 while test $# != 0
 do
 	case "$1" in
@@ -587,6 +618,9 @@ first and then run 'git rebase --continue' again."
 	--root)
 		REBASE_ROOT=t
 		;;
+	--autosquash)
+		AUTOSQUASH=t
+		;;
 	--onto)
 		shift
 		ONTO=$(git rev-parse --verify "$1") ||
@@ -746,6 +780,7 @@ first and then run 'git rebase --continue' again."
 		fi
 
 		test -s "$TODO" || echo noop >> "$TODO"
+		test -n "$AUTOSQUASH" && rearrange_squash "$TODO"
 		cat >> "$TODO" << EOF
 
 # Rebase $SHORTREVISIONS onto $SHORTONTO
diff --git a/t/t3414-rebase-autosquash.sh b/t/t3414-rebase-autosquash.sh
new file mode 100755
index 0000000..161cab4
--- /dev/null
+++ b/t/t3414-rebase-autosquash.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='auto squash'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	echo 0 > file0 &&
+	git add . &&
+	test_tick &&
+	git commit -m "initial commit" &&
+	echo 0 > file1 &&
+	echo 2 > file2 &&
+	git add . &&
+	test_tick &&
+	git commit -m "first commit" &&
+	echo 3 > file3 &&
+	git add . &&
+	test_tick &&
+	git commit -m "second commit"
+'
+
+test_expect_success 'auto squash' '
+	echo 1 > file1 &&
+	git add -u &&
+	test_tick &&
+	git commit -m "!fixup first"
+	git tag final &&
+	test_tick &&
+	git rebase --autosquash -i HEAD^^^ &&
+	git log --oneline >actual &&
+	test 3 = $(wc -l <actual) &&
+	git diff --exit-code final &&
+	test 1 = "$(git cat-file blob HEAD^:file1)"
+'
+
+test_done
-- 
1.6.2.GIT

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: git rebase --interactive squash/squish/fold/rollup
  2009-06-17 18:20     ` Clemens Buchacher
@ 2009-06-18 22:31       ` Minty
  0 siblings, 0 replies; 40+ messages in thread
From: Minty @ 2009-06-18 22:31 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, John Tapsell, Clemens Buchacher

> On Wed, Jun 17, 2009 at 09:33:19AM -0700, Junio C Hamano wrote:
>> So I can see why a variant of "squash" that does not change (nor even ask
>> for a replacement of) the commit log message from the one that is being
>> amended could be useful.
>
> How about deleting the commit message header?
> [snip]
> It requires explicit removal the unwanted commit message, avoiding any
> accidents due to ambiguous keywords.

I'm quite liking this idea, albeit a subtle feature.  Which is just fine.

>From what a quick look suggests, git-rebase--interactive.sh is the
first port of call.  Along with the documentation.

I will see what I can put together, albeit not until next week now.

Thank you all for the input.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2] rebase -i --autosquash: auto-squash commits
  2009-06-18 21:55         ` [PATCH v2] rebase -i --autosquash: " Nanako Shiraishi
@ 2009-06-18 22:35           ` Alex Riesen
  2009-06-19 23:07           ` Wincent Colaiuta
  1 sibling, 0 replies; 40+ messages in thread
From: Alex Riesen @ 2009-06-18 22:35 UTC (permalink / raw)
  To: Nanako Shiraishi
  Cc: git, Junio C Hamano, Johannes Schindelin, Nicolas Sebrecht,
	Jakub Narebski, Teemu Likonen, Matthieu Moy, Sverre Rabbelier,
	John Tapsell

2009/6/18 Nanako Shiraishi <nanako3@lavabit.com>:
> Teach a new option, --autosquash, to the interactive rebase.
> When the commit log message begins with "!fixup ...", and there

Can I suggest to rename it into "--autofixup"? Or even "--auto=!fixup"?
Just so that people have one thing less to remember.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH] Re: rebase -i: auto-squash commits
  2009-06-18  8:33                 ` Johannes Schindelin
  2009-06-18  8:44                   ` Michael J Gruber
@ 2009-06-19  7:18                   ` Miles Bader
  1 sibling, 0 replies; 40+ messages in thread
From: Miles Bader @ 2009-06-19  7:18 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Nicolas Sebrecht, Nanako Shiraishi, John Tapsell, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Okay, I'll add another point that should convince you that the commit 
> message is not the good place to trigger that behavior:
>
> Interactive rebasing is about having made a quite messy patch series, 
> maybe having a few fixup commits, and then deciding how to clean it up.
>
> The decision how to clean it up is very much a rebase-time decision, not a 
> commit-time decision.

I agree.

Magic commit messages are not good for this kind of thing.

-Miles

-- 
Do not taunt Happy Fun Ball.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2] rebase -i --autosquash: auto-squash commits
  2009-06-18 21:55         ` [PATCH v2] rebase -i --autosquash: " Nanako Shiraishi
  2009-06-18 22:35           ` Alex Riesen
@ 2009-06-19 23:07           ` Wincent Colaiuta
  2009-06-20  1:46             ` Nanako Shiraishi
  1 sibling, 1 reply; 40+ messages in thread
From: Wincent Colaiuta @ 2009-06-19 23:07 UTC (permalink / raw)
  To: Nanako Shiraishi
  Cc: git, Junio C Hamano, Johannes Schindelin, Nicolas Sebrecht,
	Jakub Narebski, Teemu Likonen, Matthieu Moy, Sverre Rabbelier,
	John Tapsell

El 18/6/2009, a las 23:55, Nanako Shiraishi escribió:

> Teach a new option, --autosquash, to the interactive rebase.
> When the commit log message begins with "!fixup ...", and there
> is a commit whose title begins with the same ..., automatically
> modify the todo list of rebase -i so that the commit marked for
> squashing come right after the commit to be modified, and change
> the action of the moved commit from pick to squash.
>
> This will help the use case outlined in
>
>    From: Junio C Hamano <gitster@pobox.com>
>    Date: Wed, 17 Jun 2009 09:33:19 -0700
>    Subject: Re: git rebase --interactive squash/squish/fold/rollup
>    Message-ID: <7vvdmurfao.fsf@alter.siamese.dyndns.org>

Definitely a fairly common workflow for me. Faced with a sequence like  
this:

	[1/3] Cleanup
	[2/3] Lay groundwork
	[3/3] Implement feature
	[4/4] Doh! more cleanup that should have gone in [1/3]

I usually just let 4/4 stand as a separate commit with a message like:

	More cleanup of XYZ

	Ideally this should have been included in commit abcd1234,
	but wasn't noticed until too late.

Seeing as I'm not perfect, I don't necessarily spend time manipulating  
the history to make it appear that I really am perfect.

Even so, if asked to imagine an ideal workflow for this scenario, I  
don't really want a new switch for "git rebase -i", but rather the  
ability to do "git commit --amend" on a non-head commit. (I know this  
has come up on the list back in February under the subject "FEATURE  
suggestion git commit --amend <ref>".)

Basically, if you do the following:

	edit
	git add foo
	git commit -m "Cleanup"
	edit
	git add foo
	git commit -m "Lay groundwork"
	edit
	git add foo
	git commit -m "Implement feature"
	# doh! found stuff that should have gone in in step one!
	edit
	git add foo
	git commit --amend HEAD~3

My intention would be for git to actually:

	1. Create a temporary throw-away commit (without updating the HEAD)

	2. Do the equivalent of using "git rebase -i" to squash that  
temporary commit into the HEAD~3 commit, providing you with the  
opportunity to edit the adjust the commit message if necessary.

	3. In the event of failure to replay the other commits on top, you  
would want the process to dump you back where you started (same HEAD  
as before, with same changes staged in the index) and an error message  
informing you that the changes didn't apply cleanly and that you  
should use "git rebase -i" instead to walk through the process manually.

At least for me that would be the ideal interface to this kind of  
feature. I can't really see myself using these magic commit messages  
and the --autosquash switch.

However, the "FEATURE suggestion git commit --amend <ref>" thread  
caused a lot of objections to be raised. Things like:

	- what if <ref> is a merge?

	- what if there are merges between <ref> and the current HEAD?

	- what if the amendment breaks reapplication of later commits?

	- what if <ref> is not an ancestor of the current HEAD?

	- what if <ref> is part of more than one branch? (and would the user  
be confused if it was only rewritten on one branch?)

Basically as I see it, the kind of workflow being discussed here  
should only be for the simple case of amending really simple histories  
(basic topic branches) and should bail loudly if pretty much any of  
the above conditions are true.

Cheers,
Wincent

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2] rebase -i --autosquash: auto-squash commits
  2009-06-19 23:07           ` Wincent Colaiuta
@ 2009-06-20  1:46             ` Nanako Shiraishi
  0 siblings, 0 replies; 40+ messages in thread
From: Nanako Shiraishi @ 2009-06-20  1:46 UTC (permalink / raw)
  To: Wincent Colaiuta
  Cc: git, Junio C Hamano, Johannes Schindelin, Nicolas Sebrecht,
	Jakub Narebski, Teemu Likonen, Matthieu Moy, Sverre Rabbelier,
	John Tapsell

Quoting Wincent Colaiuta <win@wincent.com>:

> El 18/6/2009, a las 23:55, Nanako Shiraishi escribió:
> ...
>> This will help the use case outlined in
>>
>>    From: Junio C Hamano <gitster@pobox.com>
>>    Date: Wed, 17 Jun 2009 09:33:19 -0700
>>    Subject: Re: git rebase --interactive squash/squish/fold/rollup
>>    Message-ID: <7vvdmurfao.fsf@alter.siamese.dyndns.org>
>
> Definitely a fairly common workflow for me. Faced with a sequence like
> this:
>
> 	[1/3] Cleanup
> 	[2/3] Lay groundwork
> 	[3/3] Implement feature
> 	[4/4] Doh! more cleanup that should have gone in [1/3]
>
> I usually just let 4/4 stand as a separate commit with a message like:
>
> 	More cleanup of XYZ
>
> 	Ideally this should have been included in commit abcd1234,
> 	but wasn't noticed until too late.
>
> Seeing as I'm not perfect, I don't necessarily spend time manipulating
> the history to make it appear that I really am perfect.

I don't think it is about pretending to be perfect.
If you are preparing a patch series to be reviewed, it is a minimum required courtesy to the reviewers to remove such earlier mistakes before submitting.
It is called "making your series presentable."

> Even so, if asked to imagine an ideal workflow for this scenario, I
> don't really want a new switch for "git rebase -i", but rather the
> ability to do "git commit --amend" on a non-head commit. (I know this
> has come up on the list back in February under the subject "FEATURE
> suggestion git commit --amend <ref>".)

I think you didn't read the explanation by Junio (the second message I quoted) why that is only one of the options, and isn't a satisfying solution for him. He explicitly said that he doesn't want his momentum disrupted by having to go back before he finishes the series, while admitting that the way you suggest may fit other people's workflow better.

As to the extra option, I don't like it, either (my original patch didn't have it). I added it only because Johannes Schindelin objected to the patch that the feature can trigger unexpectedly.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2009-06-20  1:48 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-17 12:06 git rebase --interactive squash/squish/fold/rollup Minty
2009-06-17 12:55 ` John Tapsell
2009-06-17 13:45   ` Minty
2009-06-17 16:33   ` Junio C Hamano
2009-06-17 16:40     ` John Tapsell
2009-06-17 16:48     ` Paolo Bonzini
2009-06-17 17:05     ` John Koleszar
2009-06-17 20:50       ` John Tapsell
2009-06-17 18:20     ` Clemens Buchacher
2009-06-18 22:31       ` Minty
2009-06-17 21:33     ` [PATCH] rebase -i: auto-squash commits Nanako Shiraishi
2009-06-17 22:08       ` Johannes Schindelin
2009-06-18  0:11         ` [PATCH] " Nicolas Sebrecht
2009-06-18  5:07           ` Junio C Hamano
2009-06-18  8:06             ` Johannes Schindelin
2009-06-18  8:11               ` Jakub Narebski
2009-06-18  8:21                 ` Junio C Hamano
2009-06-18  8:26                   ` Johannes Schindelin
2009-06-18  8:17               ` Teemu Likonen
2009-06-18  8:29                 ` Johannes Schindelin
2009-06-18  8:44                   ` Teemu Likonen
2009-06-18 12:16                     ` Johannes Schindelin
2009-06-18 13:10                       ` Jakub Narebski
2009-06-18 14:04                   ` John Koleszar
2009-06-18  8:20               ` Junio C Hamano
2009-06-18  8:33                 ` Johannes Schindelin
2009-06-18  8:44                   ` Michael J Gruber
2009-06-19  7:18                   ` Miles Bader
2009-06-18 11:18                 ` Nicolas Sebrecht
2009-06-18  8:34               ` Matthieu Moy
2009-06-18  8:44                 ` Johannes Schindelin
2009-06-18  8:59                   ` Matthieu Moy
2009-06-18 10:59             ` Nicolas Sebrecht
2009-06-18  5:21       ` [PATCH] " Junio C Hamano
2009-06-18 21:55         ` [PATCH v2] rebase -i --autosquash: " Nanako Shiraishi
2009-06-18 22:35           ` Alex Riesen
2009-06-19 23:07           ` Wincent Colaiuta
2009-06-20  1:46             ` Nanako Shiraishi
2009-06-18  7:20       ` [PATCH] rebase -i: " Michael Haggerty
2009-06-18  7:54         ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.