mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-29 20:37:24 -06:00
fix(automation): a pathless PSD or PSB save leaves the document dirty (#1547)
Headless::save recorded the saved revision only for .pcraft, so saving a document back to its own PSD or PSB wrote the file correctly and still left session.list reporting dirty: true. Automation clients that use the flag could not tell a saved document from unsaved work. The test is now "was this a layered document save" (PSD, PSB or .pcraft) rather than "was this .pcraft". A flat export (PNG, JPEG, ...) stays a copy: it leaves dirty and the document's path alone. The desktop app.save path already recorded the revision for every format, so this only affects the headless JSON-lines server and the MCP stdio server, which both route through Headless::save. Tests cover the reported sequence for both .psd and .psb -- create, reopen the saved file, edit, then doc.save with no path -- plus a control that a flat export is a copy. docs/control-protocol.md now states the outcome of doc.save for the dirty flag and the document path. Co-authored-by: Space Bunny Free <chasbigham@gmail.com>
This commit is contained in:
@@ -142,10 +142,15 @@ impl Headless {
|
||||
warnings
|
||||
}
|
||||
};
|
||||
let is_native = format
|
||||
.map(|f| f.trim_start_matches('.').eq_ignore_ascii_case("pcraft"))
|
||||
.unwrap_or_else(|| target.extension().is_some_and(|e| e.eq_ignore_ascii_case("pcraft")));
|
||||
if is_native {
|
||||
// A layered document save (PSD, PSB or .pcraft) is a full write, not a flattened copy:
|
||||
// record the current revision as saved so the session stops reporting `dirty`, and make
|
||||
// the file the document's path. A flat export (PNG, JPEG, …) is a copy and leaves both
|
||||
// alone (#1547).
|
||||
let ext = format
|
||||
.map(|f| f.trim_start_matches('.').to_ascii_lowercase())
|
||||
.or_else(|| target.extension().map(|e| e.to_string_lossy().to_ascii_lowercase()));
|
||||
let layered = matches!(ext.as_deref(), Some("psd" | "psb")) || ext.as_deref() == Some(photocraft_format::EXTENSION);
|
||||
if layered {
|
||||
// Saving by index must not retarget the next automation command.
|
||||
let previously_active = self.session.active_index();
|
||||
self.session.set_active(i);
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
//! Regression for #1547: a successful save back to the document's own PSD or PSB file must
|
||||
//! record the current revision as saved, so the session stops reporting `dirty` (as `.pcraft`
|
||||
//! already did). A flat export stays a copy and leaves the dirty state and path alone.
|
||||
|
||||
use photocraft_automation::Headless;
|
||||
use serde_json::json;
|
||||
|
||||
fn tmp(name: &str) -> std::path::PathBuf {
|
||||
let dir = std::env::temp_dir().join(format!(
|
||||
"pc-layered-save-{name}-{}-{}",
|
||||
std::process::id(),
|
||||
std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos()
|
||||
));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
dir
|
||||
}
|
||||
|
||||
/// The issue's scenario: create `name`, reopen it, edit, then save with no path. The save writes
|
||||
/// the document's own file and must clear `dirty`.
|
||||
fn pathless_save_clears_dirty(dir: &std::path::Path, name: &str) {
|
||||
let path = dir.join(name);
|
||||
let mut headless = Headless::trusted_local();
|
||||
headless.handle("doc.new", json!({"width": 8, "height": 8, "background": "white"})).unwrap();
|
||||
headless.handle("doc.save", json!({"path": path.to_str().unwrap()})).unwrap();
|
||||
assert!(path.is_file(), "{name} was written");
|
||||
|
||||
// Reopen the saved file and edit it, so the pathless save below writes back over its own file.
|
||||
headless.handle("doc.open", json!({"path": path.to_str().unwrap()})).unwrap();
|
||||
headless.handle("engine.execute", json!({"command": "image.adjustments.invert", "params": {}})).unwrap();
|
||||
assert!(headless.session.active().unwrap().is_dirty(), "{name}: the edit dirties");
|
||||
|
||||
// The reported symptom: the save succeeds but the session still reports `dirty`.
|
||||
headless.handle("doc.save", json!({})).unwrap();
|
||||
assert!(!headless.session.active().unwrap().is_dirty(), "{name}: a pathless save is clean");
|
||||
let revision = headless.session.active().unwrap().revision;
|
||||
assert_eq!(headless.session.active().unwrap().saved_revision, revision, "{name}: the current revision is recorded");
|
||||
assert_eq!(headless.session.active().unwrap().path.as_deref(), path.to_str(), "{name}: the file is still the path");
|
||||
|
||||
// A second save, with no further edit, stays clean.
|
||||
headless.handle("doc.save", json!({})).unwrap();
|
||||
assert!(!headless.session.active().unwrap().is_dirty(), "{name}: a repeat save stays clean");
|
||||
|
||||
let bytes = std::fs::read(&path).unwrap();
|
||||
if name.ends_with("psd") {
|
||||
assert!(photocraft_io::is_psd(&bytes), "{name}: PSD magic");
|
||||
} else {
|
||||
assert_eq!(&bytes[..4], b"8BPS", "{name}: PSB magic");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pathless_psd_and_psb_saves_clear_dirty() {
|
||||
let dir = tmp("clean");
|
||||
pathless_save_clears_dirty(&dir, "a.psd");
|
||||
pathless_save_clears_dirty(&dir, "a.psb");
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
|
||||
/// The control from the issue: exporting a flat copy is not a document save, so it must neither
|
||||
/// clear `dirty` nor retarget the document's path. A `.pcraft` save sets the path on both the old
|
||||
/// and the new code, so this test isolates the flat-export behaviour.
|
||||
#[test]
|
||||
fn a_flat_export_is_a_copy_and_leaves_the_document_dirty() {
|
||||
let dir = tmp("copy");
|
||||
let native = dir.join("doc.pcraft");
|
||||
let png = dir.join("copy.png");
|
||||
let mut headless = Headless::trusted_local();
|
||||
headless.handle("doc.new", json!({"width": 8, "height": 8, "background": "white"})).unwrap();
|
||||
headless.handle("doc.save", json!({"path": native.to_str().unwrap()})).unwrap();
|
||||
headless.handle("engine.execute", json!({"command": "image.adjustments.invert", "params": {}})).unwrap();
|
||||
assert!(headless.session.active().unwrap().is_dirty(), "the edit dirties");
|
||||
|
||||
headless.handle("doc.save", json!({"path": png.to_str().unwrap()})).unwrap();
|
||||
assert!(png.is_file(), "the PNG copy was written");
|
||||
assert!(headless.session.active().unwrap().is_dirty(), "a flat export leaves the document dirty");
|
||||
assert_eq!(headless.session.active().unwrap().path.as_deref(), native.to_str(), "the path is not retargeted to the copy");
|
||||
|
||||
// ...and the document can still be saved back to its own native file.
|
||||
headless.handle("doc.save", json!({})).unwrap();
|
||||
assert!(!headless.session.active().unwrap().is_dirty());
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
@@ -275,7 +275,7 @@ no MCP framing, no app start-up per command. Configure its file access with the
|
||||
| `engine.commands` | `{filter?}`: registry with params docs and enablement |
|
||||
| `session.list` | open documents and the active index |
|
||||
| `doc.open` / `doc.new` | `{path}` / `file.new` params |
|
||||
| `doc.save` | `{path?, format?, quality?, index?}` (`.pcraft` native, else export by extension). Without `path` only a PSD, PSB or `.pcraft` document is written back to its own file, in its own format; anything else is an error and the file is left unchanged |
|
||||
| `doc.save` | `{path?, format?, quality?, index?}` (`.pcraft` native, else export by extension). Without `path` only a PSD, PSB or `.pcraft` document is written back to its own file, in its own format; anything else is an error and the file is left unchanged. A successful layered save (PSD, PSB or `.pcraft`) records the current revision as saved and makes the file the document's path, so `session.list` reports `dirty: false`; a flat export is a copy and leaves the document dirty |
|
||||
| `doc.inspect` | `{index?}`: same JSON as `document.inspect` |
|
||||
| `doc.render` | `{index?, maxSide? (1024; 0 = full), path?}`: PNG to `path`, else `{mime, base64}` |
|
||||
| `doc.select` / `doc.close` | `{index}` / `{index?}` |
|
||||
|
||||
Reference in New Issue
Block a user