Fix an off-by-one error in the LSP formatter (#1605)

* Fix an off-by-one error in the LSP formatter

According to [the LSP specification](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#range)
the end of a text range in the protocol is exclusive. When executing
a formatting request in the language server we want to replace the
entire document with the formatted result, including any trailing
newlines. This means we need to specify the beginning of the line after
the very last line in the document as the end of the range of text to
be replaced.

This also fixes an annoying behaviour when formatting with NLS in Helix.
Previously, Helix would insist on selecting (what looked like) the
entire document on every formatting request. With this change, Helix
refrains from doing this.

* Update LSP snapshot test

lsp/nls/src/requests/formatting.rs

@@ -16,7 +16,6 @@ pub fn handle_format_document(
     let file_id = server.cache.id_of(&SourcePath::Path(path)).unwrap();
     let text = server.cache.files().source(file_id).clone();
     let document_length = text.lines().count() as u32;
-    let last_line_length = text.lines().next_back().unwrap().len() as u32;
 
     let mut formatted: Vec<u8> = Vec::new();
     nickel_lang_core::format::format(text.as_bytes(), &mut formatted).map_err(|err| {
@@ -39,9 +38,12 @@ pub fn handle_format_document(
                 line: 0,
                 character: 0,
             },
+            // The end position is exclusive. Since we want to replace the
+            // entire document, we specify the beginning of the line after the
+            // last line in the document.
             end: Position {
-                line: document_length - 1,
-                character: last_line_length,
+                line: document_length,
+                character: 0,
             },
         },
         new_text: formatted,

lsp/nls/src/requests/formatting_external.rs

@@ -20,7 +20,6 @@ pub fn handle_format_document(
     let file_id = server.cache.id_of(&SourcePath::Path(path)).unwrap();
     let text = server.cache.files().source(file_id).clone();
     let document_length = text.lines().count() as u32;
-    let last_line_length = text.lines().next_back().unwrap().len() as u32;
 
     let Ok(mut topiary) = process::Command::new(FORMATTING_COMMAND[0])
         .args(&FORMATTING_COMMAND[1..])
@@ -62,9 +61,12 @@ pub fn handle_format_document(
                 line: 0,
                 character: 0,
             },
+            // The end position is exclusive. Since we want to replace the
+            // entire document, we specify the beginning of the line after the
+            // last line in the document.
             end: Position {
-                line: document_length - 1,
-                character: last_line_length,
+                line: document_length,
+                character: 0,
             },
         },
         new_text,

lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__formatting.ncl.snap

@@ -2,7 +2,7 @@
 source: lsp/nls/tests/main.rs
 expression: output
 ---
-[<0:0-1:8> {
+[<0:0-2:0> {
   foo = "bar",
   baz = 7
 }