Re: [PATCH v2 5/8] git-prompt: add some missing quotes
- From
avih <avihpit@yahoo.com>
- Date
- Aug 15, 2024, 17:35 UTC
- Message-ID
- <1228065843.3779090.1723743313433@mail.yahoo.com>
- In-Reply-To
- <xmqqmsldu4iu.fsf@gitster.g>
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).
Show 5 quoted lines
>> - 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').
Show 12 quoted lines
>> # 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.