From b551ab26d932f3733ba5d1ab1f38234f9c2326a0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulysse=20G=C3=A9rard?= <thevoodoos@gmail.com>
Date: Thu, 2 May 2024 16:27:44 +0200
Subject: [PATCH] context: improve cursor position detection

---
 CHANGES.md                                   |  3 +-
 src/analysis/context.ml                      | 14 +++++++--
 src/ocaml/parsing/pprintast.mli              |  1 +
 tests/test-dirs/type-enclosing/need-parens.t | 32 +++++++++++++++-----
 4 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 901ca0b628..c584885244 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -3,9 +3,10 @@ merlin NEXT_VERSION
 
   + merlin binary
     - destruct: Removal of residual patterns (#1737, fixes #1560)
-    - Do not erase fields' names when destructing punned record fields (#1734, 
+    - Do not erase fields' names when destructing punned record fields (#1734,
       fixes #1661)
     - Ignore SIGPIPE in the Merlin server process (#1746)
+    - Improve cursor position detection in longidents (#1756)
 
 merlin 4.14
 ===========
diff --git a/src/analysis/context.ml b/src/analysis/context.ml
index 30806301c0..7fba149868 100644
--- a/src/analysis/context.ml
+++ b/src/analysis/context.ml
@@ -64,8 +64,17 @@ let cursor_on_longident_end
   match lid with
   | Longident.Lident _ -> true
   | _ ->
-    let end_offset = loc.loc_end.pos_cnum in
-    let cstr_name_size = String.length name in
+    let end_offset =
+      loc.loc_end.pos_cnum in
+    let cstr_name_size =
+      (* FIXME: this is britle, but lids don't have precise enough location
+         information to handle these cases correctly. *)
+      let name_lenght = String.length name in
+      if Pprintast.needs_parens name then
+        name_lenght + 2
+      else
+        name_lenght
+    in
     let constr_pos =
       { loc.loc_end
         with pos_cnum = end_offset - cstr_name_size }
@@ -107,6 +116,7 @@ let inspect_expression ~cursor ~lid e : t =
     else Module_path
   | Texp_ident (p, lid_loc, _) ->
     let name = Path.last p in
+    log ~title:"inspect_context" "name is: [%s]" name;
     if name = "*type-error*" then
       (* For type_enclosing: it is enough to return Module_path here.
          - If the cursor was on the end of the lid typing should fail anyway
diff --git a/src/ocaml/parsing/pprintast.mli b/src/ocaml/parsing/pprintast.mli
index 4ceb5bbbb9..c9f5393dc2 100644
--- a/src/ocaml/parsing/pprintast.mli
+++ b/src/ocaml/parsing/pprintast.mli
@@ -57,3 +57,4 @@ val tyvar: Format.formatter -> string -> unit
 (* merlin *)
 val case_list : Format.formatter -> Parsetree.case list -> unit
 val protect_ident : Format.formatter -> string -> unit
+val needs_parens : string -> bool
diff --git a/tests/test-dirs/type-enclosing/need-parens.t b/tests/test-dirs/type-enclosing/need-parens.t
index 42a2927765..9e7d310097 100644
--- a/tests/test-dirs/type-enclosing/need-parens.t
+++ b/tests/test-dirs/type-enclosing/need-parens.t
@@ -1,10 +1,16 @@
-FIXME: locate on `M.(|+)` should work:
+Locate on `M.(|+)` should work:
   $ $MERLIN single locate -position 2:11 -filename test.ml <<'EOF' | \
   > jq '.value'
   > module M = struct let (+) a b = a + b end
   > let _ = M.(+)
   > EOF
-  "Not in environment 'M.+'"
+  {
+    "file": "test.ml",
+    "pos": {
+      "line": 1,
+      "col": 22
+    }
+  }
 
 Locate on `M.(+|)` should work:
   $ $MERLIN single locate -position 2:12 -filename test.ml <<'EOF' | \
@@ -21,7 +27,7 @@ Locate on `M.(+|)` should work:
   }
 
 And need spaces:
-FIXME: locate on `M.(| * )` should work:
+Locate on `M.(| * )` should work:
   $ $MERLIN single locate -position 2:11 -filename test.ml <<'EOF' | \
   > jq '.value'
   > module M = struct let ( * ) a b = a + b end
@@ -36,19 +42,31 @@ FIXME: locate on `M.(| * )` should work:
   }
 
 And need spaces:
-FIXME: locate on `M.( |* )` should work:
+Locate on `M.( |* )` should work:
   $ $MERLIN single locate -position 2:12 -filename test.ml <<'EOF' | \
   > jq '.value'
   > module M = struct let ( * ) a b = a + b end
   > let _ = M.( * )
   > EOF
-  "Not in environment 'M.*'"
+  {
+    "file": "test.ml",
+    "pos": {
+      "line": 1,
+      "col": 22
+    }
+  }
 
 And need spaces:
-FIXME: locate on `M.( *| )` should work:
+Locate on `M.( *| )` should work:
   $ $MERLIN single locate -position 2:13 -filename test.ml <<'EOF' | \
   > jq '.value'
   > module M = struct let ( * ) a b = a + b end
   > let _ = M.( * )
   > EOF
-  "Not in environment 'M.*'"
+  {
+    "file": "test.ml",
+    "pos": {
+      "line": 1,
+      "col": 22
+    }
+  }