From 38aff3ccf57141ab9e63a697e68f6a664c41c0ff Mon Sep 17 00:00:00 2001 From: krassowski Date: Sat, 4 Sep 2021 16:04:20 +0100 Subject: [PATCH 1/4] Migrate to MarkupContent and convert docstrings to Markdown --- pylsp/_utils.py | 68 +++++++++++++++++++++++++++++--- pylsp/plugins/hover.py | 29 ++++++-------- pylsp/plugins/jedi_completion.py | 36 ++++++++++++----- pylsp/plugins/rope_completion.py | 29 ++++++++++---- pylsp/plugins/signature.py | 21 ++++++++-- setup.py | 1 + test/plugins/test_completion.py | 9 ++++- test/plugins/test_hover.py | 22 +++++------ test/plugins/test_signature.py | 10 ++--- 9 files changed, 165 insertions(+), 60 deletions(-) diff --git a/pylsp/_utils.py b/pylsp/_utils.py index 92376f6c..f755e3de 100644 --- a/pylsp/_utils.py +++ b/pylsp/_utils.py @@ -6,8 +6,11 @@ import logging import os import pathlib +import re import threading +from typing import List, Optional +import docstring_to_markdown import jedi JEDI_VERSION = jedi.__version__ @@ -138,17 +141,72 @@ def _merge_dicts_(a, b): return dict(_merge_dicts_(dict_a, dict_b)) -def format_docstring(contents): - """Python doc strings come in a number of formats, but LSP wants markdown. - - Until we can find a fast enough way of discovering and parsing each format, - we can do a little better by at least preserving indentation. +def escape_markdown(contents: str) -> str: + """ + Try to make the plain text string work nicely when displayed in Markdown environment. """ + # escape markdown syntax + contents = re.sub(r'([\\*_#[\]])', r'\\\1', contents) + # preserve white space characters contents = contents.replace('\t', u'\u00A0' * 4) contents = contents.replace(' ', u'\u00A0' * 2) return contents +def wrap_signature(signature): + return '```python\n' + signature + '\n```\n' + + +SERVER_SUPPORTED_MARKUP_KINDS = {'markdown', 'plaintext'} + + +def choose_markup_kind(client_supported_markup_kinds: List[str]): + """Choose a markup kind supported by both client and the server, + + giving a priority to the markup kinds provided earlier on the client preference list""" + for kind in client_supported_markup_kinds: + if kind in SERVER_SUPPORTED_MARKUP_KINDS: + return kind + return 'markdown' + + +def format_docstring(contents: str, markup_kind: str, signatures: Optional[List[str]] = None): + """Transform the provided docstring into a MarkupContent object. + + If `markup_kind` is 'markdown' the docstring will get converted to + markdown representation using `docstring-to-markdown`; if it is + `plaintext`, it will be returned as plain text. + """ + if not isinstance(contents, str): + contents = '' + + if markup_kind == 'markdown': + try: + value = docstring_to_markdown.convert(contents) + return { + 'kind': 'markdown', + 'value': value + } + except docstring_to_markdown.UnknownFormatError: + # try to escape the Markdown syntax instead: + value = escape_markdown(contents) + + if signatures: + value = wrap_signature('\n'.join(signatures)) + '\n\n' + value + + return { + 'kind': 'markdown', + 'value': value + } + value = contents + if signatures: + value = '\n'.join(signatures) + '\n\n' + value + return { + 'kind': 'plaintext', + 'value': value + } + + def clip_column(column, lines, line_number): """ Normalise the position as per the LSP that accepts character positions > line length diff --git a/pylsp/plugins/hover.py b/pylsp/plugins/hover.py index a4d45d1c..f6ae4d7f 100644 --- a/pylsp/plugins/hover.py +++ b/pylsp/plugins/hover.py @@ -9,7 +9,7 @@ @hookimpl -def pylsp_hover(document, position): +def pylsp_hover(config, document, position): code_position = _utils.position_to_jedi_linecolumn(document, position) definitions = document.jedi_script(use_document_path=True).infer(**code_position) word = document.word_at_position(position) @@ -26,24 +26,19 @@ def pylsp_hover(document, position): if not definition: return {'contents': ''} - # raw docstring returns only doc, without signature - doc = _utils.format_docstring(definition.docstring(raw=True)) + hover_capabilities = config.capabilities.get('textDocument', {}).get('hover', {}) + supported_markup_kinds = hover_capabilities.get('contentFormat', ['markdown']) + preferred_markup_kind = _utils.choose_markup_kind(supported_markup_kinds) # Find first exact matching signature signature = next((x.to_string() for x in definition.get_signatures() if x.name == word), '') - contents = [] - if signature: - contents.append({ - 'language': 'python', - 'value': signature, - }) - - if doc: - contents.append(doc) - - if not contents: - return {'contents': ''} - - return {'contents': contents} + return { + 'contents': _utils.format_docstring( + # raw docstring returns only doc, without signature + definition.docstring(raw=True), + preferred_markup_kind, + signatures=[signature] if signature else None + ) + } diff --git a/pylsp/plugins/jedi_completion.py b/pylsp/plugins/jedi_completion.py index 41e2e573..3005ca49 100644 --- a/pylsp/plugins/jedi_completion.py +++ b/pylsp/plugins/jedi_completion.py @@ -52,7 +52,10 @@ def pylsp_completions(config, document, position): return None completion_capabilities = config.capabilities.get('textDocument', {}).get('completion', {}) - snippet_support = completion_capabilities.get('completionItem', {}).get('snippetSupport') + item_capabilities = completion_capabilities.get('completionItem', {}) + snippet_support = item_capabilities.get('snippetSupport') + supported_markup_kinds = item_capabilities.get('documentationFormat', ['markdown']) + preferred_markup_kind = _utils.choose_markup_kind(supported_markup_kinds) should_include_params = settings.get('include_params') should_include_class_objects = settings.get('include_class_objects', True) @@ -68,7 +71,8 @@ def pylsp_completions(config, document, position): ready_completions = [ _format_completion( c, - include_params, + markup_kind=preferred_markup_kind, + include_params=include_params, resolve=resolve_eagerly, resolve_label=(i < max_labels_resolve) ) @@ -81,7 +85,8 @@ def pylsp_completions(config, document, position): if c.type == 'class': completion_dict = _format_completion( c, - False, + markup_kind=preferred_markup_kind, + include_params=False, resolve=resolve_eagerly, resolve_label=(i < max_labels_resolve) ) @@ -105,12 +110,18 @@ def pylsp_completions(config, document, position): @hookimpl -def pylsp_completion_item_resolve(completion_item, document): +def pylsp_completion_item_resolve(config, completion_item, document): """Resolve formatted completion for given non-resolved completion""" shared_data = document.shared_data['LAST_JEDI_COMPLETIONS'].get(completion_item['label']) + + completion_capabilities = config.capabilities.get('textDocument', {}).get('completion', {}) + item_capabilities = completion_capabilities.get('completionItem', {}) + supported_markup_kinds = item_capabilities.get('documentationFormat', ['markdown']) + preferred_markup_kind = _utils.choose_markup_kind(supported_markup_kinds) + if shared_data: completion, data = shared_data - return _resolve_completion(completion, data) + return _resolve_completion(completion, data, markup_kind=preferred_markup_kind) return completion_item @@ -164,18 +175,25 @@ def use_snippets(document, position): not (expr_type in _ERRORS and 'import' in code)) -def _resolve_completion(completion, d): +def _resolve_completion(completion, d, markup_kind: str): # pylint: disable=broad-except completion['detail'] = _detail(d) try: - docs = _utils.format_docstring(d.docstring()) + docs = _utils.format_docstring( + d.docstring(raw=True), + signatures=[ + signature.to_string() + for signature in d.get_signatures() + ], + markup_kind=markup_kind + ) except Exception: docs = '' completion['documentation'] = docs return completion -def _format_completion(d, include_params=True, resolve=False, resolve_label=False): +def _format_completion(d, markup_kind: str, include_params=True, resolve=False, resolve_label=False): completion = { 'label': _label(d, resolve_label), 'kind': _TYPE_MAP.get(d.type), @@ -184,7 +202,7 @@ def _format_completion(d, include_params=True, resolve=False, resolve_label=Fals } if resolve: - completion = _resolve_completion(completion, d) + completion = _resolve_completion(completion, d, markup_kind) if d.type == 'path': path = osp.normpath(d.name) diff --git a/pylsp/plugins/rope_completion.py b/pylsp/plugins/rope_completion.py index b73d8412..395f7d82 100644 --- a/pylsp/plugins/rope_completion.py +++ b/pylsp/plugins/rope_completion.py @@ -4,7 +4,7 @@ import logging from rope.contrib.codeassist import code_assist, sorted_proposals -from pylsp import hookimpl, lsp +from pylsp import _utils, hookimpl, lsp log = logging.getLogger(__name__) @@ -16,10 +16,14 @@ def pylsp_settings(): return {'plugins': {'rope_completion': {'enabled': False, 'eager': False}}} -def _resolve_completion(completion, data): +def _resolve_completion(completion, data, markup_kind): + # pylint: disable=broad-except try: - doc = data.get_doc() - except AttributeError: + doc = _utils.format_docstring( + data.get_doc(), + markup_kind=markup_kind + ) + except Exception: doc = "" completion['detail'] = '{0} {1}'.format(data.scope or "", data.name) completion['documentation'] = doc @@ -47,6 +51,11 @@ def pylsp_completions(config, workspace, document, position): rope_project = workspace._rope_project_builder(rope_config) document_rope = document._rope_resource(rope_config) + completion_capabilities = config.capabilities.get('textDocument', {}).get('completion', {}) + item_capabilities = completion_capabilities.get('completionItem', {}) + supported_markup_kinds = item_capabilities.get('documentationFormat', ['markdown']) + preferred_markup_kind = _utils.choose_markup_kind(supported_markup_kinds) + try: definitions = code_assist(rope_project, document.source, offset, document_rope, maxfixes=3) except Exception as e: # pylint: disable=broad-except @@ -65,7 +74,7 @@ def pylsp_completions(config, workspace, document, position): } } if resolve_eagerly: - item = _resolve_completion(item, d) + item = _resolve_completion(item, d, preferred_markup_kind) new_definitions.append(item) # most recently retrieved completion items, used for resolution @@ -81,10 +90,16 @@ def pylsp_completions(config, workspace, document, position): @hookimpl -def pylsp_completion_item_resolve(completion_item, document): +def pylsp_completion_item_resolve(config, 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) + + completion_capabilities = config.capabilities.get('textDocument', {}).get('completion', {}) + item_capabilities = completion_capabilities.get('completionItem', {}) + supported_markup_kinds = item_capabilities.get('documentationFormat', ['markdown']) + preferred_markup_kind = _utils.choose_markup_kind(supported_markup_kinds) + + return _resolve_completion(completion, data, preferred_markup_kind) def _sort_text(definition): diff --git a/pylsp/plugins/signature.py b/pylsp/plugins/signature.py index c4c3048f..4907a6e3 100644 --- a/pylsp/plugins/signature.py +++ b/pylsp/plugins/signature.py @@ -15,28 +15,41 @@ @hookimpl -def pylsp_signature_help(document, position): +def pylsp_signature_help(config, document, position): code_position = _utils.position_to_jedi_linecolumn(document, position) signatures = document.jedi_script().get_signatures(**code_position) if not signatures: return {'signatures': []} + signature_capabilities = config.capabilities.get('textDocument', {}).get('signatureHelp', {}) + signature_information_support = signature_capabilities.get('signatureInformation', {}) + supported_markup_kinds = signature_information_support.get('documentationFormat', ['markdown']) + preferred_markup_kind = _utils.choose_markup_kind(supported_markup_kinds) + s = signatures[0] + docstring = s.docstring() + # Docstring contains one or more lines of signature, followed by empty line, followed by docstring - function_sig_lines = (s.docstring().split('\n\n') or [''])[0].splitlines() + function_sig_lines = (docstring.split('\n\n') or [''])[0].splitlines() function_sig = ' '.join([line.strip() for line in function_sig_lines]) sig = { 'label': function_sig, - 'documentation': _utils.format_docstring(s.docstring(raw=True)) + 'documentation': _utils.format_docstring( + s.docstring(raw=True), + markup_kind=preferred_markup_kind + ) } # If there are params, add those if s.params: sig['parameters'] = [{ 'label': p.name, - 'documentation': _param_docs(s.docstring(), p.name) + 'documentation': _utils.format_docstring( + _param_docs(docstring, p.name), + markup_kind=preferred_markup_kind + ) } for p in s.params] # We only return a single signature because Python doesn't allow overloading diff --git a/setup.py b/setup.py index 3f797740..07216d09 100755 --- a/setup.py +++ b/setup.py @@ -29,6 +29,7 @@ def get_version(module='pylsp'): 'jedi>=0.17.2,<0.19.0', 'python-lsp-jsonrpc>=1.0.0', 'pluggy', + 'docstring-to-markdown', 'ujson>=3.0.0', 'setuptools>=39.0.0' ] diff --git a/test/plugins/test_completion.py b/test/plugins/test_completion.py index 1afee4ac..f68ba347 100644 --- a/test/plugins/test_completion.py +++ b/test/plugins/test_completion.py @@ -160,10 +160,15 @@ def test_jedi_completion_item_resolve(config, workspace): assert 'detail' not in documented_hello_item resolved_documented_hello = pylsp_jedi_completion_item_resolve( + doc._config, completion_item=documented_hello_item, document=doc ) - assert 'Sends a polite greeting' in resolved_documented_hello['documentation'] + expected_doc = { + 'kind': 'markdown', + 'value': '```python\ndocumented_hello()\n```\n\n\nSends a polite greeting' + } + assert resolved_documented_hello['documentation'] == expected_doc def test_jedi_completion_with_fuzzy_enabled(config, workspace): @@ -457,7 +462,7 @@ def test_jedi_completion_environment(workspace): completions = pylsp_jedi_completions(doc._config, doc, com_position) assert completions[0]['label'] == 'loghub' - resolved = pylsp_jedi_completion_item_resolve(completions[0], doc) + resolved = pylsp_jedi_completion_item_resolve(doc._config, completions[0], doc) assert 'changelog generator' in resolved['documentation'].lower() diff --git a/test/plugins/test_hover.py b/test/plugins/test_hover.py index 7ac6e071..89040247 100644 --- a/test/plugins/test_hover.py +++ b/test/plugins/test_hover.py @@ -38,16 +38,16 @@ def test_numpy_hover(workspace): doc = Document(DOC_URI, workspace, NUMPY_DOC) contents = '' - assert contents in pylsp_hover(doc, no_hov_position)['contents'] + assert contents in pylsp_hover(doc._config, doc, no_hov_position)['contents'] contents = 'NumPy\n=====\n\nProvides\n' - assert contents in pylsp_hover(doc, numpy_hov_position_1)['contents'][0] + assert contents in pylsp_hover(doc._config, doc, numpy_hov_position_1)['contents']['value'] contents = 'NumPy\n=====\n\nProvides\n' - assert contents in pylsp_hover(doc, numpy_hov_position_2)['contents'][0] + assert contents in pylsp_hover(doc._config, doc, numpy_hov_position_2)['contents']['value'] contents = 'NumPy\n=====\n\nProvides\n' - assert contents in pylsp_hover(doc, numpy_hov_position_3)['contents'][0] + assert contents in pylsp_hover(doc._config, doc, numpy_hov_position_3)['contents']['value'] # https://github.com/davidhalter/jedi/issues/1746 # pylint: disable=import-outside-toplevel @@ -55,8 +55,8 @@ def test_numpy_hover(workspace): if np.lib.NumpyVersion(np.__version__) < '1.20.0': contents = 'Trigonometric sine, element-wise.\n\n' - assert contents in pylsp_hover( - doc, numpy_sin_hov_position)['contents'][0] + assert contents in pylsp_hover(doc._config, + doc, numpy_sin_hov_position)['contents']['value'] def test_hover(workspace): @@ -67,13 +67,13 @@ def test_hover(workspace): doc = Document(DOC_URI, workspace, DOC) - contents = [{'language': 'python', 'value': 'main()'}, 'hello world'] + contents = {'kind': 'markdown', 'value': '```python\nmain()\n```\n\n\nhello world'} assert { 'contents': contents - } == pylsp_hover(doc, hov_position) + } == pylsp_hover(doc._config, doc, hov_position) - assert {'contents': ''} == pylsp_hover(doc, no_hov_position) + assert {'contents': ''} == pylsp_hover(doc._config, doc, no_hov_position) def test_document_path_hover(workspace_other_root_path, tmpdir): @@ -96,6 +96,6 @@ def foo(): doc = Document(doc_uri, workspace_other_root_path, doc_content) cursor_pos = {'line': 1, 'character': 3} - contents = pylsp_hover(doc, cursor_pos)['contents'] + contents = pylsp_hover(doc._config, doc, cursor_pos)['contents'] - assert contents[1] == 'A docstring for foo.' + assert 'A docstring for foo.' in contents['value'] diff --git a/test/plugins/test_signature.py b/test/plugins/test_signature.py index 51cecb56..d9dbb8d2 100644 --- a/test/plugins/test_signature.py +++ b/test/plugins/test_signature.py @@ -46,7 +46,7 @@ def test_no_signature(workspace): sig_position = {'line': 9, 'character': 0} doc = Document(DOC_URI, workspace, DOC) - sigs = signature.pylsp_signature_help(doc, sig_position)['signatures'] + sigs = signature.pylsp_signature_help(doc._config, doc, sig_position)['signatures'] assert not sigs @@ -55,13 +55,13 @@ def test_signature(workspace): sig_position = {'line': 10, 'character': 5} doc = Document(DOC_URI, workspace, DOC) - sig_info = signature.pylsp_signature_help(doc, sig_position) + sig_info = signature.pylsp_signature_help(doc._config, doc, sig_position) sigs = sig_info['signatures'] assert len(sigs) == 1 assert sigs[0]['label'] == 'main(param1, param2)' assert sigs[0]['parameters'][0]['label'] == 'param1' - assert sigs[0]['parameters'][0]['documentation'] == 'Docs for param1' + assert sigs[0]['parameters'][0]['documentation'] == {'kind': 'markdown', 'value': 'Docs for param1'} assert sig_info['activeParameter'] == 0 @@ -71,7 +71,7 @@ def test_multi_line_signature(workspace): sig_position = {'line': 17, 'character': 5} doc = Document(DOC_URI, workspace, MULTI_LINE_DOC) - sig_info = signature.pylsp_signature_help(doc, sig_position) + sig_info = signature.pylsp_signature_help(doc._config, doc, sig_position) sigs = sig_info['signatures'] assert len(sigs) == 1 @@ -80,7 +80,7 @@ def test_multi_line_signature(workspace): 'param5=None, param6=None, param7=None, param8=None)' ) assert sigs[0]['parameters'][0]['label'] == 'param1' - assert sigs[0]['parameters'][0]['documentation'] == 'Docs for param1' + assert sigs[0]['parameters'][0]['documentation'] == {'kind': 'markdown', 'value': 'Docs for param1'} assert sig_info['activeParameter'] == 0 From d27c8aa0fed38661318a204f22a040b16370ab7c Mon Sep 17 00:00:00 2001 From: krassowski Date: Sat, 4 Sep 2021 16:14:56 +0100 Subject: [PATCH 2/4] Fix test which is only run on CI --- test/plugins/test_completion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/plugins/test_completion.py b/test/plugins/test_completion.py index f68ba347..28952c30 100644 --- a/test/plugins/test_completion.py +++ b/test/plugins/test_completion.py @@ -463,7 +463,7 @@ def test_jedi_completion_environment(workspace): assert completions[0]['label'] == 'loghub' resolved = pylsp_jedi_completion_item_resolve(doc._config, completions[0], doc) - assert 'changelog generator' in resolved['documentation'].lower() + assert 'changelog generator' in resolved['documentation']['value'].lower() def test_document_path_completions(tmpdir, workspace_other_root_path): From 9a144a5791ccd39f14b001aaf4b8fe9c6a7fffc9 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Sat, 24 Sep 2022 16:34:58 +0100 Subject: [PATCH 3/4] Restore escaping whitespaces for plain text responses --- pylsp/_utils.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pylsp/_utils.py b/pylsp/_utils.py index 317c5d5f..fb12fcbe 100644 --- a/pylsp/_utils.py +++ b/pylsp/_utils.py @@ -146,15 +146,23 @@ def _merge_dicts_(a, b): return dict(_merge_dicts_(dict_a, dict_b)) +def escape_plain_text(contents: str) -> str: + """ + Format plain text to display nicely in environments which do not respect whitespaces. + """ + contents = contents.replace('\t', '\u00A0' * 4) + contents = contents.replace(' ', '\u00A0' * 2) + return contents + + def escape_markdown(contents: str) -> str: """ - Try to make the plain text string work nicely when displayed in Markdown environment. + Format plain text to display nicely in Markdown environment. """ # escape markdown syntax contents = re.sub(r'([\\*_#[\]])', r'\\\1', contents) # preserve white space characters - contents = contents.replace('\t', '\u00A0' * 4) - contents = contents.replace(' ', '\u00A0' * 2) + contents = escape_plain_text(contents) return contents @@ -208,7 +216,7 @@ def format_docstring(contents: str, markup_kind: str, signatures: Optional[List[ value = '\n'.join(signatures) + '\n\n' + value return { 'kind': 'plaintext', - 'value': value + 'value': escape_plain_text(value) } From fe048f358101c1fc001f2bffb891415579071b9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krassowski?= <5832902+krassowski@users.noreply.github.com> Date: Sun, 25 Sep 2022 22:54:19 +0100 Subject: [PATCH 4/4] Improve docstrings as suggested in code review Co-authored-by: Carlos Cordoba --- pylsp/_utils.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pylsp/_utils.py b/pylsp/_utils.py index fb12fcbe..2c6111d8 100644 --- a/pylsp/_utils.py +++ b/pylsp/_utils.py @@ -174,9 +174,10 @@ def wrap_signature(signature): def choose_markup_kind(client_supported_markup_kinds: List[str]): - """Choose a markup kind supported by both client and the server, + """Choose a markup kind supported by both client and the server. - giving a priority to the markup kinds provided earlier on the client preference list""" + This gives priority to the markup kinds provided earlier on the client preference list. + """ for kind in client_supported_markup_kinds: if kind in SERVER_SUPPORTED_MARKUP_KINDS: return kind @@ -189,6 +190,9 @@ def format_docstring(contents: str, markup_kind: str, signatures: Optional[List[ If `markup_kind` is 'markdown' the docstring will get converted to markdown representation using `docstring-to-markdown`; if it is `plaintext`, it will be returned as plain text. + Call signatures of functions (or equivalent code summaries) + provided in optional `signatures` argument will be prepended + to the provided contents of the docstring if given. """ if not isinstance(contents, str): contents = ''