Re: [PATCH] completion: fix prompt with unset SHOWCONFLICTSTATE in nounset mode

["Ville Skyttä" <ville.skytta@iki.fi>]

On Mon, 1 Apr 2024 at 15:31, Junio C Hamano <gitster@pobox.com> wrote:
>
> Ville Skyttä <ville.skytta@iki.fi> writes:
>
> > `GIT_PS1_SHOWCONFLICTSTATE` is a user variable that might not be set,
> > causing errors when the shell is in `nounset` mode.
> >
> > Take into account on access by falling back to an empty string.
> >
> > Signed-off-by: Ville Skyttä <ville.skytta@iki.fi>
> > ---
>
> Obviously a good thing to do.
>
> A related tangent is that
>
>     $ git grep -e '$GIT_PS1' -e '${GIT_PS1_[A-Z0-9_]*}' contrib/completion/
>
> shows a hit for the line with SHOWCONFLICTSTATE, plus two lines with
> GIT_PS1_SHOWUPSTREAM that lack the "if unset then use this value".
> Do you want to do another patch to fix them, or are they good as-is
> for some reason?

I initially actually changed those very lines too when working on the
fix for the issue I faced with GIT_PS1_SHOWCONFLICTSTATE. However,
both occurrences are within __git_ps1_show_upstream, and the only call
site for that function is protected by a check on the variable that
does take possible unset state into account; the function will in the
file's current form never be called with it unset. Additionally, the
first occurrence is immediately following a line that sets the
variable, so that one is "doubly protected".

Therefore, I decided to undo those changes and not include them here.
I guess it's a matter of taste whether one finds it desirable to
protect those accesses nevertheless, but it's not strictly necessary.

Ville