Grepper isn't properly quoting search queries on Windows
In my .vimrc, the only configuration related to Grepper is the following two mappings:
nmap gs <plug>(GrepperOperator)
xmap gs <plug>(GrepperOperator)
Grepper chooses ag by default, and I'll use ag in my examples below, though I've also tried swapping ag for rg with the same results.
I created a very simple test case of a single folder named "test" whose contents are the single file named "test.txt" whose contents are the single line "test1 test2". In all further discussion, this is my present working directory and only buffer.
In Windows, if I position the cursor on column 1 and type "gs2w" (grep search two words), or if I visually select "test1 test2" and then type "gs" (grep search visual selection), then the following error is generated:
|| ERR: Error opening directory test2: No such file or directory
Strangely, the command displayed in the status line does appear to be properly (double) quoted:
[Quickfix List] ag --vimgrep -- "test1 test2"
That same command returns the expected result when typed directly at the Windows shell.
However, the error in Vim certainly indicates that ag as called by Grepper is treating the second word after the space as a path argument rather than part of the search pattern.
When I do the exact same experiments in macOS, the Grepper search is successful and no error is generated:
test.txt|1 col 1| test1 test2
Here the command displayed in the status bar is similar to Windows, though the quotes are single rather than double, which makes sense since I'm using Bash on macOS:
[Quickfix List] ag --vimgrep -- 'test1 test2'
I also did some experimentation using the Grepper prompt instead of the operator.
In macOS, I can search at the Grepper prompt for 'test1 test2' or "test1 test2" (single or double quotes) and I'll get the same successful result. This makes sense because there aren't any special characters which need to be escaped and Bash can handle both types of quotes.
However, in Windows, only the Grepper prompt search for 'test1 test2' returns a successful result, whereas "test1 test2" returns the same error as when trying to use the operator.
To me, this is particularly odd behavior since the Windows shell is only supposed to work with the double quotes rather than the single. Using single quotes directly at the Windows shell generates the same error as using double quotes at the Grepper prompt.
Hence, Grepper appears to be sending to the Windows shell the opposite quotes of what is being displayed on the Vim status line.
Here's my environment:
Windows 10 Enterprise 16299.192 vim 8.0.1473 (via Chocolatey) ag 2.1.0 (via Chocolatey) rg 0.7.1 (via Chocolatey) vim-grepper 1.4 (via vim-plug)
macoS 10.13.2 vim 8.0.1450 (via Homebrew) ag 2.1.0 (via Homebrew) rg 0.7.1 (via Homebrew) vim-grepper 1.4 (via vim-plug)
Let me know if there is anything else I can provide that would help debug. Cheers.
Hey,
I'm sorry for all these Windows bugs. Since I left uni I don't even have access to Windows anymore and I'm pondering to declare this plugin to only have experimental Windows support.
I simply don't have the capacities to do it myself right now. If any Windows person was willed to dive into the code, I'd grant them commit rights immediately.
But until then it will probably take a while to get this fixed. Sorry for the inconveniences. 🙏
I'm stuck on Windows for work, and I want to fix this plugin, so I'm diving into the code now. However, I don't have prior experience with Vimscript or the Vim debugger, so it will take me a while to grok. Also, paths, quoting, and escaping on Windows shell is just ridiculous compared to any other *nix shell, so there is also a possibility of my head exploding before a pull request.
Anyhow, as the plugin is currently written for Windows, the command is actually being passed along to powershell.exe through cmd.exe, and Powershell is stripping out the double quotes.
Thus, I tried forcing the use of cmd.exe, and this works for my example above, but it breaks when used with the -buffer and -buffers flags because single quotes are used for escaping those.
Digging in further, it appears shellslash is being set to true by the plugin, which causes the shellescape function to use single quotes rather than double quotes, which is inappropriate for Windows.
I tried commenting out all the places where shellslash was being set to true along with forcing the use of cmd.exe, and so far, this is working as expected with some basic examples.
I will continue to test this approach and report my results here.
In case anyone else is interested, this document seems to be the best guide out there regarding Vim shell escaping:
Thanks for all your input so far. I finally dusted off an old Win 10 VM.
Let's start with a fundamental question.. is there any need to ever use Powershell? Is there any downside to using cmd.exe directly even when there's Powershell available?
I play around with the code a bit and try to pay special attention to..
-
'shellslash' - quoting:
- paths with spaces
-
/and\as path separators are handled the same - does
-buffersstill work with multiple buffers
- does
findstr.exestill work? (It's a much simpler tool than the other supported grep tools)
@cheathcott
It would be nice if you could test the improve-win-support branch. The code is rather simple, but it seems to work for my arguably stupid test cases.
Would be good to know if it works for you as well and if you can figure out any corner cases that breaks it.
Ref: https://github.com/mhinz/vim-grepper/pull/151
Awesome! I'll thoroughly test the improve-win-support branch this weekend and report my findings.
Incidentally, with regard to the experiment I described earlier, I also ran into the problem with needing to escape backslash path separators. I tried decomposing the map which does both shellescape and fnamemodify into two steps.
In the first step I left shellslash=1 so forward slashes were used as path separators. Then I set shellslash=0 so double quotes were used for the shellescape.
Anyhow, the way you handled it by setting shellslash=0 first and then simply escaping the backslash path separators before doing the shellescape seems like the better approach.
Now that the changes are merged onto master.. any things left to fix?
I apologize for disappearing for a bit. I'm back to testing this change, and I definitely think it's a positive improvement since the file arguments passed to the grep tools are now properly quoted with double quotes, so the buffer and buffers flag work.
However, there remain some search pattern quoting issues with the operator because of cmd.exe's ridiculous quoting rules.
For example, if using the operator to search for a pattern which ends with a literal backslash, then my understanding is that it has to be escaped with four backslashes rather than only two backslashes like when it appears in the middle of the pattern.
Assume a file contains three lines, "a", "a\b", and "*?ab".
If you visually select "a" and use the operator, the escaped search pattern is "a\", which doesn't work.
If you visually select "a\b" and use the operator, the escaped search pattern is "a\b", which does work.
If you use the prompt to search for "a\\", then both "a" and "a\b" are found.
Also, there are other special characters, like star or question mark, for which single backslash escape does not seem to work.
If you visually select "*" and use the operator, the escaped search pattern is "\*", which does not match.
Unfortunately, I'm not even sure how to properly escape star and question mark when calling ag or rg directly from cmd.exe.
Perhaps the better approach to take with the operator is to do minimal escaping and to use the literal string flags of ag or rg instead rather than these tools assuming the pattern is a regex - which is something I think I should be doable via my grepper config.
Hmmmm. I can reproduce everything you said.
For the prompt the manual escaping is fine, since you would have to do that in the shell as well. The prompt is supposed to feel like a pre-populated shell.
But the operator should always find the selected text, of course.
I found that "\*\?" matches the *? in your example. Only "\*" alone won't match *. ~~We need to do additional escaping with ^ in that case: "^\*".~~
So, that's something that could be implemented for the operator, but this SO post makes me think that using cmd.exe in favour of powershell is the wrong way. Apparently we will never be able to escape " within our implicit double quotes. That's kind of a no-go.
EDIT:
"\"test\"" (or """test""") matches "test" just fine. Let me add the "*" and "foo\" corner cases and see where it heads.
The foo\ case can be worked around by escaping the \ twice. So it will be found by foo\\\\. foo\\ will be found by foo\\\\\\\\\ and so on.
But I haven't found any way to escape * or ? without using ag -Q. Neither with cmd.exe nor Powershell.
Just for reference: quoting could be avoided / forwarded to Neovim/Vim by not wrapping it manually in a shell (cmd.exe) probably. See https://github.com/mhinz/vim-grepper/issues/166 about using Python's shlex.split to split a string into argv.
I suggest writing all commands to a temporary batchfile like what I did https://github.com/junegunn/fzf/blob/master/plugin/fzf.vim. It works for the grep and ag commands in https://github.com/junegunn/fzf.vim.
If not, use /s and wrap the entire command with double quotes so that cmd.exe dequotes " and runs the dequoted command as is. Check cmd.exe defaults for Neovim and the functional tests for system and jobs.
I don't work at Windows-related issues, sorry. I'll review small PRs that are VimL-only, though.