Re: [PATCH] git-rebase.sh: handle keep-empty like all other options

Hi Elijah

On 11/06/18 15:42, Elijah Newren wrote:

Hi Phillip,

On Sun, Jun 10, 2018 at 12:26 PM, Phillip Wood
[off-list ref] wrote:

quoted

On 07/06/18 06:07, Elijah Newren wrote:

quoted

Signed-off-by: Elijah Newren <redacted>
---
   git-rebase.sh | 6 +-----
   1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 40be59ecc4..a56b286372 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh

@@ -276,6 +276,7 @@ do
                 ;;
         --keep-empty)
                 keep_empty=yes
+               test -z "$interactive_rebase" &&

interactive_rebase=implied

I think you need to wait until all the options have been parsed before
setting the implied interactive rebase in case the user specifies has
'--keep-empty' in an alias and specifies '--no-keep-empty' with some am
options on the command line.

Ah, indeed you are right.  Let's drop this patch then.

However, we have a bigger problem with empty commits, in that there
are really three modes rather than two:
   * Automatically drop empty commits (default for am-based rebase)
   * Automatically keep empty commits (as done with --keep-empty)
   * Halt the rebase and tell the user how to specify if they want to
keep it (default for interactive rebases)

Currently, only the first option is available to am-based rebases, and
only the second two options are available to interactive-based
rebases.  

I'm not sure that's the case, my understanding is that for an 
interactive rebase unless you give '--keep-empty' then any empty commits 
will be commented out in the todo list and therefore dropped unless 
they're uncommented when editing the list. The third case happens when a 
commit becomes empty when it's rebased (i.e. the original commit is not 
empty), I'm not sure what the am backend does for this. cherry-pick has 
'--keep-redundant-commits' for this case but that has never been added 
to rebase.

Best Wishes

Phillip

But if we want to make all three available to

interactive-based rebases, what should the command line option look
like?  --empty={drop,ask,keep}?

(And deprecate but continue to support --[no-]keep-empty?)

And should the two rebase modes really have a different default?  What
should the default be?