Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints
Thanks for working on this.
On Wed, Aug 6, 2014 at 7:59 PM, Fabian Ruch baf...@gmail.com wrote:
The to-do list commands `squash` and `fixup` apply the changes
introduced by the named commit to the tree but instead of creating
a new commit on top of the current head it replaces the previous
commit with a new commit that records the updated tree. If the
result is an empty commit git-rebase stops with the error message
You asked to amend the most recent commit, but doing so would make
it empty. You can repeat your command with --allow-empty, or you can
remove the commit entirely with git reset HEAD^.
This message is not very helpful because neither does git-rebase
support an option `--allow-empty` nor does the messages say how to
resume the rebase. Firstly, change the error message to
The squash result is empty and --keep-empty was not specified.
You can remove the squash commit now with
git reset HEAD^
Once you are down, run
s/down/done
git rebase --continue
I like the direction of this rewording, but it seems to hide the
instructions for the user who wants to keep the empty commit.
I'm not sure how to improve it. Maybe this:
The squash result is empty and --keep-empty was not specified.
You may remove the squash commit now with
git reset HEAD^
or keep it by doing nothing extra.
Continue the rebase with
git rebase --continue
If the user wishes to squash a sequence of commits into one
commit, f. i.
pick A
squash Revert A
squash A'
, it does not matter for the end result that the first squash
result, or any sub-sequence in general, is going to be empty. The
squash message is not affected at all by which commits are created
and only the commit created by the last line in the sequence will
end up in the final history. Secondly, print the error message
only if the whole squash sequence produced an empty commit.
Lastly, since an empty squash commit is not a failure to rewrite
the history as planned, issue the message above as a mere warning
and interrupt the rebase with the return value zero. The
interruption should be considered as a notification with the
chance to undo it on the spot. Specifying the `--keep-empty`
option tells git-rebase to keep empty squash commits in the
rebased history without notification.
Add tests.
Reported-by: Peter Krefting pe...@softwolves.pp.se
Signed-off-by: Fabian Ruch baf...@gmail.com
---
Hi,
Peter Krefting is cc'd as the author of the bug report Confusing
error message in rebase when commit becomes empty discussed on the
mailing list in June. Phil Hord and Jeff King both participated in
the problem discussion which ended with two proposals by Jeff.
Jeff King writes:
1. Always keep such empty commits. A user who is surprised by them
being empty can then revisit them. Or drop them by doing another
rebase without --keep-empty.
2. Notice ourselves that the end-result of the whole squash is an
empty commit, and stop to let the user deal with it.
This patch chooses the second alternative. Either way seems OK. The
crucial consensus of the discussion was to silently throw away empty
interim commits.
This patch does _not_ silently throw away empty commits. I wonder if
such behavior could be triggered with something like --no-keep-empty.
Maybe that belongs in another patch.
Fabian
git-rebase--interactive.sh| 20 +++---
t/t3404-rebase-interactive.sh | 62
+++
2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3222bf6..8820eac 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -549,7 +549,7 @@ do_next () {
squash|s|fixup|f)
# This is an intermediate commit; its message will
only be
# used in case of trouble. So use the long version:
- do_with_author output git commit
--allow-empty-message \
+ do_with_author output git commit
--allow-empty-message --allow-empty \
--amend --no-verify -F $squash_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
@@ -558,18 +558,32 @@ do_next () {
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- do_with_author git commit
--allow-empty-message \
+ do_with_author git commit
--allow-empty-message --allow-empty \
--amend --no-verify -F $fixup_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1
Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints
Hi Eric,
Eric Sunshine writes:
On Wed, Aug 6, 2014 at 7:59 PM, Fabian Ruch baf...@gmail.com wrote:
The to-do list commands `squash` and `fixup` apply the changes
introduced by the named commit to the tree but instead of creating
a new commit on top of the current head it replaces the previous
commit with a new commit that records the updated tree. If the
result is an empty commit git-rebase stops with the error message
You asked to amend the most recent commit, but doing so would make
it empty. You can repeat your command with --allow-empty, or you can
remove the commit entirely with git reset HEAD^.
This message is not very helpful because neither does git-rebase
support an option `--allow-empty` nor does the messages say how to
resume the rebase. Firstly, change the error message to
The squash result is empty and --keep-empty was not specified.
You can remove the squash commit now with
git reset HEAD^
Once you are down, run
I guess you meant: s/down/done
Same issue with the actually message in the code (below).
Fixed.
git rebase --continue
If the user wishes to squash a sequence of commits into one
commit, f. i.
pick A
squash Revert A
squash A'
, it does not matter for the end result that the first squash
result, or any sub-sequence in general, is going to be empty. The
squash message is not affected at all by which commits are created
and only the commit created by the last line in the sequence will
end up in the final history. Secondly, print the error message
only if the whole squash sequence produced an empty commit.
Lastly, since an empty squash commit is not a failure to rewrite
the history as planned, issue the message above as a mere warning
and interrupt the rebase with the return value zero. The
interruption should be considered as a notification with the
chance to undo it on the spot. Specifying the `--keep-empty`
option tells git-rebase to keep empty squash commits in the
rebased history without notification.
Add tests.
Reported-by: Peter Krefting pe...@softwolves.pp.se
Signed-off-by: Fabian Ruch baf...@gmail.com
---
Hi,
Peter Krefting is cc'd as the author of the bug report Confusing
error message in rebase when commit becomes empty discussed on the
mailing list in June. Phil Hord and Jeff King both participated in
the problem discussion which ended with two proposals by Jeff.
Jeff King writes:
1. Always keep such empty commits. A user who is surprised by them
being empty can then revisit them. Or drop them by doing another
rebase without --keep-empty.
2. Notice ourselves that the end-result of the whole squash is an
empty commit, and stop to let the user deal with it.
This patch chooses the second alternative. Either way seems OK. The
crucial consensus of the discussion was to silently throw away empty
interim commits.
Fabian
git-rebase--interactive.sh| 20 +++---
t/t3404-rebase-interactive.sh | 62
+++
2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3222bf6..8820eac 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -549,7 +549,7 @@ do_next () {
squash|s|fixup|f)
# This is an intermediate commit; its message will
only be
# used in case of trouble. So use the long version:
- do_with_author output git commit
--allow-empty-message \
+ do_with_author output git commit
--allow-empty-message --allow-empty \
--amend --no-verify -F $squash_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
@@ -558,18 +558,32 @@ do_next () {
# This is the final command of this squash/fixup
group
if test -f $fixup_msg
then
- do_with_author git commit
--allow-empty-message \
+ do_with_author git commit
--allow-empty-message --allow-empty \
--amend --no-verify -F $fixup_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG ||
exit
rm -f $GIT_DIR/MERGE_MSG
- do_with_author git commit --amend
--no-verify -F $GIT_DIR/SQUASH_MSG -e \
+ do_with_author git commit --allow-empty
--amend --no-verify -F $GIT_DIR/SQUASH_MSG -e \
${gpg_sign_opt:+$gpg_sign_opt} ||
Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints
Fabian Ruch: 2. Notice ourselves that the end-result of the whole squash is an empty commit, and stop to let the user deal with it. This patch chooses the second alternative. Either way seems OK. The crucial consensus of the discussion was to silently throw away empty interim commits. Yes, the important part is that giving good advice is better than giving bad advice. Thank you for taking your time to fix this. I haven't reviewed the changes themselves, but I am happy with the underlying idea. -- \\// Peter - http://www.softwolves.pp.se/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints
On Wed, Aug 6, 2014 at 7:59 PM, Fabian Ruch baf...@gmail.com wrote:
The to-do list commands `squash` and `fixup` apply the changes
introduced by the named commit to the tree but instead of creating
a new commit on top of the current head it replaces the previous
commit with a new commit that records the updated tree. If the
result is an empty commit git-rebase stops with the error message
You asked to amend the most recent commit, but doing so would make
it empty. You can repeat your command with --allow-empty, or you can
remove the commit entirely with git reset HEAD^.
This message is not very helpful because neither does git-rebase
support an option `--allow-empty` nor does the messages say how to
resume the rebase. Firstly, change the error message to
The squash result is empty and --keep-empty was not specified.
You can remove the squash commit now with
git reset HEAD^
Once you are down, run
I guess you meant: s/down/done
Same issue with the actually message in the code (below).
git rebase --continue
If the user wishes to squash a sequence of commits into one
commit, f. i.
pick A
squash Revert A
squash A'
, it does not matter for the end result that the first squash
result, or any sub-sequence in general, is going to be empty. The
squash message is not affected at all by which commits are created
and only the commit created by the last line in the sequence will
end up in the final history. Secondly, print the error message
only if the whole squash sequence produced an empty commit.
Lastly, since an empty squash commit is not a failure to rewrite
the history as planned, issue the message above as a mere warning
and interrupt the rebase with the return value zero. The
interruption should be considered as a notification with the
chance to undo it on the spot. Specifying the `--keep-empty`
option tells git-rebase to keep empty squash commits in the
rebased history without notification.
Add tests.
Reported-by: Peter Krefting pe...@softwolves.pp.se
Signed-off-by: Fabian Ruch baf...@gmail.com
---
Hi,
Peter Krefting is cc'd as the author of the bug report Confusing
error message in rebase when commit becomes empty discussed on the
mailing list in June. Phil Hord and Jeff King both participated in
the problem discussion which ended with two proposals by Jeff.
Jeff King writes:
1. Always keep such empty commits. A user who is surprised by them
being empty can then revisit them. Or drop them by doing another
rebase without --keep-empty.
2. Notice ourselves that the end-result of the whole squash is an
empty commit, and stop to let the user deal with it.
This patch chooses the second alternative. Either way seems OK. The
crucial consensus of the discussion was to silently throw away empty
interim commits.
Fabian
git-rebase--interactive.sh| 20 +++---
t/t3404-rebase-interactive.sh | 62
+++
2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3222bf6..8820eac 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -549,7 +549,7 @@ do_next () {
squash|s|fixup|f)
# This is an intermediate commit; its message will
only be
# used in case of trouble. So use the long version:
- do_with_author output git commit
--allow-empty-message \
+ do_with_author output git commit
--allow-empty-message --allow-empty \
--amend --no-verify -F $squash_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
@@ -558,18 +558,32 @@ do_next () {
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- do_with_author git commit
--allow-empty-message \
+ do_with_author git commit
--allow-empty-message --allow-empty \
--amend --no-verify -F $fixup_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
rm -f $GIT_DIR/MERGE_MSG
- do_with_author git commit --amend --no-verify
-F $GIT_DIR/SQUASH_MSG -e \
+ do_with_author git commit --allow-empty
--amend --no-verify -F $GIT_DIR/SQUASH_MSG -e \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1
[PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints
The to-do list commands `squash` and `fixup` apply the changes
introduced by the named commit to the tree but instead of creating
a new commit on top of the current head it replaces the previous
commit with a new commit that records the updated tree. If the
result is an empty commit git-rebase stops with the error message
You asked to amend the most recent commit, but doing so would make
it empty. You can repeat your command with --allow-empty, or you can
remove the commit entirely with git reset HEAD^.
This message is not very helpful because neither does git-rebase
support an option `--allow-empty` nor does the messages say how to
resume the rebase. Firstly, change the error message to
The squash result is empty and --keep-empty was not specified.
You can remove the squash commit now with
git reset HEAD^
Once you are down, run
git rebase --continue
If the user wishes to squash a sequence of commits into one
commit, f. i.
pick A
squash Revert A
squash A'
, it does not matter for the end result that the first squash
result, or any sub-sequence in general, is going to be empty. The
squash message is not affected at all by which commits are created
and only the commit created by the last line in the sequence will
end up in the final history. Secondly, print the error message
only if the whole squash sequence produced an empty commit.
Lastly, since an empty squash commit is not a failure to rewrite
the history as planned, issue the message above as a mere warning
and interrupt the rebase with the return value zero. The
interruption should be considered as a notification with the
chance to undo it on the spot. Specifying the `--keep-empty`
option tells git-rebase to keep empty squash commits in the
rebased history without notification.
Add tests.
Reported-by: Peter Krefting pe...@softwolves.pp.se
Signed-off-by: Fabian Ruch baf...@gmail.com
---
Hi,
Peter Krefting is cc'd as the author of the bug report Confusing
error message in rebase when commit becomes empty discussed on the
mailing list in June. Phil Hord and Jeff King both participated in
the problem discussion which ended with two proposals by Jeff.
Jeff King writes:
1. Always keep such empty commits. A user who is surprised by them
being empty can then revisit them. Or drop them by doing another
rebase without --keep-empty.
2. Notice ourselves that the end-result of the whole squash is an
empty commit, and stop to let the user deal with it.
This patch chooses the second alternative. Either way seems OK. The
crucial consensus of the discussion was to silently throw away empty
interim commits.
Fabian
git-rebase--interactive.sh| 20 +++---
t/t3404-rebase-interactive.sh | 62 +++
2 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3222bf6..8820eac 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -549,7 +549,7 @@ do_next () {
squash|s|fixup|f)
# This is an intermediate commit; its message will only
be
# used in case of trouble. So use the long version:
- do_with_author output git commit --allow-empty-message \
+ do_with_author output git commit --allow-empty-message
--allow-empty \
--amend --no-verify -F $squash_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
@@ -558,18 +558,32 @@ do_next () {
# This is the final command of this squash/fixup group
if test -f $fixup_msg
then
- do_with_author git commit --allow-empty-message
\
+ do_with_author git commit --allow-empty-message
--allow-empty \
--amend --no-verify -F $fixup_msg \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
else
cp $squash_msg $GIT_DIR/SQUASH_MSG || exit
rm -f $GIT_DIR/MERGE_MSG
- do_with_author git commit --amend --no-verify
-F $GIT_DIR/SQUASH_MSG -e \
+ do_with_author git commit --allow-empty --amend
--no-verify -F $GIT_DIR/SQUASH_MSG -e \
${gpg_sign_opt:+$gpg_sign_opt} ||
die_failed_squash $sha1 $rest
fi
rm -f $squash_msg $fixup_msg
+ if test -z $keep_empty is_empty_commit HEAD
+ then
+ echo $sha1 $state_dir/stopped-sha
+
