fix(run_command): wrong logical condition

[micheleCTDE]

Although not directly exposed, the logical condition in run_command is wrong and makes the second part of the and pointless. Instead or would do the correct evaluation.

[micheleCTDE]

Btw, great plugin! I switched from fzf.vim and fzf-lua has so much more to offer! Love it!!

[ibhagwan]

Btw, great plugin! I switched from fzf.vim and fzf-lua has so much more to offer! Love it!!

Thanks :)

Although not directly exposed, the logical condition in run_command is wrong and makes the second part of the and pointless. Instead or would do the correct evaluation.

Given this condition can’t happen with the current flow I’m curious how did you even find this issue?

[micheleCTDE]

Given this condition can’t happen with the current flow I’m curious how did you even find this issue?

There are some things I would like to implement and contribute in future, so I started from the basic: reading the code and understanding what it does :-)

[ibhagwan]

Given this condition can’t happen with the current flow I’m curious how did you even find this issue?

There are some things I would like to implement and contribute in future, so I started from the basic: reading the code and understanding what it does :-)

Sounds amazing :)

[micheleCTDE]

Two things I want to add are:

1. making the plugin work under Windows too (I use both Linux and Windows)
2. be able to specify specific winopts for lsp_code_actions. While for most lsp providers I use fullscreen, for code actions a small window near the cursor position would be better. And currently it does not seem this is possible, unless I missed something.

But don't get too excited :-) I will need some time since realistically I will only be able to spend 2-3 hours a week on this plugin, give or take.

[ibhagwan]

making the plugin work under Windows too (I use both Linux and Windows)

Oo this is a big one, gl :)

be able to specify specific winopts for lsp_code_actions. While for most lsp providers I use fullscreen, for code actions a small window near the cursor position would be better. And currently it does not seem this is possible, unless I missed something.

Why don’t you specify the winopts in the code actions mapping? As a general rule any option that can be specified via setup can be sent in a direct call, I.e.:

:lua require'fzf-lua'.lsp_code_actions({ winopts = {…} })

The options get merged and override global/provider options so you can do pretty much whatever you want.

[ibhagwan]

As for “relative to the cursor” you can do that too, take a look at #545.

[micheleCTDE]

Why don’t you specify the winopts in the code actions mapping? As a general rule any option that can be specified via setup can be sent in a direct call.

I tried specifying winops for lsp_code_actions in setup but they are not used. If I speficy winopts for lsp, they work, but they are common to all lsp providers. What I need is a different setup for lsp_code_actions from the other lsp providers. Here is an extract from my config:

fzflua.setup
{
  winopts =
  {
    height = 0.60,
    width  = 0.70,
    fullscreen = true,
    ...
  },
  ...
  ...
  lsp_code_actions =
  {
    winopts =
    {
      height = 0.20,
      width  = 0.40,
      row    = 0.50,
      col    = 0.50,
      fullscreen = false,
    },
  }

The code action window still shows up full screen.

As for “relative to the cursor” you can do that too

Yes, I had found and tried that already, but thanks for the hint anyway :-)

[ibhagwan]

they work, but they are common to all lsp providers.

You’re correct, there is currently no separation aside from symbols, can quite easily add another section for code actions but my question is why aren’t you specifying winopts as part of the call which can also be done in the mapping itself? For example mine is mapped to <leader>la and has specific winopts for a smaller size in the mapping, from https://github.com/ibhagwan/nvim-lua/blob/ed859b2c2231a2a7f66dfd9619a1d378425481b3/lua/plugins/fzf-lua/mappings.lua#L123:

map_fzf('n', '<leader>la', "lsp_code_actions", {
  desc = "code actions [LSP]",
  winopts = {
    win_height       = 0.30,
    win_width        = 0.70,
    win_row          = 0.40,
  }})

[micheleCTDE]

I finally got to the bottom of it.
I had tried to put the parameters directly into the lsp_code_actions call, but I had forgotten to add fullscreen = false, which is why it wasn't working since it was inheriting fullscreen = true from the setup script. It works now :-)

Regarding setup, IMO a user would expect all providers to behave in a similar way. Specifying winopts for files works fine, so the same should be expected for other providers too. In case of lsp, it would probably make sense to have a global lsp setup entry for general lsp-related setup, then each individual lsp command would inherit from lsp and could override as needed. What do you think?

[ibhagwan]

Regarding setup, IMO a user would expect all providers to behave in a similar way. Specifying winopts for files works fine, so the same should be expected for other providers too. In case of lsp, it would probably make sense to have a global lsp setup entry for general lsp-related setup, then each individual lsp command would inherit from lsp and could override as needed. What do you think?

I agree, a separate section for lsp.code_actions does make sense as more often than not users will want different options for it, we can add it in a similar manner to the lsp.symbols section, it was never a pressing issue as technically everything can be overridden with the direct call options:

fzf-lua/lua/fzf-lua/config.lua

Lines 508 to 520 in 6203c4f

	M.globals.lsp.symbols = {
	previewer = M._default_previewer_fn,
	prompt_postfix = '> ',
	file_icons = true and M._has_devicons,
	color_icons = true,
	git_icons = false,
	symbol_style = 1,
	symbol_hl_prefix = "CmpItemKind",
	symbol_fmt = function(s) return "["..s.."]" end,
	async_or_timeout = true,
	_actions = function() return M.globals.actions.files end,
	actions = { ["ctrl-g"] = { actions.sym_lsym } },
	}

Feel free to take a stab at it if you want the PR or I’ll do it next time I get a chance.

[micheleCTDE]

Feel free to take a stab at it if you want the PR or I’ll do it next time I get a chance.

If it is not super urgent, I will be happy to work on it and propose a PR in coming weeks. It will be a good way for me to learn more about the plugin code ;-)

[ibhagwan]

Feel free to take a stab at it if you want the PR or I’ll do it next time I get a chance.

If it is not super urgent, I will be happy to work on it and propose a PR in coming weeks. It will be a good way for me to learn more about the plugin code ;-)

560320a - guess you're left with the windows adaptation :-)

[micheleCTDE]

Great, thanks for that! 👍