Don't change file if formatting doesn't change

[fufexan]

Description

I'm using the helix editor and when formatting a file with :format while using rnix-lsp, it will replace the entire content of the file even when the formatting doesn't change.
Doing the same for a .c/.cpp file while using clangd doesn't replace the contents.

I wonder whether this is because rnix-lsp doesn't check whether the formatting has actually changed. Perhaps it would take too much to check it? I'm not sure how this works as I'm not familiar with the code behind LSP servers.

Considered alternatives

I noticed the code responsible for formatting is here and whether checking if the formatting changed is an option.

[mtoohey31]

I've done some digging on this; I don't think it would take much effort to make significant improvements to the way formatting is done. From what I can tell, the current formatting implementation simply sends the entire document through nixpkgs_fmt::reformat_node then sends the whole thing back.

This section of nixpkgs_fmt's code seems to use an edit system to update the document as it goes. I'm not 100% sure if all edits go through this method, but some basic testing seems to indicate that this might be the case, I could be wrong though.

Ideally, we could add another public function to nixpkgs_fmt that returns the edits in a usable format instead of the reformatted node/string. Then we could use that function at the point in this program responsible for formatting identified by @fufexan and instead of returning one big edit containing a completely new document, we can transform the individual edits into a format suitable for LSP then send each one.

[mtoohey31]

Ok, I've done some tinkering here and here. The code is kinda messy right now and it only kind of works (it looks like some indentation changes are getting skipped so you have to reformat twice sometimes, and things occasionally get deleted), but it seems like it should be possible to fix those issues. When I have time I'll clean it up and open PRs on both repos.

[mtoohey31]

Fixing those issues has turned out to be much harder than anticipated. I'm going to keep working on it at some point, but if someone else wants to take over and give it a go, I'd be happy to give some pointers.

[fufexan]

I'd like to give it a go, but I don't know Rust. I might learn it this summer, but I have no chance to until July when my exams end.

[mtoohey31]

No worries, good luck with exams! I've actually finally managed to make some progress. I think I'm on the right track now, but there are still more bugs to work out.

The problem is that nixpkgs-fmt reformats the document with a series of two edit sets, but a language server can only respond with one set of edits to be applied in a single transaction (from my understanding of the protocol), so the two sets of edits have to be combined into one, and merging them is really complicated (at least, I'm finding it complicated). I'm working on a package to do the merging here, but it's hard to say how long it will take my to work out the rest of the bugs.

[mtoohey31]

Ok, I believe I've ironed out most of the bugs. All 77 of nixpkgs-fmt's test cases are now reformatted correctly with minimal edits being applied instead of the whole document.

I'm still not entirely convinced it's bug-free though, so I'm going to use it myself for a little while before opening merge requests. I'm also not entirely sure where I should put the edit merging step (inside the nixpkgs-fmt crate, or inside rnix-lsp), so I have to think about that before the PR.

In the meantime, if anyone wants to use these changes, use the defaultPackage of this flake reference: github:mtoohey31/rnix-lsp/feat/improved-format-edits.

[Ma27]

I'm happy to take a look during the next days :)

[mtoohey31]

I'm happy to take a look during the next days :)

Ok, great! I'll try and get pull requests opened by tomorrow then.