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

Automatically detect if pipenv should be used for linters #1656

Closed
w0rp opened this issue Jun 15, 2018 · 9 comments
Closed

Automatically detect if pipenv should be used for linters #1656

w0rp opened this issue Jun 15, 2018 · 9 comments

Comments

@w0rp
Copy link
Member

w0rp commented Jun 15, 2018

First mentioned in a comment for #1629

pipenv --venv will print information that can be used to check if a file is instead of a pipenv managed project or not. If a file is in such a project, ALE could automatically load executables from pipenv projects instead. This pipenv detection ought to be configurable, and probably off by default, as not everyone will be using pipenv.

elebow added a commit to elebow/ale that referenced this issue Jul 13, 2018
…se-analysis#1656)

When set to true, and the buffer is currently inside a pipenv,
GetExecutable will return "pipenv", which will trigger the existing
functionality to append the correct pipenv arguments to run each linter.

Defaults to false.

I was going to implement ale#python#PipenvPresent by invoking
`pipenv --venv` or `pipenv --where`, but it seemed to be abominably
slow, even to the point where the test suite wasn't even finishing
("Tried to run tests 3 times"). The diff is:

diff --git a/autoload/ale/python.vim b/autoload/ale/python.vim
index 7baae079..8c100d41 100644
--- a/autoload/ale/python.vim
+++ b/autoload/ale/python.vim
@@ -106,5 +106,9 @@ endfunction

" Detects whether a pipenv environment is present.
function! ale#python#PipenvPresent(buffer) abort
-    return findfile('Pipfile.lock', expand('#' . a:buffer . ':p:h') . ';') isnot# ''
+    let l:cd_string = ale#path#BufferCdString(a:buffer)
+    let l:output = systemlist(l:cd_string . 'pipenv --where')[0]
+    " `pipenv --where` returns the path to the dir containing the Pipfile
+    " if in a pipenv, or some error text otherwise.
+    return strpart(l:output, 0, 18) !=# "No Pipfile present"
 endfunction

Using vim's `findfile` is much faster, behaves correctly in the majority
of situations, and also works reliably when the `pipenv` command doesn't
exist.
@klardotsh
Copy link

klardotsh commented Aug 21, 2018

@elebow Any hopes of you opening a PR of that branch to upstream here?

Additionally, it'd be nice to support poetry in a similar way (running literally works the exact same way - poetry run flake8 or whatever). The goofiness is that poetry is hyper-aggressive in creating venvs for you - as far as I can tell, the safest way to check for poetry existing is to stat pyproject.lock. I've opened a feature request in that repo to address this, python-poetry/poetry#401

@klardotsh
Copy link

Well, I see @elebow's commit actually checks for a lockfile the same way I considered doing for poetry, so maybe that's a sane route to go down (and has fewer upstream dependencies...)

elebow added a commit to elebow/ale that referenced this issue Sep 16, 2018
…se-analysis#1656)

When set to true, and the buffer is currently inside a pipenv,
GetExecutable will return "pipenv", which will trigger the existing
functionality to append the correct pipenv arguments to run each linter.

Defaults to false.

I was going to implement ale#python#PipenvPresent by invoking
`pipenv --venv` or `pipenv --where`, but it seemed to be abominably
slow, even to the point where the test suite wasn't even finishing
("Tried to run tests 3 times"). The diff is:

diff --git a/autoload/ale/python.vim b/autoload/ale/python.vim
index 7baae079..8c100d41 100644
--- a/autoload/ale/python.vim
+++ b/autoload/ale/python.vim
@@ -106,5 +106,9 @@ endfunction

" Detects whether a pipenv environment is present.
function! ale#python#PipenvPresent(buffer) abort
-    return findfile('Pipfile.lock', expand('#' . a:buffer . ':p:h') . ';') isnot# ''
+    let l:cd_string = ale#path#BufferCdString(a:buffer)
+    let l:output = systemlist(l:cd_string . 'pipenv --where')[0]
+    " `pipenv --where` returns the path to the dir containing the Pipfile
+    " if in a pipenv, or some error text otherwise.
+    return strpart(l:output, 0, 18) !=# "No Pipfile present"
 endfunction

Using vim's `findfile` is much faster, behaves correctly in the majority
of situations, and also works reliably when the `pipenv` command doesn't
exist.
@w0rp w0rp closed this as completed in 56e67c5 Sep 19, 2018
w0rp added a commit that referenced this issue Sep 19, 2018
…nters

Add python_[linter]_auto_pipenv options for python linters (fixes #1656)
@w0rp
Copy link
Member Author

w0rp commented Sep 19, 2018

Thanks to @elebow, you can now use pipenv for the Python linters if you set g:ale_python_auto_pipenv to 1, or any of the various options for each specific linter.

@cmcaine
Copy link

cmcaine commented Sep 20, 2018

You haven't set a default value for this variable and vim is giving:

E716: Key not present in Dictionary: ale_python_auto_pipenv

@elebow
Copy link
Member

elebow commented Sep 20, 2018

I thought that was covered by 169a6e2#diff-a5941a5354db7e7a717f0be61ffc56bdR4, but I see that you are right.

It seems autoload/ale/python.vim is being loaded after the ale_linters/python/* files. But setting the variable in autoload/ale.vim results in the correct behavior. I'm having trouble determining the load order of files because an echom in ale_linters/python/* doesn't seem to have any effect, but it appears that the load order is (in relevant part):

  1. autoload/ale.vim
  2. ale_linters/python/*
  3. autoload/ale/python.vim

I'm not sure whether this is intentional, but I would suggest that autoload/ale/python.vim ought to be loaded before the individual linter files if possible.

If that's not possible, then I suppose the obvious fix is to set the variable in autoload/ale.vim or some other file that is loaded early. Or just at the top of each individual linter file, though it's an inelegant solution.

@w0rp
Copy link
Member Author

w0rp commented Sep 20, 2018

See #1938. I'll fix it.

aliou pushed a commit to aliou/ale that referenced this issue Oct 6, 2018
…se-analysis#1656)

When set to true, and the buffer is currently inside a pipenv,
GetExecutable will return "pipenv", which will trigger the existing
functionality to append the correct pipenv arguments to run each linter.

Defaults to false.

I was going to implement ale#python#PipenvPresent by invoking
`pipenv --venv` or `pipenv --where`, but it seemed to be abominably
slow, even to the point where the test suite wasn't even finishing
("Tried to run tests 3 times"). The diff is:

diff --git a/autoload/ale/python.vim b/autoload/ale/python.vim
index 7baae079..8c100d41 100644
--- a/autoload/ale/python.vim
+++ b/autoload/ale/python.vim
@@ -106,5 +106,9 @@ endfunction

" Detects whether a pipenv environment is present.
function! ale#python#PipenvPresent(buffer) abort
-    return findfile('Pipfile.lock', expand('#' . a:buffer . ':p:h') . ';') isnot# ''
+    let l:cd_string = ale#path#BufferCdString(a:buffer)
+    let l:output = systemlist(l:cd_string . 'pipenv --where')[0]
+    " `pipenv --where` returns the path to the dir containing the Pipfile
+    " if in a pipenv, or some error text otherwise.
+    return strpart(l:output, 0, 18) !=# "No Pipfile present"
 endfunction

Using vim's `findfile` is much faster, behaves correctly in the majority
of situations, and also works reliably when the `pipenv` command doesn't
exist.
@kctong529
Copy link

Adding the line let g:ale_python_auto_pipenv = 1 and let pipenv_venv_path = system('pipenv --venv') to ~/.config/nvim/init.vim does not make any difference for me. When I open a .py file in nvim, ALE shows [pylint] Unable to import ....

$ pipenv run python and $ python are showing different import message, so I believe it's not a problem caused by pipenv itself.

@w0rp
Copy link
Member Author

w0rp commented Oct 15, 2018

Open a new issue with the output of :ALEInfo, so others can look at the output of that.

@LeonardMH
Copy link

@kctong529 I was also having this issue, make sure you have an applicable linter installed inside your pipenv virtual environment, it won't see your globally installed linter.

I have and pylint installed globally but it was not installed in my virtualenv, and no other linters we available, :ALEInfo helped me narrow this down to pipenv choking on one of the linters (mypy I think). So for example I fixed this with pipenv install --dev pylint.

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

No branches or pull requests

6 participants