ibhagwan / fzf-lua

Improved fzf.vim written in lua
GNU Affero General Public License v3.0
2.15k stars 141 forks source link

[BUG] option passed to `ui_select()` ignored when source is code actions #999

Closed Bekaboo closed 8 months ago

Bekaboo commented 8 months ago

Info

fzf-lua configuration ```lua local fzf = require('fzf-lua') fzf.setup({ winopts = { split = 'botright 12new', }, }) local fzf_ui = require('fzf-lua.providers.ui_select') fzf_ui.register(function(_, items) return { winopts = { split = string.format('botright %dnew', math.min(10, #items + 1)), }, } end) ```

Description

According to #755, the ui_select provider's register() function can accept a callback function to change options dynamically according to the context, I use it to change the fzf window heihgt when different number of candicates are given to vim.ui.select() and it works pretty well:

ibhagwan commented 8 months ago

This isn’t a bug, with code actions the options are inherited from lsp.code_actions: https://github.com/ibhagwan/fzf-lua/blob/faf850e03555512960b5ed1cb1fa50de6f586644/lua/fzf-lua/providers/ui_select.lua#L132-L134

If you wish to override these you need to setup lsp.code_actions: https://github.com/ibhagwan/fzf-lua/blob/faf850e03555512960b5ed1cb1fa50de6f586644/lua/fzf-lua/defaults.lua#L704-L709

Bekaboo commented 8 months ago

Hi thanks for your quick response, it there a way I can make the behavior more consistent i.e. make code actions respect the ui select options?

This isn’t a bug, with code actions the options are inherited from lsp.code_actions

Does it mean that when it comes to code actions fzf does not respect the general options for ui select? I am a bit confused. Why should code actions be different from other cases? Thanks!

ibhagwan commented 8 months ago

Does it mean that when it comes to code actions fzf does not respect the general options for ui select? I am a bit confused. Why should code actions be different from other cases? Thanks!

This goes both ways, most people don’t get as deep as customizing to this level and expect code actions to respect their setup settings.

Bekaboo commented 8 months ago

Here's what I thought: I registered fzf ui_select provider as vim.ui.select(), so any other modules calling this function can use fzf lua as an interface... it works great in all cases except for code actions. I assume nvim's LSP client will call vim.ui.select() in the code actions handler? So the code path should be the same, but why the height configuration is ignored? I am so confused 🤔

Bekaboo commented 8 months ago

I see, thanks. Do you think this can be made more consistent or we should just keep what it is right now?

ibhagwan commented 8 months ago

I see, thanks. Do you think this can be made more consistent or we should just keep what it is right now?

I'll add an option so both use cases can be satisfied

Bekaboo commented 8 months ago

Thanks, this can be closed now.

ibhagwan commented 8 months ago

We can close it once we sort this out properly, note that by doing so you'd lose the code action previewer (which was one of the main reasons I decided on inheriting from defaults).

Bekaboo commented 8 months ago

https://github.com/ibhagwan/fzf-lua/blob/faf850e03555512960b5ed1cb1fa50de6f586644/lua/fzf-lua/providers/ui_select.lua#L142

Will it work if we simply change "keep" to "force" in this line?

If no opts for ui select is passed in -- then good we are using the opts from the setup function, if there is opts for ui select then it takes precedence, in this case the user opt-in to apply the config for ui select so the change should not be breaking.

ibhagwan commented 8 months ago

https://github.com/ibhagwan/fzf-lua/blob/faf850e03555512960b5ed1cb1fa50de6f586644/lua/fzf-lua/providers/ui_select.lua#L142

Will it work if we simply change "keep" to "force" in this line?

If no opts for ui select is passed in -- then good we are using the opts from the setup function, if there is opts for ui select then it takes precedence, in this case the user opt-in to apply the config for ui select so the change should not be breaking.

I’ll do some testing when I’m not AFK and I think this could be a good solution without introducing yet another config option.

ibhagwan commented 8 months ago

@Bekaboo, try https://github.com/ibhagwan/fzf-lua/commit/3d526d24436312cf4e05fbd27452ab2d0ef383cc - it keeps the behavior as is when running :FzfLua lsp_code_actions but overrides the options with force as you suggested **only when using vim.lsp.buf.code_action() directly.

I believe this is a good compromise as users running lsp_code_actions expect their options to be fully respected.

Bekaboo commented 8 months ago

It works pretty well!

image

ibhagwan commented 8 months ago

It works pretty well!

Very nice @Bekaboo! Closing.