From: Phillip Wood Date: Thu, 07 Aug 2025 13:50:30 GMT Subject: Re: [PATCH] rebase -i: permit 'drop' of a merge commit Message-ID: <7c8b1886-e5cb-420a-894a-f0434a766117@gmail.com> In-Reply-To: On 06/08/2025 22:04, Junio C Hamano wrote: > Johannes Sixt writes: > > 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. I think there are definitely cases like this where it makes sense to require the case statements to be exhaustive. Looking at the documentation for -Wswitch [1] which is enabled by -Wall it only issues a warning when there is no default arm and the case statements are non-exhaustive. So I think we could start relying on that just by deleting the default arms where we think it makes sense for the case statements to be exhaustive. I've previously worked on a code base that enabled -Wswitch-enum which requires the case statements to be exhaustive even if there is a default arm and that was a pain in the neck. Thanks Phillip [1] https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Warning-Options.html#index-Wswitch >> 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 >