Re: [PATCH] git-prompt.sh: shorter equal upstream branch name

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 30, 2014, 22:29 UTC
Message-ID: <xmqqbnpwrbcw.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1412091370-11727-1-git-send-email-jcarsique@nuxeo.com>

Julien Carsique <julien.carsique@gmail.com> writes:

Show 27 quoted lines

> From: Julien Carsique <julien.carsique@gmail.com>
>
> When using the "name" option of GIT_PS1_SHOWUPSTREAM to show the upstream
> abbrev name, if the upstream name is the same as the local name, then some
> space could be saved in the prompt. This is especially needed on long branch
> names.
>
> Replace the upstream name with the sign '=' if equal to the local one.
> Example:    [master * u= origin/=]$
> instead of: [master * u= origin/master]$
>
> Signed-off-by: Julien Carsique <julien.carsique@gmail.com>
> ---
>  contrib/completion/git-prompt.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index c5473dc..a9aba20 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -209,6 +209,13 @@ __git_ps1_show_upstream ()
>  		if [[ -n "$count" && -n "$name" ]]; then
>  			__git_ps1_upstream_name=$(git rev-parse \
>  				--abbrev-ref "$upstream" 2>/dev/null)
> +
> +			__head=${b##refs/heads/}
> +			if [ "$__head" = "${__git_ps1_upstream_name##*/}" ]; then

When you are on your "xyz" branch that was forked off of "refs/remote/origin/xyz/xyz", $__git_ps1_upstream_name would be "origin/xyz/xyz" and $__head is "xyz" at this point. The latter matches the former after stripping */ maximally from its front (i.e. "xyz"). It is unclear if you really want to make these two match, but as long as you correctly abbreviate you would not lose information, I would imagine. I am guessing that you would want to see "origin/xyz/=" in such a case, and if you started your "xyz" off of "origin/abc/xyz", you would want "origin/abc/=".

> +				__git_ps1_upstream_name=${__git_ps1_upstream_name/$__head/=}

Now, what does ${__git_ps1_upstream_name/$__head/=} give us? This should not do regexp substitution in the first place, I would think. You made sure $__head is the tail-matching substring of the longer variable, so you chop that many bytes off of the latter.

Is this new feature something everybody should want unconditionally, or are there valid reasons why some people may not want to see this abbreviation happen (even if the implementation were not buggy)?

> +			fi
> +			unset __head

Unlike __git_ps1_upstream_name, this __head does not have to be visible outside this function. Wouldn't it be better to declare it as local (this is bash prompt and we can afford to step outside POSIX) and there is no need to "unset" after use if you did so, no?

>  			if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then
>  				p="$p \${__git_ps1_upstream_name}"
>  			else