From c85a435d38266525f8bdca8104416665719a7eab Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= <olafurpg@gmail.com>
Date: Fri, 31 May 2024 09:42:34 +0200
Subject: [PATCH] Document Synchronization: stop using logical positions
 (#1714)

As we recently discovered, logical positions interpret character
positions differently than the agent server. For example, in IDEA, the
tab character is interpret as two characters while the agent server
interprets it as one character. This PR replaces usage of logical
positions with manually calculated line/character positions.

## Test plan

Green CI
<!-- All pull requests REQUIRE a test plan:
https://sourcegraph.com/docs/dev/background-information/testing_principles

Why does it matter?

These test plans are there to demonstrate that are following industry
standards which are important or critical for our customers.
They might be read by customers or an auditor. There are meant be simple
and easy to read. Simply explain what you did to ensure
your changes are correct!

Here are a non exhaustive list of test plan examples to help you:

- Making changes on a given feature or component:
- "Covered by existing tests" or "CI" for the shortest possible plan if
there is zero ambiguity
  - "Added new tests"
- "Manually tested" (if non trivial, share some output, logs, or
screenshot)
- Updating docs:
  - "previewed locally"
  - share a screenshot if you want to be thorough
- Updating deps, that would typically fail immediately in CI if
incorrect
  - "CI"
  - "locally tested"
-->
---
 .../agent/protocol/ProtocolTextDocument.kt    | 29 +++++++------------
 .../cody/listeners/CodyCaretListener.kt       |  3 +-
 2 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/src/main/kotlin/com/sourcegraph/cody/agent/protocol/ProtocolTextDocument.kt b/src/main/kotlin/com/sourcegraph/cody/agent/protocol/ProtocolTextDocument.kt
index 6380a45729..14077c72c0 100644
--- a/src/main/kotlin/com/sourcegraph/cody/agent/protocol/ProtocolTextDocument.kt
+++ b/src/main/kotlin/com/sourcegraph/cody/agent/protocol/ProtocolTextDocument.kt
@@ -5,7 +5,7 @@ import com.intellij.codeInsight.codeVision.ui.popup.layouter.right
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.editor.Document
 import com.intellij.openapi.editor.Editor
-import com.intellij.openapi.editor.LogicalPosition
+import com.intellij.openapi.editor.event.CaretEvent
 import com.intellij.openapi.editor.event.DocumentEvent
 import com.intellij.openapi.editor.event.SelectionEvent
 import com.intellij.openapi.fileEditor.FileDocumentManager
@@ -57,19 +57,17 @@ private constructor(
 
     @RequiresEdt
     private fun getSelection(editor: Editor): Range {
-      val selectionModel = editor.selectionModel
-      val beforeStartLines =
-          editor.document.text.substring(0, selectionModel.selectionStart).lines()
-      val beforeEndLines = editor.document.text.substring(0, selectionModel.selectionEnd).lines()
-      return Range(
-          Position(max(0, beforeStartLines.size - 1), beforeStartLines.last().length),
-          Position(max(0, beforeEndLines.size - 1), beforeEndLines.last().length))
+      return editor.document.codyRange(
+          editor.selectionModel.selectionStart, editor.selectionModel.selectionEnd)
     }
 
     @RequiresEdt
     private fun getVisibleRange(editor: Editor): Range {
       val visibleArea = editor.scrollingModel.visibleArea
 
+      // As a rule of thumb, we avoid logical positions because they interpret some characters
+      // creatively (example, tab as two spaces). We use logical positions for "visible range" where
+      // 100% precision is not needed.
       val startOffset = editor.xyToLogicalPosition(visibleArea.location)
       val startOffsetLine = max(startOffset.line, 0)
       val startOffsetColumn = max(startOffset.column, 0)
@@ -84,14 +82,12 @@ private constructor(
 
     @JvmStatic
     @RequiresEdt
-    fun fromEditorWithOffsetSelection(
-        editor: Editor,
-        newPosition: LogicalPosition
-    ): ProtocolTextDocument? {
+    fun fromEditorWithOffsetSelection(editor: Editor, event: CaretEvent): ProtocolTextDocument? {
+      val caret = event.caret ?: return null
       val file = FileDocumentManager.getInstance().getFile(editor.document) ?: return null
-      val position = newPosition.codyPosition()
+      val start = editor.document.codyPosition(caret.offset)
+      val selection = Range(start, start)
       val uri = uriFor(file)
-      val selection = Range(position, position)
       return ProtocolTextDocument(
           uri = uri,
           selection = selection,
@@ -221,8 +217,3 @@ private fun Document.codyRange(startOffset: Int, endOffset: Int): Range {
 
   return Range(Position(startLine, startCharacter), Position(endLine, endCharacter))
 }
-
-// Logical positions are 0-based (!), just like in VS Code.
-private fun LogicalPosition.codyPosition(): Position {
-  return Position(this.line, this.column)
-}
diff --git a/src/main/kotlin/com/sourcegraph/cody/listeners/CodyCaretListener.kt b/src/main/kotlin/com/sourcegraph/cody/listeners/CodyCaretListener.kt
index 8f0b6f956a..93ea10afe0 100644
--- a/src/main/kotlin/com/sourcegraph/cody/listeners/CodyCaretListener.kt
+++ b/src/main/kotlin/com/sourcegraph/cody/listeners/CodyCaretListener.kt
@@ -23,8 +23,7 @@ class CodyCaretListener(val project: Project) : CaretListener {
       return
     }
 
-    ProtocolTextDocument.fromEditorWithOffsetSelection(e.editor, e.newPosition)?.let { textDocument
-      ->
+    ProtocolTextDocument.fromEditorWithOffsetSelection(e.editor, e)?.let { textDocument ->
       EditorChangesBus.documentChanged(project, textDocument)
       CodyAgentService.withAgent(project) { agent: CodyAgent ->
         agent.server.textDocumentDidChange(textDocument)