gfanto / fzf-lsp.nvim

Enable the power of fzf fuzzy search for the neovim built in lsp
MIT License
223 stars 11 forks source link

Configureable renderer per handler #31

Open mhanberg opened 2 years ago

mhanberg commented 2 years ago

Description

To allow the user to have the best experience for them, we should allow them to configure the function that is called to generate the text that is fed into fzf.

Proposal

I propose that the function has the interface.

function(item, utils), with utils being a table that has several utilities, the ansicolors utility and the strings module from plenary. This will give them the tools to align and color the output themselves.

The return value is a string with the line they want to show up in the fzf window.

The result of this function will always have the path:line:[col] appended to it, so that we can always use it and not have to have the user tell us which part of the string has the file path.

Each handler will have it's own configurable renderer.

I'm not sure yet where we would set this configuration yet. Most lua plugins have a require("myplugin").setup({}) function that takes configuration, while currently fzf-lsp.nvim uses global variables. I'm open to feedback here.

Let me know what you think, thanks!

aljoscha commented 2 years ago

Thanks for the quick reaction to this one! I don't have opinions on how this should be implemented.

gfanto commented 2 years ago

I like the idea of having a configurable handler but I'm not sure if giving the user so much freedom since each item have to be first processed to make the line showed in the fzf window and then this line processed to have back the file and line number.

The result of this function will always have the path:line:[col] appended to it, so that we can always use it and not have to have the user tell us which part of the string has the file path.

Making the user always responsible of adding the path:line:[col] to the line could be an option, but since it's just regex it can cause problems, I'm thinking for example of a python line where you can have colons and confuse a definition with a file path, just thinking out loud now. Much of the headaches can be probably solved by using your "\x01" trick, so maybe we can add the path:line:[col] ourself, let me know if you see some problems about that. Another possible way to let the user make every line he wants could be to have a way to distinguish each line and use a map to have a file <=> line mapping.

Suppose you have each line starting with a number:

1. function foo(x int) string
2. function bar(x string) int

we can simply strip the number and pass to the sink function called by fzf a table like:

{
    "1" = "foo/bar/baz.go:32"
    "2" = "foo/bar/baz.go:44"
}

P.S.: i don't really like that much adding a number before each line

Each handler will have it's own configurable renderer.

Cool 👍

I'm not sure yet where we would set this configuration yet. Most lua plugins have a require("myplugin").setup({}) function that takes configuration, while currently fzf-lsp.nvim uses global variables. I'm open to feedback here.

Yes there is already a config function which at the moment is only used to configure asynchronous handlers, I created this micro plugin when native lsp was in its early stage, so I just used vim options because every config at the moment it was written in vim script and my goal was just to wait for someone better than me to create a better plugin like Telescope, for example, unfortunately this has bothered me ever since.

Long story short i think to keep having the vim options to keep backward compatibility, but installing the handlers in the setup function.

mhanberg commented 2 years ago

Thanks for the feedback, I think we're mostly on the same page but I have one clarification.

Making the user always responsible of adding the path:line:[col]

I meant to say that the plug-in will always append this value. The user will not be responsible.