Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FR: Simplify provider_selector by accepting more than one fallback #256

Open
chrisgrieser opened this issue Oct 23, 2024 · 8 comments
Open
Labels
enhancement New feature or request

Comments

@chrisgrieser
Copy link

chrisgrieser commented Oct 23, 2024

Feature description

provider_selector right now only accepts a return value with two strings. However, I generally would find it useful to define more fallbacks. For instance, I prefer LSP as main provider, then treesitter as first fallback, and then indent if there is neither LSP nor treesitter available as fallback provider.

Right now, defining such a behavior requires a small function with some conditionals, even though for most users, a much simpler config would suffice.

Describe the solution you'd like

Make provider_selector accept more fallbacks, for example:

provider_selector = function() return { "lsp", "treesitter", "indent" } end,
-- or even
provider_selector = { "lsp", "treesitter", "indent" },

Additional context

No response

@chrisgrieser chrisgrieser added the enhancement New feature or request label Oct 23, 2024
@kevinhwang91
Copy link
Owner

Right now, defining such a behavior requires a small function with some conditionals, even though for most users, a much simpler config would suffice.

What's the issue with the function in config?

@chrisgrieser
Copy link
Author

It's unnecessarily complicated. The third person I recommended nvim-ufo too was confused about that particular setting.

Assuming the user wants to use LSP as folding provider, and treesitter as fallback, and then indent as fallback for when there is neither LSP nor treesitter available – which in my view is the best way to configure ufo – their config would have to look like this:

provider_selector = function(_, ft, _)
	local lspWithOutFolding = { "markdown", "zsh", "css", "html", "python", "json" }
	if vim.tbl_contains(lspWithOutFolding, ft) then return { "treesitter", "indent" } end
	return { "lsp", "indent" }
end,

While with my suggestions, their config can simply be:

provider_selector = { "lsp", "treesitter", "indent" },

@kevinhwang91
Copy link
Owner

kevinhwang91 commented Oct 25, 2024

Will make the document of config complicated and make users understand the work mechanism difficulty.

Even though the setup doesn't look compact, the users know how ufo to select the provider. The filetype is mapped to the provider clearly. If the filetype the users don't read frequently, they don't need to configure it, the indent provider is enough.

@kevinhwang91
Copy link
Owner

nvim-ufo/doc/example.lua

Lines 26 to 57 in c96bb3b

-- lsp->treesitter->indent
local function selectProviderWithChainByDefault()
local ftMap = {
vim = 'indent',
python = {'indent'},
git = ''
}
---@param bufnr number
---@return Promise
local function customizeSelector(bufnr)
local function handleFallbackException(err, providerName)
if type(err) == 'string' and err:match('UfoFallbackException') then
return require('ufo').getFolds(bufnr, providerName)
else
return require('promise').reject(err)
end
end
return require('ufo').getFolds(bufnr, 'lsp'):catch(function(err)
return handleFallbackException(err, 'treesitter')
end):catch(function(err)
return handleFallbackException(err, 'indent')
end)
end
require('ufo').setup({
provider_selector = function(bufnr, filetype, buftype)
return ftMap[filetype] or customizeSelector
end
})
end

@chrisgrieser
Copy link
Author

chrisgrieser commented Oct 25, 2024

Will make the document of config complicated and make users understand the work mechanism difficulty.

Well, in my view, it is exactly the opposite: the current method of configuring felt overly complex to the colleagues I helped set up nvim-ufo. But if you view it differently, I guess there is no point in arguing.

Nonetheless, thanks for the maintaining this plugin

@LukasPietzschmann
Copy link

@chrisgrieser I also was looking for the ability to specify multiple fallbacks and after coming across this issue I decided to implement it myself. You can check out my fork if you want to.

But I did not (yet) add the ability to specify the providers as a table instead of a function.


@kevinhwang91, if you change your mind about this feature, I’d be happy to submit a PR here.

And also from my side a big thank you for creating and maintaining this awesome plugin — I really appreciate it!

@kevinhwang91
Copy link
Owner

Should leave this issue open to listen to the sound.

@kevinhwang91 kevinhwang91 reopened this Oct 25, 2024
@mystilleef
Copy link

Chris' solution is simpler and straightforward especially for new users. In my opinion, it should be the default behavior.

I don't think most users want to or should know the inner workings of ufo.

I spent way too much time yesterday trying to setup lsp -> treesitter -> indent to work and still failed. Short on time, I gave up and went with treesitter -> indent since I didn't have to write a function, it just worked, saved me time, and looked sensible to me.

This provider_selector = { "lsp", "treesitter", "indent" } would have saved me an hour yesterday. So in the interest of a good user experience and time, I think Chris makes a very valid point worth considering.

Thanks for this great plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants