Skip to content
This repository was archived by the owner on Sep 21, 2019. It is now read-only.

Python dependencies as submodules #412

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Ch00k
Copy link
Contributor

@Ch00k Ch00k commented Apr 21, 2018

This adds all Python dependencies as git submodules. They will all be cloned upon plugin installation, and the user will no longer need to take any manual steps, i.e. pip install.

@@ -1,5 +1,6 @@
if !has('nvim')
execute 'silent! py3file' fnameescape(expand('<sfile>:p').'.py')
execute 'py3file' fnameescape(expand('<sfile>:p').'.py')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better not to hide the output here. Makes debugging a bit easier.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. Remove commented line then.

@Ch00k
Copy link
Contributor Author

Ch00k commented May 23, 2018

Any feedback on this?

@ktonga
Copy link
Contributor

ktonga commented May 23, 2018

I don't think it's necessary. You have to pip install neovim module anyway, so I don't see how adding two more names to the list can be problematic.

@Ch00k
Copy link
Contributor Author

Ch00k commented May 23, 2018

You don't need to pip-install anything for Vim though. I think this change improves user experience a lot for this case.

@ktonga
Copy link
Contributor

ktonga commented May 23, 2018

Fair enough. I was only thinking of Neovim.

@ktonga
Copy link
Contributor

ktonga commented May 23, 2018

I'll give it a spin today. Could you please raise a PR to update the installation instructions in the meantime? Please also include the recent change py2 -> py3

Thanks!

@@ -1,5 +1,6 @@
if !has('nvim')
execute 'silent! py3file' fnameescape(expand('<sfile>:p').'.py')
execute 'py3file' fnameescape(expand('<sfile>:p').'.py')
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. Remove commented line then.

Copy link
Contributor

@ches ches left a comment

Choose a reason for hiding this comment

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

Does this work smoothly with a range of Vim plugin managers? Which ones have been tried?

Could you also please check make lint? We used to have CI on this project but it was a private Drone installation and I'm not sure what's happened with it…

sys.path.insert(0, six_path)
websocket_path = os.path.join(os.path.dirname(__file__), 'websocket-client')
sys.path.insert(0, websocket_path)
import websocket
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be nice to hide away all the sys.path hackery in one place at early initialization, so you can keep normal-looking imports in the commonly-edited files and probably avoid linter complaints, etc. The __init__.py in ensime_shared might work, or it might have to go to autoload/ensime.vim.py and rplugin/python3/ensime.py where there is already path hackery. There is some duplication in that case, maybe there's a way to eliminate it but I didn't find a good one yet.

@ches
Copy link
Contributor

ches commented May 25, 2018

Also, why do we need six at this time?

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

Successfully merging this pull request may close these issues.

3 participants