improvement: Don't sent text on didSave #7015
Conversation
| @@ -884,9 +884,6 @@ abstract class MetalsLspService( | |||
| ): CompletableFuture[Unit] = { | |||
| val path = params.getTextDocument.getUri.toAbsolutePath | |||
| savedFiles.add(path) | |||
| // read file from disk, we only remove files from buffers on didClose. | |||
| val text = path.toInput.text | |||
There was a problem hiding this comment.
This doesn't seem necessary and for large files might actually be problematic.
tgodzik
force-pushed
the
did-save
branch
4 times, most recently
from
9fd01d9 to
af3b81b
Compare
| _ <- server.didChange(file)(_ => | ||
| code.replaceAll(allMarkersRegex, "") | ||
| ) | ||
| _ <- server.didSave(file)(identity) |
There was a problem hiding this comment.
Maybe we should do either of two:
- remove change function from
server.didChange(it will always de identity) - have
server.didSavewith the function send firstdidChangeand only laterdidSave.
There was a problem hiding this comment.
have server.didSave with the function send first didChange and only later didSave.
That proved to be complicated and broke a few things, I will change to didSave always doing identity
There was a problem hiding this comment.
Remove fn from didSave to make it explicit that it doesn't actually send anything. And also this wasn't simulating correctly how the editor would work
| _.replace("n + 1", "n + 2") | ||
| ) | ||
| _ <- server.didSave("a/src/main/scala/a/Util.scala")(identity) | ||
| _ <- server.didFocus("a/src/main/scala/a/Main.worksheet.sc") |
There was a problem hiding this comment.
I don't think we should ever need didFocus, since it may not be supported, right?
There was a problem hiding this comment.
It's just focus switched to Util.scala and the worksheet doesn't evaluate. This will work in VS Code, though wonder what happens if did focus is not supported
| @@ -785,6 +785,7 @@ final case class TestingServer( | |||
| val abspath = toPath(filename) | |||
| val oldText = abspath.toInputFromBuffers(buffers).text | |||
| val newText = fn(oldText) | |||
| buffers.put(abspath, newText) | |||
There was a problem hiding this comment.
We weren't actually updating the buffers
metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
Outdated
Show resolved
Hide resolved
Previously, we would have clients send us full text on didSave, which was never used. Now, we removed that option. I also removed reading the file from disk to buffers since that file should already be up to date form didChanged
…editor behaviour better
Previously, we would have clients send us full text on didSave, which was never used.
Now, we removed that option.
I also removed reading the file from disk to buffers since that file should already be up to date form didChanged