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.

Sign up for GitHub

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

Poetry support #35

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Poetry support #35

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
db2b40d
add local .venv dirs support
palandovalex Mar 4, 2024
6063304
debug
palandovalex Mar 4, 2024
28f0a8f
fix os relation
palandovalex Mar 4, 2024
e689014
fixes
palandovalex Mar 5, 2024
6f83647
local commit
palandovalex Mar 5, 2024
b907757
progress
palandovalex Mar 5, 2024
90a2ccc
clear from other changes
palandovalex Mar 5, 2024
c47ca68
clear changes
palandovalex Mar 5, 2024
4ad1592
clear changes
palandovalex Mar 5, 2024
b2f822b
ready for pull request
palandovalex Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions lua/swenv/api.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,24 @@ local get_micromamba_base_path = function()
end
end

local get_poetry_venvs_base_path = function()
local which_poetry = io.popen('which poetry')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm starting to get a bit worried about performance when all these functions are called and some call out to external executables. I don't use it but some users use the auto_venv which calls this on FileType. I wonder if this should be done async. This is somewhat independent from this PR but something I thought of now.

I also wonder, the current implementation supports the venvs_path setting. Would it be an option to support that being a function and instead supply this as a optional utility function which a user can use instead of the default venvs_path?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whell, i will replace sys calls this with parametrs in plugin config.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that poetry support does not require any code mutation. I can just use venvs_path.
But I'm still thinking. which is useful to replace venvs_path with venvs_paths, with one, two or many directories.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also i can set use "has_conda" 'has_pixi', 'has_micromama' param. If it set to false - functions will not be called.
Like this:

  if settings.spec_envs == nil then
    return venvs
  end
  if 'conda' in setting.spec_envs then
    vim.list_extend(venvs, get_venvs_for(get_conda_base_path(), 'conda'))
    vim.list_extend(venvs, get_conda_base_env())
  end

This will speed up the plugin a bit, but... at the cost of backward compatibility.
It may be worth making a separate branch for such a change. For example, 'nightly_speedup'.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the meantime, I agree - everything except the usual venus should be handled asynchronously.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I'm still thinking. which is useful to replace venvs_path with venvs_paths, with one, two or many directories.

Yeah I think that would be useful, maybe just support the setting to be string, table or function returning string or table? This could then be backwards compatible.

Also i can set use "has_conda" 'has_pixi', 'has_micromama' param. If it set to false - functions will not be called.
Like this:

I think that's a good idea, maybe it can default to true for now to be backwards compatible and then at least it can be disabled if speed is an issue?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #36 for the async part

which_poetry = which_poetry:read('*all')
if which_poetry == '' then
return nil
end
local p_file = io.popen('poetry config virtualenvs.path')

local poetry_root_prefix = p_file:read('*all')
poetry_root_prefix = string.gsub(poetry_root_prefix, "%s+", "")
if poetry_root_prefix == '' then
return nil
else
local base_path = Path:new(poetry_root_prefix) .. ''
return base_path
end
end

local get_pyenv_base_path = function()
local pyenv_root = vim.fn.getenv('PYENV_ROOT')
if pyenv_root == vim.NIL then
Expand All @@ -157,6 +175,7 @@ M.get_venvs = function(venvs_path)
vim.list_extend(venvs, get_venvs_for(get_micromamba_base_path(), 'micromamba'))
vim.list_extend(venvs, get_venvs_for(get_pyenv_base_path(), 'pyenv'))
vim.list_extend(venvs, get_venvs_for(get_pyenv_base_path(), 'pyenv', { only_dirs = false }))
vim.list_extend(venvs, get_venvs_for(get_poetry_venvs_base_path(), 'poetry'))
return venvs
end

Expand Down
Loading