Skip to content
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

metals-doctor-run client command has optional html arg #6387

Closed
kurnevsky opened this issue May 8, 2024 · 5 comments · Fixed by #6474
Closed

metals-doctor-run client command has optional html arg #6387

kurnevsky opened this issue May 8, 2024 · 5 comments · Fixed by #6474
Labels
bug Something that is making a piece of functionality unusable

Comments

@kurnevsky
Copy link
Contributor

Describe the bug

Metals doesn't always pass html argument with metals-doctor-run client command. For instance, when bloop process dies and metals shows message with open doctor request. This should be either documented in https://scalameta.org/metals/docs/integrations/new-editor#run-doctor-1 or fixed.

Can this happen with metals-doctor-reload as well?

Logs:

[Trace - 09:33:55 AM] Received request 'window/showMessageRequest - (3135).
Params: {
  "actions": [
    {
      "title": "Open doctor."
    }
  ],
  "type": 2,
  "message": "Bloop 1 ⚠️ : Picked up JAVA_TOOL_OPTIONS: -Dmetals.allow-multiline-string-formatting=off -Dmetals.icons=unicode"
}


[Trace - 09:33:55 AM] Sending response 'window/showMessageRequest - (3135)'. Processing request took 1960ms
Params: {
  "jsonrpc": "2.0",
  "id": "3135",
  "result": {
    "title": "Open doctor."
  }
}


[Trace - 09:33:55 AM] Received notification 'metals/executeClientCommand'.
Params: {
  "command": "metals-doctor-run",
  "arguments": []
}

Expected behavior

No response

Operating system

None

Editor/Extension

None

Version of Metals

1.3.0

Extra context or search terms

No response

@tgodzik
Copy link
Contributor

tgodzik commented May 8, 2024

Isn't it a leftover, I don't think we use that html argument anywhere, for sure not on the server. We might just need to adjust the docs. Or are you aware of a case where that argument was being used?

@kurnevsky
Copy link
Contributor Author

It's a metals/executeClientCommand which is sent from the server to a client. Other clients than for vscode can use it (and we do in emacs lsp-metals).

@tgodzik
Copy link
Contributor

tgodzik commented May 8, 2024

Sure, I understand, but it seems that metals-doctor-run is never invoked with any arguments. Should it be? I am not sure why the html arg was needed at any point.

@kurnevsky
Copy link
Contributor Author

kurnevsky commented May 8, 2024

It is invoked with html arg when client sends doctor-run command:


[Trace - 04:32:24 PM] Sending request 'workspace/executeCommand - (38)'.
Params: {
  "command": "doctor-run"
}


[Trace - 04:32:24 PM] Received notification 'metals/executeClientCommand'.
Params: {
  "command": "metals-doctor-run",
  "arguments": [
    "<h1 >Metals Doctor</h1>
    ...

@tgodzik
Copy link
Contributor

tgodzik commented May 8, 2024

Ach right, looks like we are not doing that correctly, we should dig into that.

@tgodzik tgodzik added this to Triage in Metals Issue Board via automation May 8, 2024
@tgodzik tgodzik added the bug Something that is making a piece of functionality unusable label May 13, 2024
@kasiaMarek kasiaMarek moved this from Triage to Document in Metals Issue Board Jun 3, 2024
@kasiaMarek kasiaMarek moved this from Document to In progress in Metals Issue Board Jun 3, 2024
Metals Issue Board automation moved this from In progress to Done Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that is making a piece of functionality unusable
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants