Re: [PATCH v6 4/4] git-rebase: add keep_empty flag

On Wed, Apr 18, 2012 at 03:58:53PM -0700, Junio C Hamano wrote:

Neil Horman [off-list ref] writes:

quoted

Add a command line switch to git-rebase to allow a user the ability to specify
that they want to keep any commits in a series that are empty.

When git-rebase's type is am, then this option will automatically keep any
commit that has a tree object identical to its parent.

This patch changes the default behavior of interactive rebases as well.  With
this patch, git-rebase -i will produce a revision set passed to
git-revision-editor, in which empty commits are commented out.  Empty commits
may be kept manually by uncommenting them.  If the new --keep-empty option is
used in an interactive rebase the empty commits will automatically all be
uncommented in the editor.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---

The earlier one in the series seem to be getting solid enough.  Nice.

Thanks!

quoted

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 5812222..cef290b 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh

@@ -167,6 +167,12 @@ has_action () {
 	sane_grep '^[^#]' "$1" >/dev/null
 }
 
+is_empty_commit() {
+	tree=$(git rev-parse "$1"^{tree})
+	ptree=$(git rev-parse "$1"^^{tree})
+	return $(test "$tree" = "$ptree")
+}

Could "$1" ever be a root commit without a parent?

Strictly speaking, yes.  If that happens, however, the output of git rev-parse
will be an error message that includes the passed in revision.  since tree
passes '^' while ptree passes '^^' the two revisions will always differ, and as
a result is_empty_commit will return false, and the existing behavior of git
will be followed.  So, yes, rebasing an empty tree would cause odd output in the
rev-parse calls in is_empty_comit, but the behavior of git overall would be
unaffected (which is not to say that rebasing an empty tree won't show odd
corner-case behavior, only that this changeset won't introduce any new odd
corner case behavior :) ).

If you like we can add additional checking to ensure that we explicitly catch
rev-parse errors and abort the rebase imediately, but I don't think thats
strictly necessecary.  Would you prefer that?

Regards
Neil