Re: [PATCH 3/3] Revert "bash prompt: avoid command substitution when finalizing gitstring"

From: Brandon Casey <drafnel@gmail.com>
Date: Aug 22, 2013, 00:33 UTC
Message-ID: <CA+sFfMfa422PF1inOOeTBRE7HRqL5zwCJNagx9Ya0i_LbpwQcg@mail.gmail.com>
In-Reply-To: <xmqq7gfesheu.fsf@gitster.dls.corp.google.com>

On Wed, Aug 21, 2013 at 5:22 PM, Junio C Hamano <gitster@pobox.com> wrote:

> Brandon Casey <drafnel@gmail.com> writes:
>
>> On Wed, Aug 21, 2013 at 2:47 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 16 quoted lines

>>>         # on load...
>>>         printf -v __git_printf_supports_v -- "%s" yes >/dev/null 2>&1
>>>
>>>         ...
>>>
>>>         if test "${__git_printf_supports_v}" = yes
>>>         then
>>>                 printf -v gitstring -- "$printf_format" "$gitstring"
>>>         else
>>>                 gitstring=$(printf -- "$printf_format" "$gitstring")
>>>         fi
>>
>> Yes, that appears to work.
>
> A real patch needs to be a bit more careful, though.  The variable
> needs to be cleared before all of the above,

Agreed.

> and the testing would
> want to consider that the variable may not be set (i.e. use
> "${var-}" when checking).

Why is "${var-}" necessary? Wouldn't that be equivalent to "${var}" or "$var"? We obviously wouldn't want to do 'if test $var = yes', but I would have thought it was sufficient to wrap the variable dereference in quotes as your original did.

-Brandon