search

scalameta / metals-sublime

Sublime Text package for Metals, a language server for Scala
https://packagecontrol.io/packages/LSP-metals
Apache License 2.0
16 stars 10 forks source link

Fail to send `didFocusTextDocument` in Sublime 4 #41

Closed jvican closed 2 years ago

jvican commented 2 years ago

I'm using the latest Metals Sublime release with the latest Sublime Text release (Build 4116) and I'm getting the following errors in the Metals server log panel:

metals: 2021.10.01 14:18:10 WARN  Unexpected notification params received for didFocusTextDocument: {uri=file:///Users/jvicentecantero/Code/???}

This suggests that we're not sending the didFocusTextDocument well to the Metals server and it's potentially causing misbehaviors on the server. Looks like we might need to change the ST4 implementation here

https://github.com/scalameta/metals-sublime/blob/b95de4f46262d1576cf36b65a79f7e87f577d7a1/st4/LspMetalsFocus.py#L9-L33

@ayoub-benali You're the last person that made the change, do you know what might be going on? Also, I've experienced cases where Metals is not publishing compilation errors for files that are not open. Every time it's happened has been annoying and unpredictable. I'm not sure if this is connected to that issue, but you might want to know about it. Let me know if you have any ideas here because I'd be willing to invest some time fixing the issue with missing diagnostics.

jvican commented 2 years ago

I fixed this, will make a PR, but the diagnostics issue is still happening. @rwols Do you know what might be going on? I think the LSP server is sending the diagnostics and Sublime is not showing them because the file is not open. Is that possible?

ayoub-benali commented 2 years ago

Thanks for for the fix @jvican.

As for the other issue, from my understanding it is a limitation of the LSP package on which this package rely. @rwols Is there an existing issue on the LSP repo ?

jvican commented 2 years ago

Thanks for the answer. Now that you say it, it makes sense it's a problem on the LSP package. I think this is quite a severe limitation because it gives the impression that the changes the user is doing in the editor are OK when they are really not. Delaying or hiding diagnostics for other messages ends up getting in the way of my workflow. @rwols Would it be possible to get a fix for that in the plugin? How much would it take?

rwols commented 2 years ago

Exactly what’s the problem?

rwols commented 2 years ago

Oh the diagnostics. Yeah, we only show diags for open files. I don’t have any plans to change this, but we’re always open to pull requests.

ayoub-benali commented 2 years ago

If I remember correctly the difficulty on the LSP package side is about keeping track of diagnostics for files that aren't open. @jvican I recommend you create an issue on the LSP package so that we can discuss this issue and possible solutions there.

As for the didFocusTextDocument notification just ping me for a review.

jvican commented 2 years ago

I will do that, will elaborate on why this is important for languages such as Scala.

jvican commented 2 years ago

The underlying pain described in this issue has been fixed in https://github.com/sublimelsp/LSP/issues/1868#event-5514967250 and it will be released soon.