Ask before Save As replaces another session
Save As, Save Copy In, Save As Template and Export Selected Tracks as New Session wrote over any existing file without asking. The Save As prompt proposes ~/<name>/<name>.scraft, so every new session suggests ~/Untitled/Untitled.scraft, and saving a second new session with the suggested path replaced the first one. These commands now refuse to write over an existing file unless `replace` is true; Save, Save As and Save Copy In may still write the open session's own file. Scripts, MCP agents and the control channel can't replace a session by accident. While the path in the prompt names an existing file, the prompt asks whether to replace it and its OK button becomes Replace; Enter alone doesn't replace. Saving also creates the session's folder, so a new session without audio can be saved to the suggested path. The CLI's --save names its output explicitly and keeps replacing it, like --bounce.
This commit is contained in:
grovebit
parent
86824b592a
commit
62a0b683e5
|
|
@ -187,7 +187,7 @@ fn finish(e: &mut Engine, args: &[String]) -> ExitCode {
|
|||
}
|
||||
}
|
||||
if let Some(path) = arg_value(args, "--save") {
|
||||
match e.execute("session.save", &json!({"path": path})) {
|
||||
match e.execute("session.save", &json!({"path": path, "replace": true})) {
|
||||
Ok(v) => outln!("{v}"),
|
||||
Err(err) => return fail(err),
|
||||
}
|
||||
|
|
|
|||
|
|
@ -54,7 +54,12 @@ pub fn tool_definitions() -> Vec<Value> {
|
|||
&[],
|
||||
),
|
||||
tool("open_session", "Open a .scraft session file.", json!({"path": {"type": "string"}}), &["path"]),
|
||||
tool("save_session", "Save the session (path required the first time).", json!({"path": {"type": "string"}}), &[]),
|
||||
tool(
|
||||
"save_session",
|
||||
"Save the session (path required the first time; replace: true to save over another existing file).",
|
||||
json!({"path": {"type": "string"}, "replace": {"type": "boolean"}}),
|
||||
&[],
|
||||
),
|
||||
tool(
|
||||
"import_audio",
|
||||
"Import an audio file (WAV, AIFF, FLAC, MP3, OGG, AAC/M4A, ALAC, CAF) onto a new or existing track.",
|
||||
|
|
|
|||
|
|
@ -15,32 +15,37 @@ pub fn specs() -> Vec<CommandSpec> {
|
|||
Ok(json!({"name": e.session().name, "tracks": e.session().tracks.len(), "missing": missing}))
|
||||
}),
|
||||
cmd!(noundo "session.close", "Close Session", ["File"], Some("Cmd+Shift+W"), "{}", always, |e, _| { e.replace_session(Session::default()); e.path = None; Ok(json!({})) }),
|
||||
cmd!(noundo "session.save", "Save Session", ["File"], Some("Cmd+S"), "{path?}", always, |e, p| {
|
||||
cmd!(noundo "session.save", "Save Session", ["File"], Some("Cmd+S"), "{path?, replace?: false}", always, |e, p| {
|
||||
let path = str_param(p, "path").map(str::to_string).or_else(|| e.path.clone()).ok_or_else(|| bad("session.save", "no path yet: pass `path` (Save As)"))?;
|
||||
refuse_replacing(e, "session.save", &crate::io::session_file(&path), p)?;
|
||||
let written = crate::io::save_session(e, &path)?;
|
||||
Ok(json!({"path": path, "audio_files_written": written}))
|
||||
}),
|
||||
cmd!(noundo "session.save_as", "Save Session As...", ["File"], Some("Cmd+Shift+S"), "{path}", always, |e, p| {
|
||||
cmd!(noundo "session.save_as", "Save Session As...", ["File"], Some("Cmd+Shift+S"), "{path, replace?: false}", always, |e, p| {
|
||||
let path = str_param(p, "path").ok_or_else(|| bad("session.save_as", "`path` required"))?.to_string();
|
||||
refuse_replacing(e, "session.save_as", &crate::io::session_file(&path), p)?;
|
||||
let written = crate::io::save_session(e, &path)?;
|
||||
Ok(json!({"path": path, "audio_files_written": written}))
|
||||
}),
|
||||
cmd!(noundo "session.save_copy", "Save Session Copy In...", ["File"], None, "{path}", always, |e, p| {
|
||||
cmd!(noundo "session.save_copy", "Save Session Copy In...", ["File"], None, "{path, replace?: false}", always, |e, p| {
|
||||
let path = str_param(p, "path").ok_or_else(|| bad("session.save_copy", "`path` required"))?.to_string();
|
||||
refuse_replacing(e, "session.save_copy", &crate::io::session_file(&path), p)?;
|
||||
let keep = e.path.clone();
|
||||
let written = crate::io::save_session(e, &path)?;
|
||||
e.path = keep;
|
||||
Ok(json!({"path": path, "audio_files_written": written}))
|
||||
}),
|
||||
cmd!(noundo "session.save_template", "Save As Template...", ["File"], None, "{path}", always, |e, p| {
|
||||
cmd!(noundo "session.save_template", "Save As Template...", ["File"], None, "{path, replace?: false}", always, |e, p| {
|
||||
let path = str_param(p, "path").ok_or_else(|| bad("session.save_template", "`path` required"))?.to_string();
|
||||
let file = crate::io::session_file(&path);
|
||||
refuse_replacing(e, "session.save_template", &file, p)?;
|
||||
let mut s = e.session().clone();
|
||||
for t in &mut s.tracks { for pl in &mut t.playlists { pl.clips.clear() } }
|
||||
s.sources.clear();
|
||||
s.videos.clear();
|
||||
let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?;
|
||||
std::fs::write(&path, text).map_err(|err| EngineError::Io(format!("{path}: {err}")))?;
|
||||
Ok(json!({"path": path}))
|
||||
std::fs::write(&file, text).map_err(|err| EngineError::Io(format!("{}: {err}", file.display())))?;
|
||||
Ok(json!({"path": file}))
|
||||
}),
|
||||
cmd!(noundo "session.revert", "Revert Session to Saved...", ["File"], None, "{}", always, |e, _| {
|
||||
let path = e.path.clone().ok_or_else(|| bad("session.revert", "the session has not been saved"))?;
|
||||
|
|
@ -115,8 +120,9 @@ pub fn specs() -> Vec<CommandSpec> {
|
|||
let n = crate::io::export_clips(e, &ids, &dir)?;
|
||||
Ok(json!({"written": n}))
|
||||
}),
|
||||
cmd!(noundo "file.export_selected_tracks_as_session", "Selected Tracks as New Session...", ["File", "Export"], None, "{path, tracks?}", has_selection, |e, p| {
|
||||
cmd!(noundo "file.export_selected_tracks_as_session", "Selected Tracks as New Session...", ["File", "Export"], None, "{path, tracks?, replace?: false}", has_selection, |e, p| {
|
||||
let path = str_param(p, "path").ok_or_else(|| bad("file.export_selected_tracks_as_session", "`path` required"))?.to_string();
|
||||
refuse_replacing(e, "file.export_selected_tracks_as_session", &crate::io::session_file(&path), p)?;
|
||||
let keep = tracks_required(e, "file.export_selected_tracks_as_session", p)?;
|
||||
let mut s = e.session().clone();
|
||||
s.tracks.retain(|t| keep.contains(&t.id) || t.kind == soundcraft_model::TrackKind::Master);
|
||||
|
|
@ -180,6 +186,14 @@ pub fn specs() -> Vec<CommandSpec> {
|
|||
]
|
||||
}
|
||||
|
||||
/// Writing a session over an existing file needs `replace: true` (see `io::replaces`).
|
||||
fn refuse_replacing(e: &Engine, id: &str, file: &std::path::Path, p: &Value) -> Result<()> {
|
||||
if crate::io::replaces(e, id, file) && !bool_or(p, "replace", false) {
|
||||
return Err(bad(id, format!("{} already exists: pass `replace: true` to replace it", file.display())));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn new_session(e: &mut Engine, p: &Value) -> Result<Value> {
|
||||
let name = str_param(p, "name").unwrap_or("Untitled").to_string();
|
||||
let sr =
|
||||
|
|
@ -380,6 +394,7 @@ mod tests {
|
|||
fn session_open_reports_missing_media() {
|
||||
let mut e = crate::demo::demo_engine();
|
||||
let dir = std::env::temp_dir().join(format!("soundcraft-open-missing-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
let path = dir.join("Gone.scraft").to_string_lossy().into_owned();
|
||||
e.execute("session.save_as", &json!({"path": path})).unwrap();
|
||||
let mut ok = Engine::default();
|
||||
|
|
|
|||
|
|
@ -474,8 +474,7 @@ pub fn save_copy(e: &Engine, path: &str) -> Result<usize> {
|
|||
/// Write `session` to `path`; returns the session with updated media paths, the number of audio
|
||||
/// files written and the final path.
|
||||
fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Session, usize, PathBuf)> {
|
||||
let p = PathBuf::from(path);
|
||||
let p = if p.extension().is_none() { p.with_extension(soundcraft_model::SESSION_EXTENSION) } else { p };
|
||||
let p = session_file(path);
|
||||
let dir = p.parent().map(Path::to_path_buf).unwrap_or_default();
|
||||
let audio_dir = dir.join("Audio Files");
|
||||
let mut s = session.clone();
|
||||
|
|
@ -519,6 +518,7 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess
|
|||
s.name = stem.to_string();
|
||||
}
|
||||
let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?;
|
||||
std::fs::create_dir_all(&dir).map_err(|err| EngineError::Io(format!("{}: {err}", dir.display())))?;
|
||||
// Atomic-ish write: temp file then rename.
|
||||
let tmp = p.with_extension("scraft.tmp");
|
||||
std::fs::write(&tmp, text).map_err(|err| EngineError::Io(format!("{}: {err}", tmp.display())))?;
|
||||
|
|
@ -526,6 +526,25 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess
|
|||
Ok((s, written, p))
|
||||
}
|
||||
|
||||
/// The file a session saved to `path` goes to: `path`, with `.scraft` added when it has no
|
||||
/// extension and isn't an existing file.
|
||||
pub fn session_file(path: &str) -> PathBuf {
|
||||
let p = PathBuf::from(path);
|
||||
if p.extension().is_none() && !p.is_file() { p.with_extension(soundcraft_model::SESSION_EXTENSION) } else { p }
|
||||
}
|
||||
|
||||
/// True when `command` would write a session over the existing `file`. Save, Save As and Save
|
||||
/// Copy In may write the open session's own file; other commands never ask.
|
||||
pub fn replaces(e: &Engine, command: &str, file: &Path) -> bool {
|
||||
match command {
|
||||
"session.save" | "session.save_as" | "session.save_copy" => {
|
||||
file.exists() && !e.path.as_deref().is_some_and(|own| same_path(Path::new(own), file))
|
||||
}
|
||||
"session.save_template" | "file.export_selected_tracks_as_session" => file.exists(),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
fn same_path(a: &Path, b: &Path) -> bool {
|
||||
let full = |p: &Path| if p.as_os_str().is_empty() { Path::new(".").canonicalize() } else { p.canonicalize() };
|
||||
a == b || matches!((full(a), full(b)), (Ok(x), Ok(y)) if x == y)
|
||||
|
|
@ -745,6 +764,34 @@ mod save_tests {
|
|||
assert_eq!(open_session(&mut r, &path("B/Moved.scraft")).unwrap(), vec![dir.join("A/Audio Files/Audio 1_02.wav").display().to_string()]);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn saving_over_another_session_needs_replace() {
|
||||
let dir = std::env::temp_dir().join(format!("soundcraft-replace-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
let path = dir.join("Song.scraft").to_string_lossy().into_owned();
|
||||
let mut first = with_take(0.5);
|
||||
first.execute("session.save_as", &json!({"path": path})).unwrap();
|
||||
first.execute("session.save", &json!({})).unwrap();
|
||||
first.execute("session.save_as", &json!({"path": dir.join("Song")})).unwrap();
|
||||
assert!(first.execute("session.save_template", &json!({"path": path})).is_err());
|
||||
let mut second = with_take(-0.25);
|
||||
let track = second.session().tracks[0].id.0;
|
||||
for id in ["session.save", "session.save_as", "session.save_copy", "session.save_template", "file.export_selected_tracks_as_session"] {
|
||||
let err = second.execute(id, &json!({"path": path, "tracks": [track]})).unwrap_err();
|
||||
assert!(err.to_string().contains("`replace: true`"), "{id}: {err}");
|
||||
}
|
||||
assert_eq!(std::fs::read_dir(dir.join("Audio Files")).unwrap().count(), 1);
|
||||
Engine::default().execute("session.save_as", &json!({"path": dir.join("New/New.scraft")})).unwrap();
|
||||
std::fs::copy(&path, dir.join("Song")).unwrap();
|
||||
let mut bare = Engine::default();
|
||||
open_session(&mut bare, &dir.join("Song").to_string_lossy()).unwrap();
|
||||
bare.execute("session.save", &json!({})).unwrap();
|
||||
second.execute("session.save_as", &json!({"path": path, "replace": true})).unwrap();
|
||||
let r = reopen(&path);
|
||||
assert_eq!(r.session().pool.get(r.session().sources[0].id).unwrap().buffer.channels[0][0], -0.25);
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
|
|||
|
|
@ -303,9 +303,17 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) {
|
|||
{
|
||||
*path = p;
|
||||
}
|
||||
if buttons(ui, "OK", enter) {
|
||||
let file = soundcraft_engine::io::session_file(path);
|
||||
let replaces = soundcraft_engine::io::replaces(&app.engine, cmd, &file);
|
||||
if replaces {
|
||||
ui.label(format!("“{}” already exists. Do you want to replace it?", file.file_name().unwrap_or_default().to_string_lossy()));
|
||||
}
|
||||
if buttons(ui, if replaces { "Replace" } else { "OK" }, enter && !replaces) {
|
||||
let mut v = json!({});
|
||||
v[key.as_str()] = json!(path);
|
||||
if replaces {
|
||||
v["replace"] = json!(true);
|
||||
}
|
||||
action = Some((cmd.clone(), v));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,38 @@
|
|||
//! Save As over another session asks first, and Enter alone never replaces it.
|
||||
use egui::vec2;
|
||||
use egui_kittest::{Harness, kittest::Queryable};
|
||||
use serde_json::json;
|
||||
use soundcraft_ui_egui::{Services, SoundApp, dialogs::Dialog};
|
||||
|
||||
#[test]
|
||||
fn saving_over_another_session_takes_the_replace_button() {
|
||||
let dir = std::env::temp_dir().join(format!("soundcraft-ui-replace-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let path = dir.join("Song.scraft");
|
||||
soundcraft_engine::Engine::default().execute("session.save_as", &json!({"path": path})).unwrap();
|
||||
let other = std::fs::read(&path).unwrap();
|
||||
let mut app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default());
|
||||
app.dialogs.open = Some(Dialog::PathPrompt {
|
||||
cmd: "session.save_as".into(),
|
||||
title: "Save Session As".into(),
|
||||
path: path.to_string_lossy().into(),
|
||||
key: "path".into(),
|
||||
});
|
||||
let mut h = Harness::builder().with_size(vec2(1600.0, 1000.0)).build_ui_state(
|
||||
|ui, app: &mut SoundApp| {
|
||||
app.logic(&ui.ctx().clone());
|
||||
app.ui(ui);
|
||||
},
|
||||
app,
|
||||
);
|
||||
h.run_steps(3);
|
||||
h.key_press(egui::Key::Enter);
|
||||
h.run_steps(3);
|
||||
assert_eq!(std::fs::read(&path).unwrap(), other);
|
||||
h.get_by_label("Replace").click();
|
||||
h.run_steps(3);
|
||||
assert_ne!(std::fs::read(&path).unwrap(), other);
|
||||
assert_eq!(h.state().engine.path, Some(path.to_string_lossy().into_owned()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
|
@ -24,7 +24,7 @@ claude mcp add soundcraft -- soundcraft-cli mcp --demo
|
|||
| `execute` | Run one command with JSON params. |
|
||||
| `batch` | Run several commands in order (`keep_going` optional). |
|
||||
| `inspect_session` | Verify work: tracks, clips, mixer, plugins, sends, automation, markers, selection; each `sources[]` entry includes `loaded` (decoded audio in the pool). |
|
||||
| `new_session`, `open_session`, `save_session` | Session files (`.scraft`, audio in `Audio Files/`). `open_session` returns `missing`: the media files that could not be loaded (empty when all are found). |
|
||||
| `new_session`, `open_session`, `save_session` | Session files (`.scraft`, audio in `Audio Files/`). `open_session` returns `missing`: the media files that could not be loaded (empty when all are found). `save_session` won't save over another existing file unless `replace` is true. |
|
||||
| `import_audio` | WAV, AIFF, FLAC, MP3, OGG, AAC/M4A, ALAC, CAF onto a new or existing track. |
|
||||
| `bounce_mix` | Render to WAV/AIFF/FLAC; returns peak dBFS and integrated LUFS. |
|
||||
| `parity` | Feature-parity report. |
|
||||
|
|
|
|||
Loading…
Reference in New Issue