[Bug] Preview and stage doesn't work in vim on windows

[DanSM-5]

What is the latest commit SHA in your installed vim-gitgutter? 7b0b5098e3e57be86bb96cfbf2b8902381eef57c

What vim/nvim version are you on? vim 9.1

Description

Using hunk preview <leader>hp or hunk stash <leader>hs result in an error.

Error detected while processing function gitgutter#hunk#preview[3]..<SNR>95_hunk_op[38]..gitgutter#diff#run_diff[92]..gitgutter#utility#system:
line    5:
E282: Cannot read from "C:\Users\daniel\AppData\Local\Temp\V3FE0C7.tmp"
Error detected while processing function gitgutter#hunk#preview[3]..<SNR>95_hunk_op[38]..gitgutter#diff#run_diff:
line   92:
E714: List required
Cursor is not in a hunk

The annoying part is that after the error happens other commands like GitGutterNext/PrevHunk will start failing mentioning that there are no hunks in the file. Fortunately disabling and enabling GitGutter fixes that.

Log:

  0.185444 function <SNR>50_on_exit_vim[13]..8[10]..gitgutter#process_buffer[23]..gitgutter#diff#run_diff[85]..gitgutter#async#execute[1]:
  0.185444 [async] (git -C "C:\Users\daniel\vim-config" --no-pager show --textconv :vimonly.vim > C:\Users\daniel\AppData\Local\Temp\V1T3270.tmp.1.1.vim || exit 0) && (git -C "C:\Users\daniel\vim-config" --no-pager -c "diff.autorefreshindex=0" -c "diff.noprefix=false" -c "core.safecrlf=false" diff --no-ext-diff --no-color -U0  -- C:\Users\daniel\AppData\Local\Temp\V1T3270.tmp.1.1.vim C:\Users\daniel\AppData\Local\Temp\V2M3271.tmp.1.1.vim | grep "^@@ " || exit 0)

I found that the error occurs in "autoload/gitgutter/utility.vim" in the gitgutter#utility#cmd function when calling system().

I copied a command and ran it manually

:echo system('(git -C "C:\Users\daniel\vim-config" --no-pager show --textconv :vimonly.vim > C:\Users\daniel\AppData\Local\Temp\VHPDA96.tmp.1.3.vim || exit 0) && (git -C "C:\Users\daniel\vim-config" --no-pager -c "diff.autorefreshindex=0" -c "diff.noprefix=false" -c "core.safecrlf=false" diff --no-ext-diff --no-color -U0  -- C:\Users\daniel\AppData\Local\Temp\VHPDA96.tmp.1.3.vim C:\Users\daniel\AppData\Local\Temp\VIIDA97.tmp.1.3.vim || exit 0)')

and I get this output (temporary file name changes on each run):

E282: Cannot read from "C:\Users\daniel\AppData\Local\Temp\VRRAD8F.tmp"

I found that adding a shellescape before the command is built fix the issue with the preview

diff --git a/autoload/gitgutter/diff.vim b/autoload/gitgutter/diff.vim
index 484b89d..342ea6c 100644
--- a/autoload/gitgutter/diff.vim
+++ b/autoload/gitgutter/diff.vim
@@ -143,6 +143,7 @@ function! gitgutter#diff#run_diff(bufnr, from, preserve_full_diff) abort
   let cmd .= ' || exit 0'

   let cmd .= ')'
+  let cmd = shellescape(cmd)

However it looks like that doesn't play nice with other commands as the symbols on the left does not show up with that change.

The log with the escaped command:

2228.508829 FocusGained Autocommands for "*"..function gitgutter#all[8]..gitgutter#process_buffer[23]..gitgutter#diff#run_diff[86]..gitgutter#async#execute[1]:
2228.508829 [async] "(git -C ""C:\Users\daniel\vim-config"" --no-pager show --textconv :vimonly.vim > C:\Users\daniel\AppData\Local\Temp\VHP1BE3.tmp.1.15.vim || exit 0) && (git -C ""C:\Users\daniel\vim-config"" --no-pager -c ""diff.autorefreshindex=0"" -c ""diff.noprefix=false"" -c ""core.safecrlf=false"" diff --no-ext-diff --no-color -U0  -- C:\Users\daniel\AppData\Local\Temp\VHP1BE3.tmp.1.15.vim C:\Users\daniel\AppData\Local\Temp\VII1BE4.tmp.1.15.vim | grep ""^@@ "" || exit 0)"

Experimenting with the command I noticed that if the parenthesis are omitted, it doesn't need shellescape

:echo system('git -C "C:\Users\daniel\vim-config" --no-pager show --textconv :vimonly.vim > C:\Users\daniel\AppData\Local\Temp\VHPDA96.tmp.1.3.vim || exit 0')

if the same command is called with the wrapping parenthesis but without shellescape, then created temporary file is empty

:echo system('(git -C "C:\Users\daniel\vim-config" --no-pager show --textconv :vimonly.vim > C:\Users\daniel\AppData\Local\Temp\VHPDA96.tmp.1.3.vim || exit 0)')

System information

• Windows 11 Build 22631
• Vim 9.1 from scoop

[airblade]

Thanks for such a detailed report.

I'm surprised that the error code is E282, which concerns vim reading its config files, rather than E484. I can't explain that.

Running system() to see what happens is a good idea but it doesn't necessarily match the exact way gitgutter runs commands. Gitgutter sets various shell-related options first. So try using :call gitgutter#utility#system() instead.

When the commands run asynchronously, they use cmd.exe /c, assuming has('unix') returns false.

It sounds like escaping is the problem, but who knows, maybe it would help to use a known directory for the tempfiles. You could try this (you may well have to change the single quotes to double quotes or double up the backslashes):

diff --git i/autoload/gitgutter/diff.vim w/autoload/gitgutter/diff.vim
index 484b89d..a2e70fb 100644
--- i/autoload/gitgutter/diff.vim
+++ w/autoload/gitgutter/diff.vim
@@ -4,8 +4,8 @@ let s:nomodeline = (v:version > 703 || (v:version == 703 && has('patch442'))) ?

 let s:hunk_re = '^@@ -\(\d\+\),\?\(\d*\) +\(\d\+\),\?\(\d*\) @@'

-let s:temp_from = tempname()
-let s:temp_buffer = tempname()
+let s:temp_from = 'C:\Users\daniel\gitgutter1'
+let s:temp_buffer = 'C:\Users\daniel\gitgutter2'
 let s:counter = 0

 " Returns a diff of the buffer against the index or the working tree.

If that doesn't work, then it must be an escaping problem and unfortunately I don't have an intuition for what the solution might be (I haven't used Windows for over 20 years); and even if I did I have no way to test it.

[DanSM-5]

Running system() to see what happens is a good idea but it doesn't necessarily match the exact way gitgutter runs commands. Gitgutter sets various shell-related options first. So try using :call gitgutter#utility#system() instead.

Got it. From what I saw, it sets the shell to be cmd with the right shellcmdflag /c. I have vim using the defaults that are those because most plugins expect the same.

It results in the same error though.

Error detected while processing function gitgutter#utility#system:
line    5:
E282: Cannot read from "C:\Users\daniel\AppData\Local\Temp\V3FA875.tmp"
-1

maybe it would help to use a known directory for the tempfiles. You could try this

I don't think that affects because I can guarantee "C:\Users\daniel\AppData\Local\Temp" does exists. I believe that vim always refers correctly to the Temp directory when using tempname() (as far as I've used it) but I am also careful to use a username without spaces, special characters and that is short to avoid long path names.

Anyway, I gave it a try but same result. Commands stage, undo and preview fail.

I think it is really a escaping issue in windows. I experimenting a bit trying to understand why adding a shell escape fixed things like the preview but broke the signs. The reason is this part '^@@' that is added for some commands.

The following diff seems to solve all the issues though this works only for windows, so it is not a final solution.

diff --git a/autoload/gitgutter/diff.vim b/autoload/gitgutter/diff.vim
index 484b89d..d63961b 100644
--- a/autoload/gitgutter/diff.vim
+++ b/autoload/gitgutter/diff.vim
@@ -133,7 +133,13 @@ function! gitgutter#diff#run_diff(bufnr, from, preserve_full_diff) abort

   " Pipe git-diff output into grep.
   if !a:preserve_full_diff && !empty(g:gitgutter_grep)
-    let cmd .= ' | '.g:gitgutter_grep.' '.gitgutter#utility#shellescape('^@@ ')
+    let cmd .= ' | '.g:gitgutter_grep.' '
+    let cmd = shellescape(cmd)
+    let cmd = cmd[0:-2]
+    let cmd .= gitgutter#utility#shellescape('^@@ ')
+  else
+    let cmd = shellescape(cmd)
+    let cmd = cmd[0:-2]
   endif

   " grep exits with 1 when no matches are found; git-diff exits with 1 when
@@ -142,7 +148,7 @@ function! gitgutter#diff#run_diff(bufnr, from, preserve_full_diff) abort
   " which always exits with success (0).
   let cmd .= ' || exit 0'

-  let cmd .= ')'
+  let cmd .= ')"'

   if g:gitgutter_async && gitgutter#async#available()
     call gitgutter#async#execute(cmd, a:bufnr, {

These are my observations for ^@@:

echo gitgutter#utility#shellescape('^@@')
" returns "^@@"

echo shellescape(gitgutter#utility#shellescape('^@@'))
" returns ""^@@""

The extra double quotes on that string broke the signs command. Not sure if anything else use it but I think it is safe to assume that everything that goes into the if if !a:preserve_full_diff && !empty(g:gitgutter_grep) is affected.

Breaking down the patch above, I'm creating the string

"(git -C ""C:\Users\daniel\vim-config"" --no-pager show --textconv :vimonly.vim > C:\Users\daniel\AppData\Local\Temp\VTD5A1E.tmp.1.11.vim || exit 0) && (git -C ""C:\Users\daniel\vim-config"" --no-pager -c ""diff.autorefreshindex=0"" -c ""diff.noprefix=false"" -c ""core.safecrlf=false"" diff --no-ext-diff --no-color -U0  -- C:\Users\daniel\AppData\Local\Temp\VTD5A1E.tmp.1.11.vim C:\Users\daniel\AppData\Local\Temp\VU65A1F.tmp.1.11.vim | grep"

Then removing the last " (after grep). Then adding the rest "^@@ " || exit 0 And manually closing the parenthesis and the string )"

The full command looks as follows:

"(git -C ""C:\Users\daniel\vim-config"" --no-pager show --textconv :vimonly.vim > C:\Users\daniel\AppData\Local\Temp\VTD5A1E.tmp.1.11.vim || exit 0) && (git -C ""C:\Users\daniel\vim-config"" --no-pager -c ""diff.autorefreshindex=0"" -c ""diff.noprefix=false"" -c ""core.safecrlf=false"" diff --no-ext-diff --no-color -U0  -- C:\Users\daniel\AppData\Local\Temp\VTD5A1E.tmp.1.11.vim C:\Users\daniel\AppData\Local\Temp\VU65A1F.tmp.1.11.vim | grep "^@@ " || exit 0)"

If that doesn't work, then it must be an escaping problem and unfortunately I don't have an intuition for what the solution might be (I haven't used Windows for over 20 years); and even if I did I have no way to test it.

I understand. If you need help, I can test and try to provide as much information as possible. I'm not that confortable with vimscript to be able to create a fix myself as I originally attempted but I hope this information can help you.

[DanSM-5]

As a note, the ^ is a special character for cmd as it is the escape character (like bashslash in bash). That may be the cause for the "^@@" string to break.

[airblade]

What happens if you do let g:gitgutter_grep = ""?

Gitgutter run the diff output through grep to extract just the hunk headers. But if you tell it not to bother with grep, it just filters the diff output itself, which is completely fine.

Turning off grep would mean the commands it runs won't have ^@@ in them.

[DanSM-5]

In parts

Simply setting let g:gitgutter_grep = "" does nothing. Same issue.

Setting let g:gitgutter_grep = "" and adding a shellescape let cmd = shellescape(cmd) before gitgutter#utility#shellescape fixes the issue. It is like you said, it avoids ^@@, thus it is not an issue anymore.

is performance affected if there is no grep involved?

I ran a couple of additional tests to understand a bit more why it fails. Turns out that the ^ is not the main reason, neither the @@ but the space after them.

This works

   let cmd .= ' | '.g:gitgutter_grep.' '.gitgutter#utility#shellescape('^@@')
   ...
   let cmd = shellescape(cmd)

but this doesn't

   let cmd .= ' | '.g:gitgutter_grep.' '.gitgutter#utility#shellescape('^@@ ')
   ...
   let cmd = shellescape(cmd)

All that said, how important is to match the space? It doesn't look like it can wrongly match other lines but I haven't tried in big diffs that include stuff like @ symbols.

I'm fine to leave it without grep in windows but testing a couple of things I managed to have things working with few changes.

diff --git a/autoload/gitgutter/diff.vim b/autoload/gitgutter/diff.vim
index 484b89d..7b9a139 100644
--- a/autoload/gitgutter/diff.vim
+++ b/autoload/gitgutter/diff.vim
@@ -133,7 +133,7 @@ function! gitgutter#diff#run_diff(bufnr, from, preserve_full_diff) abort

   " Pipe git-diff output into grep.
   if !a:preserve_full_diff && !empty(g:gitgutter_grep)
-    let cmd .= ' | '.g:gitgutter_grep.' '.gitgutter#utility#shellescape('^@@ ')
+    let cmd .= ' | '.g:gitgutter_grep.' :::'
   endif

   " grep exits with 1 when no matches are found; git-diff exits with 1 when
@@ -144,6 +144,12 @@ function! gitgutter#diff#run_diff(bufnr, from, preserve_full_diff) abort

   let cmd .= ')'

+  if gitgutter#utility#windows()
+    let cmd = shellescape(cmd)
+  endif
+
+  let cmd = substitute(cmd, ':::', gitgutter#utility#shellescape('^@@ '), '')
+
   if g:gitgutter_async && gitgutter#async#available()
     call gitgutter#async#execute(cmd, a:bufnr, {
           \   'out': function('gitgutter#diff#handler'),

I did a quick test under wsl and it works fine there too though not sure how bad could it be to add the unconditional call to substitute or if it is better to check again !a:preserve_full_diff && !empty(g:gitgutter_grep) to put it under an if.

[airblade]

is performance affected if there is no grep involved?

I doubt it. It made a difference 10 or more years ago but now Vim and computers are much faster so it's probably not worth the increase in complexity to pipe through grep. If/when I get around to modernising vim-gitgutter, I'll probably remove the grep stuff.

Turns out that the ^ is not the main reason, neither the @@ but the space after them.

That is weird! I don't understand it at all.

The space after @@ isn't necessary; gitgutter will do its own internal grep anyway.

If you take out the space after @@, and turn off grep, does everything start to work?

[DanSM-5]

I doubt it. It made a difference 10 or more years ago but now Vim and computers are much faster so it's probably not worth the increase in complexity to pipe through grep. If/when I get around to modernising vim-gitgutter, I'll probably remove the grep stuff.

Got it, I also didn't saw any meaningful difference.

Turns out that the ^ is not the main reason, neither the @@ but the space after them.

That is weird! I don't understand it at all.

Me neither. I couldn't get why the space there is significant but it happens directly in cmd.exe as well...

REM The command ""^@@"" without space works
cmd /c "(git -C ""C:\Users\daniel\vim-config"" --no-pager show --textconv :vimonly.vim > C:\Users\daniel\AppData\Local\Temp\VLX4C02.tmp.1.3.vim || exit 0) && (git -C ""C:\Users\daniel\vim-config"" --no-pager -c ""diff.autorefreshindex=0"" -c ""diff.noprefix=false"" -c ""core.safecrlf=false"" diff --no-ext-diff --no-color -U0  -- C:\Users\daniel\AppData\Local\Temp\VLX4C02.tmp.1.3.vim C:\Users\daniel\AppData\Local\Temp\VMQ4C03.tmp.1.3.vim | rg ""^@@"" -- || exit 0)"
@@ -25 +25 @@ something

REM The same command with ""^@@ "" doesn't
cmd /c "(git -C ""C:\Users\daniel\vim-config"" --no-pager show --textconv :vimonly.vim > C:\Users\daniel\AppData\Local\Temp\VLX4C02.tmp.1.3.vim || exit 0) && (git -C ""C:\Users\daniel\vim-config"" --no-pager -c ""diff.autorefreshindex=0"" -c ""diff.noprefix=false"" -c ""core.safecrlf=false"" diff --no-ext-diff --no-color -U0  -- C:\Users\daniel\AppData\Local\Temp\VLX4C02.tmp.1.3.vim C:\Users\daniel\AppData\Local\Temp\VMQ4C03.tmp.1.3.vim | rg ""^@@ "" -- || exit 0)"
: The system cannot find the path specified. (os error 3)

I couldn't figure anything out besides avoiding the space or the doubled quotation.

If you take out the space after @@, and turn off grep, does everything start to work?

Either is fine as long as you add an extra shellescape (at least for windows).

diff --git a/autoload/gitgutter/diff.vim b/autoload/gitgutter/diff.vim
index 484b89d..c65b45f 100644
--- a/autoload/gitgutter/diff.vim
+++ b/autoload/gitgutter/diff.vim
@@ -133,7 +133,7 @@ function! gitgutter#diff#run_diff(bufnr, from, preserve_full_diff) abort

   " Pipe git-diff output into grep.
   if !a:preserve_full_diff && !empty(g:gitgutter_grep)
-    let cmd .= ' | '.g:gitgutter_grep.' '.gitgutter#utility#shellescape('^@@ ')
+    let cmd .= ' | '.g:gitgutter_grep.' '.gitgutter#utility#shellescape('^@@')
   endif

   " grep exits with 1 when no matches are found; git-diff exits with 1 when
@@ -144,6 +144,8 @@ function! gitgutter#diff#run_diff(bufnr, from, preserve_full_diff) abort

   let cmd .= ')'

+  let cmd = shellescape(cmd)
+
   if g:gitgutter_async && gitgutter#async#available()
     call gitgutter#async#execute(cmd, a:bufnr, {
           \   'out': function('gitgutter#diff#handler'),

or

diff --git a/autoload/gitgutter/diff.vim b/autoload/gitgutter/diff.vim
index 484b89d..35a22df 100644
--- a/autoload/gitgutter/diff.vim
+++ b/autoload/gitgutter/diff.vim
@@ -144,6 +144,8 @@ function! gitgutter#diff#run_diff(bufnr, from, preserve_full_diff) abort

   let cmd .= ')'

+  let cmd = shellescape(cmd)
+
   if g:gitgutter_async && gitgutter#async#available()
     call gitgutter#async#execute(cmd, a:bufnr, {
           \   'out': function('gitgutter#diff#handler'),

and setting let g:gitgutter_grep = "".

[DanSM-5]

One more finding. I decided to try neovim (NVIM v0.10.1) instead of vim with no changes (not even let g:gitgutter_grep = "") and it works just fine. No changes needed. So maybe this is a vim only issue in windows. I don't know if neovim handles better the calls to system() or if vim has a bug in its own.

That being said, I'll probably just keep using neovim for the time being.

For the issue, not sure if you would prefer to keep it open for visibility or just close it if nothing else will be done. I'll let that up to you.

[airblade]

Thanks for doing all that research!

It's interesting that it works as-is on Neovim. Although gitgutter runs the same commands on Vim and Neovim, it runs them slightly differently due to the slightly different APIs for jobs. Either there's a bug in Vim on Windows or gitgutter is executing commands slightly wrongly.

Coincidentally (?) the most recent commit (7b0b509) changed the way Neovim executes async commands on Windows. I wonder if a similar change would work for vim. I don't think it will because it won't execute the command in a shell, but since I don't understand why the existing methods don't work, it might be worth a quick test – if you want to.

diff --git i/autoload/gitgutter/async.vim w/autoload/gitgutter/async.vim
index 988e4a2..1b25cc9 100644
--- i/autoload/gitgutter/async.vim
+++ w/autoload/gitgutter/async.vim
@@ -46,7 +46,7 @@ function! s:build_command(cmd)
   endif

   if has('win32')
-    return has('nvim') ? a:cmd : 'cmd.exe /c '.a:cmd
+    return a:cmd
   endif

   throw 'unknown os'

I'm happy you found a fix for vim – the extra `shellescape() – but I'm reluctant to apply it because nobody else has mentioned this problem. I assume the current code must be working for other people using gitgutter on Windows (with the underlying assumption that there are other people using gitgutter on Windows, though I have no idea how many).

Thanks for your patience and detailed comments!

I'll leave this issue open for the time being to help other people find it.

[DanSM-5]

I tried that last change but it didn't work. Worse it breaks the signs showing up as well.

I'm fine with keeping it as it is if no one else is having issues.