leyoda/cosmic-text
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix: Undo and redo correctly updates editor modified status (#244)

Browse source 
* Set an index for the last saved change

I added an index that represents the last saved change.

Editors are considered to be unsaved or modified if the current change
is different from the save index. In other words, if the last saved
change is `5`, undoing or redoing past that change should indicate that
the editor has been modified.

This is needed to fix two bugs in COSMIC Edit:
* https://github.com/pop-os/cosmic-edit/issues/116
* https://github.com/pop-os/cosmic-edit/issues/128

* Unit test that confirms pivot logic works

I'll most likely simplify the API as end users don't have a way to
cleanly use `Pivot::Exact` without access to the internal command
buffer.

* Simplify save point API

* Implement more save point unit tests

A unit test for an edge case currently fails but normal usage works.

* Fix edge case for empty command index and pivot

* More save point unit tests for common use cases
This commit is contained in:
Joshua Megnauth 2024-03-25 14:55:44 +00:00 • committed by GitHub
parent b08676909f
commit ff5501d9a3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 295 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

78
src/edit/vi.rs
View file

@ -31,13 +31,14 @@ fn finish_change<'buffer, E: Edit<'buffer>>(
editor: &mut E,
commands: &mut cosmic_undo_2::Commands<Change>,
changed: &mut bool,
pivot: Option<usize>,
) -> Option<Change> {
//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<Change>, pivot: Option<usize>) -> 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<Change>,
changed: bool,
save_pivot: Option<usize>,
}
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<Change> {
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;
}

227
tests/editor_modified_state.rs Normal file
View file

@ -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<SyntaxSystem> = 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!()
// }