linux-cultist / venv-selector.nvim

Allows selection of python virtual environment from within neovim
MIT License
378 stars 40 forks source link

Make changing the PYTHON path into a hook as well #110

Closed StefanBRas closed 1 month ago

StefanBRas commented 2 months ago

Hi,

I'm using this plugin, but it's mostly for setting the venv path for pyright. But since it changes the python path itself, it breaks some other parts of my workflow - for example I'll often activate a venv, then open a terminal to run something in another project but this will not use the python binary from the previously selected venv.

As far as I can see, there's no way to avoid this in the current setup. Could setting/changing the python path also be made into a hook? Then I could just choose not to use it in my setup

linux-cultist commented 2 months ago

Yes, this is one of the things I'm going to add to the new version of the plugin I'm working on. The code has been rewritten and is now a lot simpler to extend and configure. If you are interested in the work-in-progress version, you can grab the regexp branch and try it out. Or you can wait for a few weeks until it's completely ready.

linux-cultist commented 1 month ago

Just added a couple of booleans flags for controlling this to the regexp branch. For this use case it seemed simpler to just have options you can set, instead of a callback function.

So if you want to check it out, here is what you can do to try the branch:

Configuration for lazy.nvim:

return {
  "linux-cultist/venv-selector.nvim",
    dependencies = {
      "neovim/nvim-lspconfig", 
      "mfussenegger/nvim-dap", "mfussenegger/nvim-dap-python", --optional
      { "nvim-telescope/telescope.nvim", branch = "0.1.x", dependencies = { "nvim-lua/plenary.nvim" } },
    },
  lazy = false,
  branch = "regexp", -- This is the regexp branch, use this until its merged with the main branch later
  config = function()
      require("venv-selector").setup()
    end,
    keys = {
      { ",v", "<cmd>VenvSelect<cr>" },
    },
},

Plugin config with your options (read README for regexp branch to find out how you add your own searches if the default ones are not enough).

      require("venv-selector").setup {
        settings = {
          options = {
           activate_venv_in_terminal = false,   -- activate the selected python interpreter in terminal windows opened from neovim
           set_environment_variables = false,   -- sets VIRTUAL_ENV or CONDA_PREFIX environment variables
          },
        },
      }
linux-cultist commented 1 month ago

The regexp branch shouldnt modify your path or set any environment variables if you use the options above. Im closing the issue and hope you try it and let me know if it works. :)

StefanBRas commented 1 month ago

Hey again! Sorry, I didn't get around to testing it until now.

I initially had some problems that I can't recreate, so maybe it was a botched installation. The activate_venv_in_terminal and set_environment_variables seems to work great, however some other things kinda broke for me:

My lazy config looks like this (on Mac):

  {
    "linux-cultist/venv-selector.nvim",
    dependencies = {
      "neovim/nvim-lspconfig",
      { "nvim-telescope/telescope.nvim", branch = "0.1.x", dependencies = { "nvim-lua/plenary.nvim" } },
    },
    lazy = false,
    branch = "regexp", -- This is the regexp branch, use this until its merged with the main branch later
    config = function()
      require("venv-selector").setup({
        settings = {
          options = {
            debug = true, -- enables you to run the VenvSelectLog command to view debug logs
            fd_binary_name = "fd", -- plugin looks for `fd` or `fdfind` but you can set something else here
            enable_default_searches = true, -- switches all default searches on/off
            activate_venv_in_terminal = false, -- activate the selected python interpreter in terminal windows opened from neovim
            set_environment_variables = false, -- sets VIRTUAL_ENV or CONDA_PREFIX environment variables
            show_telescope_search_type = true, -- shows the name of the search in telescope
            notify_user_on_venv_activation = true, -- notifies user on activation of the virtual env
            search_timeout = 5, -- if a search takes longer than this many seconds, stop it and alert the user
          },
        },
      })
    end,
  },

By setting notify_user_on_venv_activation I can just ignore that it seems to activate it multiple times and with this on, I prefer this branch to main.

Anyway, thanks for doing it! Let me know if you need more debug information or want me to run something

linux-cultist commented 1 month ago

Hi,

Thanks for testing the new version. :)

Lets start with the speed issue since i think thats the easiest to fix. By default it tries to search all hidden files in your current working directory but the good news is, you can change how it searches.

One example: If you know you dont have hidden venvs in your working directory, you should disable looking for hidden files in the search.

The default cwd search (cwd = current working directory) looks like this (taken from source code here:

cwd = {
    command = "$FD '/bin/python$' $CWD --full-path --color never -E /proc -HI -a -L",
},

if you want to not search hidden files, you can just remove the -H from the command and write your own CWD search:

settings = {
  search = {
    cwd = {
      command = "$FD '/bin/python$' $CWD --full-path --color never -E /proc -I -a -L",
    },
  },
}

Or if you know your venvs are in some location, you can write your own search for that directory and disable the default cwd search:

settings = {
  search = {
    cwd = false, -- setting this to false disables the default cwd search
    my_search = {
      command = "fd /bin/python$ ~/Code --full-path -I -a -L" -- read up on the fd flags so it searches what you need
    }
  },
}

Its very flexible now and it should be lighting fast if you avoid searching paths you dont need.

There is also a default timeout of 5 seconds because its not supposed to take that long. If it does, you should write your own searches for your own usecase. :)

And it matters where you are when you start neovim. If you are in your $HOME, it will have to search every file in your home directory (the cwd search) plus all hidden files (by default). So its best to have some project directory with code and open neovim in that one. Try to go into some folder with your code and open neovim there and run VenvSelect. Should be instant.

linux-cultist commented 1 month ago

Its supposed to activate the last venv that was used in that working directory. If you have different directories for projects, it should keep track of which venv you opened in which working directory. Let me know if it doesnt do this because then its a bug. I will also test on my own machine and see if I can spot bugs here.

I will add this option, good idea.

I agree this is annoying. There is an option for this that is default true, but I will change it to false now since I agree with you.

options.notify_user_on_venv_activation = true

linux-cultist commented 1 month ago

Added option enable_cached_venvs that you can set to false to not automatically set a venv.

linux-cultist commented 1 month ago

Also made the default cwd search skip some well known large directories to speed things up:

StefanBRas commented 1 month ago

I'm working in a fairly large monorepo, which is probably why it was so slow. I updated to the newest commit and the speed isn't an issue anymore - it takes around 1s. I still think it feels more smooth if the telescope picker is rendered instantly and then filled with results as they come in (and my impression is the telescope is built for that?) but it's not an issue for me anymore. Thanks!

However with the default settings I now get errors if I don't have the expected directories:

2024-05-22 07:01:11 [ERROR]: Error: [fd error]: Search path '/Users/sras/.conda/envs' is not a directory.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: No valid search paths given.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: Search path '/Users/sras/.pyenv/versions' is not a directory.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: No valid search paths given.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: Search path '/Users/sras/Library/Application/Support/hatch/env/virtual' is not a directory.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: No valid search paths given.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: Search path '/Users/sras/.virtualenvs' is not a directory.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: No valid search paths given.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: Search path '/Users/sras/.local/share/pipx/venvs' is not a directory.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: No valid search paths given.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: Search path '/opt/anaconda/bin' is not a directory.
2024-05-22 07:01:11 [ERROR]: Error: [fd error]: No valid search paths given.

I can just disable those searches, but it should probably hide those errors by default.

Also I have pipx installed, but it's located at ~/.local/pipx/venvs not ~/.local/share/pipx/venvs.

enable_cached_venvs works perfectly and also seems stop the multiple notifications whenever I open a new file.

StefanBRas commented 1 month ago

Also a minor thing: If you disable all searches you get nothing the first time you run, and then next times you will get:

2024-05-22 07:25:36 [INFO]: Aborting search - previous search still running.

It's not a big issue, but it happened to me because I had disabled default searches and then manually set the ones I needed. However it looks like it's disabled by name, so when I made a new search called workspace it was also disabled.

I found out that you could just set the default searches you didn't want to false, so it's not a problem for me anymore

linux-cultist commented 1 month ago

Very good feedback! The errors are supposed to be shown in the debug log only but I made some refactorings to log errors both to the debug log and print them to the user. This is not what we want for this kind of error, so I will fix that tonight. Thank you!

And I can look into making telescope fill with results as they come in, I agree it's a better experience. :)

And very good info about default search message, will look into that too.

Thanks again and keep giving feedback :)

StefanBRas commented 1 month ago

Great, and thanks for the quick response. If other things comes up, should I rather make new issues for it?

linux-cultist commented 1 month ago

Yes please! I'm keeping track of this thread but it's always easier to have new issues to work on there on the issues tab. Hope you don't find anymore though :)

linux-cultist commented 1 month ago

OK, some of your bugs should be fixed now:

Im waiting for the weekend to work on the realtime telescope update since its a bit more time consuming. :)

linux-cultist commented 1 month ago

If you update the plugin now, you should notice how telescope opens right away and how results come in as they are found. Thanks a lot for noticing and commenting on this, since its a much better experience for the user. Its also how the old version works, and its something it did better. But not anymore!

Try it and let me know if it feels intuitive or if something further can be improved. Thank you!

StefanBRas commented 1 month ago

Just tried it out, works great. Also nice with the indication of the currently active venv in the lsit

Some feedback:

linux-cultist commented 1 month ago

Thanks for the feedback, always appriciate it :)

I was trying to change the title of the window while it was searching, but the telescope picker doesn't take any options in that situation where it just refreshes the view. But will see if there is another way. It would be quite nice.

And sure, it would be fairly easy to just let users add directories they want to search and exclude in a list in options. You are right, this is probably good enough for many users. I will think about some way of implementing this.

And since I'm sorting the results anyway, I could probably include some intelligence there and take into account which file and directory is currently opened and put relevant venv paths at the top. Again a nice idea, will play a bit with the it.

I will later create some proper issues from from this as feature requests.

Thanks!