Added support for bash.exe

[ociule]

Hello there,

I noticed (through using pipenv, which uses click-completion) that there's no full support for bash on windows.

I tested the scripts click-completion generates for bash and they work on bash.exe. It's just the internal shell handling code that needed some modifications.

I'm available and willing to improve the patch.

[glehmann]

it used to work — we really need a test suite to check for regressions.

return parent.name().replace('.exe', ''), line 379, should be enough to deal with bash on windows. Could you check what is the output of get_auto_shell() on your system?

I don't like much the idea of a bash.exe in the shell types. Really just bash should be necessary :)

[ociule]

get_auto_shell works fine, as indeed it does the replace('.exe', '')

However, my fix is needed when not using get_auto_shell, by passing the shell param to install() or get_code(). Pipenv does this, as they do not want to use get_auto_shell, as that calls psutils, which is too slow.

Declaring that 'bash' is the cannonical identifier for bash on all platforms could work, you just need to make it clear in the docs. It will be up for code calling click-completion to setup the shell param with the correct value then.

Btw, an Enum would make this clear, instead of a string value. The shell param could take an Enum that could only take specific values.

[glehmann]

ok, I've just looked at the pipenv code :) Indeed it is not returning the expected shell value — it should be easy enough to fix on their side.

As for anything that could make the code easier to use, we are open to suggestions and/or pull requests :) I've not used the enum in python yet, but it indeed seems like a logical way to implement it. We just need to make sure it is backward compatible with the current code.

Do you have a pointer on how slow psutil can be? I never experienced any speed problem with it.

And I like how they are using eval to load the code generated by click-completion — a nice way to make sure that the completion code is up to date!

[ociule]

Yes, there are ways to make it backwards compatible, by accepting both a string or the Enum as values for the parameter, and documenting just the Enum, so that all future code will use it.

No idea personally about psutil, but I know that the pipenv devs have made lots of effort to keep pipenv fast, and that included remaking code that was using psutil. They've even vendored dependencies to patch them.

Here's a discussion mentioning this: https://github.com/pypa/pipenv/pull/2352#issuecomment-397223816

[glehmann]

So I think this problem is fixed either in shelligham or in pipenv, and the missing documentation on possible shell types in addressed in d44413140e35a9486e0fa9d345860872d3310fb6. Do we still need this PR?

[ociule]

No, we're good.