mhinz / vim-signify

:heavy_plus_sign: Show a diff using Vim its sign column.
MIT License
2.69k stars 104 forks source link

Deleted line markers not shown when working in Fossil checkout #282

Closed otherjoel closed 5 years ago

otherjoel commented 5 years ago

When working inside a Git repo, deleted-line markers are showing up properly in the gutter as you would expect. However, when working within a Fossil checkout, deleted-line markers do not appear.

In the example screenshot below, I am editing a file inside a Fossil checkout, and have just deleted a line at the cursor position with dd and saved the buffer. Note that, although there are markers shown for there is no marker shown for the deleted line.

Below the screenshot is the output of the :SignifyDebug command.

screen shot 2019-02-03 at 2 04 00 pm
git diff --no-color --no-ext-diff -U0 -- 'pollen.rkt'
=====================================================
Not a git repository
To compare two paths outside a working tree:
usage: git diff [--no-index] <path> <path>

fossil diff --unified -c 0 -- '[PATH/TO]/pollen.rkt'
==================================================================================
Index: pollen.rkt
==================================================================
--- pollen.rkt
+++ pollen.rkt
@@ -28,1 +0,0 @@
-                     syntax/parse
@@ -0,0 +32,1 @@
+TEST
@@ -35,1 +34,1 @@
-         "template-html.rkt"
+         "snippets-html.rkt"
@@ -39,1 +38,1 @@
-         (all-from-out "crystalize.rkt" "template-html.rkt"))
+         (all-from-out "crystalize.rkt" "snippets-html.rkt"))

Press ENTER or type command to continue

The result is the same whether using vim 8.1 inside a terminal emulator, or MacVim 8.1.577. The result is also the same regardless of which color scheme I am using.

Below are all the Signify-related lines from my vimrc:

let g:signify_vcs_list = [ 'git', 'fossil' ]
let g:signify_realtime = 0                   " realtime=1 results in continuous autosave!
let g:signify_sign_change = '~'
mhinz commented 5 years ago

Hmm, this line in the diff output looks odd:

@@ -28,1 +0,0 @@

With one line deleted at line 28, our code would expect -28,1 +27,0 or -28 +27,0 instead. And that's exactly what the output of git diff -U0 and diff -U0 looks like.

So, either our code is too strict or fossil's diff output is broken.

This is what our code does: https://github.com/mhinz/vim-signify/blob/3332ee2cd7328ed3011becc9cfec93d865e01cf2/autoload/sy/sign.vim#L88-L106

-28,1 +0,0 is parsed into old_line = 28, old_count = 1, new_line = 0 (here we'd expect 27 instead), new_count = 0.

So.. if you delete line 28, does it set the "deleted first line" sign on line 1 instead? :sign place should show something like:

line=1  id=262  name=SignifyRemoveFirstLine

Is this analysis correct so far?

otherjoel commented 5 years ago

Lo and behold, that’s exactly what happens, and there is a mark up at line 1.

I can say this is fossil’s output as of both versions 2.7 and the Jan 26 checkin of version 2.8. Maybe I ought to post in Fossil forums to see if anything changed, or if there is some accepted standard of diff output/behaviour that they try to adhere to.

mhinz commented 5 years ago

Please upgrade the plugin and make sure it's working now.

otherjoel commented 5 years ago

After updating to 64b226c I get the following error, which flashes on screen for only a moment. This error happens when opening any Fossil repo file that has a deleted line relative to that file’s most recent checkin on the current branch — or immediately after saving a file wherein I have deleted a line that is present in that file’s most recent checkin.

screen shot 2019-02-04 at 8 53 28 am
otherjoel commented 5 years ago

Also: discussion of this issue is also ongoing with Fossil devs: https://fossil-scm.org/forum/forumpost/d64c194361

mhinz commented 5 years ago

The developers seem to be willed to make a change, but in a corporate environment you often cannot assume the latest versions, so it should still be worked around in this plugin.

I pushed another try. Please upgrade and test again.

otherjoel commented 5 years ago

After updating and checking out your fossil branch, it works properly!

it should still be worked around in this plugin.

Very much agree. Thank you for your efforts on this!

mhinz commented 5 years ago

After updating and checking out your fossil branch, it works properly!

Yeah, I just noticed that I pushed it to the wrong branch.. Super Bowl night is taking its toll. :-) Pushed it to master as well now.

Very much agree. Thank you for your efforts on this!

Well, thank you for telling me about this rather grave bug!

otherjoel commented 5 years ago

Just to close the loop, the fix has been checked in to the Fossil source, and the new binary still works great with this plugin.