Hi Brian
On 19/01/2024 05:59, brianmlyles@xxxxxxxxx wrote:
From: Brian Lyles <brianmlyles@xxxxxxxxx>
Both of these pages document very similar `--empty` options, but with
different styles. This commit aims to make them more consistent.
I think that's reasonable though the options they are worded as doing
different things. For "am" it talks about the patch being empty - i.e. a
patch of an empty commit whereas for "rebase" the option applies to
non-empty commits that become empty. What does "am" do if you try to
apply a patch whose changes are already present?
If you're aiming for consistency then it would be worth listing the
possible values in the same order for each command.
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index b4526ca246..3ee85f6d86 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -293,13 +293,20 @@ See also INCOMPATIBLE OPTIONS below.
How to handle commits that are not empty to start and are not
clean cherry-picks of any upstream commit, but which become
empty after rebasing (because they contain a subset of already
- upstream changes). With drop (the default), commits that
- become empty are dropped. With keep, such commits are kept.
- With ask (implied by `--interactive`), the rebase will halt when
- an empty commit is applied allowing you to choose whether to
- drop it, edit files more, or just commit the empty changes.
+ upstream changes):
++
+--
+`drop`;;
+ The empty commit will be dropped. This is the default behavior.
I think it would be clearer to say "The commit" - I found "The empty
commit" confusing as the commit that is being picked isn't empty.
+`keep`;;
+ The empty commit will be kept.
+`ask`;;
+ The rebase will halt when the empty commit is applied, allowing you to
+ choose whether to drop it, edit files more, or just commit the empty
+ changes. This option is implied when `--interactive` is specified.
Other options, like `--exec`, will use the default of drop unless
`-i`/`--interactive` is explicitly specified.
Thanks for adding a bit more detail about the default, however it looks
to me like we keep commits that become empty when --exec is specified
if (options.empty == EMPTY_UNSPECIFIED) {
if (options.flags & REBASE_INTERACTIVE_EXPLICIT)
options.empty = EMPTY_STOP;
else if (options.exec.nr > 0)
options.empty = EMPTY_KEEP;
else
options.empty = EMPTY_DROP;
}
Off the top of my head I'm not sure why or if that is a good idea.
Best Wishes
Phillip
