Re: [PATCH v3 09/12] submodule--helper: understand --checkout, --merge and --rebase synonyms

> As you noted in your cover letter, this version of 09-10/12 is a lot
> cleaner and more obviously correct.
>
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> @@ -40,7 +40,9 @@ require_init=
>>  files=
>>  remote=
>>  nofetch=
>> -update=
>> +rebase=
>> +merge=
>> +checkout=
>>  custom_name=
>>  depth=
>>  progress=
>> @@ -260,7 +262,7 @@ cmd_update()
>>       force=$1
>>       ;;
>>     -r|--rebase)
>> -			update="rebase"
>> +			rebase=1
>>       ;;
>>     --reference)
>>       case "$2" in '') usage ;; esac
>> @@ -274,13 +276,13 @@ cmd_update()
>>       dissociate=1
>>       ;;
>>     -m|--merge)
>> -			update="merge"
>> +			merge=1
>>       ;;
>>     --recursive)
>>       recursive=1
>>       ;;
>>     --checkout)
>> -			update="checkout"
>> +			checkout=1
>>       ;;
>>     --recommend-shallow)
>>       recommend_shallow="--recommend-shallow"
>> @@ -341,7 +343,9 @@ cmd_update()
>>     ${init:+--init} \
>>     ${nofetch:+--no-fetch} \
>>     ${wt_prefix:+--prefix "$wt_prefix"} \
>> -		${update:+--update "$update"} \
>> +		${rebase:+--rebase} \
>> +		${merge:+--merge} \
>> +		${checkout:+--checkout} \
>>     ${reference:+"$reference"} \
>>     ${dissociate:+"--dissociate"} \
>>     ${depth:+"$depth"} \
>
> A small inelegance is that a user could theoretically pass both
> "--update={checkout,rebase,merge}" and "--{checkout,rebase,merge}",
> where one option ends of clobbering the other (Is it last one wins?).
>
> Ideally we'd check for this kind of invalid usage and die, but maybe
> it's not worth the effort since we fix this in the next patch already?
>
> This wouldn't happen if we squashed 09-10/12 together like I initially
> suggested, but then the patches would no longer be as obviously correct.
>
> Neither seems obviously better than the other, so I'm ok with this
> either way.