diff --git a/crates/shift-font/src/ir/font.rs b/crates/shift-font/src/ir/font.rs index 7bc8acbc..b7f11ac7 100644 --- a/crates/shift-font/src/ir/font.rs +++ b/crates/shift-font/src/ir/font.rs @@ -391,6 +391,13 @@ impl Font { &self.data().sources } + pub fn source_mut(&mut self, source_id: SourceId) -> Option<&mut Source> { + self.data_mut() + .sources + .iter_mut() + .find(|source| source.id() == source_id) + } + pub fn add_source(&mut self, source: Source) -> SourceId { let source_id = source.id(); let data = self.data_mut(); diff --git a/crates/shift-workspace/src/ledger.rs b/crates/shift-workspace/src/ledger.rs index 6ebcf556..6a42631e 100644 --- a/crates/shift-workspace/src/ledger.rs +++ b/crates/shift-workspace/src/ledger.rs @@ -7,7 +7,7 @@ //! a renderer reload (it lives with the workspace process), not a utility //! crash; a SQLite ledger table is the later upgrade if that ever matters. -use shift_font::{Axis, Glyph, GlyphId, GlyphLayer, Source}; +use shift_font::{Axis, Glyph, GlyphId, GlyphLayer, GlyphName, Source, SourceId}; /// Generous bound so a marathon session cannot grow memory unboundedly; /// oldest entries fall off first. @@ -26,6 +26,10 @@ pub enum LedgerStep { Axis { pre: Option, post: Option, + /// Source location values on this axis at pre time. Deleting an + /// axis strips them from every source, so restoring the axis + /// restores them too. + pre_locations: Vec<(SourceId, f64)>, }, /// Source existence. Sparse glyph-layer existence is represented by /// separate [`LedgerStep::GlyphLayer`] entries. @@ -39,6 +43,20 @@ pub enum LedgerStep { pre: Option>, post: Option>, }, + /// Glyph rename / unicode reassignment. Both sides always exist; the + /// glyph and its layers are untouched. + GlyphIdentity { + glyph_id: GlyphId, + pre: GlyphIdentity, + post: GlyphIdentity, + }, +} + +/// One side of a glyph identity change: the name and unicode assignments. +#[derive(Clone)] +pub struct GlyphIdentity { + pub name: GlyphName, + pub unicodes: Vec, } #[derive(Clone)] @@ -71,8 +89,9 @@ impl Ledger { } /// Pops the entry to undo; the caller replays its pre states and must - /// hand the entry back via [`Ledger::record_undone`] only after the - /// replay durably succeeded. + /// hand the entry back — via [`Ledger::record_undone`] after the replay + /// durably succeeded, or [`Ledger::restore_undo`] when it failed so the + /// step stays available for retry. pub fn pop_undo(&mut self) -> Option { self.undo.pop() } @@ -81,8 +100,14 @@ impl Ledger { self.redo.push(entry); } + /// Hands a popped undo entry back after a failed replay. + pub fn restore_undo(&mut self, entry: LedgerEntry) { + self.undo.push(entry); + } + /// Pops the entry to redo; hand back via [`Ledger::record_redone`] after - /// the replay durably succeeded. + /// the replay durably succeeded, or [`Ledger::restore_redo`] when it + /// failed so the step stays available for retry. pub fn pop_redo(&mut self) -> Option { self.redo.pop() } @@ -90,4 +115,9 @@ impl Ledger { pub fn record_redone(&mut self, entry: LedgerEntry) { self.undo.push(entry); } + + /// Hands a popped redo entry back after a failed replay. + pub fn restore_redo(&mut self, entry: LedgerEntry) { + self.redo.push(entry); + } } diff --git a/crates/shift-workspace/src/workspace.rs b/crates/shift-workspace/src/workspace.rs index 037c9680..b40a0862 100644 --- a/crates/shift-workspace/src/workspace.rs +++ b/crates/shift-workspace/src/workspace.rs @@ -5,14 +5,14 @@ use std::{ use shift_backends::{FontExportRequest, FontExportResult, FontExporter, font_loader::FontLoader}; use shift_font::{ - AppliedIntents, Axis, FontChange, FontChangeSet, FontIntent, FontIntentSet, Glyph, GlyphId, - GlyphLayer, Source, TouchedLayer, error::CoreError, + AppliedIntents, Axis, AxisId, FontChange, FontChangeSet, FontIntent, FontIntentSet, Glyph, + GlyphId, GlyphLayer, Source, SourceId, TouchedLayer, error::CoreError, }; use shift_source::ShiftSourcePackage; use shift_store::{ShiftStore, SourceIdentitySnapshot, WorkspaceSourceKind, WorkspaceState}; use crate::NewWorkspace; -use crate::ledger::{LayerPair, Ledger, LedgerEntry, LedgerStep}; +use crate::ledger::{GlyphIdentity, LayerPair, Ledger, LedgerEntry, LedgerStep}; use crate::source_identity::{ RecoverySelection, WorkspaceRecoveryCandidate, select_recoverable_package_workspace, source_identity_snapshot, validate_source_identity_for_save, @@ -192,9 +192,18 @@ impl FontWorkspace { ) -> Result { let mut pre_layers: Vec = Vec::new(); let mut pre_sources: Vec = Vec::new(); + let mut pre_axes: Vec = Vec::new(); + let mut pre_axis_locations: Vec<(AxisId, SourceId, f64)> = Vec::new(); for intent in &set.intents { let Some(layer_id) = intent.layer_id() else { - capture_font_level_pre_state(&self.font, intent, &mut pre_layers, &mut pre_sources); + capture_font_level_pre_state( + &self.font, + intent, + &mut pre_layers, + &mut pre_sources, + &mut pre_axes, + &mut pre_axis_locations, + ); continue; }; if pre_layers.iter().any(|pre| pre.layer.id() == *layer_id) { @@ -216,7 +225,13 @@ impl FontWorkspace { Ok((outcome, changes)) })?; - let steps = self.ledger_steps(&pre_layers, &pre_sources, &outcome); + let steps = self.ledger_steps( + &pre_layers, + &pre_sources, + &pre_axes, + &pre_axis_locations, + &outcome, + ); self.ledger.push(LedgerEntry { label, steps }); Ok(outcome) } @@ -227,6 +242,8 @@ impl FontWorkspace { &self, pre_layers: &[PreLayer], pre_sources: &[Source], + pre_axes: &[Axis], + pre_axis_locations: &[(AxisId, SourceId, f64)], outcome: &AppliedIntents, ) -> Vec { let mut steps = Vec::new(); @@ -267,7 +284,33 @@ impl FontWorkspace { .iter() .find(|axis| axis.id() == change.axis_id) .cloned(), + pre_locations: Vec::new(), }), + FontChange::AxisDeleted(change) => steps.push(LedgerStep::Axis { + pre: pre_axes + .iter() + .find(|axis| axis.id() == change.axis_id) + .cloned(), + post: None, + pre_locations: pre_axis_locations + .iter() + .filter(|(axis_id, _, _)| *axis_id == change.axis_id) + .map(|(_, source_id, value)| (source_id.clone(), *value)) + .collect(), + }), + FontChange::GlyphIdentityChanged(change) => { + steps.push(LedgerStep::GlyphIdentity { + glyph_id: change.glyph_id.clone(), + pre: GlyphIdentity { + name: change.from_name.clone(), + unicodes: change.from_unicodes.clone(), + }, + post: GlyphIdentity { + name: change.to_name.clone(), + unicodes: change.to_unicodes.clone(), + }, + }); + } FontChange::SourceCreated(change) => steps.push(LedgerStep::Source { pre: None, post: self @@ -313,7 +356,20 @@ impl FontWorkspace { post: None, }); } - _ => {} + // Every remaining change kind is layer-scoped and already + // captured by the LayerPair snapshots above. GlyphDeleted + // has no producing intent yet; replay is its only emitter, + // and replayed changes never re-enter the ledger. + FontChange::GlyphDeleted(_) + | FontChange::LayerMetricsChanged(_) + | FontChange::ContourAdded(_) + | FontChange::ContourOpenClosedChanged(_) + | FontChange::PointsAdded(_) + | FontChange::PointsDeleted(_) + | FontChange::PointSmoothChanged(_) + | FontChange::PointPositionsChanged(_) + | FontChange::AnchorPositionsChanged(_) + | FontChange::LayerGeometryReplaced(_) => {} } } @@ -322,26 +378,43 @@ impl FontWorkspace { /// Replays the most recent entry's pre states in reverse step order. /// `None` when the undo stack is empty. The echo is the same - /// replace-grade shape as `apply`. + /// replace-grade shape as `apply`. A failed replay hands the entry back + /// so the step stays available for retry. pub fn undo(&mut self) -> Result, WorkspaceError> { let Some(entry) = self.ledger.pop_undo() else { return Ok(None); }; - let outcome = self.replay(&entry, ReplaySide::Pre)?; - self.ledger.record_undone(entry); - Ok(Some(outcome)) + match self.replay(&entry, ReplaySide::Pre) { + Ok(outcome) => { + self.ledger.record_undone(entry); + Ok(Some(outcome)) + } + Err(error) => { + self.ledger.restore_undo(entry); + Err(error) + } + } } /// Replays the most recent undone entry's post states in step order. + /// A failed replay hands the entry back so the step stays available + /// for retry. pub fn redo(&mut self) -> Result, WorkspaceError> { let Some(entry) = self.ledger.pop_redo() else { return Ok(None); }; - let outcome = self.replay(&entry, ReplaySide::Post)?; - self.ledger.record_redone(entry); - Ok(Some(outcome)) + match self.replay(&entry, ReplaySide::Post) { + Ok(outcome) => { + self.ledger.record_redone(entry); + Ok(Some(outcome)) + } + Err(error) => { + self.ledger.restore_redo(entry); + Err(error) + } + } } fn replay( @@ -367,9 +440,13 @@ impl FontWorkspace { let (from, to) = side.orient(pre, post); replay_glyph(font, from, to, &mut changes, &mut touched)?; } - LedgerStep::Axis { pre, post } => { + LedgerStep::Axis { + pre, + post, + pre_locations, + } => { let (from, to) = side.orient(pre, post); - replay_axis(font, from, to, &mut changes)?; + replay_axis(font, from, to, &pre_locations, &mut changes)?; } LedgerStep::Source { pre, post } => { let (from, to) = side.orient(pre, post); @@ -390,6 +467,14 @@ impl FontWorkspace { &mut touched, )?; } + LedgerStep::GlyphIdentity { + glyph_id, + pre, + post, + } => { + let (from, to) = side.orient(pre, post); + replay_glyph_identity(font, glyph_id, from, to, &mut changes)?; + } } } @@ -568,29 +653,49 @@ fn capture_font_level_pre_state( intent: &FontIntent, pre_layers: &mut Vec, pre_sources: &mut Vec, + pre_axes: &mut Vec, + pre_axis_locations: &mut Vec<(AxisId, SourceId, f64)>, ) { - if let FontIntent::DeleteSource { source_id } = intent { - if !pre_sources.iter().any(|source| source.id() == *source_id) - && let Some(source) = font - .sources() - .iter() - .find(|source| source.id() == *source_id) - { - pre_sources.push(source.clone()); - } + match intent { + FontIntent::DeleteSource { source_id } => { + if !pre_sources.iter().any(|source| source.id() == *source_id) + && let Some(source) = font + .sources() + .iter() + .find(|source| source.id() == *source_id) + { + pre_sources.push(source.clone()); + } - for glyph in font.glyphs() { - let Some(layer) = glyph.layer_for_source(source_id.clone()) else { - continue; + for glyph in font.glyphs() { + let Some(layer) = glyph.layer_for_source(source_id.clone()) else { + continue; + }; + if pre_layers.iter().any(|pre| pre.layer.id() == layer.id()) { + continue; + } + pre_layers.push(PreLayer { + glyph_id: glyph.id(), + layer: layer.clone(), + }); + } + } + FontIntent::DeleteAxis { axis_id } => { + if pre_axes.iter().any(|axis| axis.id() == *axis_id) { + return; + } + let Some(axis) = font.axes().iter().find(|axis| axis.id() == *axis_id) else { + return; }; - if pre_layers.iter().any(|pre| pre.layer.id() == layer.id()) { - continue; + pre_axes.push(axis.clone()); + + for source in font.sources() { + if let Some(value) = source.location().get(axis_id) { + pre_axis_locations.push((axis_id.clone(), source.id(), value)); + } } - pre_layers.push(PreLayer { - glyph_id: glyph.id(), - layer: layer.clone(), - }); } + _ => {} } } @@ -604,7 +709,7 @@ enum ReplaySide { impl ReplaySide { /// Orients a state pair into (from, to) for this side. - fn orient(self, pre: Option, post: Option) -> (Option, Option) { + fn orient(self, pre: T, post: T) -> (T, T) { match self { Self::Pre => (post, pre), Self::Post => (pre, post), @@ -681,6 +786,7 @@ fn replay_axis( font: &mut shift_font::Font, from: Option, to: Option, + pre_locations: &[(SourceId, f64)], changes: &mut FontChangeSet, ) -> Result<(), WorkspaceError> { if let Some(axis) = from { @@ -691,12 +797,49 @@ fn replay_axis( if let Some(axis) = to { changes.push(FontChange::axis_created(&axis)); + let axis_id = axis.id(); font.add_axis(axis); + + // Removing the axis stripped its value from every source's + // location (and cascaded the rows out of the store), so restoring + // the axis restores those values too. Sources deleted in the same + // entry are skipped; their own Source step carries the location. + for (source_id, value) in pre_locations { + let Some(source) = font.source_mut(source_id.clone()) else { + continue; + }; + let mut location = source.location().clone(); + location.set(axis_id.clone(), *value); + source.set_location(location); + + let snapshot = source.clone(); + changes.push(FontChange::source_created(&snapshot)); + } } Ok(()) } +fn replay_glyph_identity( + font: &mut shift_font::Font, + glyph_id: GlyphId, + from: GlyphIdentity, + to: GlyphIdentity, + changes: &mut FontChangeSet, +) -> Result<(), WorkspaceError> { + font.rename_glyph(glyph_id.clone(), to.name.clone())?; + font.set_glyph_unicodes(glyph_id.clone(), to.unicodes.clone())?; + changes.push(FontChange::glyph_identity_changed( + glyph_id, + from.name, + to.name, + from.unicodes, + to.unicodes, + )); + + Ok(()) +} + fn replay_source( font: &mut shift_font::Font, from: Option, diff --git a/crates/shift-workspace/tests/workspace_test.rs b/crates/shift-workspace/tests/workspace_test.rs index 4c3ca10f..ec4bdc33 100644 --- a/crates/shift-workspace/tests/workspace_test.rs +++ b/crates/shift-workspace/tests/workspace_test.rs @@ -1,7 +1,8 @@ use std::{fs, path::PathBuf}; use shift_font::{ - Axis, AxisId, FontChange, FontIntent, FontIntentSet, GlyphId, LayerId, Location, SourceId, + AnchorId, AnchorSeed, Axis, AxisId, BooleanOp, ContourId, FontChange, FontIntent, + FontIntentSet, GlyphId, GlyphName, LayerId, Location, PointId, PointSeed, PointType, SourceId, error::CoreError, }; use shift_source::ShiftSourcePackage; @@ -758,6 +759,666 @@ fn apply_set_x_advance_rejects_missing_layer() { assert_eq!(workspace.font().glyph_count(), 0); } +fn workspace_with_layer() -> (tempfile::TempDir, FontWorkspace, LayerId) { + let temp = tempfile::tempdir().unwrap(); + let store_path = temp.path().join("working.sqlite"); + let mut workspace = FontWorkspace::create_untitled(&store_path, NewWorkspace::new()).unwrap(); + let source_id = workspace.font().default_source_id().unwrap(); + let glyph_id = create_glyph(&mut workspace, "A", vec![65]); + let layer_id = create_glyph_layer(&mut workspace, glyph_id, source_id); + + (temp, workspace, layer_id) +} + +fn add_square_contour( + workspace: &mut FontWorkspace, + layer_id: &LayerId, + origin: (f64, f64), + size: f64, +) -> (ContourId, Vec) { + let contour_id = ContourId::new(); + let point_ids: Vec = (0..4).map(|_| PointId::new()).collect(); + let (x, y) = origin; + let corners = [(x, y), (x + size, y), (x + size, y + size), (x, y + size)]; + let points = point_ids + .iter() + .zip(corners) + .map(|(id, (px, py))| PointSeed { + id: id.clone(), + x: px, + y: py, + point_type: PointType::OnCurve, + smooth: false, + }) + .collect(); + + workspace + .apply( + FontIntentSet { + intents: vec![ + FontIntent::AddContour { + layer_id: layer_id.clone(), + contour_id: contour_id.clone(), + closed: true, + }, + FontIntent::AddPoints { + layer_id: layer_id.clone(), + contour_id: Some(contour_id.clone()), + before: None, + points, + }, + ], + }, + None, + ) + .unwrap(); + + (contour_id, point_ids) +} + +fn add_anchor(workspace: &mut FontWorkspace, layer_id: &LayerId) -> AnchorId { + let anchor_id = AnchorId::new(); + workspace + .apply( + FontIntentSet { + intents: vec![FontIntent::AddAnchors { + layer_id: layer_id.clone(), + anchors: vec![AnchorSeed { + id: anchor_id.clone(), + name: Some("top".to_string()), + x: 100.0, + y: 700.0, + }], + }], + }, + None, + ) + .unwrap(); + + anchor_id +} + +/// Applies the intents to the layer, then verifies undo restores the exact +/// pre-apply layer and redo restores the exact post-apply layer. +fn assert_layer_undo_redo( + workspace: &mut FontWorkspace, + layer_id: &LayerId, + intents: Vec, +) { + let pre = workspace.font().layer(layer_id.clone()).unwrap().clone(); + workspace.apply(FontIntentSet { intents }, None).unwrap(); + let post = workspace.font().layer(layer_id.clone()).unwrap().clone(); + assert_ne!(pre, post, "intent should change the layer"); + + workspace.undo().unwrap().expect("intent should undo"); + assert_eq!(workspace.font().layer(layer_id.clone()).unwrap(), &pre); + + workspace.redo().unwrap().expect("intent should redo"); + assert_eq!(workspace.font().layer(layer_id.clone()).unwrap(), &post); +} + +#[test] +fn add_contour_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::AddContour { + layer_id: layer_id.clone(), + contour_id: ContourId::new(), + closed: true, + }], + ); +} + +#[test] +fn add_points_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + let contour_id = ContourId::new(); + workspace + .apply( + FontIntentSet { + intents: vec![FontIntent::AddContour { + layer_id: layer_id.clone(), + contour_id: contour_id.clone(), + closed: false, + }], + }, + None, + ) + .unwrap(); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::AddPoints { + layer_id: layer_id.clone(), + contour_id: Some(contour_id), + before: None, + points: vec![PointSeed { + id: PointId::new(), + x: 10.0, + y: 20.0, + point_type: PointType::OnCurve, + smooth: false, + }], + }], + ); +} + +#[test] +fn set_contour_closed_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + let (contour_id, _) = add_square_contour(&mut workspace, &layer_id, (0.0, 0.0), 100.0); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::SetContourClosed { + layer_id: layer_id.clone(), + contour_id, + closed: false, + }], + ); +} + +#[test] +fn move_points_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + let (_, point_ids) = add_square_contour(&mut workspace, &layer_id, (0.0, 0.0), 100.0); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::MovePoints { + layer_id: layer_id.clone(), + point_ids: vec![point_ids[0].clone(), point_ids[1].clone()], + coords: vec![5.0, 6.0, 105.0, 6.0], + }], + ); +} + +#[test] +fn set_point_smooth_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + let (_, point_ids) = add_square_contour(&mut workspace, &layer_id, (0.0, 0.0), 100.0); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::SetPointSmooth { + layer_id: layer_id.clone(), + point_id: point_ids[0].clone(), + smooth: true, + }], + ); +} + +#[test] +fn remove_points_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + let (_, point_ids) = add_square_contour(&mut workspace, &layer_id, (0.0, 0.0), 100.0); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::RemovePoints { + layer_id: layer_id.clone(), + point_ids: vec![point_ids[0].clone()], + }], + ); +} + +#[test] +fn add_anchors_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::AddAnchors { + layer_id: layer_id.clone(), + anchors: vec![AnchorSeed { + id: AnchorId::new(), + name: Some("top".to_string()), + x: 100.0, + y: 700.0, + }], + }], + ); +} + +#[test] +fn move_anchors_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + let anchor_id = add_anchor(&mut workspace, &layer_id); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::MoveAnchors { + layer_id: layer_id.clone(), + anchor_ids: vec![anchor_id], + coords: vec![150.0, 720.0], + }], + ); +} + +#[test] +fn remove_anchors_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + let anchor_id = add_anchor(&mut workspace, &layer_id); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::RemoveAnchors { + layer_id: layer_id.clone(), + anchor_ids: vec![anchor_id], + }], + ); +} + +#[test] +fn reverse_contour_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + let (contour_id, _) = add_square_contour(&mut workspace, &layer_id, (0.0, 0.0), 100.0); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::ReverseContour { + layer_id: layer_id.clone(), + contour_id, + }], + ); +} + +#[test] +fn translate_points_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + let (_, point_ids) = add_square_contour(&mut workspace, &layer_id, (0.0, 0.0), 100.0); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::TranslatePoints { + layer_id: layer_id.clone(), + point_ids, + dx: 10.0, + dy: -5.0, + }], + ); +} + +#[test] +fn set_x_advance_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::SetXAdvance { + layer_id: layer_id.clone(), + width: 640.0, + }], + ); +} + +#[test] +fn apply_boolean_op_undo_redo_restores_layer() { + let (_temp, mut workspace, layer_id) = workspace_with_layer(); + let (contour_a, _) = add_square_contour(&mut workspace, &layer_id, (0.0, 0.0), 100.0); + let (contour_b, _) = add_square_contour(&mut workspace, &layer_id, (50.0, 50.0), 100.0); + + assert_layer_undo_redo( + &mut workspace, + &layer_id, + vec![FontIntent::ApplyBooleanOp { + layer_id: layer_id.clone(), + contour_id_a: contour_a, + contour_id_b: contour_b, + operation: BooleanOp::Union, + }], + ); +} + +#[test] +fn update_glyph_undo_redo_restores_old_identity() { + let temp = tempfile::tempdir().unwrap(); + let store_path = temp.path().join("working.sqlite"); + let mut workspace = FontWorkspace::create_untitled(&store_path, NewWorkspace::new()).unwrap(); + let glyph_id = create_glyph(&mut workspace, "A", vec![65]); + + workspace + .apply( + FontIntentSet { + intents: vec![FontIntent::UpdateGlyph { + glyph_id: glyph_id.clone(), + new_name: GlyphName::new("A.alt").unwrap(), + new_unicodes: vec![97], + }], + }, + Some("Rename Glyph".to_string()), + ) + .unwrap(); + + let glyph = workspace.font().glyph(glyph_id.clone()).unwrap(); + assert_eq!(glyph.glyph_name().to_string(), "A.alt"); + assert_eq!(glyph.unicodes(), &[97]); + + let undone = workspace.undo().unwrap().expect("updateGlyph should undo"); + assert!(undone.changes.changes.iter().any(|change| matches!( + change, + FontChange::GlyphIdentityChanged(change) if change.glyph_id == glyph_id + ))); + let glyph = workspace.font().glyph(glyph_id.clone()).unwrap(); + assert_eq!(glyph.glyph_name().to_string(), "A"); + assert_eq!(glyph.unicodes(), &[65]); + assert_eq!( + workspace.font().glyph_id_by_name("A"), + Some(glyph_id.clone()) + ); + + let redone = workspace.redo().unwrap().expect("updateGlyph should redo"); + assert!(redone.changes.changes.iter().any(|change| matches!( + change, + FontChange::GlyphIdentityChanged(change) if change.glyph_id == glyph_id + ))); + let glyph = workspace.font().glyph(glyph_id.clone()).unwrap(); + assert_eq!(glyph.glyph_name().to_string(), "A.alt"); + assert_eq!(glyph.unicodes(), &[97]); + assert_eq!(workspace.font().glyph_id_by_name("A.alt"), Some(glyph_id)); +} + +fn weight_axis(axis_id: AxisId) -> Axis { + Axis::with_id( + axis_id, + "wght".to_string(), + "Weight".to_string(), + 100.0, + 400.0, + 900.0, + ) +} + +#[test] +fn create_axis_undo_redo_removes_and_restores_axis() { + let temp = tempfile::tempdir().unwrap(); + let store_path = temp.path().join("working.sqlite"); + let mut workspace = FontWorkspace::create_untitled(&store_path, NewWorkspace::new()).unwrap(); + let axis_id = AxisId::from_raw("axis_weight"); + + workspace + .apply( + FontIntentSet { + intents: vec![FontIntent::CreateAxis { + axis: weight_axis(axis_id.clone()), + }], + }, + Some("Create Axis".to_string()), + ) + .unwrap(); + let created = workspace.font().axes()[0].clone(); + + workspace.undo().unwrap().expect("createAxis should undo"); + assert!(workspace.font().axes().is_empty()); + + workspace.redo().unwrap().expect("createAxis should redo"); + assert_eq!(workspace.font().axes(), &[created]); +} + +#[test] +fn delete_axis_undo_redo_restores_full_axis_definition() { + let temp = tempfile::tempdir().unwrap(); + let store_path = temp.path().join("working.sqlite"); + let mut workspace = FontWorkspace::create_untitled(&store_path, NewWorkspace::new()).unwrap(); + let axis_id = AxisId::from_raw("axis_weight"); + let source_id = SourceId::from_raw("bold"); + let mut location = Location::new(); + location.set(axis_id.clone(), 700.0); + + workspace + .apply( + FontIntentSet { + intents: vec![ + FontIntent::CreateAxis { + axis: weight_axis(axis_id.clone()), + }, + FontIntent::CreateSource { + source_id: source_id.clone(), + name: "Bold".to_string(), + location, + }, + ], + }, + None, + ) + .unwrap(); + let axis = workspace.font().axes()[0].clone(); + + workspace + .apply( + FontIntentSet { + intents: vec![FontIntent::DeleteAxis { + axis_id: axis_id.clone(), + }], + }, + Some("Delete Axis".to_string()), + ) + .unwrap(); + assert!(workspace.font().axes().is_empty()); + + let undone = workspace.undo().unwrap().expect("deleteAxis should undo"); + assert!(undone.changes.changes.iter().any(|change| matches!( + change, + FontChange::AxisCreated(change) if change.axis_id == axis_id + ))); + assert_eq!(workspace.font().axes(), std::slice::from_ref(&axis)); + + // Deleting the axis also stripped source location values; undo must + // bring them back both in memory and in the store. + let restored = workspace + .font() + .sources() + .iter() + .find(|source| source.id() == source_id) + .unwrap(); + assert_eq!(restored.location().get(&axis_id), Some(700.0)); + + let store_source_id = shift_store::SourceId::new(source_id.to_string()); + let locations = workspace + .store() + .get_source_locations(&store_source_id) + .unwrap(); + assert_eq!(locations.len(), 1); + assert_eq!(locations[0].value, 700.0); + + let redone = workspace.redo().unwrap().expect("deleteAxis should redo"); + assert!(redone.changes.changes.iter().any(|change| matches!( + change, + FontChange::AxisDeleted(change) if change.axis_id == axis_id + ))); + assert!(workspace.font().axes().is_empty()); + + workspace + .undo() + .unwrap() + .expect("deleteAxis should undo again"); + assert_eq!(workspace.font().axes(), &[axis]); +} + +#[test] +fn create_source_undo_redo_removes_and_restores_source() { + let temp = tempfile::tempdir().unwrap(); + let store_path = temp.path().join("working.sqlite"); + let mut workspace = FontWorkspace::create_untitled(&store_path, NewWorkspace::new()).unwrap(); + let source_id = SourceId::from_raw("bold"); + let base_sources = workspace.font().sources().len(); + + workspace + .apply( + FontIntentSet { + intents: vec![FontIntent::CreateSource { + source_id: source_id.clone(), + name: "Bold".to_string(), + location: Location::new(), + }], + }, + Some("Create Source".to_string()), + ) + .unwrap(); + let created = workspace + .font() + .sources() + .iter() + .find(|source| source.id() == source_id) + .cloned() + .unwrap(); + + workspace.undo().unwrap().expect("createSource should undo"); + assert_eq!(workspace.font().sources().len(), base_sources); + assert!( + workspace + .font() + .sources() + .iter() + .all(|source| source.id() != source_id) + ); + + workspace.redo().unwrap().expect("createSource should redo"); + assert_eq!( + workspace + .font() + .sources() + .iter() + .find(|source| source.id() == source_id), + Some(&created) + ); +} + +#[test] +fn failed_undo_replay_hands_the_entry_back_for_retry() { + let temp = tempfile::tempdir().unwrap(); + let store_path = temp.path().join("working.sqlite"); + let mut workspace = FontWorkspace::create_untitled(&store_path, NewWorkspace::new()).unwrap(); + create_glyph(&mut workspace, "A", vec![65]); + + // Wipe the store's font rows so the undo's GlyphDeleted change has no + // row to delete and the replay fails at the persistence step. + workspace + .store_mut() + .replace_font_state(&shift_font::Font::new()) + .unwrap(); + + let error = match workspace.undo() { + Ok(_) => panic!("undo should fail when the store rejects the replay"), + Err(error) => error, + }; + assert!(matches!(error, WorkspaceError::Store(_))); + assert!(workspace.font().glyph_id_by_name("A").is_some()); + + // Repair the store; the handed-back entry must still be undoable. + let font = workspace.font().clone(); + workspace.store_mut().replace_font_state(&font).unwrap(); + + workspace + .undo() + .unwrap() + .expect("failed undo should hand the entry back for retry"); + assert_eq!(workspace.font().glyph_count(), 0); +} + +#[test] +fn failed_redo_replay_hands_the_entry_back_for_retry() { + let temp = tempfile::tempdir().unwrap(); + let store_path = temp.path().join("working.sqlite"); + let mut workspace = FontWorkspace::create_untitled(&store_path, NewWorkspace::new()).unwrap(); + let source_id = SourceId::from_raw("bold"); + + workspace + .apply( + FontIntentSet { + intents: vec![FontIntent::CreateSource { + source_id: source_id.clone(), + name: "Bold".to_string(), + location: Location::new(), + }], + }, + None, + ) + .unwrap(); + workspace + .apply( + FontIntentSet { + intents: vec![FontIntent::DeleteSource { + source_id: source_id.clone(), + }], + }, + None, + ) + .unwrap(); + workspace.undo().unwrap().expect("deleteSource should undo"); + + // Wipe the store's font rows so the redo's SourceDeleted change has no + // row to delete and the replay fails at the persistence step. + workspace + .store_mut() + .replace_font_state(&shift_font::Font::new()) + .unwrap(); + + let error = match workspace.redo() { + Ok(_) => panic!("redo should fail when the store rejects the replay"), + Err(error) => error, + }; + assert!(matches!(error, WorkspaceError::Store(_))); + assert!( + workspace + .font() + .sources() + .iter() + .any(|source| source.id() == source_id) + ); + + // Repair the store; the handed-back entry must still be redoable. + let font = workspace.font().clone(); + workspace.store_mut().replace_font_state(&font).unwrap(); + + workspace + .redo() + .unwrap() + .expect("failed redo should hand the entry back for retry"); + assert!( + workspace + .font() + .sources() + .iter() + .all(|source| source.id() != source_id) + ); +} + +#[test] +fn ledger_trims_oldest_entries_beyond_max() { + let temp = tempfile::tempdir().unwrap(); + let store_path = temp.path().join("working.sqlite"); + let mut workspace = FontWorkspace::create_untitled(&store_path, NewWorkspace::new()).unwrap(); + + // One entry more than the ledger's MAX_ENTRIES bound of 100. + for index in 0..101 { + create_glyph(&mut workspace, &format!("g{index}"), vec![]); + } + + let mut undone = 0; + while workspace.undo().unwrap().is_some() { + undone += 1; + } + + assert_eq!(undone, 100, "oldest entry should fall off the ledger"); + assert_eq!(workspace.font().glyph_count(), 1); + assert!(workspace.font().glyph_id_by_name("g0").is_some()); +} + fn fixture(path: &str) -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")) .join("../..")