vcs_info: '%' in payloads not escaped

vcs_info: '%' in payloads not escaped

[Daniel Shahaf]

To reproduce:

[[[
autoload vcs_info
vcs_info
precmd () {
        vcs_info
	print -Plr -- ${vcs_info_msg_0_} ${vcs_info_msg_1_}
}
zstyle ':vcs_info:*' formats ' (%s)-[%b]%u%c-' %m
zstyle ':vcs_info:*' actionformats ' (%s)-[%b|%a]%u%c-' %m

cd "$(mktemp -d)"
git init
echo foo > foo
git add foo
git commit -am import > /dev/null
git branch br
echo foo2 >> foo
git commit -am "left"
git checkout br
echo foo2b >> foo
git commit -am "%F{red} right"
git checkout --detach
git rebase master
]]]

At the end of the script, $vcs_info_msg_1_ is printed as
"6c0df9e1f8ebcdbeb94b08d906e068086ec76709  right (1 applied)"
with the "right (1 applied)" part in red.

This also happens after «hg branch "%F{red} foo"», etc..

I work around this hook by doing ${1//'%'/%%} in my gen-applied-string
hook.  I assume vcs_info itself should be doing that, but I'm not sure
where in the code to add that.  Is the following correct?

[[[
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_bzr b/Functions/VCS_Info/Backends/VCS_INFO_get_data_bzr
index e8c8e81..e9180b0 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_bzr
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_bzr
@@ -100,7 +100,7 @@ rrn=${bzrbase:t}
 zstyle -s ":vcs_info:${vcs}:${usercontext}:${rrn}" branchformat bzrbr || bzrbr="%b:%r"
 hook_com=( branch "${bzrinfo[2]}" revision "${bzrinfo[1]}" )
 if VCS_INFO_hook 'set-branch-format' "${bzrbr}"; then
-    zformat -f bzrbr "${bzrbr}" "b:${hook_com[branch]}" "r:${hook_com[revision]}"
+    zformat -f bzrbr "${bzrbr}" "b:${hook_com[branch]//'%'/%%}" "r:${hook_com[revision]}"
 else
     bzrbr=${hook_com[branch-replace]}
 fi
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
index 4ec87c6..6272f69 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
@@ -149,7 +149,7 @@ VCS_INFO_git_handle_patches () {
     hook_com=( applied "${git_applied_s}"     unapplied "${git_patches_unapplied}"
                applied-n ${#git_patches_applied} unapplied-n ${#git_patches_unapplied} all-n ${git_all} )
     if VCS_INFO_hook 'set-patch-format' "${gitmsg}"; then
-        zformat -f gitmisc "${gitmsg}" "p:${hook_com[applied]}" "u:${hook_com[unapplied]}" \
+        zformat -f gitmisc "${gitmsg}" "p:${hook_com[applied]//'%'/%%}" "u:${hook_com[unapplied]//'%'/%%}" \
                                           "n:${#git_patches_applied}" "c:${#git_patches_unapplied}" "a:${git_all}"
     else
         gitmisc=${hook_com[patch-replace]}
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
index 69b7db3..8c902a7 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
@@ -113,7 +113,7 @@ hook_com=( branch "${r_branch}" revision "${r_lrev}" )
 
 if VCS_INFO_hook 'set-branch-format' "${branchformat}"; then
     zformat -f branchformat "${branchformat}" \
-        "b:${hook_com[branch]}" "r:${hook_com[revision]}"
+        "b:${hook_com[branch]//'%'/%%}" "r:${hook_com[revision]}"
 else
     branchformat=${hook_com[branch-replace]}
 fi
@@ -239,9 +239,9 @@ if zstyle -T ":vcs_info:${vcs}:${usercontext}:${rrn}" get-mq \
 
     if VCS_INFO_hook 'set-patch-format' ${qstring}; then
         zformat -f hgmqstring "${hgmqstring}" \
-            "p:${hook_com[applied]}" "u:${hook_com[unapplied]}" \
+            "p:${hook_com[applied]//'%'/%%}" "u:${hook_com[unapplied]//'%'/%%}" \
             "n:${#mqpatches}" "c:${#mqunapplied}" "a:${#mqseries}" \
-            "g:${hook_com[guards]}" "G:${#mqguards}"
+            "g:${hook_com[guards]//'%'/%%}" "G:${#mqguards}"
     else
         hgmqstring=${hook_com[patch-replace]}
     fi
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_p4 b/Functions/VCS_Info/Backends/VCS_INFO_get_data_p4
index 3298849..c23f4f3 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_p4
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_p4
@@ -20,7 +20,7 @@ change="${${$(${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
-    zformat -f p4branch "${p4branch}" "b:${hook_com[branch]}" "r:${hook_com[revision]}"
+    zformat -f p4branch "${p4branch}" "b:${hook_com[branch]//'%'/%%}" "r:${hook_com[revision]}"
 else
     p4branch=${hook_com[branch-replace]}
 fi
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_svn b/Functions/VCS_Info/Backends/VCS_INFO_get_data_svn
index e1334f6..19affbd 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_svn
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_svn
@@ -52,7 +52,7 @@ rrn=${svnbase:t}
 zstyle -s ":vcs_info:${vcs}:${usercontext}:${rrn}" branchformat svnbranch || svnbranch="%b:%r"
 hook_com=( branch "${svninfo[URL]##*/}" revision "${cwdinfo[Revision]}" )
 if VCS_INFO_hook 'set-branch-format' "${svnbranch}"; then
-    zformat -f svnbranch "${svnbranch}" "b:${hook_com[branch]}" "r:${hook_com[revision]}"
+    zformat -f svnbranch "${svnbranch}" "b:${hook_com[branch]//'%'/%%}" "r:${hook_com[revision]}"
 else
     svnbranch=${hook_com[branch-replace]}
 fi
diff --git a/Functions/VCS_Info/VCS_INFO_quilt b/Functions/VCS_Info/VCS_INFO_quilt
index 4c61506..a39cd1a 100644
--- a/Functions/VCS_Info/VCS_INFO_quilt
+++ b/Functions/VCS_Info/VCS_INFO_quilt
@@ -197,7 +197,7 @@ function VCS_INFO_quilt() {
     hook_com=( applied "${applied_string}" unapplied "${unapplied_string}"
                applied-n ${#applied}       unapplied-n ${#unapplied}       all-n ${#all} )
     if VCS_INFO_hook 'set-patch-format' ${qstring}; then
-        zformat -f qstring "${qstring}" "p:${hook_com[applied]}" "u:${hook_com[unapplied]}" \
+        zformat -f qstring "${qstring}" "p:${hook_com[applied]//'%'/%%}" "u:${hook_com[unapplied]//'%'/%%}" \
                                         "n:${#applied}" "c:${#unapplied}" "a:${#all}"
     else
         qstring=${hook_com[patch-replace]}
]]]

Re: vcs_info: '%' in payloads not escaped

[Daniel Shahaf]

Daniel Shahaf wrote on Tue, Dec 27, 2016 at 15:05:07 +0000:
> I work around this hook by doing ${1//'%'/%%} in my gen-applied-string
> hook.  I assume vcs_info itself should be doing that, but I'm not sure
> where in the code to add that.  Is the following correct?
> 
> [[[
> diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
> index 4ec87c6..6272f69 100644
> --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
> +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
> @@ -149,7 +149,7 @@ VCS_INFO_git_handle_patches () {
>      hook_com=( applied "${git_applied_s}"     unapplied "${git_patches_unapplied}"
>                 applied-n ${#git_patches_applied} unapplied-n ${#git_patches_unapplied} all-n ${git_all} )
>      if VCS_INFO_hook 'set-patch-format' "${gitmsg}"; then
> -        zformat -f gitmisc "${gitmsg}" "p:${hook_com[applied]}" "u:${hook_com[unapplied]}" \
> +        zformat -f gitmisc "${gitmsg}" "p:${hook_com[applied]//'%'/%%}" "u:${hook_com[unapplied]//'%'/%%}" \
>                                            "n:${#git_patches_applied}" "c:${#git_patches_unapplied}" "a:${git_all}"

So this breaks hooks that intentionally inject coloring sequences:

  +vi-git-applied-string() {
    hook_com[applied-string]="%F{yellow}$1%f"
    ret=1
  }

I suppose we could %-escape the patch subject before we call
applied-string, but then applied-string hooks that run «echo $1»
will suddenly get doubled percent signs in there.

Or maybe that's not a supported use of applied-string.

Not sure how to proceed.

Cheers,

Daniel

Re: vcs_info: '%' in payloads not escaped

[Daniel Shahaf]

Daniel Shahaf wrote on Tue, Dec 27, 2016 at 15:05:07 +0000:
> [[[
> autoload vcs_info
> vcs_info
> precmd () {
>         vcs_info
> 	print -Plr -- ${vcs_info_msg_0_} ${vcs_info_msg_1_}
> }
> zstyle ':vcs_info:*' formats ' (%s)-[%b]%u%c-' %m
> zstyle ':vcs_info:*' actionformats ' (%s)-[%b|%a]%u%c-' %m
> 
> cd "$(mktemp -d)"
> git init
> echo foo > foo
> git add foo
> git commit -am import > /dev/null
> git branch br
> echo foo2 >> foo
> git commit -am "left"
> git checkout br
> echo foo2b >> foo
> git commit -am "%F{red} right"
> git checkout --detach
> git rebase master
> ]]]
> 
> At the end of the script, $vcs_info_msg_1_ is printed as
> "6c0df9e1f8ebcdbeb94b08d906e068086ec76709  right (1 applied)"
> with the "right (1 applied)" part in red.
> 
> This also happens after «hg branch "%F{red} foo"», etc..
> 
> I work around this hook by doing ${1//'%'/%%} in my gen-applied-string
> hook.  I assume vcs_info itself should be doing that, but I'm not sure
> where in the code to add that.

How about the following?

[[[
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
index 63109aa..84382ad 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
@@ -131,12 +131,14 @@ VCS_INFO_git_handle_patches () {
         else
             git_applied_s=""
         fi
+        git_applied_s=${git_applied_s//'%'/%%}
     else
         git_applied_s=${hook_com[applied-string]}
     fi
     hook_com=()
     if VCS_INFO_hook 'gen-unapplied-string' "${git_patches_unapplied[@]}"; then
         git_patches_unapplied=${#git_patches_unapplied}
+        git_patches_unapplied=${git_patches_unapplied//'%'/%%}
     else
         git_patches_unapplied=${hook_com[unapplied-string]}
     fi
]]]

This adds % escaping only when the hook was not called.

If this is correct then similar changes would be needed to the quilt,
hg, and other codepaths that suffer from the same problem, 

Escaping % signs in 'git_patches_unapplied' is just for future-proofing,
in case the immediately preceding line changes.

Cheers,

Daniel
[for the archives: some additional discussion about this issue occured
in users/22321]

Re: vcs_info: '%' in payloads not escaped

[Frank Terbeck]

Hey Daniel!

Daniel Shahaf wrote:
[...]
> How about the following?
>          else
>              git_applied_s=""
>          fi
> +        git_applied_s=${git_applied_s//'%'/%%}
>      else
[...]
>          git_patches_unapplied=${#git_patches_unapplied}
> +        git_patches_unapplied=${git_patches_unapplied//'%'/%%}
>      else

I honestly don't know. Isn't this like kind-of-predictable behavior
versus a — potentially — a lot of special cases? I don't think that it's
possible to get this right in the general case. It's in-band data that
is indistinguishable from data that is interpreted by something that
interprets zsh's prompt language.


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

Re: vcs_info: '%' in payloads not escaped

[Daniel Shahaf]

Frank Terbeck wrote on Thu, Jan 05, 2017 at 17:27:06 +0100:
> Hey Daniel!
> 
> Daniel Shahaf wrote:
> [...]
> > How about the following?
> >          else
> >              git_applied_s=""
> >          fi
> > +        git_applied_s=${git_applied_s//'%'/%%}
> >      else
> [...]
> >          git_patches_unapplied=${#git_patches_unapplied}
> > +        git_patches_unapplied=${git_patches_unapplied//'%'/%%}
> >      else
> 
> I honestly don't know. Isn't this like kind-of-predictable behavior
> versus a — potentially — a lot of special cases? I don't think that it's
> possible to get this right in the general case. It's in-band data that
> is indistinguishable from data that is interpreted by something that
> interprets zsh's prompt language.

A lot of special cases, how?  $git_applied_s contains a string derived
from git, so we know that any and all percent signs in it need escaping.
That's why the escaping is done before the first zformat call, and only
if user hooks were not called: because that's the only case in which we
know for certain what does and doesn't need escaping.

With this patch, the following invariant holds: immediately after the
`fi` that ends the `if VCS_INFO_hook …` condition, $git_applied_s is
properly %-escaped — either via the hook obeying the `Oddities'
contract, or (if there's no hook) via the code added by this patch.

I'm sure there's a way to escape payloads sanely, and we just need to
figure out what that is...

Cheers,

Daniel

Re: vcs_info: '%' in payloads not escaped

[Frank Terbeck]

Daniel Shahaf wrote:
> Frank Terbeck wrote on Thu, Jan 05, 2017 at 17:27:06 +0100:
[...]
>> I honestly don't know. Isn't this like kind-of-predictable behavior
>> versus a — potentially — a lot of special cases? I don't think that it's
>> possible to get this right in the general case. It's in-band data that
>> is indistinguishable from data that is interpreted by something that
>> interprets zsh's prompt language.
>
> A lot of special cases, how?  $git_applied_s contains a string derived
> from git, so we know that any and all percent signs in it need escaping.

The percent characters need escaping if the result is to be used in zsh
prompts. In other cases, like tmux window titles, they don't. But maybe
the hash sign (#) does, over there.

I think this is already non-trivial in zsh itself, because there may be
uses that exceed prompts. But in the general case, I don't think its
possible.


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

Re: vcs_info: '%' in payloads not escaped

[Bart Schaefer]

On Jan 5,  4:07pm, Daniel Shahaf wrote:
}
} How about the following?
} 
} [[[
} @@ -131,12 +131,14 @@ VCS_INFO_git_handle_patches () {
}          else
}              git_applied_s=""
}          fi
} +        git_applied_s=${git_applied_s//'%'/%%}
}      else
}          git_applied_s=${hook_com[applied-string]}
}      fi

Shouldn't that + go above the "else"?  No point in substituting on
git_applied_s when it has just been set to "" ?

With respect to the rest of the thread between you and Frank, I
understand Frank's concern but I think you have it right.

Re: vcs_info: '%' in payloads not escaped

[Daniel Shahaf]

Frank Terbeck wrote on Fri, Jan 06, 2017 at 11:41:59 +0100:
> Daniel Shahaf wrote:
> > Frank Terbeck wrote on Thu, Jan 05, 2017 at 17:27:06 +0100:
> [...]
> >> I honestly don't know. Isn't this like kind-of-predictable behavior
> >> versus a — potentially — a lot of special cases? I don't think that it's
> >> possible to get this right in the general case. It's in-band data that
> >> is indistinguishable from data that is interpreted by something that
> >> interprets zsh's prompt language.
> >
> > A lot of special cases, how?  $git_applied_s contains a string derived
> > from git, so we know that any and all percent signs in it need escaping.
> 
> The percent characters need escaping if the result is to be used in zsh
> prompts. In other cases, like tmux window titles, they don't. But maybe
> the hash sign (#) does, over there.
> 
> I think this is already non-trivial in zsh itself, because there may be
> uses that exceed prompts. But in the general case, I don't think its
> possible.

Well, of course I'm not going to write an escape function that produces
a string that can be inserted, verbatim, into any context.  That's
mathematically impossible.

However, I do think this problem is solvable.  The string we produce in
$vcs_info_msg_N_ should have a defined escaping.  I would suggest to
define that that string is prompt-escaped — which would be consistent
with existing code, such as
.
    precmd() { print -Plr -- ${vcs_info_msg_0_} ${vcs_info_msg_1_} }
.
and
.
    setopt promptsubst
    PS1='${vcs_info_msg_0_}%# '
.
— and then, if people want to use that string in tmux, they can:
.
     1	local tmux_command
     2	print -v tmux_command -Pr -- $vcs_info_msg_0_
     3	tmux set-option set-titles-string ${vcs_info_msg_0_//'#'/##}

All of these code excerpts work today, and I will commit no patch that
breaks either of them.  So I'm still not sure what your concern is.

Suppose, for example, the string that is to be printed is "Lorem ipsum
50% Foo#Bar".  In this case, after `vcs_info` returns, $vcs_info_msg_0_
is set to "Lorem ipsum 50%% Foo#Bar"; the `print -v -Pr` de-doubles the
percent; and line three escapes the hash sign.  Perfectly predictable,
well-defined, consistent, generalizable, and all those other good
adjectives...  isn't it?

Cheers,

Daniel

Re: vcs_info: '%' in payloads not escaped

[Daniel Shahaf]

Bart Schaefer wrote on Fri, Jan 06, 2017 at 07:55:24 -0800:
> On Jan 5,  4:07pm, Daniel Shahaf wrote:
> }
> } How about the following?
> } 
> } [[[
> } @@ -131,12 +131,14 @@ VCS_INFO_git_handle_patches () {
> }          else
> }              git_applied_s=""
> }          fi
> } +        git_applied_s=${git_applied_s//'%'/%%}
> }      else
> }          git_applied_s=${hook_com[applied-string]}
> }      fi
> 
> Shouldn't that + go above the "else"?  No point in substituting on
> git_applied_s when it has just been set to "" ?

I was just future-proofing the code against potential future changes in
the 'else' branch.  See users/22290 ;-)

Re: vcs_info: '%' in payloads not escaped

[Bart Schaefer]

On Jan 6,  4:40pm, Daniel Shahaf wrote:
}
} However, I do think this problem is solvable.  The string we produce in
} $vcs_info_msg_N_ should have a defined escaping.  I would suggest to
} define that that string is prompt-escaped - which would be consistent
} with existing code

Yes, having a well-defined reversible escaping of the string would be
the best way to go.  (Reversible in that e.g. ${(Q)...} removes it,
or ${(%)...} does.)

I'm not very familiar with VCS_info.  Frank seemed concerned that the
same variable might contain substrings both from git output that
had one set of quoting needs and also from other sources that had
another set of quoting needs.

If this is indeed the case then there may not be a single reversible
quoting that can be applied to the value as a whole.  To fix this we
would need to provide more variations of the string, e.g., one that
is not escaped and one that is prompt-escaped.

Re: vcs_info: '%' in payloads not escaped

[Daniel Shahaf]

Coming back to this thread...

Bart Schaefer wrote on Fri, Jan 06, 2017 at 09:27:11 -0800:
> I'm not very familiar with VCS_info.  Frank seemed concerned that the
> same variable might contain substrings both from git output that
> had one set of quoting needs and also from other sources that had
> another set of quoting needs.

That's exactly what happens.

The code flow is:

     1	if gen-applied-string hook is active:
     2	    applied_string=$(what the hook returns)
     3	else:
     4	    applied_string="literal value from the VCS"
     5	
     6	if set-patch-format hook is active:
     7	    hook_com[applied]=$applied_string
     8	    percent_m=$(what the hook returns)
     9	else:
    10	    percent_m=$(zformat ... p:${applied_string})
    11	
    12	VCS_INFO_formats … $percent_m

The catch is that both hooks have access to $applied_string, and either
of them might %-escape it and added prompt sequences (e.g., for
coloring) around it.  That is: once gen-applied-string has run, we don't
know whether it has already %-escaped the string it returns, and by
extension we don't know whether set-patch-format expects its
${hook_com[applied]} input to be literal or %-escaped.

So perhaps what we _can_ do is:

1) If *neither* hook was called, have vcs_info itself perform %-escaping
appropriately.  (That is: perform escaping on line 7 iff line 4 was
taken.)

2) Document that if *either* hook is registered, then handling
%-escaping is up to the user.

Would that address everyone's concerns?

Cheers,

Daniel
(optimizing for backwards compatibility)

Re: vcs_info: '%' in payloads not escaped

[Frank Terbeck]

Hey,

Daniel Shahaf wrote:
[...]
> 1) If *neither* hook was called, have vcs_info itself perform %-escaping
> appropriately.  (That is: perform escaping on line 7 iff line 4 was
> taken.)
>
> 2) Document that if *either* hook is registered, then handling
> %-escaping is up to the user.

Sounds like a reasonable idea to me.


Regards, Frank

[PATCH] vcs_info: Escape '%' signs in payloads

[Daniel Shahaf]

Test case: a patch whose subject is '%Sfoo%sbar'.  ('S' and 's' are
expandos both in prompts and in the 'formats' style.)
---
 Doc/Zsh/contrib.yo                               | 17 +++++++++++++++++
 Etc/BUGS                                         |  7 -------
 Functions/VCS_Info/Backends/VCS_INFO_get_data_hg |  1 +
 Functions/VCS_Info/VCS_INFO_set-patch-format     | 14 ++++++++++++++
 README                                           |  9 +++++++++
 5 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/Doc/Zsh/contrib.yo b/Doc/Zsh/contrib.yo
index a454f60..5d0696d 100644
--- a/Doc/Zsh/contrib.yo
+++ b/Doc/Zsh/contrib.yo
@@ -1344,6 +1344,12 @@ tt(branchformat), use tt(%%%%b). Sorry for this inconvenience, but it
 cannot be easily avoided. Luckily we do not clash with a lot of prompt
 expansions and this only needs to be done for those.
 
+When one of the tt(gen-applied-string), tt(gen-unapplied-string), and
+tt(set-patch-format) hooks is defined,
+applying tt(%)-escaping (`tt(foo=${foo//'%'/%%})') to the interpolated values
+for use in the prompt is the responsibility of those hooks (jointly);
+when neither of those hooks is defined, tt(vcs_info) handles escaping by itself.
+We regret this coupling, but it was required for backwards compatibility.
 
 subsect(Quilt Support)
 
@@ -1616,6 +1622,9 @@ top-most patch and so forth.
 When setting tt(ret) to non-zero, the string in
 tt(${hook_com[applied-string]}) will be
 available as tt(%p) in the tt(patch-format) and tt(nopatch-format) styles.
+This hook is, in concert with tt(set-patch-format), responsible for
+tt(%)-escaping that value for use in the prompt.
+(See the `Oddities' section.)
 )
 item(tt(gen-unapplied-string))(
 Called in the tt(git) (with tt(stgit) or during rebase), and tt(hg) (with
@@ -1629,6 +1638,9 @@ the patch next-in-line to be applied and so forth.
 When setting tt(ret) to non-zero, the string in
 tt(${hook_com[unapplied-string]}) will be available as tt(%u) in the
 tt(patch-format) and tt(nopatch-format) styles.
+This hook is, in concert with tt(set-patch-format), responsible for
+tt(%)-escaping that value for use in the prompt.
+(See the `Oddities' section.)
 )
 item(tt(gen-mqguards-string))(
 Called in the tt(hg) backend when tt(guards-string) is generated; the
@@ -1706,6 +1718,11 @@ controllable in addition to that.
 If tt(ret) is set to non-zero, the string in tt(${hook_com[patch-replace]})
 will be used unchanged instead of an expanded format from tt(patch-format) or
 tt(nopatch-format).
+
+This hook is, in concert with the tt(gen-applied-string) or
+tt(gen-unapplied-string) hooks if they are defined, responsible for
+tt(%)-escaping the final tt(patch-format) value for use in the prompt.
+(See the `Oddities' section.)
 )
 item(tt(set-message))(
 Called each time before a `tt(vcs_info_msg_)var(N)tt(_)' message is set.
diff --git a/Etc/BUGS b/Etc/BUGS
index a351464..008e337 100644
--- a/Etc/BUGS
+++ b/Etc/BUGS
@@ -15,13 +15,6 @@ It is currently impossible to time builtins.
 40106: The comp* completion-related builtins (compadd, compset, etc) are
 run with $_comp_options in effect, rather than the user's options.
 ------------------------------------------------------------------------
-40240: vcs_info: percent escapes in payloads are interpreted
-
-Example: hg branch names and quilt patch subjects that contain the literal
-string '%F{blue}', cause $vcs_info_msg_N_ to be rendered in blue.
-
-40240 has a patch, but 40241 explains why that patch is incomplete.
-------------------------------------------------------------------------
 users/20807: vcs_info quilt 'addon' mode: hook lookup context specifies
 the underlying VCS but not whether quilt is used.
 
diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
index 143eb42..d403012 100644
--- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
+++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_hg
@@ -202,6 +202,7 @@ if zstyle -T ":vcs_info:${vcs}:${usercontext}:${rrn}" get-mq \
 
     if VCS_INFO_hook 'gen-mqguards-string' "${mqguards[@]}"; then
         guards_string=${(j:,:)mqguards}
+        # TODO: %-escape extra_zformats[g:...] value
     else
         guards_string=${hook_com[guards-string]}
     fi
diff --git a/Functions/VCS_Info/VCS_INFO_set-patch-format b/Functions/VCS_Info/VCS_INFO_set-patch-format
index c5da368..cdf2d30 100644
--- a/Functions/VCS_Info/VCS_INFO_set-patch-format
+++ b/Functions/VCS_Info/VCS_INFO_set-patch-format
@@ -18,12 +18,15 @@
 # - $hook_com is overwritten and the keys 'applied', 'applied-n',
 #   'unapplied', 'unapplied-n', 'all-n' are set.
 {
+    local applied_needs_escaping='unknown'
+    local unapplied_needs_escaping='unknown'
     if VCS_INFO_hook 'gen-applied-string' "${(@P)1}"; then
         if (( ${(P)#1} )); then
             REPLY=${(P)1[1]}
         else
             REPLY=""
         fi
+        applied_needs_escaping='yes'
     else
         REPLY=${hook_com[applied-string]}
     fi
@@ -32,6 +35,7 @@
 
     if VCS_INFO_hook 'gen-unapplied-string' "${(@P)3}"; then
         REPLY=${(P)#3}
+        unapplied_needs_escaping='yes'
     else
         REPLY=${hook_com[unapplied-string]}
     fi
@@ -54,12 +58,22 @@
     hook_com[all-n]=$(( ${hook_com[applied-n]} + ${hook_com[unapplied-n]} ))
     hook_com+=( ${7:+"${(@kvP)7}"} )
     if VCS_INFO_hook 'set-patch-format' "${(P)6}"; then
+        # Escape the value for use in $PS1
+        if [[ $applied_needs_escaping == 'yes' ]]; then
+          hook_com[applied]=${hook_com[applied]//'%'/%%}
+        fi
+        if [[ $unapplied_needs_escaping == 'yes' ]]; then
+          hook_com[unapplied]=${hook_com[unapplied]//'%'/%%}
+        fi
+
         zformat -f REPLY "${(P)6}" "p:${hook_com[applied]}" "u:${hook_com[unapplied]}" \
                                         "n:${hook_com[applied-n]}" "c:${hook_com[unapplied-n]}" \
                                         "a:${hook_com[all-n]}" \
                                         ${8:+"${(@P)8}"}
     else
+        unset applied_needs_escaping unapplied_needs_escaping # the hook deals with escaping
         REPLY=${hook_com[patch-replace]}
     fi
     hook_com=()
+
 }
diff --git a/README b/README
index 26eb245..594b6f1 100644
--- a/README
+++ b/README
@@ -62,6 +62,15 @@ Note that functions including a non-leading / behave as before,
 e.g. if `dir/name' is found anywhere under a directory in $fpath it is
 loaded as a function named `dir/name'.
 
+3) vcs_info: When neither a set-patch-format nor a gen-applied-string
+(resp. gen-unapplied-string) hook is set, vcs_info now '%'-escapes the
+applied-string (resp. unapplied-string) before interpolating it into the
+patch-format string, to prevent literal `%' signs in the interpolated
+value from being interpreted as prompt escape sequences.  If you use
+${vcs_info_msg_0_} in a context other than the shell prompt, you may need
+to undo the escaping with:
+    print -v vcs_info_msg_0_ -Pr -- "${vcs_info_msg_0_}"
+
 Incompatibilities between 5.0.8 and 5.3
 ----------------------------------------
 

Re: [PATCH] vcs_info: Escape '%' signs in payloads

[Daniel Shahaf]

Daniel Shahaf wrote on Sun, Feb 05, 2017 at 08:28:13 +0000:
> +++ b/README
> @@ -62,6 +62,15 @@ Note that functions including a non-leading / behave as before,
>  e.g. if `dir/name' is found anywhere under a directory in $fpath it is
>  loaded as a function named `dir/name'.
>  
> +3) vcs_info: When neither a set-patch-format nor a gen-applied-string
> +(resp. gen-unapplied-string) hook is set, vcs_info now '%'-escapes the
> +applied-string (resp. unapplied-string) before interpolating it into the
> +patch-format string, to prevent literal `%' signs in the interpolated
> +value from being interpreted as prompt escape sequences.  If you use
> +${vcs_info_msg_0_} in a context other than the shell prompt, you may need
> +to undo the escaping with:
> +    print -v vcs_info_msg_0_ -Pr -- "${vcs_info_msg_0_}"

Another variant of this: the "Episode II" variant in Misc/vcs_info-examples
expected $vcs_info_msg_0_ to be a literal string, whereas "Episode I"
and "Episode III" expected $vcs_info_msg_0_ to be prompt-escaped.

Since we can't be backwards compatible with both of the different
semantics, I'm voting for breaking the psvar syntax, on the assumption
that it's less widely used.  (so that'd be the lesser of two evils)

The failure mode for people who upgrade to vcs_info 5.4 without applying
the following patch to their dotfiles' precmd functions is that any and
all literal % signs in patch subjects would be doubled; so a patch that
said "10% optimization" would render as "10%% optimization".

Cheers,

Daniel

P.S. Tangent: I've been wondering how many people know that if they put
%m in their actionformats, they can get information about in-progress
rebases into their prompts.  That detail seems to be buried in the
depths of the reference manual...


diff --git a/Misc/vcs_info-examples b/Misc/vcs_info-examples
index 766eb82..58dd8cf 100644
--- a/Misc/vcs_info-examples
+++ b/Misc/vcs_info-examples
@@ -31,7 +31,7 @@ precmd() {
     psvar=()
 
     vcs_info
-    [[ -n $vcs_info_msg_0_ ]] && psvar[1]="$vcs_info_msg_0_"
+    [[ -n $vcs_info_msg_0_ ]] && print -v 'psvar[1]' -Pr -- "$vcs_info_msg_0_"
 }
 
 # You can now use `%1v' to drop the $vcs_info_msg_0_ contents in your prompt;
diff --git a/README b/README
index 594b6f1..432a35e 100644
--- a/README
+++ b/README
@@ -69,8 +69,11 @@ patch-format string, to prevent literal `%' signs in the interpolated
 value from being interpreted as prompt escape sequences.  If you use
 ${vcs_info_msg_0_} in a context other than the shell prompt, you may need
 to undo the escaping with:
+
     print -v vcs_info_msg_0_ -Pr -- "${vcs_info_msg_0_}"
 
+This is also needed if $vcs_info_msg_0_ is used to set $psvar.
+
 Incompatibilities between 5.0.8 and 5.3
 ----------------------------------------