Use Treesitter for folding

[WhiteBlackGoose]

Ok so

This is a truly amazing plugin

However, folding really works like shit for some reason. It always breaks. Here's a simple example:

I know it can collapse properties and other neat stuff. However, I personally don't need it. For me, it now collapses sections just perfectly, and I keep my sanity.

[jgollenz]

How are you testing this? When I disable setlocal foldexpr=OrgmodeFoldExpr() in ftplugin/org.vim I get really wonky behavior for the foldtext. Do you mind sharing a gif of how it looks on your end.

Also, I might be missing something here, but wouldn't adding (property_drawer) to folds.scm take care of the properties as well? Seems™ to work

[WhiteBlackGoose]

It works perfectly for me, as it only collapses headers and tables.

Demo:

Also, I might be missing something here, but wouldn't adding (property_drawer) to folds.scm take care of the properties as well?

Absolutely, but I personally didn't like folding properties. Ofc you can add it

[jgollenz]

Do you mind sharing your config regarding foldexpr and foldtext? Unfortunately your changes don't seem to work for me. I can't fold tables and they break the foldtext.

In any case, I first need to figure out why we haven't been using treesitter for this up until now. If we can catch all the features we had so far also with treesitter, we can merge this.

One more thing: I'm not able to fold tables in emacs orgmode. The <TAB> is used to navigate cells there. From the gif it looks like you are folding the table from any location within the table, not just the first line. Is that correct?

[jgollenz]

btw, the tests are failing due to incorrect styling. Please run make format in the root of the repository. Thanks 👍

[WhiteBlackGoose]

Apparently I had ufo, without it, folding the table doesn't work. 🤔

I'm sorry for the confusion. But that's why it's a draft, and that's why I'm not in rush with styling changes/removing orgmode's folding.

[jgollenz]

No worries 😁 well, I'm not absolutely sure but I had the impression that folding tables was working for me yesterday. Might confuse it with property drawers though

[WhiteBlackGoose]

I'm not trying to be a dick, but currently the folding provided by the plugin works terribly. For me, it would make perfect sense remove all such functionality and move this job to things which do it well and consistently: treesitter, ufo (we probably can do it without ufo, I just don't know how).

But the plugin is great otherwise 😄 (for now, I'm on my own fork of it).

The is used to navigate cells there. From the gif it looks like you are folding the table from any location within the table, not just the first line. Is that correct?

Yes, but not with tab, but with default vim keys.

[jgollenz]

I am all for moving this to treesitter, but like I said, I first need to check what we are doing differently in our custom foldexpr and why.

Regarding the (property_drawer): I'm not a maintainer here, so I doubt I can push to your master branch. Either you add those changes or I have to create a new branch and commit with you as author. But since I can't merge my own PRs anyway, I'd prefer the former 😉

edit: but I guess you can merge my branch into yours then 🤷‍♂️

[kristijanhusak]

I didn't give this a test, but folding in here is a bit more complicated than folding in a more standard syntax like a programming language. I would suggest to also add drawer, property_drawer and block to the folds, and then we can see if that maybe can work out.

[kristijanhusak]

Gave it a quick test. This might actually work better than the current solution since it handles a few edge cases.
To summarize my review:

• Add drawer, property_drawer and block to the folds list
• Replace setlocal foldexpr=OrgmodeFoldExpr() with setlocal foldexpr=nvim_treesitter#foldexpr() in ftplugin/org.vim file
• Fix stylua formatting issues so build properly passes

[WhiteBlackGoose]

Very nice, now it works without ufo and it doesn't annoyingly autofold.

[kristijanhusak]

LGTM. @jgollenz can you also confirm if everything works as expected for you? Just want to make sure I didn't miss something. We will merge this and I'll write it on announcement issue that we switch to TS folding, just in case someone runs into some issue.

[WhiteBlackGoose]

hold up it doesn't work

[WhiteBlackGoose]

After a revisit:

• With UFO, it annoyingly autofolds (can be disabled with removing foldlevel, but then default folding doesn't work)
• Without UFO, it folds wrong again

[kristijanhusak]

Can you elaborate what's wrong without UFO?

[WhiteBlackGoose]

[WhiteBlackGoose]

There must be something that UFO overrides that I didn't

[kristijanhusak]

I think it works with UFO because it refreshes the folds fairly often, while without UFO that doesn't happen until you force edit the buffer or do zx manually. Do you have the same issues with the current orgmode folding?

[WhiteBlackGoose]

Slightly different, but close 🤔 .

[WhiteBlackGoose]

Good point about zx btw, it fixes folding in both regular orgmode and now with TS

[kristijanhusak]

I'll be merging this in since I don't see any difference in behavior between the current implementation and TS one. We can always revert it in case people find some issues with it. Thanks for the PR!

[jgollenz]

We should not forget about removing our custom foldexpr after some time, since it's dead code now.

[kristijanhusak]

We should not forget about removing our custom foldexpr after some time, since it's dead code now.

Yeah, I plan to do this once we make sure the current solution works.

[jgollenz]

Tangent: do you think we can move things from ftplugin/orgmode.vim into lua? The issue I had about foldtext was the the fillchar was set to \ , backslash+space, which my config stripped silently. A trailing space without a comment is not very explicit and in lua we would at least have 'fold: ' wrapped in quotes. I tried and it worked fine for things that are just options but it failed on the foldtext and foldexpr functions. I guess it's not possible to pass such lua functions to foldexpr without wrapping it in a vimscript function?

I actually think it's weird how this is handled in vim, given that a space just makes sense as a fillchar imo 🤷

In any case, I just think it would be handled cleaner in lua and pushes down our vimscript usage even further, which is a good thing in my book 😁

[kristijanhusak]

Since we are on v0.8+, I think we could move all the vim files to lua, except maybe syntax files. I'm just not sure how to handle these options that require a vim function, like foldtext, foldexpr, etc.. I assume we can do something like vim.opt_local.foldtext = 'v:lua.require("orgmode.org.indent").foldtext()' but I didn't give it a test. If you have something prepared create a draft PR and we can see if it works out.