vcs_info: quoting (backticks) in git_patches_applied

vcs_info: quoting (backticks) in git_patches_applied

[Daniel Hahler]

I've noticed that there is a problem when using `%m` in vcs_info,
where the default `get-applied-string` hook returns something with
backticks in it:

> 6410ed117e51a4288d06132d18e2ffdf33f51313 Add snippets `'`, `"` and `doc`

While the backticks can be replaced in the prompt itself, it might be
better to quote it in `VCS_INFO_git_handle_patches`:

    diff --git i/Functions/VCS_Info/Backends/VCS_INFO_get_data_git w/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
    index ee50be6..371e935 100644
    --- i/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
    +++ w/Functions/VCS_Info/Backends/VCS_INFO_get_data_git
    @@ -121,7 +121,7 @@ VCS_INFO_git_handle_patches () {

         if VCS_INFO_hook 'gen-applied-string' "${git_patches_applied[@]}"; then
             if (( ${#git_patches_applied} )); then
    -            git_applied_s=${git_patches_applied[1]}
    +            git_applied_s=${(qqq)git_patches_applied[1]}
             else
                 git_applied_s=""
             fi

There is probably a better place to apply the quoting, e.g. when reading
the patches from the file, or a more central place in vcs_info, because
this is likely to affect other backends, too.


Regards,
Daniel.

Re: vcs_info: quoting (backticks) in git_patches_applied

[Frank Terbeck]

Hello Daniel,

Daniel Hahler wrote:
> I've noticed that there is a problem when using `%m` in vcs_info,
> where the default `get-applied-string` hook returns something with

I'm a bit confused. There is no default gen-applied-string hook.

> backticks in it:
>
>> 6410ed117e51a4288d06132d18e2ffdf33f51313 Add snippets `'`, `"` and `doc`

And there is no such commit in zsh's git repository.

> While the backticks can be replaced in the prompt itself, it might be
> better to quote it in `VCS_INFO_git_handle_patches`:
[...]
>     -            git_applied_s=${git_patches_applied[1]}
>     +            git_applied_s=${(qqq)git_patches_applied[1]}
[...]
> There is probably a better place to apply the quoting, e.g. when reading
> the patches from the file, or a more central place in vcs_info, because
> this is likely to affect other backends, too.

I don't think we should post-process these values. The hooks should
return properly quoted data (I suppose your problem is that your hook
does returns a - maybe even single - backtick and you use the
prompt_subst option which would cause problems) as the rest of the
configuration requires.

Correct me if I'm missing something.

Regards, Frank

Re: vcs_info: quoting (backticks) in git_patches_applied

[Daniel Hahler]

On 08.10.2014 23:09, Frank Terbeck wrote:

>> I've noticed that there is a problem when using `%m` in vcs_info,
>> where the default `get-applied-string` hook returns something with

> I'm a bit confused. There is no default gen-applied-string hook.

You are right - there is no default hook, but the default handling in case of no hook.

>>> 6410ed117e51a4288d06132d18e2ffdf33f51313 Add snippets `'`, `"` and `doc`
> And there is no such commit in zsh's git repository.

The commit is not in zsh's history, but in another private/local repo/branch.

>> While the backticks can be replaced in the prompt itself, it might be
>> better to quote it in `VCS_INFO_git_handle_patches`:
> [...]
>>     -            git_applied_s=${git_patches_applied[1]}
>>     +            git_applied_s=${(qqq)git_patches_applied[1]}
> [...]
>> There is probably a better place to apply the quoting, e.g. when reading
>> the patches from the file, or a more central place in vcs_info, because
>> this is likely to affect other backends, too.
>
> I don't think we should post-process these values. The hooks should
> return properly quoted data

I've added an own gen-applied-string hook, which quotes the value, but the default behavior is to get the data via:

    elif [[ -d "${gitdir}/rebase-merge" ]]; then
        patchdir="${gitdir}/rebase-merge"
        local p
        for p in ${(f)"$(< "${patchdir}/done")"}; do
            # remove action
            git_patches_applied+=("${${(s: :)p}[2,-1]}")
        done

The quoting needs to get applied to all places where the Git sources are being read in Functions/VCS_Info/Backends/VCS_INFO_get_data_git then probably, and also in the code for other VCS (e.g. hg).

> (I suppose your problem is that your hook
> does returns a - maybe even single - backtick and you use the
> prompt_subst option which would cause problems) as the rest of the
> configuration requires.

That's correct.


Regards,
Daniel.

Re: vcs_info: quoting (backticks) in git_patches_applied

[Bart Schaefer]

On Wed, Oct 8, 2014 at 2:09 PM, Frank Terbeck <ft@bewatermyfriend.org> wrote:
> Hello Daniel,
>
> Daniel Hahler wrote:
>> backticks in it:
>>
>>> 6410ed117e51a4288d06132d18e2ffdf33f51313 Add snippets `'`, `"` and `doc`
>
> And there is no such commit in zsh's git repository.

That's not a zsh commit, it's an example from some other software's
git repository where the log message happens to use backticks.

Re: vcs_info: quoting (backticks) in git_patches_applied

[Frank Terbeck]

Daniel Hahler wrote:
> On 08.10.2014 23:09, Frank Terbeck wrote:
[...]
> The quoting needs to get applied to all places where the Git sources
> are being read in Functions/VCS_Info/Backends/VCS_INFO_get_data_git
> then probably, and also in the code for other VCS (e.g. hg).
>
>> (I suppose your problem is that your hook
>> does returns a - maybe even single - backtick and you use the
>> prompt_subst option which would cause problems) as the rest of the
>> configuration requires.
>
> That's correct.

I guess we could introduce a function that does some form of
post-processing , if a style is set. Valid settings could be "quote" or
"strip" and "function", the latter would dispatch to a named function,
for example for the user to get full control. That function could be
used everywhere where a string from an external program is used.

That way, prompt_subst users (if you don't use prompt_subst, which is
the default, none of this is an issue) can get post-processing if they
like. Actually, we could make the default depend on the state of the
option when vcs_info initialises.

Unfortunately, I can't look into it right now due to lack of spare time.


Regards, Frank