Skip to content

Commit

Permalink
Completions should prepend, not replace as it is for Scala 2 (#18803)
Browse files Browse the repository at this point in the history
By default, in Scala 2 completion prepend and only replace if there is
exact match. This PR unifies the experience between the Scala versions.
It seems more intuitive to work that way.

The change will be as follows (the order is: new logic, old logic and
scala 2 logic):


https://github.com/lampepfl/dotty/assets/48657087/c037c322-5613-4b95-a6e5-090b4e8827b6


In the future, it should be improved by changing implementation to use
`InsertReplaceEdit` instead of `TextEdit`. This will allow users to
decide which behaviour they prefer, but this has to be first implemented
in metals to properly handle the new logic, but for now the key is to
unify behaviours.
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#insertReplaceEdit
  • Loading branch information
rochala committed Nov 2, 2023
1 parent e399c43 commit dc1e35a
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ case class CompletionPos(
):

def sourcePos: SourcePosition = cursorPos.withSpan(Spans.Span(start, end))
def stripSuffixEditRange: l.Range = new l.Range(cursorPos.offsetToPos(start), cursorPos.offsetToPos(end))
def toEditRange: l.Range = cursorPos.withStart(start).withEnd(cursorPos.point).toLsp

def toEditRange: l.Range =
new l.Range(cursorPos.offsetToPos(start), cursorPos.offsetToPos(end))
end toEditRange
end CompletionPos

object CompletionPos:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,7 @@ class CompletionProvider(
indexedContext: IndexedContext
)(using ctx: Context): CompletionItem =
val printer =
ShortenedTypePrinter(search, IncludeDefaultParam.ResolveLater)(using
indexedContext
)
val editRange = completionPos.toEditRange
ShortenedTypePrinter(search, IncludeDefaultParam.ResolveLater)(using indexedContext)

// For overloaded signatures we get multiple symbols, so we need
// to recalculate the description
Expand All @@ -165,24 +162,22 @@ class CompletionProvider(
val ident = completion.insertText.getOrElse(completion.label)

def mkItem(
insertText: String,
newText: String,
additionalEdits: List[TextEdit] = Nil,
range: Option[LspRange] = None
): CompletionItem =
val nameEdit = new TextEdit(
range.getOrElse(editRange),
insertText
)
val oldText = params.text.substring(completionPos.start, completionPos.end)
val editRange = if newText.startsWith(oldText) then completionPos.stripSuffixEditRange
else completionPos.toEditRange

val textEdit = new TextEdit(range.getOrElse(editRange), newText)

val item = new CompletionItem(label)
item.setSortText(f"${idx}%05d")
item.setDetail(description)
item.setFilterText(
completion.filterText.getOrElse(completion.label)
)
item.setTextEdit(nameEdit)
item.setAdditionalTextEdits(
(completion.additionalEdits ++ additionalEdits).asJava
)
item.setFilterText(completion.filterText.getOrElse(completion.label))
item.setTextEdit(textEdit)
item.setAdditionalTextEdits((completion.additionalEdits ++ additionalEdits).asJava)
completion.insertMode.foreach(item.setInsertTextMode)

completion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1500,3 +1500,47 @@ class CompletionSuite extends BaseCompletionSuite:
|""".stripMargin,
)

@Test def `prepend-instead-of-replace` =
checkEdit(
"""|object O:
| printl@@println()
|""".stripMargin,
"""|object O:
| printlnprintln()
|""".stripMargin,
assertSingleItem = false
)

@Test def `prepend-instead-of-replace-duplicate-word` =
checkEdit(
"""|object O:
| println@@println()
|""".stripMargin,
"""|object O:
| printlnprintln()
|""".stripMargin,
assertSingleItem = false
)

@Test def `replace-when-inside` =
checkEdit(
"""|object O:
| print@@ln()
|""".stripMargin,
"""|object O:
| println()
|""".stripMargin,
assertSingleItem = false
)

@Test def `replace-exact-same` =
checkEdit(
"""|object O:
| println@@()
|""".stripMargin,
"""|object O:
| println()
|""".stripMargin,
assertSingleItem = false
)

0 comments on commit dc1e35a

Please sign in to comment.