kevinhwang91 / nvim-ufo

Not UFO in the sky, but an ultra fold in Neovim.
BSD 3-Clause "New" or "Revised" License
2.26k stars 47 forks source link

Include the bottom line inside a fold #26

Closed akinsho closed 2 years ago

akinsho commented 2 years ago

Feature description

In editors like intellij or simply using a custom fold text in vim, it is possible to include the bottom like as part of a fold like in the screenshot below

image

Note that the trailing } is displayed as part of the {...} on the folded line rather than below it on a new line.

Describe the solution you'd like

As described above it would be nice if a user could include the closing item e.g. the closing parentheses as part of a fold rather than trailing outside of it e.g.

// Desired outcome
function testThing() {...} // The closing parens are on the same line as the fold.

// Current state
function testThing() { ...
}

This would also allow something like

import {...} from "thing"

by using whatever the trailing content of the last fold line is appended to the fold text

Additional context

No response

kevinhwang91 commented 2 years ago

Depend on your folding range provider, but I found most of them can't support this feature for now. However, ufo use two interfaces to imp it, but must handle the function yourself.

-- Run me.
local async = require('async')
local promise = require('promise')
require('ufo').setup({
    provider_selector = function(bufnr, filetype)
        if filetype == 'javascript' then
            return function()
                return async(function()
                    local data = await(promise.all({
                        folds = require('ufo').getFolds('lsp', bufnr),
                        symbols = promise(function(resolve, reject)
                            -- request `textDocument/documentSymbol` to get the context you want
                            resolve('expected data')
                        end)
                    }))

                    local function handle()
                        -- handle the folds
                        -- and export the context
                        _G.ufo_context = ''
                        return data.folds
                    end

                    return handle()
                end)
            end
        end
    end,
    fold_virt_text_handler = function(virtText, lnum, endLnum, width, truncate)
        -- make your new virtText append `}` with `ufo_context`
        return virtText
    end
})

import {...} from "thing"

  1. If the modules are multiple lines, ufo can imp it like the above code snippet I post.
  2. If the modules are all in single la ine, ufo can't redrew the virtual text. For now, only work with foldminlines > 0.

Perf will regress if foldminlines == 0, need time to export how many.

akinsho commented 2 years ago

@kevinhwang91 not entirely sure if I understand exactly what this solution means really but a few things to note are that I don't really want this to be a per language thing I think (at least in my head) the idea would be that since presumably the fold is an expr there could be a general solution to just extend the range of the fold from the LSP provider by 1 as an option, so it always captures the last line. This way users who want this don't have to re-implement providers themselves just to extend the range, I don't really know how straightforward or not this is really tbh I just assumed that the mechanism used involved telling nvim where the start and end of a fold was so that it would just be a matter of adding one.

Then all that would be needed would be to append the contents of endLnum to the virtual text.

Tbh the import example was just an example so not the end of the world if that can't work

kevinhwang91 commented 2 years ago

This feature is IDE-like feature and must customize for a certain language because LSP doesn't support it. I can't find an easy way to imp this feature because lacking support from LSP, and VSCode also doesn't support it officially.

We must solve the issue of foldingranges source first and then consider the virtual text.

akinsho commented 2 years ago

@kevinhwang91 just so I understand, since I think I don't follow exactly how the internals of this plugin work. Is the implementation of the folding range given by the LSP something that cannot be altered a little? i.e. I assume but don't know for sure that the LSP gives you something like the Range of a fold e.g. start = 1, end = 3 or something, so in my head you would query the LSP for the folding range and say it returns end = 3 you can manipulate it by just adding one if this (potential new) config option is added.

Does it not work like this? I only ask because something like treesitter allows this and before using this plugin I was using treesitter to create folds, and I was just appending the v:foldend text onto the end of the foldtext.

kevinhwang91 commented 2 years ago

ufo request folding range from providers (language server or native provide like indent), return https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#foldingRange and apply them by s,efold command (but using FFI is a bit diff)

If you don't care about the perf, you can use multiple providers to hack (lsp and treesitter). AFAIK, using symbols and fold ranges from the language server will get the best perf. You also can use a middleware to enhance fold range to support it directly from the language client-side.

akinsho commented 2 years ago

If you don't care about the perf, you can use multiple providers to hack (lsp and treesitter)

I would definitely not want to avoid degrading performance tbh so wouldn't want this method.

You also can use a middleware to enhance fold range

Can you explain a bit more how this would work? Is there an vim.lsp.handers['foldingMethod'] to override or something so that I can change the value that is returned by the LSP so that it is adjusted to behave as I've described?

kevinhwang91 commented 2 years ago

Can you explain a bit more how this would work? Is there an vim.lsp.handers['foldingMethod'] to override or something so that I can change the value that is returned by the LSP so that it is adjusted to behave as I've described?

Unformatually, nvim core doesn't imp a folding range handler for now, ufo implement itself. You can use require('ufo').getFolds('lsp', bufnr) get the folding, return value is promise wrapping folding.

You should refer the snippet I post above, should work.

akinsho commented 2 years ago

I just tried this out locally, and it doesn't quite work, unfortunately. Just adding one to all the end lines of each fold and then trying to append the adjusted last line doesn't work since for example

local t = {
  field = { -- the fold end here comes back as "nested = {"
    nested = {
        value = 1
    },
  }
}

The end line is sometimes incorrect. There needs to be a smarter mechanism to determine the end rather than a brute force + 1. I guess this comes back to what you said about how the lsp handles this.

I guess one way could be when processing the folds you could fetch the lines in the buffer once and check the endLine + 1 and if it matches like closing characters e.g. /^(\]\)\})/ then it gets advanced by one otherwise it should be left alone.

Not sure if you could get a robust solution for languages with parens and braces, but maybe a lot of brittle code to take on. I think with a treesitter provider though you could get this behaviour easily since this is already how nvim-treesitter's fold mechanism already works

akinsho commented 2 years ago

Found some issues relating to this for vscode and the LSP. As a side note, it seems that the collapsedText field was added to allow custom fold text from the language server so might be cool to support, not that I know/use any that do that.

kevinhwang91 commented 2 years ago

The end line is sometimes incorrect. There needs to be a smarter mechanism to determine the end rather than a brute force + 1. I guess this comes back to what you said about how the lsp handles this.

Yes, need AST context, like symbols. Intellij doesn't handle if, else, and else if blocks.

I think with a treesitter provider though you could get this behaviour easily since this is already how nvim-treesitter's fold mechanism already works

You can wait for the treesitter provider this weekend if you like the folding of treesitter. I don't like treesitter folding because it can't fold if, else, and else if blocks totally.

Found some issues relating to this for https://github.com/microsoft/vscode/issues/3352 and https://github.com/microsoft/language-server-protocol/issues/1426. As a side note, it seems that the collapsedText field was added to allow custom fold text from the language server so might be cool to support, not that I know/use any that do that.

Folding in Neovim is a line range, not a char range, need hack. IMO, hard to imp as a plugin, but can hack by the person who needs it.

akinsho commented 2 years ago

You can wait for the treesitter provider this weekend if you like the folding of treesitter

That sounds great 🚀

I don't like treesitter folding because it can't fold if, else, and else if blocks totally.

Tbh I'd much rather trade the behaviour of folding completely even if it meant not having if/else clauses, but AFAIK treesitter can fold these as long as the folds.scm has those nodes correctly identified and contributed in upstream nvim-treesitter.

Re. the collapsed text was only mentioning in case you hadn't seen it, I didn't realize it only worked with character range folds, in which case fair enough to leave it

kevinhwang91 commented 2 years ago

The treesitter provider has done, after using treesitter provider I know why you raise this issue now.

Even using treesitter provider, ufo can't do it perfectly, for example

void test()
{
    printf("test");
}

vs

void test() {
    printf("test");
}

vs

void test() { // how to handle this comment?
    printf("test");
}

Fortunately, users can hack what behavior they want in fold_virt_text_handler, I think getting the context is simple enough if using treesitter.

akinsho commented 2 years ago

@kevinhwang91 not sure how the folds are being handled i.e. via nvim-treesitter fold queries or not, but I think the specific language edge cases can be handled in each language's folds.scm i.e. in the first example if this was c or something then I imagine that the fold range would be defined as being lines 2-4 e.g.

void test()
{...}

Regarding the comment I think when generating the fold text by default it should be everything up till the opening parens and then ellipsis closing paren, it's not reasonable to expect to see the comment IMO and if the user wants that then they can do that themselves I don't think an IDE would show that

akinsho commented 2 years ago

I was able to achieve this using the new treesitter provider

image

Using

ufo.setup({
  open_fold_hl_timeout = 0,
  provider_selector = function(_, filetype)
    return { 'treesitter', 'indent' }
  end,
  fold_virt_text_handler = function(virt_text, lnum, end_lnum, width, truncate)
    local result = {}
    local _end = end_lnum - 1
    local final_text = vim.trim(vim.api.nvim_buf_get_text(0, _end, 0, _end, -1, {})[1])
    local suffix = final_text:format(end_lnum - lnum)
    local suffix_width = vim.fn.strdisplaywidth(suffix)
    local target_width = width - suffix_width
    local cur_width = 0
    for _, chunk in ipairs(virt_text) do
      local chunk_text = chunk[1]
      local chunk_width = vim.fn.strdisplaywidth(chunk_text)
      if target_width > cur_width + chunk_width then
        table.insert(result, chunk)
      else
        chunk_text = truncate(chunk_text, target_width - cur_width)
        local hl_group = chunk[2]
        table.insert(result, { chunk_text, hl_group })
        chunk_width = vim.fn.strdisplaywidth(chunk_text)
        -- str width returned from truncate() may less than 2nd argument, need padding
        if cur_width + chunk_width < target_width then
          suffix = suffix .. (' '):rep(target_width - cur_width - chunk_width)
        end
        break
      end
      cur_width = cur_width + chunk_width
    end
    table.insert(result, { ' ⋯ ', 'NonText' })
    table.insert(result, { suffix, 'TSPunctBracket' })
    return result
  end,
})

@kevinhwang91 do you think this could be provided as an option for treesitter only or at least maybe there can be a simpler way to override or tweak the suffix of the virt text that doesn't require a user writing around 30 lines of boilerplate. Maybe something like

-- virt text is just one line not all of the different lines
virt_text_suffix = function(virt_text, lnum, end_lnum, width)
  -- insert 2 items into the list instead of doing all of the work above.
end
kevinhwang91 commented 2 years ago

@akinsho Good jobs.

do you think this could be provided as an option for treesitter only or at least maybe there can be a simpler way to override or tweak the suffix of the virt text that doesn't require a user writing around 30 lines of boilerplate. Maybe something like

For now, I'm not sure.

ranjithshegde commented 2 years ago

@akinsho I dug into the source code a bit more and made a slightly more decorative end_lines config. Please check this gist if you are interested

akinsho commented 2 years ago

@ranjithshegde that's really cool 🚀 although looking at the implementation I'm more convinced that this is best done as something configurable in the plugin since this solution really requires understanding the internals of the plugin to get this result.

You have to mess with its namespace and set your own extmarks which is fine for anyone who is confident doing that but will mean anyone else interested in this functionality will be stuck copy pasting code they likely won't understand. Also, if the internal implementation changes which this solution (which is optimal) depends on, then a user's config will also break and for the inexperienced quite confusingly.

kevinhwang91 commented 2 years ago

Please open a new issue to apply API interface if feasible.

ranjithshegde commented 2 years ago

@kevinhwang91 I was thinking of something like this

-- In lua/ufo/decorator.lua -- function onEnd(name, tick), line 110
if someProviderGetApi == "treesitter" and userDefinedCoing.enable_fold_end_virt_text then
  local end_text = api.nvim_buf_get_lines(bufnr, endLnum - 1, endLnum, false)[1]
  local end_virtText = render.getVirtText(bufnr, end_text, width, endLnum, syntax, nss)

  for _, v in ipairs(end_virtText) do
      table.insert(virtText, v)
  end
end

If you are open to something like this then I could open a PR

EDIT: If you dont want to change the internals because the above code is too opinionated (or other reasons) then maybe you could paste the gist i linked in an advanced_configuration section?

kevinhwang91 commented 2 years ago

Use the gist first, if there is no issue, I will export the local function getVirtText(bufnr, lnum, width) end and namespace to https://github.com/kevinhwang91/nvim-ufo/blob/27c466886959fad03a09dd62814e126ff8178a1a/lua/ufo.lua#L115-L118

The last param in the handler is the context the ufo used.

akinsho commented 2 years ago

I think making sure the gist works without issue first is good, but exposing even more internals only means that at least the snippet will depend less on the internals of this plugin directly, but it still won't stop users from having to implement/copy this chunky piece of code, I think it's fine to have this sort of advanced configurability, so users can go nuts, but I also think it would be nice for users to just toggle an option and have this just work especially since the implementation i.e. LOC in this plugin seem fairly small.

Also, the more the internals are exposed, the more bound you will become to the API for fear of breaking configs.

I personally think this sort of thing puts a burden on the user for what I guess is a fairly popular feature (not that I have metrics, just looking at the linked vscode issue and the no. of likes here 🤷🏿‍♂️, which is not science)

But at the end of the day I don't want to put too much pressure since this is largely all achievable for me anyway, and it's entirely up to you @kevinhwang91 if it's something you want to maintain or not. I guess with changes to the virt text handler and discussion above, I can solve my own use case. I was just more thinking of newer/other people.

kevinhwang91 commented 2 years ago

I personally think this sort of thing puts a burden on the user for what I guess is a fairly popular feature (not that I have metrics, just looking at the linked vscode issue and the no. of likes here 🤷🏿‍♂️, which is not science)

But at the end of the day I don't want to put too much pressure since this is largely all achievable for me anyway, and it's entirely up to you @kevinhwang91 if it's something you want to maintain or not. I guess with changes to the virt text handler and discussion above, I can solve my own use case. I was just more thinking of newer/other people.

I agree with you this feature is popular. The issue here is hard to handle the end line perfectly even in VSCode. As a maintainer, I can reduce the obstacles for users , but don't want to maintain the feature with unsolved issues, this is my requirement for my plugin.

ranjithshegde commented 2 years ago

@kevinhwang91 I made the config/gist after yesterday's commit 98b07fc and it works fine for me. Tried it with even unique treesitter parsers like latex, supercollider and orgmode. No problems so far.

So, putting this as an advanced example seems good.

As to getting it into codebase as in any form, I do agree that this is a rather opinionated example, and unless people are tinkering with fold methods by changing foldtext or writing their own methods, etc (which would make them sort of advanced users) I think most people would just accept and use the defaults. So for these advanced users, just using the gist or creating their own more efficient versions is fine

kevinhwang91 commented 2 years ago

@kevinhwang91 I made the config/gist after yesterday's commit 98b07fc and it works fine for me. Tried it with even unique treesitter parsers like latex, supercollider and orgmode. No problems so far.

So, putting this as an advanced example seems good.

As to getting it into codebase as in any form, I do agree that this is a rather opinionated example, and unless people are tinkering with fold methods by changing foldtext or writing their own methods, etc (which would make them sort of advanced users) I think most people would just accept and use the defaults. So for these advanced users, just using the gist or creating their own more efficient versions is fine

Open a feature request, please.

akinsho commented 2 years ago

The issue here is hard to handle the end line perfectly even in VSCode. As a maintainer, I can reduce the obstacles for users , but don't want to maintain the feature with unsolved issues, this is my requirement for my plugin.

So I think this might be the point that is getting a little confused. For treesitter this is a solved problem. In order to get it to work for all providers e.g. LSP which is what I think you are referring to, I think this will require changes in dozens of LSP since presumably they all implement this differently. I personally don't think the LSP servers are likely to all adopt this behaviour, but since treesitter is largely agnostic to that sort of issue, I think it's a good way to get this functionality for anyone who wants it. I don't think it's practical to get exactly the same type of folding behaviour from multiple sources, so if this request is not feasible for LSP I think that's fine since it's possible with treesitter

I think this is why offering this as an option if a user is using treesitter is good and fairly uncomplicated since out of the box treesitter can do this for most languages I guess. As to this being an opinionated setting, I think the point is that it's a "popular" opinion but regardless of this it would be gated by an option and not default 🤷🏿‍♂️

Anyway, for fear of making this all too confusing, either of these options is perfectly fine

  1. A gist to use that doesn't depend too heavily on internals, i.e. things like namespace are exposed to the user, and they don't have to require modules that are not maybe intended for public use/breakable
  2. An option to do auto this automatically and only when set to true and only for treesitter
ranjithshegde commented 2 years ago
  1. A gist to use that doesn't depend too heavily on internals, i.e. things like namespace are exposed to the user, and they don't have to require modules that are not maybe intended for public use/breakable
  2. An option to do auto this automatically and only when set to true and only for treesitter

@akinsho

  1. Great point! my example uses internals that was perhaps never meant for the user to tinker with
  2. For this point the below example does do the job, but keep in mind that this is my hack, after having inspected only the relevant parts of the codebase, I have not studied the whole project, so probably it is too hacky

    -- In lua/ufo/decorator.lua -- function onEnd(name, tick), line 110
    if someProviderGetApi == "treesitter" and userDefinedCoing.enable_fold_end_virt_text then
    local end_text = api.nvim_buf_get_lines(bufnr, endLnum - 1, endLnum, false)[1]
    local end_virtText = render.getVirtText(bufnr, end_text, width, endLnum, syntax, nss)
    
    for _, v in ipairs(end_virtText) do
      table.insert(virtText, v)
    end
    end
kevinhwang91 commented 2 years ago

Add enable_fold_end_virt_text is not bad.

ranjithshegde commented 2 years ago

Thanks for the commit! works like a charm!

akinsho commented 2 years ago

Going to close this out since I think there's a good enough solution that solves the issue I had

akinsho commented 2 years ago

@kevinhwang91 not a bug and also not related to this issue just not sure how else to get this information to you, you might already know don't know if you are on the neovim matrix or watching PRs but https://github.com/neovim/neovim/issues/19226 is in progress which might be interesting for you (just a random FYI not specific motivation)

kevinhwang91 commented 2 years ago

@kevinhwang91 not a bug and also not related to this issue just not sure how else to get this information to you, you might already know don't know if you are on the neovim matrix or watching PRs but neovim/neovim#19226 is in progress which might be interesting for you (just a random FYI not specific motivation)

Thanks for your info. I read the source code before I release this plugin. I think it's not worth doing this in the core if the core team wants to borrow code from Vim.

But an effective API to add folds may help others, but not me, I have already used FFI to boost.

no-more-secrets commented 1 year ago

@kevinhwang91 I took a look at the examples, but I still don't quite understand how to use this feature to remove the last } on the last line... can someone post an example? The example in the example.lua file just prints some things.

kevinhwang91 commented 1 year ago

@kevinhwang91 I took a look at the examples, but I still don't quite understand how to use this feature to remove the last } on the last line... can someone post an example? The example in the example.lua file just prints some things.

https://github.com/kevinhwang91/nvim-ufo/issues/130

gzagatti commented 10 months ago

@no-more-secrets

I manage to put something together that shows the bottom line using the provided snippet:

require'ufo'.setup({
  enable_get_fold_virt_text = true,
  fold_virt_text_handler = function(virtText, lnum, endLnum, width, truncate, ctx)
    -- include the bottom line in folded text for additional context
    local filling = ' ⋯ '
    local sufWidth = vim.fn.strdisplaywidth(suffix)
    local targetWidth = width - sufWidth
    local curWidth = 0
    table.insert(virtText, {filling, 'Folded'})
    local endVirtText = ctx.get_fold_virt_text(endLnum)
    for i, chunk in ipairs(endVirtText) do
      local chunkText = chunk[1]
      local hlGroup = chunk[2]
      if i == 1 then
          chunkText = chunkText:gsub("^%s+", "")
      end
      local chunkWidth = vim.fn.strdisplaywidth(chunkText)
      if targetWidth > curWidth + chunkWidth then
        table.insert(virtText, {chunkText, hlGroup})
      else
        chunkText = truncate(chunkText, targetWidth - curWidth)
        table.insert(virtText, {chunkText, hlGroup})
        chunkWidth = vim.fn.strdisplaywidth(chunkText)
        -- str width returned from truncate() may less than 2nd argument, need padding
        if curWidth + chunkWidth < targetWidth then
          suffix = suffix .. (' '):rep(targetWidth - curWidth - chunkWidth)
        end
        break
      end
      curWidth = curWidth + chunkWidth
    end
    return virtText
  end,
})