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

["Daniel Shahaf" <d.s@xxxxxxxxxxxxxxxxxx>]

Tim Lee wrote on Mon, 29 Mar 2021 09:30 +00:00:
> > > * 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?
> 
> Okay. I was not aware of these reasons. I am still very much a beginner
> (both to zsh and to mailing lists ...).
> 

I couldn't tell that from your original mail ☺

> > > 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 snippet below displays N/M. How would you make it display +N/-M
> instead?

Change the last line to hook_com[misc]="+${x[1]} -${x[2]}".

>     git rev-parse @{upstream} >/dev/null 2>&1 || return 0
>     local -a x=( $(git rev-list --left-right --count HEAD...@{upstream} ) )
>     hook_com[misc]="${(j:/:)x}"
> 
> > The first line is to handle a detached HEAD, I think, and should
> > presumably be added?
> 
> Why not do this instead:
> 
>     local -a x=( $(git rev-list --left-right --count HEAD...@{upstream} 2>/dev/null ) )

You mean, why not discard stderr?  Out of general principles ("Errors
shouldn't be silenced"), plus the fact that I've never had that line
generate an error that needed to be silenced — not with the «git
rev-parse … || return» right above it.

Or do you mean that line instead of the git-rev-parse(1) call too?
First, that would run the rest of the function even though $x might be
an empty array rather than a two-element one.  Two, suppose there is an
error after resolving «@{upstream}» to a commit but before rev-list
finishes (for instance, an error while walking the commits history).
With an explicit git-rev-parse(1) call that error would be reported, but
if the rev-parse and the history walking are lumped into one step any
failure from which is treated the same way, history walking failures
would be masked — even though they're probably interesting.

> ---
> Note: when replying to my email, please ensure that my email address is
> included in the 'To:' field, since I am not subscribed to the mailing
> list.

Ack, but for future reference, the customary signature delimiter is "-- "
(ASCII 0x2D 0x2D 0x20).  Many MUAs syntax-highlight that, for example.

Cheers,

Daniel