Re: [PATCH v2 5/8] git-prompt: add some missing quotes

[avih <avihpit@yahoo.com>]

 On Thursday, August 15, 2024 at 07:36:43 PM GMT+3, Junio C Hamano <gitster@pobox.com> wrote:
>> > Still in _agumemts_, no need to quote non-expandable values:
>>
> arguments.

Thanks. Will fix in v3 (after more comments unless asked otherwise).

>> -    local bad_color=$c_red
>> +    local bad_color="$c_red"
>
> Good.  I think we in the past was burned by some shells that want to
> see these assignments with "local" always quoted.

Yes. After I reached the same conclusion I noticed it was also added
to CodingGuidelines not long ago at be34b510 (CodingGuidelines: quote
assigned value in 'local var=$val').

>>      # preserve exit status
>> -    local exit=$?
>> +    local exit="$?"
>
> Well no matter what value $? has, it by definition has a few digits
> without any $IFS funnies.  Does this really matter?  I'd imagine
> that we would prefer to treat "$?" exactly the same way as "no".
>
> -        *)    return $exit
> +        *)    return "$exit"
>
> Likewise.

Two things here:

1. It can matter, because we don't control IFS. __git_ps1 is
   a function which runs in the user's shell, so if the user did
   IFS=0123, then unquoted $? or $exit can get IFS-split.
   As the commit message notes, this is unlikely to fix things in
   practice, but it will fix things with weird IFS values.

2. In general, yes, $? is only needed as yes/no, and there's only
   one place which tests $? instead of using "&&" or "||" after
   a command in this file (rev_parse_exit_code="$?"). I didn't feel
   this needs any portability fix. It works.

   But with $exit, $? is not used as yes/no, but rather to preserve
   the exit status when __git_ps1 was entered. This is important if
   the user wants the shell's last command $? at the prompt, e.g.:

   PS1='\w$(__git_ps1)$(e=$?; [ "$e" = 0 ] || echo " E:$e") \$ '

   If __git_ps1 didn't exit with the same $? it saw on entry, then
   $e will be __git_ps1's exit code rather than the exit code of
   the last command which ran in the shell, so it should be the
   same value as before and not only yes/no.