ocaml / ocaml-lsp

OCaml Language Server Protocol implementation
Other
749 stars 117 forks source link

Documentation query #1319

Closed voodoos closed 1 month ago

voodoos commented 2 months ago

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.

rgrinberg commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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:

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 commented 2 months ago

@voodoos thank you for creating this feature request.