feat(keymap): a save-as prompt, so a pathless document has somewhere to go
improvise-8tk made ZZ/:wq refuse to quit when the save fails, which is correct and unhelpful: with no file path the user is left holding "No file path — use :w <path>" and has to know to type a path inline. There was no save-as prompt to send them to. `AppMode::ExportPrompt` is not one — `CommitExport` writes CSV, so binding it here would have written a CSV file under whatever .improv name the user typed. This is the ticket's preferred option (a): a `SavePrompt` minibuffer mode built by the `TextEntry` builder, committing to `SaveAs` — `SaveAs` and not `Save`, because the prompt exists precisely when there is no path for `Save` to use, and because `SaveAs` adopts the path so the next Ctrl-S works. Bound to Ctrl-W in Normal, beside Ctrl-E's export prompt, and reachable as `:enter-mode save-prompt`. The status message now names it. `ZZ` is deliberately NOT routed into the prompt automatically; see the hand-back for the reasoning. The abort of improvise-8tk is untouched. Evidence. The binding dump diff is purely additive — one Normal binding (inherited by RecordsNormal) and the new mode's own keymap; no existing line moved. The mode is carried by the existing meta-tests rather than by new bespoke ones, and both were proved to reach it by injection: every_hint_key_resolves_in_its_mode fails when the new hint names an unbound Tab, and every_bound_command_name_is_registered fails when the commit name is typo'd. commit_save_writes_the_model_to_the_typed_path pins SaveAs-not-ExportCsv, and commit_save_on_an_empty_path_writes_nothing pins the empty-path bail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -202,6 +202,55 @@ mod tests {
|
||||
assert!(dbg.contains("ExportCsv"), "Expected ExportCsv, got: {dbg}");
|
||||
assert!(dbg.contains("Normal"), "Expected Normal mode, got: {dbg}");
|
||||
}
|
||||
|
||||
/// improvise-lae: the save prompt has to commit to `SaveAs` and not to
|
||||
/// `ExportCsv`, which is what makes it a separate mode from
|
||||
/// `ExportPrompt` rather than a second binding onto it — `ExportCsv`
|
||||
/// would write CSV under whatever `.improv` name the user typed.
|
||||
///
|
||||
/// `SaveAs` and not `Save`, because `Save` writes the path the document
|
||||
/// already has, and the whole reason this prompt exists is that there
|
||||
/// isn't one.
|
||||
#[test]
|
||||
fn commit_save_writes_the_model_to_the_typed_path() {
|
||||
let m = two_cat_model();
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let mut bufs = HashMap::new();
|
||||
bufs.insert("save".to_string(), "/tmp/typed.improv".to_string());
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.buffers = &bufs;
|
||||
let dbg = effects_debug(&CommitSave.execute(&ctx));
|
||||
assert!(
|
||||
dbg.contains("SaveAs(\"/tmp/typed.improv\")"),
|
||||
"Expected SaveAs at the typed path, got: {dbg}"
|
||||
);
|
||||
assert!(
|
||||
!dbg.contains("ExportCsv"),
|
||||
"the save prompt must not export CSV, got: {dbg}"
|
||||
);
|
||||
assert!(dbg.contains("Normal"), "Expected Normal mode, got: {dbg}");
|
||||
}
|
||||
|
||||
/// An empty prompt must not reach `SaveAs`: `SaveAs(PathBuf::from(""))`
|
||||
/// asks the backend to write to `""`, and the user then reads whatever
|
||||
/// I/O error that produces instead of "you typed nothing".
|
||||
#[test]
|
||||
fn commit_save_on_an_empty_path_writes_nothing() {
|
||||
let m = two_cat_model();
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let mut bufs = HashMap::new();
|
||||
bufs.insert("save".to_string(), " ".to_string());
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.buffers = &bufs;
|
||||
let dbg = effects_debug(&CommitSave.execute(&ctx));
|
||||
assert!(
|
||||
!dbg.contains("SaveAs"),
|
||||
"an empty path must not reach SaveAs, got: {dbg}"
|
||||
);
|
||||
assert!(dbg.contains("SetStatus"), "Expected a status, got: {dbg}");
|
||||
}
|
||||
}
|
||||
|
||||
// ── Commit commands (mode-specific buffer consumers) ────────────────────────
|
||||
@@ -546,6 +595,37 @@ impl Cmd for CommitBuffer {
|
||||
}
|
||||
}
|
||||
|
||||
/// Commit a save path from the save buffer.
|
||||
///
|
||||
/// The sibling of [`CommitExport`], and deliberately not the same command:
|
||||
/// the export prompt writes CSV, this one writes the *model* through
|
||||
/// `SaveAs`, which also adopts the path so a later plain `Save` has one
|
||||
/// (improvise-lae).
|
||||
///
|
||||
/// An empty path bails rather than reaching `SaveAs`, which would otherwise
|
||||
/// ask the storage backend to write to `""` and report its error instead of
|
||||
/// the one the user can act on.
|
||||
#[derive(Debug)]
|
||||
pub struct CommitSave;
|
||||
impl Cmd for CommitSave {
|
||||
fn name(&self) -> &'static str {
|
||||
"commit-save"
|
||||
}
|
||||
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
|
||||
let path = read_buffer(ctx, "save").trim().to_string();
|
||||
if path.is_empty() {
|
||||
return vec![
|
||||
effect::set_status("No path given — not saved"),
|
||||
effect::change_mode(AppMode::Normal),
|
||||
];
|
||||
}
|
||||
vec![
|
||||
Box::new(effect::SaveAs(std::path::PathBuf::from(path))),
|
||||
effect::change_mode(AppMode::Normal),
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
/// Commit an export from the export buffer.
|
||||
#[derive(Debug)]
|
||||
pub struct CommitExport;
|
||||
|
||||
@@ -600,6 +600,7 @@ pub fn default_registry() -> CmdRegistry {
|
||||
},
|
||||
);
|
||||
r.register_nullary(|| Box::new(CommitExport));
|
||||
r.register_nullary(|| Box::new(CommitSave));
|
||||
r.register_nullary(|| Box::new(ExecuteCommand));
|
||||
|
||||
// ── Wizard ───────────────────────────────────────────────────────────
|
||||
|
||||
@@ -603,6 +603,7 @@ mod tests {
|
||||
ModeKey::CategoryAdd => AppMode::category_add(),
|
||||
ModeKey::ItemAdd => AppMode::item_add("Type".to_string()),
|
||||
ModeKey::ExportPrompt => AppMode::export_prompt(),
|
||||
ModeKey::SavePrompt => AppMode::save_prompt(),
|
||||
ModeKey::CommandMode => AppMode::command_mode(),
|
||||
ModeKey::ImportWizard => AppMode::ImportWizard,
|
||||
ModeKey::RecordsNormal => AppMode::RecordsNormal,
|
||||
@@ -805,6 +806,7 @@ mod tests {
|
||||
(AppMode::category_add(), COMMIT_STAY),
|
||||
(AppMode::item_add("Region".into()), COMMIT_STAY),
|
||||
(AppMode::export_prompt(), COMMIT_LEAVE),
|
||||
(AppMode::save_prompt(), COMMIT_LEAVE),
|
||||
(AppMode::command_mode(), COMMIT_LEAVE),
|
||||
]
|
||||
}
|
||||
@@ -1023,7 +1025,7 @@ mod binding_dump {
|
||||
#[cfg(test)]
|
||||
mod tests_support {
|
||||
use super::*;
|
||||
pub const ALL_MODES: [ModeKey; 16] = [
|
||||
pub const ALL_MODES: [ModeKey; 17] = [
|
||||
ModeKey::Normal,
|
||||
ModeKey::Help,
|
||||
ModeKey::FormulaPanel,
|
||||
@@ -1035,6 +1037,7 @@ mod tests_support {
|
||||
ModeKey::CategoryAdd,
|
||||
ModeKey::ItemAdd,
|
||||
ModeKey::ExportPrompt,
|
||||
ModeKey::SavePrompt,
|
||||
ModeKey::CommandMode,
|
||||
ModeKey::SearchMode,
|
||||
ModeKey::ImportWizard,
|
||||
|
||||
@@ -769,7 +769,7 @@ fn save_to_current_path(
|
||||
state: &crate::ui::app::ModelState,
|
||||
) -> Result<String, String> {
|
||||
state.file_path.as_ref().map_or_else(
|
||||
|| Err("No file path — use :w <path>".to_string()),
|
||||
|| Err("No file path — Ctrl-W to name one, or :w <path>".to_string()),
|
||||
|path| {
|
||||
crate::persistence::save_to(storage, &state.workbook, path)
|
||||
.map(|()| format!("Saved to {}", path.display()))
|
||||
|
||||
@@ -89,6 +89,7 @@ pub enum ModeKey {
|
||||
CategoryAdd,
|
||||
ItemAdd,
|
||||
ExportPrompt,
|
||||
SavePrompt,
|
||||
CommandMode,
|
||||
SearchMode,
|
||||
ImportWizard,
|
||||
@@ -111,6 +112,7 @@ impl ModeKey {
|
||||
AppMode::CategoryAdd { .. } => Some(ModeKey::CategoryAdd),
|
||||
AppMode::ItemAdd { .. } => Some(ModeKey::ItemAdd),
|
||||
AppMode::ExportPrompt { .. } => Some(ModeKey::ExportPrompt),
|
||||
AppMode::SavePrompt { .. } => Some(ModeKey::SavePrompt),
|
||||
AppMode::CommandMode { .. } => Some(ModeKey::CommandMode),
|
||||
AppMode::ImportWizard => Some(ModeKey::ImportWizard),
|
||||
AppMode::RecordsNormal => Some(ModeKey::RecordsNormal),
|
||||
@@ -618,6 +620,15 @@ impl Keymaps {
|
||||
"enter-mode",
|
||||
vec!["export-prompt".into()],
|
||||
);
|
||||
// Ctrl-S saves to the known path; Ctrl-W asks for one. The pair a
|
||||
// user with no file path needs, and what the "No file path" status
|
||||
// message points at (improvise-lae).
|
||||
normal.bind_args(
|
||||
KeyCode::Char('w'),
|
||||
ctrl,
|
||||
"enter-mode",
|
||||
vec!["save-prompt".into()],
|
||||
);
|
||||
|
||||
// Search / category add
|
||||
normal.bind_args(
|
||||
@@ -907,6 +918,15 @@ impl Keymaps {
|
||||
),
|
||||
);
|
||||
|
||||
set.insert(
|
||||
ModeKey::SavePrompt,
|
||||
Arc::new(
|
||||
TextEntry::new(&AppMode::save_prompt(), "normal")
|
||||
.commit(KeyCode::Enter, "commit-save", vec![])
|
||||
.finish(),
|
||||
),
|
||||
);
|
||||
|
||||
set.insert(
|
||||
ModeKey::CommandMode,
|
||||
Arc::new(
|
||||
|
||||
@@ -175,6 +175,11 @@ pub enum AppMode {
|
||||
ExportPrompt {
|
||||
minibuf: MinibufferConfig,
|
||||
},
|
||||
/// Ask for a path to write the *model* to. The sibling of `ExportPrompt`,
|
||||
/// which writes CSV; this one commits to `SaveAs` (improvise-lae).
|
||||
SavePrompt {
|
||||
minibuf: MinibufferConfig,
|
||||
},
|
||||
/// Vim-style `:` command line
|
||||
CommandMode {
|
||||
minibuf: MinibufferConfig,
|
||||
@@ -223,6 +228,7 @@ impl AppMode {
|
||||
"records-editing" => Some(Self::records_editing()),
|
||||
"formula-edit" => Some(Self::formula_edit()),
|
||||
"export-prompt" => Some(Self::export_prompt()),
|
||||
"save-prompt" => Some(Self::save_prompt()),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
@@ -241,6 +247,7 @@ impl AppMode {
|
||||
Self::TileSelect => "TILES",
|
||||
Self::ImportWizard => "IMPORT",
|
||||
Self::ExportPrompt { .. } => "EXPORT",
|
||||
Self::SavePrompt { .. } => "SAVE AS",
|
||||
Self::CommandMode { .. } => "COMMAND",
|
||||
Self::Help => "HELP",
|
||||
Self::Quit => "QUIT",
|
||||
@@ -278,6 +285,7 @@ impl AppMode {
|
||||
Self::ImportWizard => "Space:toggle c:cycle Enter:next Esc:cancel",
|
||||
Self::Help => "h/l:pages q/Esc:close",
|
||||
Self::ExportPrompt { .. } => "Enter:export Esc:cancel — type a file path",
|
||||
Self::SavePrompt { .. } => "Enter:save Esc:cancel — type a file path",
|
||||
Self::Quit => "",
|
||||
}
|
||||
}
|
||||
@@ -297,6 +305,7 @@ impl AppMode {
|
||||
| Self::TileSelect
|
||||
| Self::ImportWizard
|
||||
| Self::ExportPrompt { .. }
|
||||
| Self::SavePrompt { .. }
|
||||
| Self::CommandMode { .. }
|
||||
| Self::Help
|
||||
| Self::Quit
|
||||
@@ -314,7 +323,8 @@ impl AppMode {
|
||||
| Self::CommandMode { minibuf, .. }
|
||||
| Self::CategoryAdd { minibuf, .. }
|
||||
| Self::ItemAdd { minibuf, .. }
|
||||
| Self::ExportPrompt { minibuf, .. } => Some(minibuf),
|
||||
| Self::ExportPrompt { minibuf, .. }
|
||||
| Self::SavePrompt { minibuf, .. } => Some(minibuf),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
@@ -400,6 +410,16 @@ impl AppMode {
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
pub fn save_prompt() -> Self {
|
||||
Self::SavePrompt {
|
||||
minibuf: MinibufferConfig {
|
||||
buffer_key: "save".into(),
|
||||
prompt: "save as: ".into(),
|
||||
color: MinibufColor::Yellow,
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// UI session-state slice: mode, cursors, panels, buffers, navigation stacks,
|
||||
|
||||
Reference in New Issue
Block a user