Closed fvictorio closed 1 year ago
I agree with everything you wrote. I'm trying to think of a way around this given that the git show main:some-new-file
is part of a compound command.
The command that's executed is something like:
cd /path/to/repo &&
(git show main:some/file > a && git diff -- a b | grep '^@@ ' || exit 0)
– where b
is a temporary file holding the buffer contents.
I don't want to execute a separate call to determine whether some/file
exists on the main
branch, because each external call adds overhead and slows things down.
I can't get too clever with the shell command because it has to work on Windows too. So I doubt whether we could do something like ... (git show main:some/file || echo '') > a ...
.
The only thing I can think of is to update the async job's stderr handlers to look for git's error message, and then either return a fake hunk header or call a new function to mark every line as added. And similarly for the synchronous diff's output.
So I doubt whether we could do something like
... (git show main:some/file || echo '') > a ...
.
That seems like the most straightforward solution though, and the command already has parentheses and ||
, so maybe it's worth trying? Happy to help anyway I can. I don't use Windows but I can try it in some VM or something.
FWIW, this seems to work:
diff --git a/autoload/gitgutter/diff.vim b/autoload/gitgutter/diff.vim
index 794cd14..5c93fa6 100644
--- a/autoload/gitgutter/diff.vim
+++ b/autoload/gitgutter/diff.vim
@@ -124,7 +124,7 @@ function! gitgutter#diff#run_diff(bufnr, from, preserve_full_diff) abort
" Write file from index to temporary file.
let index_name = gitgutter#utility#get_diff_base(a:bufnr).':'.gitgutter#utility#repo_path(a:bufnr, 1)
- let cmd .= g:gitgutter_git_executable.' '.g:gitgutter_git_args.' --no-pager show '.index_name.' > '.from_file.' && '
+ let cmd .= '('.g:gitgutter_git_executable.' '.g:gitgutter_git_args.' --no-pager show '.index_name.' || echo -n "")'.' > '.from_file.' && '
elseif a:from ==# 'working_tree'
let from_file = gitgutter#utility#repo_path(a:bufnr, 1)
I had to use echo -n
because otherwise the temp file has a newline, which causes the first sign in the buffer to be a ~
instead of a +
Thanks for trying it out. And good to hear that it seems to work.
I'm wary about tinkering with the overall command because it took so long to find a form which works everywhere. However, as you say, it already has parentheses and ||
.
I had to use echo -n because otherwise the temp file has a newline, which causes the first sign in the buffer to be a ~ instead of a +
Good point. In fact I think printf
is much more consistent than echo
. Would you mind trying that in a Windows VM?
diff --git i/autoload/gitgutter/diff.vim w/autoload/gitgutter/diff.vim
index 794cd14..faf0098 100644
--- i/autoload/gitgutter/diff.vim
+++ w/autoload/gitgutter/diff.vim
@@ -124,7 +124,7 @@ function! gitgutter#diff#run_diff(bufnr, from, preserve_full_diff) abort
" Write file from index to temporary file.
let index_name = gitgutter#utility#get_diff_base(a:bufnr).':'.gitgutter#utility#repo_path(a:bufnr, 1)
- let cmd .= g:gitgutter_git_executable.' '.g:gitgutter_git_args.' --no-pager show '.index_name.' > '.from_file.' && '
+ let cmd .= '('.g:gitgutter_git_executable.' '.g:gitgutter_git_args.' --no-pager show '.index_name.' || printf "") > '.from_file.' && '
elseif a:from ==# 'working_tree'
let from_file = gitgutter#utility#repo_path(a:bufnr, 1)
Ok I was finally able to try this after installing vim and git in my wife's laptop and dealing with the fact that not all computers have caps lock mapped to ctrl, ANYWAY:
First thing, printf
doesn't seem to exist in cmd or powershell, and I confirmed that using it in the command makes the plugin stop working on Windows.
echo
does work, but -n
is not a valid flag in windows (and ""
is printed verbatim). This means that when you go to a new file and preview the hunk, it will tell you that the previous version had -n ""
as its content. Not great.
So I tried using just echo
on its own, but that prints ECHO is on.
, which is even worse. Apparently you can use echo:
to print an empty line, but that will obviously not work on linux.
So I'm not sure what's the best course of action here. I guess we could use gitgutter#utility#windows
and change the command based on that, but that function is barely used in the codebase, so I guess you try to avoid those kinds of fixes.
Windows is a pain! And there are several shell variations: cmd, powershell, cygwin, bash installed by the git installer, etc.
We could continue with this approach and change the command to echo:
, perhaps using gitgutter#utility#windows() && s:dos_shell()
(or simply make dos_shell()
into gitgutter#utility#dos_shell()
and use that alone).
I'd almost prefer to say that the first time a buffer is diffed, if g:gitgutter_diff_base
is not empty we make an extra system call to see if the file exists in the diff base, and cache the result in a buffer variable. If the file doesn't exist in the diff base, we change the overall command to git diff <g:gitgutter_diff_base> -- <file>
. But that isn't quite what we want because gitgutter shows the diff between the buffer contents and the diff base, not the working tree and the diff base. But the only way cross-platform way to diff the buffer contents is to write it to a temp file, and then we have to do git diff -- a b
and we're back where we started.
We could continue with this approach and change the command to
echo:
, perhaps usinggitgutter#utility#windows() && s:dos_shell()
(or simply makedos_shell()
intogitgutter#utility#dos_shell()
and use that alone).
Makes sense, I'll give it a try and then test it in as many windows shells as I can (if you have a full list somewhere, let me know).
Actually I think I have a way to avoid all that.
The first time a buffer is diffed, if g:gitgutter_diff_base
is not empty we make an extra system call to see if the file exists in the diff base, and cache the result in a buffer variable. If the file doesn't exist in the diff base, we skip running the diff commands and instead return a fake hunk header showing that the whole file is new.
Ah, that makes sense. It seems a bit out of my depth, but I can give it a try if you don't have time to do it yourself. And if you do it, I'm happy to help with the manual testing on windows (well, not happy, but, you know).
I'll try to get to it over the next day or two.
44dbd57dd19e8ec58b2a50c787c8ae7bb231c145
Vim 8.2
Problem
I normally do something like
let g:gitgutter_diff_base="main"
when reviewing a pull request. This works great except for added files. I would expect them to show the+
sign in each line, but nothing is shown.This is different from https://github.com/airblade/vim-gitgutter/issues/461, which is about new files. The answer there is that git doesn't know about those files, so nothing is shown. I agree with that. But in this scenario, if you do
git diff main -- some-new-file
, you do get a diff with all the lines added.Possible cause
I tried to investigate this a bit, and I found that one of the commands that end up being executed is something like:
which produces a
fatal: path 'some-new-file' exists on disk, but not in 'main'
error.