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

Documentation query #1319

Closed
voodoos opened this issue Jun 24, 2024 · 6 comments · Fixed by #1336
Closed

Documentation query #1319

voodoos opened this issue Jun 24, 2024 · 6 comments · Fixed by #1336
Assignees
Labels
enhancement New feature or request

Comments

@voodoos
Copy link
Collaborator

voodoos commented Jun 24, 2024

In the objective of restoring classic Merlin workflows in text-based editors we need a custom query to get a symbol's documentation.

Proposed interface:

Client capability:

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

Query:

export interface GetDocParams extends TextDocumentPositionParams
{
    /**
     * An optional identifier. If provided, documentation for this ident 
     * is looked up from the environment at the given position. Else the 
     * server will look for the documentation of the identifier under 
     * the cursor if used.
     */
    identifier?:string;

    /**
     * Optionally override the result's format. 
     */
    contentFormat?:MarkupKind;
}

Response:

result: GetDoc | null
export interface GetDoc {
    /**
     * The documentation
     */
    doc: MarkupContent;

}

We could also have the format choice be made a part of t capability, but I am not sure this is worth the added complexity ?

A response with null result is returned of the identifier doesn't have documentation.
An error is returned if the identifier is invalid.

@voodoos voodoos added the enhancement New feature or request label Jun 24, 2024
@rgrinberg
Copy link
Member

We could also have the format choice be made a part of t capability, but I am not sure this is worth the added complexity ?

You could default to the client capability provided when the client initialized the connection. I'd probably just do that and drop the format argument altogether. I don't really see a concrete use case for it. Perhaps you have one?

@voodoos
Copy link
Collaborator Author

voodoos commented Jun 26, 2024

The goal of this query is to provide more flexibility to text-base editors users and restore some vanila-merlin workflows. The output might be automatically processed, or users might prefer to see the doc in ocamldoc syntax as it is the case today in emacs and vim. (without having to disable markdown all-together).

Following the initially given capabilities by default feels like the right thing to do (I edited my spec), but I think we should provide the option to override it in the query.

@awilliambauer do you have an opinion on this matter ?

@rgrinberg
Copy link
Member

Anyway I don't have a principled objection. I just think it would be a shame if you did the extra work to implement and nobody would end up using it. So I would suggest that you have at least n=1 users willing to use this.

@awilliambauer
Copy link
Contributor

I likewise don't have an objection to an option to overwrite the format specified at initialization, but I do expect a given editor will always want the response in a particular format, and won't need to the ability to override it on the fly.

@voodoos
Copy link
Collaborator Author

voodoos commented Jul 2, 2024

I likewise don't have an objection to an option to overwrite the format specified at initialization, but I do expect a given editor will always want the response in a particular format, and won't need to the ability to override it on the fly.

Well, it's hard to anticipate, but we could imagine editors using the same command to implement multiple features. If we take that query for example we could imagine several usages:

  • Display the documentation to the user. Here having the doc translated to markdown might be a better choice.
  • Copy the documentation, or perform refactoring / analysis, which require the actual source.

Since we write these custom queries to workaround lsp's rigidity I think we should either always return "raw" results or have the possibility to return "raw" results, so that they enable emergent behavior from the editors.

The additional dev. and maintenance time for having it configurable at the query level is very minimal (since we already need that logic to obey the "global" capability) and the result will be more flexible.

You could default to the client capability provided when the client initialized the connection

Having a closer look, other queries individually have capabilities for this. For example completion has documentationFormat?: MarkupKind[]; while hover has contentFormat?: MarkupKind[]; . So it looks like we really should have a capability for our custom query that has an appropriate configuration field. I will update the spec.

Thanks both of you for your feedback!

@PizieDust
Copy link
Contributor

@voodoos thank you for creating this feature request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants