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