Fixing the automatic fold expansion (#1004)

[Chillee]

One of the most requested issues (and one of the things that bothers me the most) is that j and k automatically open up folds. #1004

However, this problem is kinda tricky to solve due to VScode having a somewhat limited API when it comes to folds.

The one thing that's exposed is "cursorMove". Although it says it should move "logically", the only command that allowed me to skip folds was "cursorMove" with "wrappedLine" set to true.

Thus, if you do the equivalent of a "gj" in Vim (which skips over folds without opening them), and then check if you've moved more than one line down, you know if you're over a fold or not.

Then, you can do a "gk" to get back* to where you are before if you're not over a fold, and follow up with a regular "j".

• There's one bug with gj that causes us to drop "desiredColumn" information. Check Issue with cursor position using visual line movement (gj and gk) #1323 and WrappedLine information in API microsoft/vscode#23045 for more information on that. This is probably the biggest issue with this potential fix. I think it's possible to fix it, but it might fairly involved. If you play around with this build, I think you'll find that this is the biggest pain point that this fix introduces. UPDATE: PR WIP: Fixes gj and gk cursor issues. #1561 should fix this. I've also integrated PR WIP: Fixes gj and gk cursor issues. #1561 into this fix, so this is now a superset of PR WIP: Fixes gj and gk cursor issues. #1561.

There's also a couple things that are broken from this change, like up and down arrows, and using j and k in visual mode. However, I think those can be fixed.

This is most definitely not an optimal solution (it's really hackish), but I think the performance hit may be negligible (many of our other operations are a lot more expensive), and it may be good enough while we wait for VSCode to extend their API.

It's also similar to the approach @xconverge wrote about in #1467.

In general, this approach has some other bugs that are fairly hard to fix (mostly related to desiredColumn issues). (UPDATE: I've fixed most of the desiredColumn issues). Due to the myriad issues (and the general hackishness of this approach), I would propose that we add this behind a flag, as a temporary fix for fold related issues. Looking at the issue thread, it seems to be a major pain point (it is for me too).

Overall, I'd just like some feedback on this approach. I realize the code quality isn't great, but think of this as a prototype that'll be rewritten if we decide it's worth it. If it's worth exploring, I can try fixing gj and the other related bugs. (UPDATE: I've fixed the gj bugs).

[johnfn]

This is really awesome man. It's really appreciated.

I'm going to be busy this weekend with Ludum Dare (ludumdare.com), so I can't check it out immediately, but I will soon.

Thanks again!

[Chillee]

No rush. Ludum dare is awesome! If only I didn't have a ton of work piled up over the next week :(

[Chillee]

I actually have a fix for 'gj' and 'gk' coming up that I think should make this PR a bit better.

[Chillee]

I committed the gj/gk fix. To me at least, it feels pretty good for navigating around the codebase with j and k.

I'm not really sure why my first commit is breaking so much stuff though. A lot of the tests it broke are unrelated.

[Chillee]

Alright I changed some things around and added it behind a flag. I think this is ready for merging; the only question is whether the flag should be on by default, or whether it should even be included at all.

I vote that it should definitely be included, at least under a flag.

This is currently complete in my eyes.

[Chillee]

I figured out a solution that's pretty solid, if I say so myself. This also fixes #1323 now as well.

[Chillee]

Actually, I think I have an even better solution (potentially). It'll need to wait a couple days though.

[Chillee]

Nevermind, my proper fix isn't possible right now. I'll keep the version that was working.

[Chillee]

Ok, as the current fold fix doesn't break anything else, is behind a flag, and isn't too bad of a fix, I'm planning on merging this at the end of the day (unless somebody objects). @rebornix @johnfn @xconverge

[Chillee]

Alright, I'm gonna merge this one now. I tried to do the thing that @rebornix was talking about, but it's not quite working out...

[ZuBB]

does anyone know when we can expect next release with this change?

ps: thanks a lot @Chillee! you are my hero 🙇

[Chillee]

Probably within the next day or two. @ZuBB Just so you don't get your hopes up too high, it's definitely not a perfect fix. It's a hack through and through, and leads to some (relatively) minor sideffects with j and k, namely that you'll see some flickering on wrapped lines (which is why it's not enabled by default).

However, it's definitely a huge improvement for folded lines, and I think it'll satisfy the needs of most users who care about folds.

[ZuBB]

@Chillee thanks for details on how it will work. Cant wait to try it 😄

[Chillee]

You'll also need to enable vim.foldfix to get the change, once it's released.

[ZuBB]

thanks for hint. I am sure it be usefull for others too

[Chillee]

@ZuBB We just pushed out a new release, so you can now try the vim.foldfix option!

[ZuBB]

thanks for a notification!

[elsonwx]

the M(jump to the middle of screen),nj(down number lines),nk(up number lines) seems still very troubled

[Chillee]

@elsonwx Unluckily those are things that are very tricky to fix. Folds have always been a tricky issue.