diff --git a/crates/pgt_workspace/src/workspace/server/change.rs b/crates/pgt_workspace/src/workspace/server/change.rs index c8799922..cdd5a569 100644 --- a/crates/pgt_workspace/src/workspace/server/change.rs +++ b/crates/pgt_workspace/src/workspace/server/change.rs @@ -65,6 +65,8 @@ impl Document { // when we recieive more than one change, we need to push back the changes based on the // total range of the previous ones. This is because the ranges are always related to the original state. + // BUT: only for the statement range changes, not for the text changes + // this is why we pass both varaints to apply_change let mut changes = Vec::new(); let mut offset: i64 = 0; @@ -86,9 +88,9 @@ impl Document { change }; - changes.extend(self.apply_change(adjusted_change)); + changes.extend(self.apply_change(adjusted_change, change)); - offset += change.change_size(); + offset += adjusted_change.change_size(); } self.version = change.version; @@ -240,9 +242,17 @@ impl Document { } /// Applies a single change to the document and returns the affected statements - fn apply_change(&mut self, change: &ChangeParams) -> Vec { + /// + /// * `change`: The range-adjusted change to use for statement changes + /// * `original_change`: The original change to use for text changes (yes, this is a bit confusing, and we might want to refactor this entire thing at some point.) + fn apply_change( + &mut self, + change: &ChangeParams, + original_change: &ChangeParams, + ) -> Vec { // if range is none, we have a full change if change.range.is_none() { + // doesnt matter what change since range is null return self.apply_full_change(change); } @@ -255,7 +265,7 @@ impl Document { let change_range = change.range.unwrap(); let previous_content = self.content.clone(); - let new_content = change.apply_to_text(&self.content); + let new_content = original_change.apply_to_text(&self.content); // we first need to determine the affected range and all affected statements, as well as // the index of the prev and the next statement, if any. The full affected range is the @@ -1560,28 +1570,29 @@ mod tests { fn multiple_deletions_at_once() { let path = PgTPath::new("test.sql"); - let mut doc = Document::new("\n\n\n\nALTER TABLE ONLY \"public\".\"sendout\"\n ADD CONSTRAINT \"sendout_organisation_id_fkey\" FOREIGN -KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n".to_string(), 0); + let mut doc = Document::new("ALTER TABLE ONLY public.omni_channel_message ADD CONSTRAINT omni_channel_message_organisation_id_fkey FOREIGN KEY (organisation_id) REFERENCES public.organisation(id) ON UPDATE RESTRICT ON DELETE CASCADE;".to_string(), 0); let change = ChangeFileParams { path: path.clone(), version: 1, changes: vec![ ChangeParams { - range: Some(TextRange::new(31.into(), 38.into())), - text: "te".to_string(), + range: Some(TextRange::new(60.into(), 80.into())), + text: "sendout".to_string(), }, ChangeParams { - range: Some(TextRange::new(60.into(), 67.into())), - text: "te".to_string(), + range: Some(TextRange::new(24.into(), 44.into())), + text: "sendout".to_string(), }, ], }; let changed = doc.apply_file_change(&change); - assert_eq!(doc.content, "\n\n\n\nALTER TABLE ONLY \"public\".\"te\"\n ADD CONSTRAINT \"te_organisation_id_fkey\" FOREIGN -KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n"); + assert_eq!( + doc.content, + "ALTER TABLE ONLY public.sendout ADD CONSTRAINT sendout_organisation_id_fkey FOREIGN KEY (organisation_id) REFERENCES public.organisation(id) ON UPDATE RESTRICT ON DELETE CASCADE;" + ); assert_eq!(changed.len(), 2); @@ -1592,19 +1603,18 @@ KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDA fn multiple_additions_at_once() { let path = PgTPath::new("test.sql"); - let mut doc = Document::new("\n\n\n\nALTER TABLE ONLY \"public\".\"sendout\"\n ADD CONSTRAINT \"sendout_organisation_id_fkey\" FOREIGN -KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n".to_string(), 0); + let mut doc = Document::new("ALTER TABLE ONLY public.sendout ADD CONSTRAINT sendout_organisation_id_fkey FOREIGN KEY (organisation_id) REFERENCES public.organisation(id) ON UPDATE RESTRICT ON DELETE CASCADE;".to_string(), 0); let change = ChangeFileParams { path: path.clone(), version: 1, changes: vec![ ChangeParams { - range: Some(TextRange::new(31.into(), 38.into())), + range: Some(TextRange::new(47.into(), 54.into())), text: "omni_channel_message".to_string(), }, ChangeParams { - range: Some(TextRange::new(60.into(), 67.into())), + range: Some(TextRange::new(24.into(), 31.into())), text: "omni_channel_message".to_string(), }, ], @@ -1612,8 +1622,10 @@ KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDA let changed = doc.apply_file_change(&change); - assert_eq!(doc.content, "\n\n\n\nALTER TABLE ONLY \"public\".\"omni_channel_message\"\n ADD CONSTRAINT \"omni_channel_message_organisation_id_fkey\" FOREIGN -KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n"); + assert_eq!( + doc.content, + "ALTER TABLE ONLY public.omni_channel_message ADD CONSTRAINT omni_channel_message_organisation_id_fkey FOREIGN KEY (organisation_id) REFERENCES public.organisation(id) ON UPDATE RESTRICT ON DELETE CASCADE;" + ); assert_eq!(changed.len(), 2); @@ -1663,6 +1675,38 @@ KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDA assert_document_integrity(&doc); } + #[test] + fn test_content_out_of_sync() { + let path = PgTPath::new("test.sql"); + let initial_content = "select 1, 2, 2232231313393319 from unknown_users;\n"; + + let mut doc = Document::new(initial_content.to_string(), 0); + + let change1 = ChangeFileParams { + path: path.clone(), + version: 1, + changes: vec![ + ChangeParams { + range: Some(TextRange::new(29.into(), 29.into())), + text: "3".to_string(), + }, + ChangeParams { + range: Some(TextRange::new(30.into(), 30.into())), + text: "1".to_string(), + }, + ], + }; + + let _changes = doc.apply_file_change(&change1); + + assert_eq!( + doc.content, + "select 1, 2, 223223131339331931 from unknown_users;\n" + ); + + assert_document_integrity(&doc); + } + #[test] fn test_comments_only() { let path = PgTPath::new("test.sql");