Respect option follow_imports in goto_assignments call#404
Respect option follow_imports in goto_assignments call#404gatesn merged 5 commits intopalantir:developpalantir/python-language-server:developfrom st4lk:add-option-follow-importsst4lk/python-language-server:add-option-follow-importsCopy head branch name to clipboard
follow_imports in goto_assignments call#404Conversation
gatesn
left a comment
There was a problem hiding this comment.
Generally open to allowing more configuration here, but would like the config documented in package.json.
pyls/plugins/definition.py
Outdated
| def pyls_definitions(document, position): | ||
| definitions = document.jedi_script(position).goto_assignments() | ||
| def pyls_definitions(config, document, position): | ||
| expected_settings = {'follow_imports', 'follow_builtin_imports'} |
There was a problem hiding this comment.
Don't tend to like defensively checking this.
pyls/plugins/definition.py
Outdated
| definitions = document.jedi_script(position).goto_assignments() | ||
| def pyls_definitions(config, document, position): | ||
| expected_settings = {'follow_imports', 'follow_builtin_imports'} | ||
| params = {k: v for k, v in config.settings().items() if k in expected_settings} |
There was a problem hiding this comment.
Config should be namespaced by the plugin, e.g. pyls.plugins.jedi_definition https://github.com/palantir/python-language-server/blob/develop/vscode-client/package.json#L38
|
Updated |
|
@st4lk there are a couple of test failures: E TypeError: pyls_definitions() takes exactly 3 arguments (2 given) |
|
@tmc fixed |
|
@gatesn ping |
pyls/plugins/definition.py
Outdated
| def pyls_definitions(document, position): | ||
| definitions = document.jedi_script(position).goto_assignments() | ||
| def pyls_definitions(config, document, position): | ||
| params = {k: v for k, v in config.plugin_settings('jedi_definition').items() if k} |
There was a problem hiding this comment.
What does if k do here? Do you mean if v ? If you do, I think it should be if v is not None. Else if someone passes False for a property that jedi defaults to True, then the override won't be applied.
|
can we get a release cut for this? |
Add an ability to specify
follow_imports,follow_builtin_importsoptions ingoto_assignmentscall: https://jedi.readthedocs.io/en/latest/docs/api.html#jedi.Script.goto_assignments