Skip to content

Conversation

@lgeiger
Copy link
Contributor

@lgeiger lgeiger commented Dec 28, 2017

This allows the user to choose between Rope and Jedi for completions.

It also remove the pluggy racing since Jedi's cache isn't thread safe.

Closes #189

Copy link
Contributor

@gatesn gatesn left a comment

Choose a reason for hiding this comment

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

i think we should leave just the single completions hook, and users can enable one, the other, or both.


@hookspec
def pyls_completions(config, workspace, document, position):
def pyls_jedi_completions(config, workspace, document, position):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why have two plugin hooks? Why not just control which implementation runs using the configuration?

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 don't think users should be able to use both completion plugins side by side since this will probably result in duplicated completion results and other weird behaviour, or am I missing something.

I can refactor it to only use on completion hook which runs either jedi or rope depending on the config. What do you think?

@lgeiger lgeiger force-pushed the remove-completion-racing branch from 76a6db6 to 818a0a8 Compare January 2, 2018 19:17
@gatesn
Copy link
Contributor

gatesn commented Jan 5, 2018

Apologies for the slow review here, I’m still on vacation until the 9th January. Hope you understand!

@lgeiger
Copy link
Contributor Author

lgeiger commented Jan 5, 2018

Apologies for the slow review here, I’m still on vacation until the 9th January. Hope you understand!

No problem. Happy holidays!

This allows the user to choose between Rope and Jedi for completions.

It also remove the pluggy racing since Jedi's cache isn't thread safe.

Closes palantir#189
@lgeiger lgeiger force-pushed the remove-completion-racing branch from 818a0a8 to f86c66e Compare January 10, 2018 10:21
@gatesn
Copy link
Contributor

gatesn commented Jan 10, 2018

@lgeiger do you mind if I take over this PR?

@lgeiger
Copy link
Contributor Author

lgeiger commented Jan 10, 2018

do you mind if I take over this PR?

Go ahead!

@gatesn gatesn mentioned this pull request Jan 10, 2018
@gatesn gatesn closed this in #222 Jan 14, 2018
@lgeiger lgeiger deleted the remove-completion-racing branch January 14, 2018 12:35
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.

2 participants