Feat/add option to use fzf

[bosvik]

Refactored picker and added fzf-lua picker
Optionally use fzf-lua or telescope

@GustavEikaas apologies, it is really quick and dirty and not working for fzf-lua just yet but would like to get some feedback on the principle

[GustavEikaas]

Awesome!❤
I dabbled a bit with the same yesterday and I got everything except :Dotnet secrets to work. It was kinda tricky with the previewer.

I noticed it shows a preview on most commands, which it shouldnt do. But you're definitely on to something!

[bosvik]

sorry had no time to work on this. changed job and fully occupied wrapping my head around that

[GustavEikaas]

Dont worry about it. Always a lot to take in when changing jobs, good luck!

[bosvik]

picking up this again. do you mind sharing your on_package_selected for nuget fzf? hate to do extra work if not needed 😂

[bosvik]

should be in the config function? picker = 'telescope' -- 'fzf', 'mini.picker', 'snacks.picker' ?

[GustavEikaas]

• Do we want to support mini.picker and snacks.picker? I havent used either, are they easy to implement?
• Maybe just do fzf in this pr and then mini.picker and snacks.picker in separate PRs?

on_package_selected for nuget fzf, this one?

---@param cb function
local function fzf_nuget_search(cb)
  fzf.fzf_live('dotnet package search <query> --format json | jq ".searchResult | .[] | .packages | .[] | .id"', {
    fn_transform = function(line) return line:gsub('"', ""):gsub("\r", ""):gsub("\n", "") end,
    actions = {
      ["default"] = function(selected)
        local package = selected[1]
        cb(package)
      end,
    },
  })
end

[bosvik]

• Do we want to support mini.picker and snacks.picker? I havent used either, are they easy to implement?
• Maybe just do fzf in this pr and then mini.picker and snacks.picker in separate PRs?

on_package_selected for nuget fzf, this one?

---@param cb function
local function fzf_nuget_search(cb)
  fzf.fzf_live('dotnet package search <query> --format json | jq ".searchResult | .[] | .packages | .[] | .id"', {
    fn_transform = function(line) return line:gsub('"', ""):gsub("\r", ""):gsub("\n", "") end,
    actions = {
      ["default"] = function(selected)
        local package = selected[1]
        cb(package)
      end,
    },
  })
end

Yeah let's start with fzf and think about others later.
No, i was thinking about the callback function

[GustavEikaas]

pretty sure the callback is this from the nuget.lua file add_package(package, project_path)
Long time cant really remember

[GustavEikaas]

Ah my bad I see, the on_package_selected should have an fzf-lua implementation for selecting the version. Sorry I do not have. Could probably write one quickly if you need it?

[bosvik]

i reused the add_package and seems to work just fine but did not test against secrets as i didn't configure it.

[bosvik]

i'm still thinking maybe it would make sense to select picker in the config? right now it is hit and miss in the if else clause in pickers/init.lua

[GustavEikaas]

Oh yeah definitely add it to options

[GustavEikaas]

Maybe something like

picker = "telescope" | "fzf" | nil

where nil picks whatever is installed or throws an error if neither is found?

[bosvik]

Yes that is exactly in line with what I thought of. I'll look into it as soon as I can get some time to get the secret previewer working. The picker works but previewer is just giving nil right now.

[GustavEikaas]

It's also worth considering how this will be to maintain. Say a fresh contributor makes a PR implementing some new feature. Would they make fzf and telescope pickers. Should it be co-located in some way to make it easier to update both at the same time. They kinda have to stay in sync

[bosvik]

It's also worth considering how this will be to maintain. Say a fresh contributor makes a PR implementing some new feature. Would they make fzf and telescope pickers. Should it be co-located in some way to make it easier to update both at the same time. They kinda have to stay in sync

How do you feel about removing the need to use telescope/fzf-lua altogether. make it optional and use vim.ui.select for the things that don't need a preview and only optionally use fzf-lua or telescope?

[GustavEikaas]

The thought has crossed my mind before. My main reasons for not doing it is

• Not requested by anyone yet
• My general perception is that most people either use fzf or telescope
• It takes some work to implement but also puts a cost on future implementations relying on pickers

Im not against it, Im just not convinced its worth spending time on, what do you think?

[bosvik]

The picker and pick_sync is pretty straight forward to implement. For the migration and preview_pickers it is not possible to get a preview so it may not be needed. I can push an update to this PR where all pickers are implemented for telescope, fzf and plain vim.ui.select. Take a look at it and try it out? it does need some cleaning up though lol.

[GustavEikaas]

Sure

[bosvik]

I was thinking maybe just use vim.ui.select for the simple pickers that don't need preview, picker and pick_sync. No need to add extra overhead by extra implementation of those. For secrets and migrations we implement optional fzf/telescope? For nuget fzf works best among the three. Well try them all three and I’ll adjust according to your input

[GustavEikaas]

Just tested this, amazing work!
Functionality is 100%, there is something missing on documentation/setup.
I like that telescope is default chosen in options because it means existing users will never notice this change. We should add some info to the readme. Also not sure about the picker == nil scenario. If the default is telescope and telescope is mentioned in the installation section then picker = nil is pretty much picker = "vim.ui.select". IMO the pickers nullability is just confusing as it is.

We could do default options.picker = nil, and do pcall require around telescope, then fzf and if none of them is installed then fallback to vim.ui.select. Or we could just remove picker nullability and change it to picker = "telescope" | "fzf" | "vim.ui.select"

Thoughts? Other than that AWESOME WORK, this is an amazing PR!

[bosvik]

Glad you liked it 😁. Yeah need to add something to the docs too but we need to decide how to address the picker selection first. Now I am starting to lean towards removing it from the options again and just do

local has_telescope = pcall(require, ”telescope”)
local has_fzf = pcall(require, ”fzf-lua”)

if has_telescope then
   …
elseif has_fzf then
   …
else use_base
end

This way it does not effect existing users and it will fallback to using vim.ui.select for users not using either. We should put telescope and fzf as optional dependencies and remove telescope in the dependencies section in the examples?
I also realized we introduce a dependency on jq in the case of fzf (nuget picker). Should document that too.

[GustavEikaas]

One thing we should take into account is users who have telescope because it is required by a plugin and fzf because they prefer it. I think it belongs in the options.

As for removing the dependency on telescope, I dont think its a good idea IMO. telescope or FZF will give you a way better experience than vim.ui.select. So for the "superusers" its nice to support vim.ui.select but for people who are just configuring the plugin with no options I would like telescope to be default. We can update the readme to reflect that telescope is an optional plugin but we should still keep it in the dep array in the example

Yeah JQ has been used internally for some functions for a while, kinda just forgot to add it to readme. But it should be there

[bosvik]

Alright! I'll make the adjustments as soon as I get back home.

[bosvik]

Please have a look at it. I added a notification if picker is misconfigured, I hope you agree it makes sense.