scalameta / metals

Scala language server with rich IDE features 🚀
https://scalameta.org/metals/
Apache License 2.0
2.07k stars 323 forks source link

Completion candidates are missing parameters in `textEdit.newText` #657

Closed iravid closed 5 years ago

iravid commented 5 years ago

Hi, not sure if this is implemented or not, but I'm seeing all completion candidates coming in to the client with a barebones newText. For example, this is a completion on CompletableFuture#get:

{
  "jsonrpc": "2.0",
  "id": 214,
  "result": {
    "label": "get(timeout: Long, unit: TimeUnit): ResponseBytes[GetObjectResponse]",
    "kind": 8,
    "detail": "(timeout: Long, unit: TimeUnit): ResponseBytes[GetObjectResponse]",
    "documentation": {
      "kind": "markdown",
      "value": "Waits if necessary for at most the given time for this future\nto complete, and then returns its result, if available."
    },
    "sortText": "00011",
    "filterText": "get",
    "insertTextFormat": 2,
    "textEdit": {
      "range": {
        "start": {
          "line": 267,
          "character": 11
        },
        "end": {
          "line": 267,
          "character": 11
        }
      },
      "newText": "get($0)"
    },
    "data": {
      "target": "<redacted>",
      "symbol": "java/util/concurrent/CompletableFuture#get(+1)."
    }
  }
}

I'm using Emacs with lsp-mode.

olafurpg commented 5 years ago

Thanks for reporting! What is your expected value for newText? The “get($0)” format looks good to me, the $0 placeholder indicates where the cursor should go.

iravid commented 5 years ago

Hey @olafurpg!

From reading CompletionItemResolver.scala:25, I'd expect it to be get(${0:timeout}, ${1:unit}).

iravid commented 5 years ago

Ah, sorry, I just noticed that only applies to OverrideKind which I assume is for completing method overrides.

It'd be nice to have this for all completions (including Scala symbols). ENSIME used to have this feature and it was really helpful.

gabro commented 5 years ago

@iravid not exactly what you're asking, but potentially related: does your editor/lsp client support signatureHelp?

Here's an example in VSCode, notice how it display info for the appropriate overload as it becomes unambiguous:

2019-04-13 09 16 48

olafurpg commented 5 years ago

If the editor supports CompletionItem.command, the server property -Dmetals.signature-help.command can be configured to automatically trigger signature help when completing a method symbol with non-empty parameters. See https://scalameta.org/metals/docs/editors/new-editor.html#dmetalssignature-helpcommand

I believe signature help is more suitable for this use-case than the snippet get(${0:timeout}, ${1:unit}) because signature help shows the parameter types, documentation and also makes it possible to cycle through overloaded method signatures.

iravid commented 5 years ago

Hey @olafurpg and @gabro - thanks for the answer! I spent some time with the current solution and I agree that it's suitable.

I personally find signature expansion pretty useful for methods with higher order functions, but that's subjective. Shall I close this issue?

olafurpg commented 5 years ago

Sounds good, let's close this issue as wontfix.