Re: [PATCH 3/3] Change colors to be based on git status -sb in color mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]