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

fix: CheckHealth doesn't work in neovim #765

Merged
merged 2 commits into from
Dec 30, 2017

Conversation

tMinamiii
Copy link
Contributor

Hi. I found CheckHealth bug in neovim.
CheckHealth calls display_debug_info().
However, error occured when loading jedi_vim.jedi.Script('')._evaluator.sys_path,
because Evaluator.sys_path is unavailable variable.

CheckHealth calls display_debug_info().
However, error occured when loading jedi_vim.jedi.Script('')._evaluator.sys_path,
because Evaluator.sys_path is unavailable variable.
@davidhalter
Copy link
Owner

If this broke, I'm sorry. It will break again once the virtualenv branch is merged. But then we'll probably have a public API to ask for this stuff.

@tMinamiii
Copy link
Contributor Author

Oh I see.
Sorry, I did not check virtualenv branch.
This change will break again certainly.
Thanks!

@tMinamiii tMinamiii closed this Dec 28, 2017
@blueyed
Copy link
Collaborator

blueyed commented Dec 28, 2017

I think we should re-open and actually try to fix it, by handling the different variants (i.e. the current one, the one from this PR, and the one from the virtualenv branch).

@narona
Can you update the PR, please?

@davidhalter
Copy link
Owner

Please don't try to work with the virtualenv branch, yet. It's work in progress and we can integrate it as soon as it's in good shape and the API is final.

As for the rest, I agree that here in this pull request we should try to handle both those variants.

@tMinamiii
Copy link
Contributor Author

Thanks for replies.
Should I new PR when virtualenv branch development is finished?
(I'm Sorry I'm not familiar with github.)

@blueyed
Copy link
Collaborator

blueyed commented Dec 29, 2017

No, I would suggest updating this PR to handle both the old and new "API".

script_evaluator = jedi_vim.jedi.Script('')._evaluator
try:
    sys_path = script_evaluator.project.sys_path:
except AttributeError:
    sys_path = script_evaluator.sys_path

@tMinamiii
Copy link
Contributor Author

ahh, I have misunderstood that new API will be added.
I'm sorry for lack of my English proficiency.
I will update my PR.

Evaluator.project.sys_path is temporary fix, because Evaluator.sys_path is correct API.
So, we handle the both the old and new API in preparation for merging virtualenv branch.
@davidhalter davidhalter merged commit 0b9bbc3 into davidhalter:master Dec 30, 2017
@davidhalter
Copy link
Owner

Thanks!

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

Successfully merging this pull request may close these issues.

3 participants