Re: [PATCH 2/2] show color hints based on state of the git tree

Simon Oosthoek venit, vidit, dixit 15.10.2012 11:01:

On 10/15/2012 10:23 AM, Michael J Gruber wrote:

quoted

Sorry for being late ($DAYJOB and such), but I just noticed this is on
next already:

quoted

+			if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then

You're missing the "S" here (HINTS).

indeed, well spotted!
My test setup was apparently configured without the S and the commented 
uses all had HINTS...

quoted

quoted

+				local c_red='\e[31m'
+				local c_green='\e[32m'
+				local c_yellow='\e[33m'
+				local c_lblue='\e[1;34m'
+				local c_purple='\e[35m'
+				local c_cyan='\e[36m'
+				local c_clear='\e[0m'
+				local branchstring="$c${b##refs/heads/}"
+				local branch_color="$c_green"
+				local flags_color="$c_cyan"
+
+				if [ "$w" = "*" ]; then
+					branch_color="$c_red"
+				elif [ -n "$i" ]; then
+					branch_color="$c_yellow"
+				fi
+
+				# Setting PS1 directly with \[ and \] around colors
+				# is necessary to prevent wrapping issues!
+				PS1="$PS1\[$branch_color\]$branchstring\[$c_clear\]"
+				if [ -n "$f" ]; then
+					PS1="$PS1 \[$flags_color\]$f\[$c_clear\]"
+				fi
+			else
+				PS1="$PS1$c${b##refs/heads/}${f:+ $f}$r$p"
+			fi
+			PS1="$PS1)$ps1pc_end"
+		else
+			# NO color option unless in PROMPT_COMMAND mode
  			printf -- "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
  		fi
  	fi

I'm afraid I don't like this coloring style at all because it is
inconsistent with the color usage of "git status -sb". First of all, the
colors are different, and second, the fact *what* is colored is
different. I had suggested following "git status -sb" for a good reason.
It colors a branch green and a detached head red. It colors "change"
(A/D/M/R) as red/green depending on non-cached/cached, so that's how */+
should be. Your call for $/% (I'd leave them uncolored).[*][**]

This way works for me. The coloring scheme itself is probably quite 
personal and can, and probably should, be modified by end-users.
(Whether the current code is suitable for user-modification is another 
matter)

It really doesn't matter much what works for you, and it doesn't matter
what works for me either. The point is: What works for most users?

I'm quite unfamiliar with the color coding of git (I hadn't enabled that 
option), I suppose consistency would be better, but then you'd have to 
add some code to figure out which color is used for what in git's output 
and convert that to the code setting the colors here.

As a starter, you could use the default colors which git uses. Querying
git for the colors could be the next, optional step.

As for the characters used, I think there's a good reason not to use the 
ones git uses in the prompt. The characters in git status output are put 
in front of the files they apply to, in the prompt you only get a 
summarized output. And perhaps that argument could be extended to the 
use of the colors as well, I prefer to know whether I have uncommitted 
changes and in that category I want to know whether I already staged 
them or not.

Well, sure. The prompt could show any of AMCRD if you have any of those
changes. * + are shortcuts saying you have any of those.

If I want to know which files are unstaged/uncommitted I do git status 
to see that.

Sure. I don't propose putting file names in the prompt ;)

quoted

I think it's very confusing to have completely different schemes (not
just themes) for two versions of the same information: concise status
information.

So, please try and follow "git status -sb".

I think there are different levels of conciseness. And I see "git status 
-sb" uses green for staged modified files, which would be confusing to me.

...only because you don't know the color coding scheme. It's green
because those changes are saved somewhere (in the index) and would even
survice a branch switch.

quoted

Michael

[*] Really, there's nothin "red" about a branch when there are cached or
non-cached changes. They are changes wrt. to what's in HEAD resp. the
index, no matter what HEAD is.

Do you mean that red (unstaged) and yellow (staged) are both "red" to you?

No, I said red for unstaged, green for staged for the characters, i.e. a
* would always be red if present, a + always green if present.

For the "branch name", it would be red for detached head and green for
checked out branch.

quoted

[**] Also, coloring the status characters opens the way to even using
the same characters as "status -sb" (ADM).

Perhaps, but that would be confusing to me ;-)

If that is case, you can try and propose changing that scheme...

But there's really a good reason for it. Cached (staged) changes are
"safe" because they are saved in the index, and also they are "good to
go into the next commit", i.e. the commit traffic lights are green!

Michael