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

Consider using docstring_to_markdown for markdown hover and documentation #22

Closed
krassowski opened this issue Apr 25, 2021 · 4 comments · Fixed by #80
Closed

Consider using docstring_to_markdown for markdown hover and documentation #22

krassowski opened this issue Apr 25, 2021 · 4 comments · Fixed by #80
Milestone

Comments

@krassowski
Copy link
Contributor

Following up on the previous discussions:

I would like to propose the integration of docstring-to-markdown which is a small converter that handles conversion of docstrings, including:

  • PEP 287 compliant reStructuredText docstrings and PEP 257 compliant docstrings
  • reStructuredText with common Sphinx extensions to a degree sufficient to cover majority of docstrings in popular Python projects such as numpy, pandas, or sklearn, including special cases specific to numpy documentation.
  • rejects to convert ambiguous docstrings to prevent malformated text if it turns out to be using a different format; in such case the LSP server should return plain text rather than markdown (or try using a different converter)

Recognition of other docstring styles is planned. The docstring-to-markdown was successfully adopted in jedi-lanaguage-server. I also used it for quite a while on my fork without any major issues. The performance impact of doing on the flight conversion is negligible when only returning the completion in completionItem/resolve (#1, to be updated); I did not test the performance impact in other scenarios.

I would be happy to transfer the project to the python-lsp organization. Leaving this up for discussion as I am sure there will be different takes and opinions.

Alternatives:

While docstring-to-markdown uses simple parsers and regular expressions other solutions are possible. A number of other solutions and packages was tried before I decided to develop docstring-to-markdown (see pappasam/jedi-language-server#22 (comment)) and it appeared the best solution at that time, however other approaches may be feasible in the future:

  • converting reStructuredText to HTML and converting from HTML to Markdown
  • converting reStructuredText to docutils internal nodes format and converting those to Markdown
    The above approaches are likely to be more computation intensive and the first step implies building an entire documentation of a project in order to resolve references
  • ➕ having the resolved references is nice because the "See also" section can be made interactive (i.e. the links work)
  • ➖ the added overhead may be prohibitive for performing such conversion during the runtime (too long for the user)
  • 😐 it is not really that important to have the links in the "See also" section in completer/hover documentation - IMO the most important piece of information is the signature, description of arguments and returned values.
  • 😐 it is possible to pre-build the Markdown docstrings for large, frequently used projects such as numpy, pandas etc, but this would not work for custom large codebases and may require a significant effort given the large variety of documentation tools (reStructuredText/Sphinx/MyST and extensions).
@krassowski
Copy link
Contributor Author

Please see my reference integration on my fork, here.

@ccordoba12
Copy link
Member

Is it possible to send plain text and Markdown at the same time? The thing is we don't need Markdown in Spyder.

The other alternative would be to add a config option to control what format is used in the server.

@krassowski
Copy link
Contributor Author

No, you cannot send both. A config option should not be needed either; according to the LSP specification if the client does not support a particular MarkupKind it should register their capabilities accordingly, using HoverClientCapabilities.contentFormat, SignatureHelpClientCapabilities.documentationFormat and CompletionClientCapabilities.completionItem.documentationFormat and the server should return response in the format that the client can handle:

export interface HoverClientCapabilities {
	/**
	 * Whether hover supports dynamic registration.
	 */
	dynamicRegistration?: boolean;

	/**
	 * Client supports the following content formats if the content
	 * property refers to a `literal of type MarkupContent`.
	 * The order describes the preferred format of the client.
	 */
	contentFormat?: MarkupKind[];
}
export interface SignatureHelpClientCapabilities {
	/* ... */

	/**
	 * The client supports the following `SignatureInformation`
	 * specific properties.
	 */
	signatureInformation?: {
		/**
		 * Client supports the following content formats for the documentation
		 * property. The order describes the preferred format of the client.
		 */
		documentationFormat?: MarkupKind[];
                /* ... */
        }
       /* ... */
}
export interface CompletionClientCapabilities {
	/* ... */

	/**
	 * The client supports the following `CompletionItem` specific
	 * capabilities.
	 */
	completionItem?: {
                /* ... */

		/**
		 * Client supports the following content formats for the documentation
		 * property. The order describes the preferred format of the client.
		 */
		documentationFormat?: MarkupKind[];
                /* ... */
      }
}

@ccordoba12
Copy link
Member

Ok, sounds good then

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 a pull request may close this issue.

2 participants