Re: [PATCH] Improve vcs_info example for ahead/behind git commits

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Tim Lee wrote on Mon, Mar 29, 2021 at 05:36:28 +0800:
> This patch improves the example hook that shows +N/-N when the
> local git branch is ahead-of or behind the remote HEAD.

Thanks.  Review:

> Improvements:
> 
> * Use git-rev-list's `--count` option instead of piping to `wc -l`.
>   `--count` has been available since git 1.7.2
>   (https://github.com/git/git/commit/f69c501832ecd6880602c55565508e70c3a013d5)

Sure.

> * Remove unnecessary use of `${hook_com[branch]}` because `@{upstream}`
>   defaults to the current branch when no branch name is provided.
>   `@{upstream}` was introduced in git 1.7.0
>   (https://github.com/git/git/commit/28fb84382b0eb728534dbe2972bbfec3f3d83dd9)

I'm not sure that's unnecessary, for two reasons:

1. hook_com[branch], as opposed to hook_com[branch_orig], may have been
changed by a previous hook.

2. There may be educational value to demonstrating a use of
hook_com[branch], even if it's implied.

WDYT?

> 
> diff --git a/Misc/vcs_info-examples b/Misc/vcs_info-examples
> index 94b8a7b5e..36d4d3bf8 100644
> --- a/Misc/vcs_info-examples
> +++ b/Misc/vcs_info-examples
> @@ -179,14 +179,18 @@ function +vi-git-st() {
>      local ahead behind
>      local -a gitstatus
> 
> -    # for git prior to 1.7
> +    # for git prior to 1.7.0
>      # ahead=$(git rev-list origin/${hook_com[branch]}..HEAD | wc -l)
> -    ahead=$(git rev-list ${hook_com[branch]}@{upstream}..HEAD 2>/dev/null | wc -l)
> +    # for git 1.7.0 and 1.7.1
> +    # ahead=$(git rev-list @{upstream}..HEAD 2>/dev/null | wc -l)
> +    ahead=$(git rev-list --count @{upstream}..HEAD 2>/dev/null)
>      (( $ahead )) && gitstatus+=( "+${ahead}" )

The version of this function in my zshrc starts like this:
.
    git rev-parse @{upstream} >/dev/null 2>&1 || return 0
    local -a x=( $(git rev-list --left-right --count HEAD...@{upstream} ) )

The first line is to handle a detached HEAD, I think, and should
presumably be added?

The second line saves a fork(), which is valuable on some platforms.

(I actually posted my function in users/21810, but didn't notice these
two issues back then.  Daniel Hahler posted his version too in
users/21813.)

Other than that, the patch LGTM.  Personally I might not have bothered
to add instructions for a 2010-vintage Git in 2021, though.

Cheers,

Daniel

> -    # for git prior to 1.7
> +    # for git prior to 1.7.0
>      # behind=$(git rev-list HEAD..origin/${hook_com[branch]} | wc -l)
> -    behind=$(git rev-list HEAD..${hook_com[branch]}@{upstream} 2>/dev/null | wc -l)
> +    # for git 1.7.0 and 1.7.1
> +    # behind=$(git rev-list HEAD..@{upstream} 2>/dev/null | wc -l)
> +    behind=$(git rev-list --count HEAD..@{upstream} 2>/dev/null)
>      (( $behind )) && gitstatus+=( "-${behind}" )
> 
>      hook_com[misc]+=${(j:/:)gitstatus}
>