vim-coiled-snake breaks vim diff for python files

[mikeboiko]

vim-coiled-snake breaks regular vim diff for python files by changing the fold settings

For example, if I run: vim -d test1.py test2.py, I initially see a diff with proper folding

If I move the cursor over from the left pane to the right pane, my folding breaks

As soon as I disable this plugin, everything starts working normal again.

This is my vim version:

VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Nov 15 2020 16:46:37)
Included patches: 1-1989
Compiled by Arch Linux

I love this plugin - thanks for all the good work!

[subnut]

@mikeboiko Will you please try #28 ? Thanks!

[mikeboiko]

@subnut, thanks for the quick reply. I checked out #28 and I'm still experiencing the same problem.
Please let me know if you have any other ideas.

[subnut]

@mikeboiko Please update to master branch... See if it has been fixed..

[mikeboiko]

@subnut, I just pulled the master branch and tried again but the problem still exists.
Let me know if you need me to dump any detailed logs.

[subnut]

I am sorry but I am unable to reproduce the bug.....
Could you please post an example?

[mikeboiko]

No problem, it sounds like the problem may be with my specific .vimrc
It's just strange that it worked fine for over a year and then suddenly started behaving like this. Perhaps a change in the latest version of vim though.
Regardless, I'm going to do some testing/digging and I'll get back to you with my findings.

[subnut]

@mikeboiko Many changes were implemented in this plugin the last month....
So it could be this plugin, too.

Try a minimal .vimrc with only this plugin enabled, and see if the problem persists.

e.g since you use vim-plug, make a file at your home directory called temp.vimrc with this contents -

call plug#begin(vimPlugDir)
Plug 'kalekundert/vim-coiled-snake'
call plug#end()

and then start vim with vim -u ~/temp.vimrc -d ~/temp/test1.py ~/temp/test2.py

Please report if the problem still occurs

[subnut]

@kalekundert Can you please check if you can reproduce this issue? I am unable to..

[mikeboiko]

@subnut, I haven't gotten to the core of the issue yet.
However, I did do a test on a clean vimrc as you suggested. The problem doesn't exist on a clean vimrc.
Once I figure out what the conflict is with my .vimrc and the best solution, I will post that info here.

[kalekundert]

I can reproduce something at least similar to this issue. I used files from the tests directory, since they're specifically configured to be sandboxed from other config files present on the system. Here are the exact commands I used:

$ cd vim-coiled-snake/tests
$ ./run_vim.sh -d test_everything.py test_sandbox.py

In this case, the .vimrc file is:

set cpoptions-=C

call plug#begin()
Plug '~/vim-coiled-snake'
Plug 'Konfekt/FastFold'
call plug#end()

set foldcolumn=5
set signcolumn=yes
set number

With this setup, I could cause the folds to get out-of-sync (i.e. opening/closing a fold in one split wouldn't affect the other split) by moving the cursor to the right split, then moving it back to the left split. This behavior only happened when the vim-coiled-snake plugin was present, and was unaffected if the rest of the lines in the vimrc were removed. Notably, the foldmethod setting in the right split changes from diff to manual after moving back and forth. Resetting this setting to diff fixes the behavior until the cursor moves back to the other split.

So I do think this is a coiled-snake bug, but I'm still not sure exactly what's going wrong or how to fix it.

[mikeboiko]

@kalekundert, yea I tested it again with a similar .virmc to the one you posted and I am getting the error.
Not sure why I wasn't getting this error on when I tested a few days ago on my other computer.
But yes, I agree that there seems to be a bug in this plugin.

[kalekundert]

I think I figured out what was going on. Basically, the FileType autocommand was being triggered before the &diff setting was being set, which was causing the plugin to initialize itself incorrectly. Let me know if this fixes the problem for you.

[kalekundert]

Also let me know if you have any other problems. I pushed a fairly major change earlier today, and while it works on all the examples I have, it's possible that there will be some regressions.

[mikeboiko]

Hey @kalekundert, I just tested the latest master commit and this problem has been fixed!
I will definitely let you guys know if I run into any new issues.
Thanks again for the awesome plugin!

[subnut]

Basically, the FileType autocommand was being triggered before the &diff setting was being set

That's why! 😲
During my (scarce) testing, it did seem to me that something was off,
but I couldn't quite understand what was causing it.

Great find!

(I know I should've tested better. I usually do. But my exams were going on....)

[kalekundert]

No worries, this was a pretty obscure bug.

[mikeboiko]

@kalekundert, I did notice another bug when performing diffs with the vim-fugitive plugin.
I'm sure you're at least a little familiar with this tpope plugin, but to summarize, you can run :Gdiff to show the diff as compared to the last git commit.
If I'm performing a vim diff with :Gdiff as opposed to vim -d file1.py file2.py, my original bug still exists.
This used to work perfectly fine, but broke a couple of months ago. This is actually how I noticed the bug initially.
Let me know if you have any thoughts. I can create a new issue if that makes it cleaner.

[kalekundert]

Hmm, ok, I'll look into that. I'm not totally surprised, because diff-mode is handled as a little bit of a special case when the plugin starts up, so this sounds like the kind of corner case that might slip through.

FYI, the reason this broke is that we changed how coiled snake initializes itself. Previously all of its settings were "buffer local".
But fold-related settings are supposed to be "window local", e.g. so you could have two different views of the same buffer with different folding rules. I'm not sure how common it is for this distinction to really matter, but it's definitely how things should be. To make this change, we had to manually do some initialization steps that were previously done by vim, and that seems to be the source of the diff-related problems. Thanks for bearing with us, though.

No need to make a new issue, we can stick with this one.

[mikeboiko]

Sounds good, thanks @kalekundert
Yes, it definitely makes sense to use "window local" settings for folding.

[subnut]

Yeah.. while making the options window local fixed some issues, it made new issues regarding diff.
Maybe we're doing something wrong...?

But atleast we have confidence that we are doing one thing right, keeping window-local things window-local, and not buffer-local.

I'm sure that we'll figure out the solution to this problem soon. 👍

EDIT: #5 Was fixed by the above-mentioned change

[mikeboiko]

lol I know how it is - usually I'm happy if I'm fixing more things than I'm breaking :D

[subnut]

@kalekundert I just got an idea...
How about this -
ftplugin/python/coiledsnake.vim

let b:coiled_snake_should_fold = 1

And then, whenever we enter a new window, we first check for this variable.

If that variable exists, we next check whether :set foldmethod and :set foldexpr and :set foldtext are set to their default (global) values. If yes, we can implement our folding. If no, that means something is going on (diff-mode, user's custom function, etc.) and we shouldn't fold.

[subnut]

@mikeboiko I just implemented what I mentioned above in #31. Could you please check whether that fixes this issue?

[mikeboiko]

@subnut, wow I appreciate you working on this so late! I'm in Canada and it's 2:56 PM here.
I just tested the latest code by running gh pr checkout 31 and I kept getting error messages every time I went into a new file. I wasn't able to successfully run :Gdiff

For example this is the output of my error messages

|| Error detected while processing function ctrlp#init[30]..FileType Autocommands for "*":
|| E117: Unknown function: <SNR>47_OnFileType
|| "scrape-inventory.py" 368L, 13758B
|| E117: Unknown function: <SNR>47_OnFileType
|| Error detected while processing function fugitive#Diffsplit[123]..function fugitive#Diffsplit[106]..BufReadCmd Autocommands for "fugitive://*//*"..BufRead Autocommands for "*.py"..FileType Autocommands for "*":
|| E117: Unknown function: <SNR>47_OnFileType

[mikeboiko]

@subnut, it works great now! Thank you for your dedication and hard work!

[subnut]

Whew. Finally!

@kalekundert please check if it works on your
side too, and please report bugs, if any, at #31

@mikeboiko thank you for being patient, and
reporting the bugs in the first place 👍

@subnut thank you for taking the time to
solve this even at 5:00 in the morning.

---

Guess where I live?

[subnut]

@mikeboiko does vim -d file1.py file2.py still work? Could you check it once?

I ask because in #31 I removed that part of the code that @kalekundert had implemented to make it work.

[mikeboiko]

@subnut, yep the regular vim diff still works good.

[mikeboiko]

Ok, I did just discover another bug on the subnut-fix-27 branch.
In this gif, I open 2 different python files and when I switch between them, my folds get opened/closed for some reason.

[subnut]

@mikeboiko You have :set foldmethod=marker in your .vimrc?

This is expected behaviour for now, because the folding is set/reset when the cursor moves into a window.
I am still looking for a reliable way to decide when a buffer is shown/removed from a window without having the cursor in it....

Do let me know if you have any ideas..

[subnut]

@mikeboiko I just pushed a new commit on the subnut-fix-27 branch. Do let me know if that fixes the issue.
Also, if it does solve the issue, please remember to check whether the previous issues in this thread remain fixed,
or whether the new commit re-introduces any of the issues. (I don't think it will...)

[mikeboiko]

@subnut, yep all the scenarios are working well now. thank you for that!

[mikeboiko]

@kalekundert and @subnut, it seems that commit 431d705 brought this problem back.
If I checkout HEAD^^ (5807a09), the problem is gone agin.
Could you take a look please?

[kalekundert]

Thanks for bringing this up. I'll look into it as soon as I can, hopefully this weekend.

[mikeboiko]

Thanks for bringing this up. I'll look into it as soon as I can, hopefully this weekend.

@kalekundert, just wanted to see if you had a chance to look at this yet? Thanks.