Skip to content

Commit

Permalink
Document Synchronization: stop using logical positions (#1714)
Browse files Browse the repository at this point in the history
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"
-->
  • Loading branch information
olafurpg authored May 31, 2024
1 parent 4defc6b commit c85a435
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit c85a435

Please sign in to comment.