Re: [PATCH] rebase -i: permit 'drop' of a merge commit
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Aug 7, 2025, 13:50 UTC
- Message-ID
- <7c8b1886-e5cb-420a-894a-f0434a766117@gmail.com>
- In-Reply-To
- <xmqqjz3gtb4w.fsf@gitster.g>
On 06/08/2025 22:04, Junio C Hamano wrote:
Show 9 quoted lines
> Johannes Sixt <j6t@kdbg.org> 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
Show 29 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
>