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.
> 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.
> 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...
