Disable OrganizeImports commands if they are not available #6595
Conversation
Rather than having Metals show a warning message, leave it to the client to decide on what to do with the reason for why they are disabled, and if/how it should be displayed.
| val hasUnused = params | ||
| .getContext() | ||
| .getDiagnostics() | ||
| .asScala | ||
| .collect { case ScalacDiagnostic.UnusedImport(name) => name } | ||
| .nonEmpty | ||
|
|
||
| if (hasUnused && !diagnostics.hasDiagnosticError(file)) { | ||
| true | ||
| if (!hasUnused) { |
There was a problem hiding this comment.
This should arguably also check if (!isScalaOrSbt(file)), as the other one does?
There was a problem hiding this comment.
That would make sense, though I don't think Metals will be invoked for anything else than Scala and Java files. Maybe we should be explicit about not doing organize import on Java
| val hasUnused = params | ||
| .getContext() | ||
| .getDiagnostics() | ||
| .asScala | ||
| .collect { case ScalacDiagnostic.UnusedImport(name) => name } | ||
| .nonEmpty | ||
|
|
||
| if (hasUnused && !diagnostics.hasDiagnosticError(file)) { | ||
| true | ||
| if (!hasUnused) { |
There was a problem hiding this comment.
That would make sense, though I don't think Metals will be invoked for anything else than Scala and Java files. Maybe we should be explicit about not doing organize import on Java
| Some("Only supported for Scala and sbt files") | ||
| } else if (diagnostics.hasDiagnosticError(file)) { | ||
| scribe.info(canNotOrganizeImportsWithErrors) | ||
| Some(canNotOrganizeImportsWithErrors) | ||
| } else { |
There was a problem hiding this comment.
We are now missing one of the conditions isSourceOrganizeImportCalled(params) and organize imports action is returned everywhere. We want to return it on unused imports and also when explicitly called. Maybe we should still keep isCallAllowed for that and also check that flag?
|
It'll be a week or two before I'll have time to get back to this, so if anyone else wants to address the comments, please feel free to do so. Otherwise I will get it done (and Zed is being a bit annoying even with those changes, but otherwise it's quite fun overall, so I have some strong personal motivations), but definitely not this week. |
There is no hurry! Just let us know if you have any problems or are unable to finish it. |
Rather than having Metals show a warning message, leave it to the client to decide on what to do with the reason for why they are disabled, and if/how it should be displayed.
Fixes scalameta/metals-zed#14
Then it looks like this in VS Code:
