Skip to content

Commit 4cb12df

Browse files
authored
fix: broken text after receiving batched changes (#413)
1 parent b114b81 commit 4cb12df

File tree

1 file changed

+62
-18
lines changed
  • crates/pgt_workspace/src/workspace/server

1 file changed

+62
-18
lines changed

crates/pgt_workspace/src/workspace/server/change.rs

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ impl Document {
6565

6666
// when we recieive more than one change, we need to push back the changes based on the
6767
// total range of the previous ones. This is because the ranges are always related to the original state.
68+
// BUT: only for the statement range changes, not for the text changes
69+
// this is why we pass both varaints to apply_change
6870
let mut changes = Vec::new();
6971

7072
let mut offset: i64 = 0;
@@ -86,9 +88,9 @@ impl Document {
8688
change
8789
};
8890

89-
changes.extend(self.apply_change(adjusted_change));
91+
changes.extend(self.apply_change(adjusted_change, change));
9092

91-
offset += change.change_size();
93+
offset += adjusted_change.change_size();
9294
}
9395

9496
self.version = change.version;
@@ -240,9 +242,17 @@ impl Document {
240242
}
241243

242244
/// Applies a single change to the document and returns the affected statements
243-
fn apply_change(&mut self, change: &ChangeParams) -> Vec<StatementChange> {
245+
///
246+
/// * `change`: The range-adjusted change to use for statement changes
247+
/// * `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.)
248+
fn apply_change(
249+
&mut self,
250+
change: &ChangeParams,
251+
original_change: &ChangeParams,
252+
) -> Vec<StatementChange> {
244253
// if range is none, we have a full change
245254
if change.range.is_none() {
255+
// doesnt matter what change since range is null
246256
return self.apply_full_change(change);
247257
}
248258

@@ -255,7 +265,7 @@ impl Document {
255265

256266
let change_range = change.range.unwrap();
257267
let previous_content = self.content.clone();
258-
let new_content = change.apply_to_text(&self.content);
268+
let new_content = original_change.apply_to_text(&self.content);
259269

260270
// we first need to determine the affected range and all affected statements, as well as
261271
// the index of the prev and the next statement, if any. The full affected range is the
@@ -1560,28 +1570,29 @@ mod tests {
15601570
fn multiple_deletions_at_once() {
15611571
let path = PgTPath::new("test.sql");
15621572

1563-
let mut doc = Document::new("\n\n\n\nALTER TABLE ONLY \"public\".\"sendout\"\n ADD CONSTRAINT \"sendout_organisation_id_fkey\" FOREIGN
1564-
KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n".to_string(), 0);
1573+
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);
15651574

15661575
let change = ChangeFileParams {
15671576
path: path.clone(),
15681577
version: 1,
15691578
changes: vec![
15701579
ChangeParams {
1571-
range: Some(TextRange::new(31.into(), 38.into())),
1572-
text: "te".to_string(),
1580+
range: Some(TextRange::new(60.into(), 80.into())),
1581+
text: "sendout".to_string(),
15731582
},
15741583
ChangeParams {
1575-
range: Some(TextRange::new(60.into(), 67.into())),
1576-
text: "te".to_string(),
1584+
range: Some(TextRange::new(24.into(), 44.into())),
1585+
text: "sendout".to_string(),
15771586
},
15781587
],
15791588
};
15801589

15811590
let changed = doc.apply_file_change(&change);
15821591

1583-
assert_eq!(doc.content, "\n\n\n\nALTER TABLE ONLY \"public\".\"te\"\n ADD CONSTRAINT \"te_organisation_id_fkey\" FOREIGN
1584-
KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n");
1592+
assert_eq!(
1593+
doc.content,
1594+
"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;"
1595+
);
15851596

15861597
assert_eq!(changed.len(), 2);
15871598

@@ -1592,28 +1603,29 @@ KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDA
15921603
fn multiple_additions_at_once() {
15931604
let path = PgTPath::new("test.sql");
15941605

1595-
let mut doc = Document::new("\n\n\n\nALTER TABLE ONLY \"public\".\"sendout\"\n ADD CONSTRAINT \"sendout_organisation_id_fkey\" FOREIGN
1596-
KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n".to_string(), 0);
1606+
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);
15971607

15981608
let change = ChangeFileParams {
15991609
path: path.clone(),
16001610
version: 1,
16011611
changes: vec![
16021612
ChangeParams {
1603-
range: Some(TextRange::new(31.into(), 38.into())),
1613+
range: Some(TextRange::new(47.into(), 54.into())),
16041614
text: "omni_channel_message".to_string(),
16051615
},
16061616
ChangeParams {
1607-
range: Some(TextRange::new(60.into(), 67.into())),
1617+
range: Some(TextRange::new(24.into(), 31.into())),
16081618
text: "omni_channel_message".to_string(),
16091619
},
16101620
],
16111621
};
16121622

16131623
let changed = doc.apply_file_change(&change);
16141624

1615-
assert_eq!(doc.content, "\n\n\n\nALTER TABLE ONLY \"public\".\"omni_channel_message\"\n ADD CONSTRAINT \"omni_channel_message_organisation_id_fkey\" FOREIGN
1616-
KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDATE RESTRICT ON DELETE CASCADE;\n");
1625+
assert_eq!(
1626+
doc.content,
1627+
"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;"
1628+
);
16171629

16181630
assert_eq!(changed.len(), 2);
16191631

@@ -1663,6 +1675,38 @@ KEY (\"organisation_id\") REFERENCES \"public\".\"organisation\"(\"id\") ON UPDA
16631675
assert_document_integrity(&doc);
16641676
}
16651677

1678+
#[test]
1679+
fn test_content_out_of_sync() {
1680+
let path = PgTPath::new("test.sql");
1681+
let initial_content = "select 1, 2, 2232231313393319 from unknown_users;\n";
1682+
1683+
let mut doc = Document::new(initial_content.to_string(), 0);
1684+
1685+
let change1 = ChangeFileParams {
1686+
path: path.clone(),
1687+
version: 1,
1688+
changes: vec![
1689+
ChangeParams {
1690+
range: Some(TextRange::new(29.into(), 29.into())),
1691+
text: "3".to_string(),
1692+
},
1693+
ChangeParams {
1694+
range: Some(TextRange::new(30.into(), 30.into())),
1695+
text: "1".to_string(),
1696+
},
1697+
],
1698+
};
1699+
1700+
let _changes = doc.apply_file_change(&change1);
1701+
1702+
assert_eq!(
1703+
doc.content,
1704+
"select 1, 2, 223223131339331931 from unknown_users;\n"
1705+
);
1706+
1707+
assert_document_integrity(&doc);
1708+
}
1709+
16661710
#[test]
16671711
fn test_comments_only() {
16681712
let path = PgTPath::new("test.sql");

0 commit comments

Comments
 (0)