Re: [PATCH 1/3] submodule: clean up subsititions in script
- From
Casey Fitzpatrick <kcghost@gmail.com>
- Date
- May 3, 2018, 10:46 UTC
- Message-ID
- <CAEp-SHXPKJNuc+dhkZu=Vr=ju8AMHq6Rd851vNOV3WUvOQnrLg@mail.gmail.com>
- In-Reply-To
- <xmqqy3h2vbqj.fsf@gitster-ct.c.googlers.com>
Thanks I will amend and re-submit (this time with -v$N, I apologize for creating a confusing mess in everyone's email clients :))
On Wed, May 2, 2018 at 1:59 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 34 quoted lines
> Casey Fitzpatrick <kcghost@gmail.com> writes:
>
>> 'recommend_shallow' and 'jobs' variables do not need quotes (they never contain
>> spaces) and do not require any additional prefix, therefore remove the
>> unnecessary subsitition.
>
> The resulting patch is good, but "they never contain spaces" is not
> a very good rationale. The real reason is that (1) we use them only
> to hold a single token value (or leave them empty) in the current
> code, and (2) if the feature they represent is enhanced in the
> future to make them multi-token options (e.g. we may allow $jobs to
> contain, in addition to "--jobs=2", "--jobs 2" for whatever reason
> later), it is likely that we would want these multi-tokens split at
> $IFS (e.g. "--jobs" and "2" get passed as separate option, not a
> single "--jobs 2" string).
>
>> 'progress' is a boolean value. Treat it like the other boolean values in the
>> script by using a substitution.
>
> This is OK.
>
>> git submodule--helper update-clone ${GIT_QUIET:+--quiet} \
>> - ${progress:+"$progress"} \
>> + ${progress:+"--progress"} \
>> ${wt_prefix:+--prefix "$wt_prefix"} \
>> ${prefix:+--recursive-prefix "$prefix"} \
>> ${update:+--update "$update"} \
>> ${reference:+"$reference"} \
>> ${depth:+--depth "$depth"} \
>> - ${recommend_shallow:+"$recommend_shallow"} \
>> - ${jobs:+$jobs} \
>> + $recommend_shallow \
>> + $jobs \
>