On Thursday, August 15, 2024 at 07:36:43 PM GMT+3, Junio C Hamano <gitster@xxxxxxxxx> 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.
