bugfix: Fix issues with select in debugger completions

Previously, we set the wrong start of the expression since we actually need to calculate that when insertText is used. Now, we calculate it based on the current identifier. We should use TextEdit in the future, which should work better. I also realized that start actually needs to take into account if the client starts at 1 or 0.
scalameta · Aug 8, 2023 · a1ecf2c · a1ecf2c
1 parent 0660721
commit a1ecf2c
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 5 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala
@@ -288,11 +288,22 @@ class Compilers(
       Nil
     }
 
+  /**
+   * Calculates completions for a expression evaluator at breakpointPosition
+   *
+   * @param path path to file containing ht ebreakpoint
+   * @param breakpointPosition actual breakpoint position
+   * @param token cancel token for the compiler
+   * @param expression expression that is currently being types
+   * @param isZeroBased whether the client supports starting at 0 or 1 index
+   * @return
+   */
   def debugCompletions(
       path: AbsolutePath,
       breakpointPosition: LspPosition,
       token: CancelToken,
       expression: d.CompletionsArguments,
+      isZeroBased: Boolean,
   ): Future[Seq[d.CompletionItem]] = {
 
     /**
@@ -342,6 +353,16 @@ class Compilers(
 
             val rangeEnd =
               lineStart + expressionOffset(expressionText, indentation) + 1
+
+            /**
+             * Calculate the start if insertText is used for item, which does not declare an exact start.
+             */
+            def insertStart = {
+              var i = rangeEnd - 1
+              while (modified.charAt(i).isLetterOrDigit) i -= 1
+              if (isZeroBased) i else i + 1
+            }
+
             val offsetParams = CompilerOffsetParams(
               path.toURI,
               modified,
@@ -377,8 +398,7 @@ class Compilers(
                       adjustStart,
                       Position.Range(
                         input.copy(value = modified),
-                        // account for the added ;
-                        lineStart + 1,
+                        insertStart,
                         rangeEnd,
                       ),
                     )
@@ -1140,7 +1160,7 @@ class Compilers(
   private def toDebugCompletionItem(
       item: CompletionItem,
       adjustStart: Int,
-      insertTextPosition: Position,
+      insertTextPosition: Position.Range,
   ): d.CompletionItem = {
     val debugItem = new d.CompletionItem()
     debugItem.setLabel(item.getLabel())
@@ -1174,6 +1194,7 @@ class Compilers(
       debugItem.setSelectionStart(selection)
     }
 
+    debugItem.setDetail(item.getDetail())
     debugItem.setText(fullText.replace("$0", ""))
     debugItem.setStart(start)
     debugItem.setType(toDebugCompletionType(item.getKind()))

diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/ClientConfigurationAdapter.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/ClientConfigurationAdapter.scala
@@ -20,7 +20,7 @@ import org.eclipse.lsp4j.debug.SourceBreakpoint
  */
 private[debug] final case class ClientConfigurationAdapter(
     pathFormat: String,
-    linesStartAt1: Boolean,
+    val linesStartAt1: Boolean,
     sourceMapper: SourceMapper,
 ) {
   // The scala-debug-adapter uses the JVM class file format

diff --git a/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala b/metals/src/main/scala/scala/meta/internal/metals/debug/DebugProxy.scala
@@ -135,6 +135,7 @@ private[debug] final class DebugProxy(
           new Position(frame.getLine() - 1, 0),
           EmptyCancelToken,
           args,
+          isZeroBased = !clientAdapter.linesStartAt1,
         )
       }
       completions

diff --git a/tests/unit/src/test/scala/tests/debug/CompletionDapSuite.scala b/tests/unit/src/test/scala/tests/debug/CompletionDapSuite.scala
@@ -250,7 +250,7 @@ class CompletionDapSuite
            |scale(int arg0, float arg1)
            |startsWith(java.lang.String arg0, int arg1)
            |""".stripMargin,
-    expectedEdit = "serialVersionUID",
+    expectedEdit = "name.serialVersionUID",
     topLines = Some(5),
   )(
     """|/a/src/main/java/a/Main.java