Re: [PATCH] rebase -i: permit 'drop' of a merge commit
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 6, 2025, 21:04 UTC
- Message-ID
- <xmqqjz3gtb4w.fsf@gitster.g>
- In-Reply-To
- <37f6e34c-91aa-4e55-88e1-019d2e042df3@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
Show 13 quoted lines
> 4c063c82e9 (rebase -i: improve error message when picking merge, > 2024-05-30) added advice texts for cases when a merge commit is > passed as argument of sequencer command that cannot operate with > a merge commit. However, it forgot about the 'drop' command, so > that in this case the BUG() in the default branch is reached. > > Handle 'drop' like 'merge', i.e., permit it without a message. > > Signed-off-by: Johannes Sixt <j6t@kdbg.org> > --- > sequencer.c | 1 + > t/t3404-rebase-interactive.sh | 1 + > 2 files changed, 2 insertions(+)
Thanks. Now I understand why some people are sometimes tempted to omit the default arm in switch() and allow compilers complain when explicit case arms are not exhaustive. I am not saying we should do so, and I am not convinced that it is a good idea (there are cases you cannot afford to be exhausitive, yet the cases your particular switch must care about are multiple to make an if/else if cascade impractical). But this is one of the case it might make sense.
Show 28 quoted lines
> diff --git a/sequencer.c b/sequencer.c
> index aaf2e4df64..9ae40a91b2 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2720,8 +2720,9 @@ static int check_merge_commit_insn(enum todo_command command)
> case TODO_SQUASH:
> return error(_("cannot squash merge commit into another commit"));
>
> case TODO_MERGE:
> + case TODO_DROP:
> return 0;
>
> default:
> BUG("unexpected todo_command");
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 6bac217ed3..34d6ad0770 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -2262,8 +2262,9 @@ rebase_setup_and_clean () {
> reword $oid
> edit $oid
> fixup $oid
> squash $oid
> + drop $oid # acceptable, no advice
> EOF
> (
> set_replace_editor todo &&
> test_must_fail git rebase -i HEAD 2>actual