zsh-workers
 help / color / mirror / Atom feed
* [PATCH] Improve vcs_info example for ahead/behind git commits
@ 2021-03-28 21:36 Tim Lee
  2021-03-29  7:06 ` Daniel Shahaf
  0 siblings, 1 reply; 12+ messages in thread
From: Tim Lee @ 2021-03-28 21:36 UTC (permalink / raw)
  To: zsh-workers

This patch improves the example hook that shows +N/-N when the
local git branch is ahead-of or behind the remote HEAD.

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)
* 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)


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}" )

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-03-28 21:36 [PATCH] Improve vcs_info example for ahead/behind git commits Tim Lee
@ 2021-03-29  7:06 ` Daniel Shahaf
  2021-03-29  9:30   ` Tim Lee
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Shahaf @ 2021-03-29  7:06 UTC (permalink / raw)
  To: Tim Lee; +Cc: zsh-workers

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-03-29  7:06 ` Daniel Shahaf
@ 2021-03-29  9:30   ` Tim Lee
  2021-03-29  9:52     ` Daniel Shahaf
  0 siblings, 1 reply; 12+ messages in thread
From: Tim Lee @ 2021-03-29  9:30 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

> > * 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 ...).

> > 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?

    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 ) )


---
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.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-03-29  9:30   ` Tim Lee
@ 2021-03-29  9:52     ` Daniel Shahaf
  2021-03-29 10:39       ` Tim Lee
  2021-03-29 15:20       ` Lawrence Velázquez
  0 siblings, 2 replies; 12+ messages in thread
From: Daniel Shahaf @ 2021-03-29  9:52 UTC (permalink / raw)
  To: Tim Lee; +Cc: zsh-workers

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-03-29  9:52     ` Daniel Shahaf
@ 2021-03-29 10:39       ` Tim Lee
  2021-03-29 11:50         ` Daniel Shahaf
  2021-03-29 15:20       ` Lawrence Velázquez
  1 sibling, 1 reply; 12+ messages in thread
From: Tim Lee @ 2021-03-29 10:39 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

> > > > 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]}".

But hook_com[misc]="+${x[1]} -${x[2]}" would display a '+0' and/or '-0'
when the local git branch is not ahead-of or behind the remote branch.

The existing example does not display '+0' or '-0'.

> >     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?

Yes, I mean: instead of this:

     git rev-parse @{upstream} >/dev/null 2>&1 || return 0
     local -a x=( $(git rev-list --left-right --count HEAD...@{upstream} ) )

Replace both of the lines above with this:

     local -a x=( $(git rev-list --left-right --count HEAD...@{upstream} 2>/dev/null ) )

> 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.

Thank you for all this information. I've learned so much today. :-)


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-03-29 10:39       ` Tim Lee
@ 2021-03-29 11:50         ` Daniel Shahaf
  2021-04-10 20:51           ` Lawrence Velázquez
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Shahaf @ 2021-03-29 11:50 UTC (permalink / raw)
  To: Tim Lee; +Cc: zsh-workers

Tim Lee wrote on Mon, 29 Mar 2021 10:39 +00:00:
> > > > > 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]}".
> 
> But hook_com[misc]="+${x[1]} -${x[2]}" would display a '+0' and/or '-0'
> when the local git branch is not ahead-of or behind the remote branch.
> 
> The existing example does not display '+0' or '-0'.

You didn't specify this when you asked.  Anyway, if you want not to show
+0 or -0, then add the appropriate logic — e.g., «if (( x[2] )); then».
We're on -workers@ so I thought that'd go without saying.

A bit further out along the "readable"–"line noise" spectrum there is
this alternative:

  hook_com[misc]="+$x[1]/-$x[2]"
  hook_com[misc]=${${hook_com[misc]#'+0/'}%'/-0'}

Cheers,

Daniel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-03-29  9:52     ` Daniel Shahaf
  2021-03-29 10:39       ` Tim Lee
@ 2021-03-29 15:20       ` Lawrence Velázquez
  1 sibling, 0 replies; 12+ messages in thread
From: Lawrence Velázquez @ 2021-03-29 15:20 UTC (permalink / raw)
  To: Daniel Shahaf, Tim Lee; +Cc: zsh-workers

On Mon, Mar 29, 2021, at 5:52 AM, Daniel Shahaf wrote:
> Tim Lee wrote on Mon, 29 Mar 2021 09:30 +00:00:
> > ---
> > 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.

what, too good for cc/bcc? ;)

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

[whoa.gif]

vq


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-03-29 11:50         ` Daniel Shahaf
@ 2021-04-10 20:51           ` Lawrence Velázquez
  2021-04-13 11:34             ` Daniel Shahaf
  0 siblings, 1 reply; 12+ messages in thread
From: Lawrence Velázquez @ 2021-04-10 20:51 UTC (permalink / raw)
  To: Tim Lee; +Cc: Daniel Shahaf, zsh-workers

Anything else on this?

vq


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-04-10 20:51           ` Lawrence Velázquez
@ 2021-04-13 11:34             ` Daniel Shahaf
  2021-04-13 11:58               ` Tim Lee
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Shahaf @ 2021-04-13 11:34 UTC (permalink / raw)
  To: Tim Lee, Lawrence Velázquez; +Cc: zsh-workers

Lawrence Velázquez wrote on Sat, Apr 10, 2021 at 16:51:43 -0400:
> Anything else on this?

Tim: Would you have time to address the remaining points from the
original review, https://zsh.org/workers/48306, either by implementing
them and sending a revised patch, or by counterarguing them?

Cheers,

Daniel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-04-13 11:34             ` Daniel Shahaf
@ 2021-04-13 11:58               ` Tim Lee
  2021-04-13 14:01                 ` Daniel Shahaf
  0 siblings, 1 reply; 12+ messages in thread
From: Tim Lee @ 2021-04-13 11:58 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Lawrence Velázquez, zsh-workers

> Tim: Would you have time to address the remaining points from the
> original review, https://zsh.org/workers/48306, either by implementing
> them and sending a revised patch, or by counterarguing them?

I think I'll just porpose something simple, since I'm not very familiar
with ZSH.

Patch:

diff --git a/Misc/vcs_info-examples b/Misc/vcs_info-examples
index 94b8a7b5e..d33d0ceed 100644
--- a/Misc/vcs_info-examples
+++ b/Misc/vcs_info-examples
@@ -179,14 +179,10 @@ function +vi-git-st() {
     local ahead behind
     local -a gitstatus

-    # for git prior to 1.7
-    # 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)
+    ahead=$(git rev-list --count ${hook_com[branch]}@{upstream}..HEAD 2>/dev/null)
     (( $ahead )) && gitstatus+=( "+${ahead}" )

-    # for git prior to 1.7
-    # 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)
+    behind=$(git rev-list --count HEAD..${hook_com[branch]}@{upstream} 2>/dev/null)
     (( $behind )) && gitstatus+=( "-${behind}" )

     hook_com[misc]+=${(j:/:)gitstatus}


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-04-13 11:58               ` Tim Lee
@ 2021-04-13 14:01                 ` Daniel Shahaf
  2021-04-13 15:01                   ` Tim Lee
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Shahaf @ 2021-04-13 14:01 UTC (permalink / raw)
  To: Tim Lee; +Cc: Lawrence Velázquez, zsh-workers

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

Tim Lee wrote on Tue, Apr 13, 2021 at 19:58:30 +0800:
> > Tim: Would you have time to address the remaining points from the
> > original review, https://zsh.org/workers/48306, either by implementing
> > them and sending a revised patch, or by counterarguing them?
> 
> I think I'll just porpose something simple, since I'm not very familiar
> with ZSH.
> 
> Patch:

Thanks, applied, and here's a patch series for implementing the other
outstanding points.  Would you review it, please?  See anything that
could go wrong, or that's missing?

Cheers,

Daniel



[-- Attachment #2: 0001-vcs_info-git-docs-ahead-behind-commits-Don-t-run.patch.txt --]
[-- Type: text/plain, Size: 850 bytes --]

From 8a521515ce88802846df8884cabdd3966d0f32a8 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Tue, 13 Apr 2021 13:56:36 +0000
Subject: [PATCH 1/2] vcs_info git docs: ahead/behind commits: Don't run
 rev-list when that would fail

---
 Misc/vcs_info-examples | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Misc/vcs_info-examples b/Misc/vcs_info-examples
index d33d0ceed..065ea6cb3 100644
--- a/Misc/vcs_info-examples
+++ b/Misc/vcs_info-examples
@@ -179,6 +179,9 @@ function +vi-git-st() {
     local ahead behind
     local -a gitstatus
 
+    # Exit early in case the worktree is on a detached HEAD
+    git rev-parse ${hook_com[branch]}@{upstream} >/dev/null 2>&1 || return 0
+
     ahead=$(git rev-list --count ${hook_com[branch]}@{upstream}..HEAD 2>/dev/null)
     (( $ahead )) && gitstatus+=( "+${ahead}" )
 

[-- Attachment #3: 0002-vcs_info-git-docs-ahead-behind-commits-Reduce-th.patch.txt --]
[-- Type: text/plain, Size: 1245 bytes --]

From d084af57318a98f5e3ff197e269f42611e5d2443 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Tue, 13 Apr 2021 13:56:31 +0000
Subject: [PATCH 2/2] vcs_info git docs: ahead/behind commits: Reduce the
 number of forks

---
 Misc/vcs_info-examples | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Misc/vcs_info-examples b/Misc/vcs_info-examples
index 065ea6cb3..ba94cca29 100644
--- a/Misc/vcs_info-examples
+++ b/Misc/vcs_info-examples
@@ -182,10 +182,14 @@ function +vi-git-st() {
     # Exit early in case the worktree is on a detached HEAD
     git rev-parse ${hook_com[branch]}@{upstream} >/dev/null 2>&1 || return 0
 
-    ahead=$(git rev-list --count ${hook_com[branch]}@{upstream}..HEAD 2>/dev/null)
-    (( $ahead )) && gitstatus+=( "+${ahead}" )
+    local -a ahead_and_behind=(
+        $(git rev-list --left-right --count HEAD...${hook_com[branch]}@{upstream} 2>/dev/null)
+    )
 
-    behind=$(git rev-list --count HEAD..${hook_com[branch]}@{upstream} 2>/dev/null)
+    ahead=${ahead_and_behind[1]}
+    behind=${ahead_and_behind[2]}
+
+    (( $ahead )) && gitstatus+=( "+${ahead}" )
     (( $behind )) && gitstatus+=( "-${behind}" )
 
     hook_com[misc]+=${(j:/:)gitstatus}

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] Improve vcs_info example for ahead/behind git commits
  2021-04-13 14:01                 ` Daniel Shahaf
@ 2021-04-13 15:01                   ` Tim Lee
  0 siblings, 0 replies; 12+ messages in thread
From: Tim Lee @ 2021-04-13 15:01 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Lawrence Velázquez, zsh-workers

> Thanks, applied, and here's a patch series for implementing the other
> outstanding points.  Would you review it, please?  See anything that
> could go wrong, or that's missing?

Looks good to me. I would use this myself. However, you should take my
review with a grain of salt.


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-04-13 15:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-28 21:36 [PATCH] Improve vcs_info example for ahead/behind git commits Tim Lee
2021-03-29  7:06 ` Daniel Shahaf
2021-03-29  9:30   ` Tim Lee
2021-03-29  9:52     ` Daniel Shahaf
2021-03-29 10:39       ` Tim Lee
2021-03-29 11:50         ` Daniel Shahaf
2021-04-10 20:51           ` Lawrence Velázquez
2021-04-13 11:34             ` Daniel Shahaf
2021-04-13 11:58               ` Tim Lee
2021-04-13 14:01                 ` Daniel Shahaf
2021-04-13 15:01                   ` Tim Lee
2021-03-29 15:20       ` Lawrence Velázquez

zsh-workers

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/zsh-workers

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 zsh-workers zsh-workers/ http://inbox.vuxu.org/zsh-workers \
		zsh-workers@zsh.org
	public-inbox-index zsh-workers

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/zsh/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git