[PATCH] rebase -i: mark commits that begin empty in todo editor

[PATCH] rebase -i: mark commits that begin empty in todo editor

[Elijah Newren via GitGitGadget]

From: Elijah Newren <newren@gmail.com>

While many users who intentionally create empty commits do not want them
thrown away by a rebase, there are third-party tools that generate empty
commits that a user might not want.  In the past, users have used rebase
to get rid of such commits (a side-effect of the fact that the --apply
backend is not currently capable of keeping them).  While such users
could fire up an interactive rebase and just remove the lines
corresponding to empty commits, that might be difficult if the
third-party tool generates many of them.  Simplify this task for users
by marking such lines with a suffix of " # empty" in the todo list.

Suggested-by: Sami Boukortt <sami@boukortt.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
    rebase -i: mark commits that begin empty in todo editor
    
    If this isn't enough, we could talk about resurrecting --no-keep-empty
    (and making --keep-empty just exist to countermand an earlier
    --no-keep-empty), but perhaps this is good enough?

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-757%2Fnewren%2Frebase-mark-empty-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-757/newren/rebase-mark-empty-v1
Pull-Request: https://github.com/git/git/pull/757

 Documentation/git-rebase.txt | 3 ++-
 sequencer.c                  | 5 +++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index f7a6033607f..8ab0558aca2 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -620,7 +620,8 @@ commits that started empty, though these are rare in practice.  It
 also drops commits that become empty and has no option for controlling
 this behavior.
 
-The merge backend keeps intentionally empty commits.  Similar to the
+The merge backend keeps intentionally empty commits (though with -i
+they are marked as empty in the todo list editor).  Similar to the
 apply backend, by default the merge backend drops commits that become
 empty unless -i/--interactive is specified (in which case it stops and
 asks the user what to do).  The merge backend also has an
diff --git a/sequencer.c b/sequencer.c
index e528225e787..ce9fd27a878 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4656,6 +4656,9 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 			strbuf_addf(&buf, "%s %s %s", cmd_pick,
 				    oid_to_hex(&commit->object.oid),
 				    oneline.buf);
+			if (is_empty)
+				strbuf_addf(&buf, " %c empty",
+					    comment_line_char);
 
 			FLEX_ALLOC_STR(entry, string, buf.buf);
 			oidcpy(&entry->entry.oid, &commit->object.oid);
@@ -4861,6 +4864,8 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
 		strbuf_addf(out, "%s %s ", insn,
 			    oid_to_hex(&commit->object.oid));
 		pretty_print_commit(&pp, commit, out);
+		if (is_empty)
+			strbuf_addf(out, " %c empty", comment_line_char);
 		strbuf_addch(out, '\n');
 	}
 	return 0;

base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925
-- 
gitgitgadget

Re: [PATCH] rebase -i: mark commits that begin empty in todo editor

[Junio C Hamano]

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> While many users who intentionally create empty commits do not want them
> thrown away by a rebase, there are third-party tools that generate empty
> commits that a user might not want.  In the past, users have used rebase
> to get rid of such commits (a side-effect of the fact that the --apply
> backend is not currently capable of keeping them).  While such users
> could fire up an interactive rebase and just remove the lines
> corresponding to empty commits, that might be difficult if the
> third-party tool generates many of them.  Simplify this task for users
> by marking such lines with a suffix of " # empty" in the todo list.
>
> Suggested-by: Sami Boukortt <sami@boukortt.com>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>     rebase -i: mark commits that begin empty in todo editor
>     
>     If this isn't enough, we could talk about resurrecting --no-keep-empty
>     (and making --keep-empty just exist to countermand an earlier
>     --no-keep-empty), but perhaps this is good enough?

This does look like an unsatisfying substitute for the real thing,
but perhaps looking for " # empty" and turning them a drop is simple
enough?  Emacs types may do something like

    C-x h C-u M-| 
    sed -e '/ # empty$/s/^pick /drop /'
    <RET>

and vi folks have something similar that begins with a ':'.

But it would not beat just being able to say "--no-keep-empty" (or
"--discard-empty"), would it?

On the other hand, even if there were "--discard-empty" available,
there may still be two classes of empty ones (e.g. ones that were
created by third-party tools, the others that were deliberately
empty) that the user may need and want to sift through, and for such 
a use case, the marking this patch does would be very valuable.

So, from that point of view, this may not be enough, but a "throw
away all" option is not enough, either.  We'd want to have both to
serve such users better.

Thanks.

> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-757%2Fnewren%2Frebase-mark-empty-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-757/newren/rebase-mark-empty-v1
> Pull-Request: https://github.com/git/git/pull/757
>
>  Documentation/git-rebase.txt | 3 ++-
>  sequencer.c                  | 5 +++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index f7a6033607f..8ab0558aca2 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -620,7 +620,8 @@ commits that started empty, though these are rare in practice.  It
>  also drops commits that become empty and has no option for controlling
>  this behavior.
>  
> -The merge backend keeps intentionally empty commits.  Similar to the
> +The merge backend keeps intentionally empty commits (though with -i
> +they are marked as empty in the todo list editor).  Similar to the
>  apply backend, by default the merge backend drops commits that become
>  empty unless -i/--interactive is specified (in which case it stops and
>  asks the user what to do).  The merge backend also has an
> diff --git a/sequencer.c b/sequencer.c
> index e528225e787..ce9fd27a878 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4656,6 +4656,9 @@ static int make_script_with_merges(struct pretty_print_context *pp,
>  			strbuf_addf(&buf, "%s %s %s", cmd_pick,
>  				    oid_to_hex(&commit->object.oid),
>  				    oneline.buf);
> +			if (is_empty)
> +				strbuf_addf(&buf, " %c empty",
> +					    comment_line_char);
>  
>  			FLEX_ALLOC_STR(entry, string, buf.buf);
>  			oidcpy(&entry->entry.oid, &commit->object.oid);
> @@ -4861,6 +4864,8 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
>  		strbuf_addf(out, "%s %s ", insn,
>  			    oid_to_hex(&commit->object.oid));
>  		pretty_print_commit(&pp, commit, out);
> +		if (is_empty)
> +			strbuf_addf(out, " %c empty", comment_line_char);
>  		strbuf_addch(out, '\n');
>  	}
>  	return 0;
>
> base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925

Re: [PATCH] rebase -i: mark commits that begin empty in todo editor

[Bryan Turner]

On Thu, Apr 9, 2020 at 5:50 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > While many users who intentionally create empty commits do not want them
> > thrown away by a rebase, there are third-party tools that generate empty
> > commits that a user might not want.  In the past, users have used rebase
> > to get rid of such commits (a side-effect of the fact that the --apply
> > backend is not currently capable of keeping them).  While such users
> > could fire up an interactive rebase and just remove the lines
> > corresponding to empty commits, that might be difficult if the
> > third-party tool generates many of them.  Simplify this task for users
> > by marking such lines with a suffix of " # empty" in the todo list.
> >
> > Suggested-by: Sami Boukortt <sami@boukortt.com>
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >     rebase -i: mark commits that begin empty in todo editor
> >
> >     If this isn't enough, we could talk about resurrecting --no-keep-empty
> >     (and making --keep-empty just exist to countermand an earlier
> >     --no-keep-empty), but perhaps this is good enough?
>
> This does look like an unsatisfying substitute for the real thing,
> but perhaps looking for " # empty" and turning them a drop is simple
> enough?  Emacs types may do something like
>
>     C-x h C-u M-|
>     sed -e '/ # empty$/s/^pick /drop /'
>     <RET>
>
> and vi folks have something similar that begins with a ':'.
>
> But it would not beat just being able to say "--no-keep-empty" (or
> "--discard-empty"), would it?
>
> On the other hand, even if there were "--discard-empty" available,
> there may still be two classes of empty ones (e.g. ones that were
> created by third-party tools, the others that were deliberately
> empty) that the user may need and want to sift through, and for such
> a use case, the marking this patch does would be very valuable.
>
> So, from that point of view, this may not be enough, but a "throw
> away all" option is not enough, either.  We'd want to have both to
> serve such users better.

This was why I wondered whether it might be worth extending the
--empty option to add another possible value, like "drop-all", that
would allow the caller to say they want to drop all empty
commits--both those that started out empty and those that became
empty. That fourth mode would be distinct from the existing three.
(I'm not sure what to call said mode; "drop-all" looks odd alongside
the existing "drop". I just say "drop-all" here to help illustrate the
idea.)

Re: [PATCH] rebase -i: mark commits that begin empty in todo editor

[Junio C Hamano]

Bryan Turner <bturner@atlassian.com> writes:

>> So, from that point of view, this may not be enough, but a "throw
>> away all" option is not enough, either.  We'd want to have both to
>> serve such users better.
>
> This was why I wondered whether it might be worth extending the
> --empty option to add another possible value, like "drop-all", that
> would allow the caller to say they want to drop all empty
> commits--both those that started out empty and those that became
> empty.

You are talking about "empty from the beginning" and "has become
empty due to rebasing", but the use case you quoted and responding
to is not about the distinction between those two.

The original scenario that triggered this thread was to clean a
history the user created with git-imerge.  Part of using the tool,
apparently, you can choose to leave empty commits the tool
internally needs to create.  To the "git rebase" command that is
tasked to clean up the history, an empty commit that was left due to
what imerge did, and an empty commit that was originally created by
the end user (perhaps deliberately in order to server as some sort
of a marker) look the same---they both are "empty from the
beginning", not "commits that became no-op as the result of
rebasing".

And in order to help manually sift these two apart, " # empty"
marker would help, as there are three kinds of "pick" in the
instruction stream.  Commits that are not no-op (i.e. without the 
" # empty" mark), commits that imerge made no-op (the user wants
to get rid of) and commits that are no-op because the user wanted
them to be.  The latter two classes would be marked with " # empty"
and the user can selectively apply s/pick/drop/ to them.

Re: [PATCH] rebase -i: mark commits that begin empty in todo editor

[Lia Rosas]

Sent from my iPhone