Signify Broken for Custom Perforce-Based VCS

[jgehrig]

A recent update to vim-signify breaks my configuration.

I am using vim-signify with a proprietary Perforce-Based VCS. It is mostly identical to Perforce, except with a different executable name. Very strange, I know...

Previously, I had everything working with the following values set:

let g:signify_vcs_list = [ 'perforce', 'hg', 'git' ]
let g:signify_vcs_cmds = { 'perforce': '/* Custom VCS Diff Command */' }

Now the output from :SignifyDebug shows the error Not a git repository.

Maybe this is an instance of RTFM, but nothing has jumped out at me yet. I suspect this is caused by some of the recent async/detection changes. Since my scenario/configuration is so unique, I will spend some time debugging the issue.

I read through Issue #319, and is sounds similar but unrelated. As you suggested there, I will revert to the legacy branch for now.

[mhinz]

That probably happened when we switched to saveless sign updates. Now there are two ways to get the diff:

1. :h g:signify_vcs_cmds for when the buffer is unmodified, and
2. :h g:signify_vcs_cmds_diffmode for when the buffer is modified (it's a tad slower because it uses tempfiles to avoid saving the buffer, that's why we default to 1. if possible).

So, I think it should work if you remove g:signify_vcs_list and add:

let g:signify_vcs_cmds_diffmode = { 'perforce': 'your custom VCS equivalent to "p4 print %f"' }

I'll split up :h g:signify_vcs_cmds to a new section that explains how to add custom VCS tools.

[jgehrig]

I did see the config changes for real-time diffs, but somehow the repository type detection still fails.

For example, with the following config:

" Same value as before, works for legacy
let g:signify_vcs_cmds = { 'perforce': 'sd info >NUL 2>&1 && cmd /C "set SDDIFF=C:\gnuwin32\bin\diff.exe -dU0&sd diff  %f"' }

" New config value, works in terminal
let g:signify_vcs_cmds_diffmode = { 'perforce': 'sd print %f' }

I can validate both commands are working from a terminal. For g:signify_vcs_cmds, I get a unified diff of the on-disk changes. For g:signify_vcs_cmds_diffmode, I get the unmodified repository file.

However, the detection logic does not work for this configuration. The output from :SignifyDebug still shows the error Not a git repository.

How does the latest version of vim-signify detect the VCS type? Is there a good way to debug the output of the perforce commands specifically (force VCS type)?

Thanks for the help!

[mhinz]

For general debugging you can use :verb write, which enables verbose mode just for that one command. But in your case, you want it from the very beginning, so start Vim with vim -V1. You will see messages right at start, but click any key first to make them disappear, and then check :messages again. Now you will see all messages. This is needed due to the async nature of this plugin. You should see all VCS that get tested for (when they were found to be executable beforehand).

But.. I think I see the issue. You overwrite 'perforce', but perforce itself is not installed, right? I think it skips 'perforce', because no executable p4 is found (because it's actually sd in your case).

[jgehrig]

But.. I think I see the issue. You overwrite 'perforce', but perforce itself is not installed, right? I think it skips 'perforce', because no executable p4 is found (because it's actually sd in your case).

Correct, there is no p4.exe in %PATH%. However, there is a sd.exe which is essentially equivalent.

I figured it was something like this... It sounds like there was a change to the VCS detection logic?

A quick and dirty patch confirms your hypothesis:

diff --git a/autoload/sy/repo.vim b/autoload/sy/repo.vim
index c83d0c9..7eaaa93 100644
--- a/autoload/sy/repo.vim
+++ b/autoload/sy/repo.vim
@@ -619,7 +619,7 @@ if executable(s:difftool)
         \ 'cvs':      'cvs',
         \ 'rcs':      'rcsdiff',
         \ 'accurev':  'accurev',
-        \ 'perforce': 'p4',
+        \ 'perforce': 'sd',
         \ 'tfs':      'tf'
         \ }
 else
@@ -632,7 +632,7 @@ else
         \ 'cvs':      'cvs',
         \ 'rcs':      'rcsdiff',
         \ 'accurev':  'accurev',
-        \ 'perforce': 'p4',
+        \ 'perforce': 'sd',
         \ 'tfs':      'tf'
         \ }
 endif

Would you be receptive to a Pull Request that publicly exposes s:vcs_dict and allows for user customization? Or is there a better way to handle this?

I think such a change would be easy, and resolves my issues with the new async architecture...

[mhinz]

Oh, I had already written up something.. :) Please check if the latest master is working for you.

[mhinz]

I could also add a change that allows to add new VCS. Then you could do something like this:

let g:signify_vcs_cmds          = { 'sd': ... }
let g:signify_vcs_cmds_diffmode = { 'sd': ... }

function! sy#repo#check_diff_sd(exitval, diff) abort
  return a:exitval ? [0, []] : [1, a:diff]
endfunction

The naming of the function is important here, as it will be used to check if a returned diff is valid or not.

[jgehrig]

No worries... Thanks for writing up a fix so quickly @mhinz!

I just tried out your changes to master, and everything seems to work with the following configuration:

" Necessary because the diff.exe in my `%PATH% is odd...
let g:signify_difftool = 'C:\gnuwin32\bin\diff.exe'

" Remap Perforce commands for my custom VCS
let g:signify_vcs_cmds = { 'perforce': 'sd info >NUL 2>&1 && cmd /C "set SDDIFF=C:\gnuwin32\bin\diff.exe -dU0&sd diff  %f"' }
let g:signify_vcs_cmds_diffmode = { 'perforce': 'sd print %f' }

I do like the idea of adding completely custom VCSs... That seems like a cleaner configuration. With that said, the current mechanism works fine too.

[mhinz]

Yup, adding that should be easy. The boring part is coming up with a clear documentation. But I need to add a new section for all things custom VCS anyway, so let's do all this in one go now!

[jgehrig]

Documentation, necessary but never fun! Let me know if I can help in any way.

[jgehrig]

@mhinz

I think something might be broken in master. Currently vim-signify + Perforce makes vim non-responsive. I suspect something has changed, and buffer modifications trigger diff check/update? This behavior would be bad for Perforce servers.

If you are not aware of recent changes that could cause this, I will investigate further at some point...

[aktau]

This also broke some other stuff. For example, I used to use:

'perforce': 'DIFF=%d" -U0" vcsdiff %f || [[ $? == 1 ]]',

Where vcsdiff is a shim that runs something that's faster for my specific use case.

Which meant that I could run vcsdiff from the command-line and it would print nice colorized diff (the default if $DIFF is unset is git diff --histogram --no-index. But running it from vim-signify made it output the format it expected.

I think this is now broken because of lines like https://github.com/mhinz/vim-signify/blob/56db16f8d3825c4d066c2faf05315c2b208cd5f5/autoload/sy/repo.vim#L654

Since

:echo executable('DIFF=diff vcsdiff')
0

I'm not sure if this is intentional, since there is still support for wrapping in a shell: https://github.com/mhinz/vim-signify/blob/56db16f8d3825c4d066c2faf05315c2b208cd5f5/autoload/sy/repo.vim#L479

EDIT: Confirmed. My current workaround is:

'perforce': 'true && DIFF=%d" -U0" vcsdiff %f || [[ $? == 1 ]]',

[wrengr]

I could also add a change that allows to add new VCS. Then you could do something like this:

let g:signify_vcs_cmds          = { 'sd': ... }
let g:signify_vcs_cmds_diffmode = { 'sd': ... }

I'm using a different custom Perforce-like VCS than the original poster, but I would love to have this functionality. (Based on the documentation I had assumed this sort of thing would work already; and when searching for clues why it didn't, that's how I found this ticket :)