diff --git a/src/edit/vi.rs b/src/edit/vi.rs index 4e50bfb691..e1913ccd2f 100644 --- a/src/edit/vi.rs +++ b/src/edit/vi.rs @@ -31,13 +31,14 @@ fn finish_change<'buffer, E: Edit<'buffer>>( editor: &mut E, commands: &mut cosmic_undo_2::Commands, changed: &mut bool, + pivot: Option, ) -> Option { //TODO: join changes together match editor.finish_change() { Some(change) => { if !change.items.is_empty() { commands.push(change.clone()); - *changed = true; + *changed = eval_changed(commands, pivot); } Some(change) } @@ -45,6 +46,24 @@ fn finish_change<'buffer, E: Edit<'buffer>>( } } +/// Evaluate if an [`ViEditor`] changed based on its last saved state. +fn eval_changed(commands: &cosmic_undo_2::Commands, pivot: Option) -> bool { + // Editors are considered modified if the current change index is unequal to the last + // saved index or if `pivot` is None. + // The latter case handles a never saved editor with a current command index of None. + // Check the unit tests for an example. + match (commands.current_command_index(), pivot) { + (Some(current), Some(pivot)) => current != pivot, + // Edge case for an editor with neither a save point nor any changes. + // This could be a new editor or an editor without a save point where undo() is called + // until the editor is fresh. + (None, None) => false, + // Default to true because it's safer to assume a buffer has been modified so as to not + // lose changes + _ => true, + } +} + fn search<'buffer, E: Edit<'buffer>>(editor: &mut E, value: &str, forwards: bool) -> bool { let mut cursor = editor.cursor(); let start_line = cursor.line; @@ -167,6 +186,7 @@ pub struct ViEditor<'syntax_system, 'buffer> { search_opt: Option<(String, bool)>, commands: cosmic_undo_2::Commands, changed: bool, + save_pivot: Option, } impl<'syntax_system, 'buffer> ViEditor<'syntax_system, 'buffer> { @@ -179,6 +199,7 @@ impl<'syntax_system, 'buffer> ViEditor<'syntax_system, 'buffer> { search_opt: None, commands: cosmic_undo_2::Commands::new(), changed: false, + save_pivot: None, } } @@ -233,6 +254,20 @@ impl<'syntax_system, 'buffer> ViEditor<'syntax_system, 'buffer> { self.changed = changed; } + /// Set current change as the save (or pivot) point. + /// + /// A pivot point is the last saved index. Anything before or after the pivot indicates that + /// the editor has been changed or is unsaved. + /// + /// Undoing changes down to the pivot point sets the editor as unchanged. + /// Redoing changes up to the pivot point sets the editor as unchanged. + /// + /// Undoing or redoing changes beyond the pivot point sets the editor to changed. + pub fn save_point(&mut self) { + self.save_pivot = Some(self.commands.current_command_index().unwrap_or_default()); + self.changed = false; + } + /// Set passthrough mode (true will turn off vi features) pub fn set_passthrough(&mut self, passthrough: bool) { if passthrough != self.passthrough { @@ -251,9 +286,8 @@ impl<'syntax_system, 'buffer> ViEditor<'syntax_system, 'buffer> { log::debug!("Redo"); for action in self.commands.redo() { undo_2_action(&mut self.editor, action); - //TODO: clear changed flag when back to last saved state? - self.changed = true; } + self.changed = eval_changed(&self.commands, self.save_pivot); } /// Undo a change @@ -261,9 +295,8 @@ impl<'syntax_system, 'buffer> ViEditor<'syntax_system, 'buffer> { log::debug!("Undo"); for action in self.commands.undo() { undo_2_action(&mut self.editor, action); - //TODO: clear changed flag when back to last saved state? - self.changed = true; } + self.changed = eval_changed(&self.commands, self.save_pivot); } #[cfg(feature = "swash")] @@ -550,7 +583,12 @@ impl<'syntax_system, 'buffer> Edit<'buffer> for ViEditor<'syntax_system, 'buffer } fn finish_change(&mut self) -> Option { - finish_change(&mut self.editor, &mut self.commands, &mut self.changed) + finish_change( + &mut self.editor, + &mut self.commands, + &mut self.changed, + self.save_pivot, + ) } fn action(&mut self, font_system: &mut FontSystem, action: Action) { @@ -564,7 +602,12 @@ impl<'syntax_system, 'buffer> Edit<'buffer> for ViEditor<'syntax_system, 'buffer if self.passthrough { editor.action(font_system, action); // Always finish change when passing through (TODO: group changes) - finish_change(editor, &mut self.commands, &mut self.changed); + finish_change( + editor, + &mut self.commands, + &mut self.changed, + self.save_pivot, + ); return; } @@ -589,7 +632,12 @@ impl<'syntax_system, 'buffer> Edit<'buffer> for ViEditor<'syntax_system, 'buffer log::debug!("Pass through action {:?}", action); editor.action(font_system, action); // Always finish change when passing through (TODO: group changes) - finish_change(editor, &mut self.commands, &mut self.changed); + finish_change( + editor, + &mut self.commands, + &mut self.changed, + self.save_pivot, + ); return; } }; @@ -620,7 +668,12 @@ impl<'syntax_system, 'buffer> Edit<'buffer> for ViEditor<'syntax_system, 'buffer return; } Event::ChangeFinish => { - finish_change(editor, &mut self.commands, &mut self.changed); + finish_change( + editor, + &mut self.commands, + &mut self.changed, + self.save_pivot, + ); return; } Event::Delete => Action::Delete, @@ -687,7 +740,12 @@ impl<'syntax_system, 'buffer> Edit<'buffer> for ViEditor<'syntax_system, 'buffer } } } - finish_change(editor, &mut self.commands, &mut self.changed); + finish_change( + editor, + &mut self.commands, + &mut self.changed, + self.save_pivot, + ); } return; } diff --git a/tests/editor_modified_state.rs b/tests/editor_modified_state.rs new file mode 100644 index 0000000000..01ac00888c --- /dev/null +++ b/tests/editor_modified_state.rs @@ -0,0 +1,227 @@ +#![cfg(feature = "vi")] + +use std::sync::OnceLock; + +use cosmic_text::{Buffer, Cursor, Edit, Metrics, SyntaxEditor, SyntaxSystem, ViEditor}; + +static SYNTAX_SYSTEM: OnceLock = OnceLock::new(); + +// New editor for tests +fn editor() -> ViEditor<'static, 'static> { + // More or less copied from cosmic-edit + let font_size: f32 = 14.0; + let line_height = (font_size * 1.4).ceil(); + + let metrics = Metrics::new(font_size, line_height); + let buffer = Buffer::new_empty(metrics); + let editor = SyntaxEditor::new( + buffer, + SYNTAX_SYSTEM.get_or_init(SyntaxSystem::new), + "base16-eighties.dark", + ) + .expect("Default theme `base16-eighties.dark` should be found"); + + ViEditor::new(editor) +} + +// Tests that inserting into an empty editor correctly sets the editor as modified. +#[test] +fn insert_in_empty_editor_sets_changed() { + let mut editor = editor(); + + assert!(!editor.changed()); + editor.start_change(); + editor.insert_at(Cursor::new(0, 0), "Robert'); DROP TABLE Students;--", None); + editor.finish_change(); + assert!(editor.changed()); +} + +// Tests an edge case where a save point is never set. +// Undoing changes should set the editor back to unmodified. +#[test] +fn insert_and_undo_in_unsaved_editor_is_unchanged() { + let mut editor = editor(); + + assert!(!editor.changed()); + editor.start_change(); + editor.insert_at(Cursor::new(0, 0), "loop {}", None); + editor.finish_change(); + assert!(editor.changed()); + + // Undoing the above change should set the editor as unchanged even if the save state is unset + editor.start_change(); + editor.undo(); + editor.finish_change(); + assert!(!editor.changed()); +} + +#[test] +fn undo_to_save_point_sets_editor_to_unchanged() { + let mut editor = editor(); + + // Latest saved change is the first change + editor.start_change(); + let cursor = editor.insert_at(Cursor::new(0, 0), "Ferris is Rust's ", None); + editor.finish_change(); + assert!( + editor.changed(), + "Editor should be set to changed after insertion" + ); + editor.save_point(); + assert!( + !editor.changed(), + "Editor should be set to unchanged after setting a save point" + ); + + // A new insert should set the editor as modified and the pivot should still be on the first + // change from earlier + editor.start_change(); + editor.insert_at(cursor, "mascot", None); + editor.finish_change(); + assert!( + editor.changed(), + "Editor should be set to changed after inserting text after a save point" + ); + + // Undoing the latest change should set the editor to unmodified again + editor.start_change(); + editor.undo(); + editor.finish_change(); + assert!( + !editor.changed(), + "Editor should be set to unchanged after undoing to save point" + ); +} + +#[test] +fn redoing_to_save_point_sets_editor_as_unchanged() { + let mut editor = editor(); + + // Initial change + assert!( + !editor.changed(), + "Editor should start in an unchanged state" + ); + editor.start_change(); + editor.insert_at(Cursor::new(0, 0), "editor.start_change();", None); + editor.finish_change(); + assert!( + editor.changed(), + "Editor should be set as modified after insert() and finish_change()" + ); + editor.save_point(); + assert!( + !editor.changed(), + "Editor should be unchanged after setting a save point" + ); + + // Change to undo then redo + editor.start_change(); + editor.insert_at(Cursor::new(1, 0), "editor.finish_change()", None); + editor.finish_change(); + assert!( + editor.changed(), + "Editor should be set as modified after insert() and finish_change()" + ); + editor.save_point(); + assert!( + !editor.changed(), + "Editor should be unchanged after setting a save point" + ); + + editor.undo(); + assert!( + editor.changed(), + "Undoing past save point should set editor as changed" + ); + editor.redo(); + assert!( + !editor.changed(), + "Redoing to save point should set editor as unchanged" + ); +} + +#[test] +fn redoing_past_save_point_sets_editor_to_changed() { + let mut editor = editor(); + + // Save point change to undo to and then redo past. + editor.start_change(); + editor.insert_string("Walt Whitman ", None); + editor.finish_change(); + + // Set save point to the change above. + assert!( + editor.changed(), + "Editor should be set as modified after insert() and finish_change()" + ); + editor.save_point(); + assert!( + !editor.changed(), + "Editor should be unchanged after setting a save point" + ); + + editor.start_change(); + editor.insert_string("Allen Ginsberg ", None); + editor.finish_change(); + + editor.start_change(); + editor.insert_string("Jack Kerouac ", None); + editor.finish_change(); + + assert!(editor.changed(), "Editor should be modified insertion"); + + // Undo to Whitman + editor.undo(); + editor.undo(); + assert!( + !editor.changed(), + "Editor should be unmodified after undoing to the save point" + ); + + // Redo to Kerouac + editor.redo(); + editor.redo(); + assert!( + editor.changed(), + "Editor should be modified after redoing past the save point" + ); +} + +#[test] +fn undoing_past_save_point_sets_editor_to_changed() { + let mut editor = editor(); + + editor.start_change(); + editor.insert_string("Robert Fripp ", None); + editor.finish_change(); + + // Save point change to undo past. + editor.start_change(); + editor.insert_string("Thurston Moore ", None); + editor.finish_change(); + + assert!(editor.changed(), "Editor should be changed after insertion"); + editor.save_point(); + assert!( + !editor.changed(), + "Editor should be unchanged after setting a save point" + ); + + editor.start_change(); + editor.insert_string("Kim Deal ", None); + editor.finish_change(); + + // Undo to the first change + editor.undo(); + editor.undo(); + assert!( + editor.changed(), + "Editor should be changed after undoing past save point" + ); +} + +// #[test] +// fn undo_all_changes() { +// unimplemented!() +// }