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

From: Richard Hansen <rhansen@bbn.com>
Date: Sep 30, 2014, 20:44 UTC
Message-ID: <542B1623.2070109@bbn.com>
In-Reply-To: <1412091370-11727-1-git-send-email-jcarsique@nuxeo.com>

On 2014-09-30 11:36, Julien Carsique wrote:

Show 10 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]$

Seems like a good idea to me.

Show 5 quoted lines

> 
> Signed-off-by: Julien Carsique <julien.carsique@gmail.com>
> ---
>  contrib/completion/git-prompt.sh | 7 +++++++
>  1 file changed, 7 insertions(+)

Please add some new tests in t/9903-bash-prompt.sh.  In particular:
  * upstream ref in refs/heads
  * upstream is git-svn
  * branch names containing slashes

Show 11 quoted lines

> 
> 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/}

To avoid colliding with other stuff, this variable should either be local or prefixed with '__git_ps1'.

> +			if [ "$__head" = "${__git_ps1_upstream_name##*/}" ]; then

This comparison breaks on branches containing a slash (e.g., foo/bar).

Also, how does this interact with git-svn? (I don't use git-svn so I'm not very familiar with how it manages refs.)

Assuming remote names can't contain a slash (which I think is true), a safer approach might be parse the full ref and special-case refs/remotes:

    __git_ps1_upstream_name=$(git rev-parse \
        --abbrev-ref "${upstream}" 2>/dev/null)
    local tmp
    tmp=$(git rev-parse --symbolic-full-name "${upstream}" 2>/dev/null)
    case ${tmp} in
    refs/remotes/*)
        # todo: can ${b} be something other than refs/heads/* here?
        [ "${__git_ps1_upstream_name#*/}" != "${b#refs/heads/}" ] \
            || __git_ps1_upstream_name=${__git_ps1_upstream_name%%/*}/\=
        ;;
    esac

Additional cases could be added to handle git-svn if needed.

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

  * This could break if ${__head} contains any pattern-special
    characters.

  * While this syntax works in both Bash and Zsh (assuming no
    pattern-special characters), my preference is to stick to POSIX[1]
    when possible.  For example, assuming the upstream name is
    always in refs/remotes (which is not true, but this is an example)
    and remote names can't contain a '/', you could do this:

        __git_ps1_upstream_name=${__git_ps1_upstream_name%%/*}/\=

  * I don't think the CodingGuidelines explicitly prohibit long lines
    for shell code, and this file already contains plenty of long
    lines, but I really dislike lines longer than 80 characters.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html

Show 6 quoted lines

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

-Richard