forked from 1b-agent/fieldcraft-upstream
Undo: a new model is untitled, and undoing it brings the old file back (#214)
geometry.open with "model": "new" cleared the model but kept the session's file path, so File > Save wrote the new model over the previous one. The path and the revision saved there (which document the session is) were not part of the undo state. A step now records the document when its command changed it: the new model is untitled and unsaved; undo restores the old file and its clean state, redo the untitled model (or the file it was saved to since). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UhhZMBTviDQxZvwnyP89fo
This commit is contained in:
1 parent
2188d891ff
commit
06b98f01c7
3 files changed
+90
-4
No files matched your search
@@ -14,7 +14,7 @@ use crate::{CommandKind, CommandSpec, EngineError, Result, Session};
|
|||||||
pub const OPEN: CommandSpec = CommandSpec {
|
pub const OPEN: CommandSpec = CommandSpec {
|
||||||
id: "geometry.open",
|
id: "geometry.open",
|
||||||
label: "Open Design",
|
label: "Open Design",
|
||||||
params: r#"{"path": "part.solidcraft", "model"?: "keep"} — rebuilds the SolidCraft design and makes its bodies the geometry. `model`: "keep" (default) keeps the mesh and every definition, so opening the same design again after an edit picks up the change and sets follow their persistent face names; "new" starts a new model with the design (no mesh, sets, materials, sections, BCs, loads or steps; the unit system stays). The reply's `related` is false when the model (before the open) refers to bodies (mesh tags, set geometry) and the design has none of them, and `stale` lists what no longer resolves: `mesh_bodies` (bodies the mesh is tagged with that the design lacks) and `sets` (sets naming a body or face it lacks). One undo step"#,
|
params: r#"{"path": "part.solidcraft", "model"?: "keep"} — rebuilds the SolidCraft design and makes its bodies the geometry. `model`: "keep" (default) keeps the mesh and every definition, so opening the same design again after an edit picks up the change and sets follow their persistent face names; "new" starts a new model with the design (no mesh, sets, materials, sections, BCs, loads or steps; the unit system stays; the model is untitled until saved, and undo restores the previous file). The reply's `related` is false when the model (before the open) refers to bodies (mesh tags, set geometry) and the design has none of them, and `stale` lists what no longer resolves: `mesh_bodies` (bodies the mesh is tagged with that the design lacks) and `sets` (sets naming a body or face it lacks). One undo step"#,
|
||||||
form: Some(super::forms::OPEN_DESIGN),
|
form: Some(super::forms::OPEN_DESIGN),
|
||||||
kind: CommandKind::Model,
|
kind: CommandKind::Model,
|
||||||
run: open,
|
run: open,
|
||||||
@@ -68,6 +68,9 @@ fn open(s: &mut Session, p: &Value) -> Result<Value> {
|
|||||||
s.surfaces.clear();
|
s.surfaces.clear();
|
||||||
s.sources.meshes.clear();
|
s.sources.meshes.clear();
|
||||||
s.results_view = None;
|
s.results_view = None;
|
||||||
|
// A new model is untitled: saving asks for a file instead of overwriting the old model's
|
||||||
|
// (undo brings the old file back with the old model).
|
||||||
|
s.set_document(Default::default());
|
||||||
fit.mesh_bodies.clear();
|
fit.mesh_bodies.clear();
|
||||||
fit.sets.clear();
|
fit.sets.clear();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -6,6 +6,11 @@
|
|||||||
//! journal always replays to the current model, and the undo steps are the journal's last
|
//! journal always replays to the current model, and the undo steps are the journal's last
|
||||||
//! entries, newest last. A new command clears the redo stack.
|
//! entries, newest last. A new command clears the redo stack.
|
||||||
//!
|
//!
|
||||||
|
//! A step also carries the session's [`Document`] (its file and the revision saved there) when
|
||||||
|
//! its command changed which document the session is, as `geometry.open` with `"model": "new"`
|
||||||
|
//! does: undo makes the previous file current again, redo the new untitled model. Other steps
|
||||||
|
//! leave the document alone, so saving between edits and then undoing them keeps the file.
|
||||||
|
//!
|
||||||
//! History is bounded by step count ([`MAX_UNDO_STEPS`]) and by an estimate of the memory the
|
//! History is bounded by step count ([`MAX_UNDO_STEPS`]) and by an estimate of the memory the
|
||||||
//! kept states hold ([`DEFAULT_UNDO_MEMORY`], [`Session::set_undo_memory`]): when either is
|
//! kept states hold ([`DEFAULT_UNDO_MEMORY`], [`Session::set_undo_memory`]): when either is
|
||||||
//! exceeded, the oldest undo steps go first, then the redo steps farthest from the current state.
|
//! exceeded, the oldest undo steps go first, then the redo steps farthest from the current state.
|
||||||
@@ -46,6 +51,16 @@ pub(crate) struct State {
|
|||||||
surfaces: Vec<Arc<TaggedSurface>>,
|
surfaces: Vec<Arc<TaggedSurface>>,
|
||||||
sources: fieldcraft_io::Geometry,
|
sources: fieldcraft_io::Geometry,
|
||||||
revision: u64,
|
revision: u64,
|
||||||
|
/// The document before the step, when the step changed it.
|
||||||
|
pub(crate) document: Option<Document>,
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Which document the session is: the `.fieldcraft` file it belongs to and the revision saved
|
||||||
|
/// there ([`Session::is_dirty`]).
|
||||||
|
#[derive(Debug, Clone, Default, PartialEq, Eq)]
|
||||||
|
pub(crate) struct Document {
|
||||||
|
pub(crate) path: Option<String>,
|
||||||
|
pub(crate) saved_revision: Option<u64>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
@@ -147,6 +162,19 @@ impl Session {
|
|||||||
surfaces: self.surfaces.clone(),
|
surfaces: self.surfaces.clone(),
|
||||||
sources: self.sources.clone(),
|
sources: self.sources.clone(),
|
||||||
revision: self.revision,
|
revision: self.revision,
|
||||||
|
document: None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(crate) fn document(&self) -> Document {
|
||||||
|
Document { path: self.path.clone(), saved_revision: self.saved_revision }
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Make `document` current; returns the one it replaces.
|
||||||
|
pub(crate) fn set_document(&mut self, document: Document) -> Document {
|
||||||
|
Document {
|
||||||
|
path: std::mem::replace(&mut self.path, document.path),
|
||||||
|
saved_revision: std::mem::replace(&mut self.saved_revision, document.saved_revision),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -158,6 +186,7 @@ impl Session {
|
|||||||
surfaces: std::mem::replace(&mut self.surfaces, state.surfaces),
|
surfaces: std::mem::replace(&mut self.surfaces, state.surfaces),
|
||||||
sources: std::mem::replace(&mut self.sources, state.sources),
|
sources: std::mem::replace(&mut self.sources, state.sources),
|
||||||
revision: std::mem::replace(&mut self.revision, state.revision),
|
revision: std::mem::replace(&mut self.revision, state.revision),
|
||||||
|
document: state.document.map(|d| self.set_document(d)),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -218,7 +218,7 @@ impl Session {
|
|||||||
let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.chars().take(100).collect()))?;
|
let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.chars().take(100).collect()))?;
|
||||||
// Session commands (open, save, undo, redo) change the session only in their last step,
|
// Session commands (open, save, undo, redo) change the session only in their last step,
|
||||||
// after everything that can fail, so they need no copy of the model to restore.
|
// after everything that can fail, so they need no copy of the model to restore.
|
||||||
let before = (spec.kind == CommandKind::Model).then(|| self.snapshot());
|
let before = (spec.kind == CommandKind::Model).then(|| (self.snapshot(), self.document()));
|
||||||
let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| (spec.run)(self, params)));
|
let outcome = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| (spec.run)(self, params)));
|
||||||
let result = match outcome {
|
let result = match outcome {
|
||||||
Ok(r) => r,
|
Ok(r) => r,
|
||||||
@@ -228,10 +228,15 @@ impl Session {
|
|||||||
Err(EngineError::Internal { command: id.to_string(), message })
|
Err(EngineError::Internal { command: id.to_string(), message })
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
if let Some(before) = before {
|
if let Some((mut before, document)) = before {
|
||||||
|
// A command that makes the session another document (a new model) keeps the old
|
||||||
|
// one in its undo step.
|
||||||
|
if self.document() != document {
|
||||||
|
before.document = Some(document);
|
||||||
|
}
|
||||||
if result.is_err() {
|
if result.is_err() {
|
||||||
self.restore(before);
|
self.restore(before);
|
||||||
} else if self.differs_from(&before) {
|
} else if before.document.is_some() || self.differs_from(&before) {
|
||||||
self.journal.push((id.to_string(), params.clone()));
|
self.journal.push((id.to_string(), params.clone()));
|
||||||
self.revision = next_revision();
|
self.revision = next_revision();
|
||||||
self.record_undo(before);
|
self.record_undo(before);
|
||||||
@@ -716,4 +721,53 @@ mod tests {
|
|||||||
assert!(s.execute("geometry.open", &json!({"path": path, "model": "discard"})).is_err());
|
assert!(s.execute("geometry.open", &json!({"path": path, "model": "discard"})).is_err());
|
||||||
assert!(s.design.is_none() && s.model == defined.model);
|
assert!(s.design.is_none() && s.model == defined.model);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_new_model_is_untitled_and_undo_brings_the_file_back() {
|
||||||
|
let (dir, design) = sample_design_file(&Plate::default());
|
||||||
|
let file = dir.0.join("a.fieldcraft").to_string_lossy().into_owned();
|
||||||
|
let mut s = defined_model("Seal", "no-such-face");
|
||||||
|
s.execute("file.save_as", &json!({"path": file})).unwrap();
|
||||||
|
assert!(!s.is_dirty());
|
||||||
|
|
||||||
|
s.execute("geometry.open", &json!({"path": design, "model": "new"})).unwrap();
|
||||||
|
assert_eq!(s.path, None, "a new model must not save over the old model's file");
|
||||||
|
assert!(s.is_dirty());
|
||||||
|
assert!(s.execute("file.save", &json!({})).is_err(), "save asks for a file");
|
||||||
|
|
||||||
|
s.execute("edit.undo", &json!({})).unwrap();
|
||||||
|
assert_eq!(s.path.as_deref(), Some(file.as_str()));
|
||||||
|
assert!(!s.is_dirty(), "undo returns to the saved state of the old file");
|
||||||
|
|
||||||
|
s.execute("edit.redo", &json!({})).unwrap();
|
||||||
|
assert_eq!(s.path, None);
|
||||||
|
assert!(s.is_dirty());
|
||||||
|
|
||||||
|
// Saving the new model and then editing it: undoing the edit keeps the new file.
|
||||||
|
let b = dir.0.join("b.fieldcraft").to_string_lossy().into_owned();
|
||||||
|
s.execute("file.save_as", &json!({"path": b})).unwrap();
|
||||||
|
s.execute("model.material.create", &json!({"name": "Al", "elastic": {"type": "isotropic", "young": 70000, "poisson": 0.33}})).unwrap();
|
||||||
|
s.execute("edit.undo", &json!({})).unwrap();
|
||||||
|
assert_eq!(s.path.as_deref(), Some(b.as_str()));
|
||||||
|
assert!(!s.is_dirty());
|
||||||
|
// Undoing the open after that save returns to the old file; redo to the saved new one.
|
||||||
|
s.execute("edit.undo", &json!({})).unwrap();
|
||||||
|
assert_eq!(s.path.as_deref(), Some(file.as_str()));
|
||||||
|
s.execute("edit.redo", &json!({})).unwrap();
|
||||||
|
assert_eq!(s.path.as_deref(), Some(b.as_str()));
|
||||||
|
assert!(!s.is_dirty());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn keeping_the_model_keeps_its_file_and_a_sample_has_none() {
|
||||||
|
let (dir, design) = sample_design_file(&Plate::default());
|
||||||
|
let file = dir.0.join("a.fieldcraft").to_string_lossy().into_owned();
|
||||||
|
let mut s = defined_model("Seal", "no-such-face");
|
||||||
|
s.execute("file.save_as", &json!({"path": file})).unwrap();
|
||||||
|
s.execute("geometry.open", &json!({"path": design})).unwrap();
|
||||||
|
assert_eq!(s.path.as_deref(), Some(file.as_str()));
|
||||||
|
s.execute("file.sample", &json!({"name": "cantilever"})).unwrap();
|
||||||
|
assert_eq!(s.path, None, "a sample must not save over the last opened file");
|
||||||
|
assert!(s.execute("file.save", &json!({})).is_err());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user