feat: improved reformat edits

[mtoohey31]

Summary & Motivation

The current method of reformatting documents consists of replacing the whole document with a single edit containing the reformatted version. This is sub-optimal for two main reasons:

1. It is inefficient, which could potentially be an issue with extremely large documents.
2. It can result in loosing your cursor position in the document in some editors, as @fufexan and I have experienced with helix.

This pull request updates the reformatting so that it sends multiple small edits, instead of one huge edit.

Further context

Since accomplishing this requires exposing the edits that nixpkgs-fmt uses internally, this depends on nixpkgs-fmt#296.

This pull request also adds one additional dependency: textedit-merge, which I wrote in the proccess of creating this PR. nixpkgs-fmt reformats the document with two sets of edits (spacing edits, then indentation edits), however, from my understanding, the language server protocol requires that the response to a reformatting request be a single set of textedits. Since the ranges in the indentation edits used by nixpkgs-fmt refer to the state of the document after the spacing edits have been applied, a somewhat complex algorithm is required to merge the two sets of edits into a single set of edits that can be applied in one step.

One implementation detail that I would like a second opinion on are the data types:

• returned by the new nixpkgs_fmt::reformat_edits function
• accepted by textedit_merge::merge
• returned by textedit_merge::merge

The current choices for these data types ((Vec<AtomEdit>, Vec<AtomEdit>), &[(Range<usize>, AsRef<str>)], and Vec<(Range<usize>, String)> respectively) were chosen with the following goals in mind:

• minimal extra work being done in nixpkgs_fmt::reformat_edits (so that other users of this function can use it efficiently)
• no dependencies in textedit-merge (so that other users, if there ever are any, aren't tied to additional dependencies)

The main shortcoming of the current types is that the edits must be transformed twice, once from Vec<AtomEdit> to Vec<(Range<usize>, AsRef<str>)> for merging, then a second time from Vec<(Range<usize>, String)> to Vec<TextEdit> so they can be returned in the response to the reformatting request. If there's a way to do things that still achieves both of those goals while avoiding the double transformation, please let me know!

Finally, it would be dishonest of me if I didn't mention that I'm not 100% confident that textedit-merge is bug-free. I've tested reformatting on nixpkgs-fmt's test cases and it works correctly on all of them now, but while I was working on it, I missed quite a few edge cases that those tests brought to light. If anyone can think of edge cases that might break edit merging, please give them a try! I'd much rather fix bugs with textedit-merge before merging this than find out about them later, as many of the bugs that I encountered during development resulted in panics since they involved indexing strings by invalid ranges.

Closes #81.

[mtoohey31]

Nice work, mostly some questions regarding the implementation here and in the corresponding nixpkgs-fmt PR.

Also, do you think you'd be able to write a small integration test in src/tests.rs? :)

Thanks! I've responded to the questions here and on https://github.com/nix-community/nixpkgs-fmt/pull/296; let me know if there's anything that's still unclear. I'd be happy to add the two tests you mentioned, though I might not get to it until later in the week.

[mtoohey31]

I've added an integration test for reformatting in 5ffd970543a4d491051825f6324d219bc7a188bb. The one thing I noticed while doing that is that editor indentation preferences are not respected. I don't think there's anything we can do about that though as long as we're using nixpkgs-fmt, since it doesn't seem to support configurable indentation.

[mtoohey31]

Ok, I've rebased things against the latest master and switched the remote for nixpkgs-fmt now that https://github.com/nix-community/nixpkgs-fmt/pull/296 has been merged. I assume we'd like to wait for a compatible release of that crate before merging this though?

[mtoohey31]

@Ma27 I'd like to get this merged at some point; I've been using these changes for about two months and things seem good. Would you be able to create a new release of https://github.com/nix-community/nixpkgs-fmt on crates.io, or should I open an issue on that repository?

[Ma27]

I resigned as maintainer here, cc @aaronjanse

[mtoohey31]

I'm closing this because I've switched to using https://github.com/oxalica/nil, so I haven't been using or maintaining these changes for a while. Also, at one point while I was still using these changes, I did run into one situation where applying the formatting broke the code. So I think there must be a bug somewhere in either nixpkgs-fmt or in the textedit-merge package that I wrote for this merge request, but I don't have an example to reproduce the issue unfortunately...