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

Feature/completion item/resolve #25

Merged
1 change: 1 addition & 0 deletions CONFIGURATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ This server can be configured using `workspace/didChangeConfiguration` method. E
| `pylsp.plugins.jedi_completion.include_params` | `boolean` | Auto-completes methods and classes with tabstops for each parameter. | `true` |
| `pylsp.plugins.jedi_completion.include_class_objects` | `boolean` | Adds class objects as a separate completion item. | `true` |
| `pylsp.plugins.jedi_completion.fuzzy` | `boolean` | Enable fuzzy when requesting autocomplete. | `false` |
| `pylsp.plugins.jedi_completion.eager` | `boolean` | Resolve documentation and detail eagerly. | `false` |
| `pylsp.plugins.jedi_definition.enabled` | `boolean` | Enable or disable the plugin. | `true` |
| `pylsp.plugins.jedi_definition.follow_imports` | `boolean` | The goto call will follow imports. | `true` |
| `pylsp.plugins.jedi_definition.follow_builtin_imports` | `boolean` | If follow_imports is True will decide if it follow builtin imports. | `true` |
Expand Down
5 changes: 5 additions & 0 deletions pylsp/config/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@
"default": false,
"description": "Enable fuzzy when requesting autocomplete."
},
"pylsp.plugins.jedi_completion.eager": {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know how we should manage these kind of configuration options that apply to both jedi, rope and possibly other completion backends

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 splitting them by plugin is fine because some plugins may be very fast to eagerly resolve completions and some may be slow. It might be beneficial to let user/IDE make this choice (not necessarily today with rope and jedi as only completion providers, but potentially in the future).

This made me realise I missed adding the setting to schema for rope...

"type": "boolean",
"default": false,
"description": "Resolve documentation and detail eagerly."
},
"pylsp.plugins.jedi_definition.enabled": {
"type": "boolean",
"default": true,
Expand Down
5 changes: 5 additions & 0 deletions pylsp/hookspecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ def pylsp_completions(config, workspace, document, position):
pass


@hookspec(firstresult=True)
def pylsp_completion_item_resolve(config, workspace, document, completion_item):
pass


@hookspec
def pylsp_definitions(config, workspace, document, position):
pass
Expand Down
40 changes: 35 additions & 5 deletions pylsp/plugins/jedi_completion.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,13 @@
@hookimpl
def pylsp_completions(config, document, position):
"""Get formatted completions for current code position"""
# pylint: disable=too-many-locals

settings = config.plugin_settings('jedi_completion', document_path=document.path)
resolve_eagerly = settings.get('eager', False)
code_position = _utils.position_to_jedi_linecolumn(document, position)

code_position["fuzzy"] = settings.get("fuzzy", False)
code_position['fuzzy'] = settings.get('fuzzy', False)
completions = document.jedi_script(use_document_path=True).complete(**code_position)

if not completions:
Expand All @@ -60,17 +63,37 @@ def pylsp_completions(config, document, position):
for c in completions
]

# TODO split up once other improvements are merged
if include_class_objects:
for c in completions:
krassowski marked this conversation as resolved.
Show resolved Hide resolved
if c.type == 'class':
completion_dict = _format_completion(c, False)
completion_dict = _format_completion(c, False, resolve=resolve_eagerly)
completion_dict['kind'] = lsp.CompletionItemKind.TypeParameter
completion_dict['label'] += ' object'
ready_completions.append(completion_dict)

for completion_dict in ready_completions:
completion_dict['data'] = {
'doc_uri': document.uri
}

# most recently retrieved completion items, used for resolution
document.shared_data['LAST_JEDI_COMPLETIONS'] = {
# label is the only required property; here it is assumed to be unique
completion['label']: (completion, data)
for completion, data in zip(ready_completions, completions)
}

return ready_completions or None


@hookimpl
def pylsp_completion_item_resolve(completion_item, document):
"""Resolve formatted completion for given non-resolved completion"""
completion, data = document.shared_data['LAST_JEDI_COMPLETIONS'].get(completion_item['label'])
return _resolve_completion(completion, data)


def is_exception_class(name):
"""
Determine if a class name is an instance of an Exception.
Expand Down Expand Up @@ -121,16 +144,23 @@ def use_snippets(document, position):
not (expr_type in _ERRORS and 'import' in code))


def _format_completion(d, include_params=True):
def _resolve_completion(completion, d):
completion['detail'] = _detail(d)
completion['documentation'] = _utils.format_docstring(d.docstring())
return completion


def _format_completion(d, include_params=True, resolve=False):
completion = {
'label': _label(d),
'kind': _TYPE_MAP.get(d.type),
'detail': _detail(d),
'documentation': _utils.format_docstring(d.docstring()),
'sortText': _sort_text(d),
'insertText': d.name
}

if resolve:
completion = _resolve_completion(completion, d)

if d.type == 'path':
path = osp.normpath(d.name)
path = path.replace('\\', '\\\\')
Expand Down
52 changes: 41 additions & 11 deletions pylsp/plugins/rope_completion.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,26 @@
@hookimpl
def pylsp_settings():
# Default rope_completion to disabled
return {'plugins': {'rope_completion': {'enabled': False}}}
return {'plugins': {'rope_completion': {'enabled': False, 'eager': False}}}


def _resolve_completion(completion, data):
try:
doc = data.get_doc()
except AttributeError:
doc = ""
completion['detail'] = '{0} {1}'.format(data.scope or "", data.name)
completion['documentation'] = doc
return completion


@hookimpl
def pylsp_completions(config, workspace, document, position):
# pylint: disable=too-many-locals

settings = config.plugin_settings('rope_completion', document_path=document.path)
resolve_eagerly = settings.get('eager', False)

# Rope is a bit rubbish at completing module imports, so we'll return None
word = document.word_at_position({
# The -1 should really be trying to look at the previous word, but that might be quite expensive
Expand All @@ -41,22 +56,37 @@ def pylsp_completions(config, workspace, document, position):
definitions = sorted_proposals(definitions)
new_definitions = []
for d in definitions:
try:
doc = d.get_doc()
except AttributeError:
doc = None
new_definitions.append({
item = {
'label': d.name,
'kind': _kind(d),
'detail': '{0} {1}'.format(d.scope or "", d.name),
'documentation': doc or "",
'sortText': _sort_text(d)
})
'sortText': _sort_text(d),
'data': {
'doc_uri': document.uri
}
}
if resolve_eagerly:
item = _resolve_completion(item, d)
new_definitions.append(item)

# most recently retrieved completion items, used for resolution
document.shared_data['LAST_ROPE_COMPLETIONS'] = {
# label is the only required property; here it is assumed to be unique
completion['label']: (completion, data)
for completion, data in zip(new_definitions, definitions)
}

definitions = new_definitions

return definitions or None


@hookimpl
def pylsp_completion_item_resolve(completion_item, document):
"""Resolve formatted completion for given non-resolved completion"""
completion, data = document.shared_data['LAST_ROPE_COMPLETIONS'].get(completion_item['label'])
return _resolve_completion(completion, data)


def _sort_text(definition):
""" Ensure builtins appear at the bottom.
Description is of format <type>: <module>.<item>
Expand All @@ -72,7 +102,7 @@ def _sort_text(definition):


def _kind(d):
""" Return the VSCode type """
""" Return the LSP type """
MAP = {
'none': lsp.CompletionItemKind.Value,
'type': lsp.CompletionItemKind.Class,
Expand Down
11 changes: 9 additions & 2 deletions pylsp/python_lsp.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ def capabilities(self):
'resolveProvider': False, # We may need to make this configurable
},
'completionProvider': {
'resolveProvider': False, # We know everything ahead of time
'triggerCharacters': ['.']
'resolveProvider': True, # We could know everything ahead of time, but this takes time to transfer
Copy link
Contributor

Choose a reason for hiding this comment

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

If this value is True, then it means that eager is False, right? Then if a client decides to have all the information ready and sets eager to True, then it is their responsibility to not calling completionResolve?

Copy link
Contributor Author

@krassowski krassowski Jul 12, 2021

Choose a reason for hiding this comment

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

If this value is True, then it means that eager is False, right?

IMO no, eager still can be True. resolveProvider just declares that server will handle completionItem/resolve requests; it does not forbid server to return resolved items in the first place.

if a client decides to have all the information ready and sets eager to True, then it is their responsibility to not calling completionResolve?

No, the client is still allowed to call completionItem/resolve, it will be just inefficient for them to do so.

'triggerCharacters': ['.'],
},
'documentFormattingProvider': True,
'documentHighlightProvider': True,
Expand Down Expand Up @@ -250,6 +250,10 @@ def completions(self, doc_uri, position):
'items': flatten(completions)
}

def completion_item_resolve(self, completion_item):
doc_uri = completion_item.get('data', {}).get('doc_uri', None)
return self._hook('pylsp_completion_item_resolve', doc_uri, completion_item=completion_item)

def definitions(self, doc_uri, position):
return flatten(self._hook('pylsp_definitions', doc_uri, position=position))

Expand Down Expand Up @@ -296,6 +300,9 @@ def signature_help(self, doc_uri, position):
def folding(self, doc_uri):
return flatten(self._hook('pylsp_folding_range', doc_uri))

def m_completion_item__resolve(self, **completionItem):
return self.completion_item_resolve(completionItem)

def m_text_document__did_close(self, textDocument=None, **_kwargs):
workspace = self._match_uri_to_workspace(textDocument['uri'])
workspace.rm_document(textDocument['uri'])
Expand Down
1 change: 1 addition & 0 deletions pylsp/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ def __init__(self, uri, workspace, source=None, version=None, local=True, extra_
self.path = uris.to_fs_path(uri)
self.dot_path = _utils.path_to_dot_name(self.path)
self.filename = os.path.basename(self.path)
self.shared_data = {}

self._config = workspace._config
self._workspace = workspace
Expand Down
29 changes: 28 additions & 1 deletion test/plugins/test_completion.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from pylsp import uris, lsp
from pylsp.workspace import Document
from pylsp.plugins.jedi_completion import pylsp_completions as pylsp_jedi_completions
from pylsp.plugins.jedi_completion import pylsp_completion_item_resolve as pylsp_jedi_completion_item_resolve
from pylsp.plugins.rope_completion import pylsp_completions as pylsp_rope_completions
from pylsp._utils import JEDI_VERSION

Expand Down Expand Up @@ -44,6 +45,10 @@ def everyone(self, a, b, c=None, d=2):
print Hello().world

print Hello().every

def documented_hello():
\"\"\"Sends a polite greeting\"\"\"
pass
"""


Expand Down Expand Up @@ -139,6 +144,26 @@ def test_jedi_completion(config, workspace):
pylsp_jedi_completions(config, doc, {'line': 1, 'character': 1000})


def test_jedi_completion_item_resolve(config, workspace):
# Over the blank line
com_position = {'line': 8, 'character': 0}
doc = Document(DOC_URI, workspace, DOC)
completions = pylsp_jedi_completions(config, doc, com_position)

items = {c['label']: c for c in completions}

documented_hello_item = items['documented_hello()']

assert 'documentation' not in documented_hello_item
assert 'detail' not in documented_hello_item

resolved_documented_hello = pylsp_jedi_completion_item_resolve(
completion_item=documented_hello_item,
document=doc
)
assert 'Sends a polite greeting' in resolved_documented_hello['documentation']


def test_jedi_completion_with_fuzzy_enabled(config, workspace):
# Over 'i' in os.path.isabs(...)
config.update({'plugins': {'jedi_completion': {'fuzzy': True}}})
Expand Down Expand Up @@ -410,7 +435,9 @@ def test_jedi_completion_environment(workspace):
# After 'import logh' with new environment
completions = pylsp_jedi_completions(doc._config, doc, com_position)
assert completions[0]['label'] == 'loghub'
assert 'changelog generator' in completions[0]['documentation'].lower()

resolved = pylsp_jedi_completion_item_resolve(completions[0], doc)
assert 'changelog generator' in resolved['documentation'].lower()


def test_document_path_completions(tmpdir, workspace_other_root_path):
Expand Down