Simon Oosthoek <s.oosthoek@xxxxxxxxx> writes: > this patch is an additional patch to the previous series of two. > @@ -325,35 +323,45 @@ __git_ps1 () > > local f="$w$i$s$u" > if [ $pcmode = yes ]; then > + if [ -n "${GIT_PS1_SHOWCOLORHINTS-}" ]; then > local c_red='\e[31m' > local c_green='\e[32m' > local c_lblue='\e[1;34m' > local c_clear='\e[0m' > local branchstring="$c${b##refs/heads/}" > + local branch_color="$c_clear" > + local flags_color="$c_lblue" > > + case "$b" in > + \(*\)) branch_color="$c_red" > + ;; > + *) local branch_color="$c_green" > + ;; > + esac Why "local" only on one side and not the other? Also switching on "$b" and relying on how it happens to be formatted inside parentheses when detached (which can very well change in the future) looks like a fragile bug waiting to be triggered. > # Setting PS1 directly with \[ and \] around colors > # is necessary to prevent wrapping issues! > + PS1="$ps1pc_start (\[$branch_color\]$branchstring\[$c_clear\]" > + > + if [ -n "$w$i$s$u$r$p" ]; then > + PS1="$PS1 " > + fi > + if [ "$w" = "*" ]; then > + PS1="$PS1\[$c_red\]$w" > + fi > + if [ -n "$i" ]; then > + PS1="$PS1\[$c_green\]$i" > + fi You gave nice symbolic names to branch_color and flags_color, but these two use hardcoded colors? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html