f00cb15 · helix · git.iko.soy

git.iko.soy/helix

on master help sign in

core: Improve changeset composition behavior.

f00cb15Blaž Hrastnik6 years ago

helix-core/src/transaction.rs+69−32

compare with master

unifiedsplit

@@ -27,15 +27,35 @@ pub struct ChangeSet {

27 pub(crate) changes: Vec<Operation>,

28 /// The required document length. Will refuse to apply changes unless it matches.

29 len: usize,

30+ len_after: usize,

31+}

32+

33+impl Default for ChangeSet {

34+ fn default() -> Self {

35+ Self {

36+ changes: Vec::new(),

37+ len: 0,

38+ len_after: 0,

39+ }

40+ }

30}

41}

32impl ChangeSet {

43impl ChangeSet {

44+ pub fn with_capacity(capacity: usize) -> Self {

45+ Self {

46+ changes: Vec::with_capacity(capacity),

47+ len: 0,

48+ len_after: 0,

49+ }

50+ }

51+

33 #[must_use]

52 #[must_use]

34 pub fn new(doc: &Rope) -> Self {

53 pub fn new(doc: &Rope) -> Self {

35 let len = doc.len_chars();

54 let len = doc.len_chars();

36 Self {

55 Self {

37− changes: vec![Operation::Retain(len)],

56+ changes: Vec::new(),

38 len,

57 len,

58+ len_after: len,

39 }

59 }

40 }

60 }

@@ -47,21 +67,6 @@ impl ChangeSet {

47 &self.changes

67 &self.changes

48 }

68 }

50− #[must_use]

51− fn len_after(&self) -> usize {

52− use Operation::*;

53−

54− let mut len = 0;

55− for change in &self.changes {

56− match change {

57− Retain(i) => len += i,

58− Insert(s) => len += s.chars().count(),

59− Delete(_) => (),

60− }

61− }

62− len

63− }

64−

65 // Changeset builder operations: delete/insert/retain

70 // Changeset builder operations: delete/insert/retain

66 fn delete(&mut self, n: usize) {

71 fn delete(&mut self, n: usize) {

67 use Operation::*;

72 use Operation::*;

@@ -69,6 +74,8 @@ impl ChangeSet {

69 return;

74 return;

70 }

75 }

77+ self.len += n;

78+

72 if let Some(Delete(count)) = self.changes.last_mut() {

79 if let Some(Delete(count)) = self.changes.last_mut() {

73 *count += n;

80 *count += n;

74 } else {

81 } else {

@@ -83,6 +90,8 @@ impl ChangeSet {

83 return;

90 return;

84 }

91 }

93+ self.len_after += fragment.len();

94+

86 let new_last = match self.changes.as_mut_slice() {

95 let new_last = match self.changes.as_mut_slice() {

87 [.., Insert(prev)] | [.., Insert(prev), Delete(_)] => {

96 [.., Insert(prev)] | [.., Insert(prev), Delete(_)] => {

88 prev.push_tendril(&fragment);

97 prev.push_tendril(&fragment);

@@ -101,6 +110,9 @@ impl ChangeSet {

101 return;

110 return;

102 }

111 }

103

112

113+ self.len += n;

114+ self.len_after += n;

115+

104 if let Some(Retain(count)) = self.changes.last_mut() {

116 if let Some(Retain(count)) = self.changes.last_mut() {

105 *count += n;

117 *count += n;

106 } else {

118 } else {

@@ -112,7 +124,13 @@ impl ChangeSet {

112 /// In other words, If `this` goes `docA` → `docB` and `other` represents `docB` → `docC`, the

124 /// In other words, If `this` goes `docA` → `docB` and `other` represents `docB` → `docC`, the

113 /// returned value will represent the change `docA` → `docC`.

125 /// returned value will represent the change `docA` → `docC`.

114 pub fn compose(self, other: Self) -> Self {

126 pub fn compose(self, other: Self) -> Self {

115− debug_assert!(self.len_after() == other.len);

127+ debug_assert!(self.len_after == other.len);

128+

129+ // composing fails in weird ways if one of the sets is empty

130+ // a: [] len: 0 len_after: 1 | b: [Insert(Tendril<UTF8>(inline: "\n")), Retain(1)] len 1

131+ if self.changes.is_empty() {

132+ return other;

133+ }

116

134

117 let len = self.changes.len();

135 let len = self.changes.len();

118

136

@@ -122,10 +140,7 @@ impl ChangeSet {

122 let mut head_a = changes_a.next();

140 let mut head_a = changes_a.next();

123 let mut head_b = changes_b.next();

141 let mut head_b = changes_b.next();

124

142

125− let mut changes = Self {

126− len: self.len,

127− changes: Vec::with_capacity(len), // TODO: max(a, b), shrink_to_fit() afterwards

143+ let mut changes = Self::with_capacity(len); // TODO: max(a, b), shrink_to_fit() afterwards

128− };

129

144

130 loop {

145 loop {

131 use std::cmp::Ordering;

146 use std::cmp::Ordering;

@@ -228,6 +243,9 @@ impl ChangeSet {

228 };

243 };

229 }

244 }

230

245

246+ // starting len should still equal original starting len

247+ debug_assert!(changes.len == self.len);

248+

231 changes

249 changes

232 }

250 }

233

251

@@ -251,7 +269,8 @@ impl ChangeSet {

251 pub fn invert(&self, original_doc: &Rope) -> Self {

269 pub fn invert(&self, original_doc: &Rope) -> Self {

252 assert!(original_doc.len_chars() == self.len);

270 assert!(original_doc.len_chars() == self.len);

253

271

254− let mut changes = Vec::with_capacity(self.changes.len());

272+ let mut changes = Self::with_capacity(self.changes.len());

273+

255 let mut pos = 0;

274 let mut pos = 0;

256 let mut len = 0;

275 let mut len = 0;

257

276

@@ -259,24 +278,22 @@ impl ChangeSet {

259 use Operation::*;

278 use Operation::*;

260 match change {

279 match change {

261 Retain(n) => {

280 Retain(n) => {

262− changes.push(Retain(*n));

281+ changes.retain(*n);

263 pos += n;

282 pos += n;

264− len += n;

265 }

283 }

266 Delete(n) => {

284 Delete(n) => {

267 let text = Cow::from(original_doc.slice(pos..pos + *n));

285 let text = Cow::from(original_doc.slice(pos..pos + *n));

268− changes.push(Insert(Tendril::from_slice(&text)));

286+ changes.insert(Tendril::from_slice(&text));

269 pos += n;

287 pos += n;

270 }

288 }

271 Insert(s) => {

289 Insert(s) => {

272 let chars = s.chars().count();

290 let chars = s.chars().count();

273− changes.push(Delete(chars));

291+ changes.delete(chars);

274− len += chars;

275 }

292 }

276 }

293 }

277 }

294 }

278

295

279− Self { changes, len }

296+ changes

280 }

297 }

281

298

282 /// Returns true if applied successfully.

299 /// Returns true if applied successfully.

@@ -309,7 +326,7 @@ impl ChangeSet {

309 /// `true` when the set is empty.

326 /// `true` when the set is empty.

310 #[inline]

327 #[inline]

311 pub fn is_empty(&self) -> bool {

328 pub fn is_empty(&self) -> bool {

312− matches!(self.changes.as_slice(), [] | [Operation::Retain(_)])

329+ self.changes.is_empty()

313 }

330 }

314

331

315 /// Map a position through the changes.

332 /// Map a position through the changes.

@@ -458,8 +475,8 @@ impl Transaction {

458 I: IntoIterator<Item = Change> + ExactSizeIterator,

475 I: IntoIterator<Item = Change> + ExactSizeIterator,

459 {

476 {

460 let len = doc.len_chars();

477 let len = doc.len_chars();

478+

461− let acc = Vec::with_capacity(2 * changes.len() + 1); // rough estimate

479+ let mut changeset = ChangeSet::with_capacity(2 * changes.len() + 1); // rough estimate

462− let mut changeset = ChangeSet { changes: acc, len };

463

480

464 // TODO: verify ranges are ordered and not overlapping or change will panic.

481 // TODO: verify ranges are ordered and not overlapping or change will panic.

465

482

@@ -576,11 +593,13 @@ mod test {

576 Insert("abc".into()),

593 Insert("abc".into()),

577 ],

594 ],

578 len: 8,

595 len: 8,

596+ len_after: 15,

579 };

597 };

580

598

581 let b = ChangeSet {

599 let b = ChangeSet {

582 changes: vec![Delete(10), Insert("world".into()), Retain(5)],

600 changes: vec![Delete(10), Insert("world".into()), Retain(5)],

583 len: 15,

601 len: 15,

602+ len_after: 10,

584 };

603 };

585

604

586 let mut text = Rope::from("hello xz");

605 let mut text = Rope::from("hello xz");

@@ -597,8 +616,9 @@ mod test {

597 use Operation::*;

616 use Operation::*;

598

617

599 let changes = ChangeSet {

618 let changes = ChangeSet {

600− changes: vec![Retain(4), Delete(5), Insert("test".into()), Retain(3)],

619+ changes: vec![Retain(4), Insert("test".into()), Delete(5), Retain(3)],

601 len: 12,

620 len: 12,

621+ len_after: 11,

602 };

622 };

603

623

604 let doc = Rope::from("世界3 hello xz");

624 let doc = Rope::from("世界3 hello xz");

@@ -627,6 +647,7 @@ mod test {

627 let cs = ChangeSet {

647 let cs = ChangeSet {

628 changes: vec![Retain(4), Insert("!!".into()), Retain(4)],

648 changes: vec![Retain(4), Insert("!!".into()), Retain(4)],

629 len: 8,

649 len: 8,

650+ len_after: 10,

630 };

651 };

631

652

632 assert_eq!(cs.map_pos(0, Assoc::Before), 0); // before insert region

653 assert_eq!(cs.map_pos(0, Assoc::Before), 0); // before insert region

@@ -638,6 +659,7 @@ mod test {

638 let cs = ChangeSet {

659 let cs = ChangeSet {

639 changes: vec![Retain(4), Delete(4), Retain(4)],

660 changes: vec![Retain(4), Delete(4), Retain(4)],

640 len: 12,

661 len: 12,

662+ len_after: 8,

641 };

663 };

642 assert_eq!(cs.map_pos(0, Assoc::Before), 0); // at start

664 assert_eq!(cs.map_pos(0, Assoc::Before), 0); // at start

643 assert_eq!(cs.map_pos(4, Assoc::Before), 4); // before a delete

665 assert_eq!(cs.map_pos(4, Assoc::Before), 4); // before a delete

@@ -655,6 +677,7 @@ mod test {

655 Delete(2),

677 Delete(2),

656 ],

678 ],

657 len: 4,

679 len: 4,

680+ len_after: 4,

658 };

681 };

659 assert_eq!(cs.map_pos(2, Assoc::Before), 2);

682 assert_eq!(cs.map_pos(2, Assoc::Before), 2);

660 assert_eq!(cs.map_pos(2, Assoc::After), 2);

683 assert_eq!(cs.map_pos(2, Assoc::After), 2);

@@ -717,4 +740,18 @@ mod test {

717 assert_eq!(changes.changes, &[Insert("hello".into())]);

740 assert_eq!(changes.changes, &[Insert("hello".into())]);

718 // instead of insert h, insert e, insert l, insert l, insert o

741 // instead of insert h, insert e, insert l, insert l, insert o

719 }

742 }

743+

744+ #[test]

745+ fn combine_with_empty() {

746+ let empty = Rope::from("");

747+ let mut a = ChangeSet::new(&empty);

748+

749+ let mut b = ChangeSet::new(&empty);

750+ b.insert("a".into());

751+

752+ let changes = a.compose(b);

753+

754+ use Operation::*;

755+ assert_eq!(changes.changes, &[Insert("a".into())]);

756+ }

720}

757}