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

feat: native (vim.ui.select) and fzf-lua pickers #188

Open
wants to merge 9 commits into
base: regexp
Choose a base branch
from

Conversation

stefanboca
Copy link

Closes #142

This PR implements a new native picker (using vim.ui.select) and fzf-lua picker. The picker can be manually selected via the picker config option, or by default is automatically selected based on what is installed (by setting picker = "auto"), with a fallback on the native picker if neither telescope or fzf-lua are available.

Some notes:

  • the native picker only shows results once the search is done, and without color. AFAIK these are both limitations of the api.
  • the fzf-lua picker only sorts and deduplicates results once the search is done. This might result in some flickering, but I'm not sure if there's an elegant solution to sorting/dedup wile the search is live. This isn't really an issue for quick searches because only the final results are displayed.
  • IMO the following config options should be renamed, because they apply to all pickers and not just telescope: show_telescope_search_type, on_telescope_result_callback, and telescope_active_venv_color

I'm also open to changing the format for results. Currently, the native picker looks like this:
image
and the fzf picker looks like this:
image
(with show_telescope_search_type enabled)

Please feel free to make suggestions and let me know what you think!

cc @akthe-at

file

This is to make it easier to add new pickers
Additionally, the entrypoint of search has changed from `search.New` to
`gui.open`, because `run_search` is now passed a picker object
...and allow automatically detecting installed pickers. When no
supported pickers are found, fall back to "native" picker, which uses
`vim.ui.select` (not yet implemented).
Unfortunately, there is no refresh functionality because afaik
`vim.ui.select` doesn't support it.
lua/venv-selector/gui.lua Outdated Show resolved Hide resolved
@akthe-at
Copy link

akthe-at commented Jan 4, 2025

I installed telescope.nvim to test this PR out so the problem could be in my setup but when trying to search with picker = "telescope" and using the ctrl+r search refresh, I was hit with this error msg:

 Error  05:09:34 msg_show.emsg E5108: Error executing lua: ...s/venv-selector.nvim/lua/venv-selector/gui/telescope.lua:44: attempt to call field 'run_search' (a nil value)
stack traceback:
	...s/venv-selector.nvim/lua/venv-selector/gui/telescope.lua:44: in function 'key_func'
	...hare/nvim/lazy/telescope.nvim/lua/telescope/mappings.lua:293: in function <...hare/nvim/lazy/telescope.nvim/lua/telescope/mappings.lua:292>

@akthe-at
Copy link

akthe-at commented Jan 4, 2025

When using picker = "fzf-lua", if I activate a virtual env with venv-selector, it works just great...if I try to activate the same venv again that is currently activated ( now green in the picker) I get hit with this error:

Info  05:13:55 notify.info VenvSelect Registered '/home/aktheat/projects/fe_str/.venv/bin/python' with basedpyright LSP.
   Error  05:14:01 notify.error [Fzf-lua] fn_selected threw an error: ...jects/venv-selector.nvim/lua/venv-selector/gui/utils.lua:134: attempt to index local 'entry' (a nil value)
stack traceback:
	...heat/.local/share/nvim/lazy/fzf-lua/lua/fzf-lua/core.lua:242: in function '__index'
	...jects/venv-selector.nvim/lua/venv-selector/gui/utils.lua:134: in function 'select'
	...cts/venv-selector.nvim/lua/venv-selector/gui/fzf-lua.lua:24: in function 'action'
	...t/.local/share/nvim/lazy/fzf-lua/lua/fzf-lua/actions.lua:117: in function 'act'
	...heat/.local/share/nvim/lazy/fzf-lua/lua/fzf-lua/core.lua:148: in function 'fn_selected'
	...heat/.local/share/nvim/lazy/fzf-lua/lua/fzf-lua/core.lua:235: in function <...heat/.local/share/nvim/lazy/fzf-lua/lua/fzf-lua/core.lua:234>
	[C]: in function 'xpcall'
	...heat/.local/share/nvim/lazy/fzf-lua/lua/fzf-lua/core.lua:234: in function <...heat/.local/share/nvim/lazy/fzf-lua/lua/fzf-lua/core.lua:226>

This does not occur if you select a different venv.

Edt: This also does not happen if you are using the native picker

@akthe-at
Copy link

akthe-at commented Jan 4, 2025

Nice job @stefanboca ...This is way better implemented then I was ever able to come close to. I think you've done some great stuff here. I dug in quickly and found a few items that might need to be fixed up/polished up. I will keep using this PR as my daily driver and see if I can find anything else. I did not review the actual code itself yet!

@stefanboca stefanboca force-pushed the sb/push-rlpxsqmllxtz branch from 7b9e449 to a4f5489 Compare January 5, 2025 00:37
@stefanboca
Copy link
Author

@akthe-at I believe I've fixed the issues you've mentioned. Could you verify?

@stefanboca stefanboca force-pushed the sb/push-rlpxsqmllxtz branch from c50d437 to fde607d Compare January 5, 2025 01:02
fzf strips ansi color codes from the output, so the output returned by
fzf and the key in `self.entries` were different.
@stefanboca stefanboca force-pushed the sb/push-rlpxsqmllxtz branch from fde607d to e7b8c42 Compare January 5, 2025 01:03
@akthe-at
Copy link

akthe-at commented Jan 5, 2025

I can confirm my issues were resolved with the latest commits, 100% fire. Nice Job!

@stefanboca
Copy link
Author

That's great to hear! Please let me know if you run into any more issues.

@linux-cultist
Copy link
Owner

Looks like some great progress! I will try the code also as daily driver for a few days. :)

@mehalter
Copy link
Contributor

This is awesome! Looking forward to this addition! It might also be worth adding official snacks.picker support as well now that that is out and growing in popularity. Currently just using the native picker works well with it but it would be cool to get pretty formatting

@lauritzv
Copy link

lauritzv commented Feb 6, 2025

Hi! This is great!
Only issue I've run into is getting frequent notifications "VenvSelect: Registered "some file path.py" with pyright LSP" i.e whenever I open a python file buffer. This is only when having the option notify_user_on_venv_activation = true.

I'm running LazyVim with fzf-lua, without telescope and my setup is copied from lazyvim extras python lang setup with the exception of the venv-selection portion:

  {
    "stefanboca/venv-selector.nvim",
    branch = "sb/push-rlpxsqmllxtz",
    cmd = "VenvSelect",
    opts = {
      settings = {
        options = {
          notify_user_on_venv_activation = true,
        }, ......

@neolooong
Copy link
Contributor

getting frequent notifications "VenvSelect: Registered "some file path.py" with pyright LSP" i.e whenever I open a python file buffer. This is only when having the option notify_user_on_venv_activation = true.

I think this is related to the #187.

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

Successfully merging this pull request may close these issues.

6 participants