-
Notifications
You must be signed in to change notification settings - Fork 395
Respect client capabilities for completion item snippets #1057
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,9 @@ import scala.meta.internal.tokenizers.Chars | |
| */ | ||
| trait Completions { this: MetalsGlobal => | ||
|
|
||
| val clientSupportsSnippets: Boolean = | ||
| metalsConfig.isCompletionSnippetsEnabled() | ||
|
|
||
| /** | ||
| * A member for symbols on the classpath that are not in scope, produced via workspace/symbol. | ||
| */ | ||
|
|
@@ -765,11 +768,11 @@ trait Completions { this: MetalsGlobal => | |
| if (text.charAt(lit.pos.start - 1) != 's') | ||
| List(new l.TextEdit(lit.pos.withEnd(lit.pos.start).toLSP, "s")) | ||
| else Nil | ||
| val dolarEdits = for { | ||
| val dollarEdits = for { | ||
| i <- lit.pos.start to (lit.pos.end - CURSOR.length()) | ||
| if text.charAt(i) == '$' && i != interpolator.dollar | ||
| } yield new l.TextEdit(pos.source.position(i).withEnd(i).toLSP, "$") | ||
| interpolatorEdit ++ dolarEdits | ||
| interpolatorEdit ++ dollarEdits | ||
| } | ||
|
|
||
| def newText(sym: Symbol): String = { | ||
|
|
@@ -793,6 +796,7 @@ trait Completions { this: MetalsGlobal => | |
|
|
||
| val filter: String = | ||
| text.substring(lit.pos.start, pos.point - interpolator.name.length) | ||
|
|
||
| override def contribute: List[Member] = { | ||
| metalsScopeMembers(pos).collect { | ||
| case s: ScopeMember | ||
|
|
@@ -985,7 +989,7 @@ trait Completions { this: MetalsGlobal => | |
| allParams.exists(param => param.name.startsWith(prefix)) | ||
| val isExplicitlyCalled = suffix.startsWith(prefix) | ||
| val hasParamsToFill = allParams.count(!_.hasDefault) > 1 | ||
| if ((shouldShow || isExplicitlyCalled) && hasParamsToFill) { | ||
| if ((shouldShow || isExplicitlyCalled) && hasParamsToFill && clientSupportsSnippets) { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this disables the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's disable it for now, I will think it over to see if it makes sense without the snippets. |
||
| val editText = allParams.zipWithIndex | ||
| .collect { | ||
| case (param, index) if !param.hasDefault => | ||
|
|
@@ -1202,7 +1206,11 @@ trait Completions { this: MetalsGlobal => | |
| private def signature = printer.defaultMethodSignature() | ||
| private def edit = new l.TextEdit( | ||
| range, | ||
| s"$filterText$signature = $${0:???}" | ||
| if (clientSupportsSnippets) { | ||
| s"$filterText$signature = $${0:???}" | ||
| } else { | ||
| s"$filterText$signature = ???" | ||
| } | ||
| ) | ||
| } | ||
|
|
||
|
|
@@ -1259,7 +1267,11 @@ trait Completions { this: MetalsGlobal => | |
| "match", | ||
| new l.TextEdit( | ||
| editRange, | ||
| "match {\n\tcase$0\n}" | ||
| if (clientSupportsSnippets) { | ||
| "match {\n\tcase$0\n}" | ||
| } else { | ||
| "match" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. avoid the braces if we have no snippets. Inserting braces will result in a cursor out of place ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my point of view, it is bad to end up with
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for the feedback! I'm actively avoiding that pattern, except in some cases where the completion is still very useful and/or it's opt-in (e.g. it's not the default case) |
||
| } | ||
| ), | ||
| completionsSymbol("match"), | ||
| label = Some("match"), | ||
|
|
@@ -1273,7 +1285,11 @@ trait Completions { this: MetalsGlobal => | |
| tail | ||
| .map(_.edit.getNewText()) | ||
| .mkString( | ||
| s"match {\n\t${head.edit.getNewText} $$0\n\t", | ||
| if (clientSupportsSnippets) { | ||
| s"match {\n\t${head.edit.getNewText} $$0\n\t" | ||
| } else { | ||
| s"match {\n\t${head.edit.getNewText}\n\t" | ||
| }, | ||
| "\n\t", | ||
| "\n}" | ||
| ) | ||
|
|
@@ -1408,7 +1424,10 @@ trait Completions { this: MetalsGlobal => | |
| if (definitions.isTupleType(parents.selector)) { | ||
| result += new TextEditMember( | ||
| "case () =>", | ||
| new l.TextEdit(editRange, "case ($0) =>"), | ||
| new l.TextEdit( | ||
| editRange, | ||
| if (clientSupportsSnippets) "case ($0) =>" else "case () =>" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not brilliant, but again, it's opt-in. |
||
| ), | ||
| parents.selector.typeSymbol, | ||
| label = Some(s"case ${parents.selector} =>"), | ||
| command = metalsConfig.parameterHintsCommand().asScala | ||
|
|
@@ -1466,8 +1485,10 @@ trait Completions { this: MetalsGlobal => | |
| val label = s"case $pattern =>" | ||
| new TextEditMember( | ||
| filterText = label, | ||
| edit = | ||
| new l.TextEdit(editRange, label + (if (isSnippet) " $0" else "")), | ||
| edit = new l.TextEdit( | ||
| editRange, | ||
| label + (if (isSnippet && clientSupportsSnippets) " $0" else "") | ||
| ), | ||
| sym = sym, | ||
| label = Some(label), | ||
| additionalTextEdits = autoImports | ||
|
|
@@ -1482,7 +1503,8 @@ trait Completions { this: MetalsGlobal => | |
| s"case _: $name", | ||
| new l.TextEdit( | ||
| editRange, | ||
| if (isSnippet) s"case $${0:_}: $name$suffix => " | ||
| if (isSnippet && clientSupportsSnippets) | ||
| s"case $${0:_}: $name$suffix => " | ||
| else s"case _: $name$suffix =>" | ||
| ), | ||
| sym, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| package tests.pc | ||
|
|
||
| import tests.BaseCompletionSuite | ||
| import scala.meta.pc.PresentationCompilerConfig | ||
| import scala.meta.internal.pc.PresentationCompilerConfigImpl | ||
|
|
||
| object CompletionSnippetNegSuite extends BaseCompletionSuite { | ||
|
|
||
| override def config: PresentationCompilerConfig = | ||
| PresentationCompilerConfigImpl( | ||
| isCompletionSnippetsEnabled = false | ||
| ) | ||
|
|
||
| checkSnippet( | ||
| "member", | ||
| """ | ||
| |object Main { | ||
| | List.appl@@ | ||
| |} | ||
| |""".stripMargin, | ||
| """|apply | ||
| |unapplySeq | ||
| |""".stripMargin, | ||
| compat = Map( | ||
| "2.13" -> | ||
| // the second apply is from scala/collection/BuildFrom#apply(), introduced in 2.13 | ||
| """|apply | ||
| |unapplySeq | ||
| |apply | ||
| |""".stripMargin | ||
| ) | ||
| ) | ||
|
|
||
| checkSnippet( | ||
| "scope", | ||
| """ | ||
| |object Main { | ||
| | printl@@ | ||
| | | ||
| |} | ||
| |""".stripMargin, | ||
| """|println() | ||
| |println | ||
| |""".stripMargin | ||
| ) | ||
|
|
||
| checkSnippet( | ||
| "java-nullary", | ||
| """ | ||
| |class Foo { | ||
| | override def toString = "Foo" | ||
| |} | ||
| |object Main { | ||
| | new Foo().toStrin@@ | ||
| | | ||
| |} | ||
| |""".stripMargin, | ||
| // even if `Foo.toString` is nullary, it overrides `Object.toString()` | ||
| // which is a Java non-nullary method with an empty parameter list. | ||
| """|toString() | ||
| |""".stripMargin | ||
| ) | ||
|
|
||
| checkSnippet( | ||
| "type", | ||
| s"""|object Main { | ||
| | val x: scala.IndexedSe@@ | ||
| |} | ||
| |""".stripMargin, | ||
| // It's expected to have two separate results, one for `object IndexedSeq` and one for `type IndexedSeq[T]`. | ||
| """|IndexedSeq | ||
| |IndexedSeq | ||
| |""".stripMargin | ||
| ) | ||
|
|
||
| } |
Uh oh!
There was an error while loading. Please reload this page.