Ron89 / thesaurus_query.vim

Multi-language Thesaurus Query and Replacement plugin for Vim/NeoVim
http://www.vim.org/scripts/script.php?script_id=5341
Apache License 2.0
220 stars 23 forks source link

Ask user to use correct python executable in Win32 vim #38

Closed idbrii closed 5 years ago

idbrii commented 5 years ago

Fix nonsensical error message when thesaurus module is missing.

Previously, I'd get:

Please use 'c:appsvimvim81gvim.exe -m pip install thesaurus --user --upgrade' to install the latest package. Have you installed it now?

(Backslashes get treated as escapes so path looks weird.)

vim and gvim on Windows report gvim.exe as sys.executable instead of python. Tell user to use python instead. Ask for python executed within vim to increase chances it's the right python.

Tested error message on Win10 gvim.exe and vim.exe 8.1.527 and Ubuntu (WSL) vim 7.4.52.

On Windows I now get:

Please use ':!python -m pip install thesaurus --user --upgrade' to install the latest package. Have you installed it now?

Ron89 commented 5 years ago

Are you sure the Python embedded in Windows' Vim can perform the installation successfully? If I remember correctly, the embedded Python script cannot spawn sub-process. The prompt displayed in the message box is to request user to install the relied package with command line prompt.

idbrii commented 5 years ago

Using :!python wouldn't use the embedded python. It would use the first python found in $PATH.

I've frequently installed pip packages this way, so I can confirm that it works. Copy pasting !python -m pip install thesaurus --user --upgrade into gvim pops up a pip window telling me I've satisfied all dependencies.

Ron89 commented 5 years ago

Ah, how foolish of me... ! is the shell escape... Yes, you are right. Using vim escape to directly install it is also a better idea than prompt user to install it from commandline, in that you don't need to jump out of the editor. In that case, we might also want to change the interface of this installation prompt in the near future. Thanks for the PR!