From dda8ffdde305fcb4f07f8accc5b786fd62ed39a4 Mon Sep 17 00:00:00 2001
From: PizieDust <playersrebirth@gmail.com>
Date: Wed, 18 Sep 2024 13:31:59 +0200
Subject: [PATCH] return a single position instead of a range

---
 .../docs/ocamllsp/merlinJump-spec.md          | 17 ++++--------
 .../src/custom_requests/req_merlin_jump.ml    | 22 +++++++---------
 ocaml-lsp-server/test/e2e-new/merlin_jump.ml  | 26 ++++++-------------
 3 files changed, 22 insertions(+), 43 deletions(-)

diff --git a/ocaml-lsp-server/docs/ocamllsp/merlinJump-spec.md b/ocaml-lsp-server/docs/ocamllsp/merlinJump-spec.md
index be4ab8326..0e5145611 100644
--- a/ocaml-lsp-server/docs/ocamllsp/merlinJump-spec.md
+++ b/ocaml-lsp-server/docs/ocamllsp/merlinJump-spec.md
@@ -29,21 +29,14 @@ export interface JumpParams extends TextDocumentPositionParams
 
 ```js
 result: Jump | String
-export interface Jump extends Location {
-    uri: string;
-    range: {
-        start: Position;
-        end: Position;
-    }
+export interface Jump extends TextDocumentPositionParams {
 }
 ```
 
 - result:
     - Type: Jump or string
-    - Description: If the jump is successful, a list of locations is returned where the first location is the most relevant one. If no relevant jump location is found, the result will be a string "no matching target" or an error message.
+    - Description: If the jump is successful, a position and document path is returned. If no relevant jump location is found, the result will be a string "no matching target" or an error message.
     - Jump:
-        - Type: Location[], A list of Location objects representing the potential targets of the jump.
-            - Location:
-                - uri: The URI of the document where the jump target is located.
-                - range: The range within the document where the jump target is located. Both start and end positions are the same as the jump target location.
-    The Location type is the same type returned by `Goto` requests such as `goto-definition`, `goto-declaration` and `goto-typeDefinition`.
+        - Type: TextDocumentPositionParams
+            - `Position`: The position to jump to
+            - `TextDocumentIdentifier`: the document path which contains this position (ideally the same document as the request)
diff --git a/ocaml-lsp-server/src/custom_requests/req_merlin_jump.ml b/ocaml-lsp-server/src/custom_requests/req_merlin_jump.ml
index 90ae35dae..a036e7ab0 100644
--- a/ocaml-lsp-server/src/custom_requests/req_merlin_jump.ml
+++ b/ocaml-lsp-server/src/custom_requests/req_merlin_jump.ml
@@ -31,17 +31,10 @@ module JumpParams = struct
 end
 
 module Jump = struct
-  type t = [ `Location of Location.t list ]
+  type t = Lsp.Types.TextDocumentPositionParams.t
 
-  let yojson_of_t t = `List (List.map ~f:Location.yojson_of_t t)
-
-  let t_of_yojson json =
-    match json with
-    | `List lst ->
-      let locations = List.map ~f:Location.t_of_yojson lst in
-      `Location locations
-    | _ -> failwith "Invalid JSON for Jump.t"
-  ;;
+  let yojson_of_t t = TextDocumentPositionParams.yojson_of_t t
+  let t_of_yojson json = Lsp.Types.TextDocumentPositionParams.t_of_yojson json
 end
 
 type t = Jump.t
@@ -78,7 +71,10 @@ let on_request ~params state =
           (match Position.of_lexical_position pos with
            | None -> Fiber.return `Null
            | Some position ->
-             let range = { Range.start = position; end_ = position } in
-             let locs = [ { Location.range; uri } ] in
-             Fiber.return (Jump.yojson_of_t locs))))
+             let loc =
+               Lsp.Types.TextDocumentPositionParams.create
+                 ~position
+                 ~textDocument:(TextDocumentIdentifier.create ~uri)
+             in
+             Fiber.return (Jump.yojson_of_t loc))))
 ;;
diff --git a/ocaml-lsp-server/test/e2e-new/merlin_jump.ml b/ocaml-lsp-server/test/e2e-new/merlin_jump.ml
index 58aa5b358..55bcb1227 100644
--- a/ocaml-lsp-server/test/e2e-new/merlin_jump.ml
+++ b/ocaml-lsp-server/test/e2e-new/merlin_jump.ml
@@ -45,15 +45,10 @@ match x with
   Util.test ~line ~character ~target ~source;
   [%expect
     {|
-  [
-    {
-      "range": {
-        "end": { "character": 2, "line": 4 },
-        "start": { "character": 2, "line": 4 }
-      },
-      "uri": "file:///test.ml"
-    }
-  ] |}]
+  {
+    "position": { "character": 2, "line": 4 },
+    "textDocument": { "uri": "file:///test.ml" }
+  } |}]
 ;;
 
 let%expect_test "Get location of a the module" =
@@ -83,15 +78,10 @@ end|}
   Util.test ~line ~character ~target ~source;
   [%expect
     {|
-  [
-    {
-      "range": {
-        "end": { "character": 2, "line": 7 },
-        "start": { "character": 2, "line": 7 }
-      },
-      "uri": "file:///test.ml"
-    }
-  ] |}]
+  {
+    "position": { "character": 2, "line": 7 },
+    "textDocument": { "uri": "file:///test.ml" }
+  } |}]
 ;;
 
 let%expect_test "Same line should output no locations" =