Message ID | pull.757.git.git.1586474800276.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | rebase -i: mark commits that begin empty in todo editor | expand |
"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
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.)
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.
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;