lsp: edit events change ranges need to affect each other.

helix-lsp/src/client.rs

@@ -247,19 +247,47 @@ impl Client {
         .await
     }
 
+    // TODO: this is dumb. TextEdit describes changes to the initial doc (concurrent), but
+    // TextDocumentContentChangeEvent describes a series of changes (sequential).
+    // So S -> S1 -> S2, meaning positioning depends on the previous edits.
+    //
+    // Calculation is therefore a bunch trickier.
     pub fn changeset_to_changes(
         old_text: &Rope,
+        new_text: &Rope,
         changeset: &ChangeSet,
     ) -> Vec<lsp::TextDocumentContentChangeEvent> {
         let mut iter = changeset.changes().iter().peekable();
         let mut old_pos = 0;
+        let mut new_pos = 0;
 
         let mut changes = Vec::new();
 
         use crate::util::pos_to_lsp_pos;
         use helix_core::Operation::*;
 
+        // TODO: stolen from syntax.rs, share
+        use helix_core::RopeSlice;
+        fn traverse(pos: lsp::Position, text: RopeSlice) -> lsp::Position {
+            let lsp::Position {
+                mut line,
+                mut character,
+            } = pos;
+
+            // TODO: there should be a better way here
+            for ch in text.chars() {
+                if ch == '\n' {
+                    line += 1;
+                    character = 0;
+                } else {
+                    character += ch.len_utf16() as u32;
+                }
+            }
+            lsp::Position { line, character }
+        }
+
         let old_text = old_text.slice(..);
+        let new_text = new_text.slice(..);
 
         // TODO: verify this function, specifically line num counting
 
@@ -271,10 +299,12 @@ impl Client {
             let mut old_end = old_pos + len;
 
             match change {
-                Retain(_) => {}
+                Retain(i) => {
+                    new_pos += i;
+                }
                 Delete(_) => {
-                    let start = pos_to_lsp_pos(&old_text, old_pos);
+                    let start = pos_to_lsp_pos(&new_text, new_pos);
-                    let end = pos_to_lsp_pos(&old_text, old_end);
+                    let end = traverse(start, old_text.slice(old_pos..old_end));
 
                     // deletion
                     changes.push(lsp::TextDocumentContentChangeEvent {
@@ -284,12 +314,14 @@ impl Client {
                     });
                 }
                 Insert(s) => {
-                    let start = pos_to_lsp_pos(&old_text, old_pos);
+                    let start = pos_to_lsp_pos(&new_text, new_pos);
+
+                    new_pos += s.chars().count();
 
                     // a subsequent delete means a replace, consume it
                     let end = if let Some(Delete(len)) = iter.peek() {
                         old_end = old_pos + len;
-                        let end = pos_to_lsp_pos(&old_text, old_end);
+                        let end = traverse(start, old_text.slice(old_pos..old_end));
 
                         iter.next();
 
@@ -318,6 +350,7 @@ impl Client {
         &self,
         text_document: lsp::VersionedTextDocumentIdentifier,
         old_text: &Rope,
+        new_text: &Rope,
         changes: &ChangeSet,
     ) -> Result<()> {
         // figure out what kind of sync the server supports
@@ -343,7 +376,9 @@ impl Client {
                     text: "".to_string(),
                 }] // TODO: probably need old_state here too?
             }
-            lsp::TextDocumentSyncKind::Incremental => Self::changeset_to_changes(old_text, changes),
+            lsp::TextDocumentSyncKind::Incremental => {
+                Self::changeset_to_changes(old_text, new_text, changes)
+            }
             lsp::TextDocumentSyncKind::None => return Ok(()),
         };
 

helix-view/src/document.rs

@@ -183,6 +183,7 @@ impl Document {
                 let notify = language_server.text_document_did_change(
                     self.versioned_identifier(),
                     &old_doc,
+                    self.text(),
                     transaction.changes(),
                 );
 
@@ -301,8 +302,8 @@ mod test {
 
         let transaction = Transaction::insert(&doc.state, " world".into());
         let old_doc = doc.state.clone();
-        let changes = Client::changeset_to_changes(&old_doc.doc, transaction.changes());
         doc.apply(&transaction);
+        let changes = Client::changeset_to_changes(&old_doc.doc, doc.text(), transaction.changes());
 
         assert_eq!(
             changes,
@@ -320,8 +321,8 @@ mod test {
 
         let transaction = transaction.invert(&old_doc);
         let old_doc = doc.state.clone();
-        let changes = Client::changeset_to_changes(&old_doc.doc, transaction.changes());
         doc.apply(&transaction);
+        let changes = Client::changeset_to_changes(&old_doc.doc, doc.text(), transaction.changes());
 
         // line: 0-based.
         // col: 0-based, gaps between chars.
@@ -343,22 +344,48 @@ mod test {
 
         // replace
 
+        // also tests that changes are layered, positions depend on previous changes.
+
         doc.state.selection = Selection::single(0, 5);
-        let transaction = Transaction::change_by_selection(&doc.state, |range| {
+        let transaction = Transaction::change(
-            (range.from(), range.to(), Some("aeiou".into()))
+            &doc.state,
-        });
+            vec![(0, 2, Some("aei".into())), (3, 5, Some("ou".into()))].into_iter(),
-        let changes = Client::changeset_to_changes(&doc.state.doc, transaction.changes());
+        );
+        // aeilou
+        doc.apply(&transaction);
+        let changes =
+            Client::changeset_to_changes(&doc.state.doc, doc.text(), transaction.changes());
 
         assert_eq!(
             changes,
-            &[lsp::TextDocumentContentChangeEvent {
+            &[
-                range: Some(lsp::Range::new(
+                // 0 1 2 3 4 5
-                    lsp::Position::new(0, 0),
+                // |h|e|l|l|o|
-                    lsp::Position::new(0, 5)
+                // ----
-                )),
+                //
-                text: "aeiou".into(),
+                // aeillo
-                range_length: None,
+                lsp::TextDocumentContentChangeEvent {
-            }]
+                    range: Some(lsp::Range::new(
+                        lsp::Position::new(0, 0),
+                        lsp::Position::new(0, 2)
+                    )),
+                    text: "aei".into(),
+                    range_length: None,
+                },
+                // 0 1 2 3 4 5 6
+                // |a|e|i|l|l|o|
+                //         -----
+                //
+                // aeilou
+                lsp::TextDocumentContentChangeEvent {
+                    range: Some(lsp::Range::new(
+                        lsp::Position::new(0, 4),
+                        lsp::Position::new(0, 6)
+                    )),
+                    text: "ou".into(),
+                    range_length: None,
+                }
+            ]
         );
     }
 }