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 custom query #1336

Merged
merged 23 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@

- Add custom [`ocamllsp/typeEnclosing`](https://github.com/ocaml/ocaml-lsp/blob/109801e56f2060caf4487427bede28b824f4f1fe/ocaml-lsp-server/docs/ocamllsp/typeEnclosing-spec.md) request (#1304)

- Add custom [`ocamllsp/getDocumentation`](/ocaml-lsp-server/docs/ocamllsp/getDocumentation-spec.md) request (#1336)


## Fixes

Expand Down
48 changes: 48 additions & 0 deletions ocaml-lsp-server/docs/ocamllsp/getDocumentation-spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Documentation Request

## Description

This custom request allows `odoc` documentation to be gotten without using Hover.

## Client capability

```js
export interface GetDocClientCapabilities {
contentFormat: MarkupKind[];
}
```
- `contentFormat`: 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.

## Server capability

- property name: `handleDocumentation`
- property type: `boolean`


## Request

```js
export interface GetDocParams extends TextDocumentPositionParams
{
identifier?: string;
contentFormat?:MarkupKind;
}
```
- method : `ocamllsp/getDocumentation`
- params :
- `position`: The position of the cursor.
Copy link
Contributor

Choose a reason for hiding this comment

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

position does not appear in the object type above - is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, since the interface itself extends TextDocumentPositionParams which contains both the Document Uri and the Position

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can probably provide a link to the TextDocumentPositionParams and remove the description of position (since textDocument, the TextDocumentIdentifier is not mentionned in the documentation).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure that it is right. I think that you should explain that the param should be a TextDocumentPositionParams (with a link to the specification) with additonal ones, enumerated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xvw I just pushed a commit with some documentation changes. Can you check if it's more structured?

- `identifier`: 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.
- `contentFormat`: Optionally override the result's format. Could be `Plaintext` or `Markdown`.

## Response

```js
result: GetDoc | null
export interface GetDoc {
doc: MarkupContent;
}

```
- `doc`: The documentation found
- A response with null result is returned if the identifier doesn't have documentation.
- An error is returned if the identifier is invalid.
1 change: 1 addition & 0 deletions ocaml-lsp-server/src/custom_requests/custom_request.ml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ module Switch_impl_intf = Req_switch_impl_intf
module Typed_holes = Req_typed_holes
module Type_enclosing = Req_type_enclosing
module Wrapping_ast_node = Req_wrapping_ast_node
module Documentation = Req_documentation
PizieDust marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions ocaml-lsp-server/src/custom_requests/custom_request.mli
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ module Switch_impl_intf = Req_switch_impl_intf
module Typed_holes = Req_typed_holes
module Type_enclosing = Req_type_enclosing
module Wrapping_ast_node = Req_wrapping_ast_node
module Documentation = Req_documentation
112 changes: 112 additions & 0 deletions ocaml-lsp-server/src/custom_requests/req_documentation.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
open Import
module TextDocumentPositionParams = Lsp.Types.TextDocumentPositionParams

let meth = "ocamllsp/getDocumentation"
let capability = "handleDocumentation", `Bool true
PizieDust marked this conversation as resolved.
Show resolved Hide resolved

module GetDocClientCapabilities = struct
type _t = { contentFormat : MarkupKind.t list }

let _yojson_of_t { contentFormat } =
`Assoc
(`List (List.map ~f:(fun format -> MarkupKind.yojson_of_t format) contentFormat))
;;
end

module GetDocParams = struct
type t =
{ text_document : TextDocumentIdentifier.t
; position : Position.t
; identifier : string option
; contentFormat : MarkupKind.t option
}

let t_of_yojson json =
let open Yojson.Safe.Util in
let textDocumentPosition = Lsp.Types.TextDocumentPositionParams.t_of_yojson json in
let identifier = json |> member "identifier" |> to_option to_string in
let contentFormat =
json |> member "contentFormat" |> to_option MarkupKind.t_of_yojson
in
{ position = textDocumentPosition.position
; text_document = textDocumentPosition.textDocument
; identifier
; contentFormat
}
;;

let yojson_of_t { text_document; position; identifier; contentFormat } =
let identifier =
match identifier with
| Some ident -> [ "identifier", `String ident ]
| None -> []
in
let contentFormat =
match contentFormat with
| Some format -> [ "contentFormat", MarkupKind.yojson_of_t format ]
| None -> []
in
`Assoc
((("textDocument", TextDocumentIdentifier.yojson_of_t text_document)
:: ("position", Position.yojson_of_t position)
:: identifier)
@ contentFormat)
;;
end

module GetDoc = struct
type t = { doc : MarkupContent.t }

let yojson_of_t { doc } = `Assoc [ "doc", MarkupContent.yojson_of_t doc ]

let t_of_yojson json =
let open Yojson.Safe.Util in
let doc = json |> member "doc" |> MarkupContent.t_of_yojson in
{ doc }
;;

let create ~kind ~value = MarkupContent.create ~kind ~value
end

type t = GetDoc.t

let t_of_yojson json = GetDoc.t_of_yojson json

module Request_params = struct
type t = GetDocParams.t

let yojson_of_t t = GetDocParams.yojson_of_t t

let create ~text_document ~position ?(identifier = None) ?(contentFormat = None) () : t =
{ text_document; identifier; contentFormat; position }
;;
end

let dispatch ~merlin ~position ~identifier ~contentFormat =
Document.Merlin.with_pipeline_exn merlin (fun pipeline ->
let position = Position.logical position in
let query = Query_protocol.Document (identifier, position) in
let result = Query_commands.dispatch pipeline query in
match result with
| `No_documentation | `Invalid_context | `Not_found _ -> `Null
| `Builtin value | `File_not_found value | `Found value | `Not_in_env value ->
let kind =
match contentFormat with
| Some format -> format
| None -> MarkupKind.PlainText
in
GetDoc.yojson_of_t { doc = GetDoc.create ~kind ~value })
;;

let on_request ~params state =
Fiber.of_thunk (fun () ->
let params = (Option.value ~default:(`Assoc []) params :> Yojson.Safe.t) in
let GetDocParams.{ text_document; position; identifier; contentFormat } =
GetDocParams.t_of_yojson params
in
let uri = text_document.uri in
let doc = Document_store.get state.State.store uri in
match Document.kind doc with
| `Other -> Fiber.return `Null
| `Merlin merlin -> dispatch ~merlin ~position ~identifier ~contentFormat)
;;
22 changes: 22 additions & 0 deletions ocaml-lsp-server/src/custom_requests/req_documentation.mli
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
open Import

module Request_params : sig
type t

val yojson_of_t : t -> Json.t

val create
: text_document:TextDocumentIdentifier.t
-> position:Position.t
-> ?identifier:string option
-> ?contentFormat:MarkupKind.t option
-> unit
-> t
end

type t

val t_of_yojson : Json.t -> t
val meth : string
val capability : string * [> `Bool of bool ]
val on_request : params:Jsonrpc.Structured.t option -> State.t -> Json.t Fiber.t
2 changes: 2 additions & 0 deletions ocaml-lsp-server/src/ocaml_lsp_server.ml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ let initialize_info (client_capabilities : ClientCapabilities.t) : InitializeRes
; Req_hover_extended.capability
; Req_merlin_call_compatible.capability
; Req_type_enclosing.capability
; Req_documentation.capability
] )
]
in
Expand Down Expand Up @@ -507,6 +508,7 @@ let on_request
; Req_typed_holes.meth, Req_typed_holes.on_request
; Req_merlin_call_compatible.meth, Req_merlin_call_compatible.on_request
; Req_type_enclosing.meth, Req_type_enclosing.on_request
; Req_documentation.meth, Req_documentation.on_request
; Req_wrapping_ast_node.meth, Req_wrapping_ast_node.on_request
; ( Semantic_highlighting.Debug.meth_request_full
, Semantic_highlighting.Debug.on_request_full )
Expand Down
167 changes: 167 additions & 0 deletions ocaml-lsp-server/test/e2e-new/documentation.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
open Test.Import
module Req = Ocaml_lsp_server.Custom_request.Documentation

module Util = struct
let call_documentation ~position ?(identifier = None) ?(contentFormat = None) client =
let uri = DocumentUri.of_path "test.ml" in
let text_document = TextDocumentIdentifier.create ~uri in
let params =
Req.Request_params.create ~text_document ~position ~identifier ~contentFormat ()
|> Req.Request_params.yojson_of_t
|> Jsonrpc.Structured.t_of_yojson
|> Option.some
in
let req =
Lsp.Client_request.UnknownRequest { meth = "ocamllsp/getDocumentation"; params }
in
Client.request client req
;;

let print_documentation result =
PizieDust marked this conversation as resolved.
Show resolved Hide resolved
result |> Yojson.Safe.pretty_to_string ~std:false |> print_endline
;;

let test ~line ~character ?identifier ?contentFormat source =
let position = Position.create ~character ~line in
let contentFormat =
match contentFormat with
| Some "markdown" -> Some MarkupKind.Markdown
| Some "plaintext" | _ -> Some MarkupKind.PlainText
in
let request client =
let open Fiber.O in
let+ response = call_documentation ~position ~identifier ~contentFormat client in
print_documentation response
in
Helpers.test source request
;;
end

let%expect_test "Documentation of simple type with no contentFormat and no identifier" =
let source = "type tree (** This is a comment *)" in
let line = 0 in
let character = 7 in
Util.test ~line ~character source;
[%expect {| { "doc": { "kind": "plaintext", "value": "This is a comment" } } |}]
;;

let%expect_test "Documentation of simple type with contentFormat set to markdown" =
let source = "type tree (** This is another comment *)" in
let line = 0 in
let character = 7 in
let contentFormat = "markdown" in
Util.test ~line ~character ~contentFormat source;
[%expect {| { "doc": { "kind": "markdown", "value": "This is another comment" } } |}]
;;

let%expect_test "Documentation of simple type with an identifier and contentFormat" =
let source =
"{|type tree (** This is another comment *)\n List.iter ~f:(fun x -> x*x) [2;4]|}"
in
let line = 0 in
let character = 7 in
let identifier = "List" in
let contentFormat = "markdown" in
Util.test ~line ~character ~identifier ~contentFormat source;
[%expect
{|
{
"doc": {
"kind": "markdown",
"value": "List operations.\n\n Some functions are flagged as not tail-recursive. A tail-recursive\n function uses constant stack space, while a non-tail-recursive function\n uses stack space proportional to the length of its list argument, which\n can be a problem with very long lists. When the function takes several\n list arguments, an approximate formula giving stack usage (in some\n unspecified constant unit) is shown in parentheses.\n\n The above considerations can usually be ignored if your lists are not\n longer than about 10000 elements.\n\n The labeled version of this module can be used as described in the\n {!StdLabels} module."
}
} |}]
;;

let%expect_test "Documentation of simple type with an identifier and no contentFormat" =
let source =
"{|type tree (** This is another comment *)\n List.iter ~f:(fun x -> x*x) [2;4]|}"
in
let line = 0 in
let character = 7 in
let identifier = "List" in
Util.test ~line ~character ~identifier source;
[%expect
{|
{
"doc": {
"kind": "plaintext",
"value": "List operations.\n\n Some functions are flagged as not tail-recursive. A tail-recursive\n function uses constant stack space, while a non-tail-recursive function\n uses stack space proportional to the length of its list argument, which\n can be a problem with very long lists. When the function takes several\n list arguments, an approximate formula giving stack usage (in some\n unspecified constant unit) is shown in parentheses.\n\n The above considerations can usually be ignored if your lists are not\n longer than about 10000 elements.\n\n The labeled version of this module can be used as described in the\n {!StdLabels} module."
}
} |}]
xvw marked this conversation as resolved.
Show resolved Hide resolved
;;

let%expect_test "Documentation when List module is shadowed" =
let source =
"{|\n\
List.iter ~f:(fun x -> x*x) [2;4]\n\
\ module List = struct\n\
\ (** This is my custom list module *)\n\
\ let rec iter ~f = function (** This is the custom iter module *)\n\
\ | [] -> () (** This is when the list is empty *)\n\
\ | x :: xs -> f x; iter ~f xs\n\
end\n\
List.iter ~f:(fun x -> x*x) [2;4]\n\
|}"
in
let line = 2 in
let character = 6 in
let identifier = "List.iter" in
Util.test ~line ~character ~identifier source;
[%expect
{|
{
"doc": {
"kind": "plaintext",
"value": "[iter f [a1; ...; an]] applies function [f] in turn to\n [a1; ...; an]. It is equivalent to\n [begin f a1; f a2; ...; f an; () end]."
}
} |}]
;;

let%expect_test "Documentation when List module is shadowed" =
let source =
"{|\n\
List.iter ~f:(fun x -> x*x) [2;4]\n\
module List = struct\n\
\ (** This is my custom list module *)\n\
\ let rec iter ~f = function (** This is the custom iter module *)\n\
\ | [] -> () (** This is when the list is empty *)\n\
\ | x :: xs -> f x; iter ~f xs\n\
end\n\
Base.List.iter ~f:(fun x -> x*x) [2;4]\n\
|}"
in
let line = 7 in
let character = 12 in
let identifier = "Base.List.iter" in
Util.test ~line ~character ~identifier source;
[%expect {|
{ "doc": { "kind": "plaintext", "value": "Base.List.iter" } } |}]
;;

(* TODO: Open Issue in Merlin to investigate while this doesnt return documentation of the custom List module*)
let%expect_test "Documentation when List module is shadowed" =
let source =
"{|\n\
List.iter ~f:(fun x -> x*x) [2;4]\n\
module List = struct\n\
\ (** This is my custom list module *)\n\
\ let rec iter ~f = function (** This is the custom iter module *)\n\
\ | [] -> () (** This is when the list is empty *)\n\
\ | x :: xs -> f x; iter ~f xs\n\
end\n\
Base.List.iter ~f:(fun x -> x*x) [2;4]\n\
|}"
in
let line = 2 in
let character = 9 in
Util.test ~line ~character source;
[%expect
{|
{
"doc": {
"kind": "plaintext",
"value": "List operations.\n\n Some functions are flagged as not tail-recursive. A tail-recursive\n function uses constant stack space, while a non-tail-recursive function\n uses stack space proportional to the length of its list argument, which\n can be a problem with very long lists. When the function takes several\n list arguments, an approximate formula giving stack usage (in some\n unspecified constant unit) is shown in parentheses.\n\n The above considerations can usually be ignored if your lists are not\n longer than about 10000 elements.\n\n The labeled version of this module can be used as described in the\n {!StdLabels} module."
}
} |}]
;;
1 change: 1 addition & 0 deletions ocaml-lsp-server/test/e2e-new/dune
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
syntax_doc_tests
test
type_enclosing
documentation
with_pp
with_ppx
workspace_change_config))))
Loading
Loading