Re: [PATCH 0/4] In PS1 prompt, make upstream state indicators consistent with other state indicators

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 25, 2022, 12:22 UTC
Message-ID: <220225.86czjb874f.gmgdl@evledraar.gmail.com>
In-Reply-To: <pull.1162.git.1645789446.gitgitgadget@gmail.com>

On Fri, Feb 25 2022, Justin Donnelly via GitGitGadget wrote:

I couldn't find any glaring issues here on a quick review, just a note.

Show 7 quoted lines

> These patches are about the characters and words that can be configured to
> display in the PS1 prompt after the branch name. I've been unable to find a
> consistent terminology. I refer to them as follows: [short | long] [type]
> state indicator where short is for characters (e.g. ?), long is for words
> (e.g. |SPARSE), and type is the type of indicator (e.g. sparse or upstream).
> I'd be happy to change the commit messages to a different terminology if
> that's preferred.

I think that terminology is correct, in case you haven't seen it git-for-each-ref(1) talks about the "short" here as "short", "trackshort" etc.

Show 12 quoted lines

> There are a few inconsistencies with the PS1 prompt upstream state indicator
> (GIT_PS1_SHOWUPSTREAM).
>
>  * With GIT_PS1_SHOWUPSTREAM="auto", if there are no other short state
>    indicators (e.g. + for staged changes, $ for stashed changes, etc.), the
>    upstream state indicator appears adjacent to the branch name (e.g.
>    (main=)) instead of being separated by SP or GIT_PS1_STATESEPARATOR (e.g.
>    (main =)).
>  * If there are long state indicators (e.g. |SPARSE), a short upstream state
>    indicator (i.e. GIT_PS1_SHOWUPSTREAM="auto") is to the right of the long
>    state indicator (e.g. (main +|SPARSE=)) instead of with the other short
>    state indicators (e.g. (main +=|SPARSE)).

I think it would really help to in each commit message have a before/after comparison of the relevant PS1 output that's being changed.

I'm not sure how to readthis example. So before we said "main +=|SPARSE" but now we'll say "main +|SPARSE=", but without sparse we'll say "main="?

Aren't both of those harder to read than they need to be, shouldn't it be closer to:

    main= |SPARSE

Or:

    main= |+SPARSE

Or:

    main= +|SPARSE

I can't recall what the "+" there is (if any).

I.e. the "=" refers to the ahead/behind state of "main, it seems odd in both versions of your example that we're splitting it off from "main" because we have "SPARSE" too.

But maybe I'm missing something...