Regression: saving buffer will affect folding

[wookayin]

Neovim version (nvim -v | head -n1)

NVIM v0.8.0-dev+1856-g58323b1fe

Operating system/version

macOS 12

How to reproduce the issue

1. Open some fold manually (with zr, etc.) while &foldlevel remaining unchanged
2. Save the buffer, e.g., :w

nvim-ufo config is quite minimal, e.g.,

  require 'ufo'.setup {
    open_fold_hl_timeout = 150,
  }

FYI, the file I was working on is a python file where foldmethod=expr.

Expected behavior

All open/close folds should remain intact and untouched. This would include all folds that were manually opened or closed. This behavior was fine in d99d722.

Actual behavior

Bug: Folds will close (or open) as per the &foldlevel variable.

After doing git bisect, I think the first commit that breaks the behavior is d0bf0cb.

[kevinhwang91]

It's expected, using below config to avoid:

local ftMap = {
    python = ''
}
require('ufo').setup({
    provider_selector = function(bufnr, filetype)
        return ftMap[filetype]
    end
})

ufo only respect diff and marker foldmethod if the provider_selector return not '' value.

[kevinhwang91]

This sounds overbearing, but I don't think it is necessary to use other foldmethods after the treesitter provider is added. Only few filetype buffer will use a specific expr, IMO.

[wookayin]

Thanks, so it's related to the "fold provider" as in the README: https://github.com/kevinhwang91/nvim-ufo#customize-provider-selector

Is the fold provider supposed to invalidate all the existing folds (upon saving) and update to the new fold by default and that's what you intended? What if other foldmethod is used, such as indent/filetype? Would we still lose the fold as soon as folds are updated? I would not agree with the default behavior of losing folds whenever the folds get updated for some reason, but if that's the case I could always disable the fold provider.

FYI, I am using foldmethod=expr with treesitter folding, foldexpr=nvim_treesitter#foldexpr().

[kevinhwang91]

foldmethod is window opt, which may be changed by the user and other plugins. The new window opts in Neovim will inherit the last one which makes ufo doesn't update if ufo only updates in manual foldmethod. I don't want to waste time to solve this non-sense issue. Users should make sure the fold works well if they use ufo and other fold plugins.

You can disable all the fold providers if you want.

[wookayin]

Thanks for your reply. What I've learned after reading your comments and documentations carefully again is that nvim-ufo works with foldmethod=manual (but actual folds are obtained from the provider, through foldexpr, or whatever provider strategies). The foldmethod=expr is a very usual setup if nvim-ufo were not used, so I happened to have that option.

So the behavior in this issue seems to be my misunderstanding and now I can get understand why you thought the issue is "non-sense", probably due to my ignorance of how nvim-ufo works and what it expects, but I would like to mention that the mechanism and implementation details are not necessarily crystal clear for end users (from what can be tell from README). With all due respect, I don't think it is a non-sense to feel the unexpected behavior itself where folding is lost whenever saving (regarldess foldmethod=manual or foldmethod=expr) is inconvenient.

Anyway, thanks for providing the workaround and your efforts for the great plugin!

[kevinhwang91]

I will add some details on how ufo works after treesitter provider is added.

If you have read the codebase of ufo, you will find that ufo has made great efforts to improve its performance and defeat any fold plugins.

Theoretically, perf of treesitter provider will be better than set foldexpr=nvim_treesitter#foldexpr() in ufo.

[wookayin]

If one uses the treesitter provider, the workaround of disabling provider (empty string "") is no longer valid. As soon as one saves the buffer, all folds will be reset (or everything gets closed in my case) as per foldlevel. So, provided treesitter fold provider is used, how am I supposed to stop all the folds reset?

[kevinhwang91]

the workaround of disabling provider (empty string "") is no longer valid.

Can't reproduce it, post your setup.

As soon as one saves the buffer, all folds will be reset (or everything gets closed in my case) as per foldlevel. So, provided treesitter fold provider is used, how am I supposed to stop all the folds reset?

Increase your foldlevel, Once apply folds with foldmethod=manual, Neovim will respect the foldlevel and close folds smaller than foldlevel.

[wookayin]

My config is very minimal, as in #6:

  require 'ufo'.setup {
    open_fold_hl_timeout = 150,
    provider_selector = function(bufnr, filetype)
      return {'treesitter', 'indent'}
    end,
  }

the workaround of disabling provider (empty string "") is no longer valid.

Can't reproduce it, post your setup.

This refers to a logical reasoning that the empty-string provider cannot be used any more because treesitter provider is used instead.

Increase your foldlevel, Once apply folds with foldmethod=manual, Neovim will respect the foldlevel and close folds smaller than foldlevel.

No matter what foldlevel value is used, manually open/closed folds should remain untouched when saving, leaving the insert mode, etc. --- as soon as fold is updated and treesitter parse tree is updated. This is the behavior of vim's folding and what we used to have before (or without the nvim-ufo plugin).

Please look at the video demonstration:

[kevinhwang91]

This refers to a logical reasoning that the empty-string provider cannot be used any more because treesitter provider is used instead.

Please look at the config #6 I posted carefully.

Please look at the video demonstration:

set foldlevel=99 foldlevelstart=-1

[wookayin]

Of course I read them carefully. The effective configs are actually identical, I just wanted to use treesitter provider for the demonstration purposes. Indeed the exact same config in #6 doesn't change anything and irrelevant to the behavior, as long as treesitter provider is used.

Having the maximum possible foldlevel set foldlevel=99 foldlevelstart=-1 would prevent all the folds being closed when updated, but this is not a perfect solution because one may still change the foldlevel while editing. For example, zM (close all the folds) will make foldlevel=0 and zR (open all the folds) will make foldlevel=99 (or actually the deepest fold level). I would like to emphasize again that no matter what foldlevel is used, the folds that were open/closed manually by users should remain untouched. Please compare the behavior without nvim-ufo being used)

What would I be missing?

[kevinhwang91]

Of course I read them carefully. The effective configs are actually identical, I just wanted to use treesitter provider for the demonstration purposes. Indeed the exact same config in #6 doesn't change anything and irrelevant to the behavior, as long as treesitter provider is used.

open a new issue and use a minimal config please.

Having the maximum possible foldlevel set foldlevel=99 foldlevelstart=-1 would prevent all the folds being closed when updated, but this is not a perfect solution because one may still change the foldlevel while editing. For example, zM (close all the folds) will make foldlevel=0 and zR (open all the folds) will make foldlevel=99 (or actually the deepest fold level).

Please check out #7, need to remap zM and zR.

I would like to emphasize again that no matter what foldlevel is used, the folds that were open/closed manually by users should remain untouched.

Can't fix it, this is the limit of manual. I have read the source code, FFI also can't work.

[wookayin]

OK Thanks. I'd like to mention that the config was already minimal and the issues I am mentioning still lie in the same context. But if you mind, for this issue I'll be just simply sticking to 'NO' fold provider.

I know you do have a good, inevitable reason nvim-ufo needs to remap or mimick existing fold-related behaviors (#7) to overcome the limitation, but from an end user's perspective that is not necessarily obvious and such discrepancies can make a confusion. Thanks for your explanation -- look forward to documentation. When the treesitter feature is complete and becomes more stable to use with other bugs you mentioned are fixed, I'll revisit and open a new issue if there is any other problem.

[kevinhwang91]

but from an end user's perspective that is not necessarily obvious and such discrepancies can make a confusion.

Will add the document to README later. For me, writing a document is a tough task.