Update ranges on Scala CLI actionable diagnostics for didChange
#5487
Conversation
LaurenceWarne
force-pushed
the
update-actionable-diagnostic-range
branch
from
7fffe4f
to
6f4dfe9
Compare
| textEdit.setRange(revisedRange.getOrElse(range)) | ||
| textEdit.toJsonObject | ||
| // TODO also update bsp scala diagnostics, e.g. ScalaDiagnostic(Right(textEdits)) | ||
| case diagnostic => diagnostic.getData |
There was a problem hiding this comment.
Currently, no token match means we set the diagnostic data of the fresh diagnostic to the old, possibly out of date data. This maintains the current behaviour, though I was thinking it might make more sense just to set the data as empty so the out of date code action doesn't appear?
There was a problem hiding this comment.
I was thinking it might make more sense just to set the data as empty so the out of date code action doesn't appear
Hmm... that might be a good idea.
Btw. Did you find any issues with updating also the Right branch of ScalaDiagnostic? We will be deprecating just sending TextEdit with the next ScalaCLI version, so would probably be good to do both. I can work on that if you don't have time.
There was a problem hiding this comment.
Yeah it looks something similar can be done, happy to include it in this PR if you're ok with that 👍 . Is there anything using the new ScalaDiagnostic I can use to test it out?
There was a problem hiding this comment.
That would be cool! I think you can test it out with the latest nightly of ScalaCLI. scala-cli --cli-version nightly setup-ide . should work I think on a sample project.
There was a problem hiding this comment.
Actually, that might not work. I modified my json config in .bsp to:
{
"name": "scala-cli",
"argv": [
"scala",
"--cli-version",
"1.0.2-41-gdfd53395-SNAPSHOT",
"bsp",
"--json-options",
"/home/tgodzik/Documents/workspaces/hello/.scala-build/ide-options-v2.json",
"/home/tgodzik/Documents/workspaces/hello"
],
"version": "1.0.2-41-gdfd53395-SNAPSHOT",
"bspVersion": "2.1.0-M5",
"languages": [
"scala",
"java"
]
}
There was a problem hiding this comment.
Thanks! I had to change "scala" to "scala-cli", but it looks to have done the trick.
| shouldAdjustWithinToken(diagnostic), | ||
| fallbackToNearest = false, | ||
| ) | ||
| .map(_.toJsonObject) |
There was a problem hiding this comment.
I found setting the data to empty when it was out of date a bit trickier than expected, since toRevised for ranges only returns None when the source doesn't tokenize. I added a param fallbackToNearest to turn this off and so return None also when there's no match.
| fallbackToNearest = fallbackToNearest, | ||
| ) | ||
| } | ||
| .foldLeft[Option[Seq[l.TextEdit]]](Some(Seq.empty)) { |
There was a problem hiding this comment.
This is traverse, though I didn't see it defined anywhere or pulled in from a 3rd party lib 😅
|
Could you rebase your PR? Sorry for letting you wait for so long I was trying to figure out if we can reduce the amount of added code, but it doesn't seem like we can. We'll probably delete part of it later on after text edit part is no longer supported. |
LaurenceWarne
force-pushed
the
update-actionable-diagnostic-range
branch
from
d9f0b47
to
a5e20dd
Compare
|
No problem - I've rebased |
|
Thanks! |
Hi, this PR updates code action ranges on Scala CLI actionable diagnostics (and other diagnostics with a sole code action attached) on
didChange(file not saved) usingTokenEditDistance:did-change.mp4
I think this could be extended to work with bsp
ScalaDiagnostic, I've added a TODO below 🙂