VCS_INFO_get_data_svn:31: bad set of key/value pairs for associative array

zsh-workers
 help / color / mirror / code / Atom feed

* VCS_INFO_get_data_svn:31: bad set of key/value pairs for associative array
@ 2014-10-22  4:24 Manfred Lotz
  2014-10-22  6:22 ` Bart Schaefer
  0 siblings, 1 reply; 9+ messages in thread
From: Manfred Lotz @ 2014-10-22  4:24 UTC (permalink / raw)
  To: zsh-workers

Hi there,
In my .zshrc I have defined a function svn like this


function svn () {
   ...
}


where I do additional things when invoking svn.


After upgrading to zsh 5.0.7-1 in Fedora 20 I get this error if I
happen to be in a svn repository:

VCS_INFO_get_data_svn:31: bad set of key/value pairs for associative
array


Local fix is to rename the function to svn1 and define an alias

alias svn='svn1'


Line 31 in VCS_INFO_get_data_svn is:
  cwdinfo=(${(kv)svninfo})


Could anybody tell me what's going on? 



-- 
Thanks,
Manfred


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

* Re: VCS_INFO_get_data_svn:31: bad set of key/value pairs for associative array
  2014-10-22  4:24 VCS_INFO_get_data_svn:31: bad set of key/value pairs for associative array Manfred Lotz
@ 2014-10-22  6:22 ` Bart Schaefer
  2014-10-22  9:28   ` [PATCH] vcs_info: Use ‘command’ prefix to call version control programs Frank Terbeck
  0 siblings, 1 reply; 9+ messages in thread
From: Bart Schaefer @ 2014-10-22  6:22 UTC (permalink / raw)
  To: zsh-workers

On Oct 22,  6:24am, Manfred Lotz wrote:
}
} In my .zshrc I have defined a function svn
} 
} After upgrading to zsh 5.0.7-1 in Fedora 20 I get this error if I
} happen to be in a svn repository:
} 
} VCS_INFO_get_data_svn:31: bad set of key/value pairs for associative
} array

In Functions/VCS_Info/Backends/VCS_INFO_get_data_svn there are a few
places where calls to "${vcs_comm[cmd]} info ..." are made; the output
of this is then parsed.  Your wrapper must be producing different output
which can't be parsed properly.

I suppose VCS_INFO_get_data_svn ought to use "command ${vcs_comm[cmd]}"
instead to bypass any such wrapper functions.  There are around three
dozen places in Functions/VCS_Info/Backends/* that could similarly use
the "command" prefix if we want to support Manfred's style of use.

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

* [PATCH] vcs_info: Use ‘command’ prefix to call version control programs
  2014-10-22  6:22 ` Bart Schaefer
@ 2014-10-22  9:28   ` Frank Terbeck
  2014-10-22 10:53     ` Daniel Shahaf
  0 siblings, 1 reply; 9+ messages in thread
From: Frank Terbeck @ 2014-10-22  9:28 UTC (permalink / raw)
  To: zsh-workers; +Cc: Bart Schaefer, Manfred Lotz

Bart Schaefer wrote:
> On Oct 22,  6:24am, Manfred Lotz wrote:
[...]
> } VCS_INFO_get_data_svn:31: bad set of key/value pairs for associative
> } array
[...]
> I suppose VCS_INFO_get_data_svn ought to use "command ${vcs_comm[cmd]}"
> instead to bypass any such wrapper functions.  There are around three
> dozen places in Functions/VCS_Info/Backends/* that could similarly use
> the "command" prefix if we want to support Manfred's style of use.

This patch implements Bart's suggestion.
---

    I think this is reasonable. I hope, people don't count on
    wrapper functions to work to achieve some sort of sinister
    hack. :)

 Functions/VCS_Info/Backends/VCS_INFO_detect_git      |  4 ++--
 Functions/VCS_Info/Backends/VCS_INFO_detect_p4       |  4 ++--
 Functions/VCS_Info/Backends/VCS_INFO_detect_tla      |  2 +-
 Functions/VCS_Info/Backends/VCS_INFO_get_data_bzr    |  8 ++++----
 Functions/VCS_Info/Backends/VCS_INFO_get_data_fossil |  2 +-
 Functions/VCS_Info/Backends/VCS_INFO_get_data_git    | 18 +++++++++---------
 Functions/VCS_Info/Backends/VCS_INFO_get_data_hg     |  2 +-
 Functions/VCS_Info/Backends/VCS_INFO_get_data_mtn    |  2 +-
 Functions/VCS_Info/Backends/VCS_INFO_get_data_p4     |  4 ++--
 Functions/VCS_Info/Backends/VCS_INFO_get_data_svn    |  8 ++++----
 Functions/VCS_Info/Backends/VCS_INFO_get_data_tla    |  2 +-
 Functions/VCS_Info/VCS_INFO_quilt                    |  2 +-
 12 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/Functions/VCS_Info/Backends/VCS_INFO_detect_git b/Functions/VCS_Info/Backends/VCS_INFO_detect_git
index 61bc483..5db0a1d 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_detect_git
+++ b/Functions/VCS_Info/Backends/VCS_INFO_detect_git
@@ -6,8 +6,8 @@ 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]} && command ${vcs_comm[cmd]} rev-parse --is-inside-work-tree &> /dev/null ; then
+    vcs_comm[gitdir]="$(command ${vcs_comm[cmd]} rev-parse --git-dir 2> /dev/null)" || 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_detect_p4 b/Functions/VCS_Info/Backends/VCS_INFO_detect_p4
index 377453f..fdf3543 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_detect_p4
+++ b/Functions/VCS_Info/Backends/VCS_INFO_detect_p4
@@ -21,7 +21,7 @@ VCS_INFO_p4_get_server() {
   setopt extendedglob
 
   local -a settings
-  settings=(${(f)"$(${vcs_comm[cmd]} set)"})
+  settings=(${(f)"$(command ${vcs_comm[cmd]} set)"})
   serverport=${${settings[(r)P4PORT=*]##P4PORT=}%% *}
   case $serverport in
     (''|:)
@@ -54,7 +54,7 @@ VCS_INFO_detect_p4() {
       VCS_INFO_p4_get_server
       [[ -n $vcs_info_p4_dead_servers[$serverport] ]] && return 1
     fi
-    if p4where="$(${vcs_comm[cmd]} where 2>&1)"; then
+    if p4where="$(command ${vcs_comm[cmd]} where 2>&1)"; then
       return 0
     fi
     if [[ $p4where = *"Connect to server failed"* ]]; then
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_detect_tla b/Functions/VCS_Info/Backends/VCS_INFO_detect_tla
index ac4dbd7..2d50ff1 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_detect_tla
+++ b/Functions/VCS_Info/Backends/VCS_INFO_detect_tla
@@ -7,5 +7,5 @@ setopt localoptions NO_shwordsplit
 [[ $1 == '--flavours' ]] && return 1
 
 VCS_INFO_check_com ${vcs_comm[cmd]} || return 1
-vcs_comm[basedir]="$(${vcs_comm[cmd]} tree-root 2> /dev/null)" && return 0
+vcs_comm[basedir]="$(command ${vcs_comm[cmd]} tree-root 2> /dev/null)" && return 0
 return 1
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_bzr b/Functions/VCS_Info/Backends/VCS_INFO_get_data_bzr
index cae1a3b..1767fff 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_bzr
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_bzr
@@ -9,7 +9,7 @@ local -a bzrinfo
 local -xA hook_com bzr_info
 
 VCS_INFO_bzr_get_info() {
-    bzrinfo=( ${(s.:.)$( ${vcs_comm[cmd]} version-info --custom \
+    bzrinfo=( ${(s.:.)$( command ${vcs_comm[cmd]} version-info --custom \
         --template="{revno}:{branch_nick}:{clean}")} )
     if zstyle -t ":vcs_info:${vcs}:${usercontext}:${rrn}" "check-for-changes"
     then
@@ -22,7 +22,7 @@ VCS_INFO_bzr_get_info() {
 
 VCS_INFO_bzr_get_info_restricted() {
     # we are forbidden from fetching info on bound branch from remote repository
-    bzrinfo=( $(${vcs_comm[cmd]} revno) ${bzrbase:t} )
+    bzrinfo=( $(command ${vcs_comm[cmd]} revno) ${bzrbase:t} )
     if zstyle -t ":vcs_info:${vcs}:${usercontext}:${rrn}" "check-for-changes" && \
        [[ ! $bzr_type == lightweigth ]]
     then
@@ -34,7 +34,7 @@ VCS_INFO_bzr_get_changes() {
     local -A counts
     local line flag
     bzr_changes=$(
-        ${vcs_comm[cmd]} stat -SV | while read flag line
+        command ${vcs_comm[cmd]} stat -SV | while read flag line
         do
             counts[${flag}]=$(( ${counts[${flag}]:-0} + 1 ))
         done
@@ -55,7 +55,7 @@ if zstyle -t ":vcs_info:${vcs}:${usercontext}:${rrn}" "use-simple" ; then
     fi
 else
     # Parse the output of 'bzr info' into associative array bzr_info
-    ${vcs_comm[cmd]} info | {
+    command ${vcs_comm[cmd]} info | {
         local line key value dirtype
         read dirtype
         grep '^[ a-zA-Z0-9]\+: ' | while read line
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_fossil b/Functions/VCS_Info/Backends/VCS_INFO_get_data_fossil
index fd0f838..ed9a9c7 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_fossil
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_fossil
@@ -7,7 +7,7 @@ local a b
 local -A fsinfo
 local fshash fsbranch changed merging action
 
-${vcs_comm[cmd]} status |
+command ${vcs_comm[cmd]} status |
    while IFS=: read a b; do
       fsinfo[${a//-/_}]="${b## #}"
    done
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
index 48d552f..d859781 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
@@ -72,7 +72,7 @@ VCS_INFO_git_getaction () {
 
 VCS_INFO_git_getbranch () {
     local gitdir=$1 tmp actiondir
-    local gitsymref="${vcs_comm[cmd]} symbolic-ref HEAD"
+    local gitsymref="command ${vcs_comm[cmd]} symbolic-ref HEAD"
 
     actiondir=''
     for tmp in "${gitdir}/rebase-apply" \
@@ -102,7 +102,7 @@ VCS_INFO_git_getbranch () {
         gitbranch="$(${(z)gitsymref} 2> /dev/null)"
 
         if [[ $? -ne 0 ]] ; then
-            gitbranch="refs/tags/$(${vcs_comm[cmd]} describe --all --exact-match HEAD 2>/dev/null)"
+            gitbranch="refs/tags/$(command ${vcs_comm[cmd]} describe --all --exact-match HEAD 2>/dev/null)"
 
             if [[ $? -ne 0 ]] ; then
                 gitbranch="${${"$(< $gitdir/HEAD)"}[1,7]}..."
@@ -153,10 +153,10 @@ VCS_INFO_git_handle_patches () {
 
 gitdir=${vcs_comm[gitdir]}
 VCS_INFO_git_getbranch ${gitdir}
-gitbase=$( ${vcs_comm[cmd]} rev-parse --show-toplevel )
+gitbase=$( command ${vcs_comm[cmd]} rev-parse --show-toplevel )
 rrn=${gitbase:t}
 if zstyle -t ":vcs_info:${vcs}:${usercontext}:${rrn}" get-revision ; then
-    gitsha1=$(${vcs_comm[cmd]} rev-parse --quiet --verify HEAD)
+    gitsha1=$(command ${vcs_comm[cmd]} rev-parse --quiet --verify HEAD)
 else
     gitsha1=''
 fi
@@ -173,20 +173,20 @@ elif zstyle -t ":vcs_info:${vcs}:${usercontext}:${rrn}" "check-for-staged-change
     querystaged=1
 fi
 if (( querystaged || queryunstaged )) && \
-   [[ "$(${vcs_comm[cmd]} rev-parse --is-inside-work-tree 2> /dev/null)" == 'true' ]] ; then
+   [[ "$(command ${vcs_comm[cmd]} rev-parse --is-inside-work-tree 2> /dev/null)" == 'true' ]] ; then
     # Default: off - these are potentially expensive on big repositories
     if (( queryunstaged )) ; then
-        ${vcs_comm[cmd]} diff --no-ext-diff --ignore-submodules=dirty --quiet --exit-code ||
+        command ${vcs_comm[cmd]} diff --no-ext-diff --ignore-submodules=dirty --quiet --exit-code ||
             gitunstaged=1
     fi
     if (( querystaged )) ; then
-        if ${vcs_comm[cmd]} rev-parse --quiet --verify HEAD &> /dev/null ; then
-            ${vcs_comm[cmd]} diff-index --cached --quiet --ignore-submodules=dirty HEAD 2> /dev/null
+        if command ${vcs_comm[cmd]} rev-parse --quiet --verify HEAD &> /dev/null ; then
+            command ${vcs_comm[cmd]} diff-index --cached --quiet --ignore-submodules=dirty HEAD 2> /dev/null
             (( $? && $? != 128 )) && gitstaged=1
         else
             # empty repository (no commits yet)
             # 4b825dc642cb6eb9a060e54bf8d69288fbee4904 is the git empty tree.
-            ${vcs_comm[cmd]} diff-index --cached --quiet --ignore-submodules=dirty 4b825dc642cb6eb9a060e54bf8d69288fbee4904 2>/dev/null
+            command ${vcs_comm[cmd]} diff-index --cached --quiet --ignore-submodules=dirty 4b825dc642cb6eb9a060e54bf8d69288fbee4904 2>/dev/null
             (( $? && $? != 128 )) && gitstaged=1
         fi
     fi
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
index cedaf56..13046e1 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
@@ -53,7 +53,7 @@ if zstyle -t ":vcs_info:${vcs}:${usercontext}:${rrn}" get-revision ; then
             "check-for-changes" || hgid_args+=( -r. )
 
         local HGRCPATH
-        HGRCPATH="/dev/null" ${vcs_comm[cmd]} ${(z)hgid_args} 2> /dev/null \
+        HGRCPATH="/dev/null" command ${vcs_comm[cmd]} ${(z)hgid_args} 2> /dev/null \
             | read -r r_csetid r_lrev r_branch
     fi
 fi
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_mtn b/Functions/VCS_Info/Backends/VCS_INFO_get_data_mtn
index 0a8064c..197540c 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_mtn
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_mtn
@@ -7,6 +7,6 @@ local mtnbranch mtnbase
 
 mtnbase=${vcs_comm[basedir]}
 rrn=${mtnbase:t}
-mtnbranch=${${(M)${(f)"$( ${vcs_comm[cmd]} status )"}:#(#s)Current branch:*}/*: /}
+mtnbranch=${${(M)${(f)"$( command ${vcs_comm[cmd]} status )"}:#(#s)Current branch:*}/*: /}
 VCS_INFO_formats '' "${mtnbranch}" "${mtnbase}" '' '' '' ''
 return 0
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_p4 b/Functions/VCS_Info/Backends/VCS_INFO_get_data_p4
index 430cfa6..e2a54d1 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_p4
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_p4
@@ -8,7 +8,7 @@ local p4base a b
 local -A p4info
 local -xA hook_com
 
-${vcs_comm[cmd]} info | while IFS=: read a b; do p4info[${a// /_}]="${b## #}"; done
+command ${vcs_comm[cmd]} info | while IFS=: read a b; do p4info[${a// /_}]="${b## #}"; done
 p4base=${vcs_comm[basedir]}
 
 # We'll use the client name as the branch; close enough.
@@ -16,7 +16,7 @@ local p4branch change
 # We'll use the latest change number to which the hierarchy from
 # here down is synced as the revision.
 # I suppose the following might be slow on a tortuous client view.
-change="${${$(${vcs_comm[cmd]} changes -m 1 ...\#have)##Change }%% *}"
+change="${${$(command ${vcs_comm[cmd]} changes -m 1 ...\#have)##Change }%% *}"
 zstyle -s ":vcs_info:${vcs}:${usercontext}:${rrn}" branchformat p4branch || p4branch="%b:%r"
 hook_com=( branch "${p4info[Client_name]}" revision "${change}" )
 if VCS_INFO_hook 'set-branch-format' "${p4branch}"; then
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_svn b/Functions/VCS_Info/Backends/VCS_INFO_get_data_svn
index e56afee..a37d240 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_svn
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_svn
@@ -16,11 +16,11 @@ svninfo=()
 # resolved, here is a workaround that will get things done, without using it.
 # Clumsily, but that's life.
 local -a dat
-dat=( ${(f)"$(${vcs_comm[cmd]} info --non-interactive 2>&1)"} )
+dat=( ${(f)"$(command ${vcs_comm[cmd]} info --non-interactive 2>&1)"} )
 rc=$?
 (( rc != 0 )) && return 1
 # The following line is the real code, the following is the workaround.
-#${vcs_comm[cmd]} info --non-interactive \
+#command ${vcs_comm[cmd]} info --non-interactive \
 print -l "${dat[@]}" \
 |& while IFS=: read a b; do
     svninfo[${a// /_}]="${b## #}"
@@ -34,12 +34,12 @@ cwdinfo=(${(kv)svninfo})
 if (( ${+svninfo[Working_Copy_Root_Path]} )); then
   # svn 1.7+
   svnbase=${svninfo[Working_Copy_Root_Path]}
-  ${vcs_comm[cmd]} info --non-interactive -- "${svnbase}" | while IFS=: read a b; do svninfo[${a// /_}]="${b## #}"; done
+  command ${vcs_comm[cmd]} info --non-interactive -- "${svnbase}" | while IFS=: read a b; do svninfo[${a// /_}]="${b## #}"; done
 else
   # svn 1.0-1.6
   while [[ -d "${svnbase}/../.svn" ]]; do
       parentinfo=()
-      ${vcs_comm[cmd]} info --non-interactive -- "${svnbase}/.." | while IFS=: read a b; do parentinfo[${a// /_}]="${b## #}"; done
+      command ${vcs_comm[cmd]} info --non-interactive -- "${svnbase}/.." | while IFS=: read a b; do parentinfo[${a// /_}]="${b## #}"; done
       [[ ${parentinfo[Repository_UUID]} != ${svninfo[Repository_UUID]} ]] && break
       svninfo=(${(kv)parentinfo})
       svnbase="${svnbase}/.."
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_tla b/Functions/VCS_Info/Backends/VCS_INFO_get_data_tla
index f015e0c..9bdfd6a 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_tla
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_tla
@@ -8,6 +8,6 @@ local tlabase tlabranch
 tlabase="$(VCS_INFO_realpath ${vcs_comm[basedir]})"
 rrn=${tlabase:t}
 # tree-id gives us something like 'foo@example.com/demo--1.0--patch-4', so:
-tlabranch=${${"$( ${vcs_comm[cmd]} tree-id )"}/*\//}
+tlabranch=${${"$( command ${vcs_comm[cmd]} tree-id )"}/*\//}
 VCS_INFO_formats '' "${tlabranch}" "${tlabase}" '' '' '' ''
 return 0
diff --git a/Functions/VCS_Info/VCS_INFO_quilt b/Functions/VCS_Info/VCS_INFO_quilt
index db15dda..cca1393 100644
--- a/Functions/VCS_Info/VCS_INFO_quilt
+++ b/Functions/VCS_Info/VCS_INFO_quilt
@@ -134,7 +134,7 @@ function VCS_INFO_quilt() {
         # This zstyle call needs to be moved further up if `quilt' needs
         # to be run in more places than this one.
         zstyle -s "${context}" quiltcommand quiltcommand || quiltcommand='quilt'
-        unapplied=( ${(f)"$(QUILT_PATCHES=$patches $quiltcommand --quiltrc /dev/null unapplied 2> /dev/null)"} )
+        unapplied=( ${(f)"$(QUILT_PATCHES=$patches command $quiltcommand --quiltrc /dev/null unapplied 2> /dev/null)"} )
         unapplied=( ${unapplied:#} )
     else
         unapplied=()
-- 
2.1.0.60.g85f0837


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

* Re: [PATCH] vcs_info: Use ‘command’ prefix to call version control programs
  2014-10-22  9:28   ` [PATCH] vcs_info: Use ‘command’ prefix to call version control programs Frank Terbeck
@ 2014-10-22 10:53     ` Daniel Shahaf
  2014-10-22 12:03       ` Frank Terbeck
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Shahaf @ 2014-10-22 10:53 UTC (permalink / raw)
  To: Frank Terbeck; +Cc: zsh-workers, Bart Schaefer, Manfred Lotz

Frank Terbeck wrote on Wed, Oct 22, 2014 at 11:28:24 +0200:
> Bart Schaefer wrote:
> > On Oct 22,  6:24am, Manfred Lotz wrote:
> [...]
> > } VCS_INFO_get_data_svn:31: bad set of key/value pairs for associative
> > } array
> [...]
> > I suppose VCS_INFO_get_data_svn ought to use "command ${vcs_comm[cmd]}"
> > instead to bypass any such wrapper functions.  There are around three
> > dozen places in Functions/VCS_Info/Backends/* that could similarly use
> > the "command" prefix if we want to support Manfred's style of use.
> 
> This patch implements Bart's suggestion.
> ---
> 
>     I think this is reasonable. I hope, people don't count on
>     wrapper functions to work to achieve some sort of sinister
>     hack. :)
> 

I hate to say this after you already wrote the patch, but:

There are legitimate use-cases for wrapper functions, and your patch
would break them.  (For example, someone might use a wrapper function
to dispatch to a different svn binary depending on the working copy they
are in.)

Alternative solutions include:

- Asking people who write svn() functions that vcs_info cannot use to
  set the 'command' style to /usr/local/bin/svn, e.g.,

    zstyle ':vcs_info:svn:*' command =svn

  (I assume most svn() wrapper functions out there are compatible with
  vcs_info.)

- Patching the wrapper function to behave differently when invoked by
  vcs_info.  The different behaviour could be, for example, adding
  --non-interactive or dispatching to /usr/local/bin/svn without any
  wrapping logic.  I'm not sure what the best way to detect "being
  invoked by vcs_info" would be --- perhaps "[[ ! -t 1 ]]"?

I would also be interested in knowing what the specific problem was in
the interaction between the OP's wrapper and vcs_info.  (And in the
value of `svn --version -q`.)

Cheers,

Daniel


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

* Re: [PATCH] vcs_info: Use ‘command’ prefix to call version control programs
  2014-10-22 10:53     ` Daniel Shahaf
@ 2014-10-22 12:03       ` Frank Terbeck
  2014-10-22 12:42         ` Aaron Schrab
  2014-10-22 12:54         ` [PATCH] " Manfred Lotz
  0 siblings, 2 replies; 9+ messages in thread
From: Frank Terbeck @ 2014-10-22 12:03 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: zsh-workers, Bart Schaefer, Manfred Lotz

Hey Daniel,

Daniel Shahaf wrote:
> Frank Terbeck wrote on Wed, Oct 22, 2014 at 11:28:24 +0200:
[...]
>>     I think this is reasonable. I hope, people don't count on
>>     wrapper functions to work to achieve some sort of sinister
>>     hack. :)
>> 
>
> I hate to say this after you already wrote the patch, but:

No worries. Changing those lines took like five minutes. :)

> There are legitimate use-cases for wrapper functions, and your patch
> would break them.  (For example, someone might use a wrapper function
> to dispatch to a different svn binary depending on the working copy they
> are in.)

I was afraid someone might say that. :)

> Alternative solutions include:
>
> - Asking people who write svn() functions that vcs_info cannot use to
>   set the 'command' style to /usr/local/bin/svn, e.g.,
>
>     zstyle ':vcs_info:svn:*' command =svn
>
>   (I assume most svn() wrapper functions out there are compatible with
>   vcs_info.)

That style could also be used to switch to a different binary, depending
on the current working copy. That's kind of what that style was
introduced for.

> - Patching the wrapper function to behave differently when invoked by
>   vcs_info.  The different behaviour could be, for example, adding
>   --non-interactive or dispatching to /usr/local/bin/svn without any
>   wrapping logic.  I'm not sure what the best way to detect "being
>   invoked by vcs_info" would be --- perhaps "[[ ! -t 1 ]]"?

I'd probably check whether $vcs is set. Although, someone else might use
that particular variable output of vcs_info, so I don't know.

> I would also be interested in knowing what the specific problem was in
> the interaction between the OP's wrapper and vcs_info.  (And in the
> value of `svn --version -q`.)

The problem is probably with the wrapper, rather than subversion. But
sure, I would be nice to know.


About resolving this issue, I'm fine either way. I think Daniel has a
point about setting the ‘command’ style, if you're using a wrapper
function that's incompatible with the usual behaviour of a backend's
external program.


Regards, Frank


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

* Re:  vcs_info: Use ‘command’ prefix to call version control programs
  2014-10-22 12:03       ` Frank Terbeck
@ 2014-10-22 12:42         ` Aaron Schrab
  2014-10-22 13:43           ` Frank Terbeck
  2014-10-22 12:54         ` [PATCH] " Manfred Lotz
  1 sibling, 1 reply; 9+ messages in thread
From: Aaron Schrab @ 2014-10-22 12:42 UTC (permalink / raw)
  To: Frank Terbeck; +Cc: Daniel Shahaf, zsh-workers, Bart Schaefer, Manfred Lotz

At 14:03 +0200 22 Oct 2014, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>That style could also be used to switch to a different binary, 
>depending on the current working copy. That's kind of what that style 
>was introduced for.

But somebody using a wrapper for that would likely want it to be used 
for things other than vcs_info, and the style wouldn't help there.

>About resolving this issue, I'm fine either way. I think Daniel has a
>point about setting the ‘command’ style, if you're using a wrapper
>function that's incompatible with the usual behaviour of a backend's
>external program.

Setting the command style would also work if the problematic wrapper is 
implemented as a stand-alone executable rather than as a shell function; 
using the `command` prefix only helps with incompatible functions.


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

* Re: vcs_info: Use ‘command’ prefix to call version control programs
  2014-10-22 12:42         ` Aaron Schrab
@ 2014-10-22 13:43           ` Frank Terbeck
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Terbeck @ 2014-10-22 13:43 UTC (permalink / raw)
  To: zsh-workers

Aaron Schrab wrote:
> At 14:03 +0200 22 Oct 2014, Frank Terbeck <ft@bewatermyfriend.org> wrote:
>> That style could also be used to switch to a different binary, depending on
>> the current working copy. That's kind of what that style was introduced for.
>
> But somebody using a wrapper for that would likely want it to be used for
> things other than vcs_info, and the style wouldn't help there.

True. This is compelling enough for me to back out of my patch.


Regards, Frank


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

* Re: [PATCH] vcs_info: Use ‘command’ prefix to call version control programs
  2014-10-22 12:03       ` Frank Terbeck
  2014-10-22 12:42         ` Aaron Schrab
@ 2014-10-22 12:54         ` Manfred Lotz
  2014-10-22 13:51           ` Frank Terbeck
  1 sibling, 1 reply; 9+ messages in thread
From: Manfred Lotz @ 2014-10-22 12:54 UTC (permalink / raw)
  To: zsh-workers

Thanks all for answering.

On Wed, 22 Oct 2014 14:03:13 +0200
Frank Terbeck <ft@bewatermyfriend.org> wrote:

> Hey Daniel,
> 
> Daniel Shahaf wrote:
> > Frank Terbeck wrote on Wed, Oct 22, 2014 at 11:28:24 +0200:
> [...]
> >>     I think this is reasonable. I hope, people don't count on
> >>     wrapper functions to work to achieve some sort of sinister
> >>     hack. :)
> >> 
> >
> > I hate to say this after you already wrote the patch, but:
> 
> No worries. Changing those lines took like five minutes. :)
> 
> > There are legitimate use-cases for wrapper functions, and your patch
> > would break them.  (For example, someone might use a wrapper
> > function to dispatch to a different svn binary depending on the
> > working copy they are in.)
> 
> I was afraid someone might say that. :)
> 

I agree there are use-cases for a wrapper function. :-)

Although in my case I could fix it easily in my function svn() because
I always issue a debug message to stdout (in this case stderr would
be better I have to admit) which makes normal parsing of the svn command
fail if function svn gets called instead of command svn. After removing
it all is fine now.

Nevertheless, I think a wrapper function isn't illegal and shouldn't
fail. 


-- 
Manfred




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

* Re: [PATCH] vcs_info: Use ‘command’ prefix to call version control programs
  2014-10-22 12:54         ` [PATCH] " Manfred Lotz
@ 2014-10-22 13:51           ` Frank Terbeck
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Terbeck @ 2014-10-22 13:51 UTC (permalink / raw)
  To: Manfred Lotz; +Cc: zsh-workers

Manfred Lotz wrote:
[...]
> I agree there are use-cases for a wrapper function. :-)
>
> Although in my case I could fix it easily in my function svn() because
> I always issue a debug message to stdout (in this case stderr would
> be better I have to admit) which makes normal parsing of the svn command
> fail if function svn gets called instead of command svn. After removing
> it all is fine now.

You can keep your function if you do this (as Daniel suggested):

    zstyle ':vcs_info:svn:*' command =svn

This assumes, you have the EQUALS option set to make the =svn expansion
work (this is the default behaviour of zsh). You could otherwise do
this:

    zstyle ':vcs_info:svn:*' command ${commands[svn]}

> Nevertheless, I think a wrapper function isn't illegal and shouldn't
> fail. 

If you change its behaviour in incompatible ways, there is no way to
make this work while still allowing the use of wrapper functions within
vcs_info.

Either your wrapper behaves like the original program would for normal
program invocations or vcs_info always uses the command instead of any
functions that may be defined. You can't have it both ways.

Anyway. I think using an explicit ‘command’ style with incompatible
wrappers is the best way to ensure functionality as well as backwards
compatibility.

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925

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

end of thread, other threads:[~2014-10-22 14:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-22  4:24 VCS_INFO_get_data_svn:31: bad set of key/value pairs for associative array Manfred Lotz
2014-10-22  6:22 ` Bart Schaefer
2014-10-22  9:28   ` [PATCH] vcs_info: Use ‘command’ prefix to call version control programs Frank Terbeck
2014-10-22 10:53     ` Daniel Shahaf
2014-10-22 12:03       ` Frank Terbeck
2014-10-22 12:42         ` Aaron Schrab
2014-10-22 13:43           ` Frank Terbeck
2014-10-22 12:54         ` [PATCH] " Manfred Lotz
2014-10-22 13:51           ` Frank Terbeck

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