Shell parsing for preview command in Windows is incompatible with fzf#shellescape and cmd.exe

[janlazo]

• Category
  • fzf binary
  • fzf-tmux script
  • Key bindings
  • Completion
  • Vim
  • Neovim
  • Etc.
• OS
  • Linux
  • Mac OS X
  • Windows
  • Windows Subsystem for Linux
  • Etc.
• Shell
  • bash
  • zsh
  • fish

Backslashes in the filepath of the preview command are escaped so they don't run in Windows.
The preview script itself (ruby or bash) has no issues with filepaths with backslashes.

junegunn/fzf.vim#418 (comment) is caused by the network drive conflicting with the colon delimiter. I'll open a separate issue/PR for that.

[janlazo]

Escaping the preview command with fzf#shellescape doesn't work 😕

ie. if shellquoting is disabled in Vim

execute ':!fzf --preview' s:escape(fzf#shellescape(fzf#shellescape('C:\path\to\preview.rb {}')))

[janlazo]

Workaround from fzf.vim for escaping filenames correctly if not using a batchfile for the preview command
It shortens the filepath to the legacy 8.3 filename.
junegunn/fzf.vim@a4d4986#diff-7274b7c6c87526542ef9087f2e997dfaR42

let path = 'C:\path\to\preview_script.sh'
if has('win32')
  if has('nvim')
    let path = split(system('for %A in ("'.path.'") do echo %~sA'), "\n")[1]
  else
    let path = fnamemodify(path, ':8')
  endif
  " Required only for shell parser in fzf
  let path = escape(path, '\')
endif

Leaving this ticket open in case comes up with a better idea or makes the fzf's command parser more convenient for cmd.exe

[janlazo]

Neovim support for DOS shortname is removed in neovim/neovim#635. This hack is here to stay in fzf.vim 😢 and fnamemodify(path, ':8') and expand('%:p:8') are broken.

[janlazo]

@junegunn Do you know why a single double-quote is converted to a double backslash?
I'm testing the bash preview script in fzf.vim on a line with a single quote, https://github.com/junegunn/fzf/blob/master/plugin/fzf.vim#L2, and I get File not found error with the backslashes escaped. The preview script runs as expected because the escaped backslashes are from fzf.vim, not the fzf binary.
This happens in Vim and Neovim.

[junegunn]

Not quite sure if I follow you. Can you provide the exact steps to reproduce the problem you described?

[janlazo]

In Vim/Neovim, run Ag with preview

call fzf#vim#ag('', fzf#vim#with_preview())

Query for a double quote

plugin\fzf.vim:2:1:"
Ag> "

In the preview window, the preview script outputs File not found pluginfzf.vim because the preview script got pluginfzf.vim:2:1:\\

[junegunn]

I haven't tried it on Windows yet but it works on *nix.

On *nix, starting a command with sh is simple, we can just use ['sh', '-c', command] as the arguments for exec.Command without any preprocessing.

fzf/src/util/util_unix.go

Lines 20 to 23 in 077ae51

	// ExecCommandWith executes the given command with the specified shell
	func ExecCommandWith(shell string, command string) *exec.Cmd {
	return exec.Command(shell, "-c", command)
	}

On Windows, it's a little more complicated. I don't fully remember, but I noticed the same approach didn't work with cmd /c and I had to "shellescape" each token in the command using a third-party library.

fzf/src/util/util_windows.go

Lines 18 to 26 in 077ae51

	// ExecCommandWith executes the given command with cmd. _shell parameter is
	// ignored on Windows.
	func ExecCommandWith(_shell string, command string) *exec.Cmd {
	args, _ := shellwords.Parse(command)
	allArgs := make([]string, len(args)+1)
	allArgs[0] = "/c"
	copy(allArgs[1:], args)
	return exec.Command("cmd", allArgs...)
	}

Do you have experience with Go? You can test the code like follows:

package main

import (
	"fmt"
	"os/exec"

	"github.com/mattn/go-shellwords"
)

func main() {
	command := "echo 'foo\\bar:1:2:\"'"
	args, _ := shellwords.Parse(command)
	allArgs := make([]string, len(args)+1)
	allArgs[0] = "/c"
	copy(allArgs[1:], args)
	fmt.Printf("%#v\n", allArgs)
	out, err := exec.Command("cmd", allArgs...).Output()
	fmt.Println(out, err)
}

go get github.com/mattn/go-shellwords
go run shellesacpe.go

and it gives:

[]string{"/c", "echo", "foo\\bar:1:2:\""}
[] exec: "cmd": executable file not found in $PATH

Do you see anything strange about this?

[janlazo]

Which echo are we testing on? It's fine on the internal echo command in cmd.exe but it fails on echo.exe for cygwin. The bash preview script recevies exactly the same argument as echo.exe because it fails to handle the double quote.

cmd.exe echo

> echo foo\bar:1:2:"
foo\bar:1:2:"
> echo 'foo\bar:1:2:"'
'foo\bar:1:2:"'

cygwin echo.exe

> "echo" foo\bar:1:2:
foo\bar:1:2:
> "echo" foo\bar:1:2:"
foobar:1:2:"
> "echo" 'foo\bar:1:2:"'
foo\bar:1:2:"

[janlazo]

Windows ruby passes it as is so it handles the single double-quote case. For the bash script, I think cygwin bash itself is the issue because it requires double escaping, one for cmd.exe and another for bash.exe.

> ruby .\preview.rb foo\bar:1:2:"
File not found: foo\bar

[junegunn]

Which echo are we testing on?

I tested it on macOS where cmd doesn't exist, and I knew it was bound to fail. Sorry for the confusion, I just wanted to show how arguments are passed.

I think cygwin bash itself is the issue

Yeah, agreed.

[janlazo]

If we port fzf#shellescape in go such that we can shellescape the preview command into a single token, is tokenization still required?

Pointless because of cmd.exe parsing and the incorrect escaping in exec_windows.go which does not consider the command name if using cmd /c.
There is additional escaping required for commands wrapped in () and escaping the command name (ie. cmd.exe, git, fzf) determines whether cmd will use an internal command or not.

I prefer to run the command as is in a batchfile (no escaping) so cmd can determine whether the internal echo command or the cygwin echo is used but it's up to you which direction to take.

[janlazo]

If the double quotes are not escaped somehow such that it is executed as is but previewCmd is wrapped in double quotes when it is run, then

cmd := exec.Command("cmd", "/s", "/c", previewCmd).Output()

should work.

:: internal echo
>cmd /s /c " echo foo\bar" "
foo\:bar"
:: cygwin echo.exe
> cmd /s /c " "echo" foo\bar" "
foo:bar

Imo, Vim and Neovim should use /s instead of sxe and sxq because /s guarantees that command is passed exactly as what user would type in an interactive prompt.
For basic commands, this is enough. However, it has the same shellescaping rules as in interactive prompt, not the batchfile, so knowing when to escape % and other special characters are harder.

[janlazo]

Have to explicity set syscall.SysProcAttr.CmdLine manually.

golang/go#15566 (comment)
https://golang.org/src/syscall/exec_windows.go

[janlazo]

Reference: https://play.golang.org/p/aU1PlbNTqM

@junegunn Can you set CmdLine and the run the preview command as is (no parsing and escaping)?
The following code runs cygwin's echo.exe and outputs foo:bar Nevermind, I submitted a PR and will fix any golang code necessary to make this work.

package main

import (
	"fmt"
	"os/exec"
	"syscall"
)

func main() {
	cmd := exec.Command("cmd")
	command := `"echo" foo\:bar"`
	cmd.SysProcAttr = &syscall.SysProcAttr{
	    HideWindow: false,
	    CmdLine: fmt.Sprintf(` /s /c "%s"`, command),
	    CreationFlags: 0,
	}
	fmt.Println(cmd.SysProcAttr.CmdLine)
	out, err := cmd.Output()
	fmt.Printf("%s\n", out)
	fmt.Println(out, err)
}

[hungpham3112]

I still have this issue. Have fzf preview support Window yet?