Repository navigation
Fix several major undo/redo issues - #979
Conversation
Dustin L. Howett (DHowett)
left a comment
Dustin L. Howett (DHowett)
left a comment
There was a problem hiding this comment.
ok I think I understand it well enough. I don't exactly see how it solves all the issues, but I trust it to an extent and have looked at every line
| /// Gap length. | ||
| gap_len: usize, | ||
| /// Increments every time the buffer is modified. | ||
| /// The current text revision. Technically, this belongs into `TextBuffer`, |
There was a problem hiding this comment.
nit: belongs to
| pub fn mark_as_dirty(&mut self) { | ||
| self.last_save_generation = self.buffer.generation().wrapping_sub(1); | ||
| // NOTE: This technically may collide after 2^32 edits. Is that a realistic concern? | ||
| self.last_save_generation = u32::MAX; |
There was a problem hiding this comment.
is this so much cheaper than an Option<u32> that it's worth the minimal risk?
There was a problem hiding this comment.
No, rather it's just a PITA to do this way. GapBuffer counts in u32 generations, right? (FWIW I did this to better support 32-bit archs, but I wouldn't mind just going with u64 on all archs. That makes wraparound effectively impossible.)
So what is a new, clean buffer? The gap buffer generation is 0. What is last_save_generation? None? None is not the same as gap buffer's Some(0)! You see where I'm getting at? PITA.
| /// It records the change in the undo stack. | ||
| fn edit_write(&mut self, text: &[u8]) { | ||
| let logical_y_before = self.cursor.logical_pos.y; | ||
| if text.is_empty() { |
There was a problem hiding this comment.
Does this indicate programmer error? Or is there a chance we get in here? Maybe an empty bracketed paste?
There was a problem hiding this comment.
It's a purely defensive addition. The function explodes with empty strings.
newline is inserted. This caused a non-adjacent
edit_writecall inan edit session, which was forbidden. We now check for non-adjacency.
something different anymore. E.g. "a", save, undo, "b" used to appear
as saved, because the generation got reset on undo. Fixed by never
decrementing the
GapBuffergeneration anymore.exactly what you saved.
grouped operation (e.g. replace-all).
occasionally merging into an older edit.
"\rX\nand deleting "X" would previously get the entire editor permanently
out of sync with reality.
and then undoing would restore the layout before the resize and get
the entire editor out of sync until the next resize. That's fixed.