Glen Choo <chooglen@xxxxxxxxxx> writes:
> 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@xxxxxxxxx> 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.
Scratch that, I got confused. A user can't invoke "git submodule update
--update=foo" because that's not allowed by git-submodule.sh.
So this version doesn't actually introduce any user-facing oddity
unless they're invoking "git submodule--helper update" (which they
really shouldn't, and we introduced it very recently anyway), and I
think this makes it better to keep these patches separate instead of
squashing them.
