zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH 0/3] vcs_info: handle nested repositories of different types
@ 2019-11-23 22:14 Aleksandr Mezin
  2019-11-23 22:14 ` [PATCH 1/3] vcs_info/git: set vcs_comm[basedir] in VCS_INFO_detect_git Aleksandr Mezin
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Aleksandr Mezin @ 2019-11-23 22:14 UTC (permalink / raw)
  To: zsh-workers; +Cc: Aleksandr Mezin

I've noticed that vcs_info doesn't correctly handle the case when a repository
of one type is nested inside of a repository of another type.

Example:
 - git repository in ~/git-repo
 - hg repository in ~/git-repo/hg-repo (it's not a submodule)
 - vcs_info called in ~/git-repo/hg-repo will output information about git-repo

To fix this, I've made following changes:
 - Modified VCS_INFO_detect_{git,cvs} to set vcs_comm[basedir] (other backends
do it)
 - Modified vcs_info to choose the repository with the longest vcs_comm[basedir]
(as an absolute path)

However, this logic may not work correctly for symlinked repositories, if
VCS_INFO_detect_* functions will continue to use :P modifier for the basedir.
Is there a reason why :P is used for basedir instead of :a?

Aleksandr Mezin (3):
  vcs_info/git: set vcs_comm[basedir] in VCS_INFO_detect_git
  vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  vcs_info: choose the nearest repository

 .../VCS_Info/Backends/VCS_INFO_detect_cvs     | 11 ++++++++--
 .../VCS_Info/Backends/VCS_INFO_detect_git     |  1 +
 .../VCS_Info/Backends/VCS_INFO_get_data_cvs   |  6 +-----
 .../VCS_Info/Backends/VCS_INFO_get_data_git   |  2 +-
 Functions/VCS_Info/vcs_info                   | 21 +++++++++++++------
 5 files changed, 27 insertions(+), 14 deletions(-)

-- 
2.24.0


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

* [PATCH 1/3] vcs_info/git: set vcs_comm[basedir] in VCS_INFO_detect_git
  2019-11-23 22:14 [PATCH 0/3] vcs_info: handle nested repositories of different types Aleksandr Mezin
@ 2019-11-23 22:14 ` Aleksandr Mezin
  2019-11-23 22:14 ` [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs Aleksandr Mezin
  2019-11-23 22:14 ` [PATCH 3/3] vcs_info: choose the nearest repository Aleksandr Mezin
  2 siblings, 0 replies; 22+ messages in thread
From: Aleksandr Mezin @ 2019-11-23 22:14 UTC (permalink / raw)
  To: zsh-workers; +Cc: Aleksandr Mezin

---
 Functions/VCS_Info/Backends/VCS_INFO_detect_git   | 1 +
 Functions/VCS_Info/Backends/VCS_INFO_get_data_git | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Functions/VCS_Info/Backends/VCS_INFO_detect_git b/Functions/VCS_Info/Backends/VCS_INFO_detect_git
index 61bc483e3..a3dbca105 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_detect_git
+++ b/Functions/VCS_Info/Backends/VCS_INFO_detect_git
@@ -8,6 +8,7 @@ setopt localoptions NO_shwordsplit
 
 if VCS_INFO_check_com ${vcs_comm[cmd]} && ${vcs_comm[cmd]} rev-parse --is-inside-work-tree &> /dev/null ; then
     vcs_comm[gitdir]="$(${vcs_comm[cmd]} rev-parse --git-dir 2> /dev/null)" || return 1
+    vcs_comm[basedir]="$( ${vcs_comm[cmd]} rev-parse --show-toplevel )" || return 1
     if   [[ -d ${vcs_comm[gitdir]}/svn ]]             ; then vcs_comm[overwrite_name]='git-svn'
     elif [[ -d ${vcs_comm[gitdir]}/refs/remotes/p4 ]] ; then vcs_comm[overwrite_name]='git-p4' ; fi
     return 0
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
index ceb4f978a..8ecd96ccc 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
@@ -138,7 +138,7 @@ VCS_INFO_git_handle_patches () {
 
 gitdir=${vcs_comm[gitdir]}
 VCS_INFO_git_getbranch ${gitdir}
-gitbase=$( ${vcs_comm[cmd]} rev-parse --show-toplevel )
+gitbase=${vcs_comm[basedir]}
 rrn=${gitbase:t}
 if zstyle -t ":vcs_info:${vcs}:${usercontext}:${rrn}" get-revision ; then
     gitsha1=$(${vcs_comm[cmd]} rev-parse --quiet --verify HEAD)
-- 
2.24.0


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

* [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-23 22:14 [PATCH 0/3] vcs_info: handle nested repositories of different types Aleksandr Mezin
  2019-11-23 22:14 ` [PATCH 1/3] vcs_info/git: set vcs_comm[basedir] in VCS_INFO_detect_git Aleksandr Mezin
@ 2019-11-23 22:14 ` Aleksandr Mezin
  2019-11-25  4:35   ` Daniel Shahaf
  2019-11-23 22:14 ` [PATCH 3/3] vcs_info: choose the nearest repository Aleksandr Mezin
  2 siblings, 1 reply; 22+ messages in thread
From: Aleksandr Mezin @ 2019-11-23 22:14 UTC (permalink / raw)
  To: zsh-workers; +Cc: Aleksandr Mezin

---
 Functions/VCS_Info/Backends/VCS_INFO_detect_cvs   | 11 +++++++++--
 Functions/VCS_Info/Backends/VCS_INFO_get_data_cvs |  6 +-----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/Functions/VCS_Info/Backends/VCS_INFO_detect_cvs b/Functions/VCS_Info/Backends/VCS_INFO_detect_cvs
index 7a5ee1eef..73b79f744 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_detect_cvs
+++ b/Functions/VCS_Info/Backends/VCS_INFO_detect_cvs
@@ -7,5 +7,12 @@ setopt localoptions NO_shwordsplit
 [[ $1 == '--flavours' ]] && return 1
 
 VCS_INFO_check_com ${vcs_comm[cmd]} || return 1
-[[ -d "./CVS" ]] && [[ -r "./CVS/Repository" ]] && return 0
-return 1
+[[ -d "./CVS" ]] && [[ -r "./CVS/Repository" ]] || return 1
+
+local cvsbase="."
+while [[ -d "${cvsbase}/../CVS" ]]; do
+    cvsbase="${cvsbase}/.."
+done
+vcs_comm[basedir]=${cvsbase:P}
+
+return 0
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_cvs b/Functions/VCS_Info/Backends/VCS_INFO_get_data_cvs
index e9d172052..1ca0e9c16 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_cvs
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_cvs
@@ -5,11 +5,7 @@
 setopt localoptions NO_shwordsplit
 local cvsbranch cvsbase
 
-cvsbase="."
-while [[ -d "${cvsbase}/../CVS" ]]; do
-    cvsbase="${cvsbase}/.."
-done
-cvsbase=${cvsbase:P}
+cvsbase=${vcs_comm[basedir]}
 cvsbranch=$(< ./CVS/Repository)
 rrn=${cvsbase:t}
 cvsbranch=${cvsbranch##${rrn}/}
-- 
2.24.0


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

* [PATCH 3/3] vcs_info: choose the nearest repository
  2019-11-23 22:14 [PATCH 0/3] vcs_info: handle nested repositories of different types Aleksandr Mezin
  2019-11-23 22:14 ` [PATCH 1/3] vcs_info/git: set vcs_comm[basedir] in VCS_INFO_detect_git Aleksandr Mezin
  2019-11-23 22:14 ` [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs Aleksandr Mezin
@ 2019-11-23 22:14 ` Aleksandr Mezin
  2019-11-25  4:33   ` Daniel Shahaf
  2 siblings, 1 reply; 22+ messages in thread
From: Aleksandr Mezin @ 2019-11-23 22:14 UTC (permalink / raw)
  To: zsh-workers; +Cc: Aleksandr Mezin

Previously vcs_info didn't handle nested repositories of different types
correctly: for example, when in a hg repository, that is cloned inside of a git
working tree (not as a submodule), it was still showing git branch.

Choose the repository that's nearest to the current working directory. And
since base directories are always parents of the current directory - just
choose the longest path.

However, this logic may not work correctly for symlinked repositories, if
VCS_INFO_detect_* functions will continue to use :P modifier for the basedir.
Is there a reason why :P is used for basedir instead of :a?
---
 Functions/VCS_Info/vcs_info | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/Functions/VCS_Info/vcs_info b/Functions/VCS_Info/vcs_info
index d67ae6bf5..640fd1a79 100644
--- a/Functions/VCS_Info/vcs_info
+++ b/Functions/VCS_Info/vcs_info
@@ -53,13 +53,13 @@ vcs_info () {
     [[ -r . ]] || return 0
 
     local pat
-    local -i found retval
+    local -i retval
     local -a enabled disabled dps
-    local usercontext vcs rrn quiltmode
+    local usercontext vcs nearest_vcs rrn quiltmode
     local -x LC_MESSAGES
     local -i maxexports
     local -a msgs
-    local -A vcs_comm hook_com backend_misc user_data
+    local -A vcs_comm nearest_vcs_comm hook_com backend_misc nearest_backend_misc user_data
 
     LC_MESSAGES=C
     if [[ -n ${LC_ALL} ]]; then
@@ -107,7 +107,6 @@ vcs_info () {
 
     VCS_INFO_maxexports
 
-    (( found = 0 ))
     for vcs in ${enabled} ; do
         [[ -n ${(M)disabled:#${vcs}} ]] && continue
         if (( ${+functions[VCS_INFO_detect_${vcs}]} == 0 )) ; then
@@ -117,15 +116,25 @@ vcs_info () {
         fi
         vcs_comm=()
         VCS_INFO_get_cmd
-        VCS_INFO_detect_${vcs} && (( found = 1 )) && break
+        if VCS_INFO_detect_${vcs}; then
+            if (( ${#vcs_comm[basedir]:a} > ${#nearest_vcs_comm[basedir]:a} )); then
+                nearest_vcs_comm=("${(kv)vcs_comm[@]}")
+                nearest_vcs="${vcs}"
+                nearest_backend_misc=("${(kv)backend_misc[@]}")
+            fi
+        fi
     done
 
-    (( found == 0 )) && {
+    [ -z "${nearest_vcs}" ] && {
         vcs='-quilt-'; quiltmode='standalone'
         VCS_INFO_quilt standalone || VCS_INFO_set --nvcs
         return 0
     }
 
+    vcs_comm=("${(kv)nearest_vcs_comm[@]}")
+    vcs="${nearest_vcs}"
+    backend_misc=("${(kv)nearest_backend_misc[@]}")
+
     VCS_INFO_hook "pre-get-data"
     retval=$?
     if (( retval == 1 )); then
-- 
2.24.0


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

* Re: [PATCH 3/3] vcs_info: choose the nearest repository
  2019-11-23 22:14 ` [PATCH 3/3] vcs_info: choose the nearest repository Aleksandr Mezin
@ 2019-11-25  4:33   ` Daniel Shahaf
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Shahaf @ 2019-11-25  4:33 UTC (permalink / raw)
  To: Aleksandr Mezin; +Cc: zsh-workers

Aleksandr Mezin wrote on Sun, Nov 24, 2019 at 04:14:43 +0600:
> However, this logic may not work correctly for symlinked repositories, if
> VCS_INFO_detect_* functions will continue to use :P modifier for the basedir.
> Is there a reason why :P is used for basedir instead of :a?

If ${foo} exists, then ${foo} and ${foo:P} are guaranteed to point to the same
inode.  That's not true for ${foo} and ${foo:a}.  What problem do you see with
use of :P?  Shouldn't your 3/3 patch use :P rather than :a?

> +++ b/Functions/VCS_Info/vcs_info
> @@ -117,15 +116,25 @@ vcs_info () {
>          fi
>          vcs_comm=()
>          VCS_INFO_get_cmd
> -        VCS_INFO_detect_${vcs} && (( found = 1 )) && break
> +        if VCS_INFO_detect_${vcs}; then
> +            if (( ${#vcs_comm[basedir]:a} > ${#nearest_vcs_comm[basedir]:a} )); then

If $vcs_comm[basedir] is /foobar and $nearest_vcs_comm[basedir] is /foo, the
condition would be true but there'd be no parent dir–subdir relationship
between the two values.

> +                nearest_vcs_comm=("${(kv)vcs_comm[@]}")
> +                nearest_vcs="${vcs}"
> +                nearest_backend_misc=("${(kv)backend_misc[@]}")
> +            fi
> +        fi
>      done

Cheers,

Daniel

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-23 22:14 ` [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs Aleksandr Mezin
@ 2019-11-25  4:35   ` Daniel Shahaf
  2019-11-25  7:31     ` Aleksandr Mezin
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Shahaf @ 2019-11-25  4:35 UTC (permalink / raw)
  To: Aleksandr Mezin; +Cc: zsh-workers

Aleksandr Mezin wrote on Sun, Nov 24, 2019 at 04:14:42 +0600:
> +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_cvs
> @@ -5,11 +5,7 @@
> -cvsbase="."
> -while [[ -d "${cvsbase}/../CVS" ]]; do
> -    cvsbase="${cvsbase}/.."
> -done
> +++ b/Functions/VCS_Info/Backends/VCS_INFO_detect_cvs
> @@ -7,5 +7,12 @@ setopt localoptions NO_shwordsplit
> +local cvsbase="."
> +while [[ -d "${cvsbase}/../CVS" ]]; do
> +    cvsbase="${cvsbase}/.."
> +done

I know you just moved this code around, but I'd like to point out that it
causes an infinite loop when /CVS exists.

Cheers,

Daniel

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-25  4:35   ` Daniel Shahaf
@ 2019-11-25  7:31     ` Aleksandr Mezin
  2019-11-25  8:16       ` Daniel Shahaf
  2019-11-25  9:13       ` Bart Schaefer
  0 siblings, 2 replies; 22+ messages in thread
From: Aleksandr Mezin @ 2019-11-25  7:31 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On Mon, Nov 25, 2019 at 10:35 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Aleksandr Mezin wrote on Sun, Nov 24, 2019 at 04:14:42 +0600:
> > +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_cvs
> > @@ -5,11 +5,7 @@
> > -cvsbase="."
> > -while [[ -d "${cvsbase}/../CVS" ]]; do
> > -    cvsbase="${cvsbase}/.."
> > -done
> > +++ b/Functions/VCS_Info/Backends/VCS_INFO_detect_cvs
> > @@ -7,5 +7,12 @@ setopt localoptions NO_shwordsplit
> > +local cvsbase="."
> > +while [[ -d "${cvsbase}/../CVS" ]]; do
> > +    cvsbase="${cvsbase}/.."
> > +done
>
> I know you just moved this code around, but I'd like to point out that it
> causes an infinite loop when /CVS exists.

Also I probably shouldn't have moved the code in the first place. If
the current directory has `CVS/Repository` in it, it means that the
directory is controlled by CVS, and maybe we shouldn't consider .git
from the parent directory (even when the parent directory also has
`CVS/Repository` in it). Though it's questionable.

Example:
~/cvs-dir/CVS/Repository
~/cvs-dir/git-repo/.git
~/cvs-dir/git-repo/CVS/Repository
~/cvs-dir/git-repo/cvs-subdir/CVS/Repository

I'm not sure what vcs_info should output for ~/cvs-dir/git-repo/cvs-subdir/

>
> Cheers,
>
> Daniel

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-25  7:31     ` Aleksandr Mezin
@ 2019-11-25  8:16       ` Daniel Shahaf
  2019-11-26  1:26         ` Aleksandr Mezin
  2019-11-25  9:13       ` Bart Schaefer
  1 sibling, 1 reply; 22+ messages in thread
From: Daniel Shahaf @ 2019-11-25  8:16 UTC (permalink / raw)
  To: Aleksandr Mezin; +Cc: zsh-workers

Aleksandr Mezin wrote on Mon, 25 Nov 2019 07:31 +00:00:
> On Mon, Nov 25, 2019 at 10:35 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > Aleksandr Mezin wrote on Sun, Nov 24, 2019 at 04:14:42 +0600:
> > > +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_cvs
> > > @@ -5,11 +5,7 @@
> > > -cvsbase="."
> > > -while [[ -d "${cvsbase}/../CVS" ]]; do
> > > -    cvsbase="${cvsbase}/.."
> > > -done
> > > +++ b/Functions/VCS_Info/Backends/VCS_INFO_detect_cvs
> > > @@ -7,5 +7,12 @@ setopt localoptions NO_shwordsplit
> > > +local cvsbase="."
> > > +while [[ -d "${cvsbase}/../CVS" ]]; do
> > > +    cvsbase="${cvsbase}/.."
> > > +done
> >
> > I know you just moved this code around, but I'd like to point out that it
> > causes an infinite loop when /CVS exists.
> 
> Also I probably shouldn't have moved the code in the first place. If
> the current directory has `CVS/Repository` in it, it means that the
> directory is controlled by CVS, and maybe we shouldn't consider .git
> from the parent directory (even when the parent directory also has
> `CVS/Repository` in it). Though it's questionable.
> 
> Example:
> ~/cvs-dir/CVS/Repository
> ~/cvs-dir/git-repo/.git
> ~/cvs-dir/git-repo/CVS/Repository
> ~/cvs-dir/git-repo/cvs-subdir/CVS/Repository
> 
> I'm not sure what vcs_info should output for ~/cvs-dir/git-repo/cvs-subdir/

How about showing the information from the worktree whose root is deeper
(= closer to cwd)?  Users can show the information for the other worktree
by setting the 'enable' style in a :vcs_info:foo:* context and calling
«vcs_info foo».

I see two possible problems:

One, the workaround isn't sufficient when the two worktrees are of the
same VCS (for example, git and git, as opposed to git and CVS) — that
is, when foo/.$vcs and foo/bar/.$vcs both exist as separate, nested
worktrees.  However, this use-case isn't well-supported by all
VCS's.  For example, which worktree «cd foo && $vcs add bar/baz» will
take effect in is anyone's guess.

Two, as I pointed out in reply to an offlist response, the existence of
$GIT_DIR and $GIT_WORK_TREE means that the two VCS's basedirs need not
be in a parent dir – subdir relationship with each other; they can be
/foo/bar and /foo/baz.  At that point, we won't be able to use worktree
root paths as discriminators.  Re-reading PEP 20, I think "Information will be
shown for whichever VCS is listed first in the 'enable' style" might be a sensible
design… but it's a puzzler.

Cheers,

Daniel

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-25  7:31     ` Aleksandr Mezin
  2019-11-25  8:16       ` Daniel Shahaf
@ 2019-11-25  9:13       ` Bart Schaefer
  2019-11-25 18:22         ` Daniel Shahaf
  1 sibling, 1 reply; 22+ messages in thread
From: Bart Schaefer @ 2019-11-25  9:13 UTC (permalink / raw)
  To: Aleksandr Mezin; +Cc: Daniel Shahaf, Zsh hackers list

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

On Sun, Nov 24, 2019, 11:32 PM Aleksandr Mezin <mezin.alexander@gmail.com>
wrote:

>
> Also I probably shouldn't have moved the code in the first place. If
> the current directory has `CVS/Repository` in it, it means that the
> directory is controlled by CVS


I have several directories that are BOTH git and CVS repositories, used for
keeping two different origins in sync.

This is admittedly an unusual situation -- we have a legacy process that
expects to fetch sources from CVS, and developers collaborating through git
-- but I would be annoyed if vcs didn't find the git data because of the
CVS subdir.

I would expect the list of repo types in the "enable" style to be handled
in the order they appear.

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-25  9:13       ` Bart Schaefer
@ 2019-11-25 18:22         ` Daniel Shahaf
  2019-11-25 18:47           ` Bart Schaefer
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Shahaf @ 2019-11-25 18:22 UTC (permalink / raw)
  To: Zsh hackers list; +Cc: Aleksandr Mezin

Bart Schaefer wrote on Mon, Nov 25, 2019 at 01:13:21 -0800:
> I have several directories that are BOTH git and CVS repositories, used for
> keeping two different origins in sync.
> 
> This is admittedly an unusual situation -- we have a legacy process that
> expects to fetch sources from CVS, and developers collaborating through git
> -- but I would be annoyed if vcs didn't find the git data because of the
> CVS subdir.

Doesn't my proposal address this?  In my proposal, if {foo,foo/bar,foo/bar/baz}/CVS
and foo/bar/.git all exist, and cwd is foo/bar/baz/, git info would be shown
because the git root, foo/bar/, is closer (deeper) than the CVS root, foo/ —
notwithstanding that foo/bar/baz/CVS exists.

To be self-contained, what I proposed is:

- If [[ ${basedirA:P} == ${basedirB:P}/* ]], show ${basedirA}'s info.
- If [[ ${basedirB:P} == ${basedirA:P}/* ]], show ${basedirB}'s info.
- Show the info for whichever VCS is listed first in the 'enable' style.

Would that DTRT in your use-case?

> I would expect the list of repo types in the "enable" style to be handled
> in the order they appear.

Do you mean, that if the enable style lists "git" ahead of "cvs", then vcs_info
would always look for git info first and only show CVS info if git wasn't
detected?  If so, see above.

Or do you mean, show _both_ the CVS and git info?  I don't see how that would
work.  Consider:

zstyle ':vcs_info:git:default:*'     formats foo1
zstyle ':vcs_info:git:default:lorem' formats foo2 bar2
zstyle ':vcs_info:cvs:default:*'     formats baz
precmd() { vcs_info default; [[ -z $vcs_info_msg_1_ ]] && print -Pr -- $vcs_info_msg_1_ }

Would this print "bar2" or "baz"?  How could the precmd function determine
where the git info ends and the cvs info begins?

Perhaps we could have vcs_info return multiple arrays rather than multiple
scalars.  For example, «typeset -a vcs_info_set_1_» could contain git info and
«typeset -a vcs_info_set_2_» could contain cvs info.  That would also address
the case about git-within-git or hg-within-hg that I mentioned earlier.
However, it begs the question, why does vcs_info today use multiple scalars
rather than a single array?

Cheers,

Daniel

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-25 18:22         ` Daniel Shahaf
@ 2019-11-25 18:47           ` Bart Schaefer
  0 siblings, 0 replies; 22+ messages in thread
From: Bart Schaefer @ 2019-11-25 18:47 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list, Aleksandr Mezin

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

On Mon, Nov 25, 2019, 10:23 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:

> Bart Schaefer wrote on Mon, Nov 25, 2019 at 01:13:21 -0800:
> > I have several directories that are BOTH git and CVS repositories, used
> for
> > keeping two different origins in sync.
>
> Doesn't my proposal address this?


Quite likely - I was writing before your proposal reached my inbox.


> I would expect the list of repo types in the "enable" style to be handled
> > in the order they appear.
>
> Do you mean, that if the enable style lists "git" ahead of "cvs", then
> vcs_info
> would always look for git info first and only show CVS info if git wasn't
> detected?


Yes.

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-25  8:16       ` Daniel Shahaf
@ 2019-11-26  1:26         ` Aleksandr Mezin
  2019-11-26  1:28           ` Aleksandr Mezin
  0 siblings, 1 reply; 22+ messages in thread
From: Aleksandr Mezin @ 2019-11-26  1:26 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On Mon, Nov 25, 2019 at 2:17 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Two, as I pointed out in reply to an offlist response, the existence of
> $GIT_DIR and $GIT_WORK_TREE means that the two VCS's basedirs need not
> be in a parent dir – subdir relationship with each other; they can be
> /foo/bar and /foo/baz.

How is it possible?

in a git repository

    $ GIT_WORK_TREE=../other-dir git rev-parse --is-inside-work-tree
    false

Work tree root should be a parent of cwd, otherwise `rev-parse
--is-inside-work-tree` returns false -> git repository not detected.
And there is only one path from cwd to the root -> all parents of cwd
are parents or subdirs to each other.

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-26  1:26         ` Aleksandr Mezin
@ 2019-11-26  1:28           ` Aleksandr Mezin
  2019-11-26  4:52             ` Daniel Shahaf
  0 siblings, 1 reply; 22+ messages in thread
From: Aleksandr Mezin @ 2019-11-26  1:28 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On Tue, Nov 26, 2019 at 7:26 AM Aleksandr Mezin
<mezin.alexander@gmail.com> wrote:
>
> On Mon, Nov 25, 2019 at 2:17 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > Two, as I pointed out in reply to an offlist response, the existence of
> > $GIT_DIR and $GIT_WORK_TREE means that the two VCS's basedirs need not
> > be in a parent dir – subdir relationship with each other; they can be
> > /foo/bar and /foo/baz.
>
> How is it possible?
>
> in a git repository
>
>     $ GIT_WORK_TREE=../other-dir git rev-parse --is-inside-work-tree
>     false
>
> Work tree root should be a parent of cwd, otherwise `rev-parse
> --is-inside-work-tree` returns false -> git repository not detected.
> And there is only one path from cwd to the root -> all parents of cwd
> are parents or subdirs to each other.

I meant "there is only one path from cwd to /" in the last sentence.

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-26  1:28           ` Aleksandr Mezin
@ 2019-11-26  4:52             ` Daniel Shahaf
  2019-11-26 10:22               ` Aleksandr Mezin
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Shahaf @ 2019-11-26  4:52 UTC (permalink / raw)
  To: Aleksandr Mezin; +Cc: zsh-workers

Aleksandr Mezin wrote on Tue, 26 Nov 2019 01:28 +00:00:
> On Tue, Nov 26, 2019 at 7:26 AM Aleksandr Mezin
> <mezin.alexander@gmail.com> wrote:
> >
> > On Mon, Nov 25, 2019 at 2:17 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > > Two, as I pointed out in reply to an offlist response, the existence of
> > > $GIT_DIR and $GIT_WORK_TREE means that the two VCS's basedirs need not
> > > be in a parent dir – subdir relationship with each other; they can be
> > > /foo/bar and /foo/baz.
> >
> > How is it possible?

% cd "$(mktemp -d)"
% hg init foo
% git init bar
% cd foo
[shows hg info]
% export GIT_WORK_TREE=${PWD/foo/bar} GIT_DIR=${PWD/foo/bar}/.git
[shows git info]
% 

> > in a git repository
> >
> >     $ GIT_WORK_TREE=../other-dir git rev-parse --is-inside-work-tree
> >     false
> >
> > Work tree root should be a parent of cwd, otherwise `rev-parse
> > --is-inside-work-tree` returns false -> git repository not detected.
> > And there is only one path from cwd to the root -> all parents of cwd
> > are parents or subdirs to each other.
> 
> I meant "there is only one path from cwd to /" in the last sentence.
>

Cheers,

Daniel

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-26  4:52             ` Daniel Shahaf
@ 2019-11-26 10:22               ` Aleksandr Mezin
  2019-11-28 11:13                 ` Daniel Shahaf
  0 siblings, 1 reply; 22+ messages in thread
From: Aleksandr Mezin @ 2019-11-26 10:22 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On Tue, Nov 26, 2019 at 10:52 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Aleksandr Mezin wrote on Tue, 26 Nov 2019 01:28 +00:00:
> > On Tue, Nov 26, 2019 at 7:26 AM Aleksandr Mezin
> > <mezin.alexander@gmail.com> wrote:
> > >
> > > On Mon, Nov 25, 2019 at 2:17 PM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > > > Two, as I pointed out in reply to an offlist response, the existence of
> > > > $GIT_DIR and $GIT_WORK_TREE means that the two VCS's basedirs need not
> > > > be in a parent dir – subdir relationship with each other; they can be
> > > > /foo/bar and /foo/baz.
> > >
> > > How is it possible?
>
> % cd "$(mktemp -d)"
> % hg init foo
> % git init bar
> % cd foo
> [shows hg info]
> % export GIT_WORK_TREE=${PWD/foo/bar} GIT_DIR=${PWD/foo/bar}/.git
> [shows git info]
> %

Interesting. I didn't notice that GIT_INFO_detect_git checks the exit
code of `git rev-parse --is-inside-work-tree`, not the actual value.
You don't even need to set GIT_WORK_TREE, GIT_DIR is enough.

But should it work like that?

>
> > > in a git repository
> > >
> > >     $ GIT_WORK_TREE=../other-dir git rev-parse --is-inside-work-tree
> > >     false
> > >
> > > Work tree root should be a parent of cwd, otherwise `rev-parse
> > > --is-inside-work-tree` returns false -> git repository not detected.
> > > And there is only one path from cwd to the root -> all parents of cwd
> > > are parents or subdirs to each other.
> >
> > I meant "there is only one path from cwd to /" in the last sentence.
> >
>
> Cheers,
>
> Daniel

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-26 10:22               ` Aleksandr Mezin
@ 2019-11-28 11:13                 ` Daniel Shahaf
  2019-11-28 21:34                   ` Daniel Shahaf
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Shahaf @ 2019-11-28 11:13 UTC (permalink / raw)
  To: Aleksandr Mezin; +Cc: zsh-workers

Aleksandr Mezin wrote on Tue, 26 Nov 2019 10:22 +00:00:
> On Tue, Nov 26, 2019 at 10:52 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > % cd foo
> > [shows hg info]
> > % export GIT_WORK_TREE=${PWD/foo/bar} GIT_DIR=${PWD/foo/bar}/.git
> > [shows git info]
> > %
> 
> Interesting. I didn't notice that GIT_INFO_detect_git checks the exit
> code of `git rev-parse --is-inside-work-tree`, not the actual value.

I think it should check both the exit code and the output.

> You don't even need to set GIT_WORK_TREE, GIT_DIR is enough.

I think it's fine to detect git when GIT_DIR is set.  After all, git
commands would work at that point, so it'll be useful to display git
info.  [I mean this in the context of "when should git be detected", not
in the context of "what to do when multiple VCS's are detected".
I think the two questions are independent.]

Cheers,

Daniel
(sorry for late answer; I'm not online much this week)

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-28 11:13                 ` Daniel Shahaf
@ 2019-11-28 21:34                   ` Daniel Shahaf
  2019-11-29 13:41                     ` Aleksandr Mezin
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Shahaf @ 2019-11-28 21:34 UTC (permalink / raw)
  To: Aleksandr Mezin; +Cc: zsh-workers

Daniel Shahaf wrote on Thu, Nov 28, 2019 at 11:13:44 +0000:
> Aleksandr Mezin wrote on Tue, 26 Nov 2019 10:22 +00:00:
> > On Tue, Nov 26, 2019 at 10:52 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > > % cd foo
> > > [shows hg info]
> > > % export GIT_WORK_TREE=${PWD/foo/bar} GIT_DIR=${PWD/foo/bar}/.git
> > > [shows git info]
> > > %
> > 
> > Interesting. I didn't notice that GIT_INFO_detect_git checks the exit
> > code of `git rev-parse --is-inside-work-tree`, not the actual value.
> 
> I think it should check both the exit code and the output.

Sorry, I didn't look closely enough.  In the example I gave (quoted above),
--is-inside-work-tree prints "false" and exits with code 0.  In that situation,
I think VCS_INFO_detect_git should detect git, and it does.  All in all,
I think the current code is correct, and could be optimized further with:

[[[
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_detect_git b/Functions/VCS_Info/Backends/VCS_INFO_detect_git
index 61bc483e3..e4191f474 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_detect_git
+++ b/Functions/VCS_Info/Backends/VCS_INFO_detect_git
@@ -6,8 +6,7 @@ setopt localoptions NO_shwordsplit
 
 [[ $1 == '--flavours' ]] && { print -l git-p4 git-svn; return 0 }
 
-if VCS_INFO_check_com ${vcs_comm[cmd]} && ${vcs_comm[cmd]} rev-parse --is-inside-work-tree &> /dev/null ; then
-    vcs_comm[gitdir]="$(${vcs_comm[cmd]} rev-parse --git-dir 2> /dev/null)" || return 1
+if VCS_INFO_check_com ${vcs_comm[cmd]} && vcs_comm[gitdir]="$(${vcs_comm[cmd]} rev-parse --git-dir 2> /dev/null)" ; then
     if   [[ -d ${vcs_comm[gitdir]}/svn ]]             ; then vcs_comm[overwrite_name]='git-svn'
     elif [[ -d ${vcs_comm[gitdir]}/refs/remotes/p4 ]] ; then vcs_comm[overwrite_name]='git-p4' ; fi
     return 0
]]]

WDYT?

Cheers,

Daniel

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-28 21:34                   ` Daniel Shahaf
@ 2019-11-29 13:41                     ` Aleksandr Mezin
  2019-11-29 15:25                       ` Mikael Magnusson
  2019-11-29 20:30                       ` Daniel Shahaf
  0 siblings, 2 replies; 22+ messages in thread
From: Aleksandr Mezin @ 2019-11-29 13:41 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers

On Fri, Nov 29, 2019 at 3:34 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Daniel Shahaf wrote on Thu, Nov 28, 2019 at 11:13:44 +0000:
> > Aleksandr Mezin wrote on Tue, 26 Nov 2019 10:22 +00:00:
> > > On Tue, Nov 26, 2019 at 10:52 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > > > % cd foo
> > > > [shows hg info]
> > > > % export GIT_WORK_TREE=${PWD/foo/bar} GIT_DIR=${PWD/foo/bar}/.git
> > > > [shows git info]
> > > > %
> > >
> > > Interesting. I didn't notice that GIT_INFO_detect_git checks the exit
> > > code of `git rev-parse --is-inside-work-tree`, not the actual value.
> >
> > I think it should check both the exit code and the output.
>
> Sorry, I didn't look closely enough.  In the example I gave (quoted above),
> --is-inside-work-tree prints "false" and exits with code 0.  In that situation,
> I think VCS_INFO_detect_git should detect git, and it does.  All in all,
> I think the current code is correct, and could be optimized further with:
>
> [[[
> diff --git a/Functions/VCS_Info/Backends/VCS_INFO_detect_git b/Functions/VCS_Info/Backends/VCS_INFO_detect_git
> index 61bc483e3..e4191f474 100644
> --- a/Functions/VCS_Info/Backends/VCS_INFO_detect_git
> +++ b/Functions/VCS_Info/Backends/VCS_INFO_detect_git
> @@ -6,8 +6,7 @@ setopt localoptions NO_shwordsplit
>
>  [[ $1 == '--flavours' ]] && { print -l git-p4 git-svn; return 0 }
>
> -if VCS_INFO_check_com ${vcs_comm[cmd]} && ${vcs_comm[cmd]} rev-parse --is-inside-work-tree &> /dev/null ; then
> -    vcs_comm[gitdir]="$(${vcs_comm[cmd]} rev-parse --git-dir 2> /dev/null)" || return 1
> +if VCS_INFO_check_com ${vcs_comm[cmd]} && vcs_comm[gitdir]="$(${vcs_comm[cmd]} rev-parse --git-dir 2> /dev/null)" ; then
>      if   [[ -d ${vcs_comm[gitdir]}/svn ]]             ; then vcs_comm[overwrite_name]='git-svn'
>      elif [[ -d ${vcs_comm[gitdir]}/refs/remotes/p4 ]] ; then vcs_comm[overwrite_name]='git-p4' ; fi
>      return 0
> ]]]
>
> WDYT?
>
> Cheers,
>
> Daniel

Then I don't see a sane way to choose which VCS to show, other than
how it's already done (first one detected from 'enabled'). For
example, a hg work tree in cwd, and GIT_WORK_TREE is set to something
else (maybe parent directory, maybe ../other-dir). Which one (hg or
git) to choose? Why?

I guess I'll have to live with my own fork of vcs_info, because any
changes there will break it for someone else.

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-29 13:41                     ` Aleksandr Mezin
@ 2019-11-29 15:25                       ` Mikael Magnusson
  2019-11-29 19:55                         ` Daniel Shahaf
  2019-11-29 20:30                       ` Daniel Shahaf
  1 sibling, 1 reply; 22+ messages in thread
From: Mikael Magnusson @ 2019-11-29 15:25 UTC (permalink / raw)
  To: Aleksandr Mezin; +Cc: Daniel Shahaf, zsh-workers

On 11/29/19, Aleksandr Mezin <mezin.alexander@gmail.com> wrote:
> Then I don't see a sane way to choose which VCS to show, other than
> how it's already done (first one detected from 'enabled'). For
> example, a hg work tree in cwd, and GIT_WORK_TREE is set to something
> else (maybe parent directory, maybe ../other-dir). Which one (hg or
> git) to choose? Why?
>
> I guess I'll have to live with my own fork of vcs_info, because any
> changes there will break it for someone else.

If you use zstyle -e to change the enable (or disable) style of
vcs_info, you should be able to decide in the way you want. (Arguably
it should use the order of the systems listed in the enable style to
decide which to prioritise). Or you can include one system in enable
for a more specific style only, eg zstyle :vcs_info:hgonly:* enable
hg, and then call "vcs_info hgonly", etc.

-- 
Mikael Magnusson

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-29 15:25                       ` Mikael Magnusson
@ 2019-11-29 19:55                         ` Daniel Shahaf
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Shahaf @ 2019-11-29 19:55 UTC (permalink / raw)
  To: zsh-workers; +Cc: Aleksandr Mezin

Mikael Magnusson wrote on Fri, Nov 29, 2019 at 16:25:44 +0100:
> Or you can include one system in enable for a more specific style only, eg
> zstyle :vcs_info:hgonly:* enable hg, and then call "vcs_info hgonly", etc.

It would be «:vcs_info:*:hgonly:*».  Docs:

http://zsh.sourceforge.net/Doc/Release/User-Contributions.html#vcs_005finfo-Configuration

Cheers,

Daniel

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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-29 13:41                     ` Aleksandr Mezin
  2019-11-29 15:25                       ` Mikael Magnusson
@ 2019-11-29 20:30                       ` Daniel Shahaf
  2019-11-29 20:52                         ` Daniel Shahaf
  1 sibling, 1 reply; 22+ messages in thread
From: Daniel Shahaf @ 2019-11-29 20:30 UTC (permalink / raw)
  To: Aleksandr Mezin; +Cc: zsh-workers

Aleksandr Mezin wrote on Fri, Nov 29, 2019 at 19:41:51 +0600:
> On Fri, Nov 29, 2019 at 3:34 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >
> > Daniel Shahaf wrote on Thu, Nov 28, 2019 at 11:13:44 +0000:
> > > Aleksandr Mezin wrote on Tue, 26 Nov 2019 10:22 +00:00:
> > > > On Tue, Nov 26, 2019 at 10:52 AM Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > > > > % cd foo
> > > > > [shows hg info]
> > > > > % export GIT_WORK_TREE=${PWD/foo/bar} GIT_DIR=${PWD/foo/bar}/.git
> > > > > [shows git info]
> > > > > %
> > > >
> > > > Interesting. I didn't notice that GIT_INFO_detect_git checks the exit
> > > > code of `git rev-parse --is-inside-work-tree`, not the actual value.
> > >
> > > I think it should check both the exit code and the output.
> >
> > Sorry, I didn't look closely enough.  In the example I gave (quoted above),
> > --is-inside-work-tree prints "false" and exits with code 0.  In that situation,
> > I think VCS_INFO_detect_git should detect git, and it does.  All in all,
> > I think the current code is correct [...]
> 
> Then I don't see a sane way to choose which VCS to show, other than
> how it's already done (first one detected from 'enabled'). For
> example, a hg work tree in cwd, and GIT_WORK_TREE is set to something
> else (maybe parent directory, maybe ../other-dir). Which one (hg or
> git) to choose? Why?

As I said in workers/44930 and workers/44936, I propose to show
information for the VCS whose worktree root is deeper; and if neither
worktree root is deeper than the other, _only then_ fall back to the
current behaviour of showing information for the VCS listed first in the
'enable' style.  I think this would address your use-case without
regressing any other.

(By "worktree root" I mean the directory shown by «git rev-parse
--show-toplevel», «svn info --show-item=wcroot», and their equivalents
for other VCS's.)

With this proposal, there are three cases to consider: ${basedirA:P}
and ${basedirB:P} may be either (a) equal, (b) siblings, or (c) parent
and child.  The proposal makes no difference for cases (a) and (b).  For
case (c), it allows the user to see info for either the parent vcs or
the child vcs by cd'ing appropriately.  That would be an improvement
over the current behaviour.

Yes, in cases (a) and (b) there is no obvious correct behaviour.
However, the fact that there's no obvious correct behaviour in a given
situation is simply a data point that should inform the design process;
nothing more, nothing less.  It doesn't make the problem unsolvable.

> I guess I'll have to live with my own fork of vcs_info, because any
> changes there will break it for someone else.

Let's not jump to conclusions.  In this case, I think you have several
courses of action open to you:

- Revise your patch series in light of the feedback received — for
  example, workers/44927 proposed two small changes to the
  «if (( ${#vcs_comm[basedir]:a} > ${#nearest_vcs_comm[basedir]:a} ))»
  line — and resubmit it.  [The changes are basically what I wrote in
  workers/44936, taking the bulleted list for an if/elif/else
  chain.  I think there already is consensus on those changes.]

- Propose some other API change; for example:

  + Make it possible to report on multiple VCS's at the same time (cf
    last paragraph of workers/44936).  This will cover not only the
    parent/child case but also the "same" and "sibling" cases.  I would
    recommend to do this by writing up a behaviour specification
    [ideally as a _pro forma_ patch to Doc/Zsh/contrib.yo] and putting
    it up for discussion, rather than by implementing anything.

  + Add a hook that gets the list of VCS's that have been recognized and
    sets $REPLY to the name of the VCS backend to show info for.

  + Document that in the the "same" and "sibling" cases, it is
    unspecified which VCS's info gets displayed — i.e., vcs_info will
    show the info for one of the backends, but we don't promise which
    one — and recommend to set styles as Mikael explained.

  + Something else.

The (( $+GIT_WORK_TREE )) && [[ -e ./.hg ]] case is an edge case.  Since
it doesn't work _today_, fixing it isn't a blocker to accepting the
patch.  It's perfectly fine to fix just the parent/child case and leave
the "same" and "sibling" cases as they are today.  (Patches should
introduce no regressions, but aren't expected to fix all existing bugs.)

In the meantime, the first two patches in this series can be applied,
can't they?  Or were you planning to revise and resubmit the CVS one to
address the (preëxisting) infinite loop?  The first two patches should
cause no behaviour change, I assume, other than making $hook_com[basedir]
available to hooks earlier.  Actually, should we document $hook_com[basedir]
in the manual?

Cheers,

Daniel

P.S.  I don't know what your background is, but in general, it's _very_
common for patch series to receive reviews and be resubmitted with
changes before they're accepted and merged; that's an everyday
occurrence, no more remarkable than seeing somebody give up their seat
on a city bus.  


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

* Re: [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs
  2019-11-29 20:30                       ` Daniel Shahaf
@ 2019-11-29 20:52                         ` Daniel Shahaf
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Shahaf @ 2019-11-29 20:52 UTC (permalink / raw)
  To: Aleksandr Mezin; +Cc: zsh-workers

Daniel Shahaf wrote on Fri, Nov 29, 2019 at 20:30:52 +0000:
> In the meantime, the first two patches in this series can be applied,
> can't they?  Or were you planning to revise and resubmit the CVS one to
> address the (preëxisting) infinite loop?  The first two patches should
> cause no behaviour change, I assume, other than making $hook_com[basedir]
> available to hooks earlier.  Actually, should we document $hook_com[basedir]
> in the manual?

Never mind, I misread the code: it's $vcs_comm[basedir], not
$hook_com[basedir].  The former is only used for communicating between
a VCS_INFO_detect_${vcs} function that uses VCS_INFO_bydir_detect and the
corresponding VCS_INFO_get_data_${vcs} function.

Incidentally, VCS_INFO_detect_cvs should probably use VCS_INFO_bydir_detect.

Cheers,

Daniel

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

end of thread, other threads:[~2019-11-29 20:53 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-23 22:14 [PATCH 0/3] vcs_info: handle nested repositories of different types Aleksandr Mezin
2019-11-23 22:14 ` [PATCH 1/3] vcs_info/git: set vcs_comm[basedir] in VCS_INFO_detect_git Aleksandr Mezin
2019-11-23 22:14 ` [PATCH 2/3] vcs_info/cvs: set vcs_comm[basedir] in VCS_INFO_detect_cvs Aleksandr Mezin
2019-11-25  4:35   ` Daniel Shahaf
2019-11-25  7:31     ` Aleksandr Mezin
2019-11-25  8:16       ` Daniel Shahaf
2019-11-26  1:26         ` Aleksandr Mezin
2019-11-26  1:28           ` Aleksandr Mezin
2019-11-26  4:52             ` Daniel Shahaf
2019-11-26 10:22               ` Aleksandr Mezin
2019-11-28 11:13                 ` Daniel Shahaf
2019-11-28 21:34                   ` Daniel Shahaf
2019-11-29 13:41                     ` Aleksandr Mezin
2019-11-29 15:25                       ` Mikael Magnusson
2019-11-29 19:55                         ` Daniel Shahaf
2019-11-29 20:30                       ` Daniel Shahaf
2019-11-29 20:52                         ` Daniel Shahaf
2019-11-25  9:13       ` Bart Schaefer
2019-11-25 18:22         ` Daniel Shahaf
2019-11-25 18:47           ` Bart Schaefer
2019-11-23 22:14 ` [PATCH 3/3] vcs_info: choose the nearest repository Aleksandr Mezin
2019-11-25  4:33   ` Daniel Shahaf

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).