junegunn / fzf

:cherry_blossom: A command-line fuzzy finder
https://junegunn.github.io/fzf/
MIT License
62.32k stars 2.35k forks source link

Escape issue in vim with latest changes on s:shellesc_cmd #3771

Closed DanSM-5 closed 2 months ago

DanSM-5 commented 2 months ago

Checklist

Output of fzf --version

0.51.0 (260a65b)

OS

Shell

Problem / Steps to reproduce

There is an issue in windows using (n)vim. The following bind used to work fine in windows

'--bind', 'ctrl-/:change-preview-window(down|hidden|)'

Now fzf is failing mentioning unknown action and I can quickly see some escaping added to the above binding

change-preview-window^(down^|hidden^|^)

If I go one commit before 835d2fb (the commit of the escape refactor) to a9811ad it works fine again.

Problematic patch:

+ let s:cmd_control_chars = ['&', '|', '<', '>', '(', ')', '@', '^', '!']
+
function! s:shellesc_cmd(arg)
-  let escaped = substitute(a:arg, '[&|<>()@^]', '^&', 'g')
-  let escaped = substitute(escaped, '%', '%%', 'g')
-  let escaped = substitute(escaped, '"', '\\^&', 'g')
-  let escaped = substitute(escaped, '\(\\\+\)\(\\^\)', '\1\1\2', 'g')
-  return '^"'.substitute(escaped, '\(\\\+\)$', '\1\1', '').'^"'
+  let e = '"'
+  let slashes = 0
+  for c in split(a:arg, '\zs')
+    if c ==# '\'
+      let slashes += 1
+    elseif c ==# '"'
+      let e .= repeat('\', slashes + 1)
+      let slashes = 0
+    elseif c ==# '%'
+      let e .= '%'
+    elseif index(s:cmd_control_chars, c) >= 0
+      let e .= '^'
+    else
+      let slashes = 0
+    endif
+    let e .= c
+  endfor
+  let e .= repeat('\', slashes) .'"'
+  return e
endfunction
DanSM-5 commented 2 months ago

I logged the resulted strings to compare

Escaped options now:

"--color=bg+:237,bg:235,spinner:214,hl:214,fg:223,header:241,info:109,pointer:109,marker:208,fg+:223,prompt:246,hl+:214"
"-m"
"--prompt"
"C:/Users/daniel/.usr_conf\\"
"--preview"
"C:\Windows\System32\bash.exe /mnt/c/Users/daniel/.cache/vimfiles/.cache/init.vim/.dein/bin/preview.sh {}"
"--bind"
"0:toggle-preview"
"--layout=reverse"
"--preview"
"bat -pp --color=always --style=numbers {}"
"--multi"
"--ansi"
"--info=inline"
"--bind"
"alt-c:clear-query"
"--bind"
"ctrl-l:change-preview-window^(down^|hidden^|^),ctrl-/:change-preview-window^(down^|hidden^|^),alt-up:preview-page-up,alt-down:preview-page-down"
"--bind"
"ctrl-s:toggle-sort"
"--cycle"
"--bind"
"alt-f:first"
"--bind"
"alt-l:last"
"--bind"
"alt-a:select-all"
"--bind"
"alt-d:deselect-all"

Escaped options pre-change:

^"--color=bg+:237,bg:235,spinner:214,hl:214,fg:223,header:241,info:109,pointer:109,marker:208,fg+:223,prompt:246,hl+:214^"
^"-m^"
^"--prompt^"
^"C:/Users/daniel/.usr_conf\\^"
^"--preview^"
^"C:\Windows\System32\bash.exe /mnt/c/Users/daniel/.cache/vimfiles/.cache/init.vim/.dein/bin/preview.sh {}^"
^"--bind^"
^"0:toggle-preview^"
^"--layout=reverse^"
^"--preview^"
^"bat -pp --color=always --style=numbers {}^"
^"--multi^"
^"--ansi^"
^"--info=inline^"
^"--bind^"
^"alt-c:clear-query^"
^"--bind^"
^"ctrl-l:change-preview-window^(down^|hidden^|^),ctrl-/:change-preview-window^(down^|hidden^|^),alt-up:preview-page-up,alt-down:preview-page-down^"
^"--bind^"
^"ctrl-s:toggle-sort^"
^"--cycle^"
^"--bind^"
^"alt-f:first^"
^"--bind^"
^"alt-l:last^"
^"--bind^"
^"alt-a:select-all^"
^"--bind^"
^"alt-d:deselect-all^"

I can see that the result is very similar but the quotes are not escaped now.

junegunn commented 2 months ago

Does this patch fix the issue?

https://github.com/junegunn/fzf/commit/5669f48343cac868eb7432950db61c9aa2383ab6

DanSM-5 commented 2 months ago

I was about to comment. This fixed the issue

@@ -86,7 +86,7 @@ endif
 let s:cmd_control_chars = ['&', '|', '<', '>', '(', ')', '@', '^', '!']

 function! s:shellesc_cmd(arg)
-  let e = '"'
+  let e = '^"'
   let slashes = 0
   for c in split(a:arg, '\zs')
     if c ==# '\'
@@ -103,7 +103,8 @@ function! s:shellesc_cmd(arg)
     endif
     let e .= c
   endfor
-  let e .= repeat('\', slashes) .'"'
+  let e .= repeat('\', slashes) .'^"'
   return e
 endfunction

but I'm unsure if getting rid of the escape on the quotes is the intention

junegunn commented 2 months ago

Please test if https://github.com/junegunn/fzf/commit/5669f48343cac868eb7432950db61c9aa2383ab6 fixes the problem. It also tries to fix other issues (for example :Rg ! from fzf.vim results in searching for ^!).

DanSM-5 commented 2 months ago

It seems like it fixed it in a quick test I did

DanSM-5 commented 2 months ago

I've done some further testing and it is working fine. A custom command broke with the changes but I was able to simplify the escaping so I'd say it is an improvement.

Before:

let fzf_rg_args = ' --glob=^"^!.git^" --glob=^"^!node_modules^" --column --line-number --no-ignore --no-heading --color=always --smart-case --hidden '

After:

let fzf_rg_args = ' --glob="!.git" --glob="!node_modules" --column --line-number --no-ignore --no-heading --color=always --smart-case --hidden '

I also checked Rg ! and it indeed searched for the exclamation mark symbol !.

Let me know if you'd like me to review something else. If not, feel free to close the issue.

junegunn commented 2 months ago

Thank you. I don't know if you have already done it, but can you also update the binary and see how it works with the latest Vim plugin? I'm attaching the one I just built on my machine from the latest source.

fzf.exe.zip

DanSM-5 commented 2 months ago

I did another round of test with the binary you attached

❯ fzf --version
0.51.0-devel (c5fb0c4)

Looks good. I didn't have any escape issue.

junegunn commented 2 months ago

Thanks a lot! Will release a new version with the fix soon.