Shell completion on Bash for Windows

[ociule]

Currently, shell completion is not supported on Bash for Windows.

If we try pipenv --completion we get a message saying "bash.exe is not supported"

I've patched pipenv/vendor/click_completion.py to support Bash completion.

All that was needed was enabling the generation of the bash script on bash.exe. The generated script already worked.

[uranusjr]

Hi, thanks for the patch! The vendor directory is for “vendoring” modules from other projects so we don’t require too many packages in site-packages. We prefer to submit patches to corresponding repositories if possible, and update the vendored modules after the upstream project accepts the patch. For this particular patch, you would need to submit the pull request to click-contrib/click-completion.

Thanks again for the contribution. I look forward to see this propagated into Pipenv!

[ociule]

Ok, I'll send the patch upstream to click-completion. I noticed there's a big version difference between the vendorized click-completion (0.21) and the latest release (0.31). Hopefully that won't be an obstacle to pulling the updated click-completion.

Meanwhile, any pipenv bash on windows user can use use my patch on their local installation.

[ociule]

There's also a simple bypass that you can use where you call pipenv --completion, so in your .bash_profile or .profile:

eval $(SHELL="bash" pipenv --completion)

This will force pipenv environments.py to detect the shell as bash instead of bash.exe.

[uranusjr]

It’s probably alright, I don’t think they changed their API, so we can just upgrade all the way to latest (I think). Send a message when it’s done and we can try it out!

[ociule]

The irony of it is, click-completion already does the right thing (removing the .exe) when using its get_auto_shell capability instead of passing the shell as a param like pipenv does, but that adds psutil as a dependency, which I'm sure you're reluctant to do.

[techalchemy]

To say we don’t want to bring in psutil is a huuuge understatement. For reference: https://github.com/pypa/pipenv/blob/master/tasks/vendoring/patches/patched/pew_psutil.patch was implemented just to remove psutil and it does the exact same thing (but much faster)

Hey @uranusjr have you ever looked at click completion? Take a look at this:

pipenv/pipenv/vendor/click_completion.py

Line 410 in 4425644

parent = psutil.Process(os.getpid()).parent()

— look familiar? Did one person write the only code to determine parent shells in wide use in python? We already reimplemented this. If psutil is a local dependency for people it explains why completion is slow. We can fix that one too.

[uranusjr]

I feel fortunate we already implemented this… that is not exactly inspiring.

[uranusjr]

Reading the source it’s not even that complication. I bet I can come up with something tonight.

[uranusjr]

It turns out we can avoid psutil without even needing to patch anything. I do want to improve our own shell detection code first before doing it, however. I think I will make a project out of this.

[ociule]

@uranusjr I've talked with the click-completion developer and they've clarified that 'bash.exe' is not a valid value for the shell parameter. So code calling click-completion.get_code should make sure to provide the right value, which is just 'bash'. How do you feel about a patch for pipenv that fixed this ? I could write that.

It shouldn't interfere with your work on remaking the shell detection code, I think.

[ociule]

The patch I'm talking about is necessary because in environments.py there's this line:

PIPENV_SHELL = os.environ.get('SHELL') or os.environ.get('PYENV_SHELL')

SHELL is set to bash.exe on Bash on Windows. shellingham never gets called in this situation.

[uranusjr]

@ociule We should strip the extension before sending it to Click-Completion (so bash.exe becomes bash0). This is an oversight, and I’ll amend it tomorrow, hopefully in time for the release. If it doesn’t make it, I also introduced a new environment variable PIPENV_SHELL that will override all detection and other environment configuration, so you can add export PIPENV_SHELL=bash in your bashrc and be guaranteed to get the correct behaviour.

[glehmann]

We never had any speed issue with psutil speed, at least for the very simple usage we have. It is an optional dependency in our code, just because it is not always distributed in binary version in pipy.

Anyway, we could has well use shelligham for what we do. It seems broken with python 2 though, and I can't find the github (or any other) repository.

@uranusjr could you point to the shelligham project page?

[glehmann]

BTW, I'm one of the click-completion developers :)

[uranusjr]

@glehmann Of course, here it is: https://github.com/sarugaku/shellingham

For myself the problem of psutil is not speed, but it has C dependencies. Pipenv tries to vendor dependencies if possible, and C dependencies are difficult to handle :(