diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index 8b85c7b5..70e8467b 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -1615,6 +1615,56 @@ mod tests { ); } + /// improvise-lae end to end: `Ctrl-W`, a path, `Enter`. + /// + /// The failing case is the interesting one, and it is what `SaveAs` + /// gaining `Save`'s abort buys the prompt. `CommitSave` emits + /// `[SaveAs, ChangeMode(Normal)]` and the keymap welds a trailing + /// `clear-buffer`, so an aborting `SaveAs` skips both: the user is left + /// in the prompt, with the path they typed still there to correct, and + /// the error on screen. Without the abort they would have been dropped + /// back to Normal with an error and an empty buffer. + #[test] + fn ctrl_w_prompts_for_a_path_and_a_failed_write_keeps_the_prompt_open() { + let mut app = App::new(Workbook::new("T"), None); + app.handle_key(KeyEvent::new(KeyCode::Char('w'), KeyModifiers::CONTROL)) + .unwrap(); + assert!( + matches!(app.view_state.mode, AppMode::SavePrompt { .. }), + "Ctrl-W should open the save prompt, got {:?}", + app.view_state.mode + ); + + let path = "/nonexistent-dir-improvise-lae/m.improv"; + for c in path.chars() { + app.handle_key(KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE)) + .unwrap(); + } + assert_eq!( + app.view_state.buffers.get("save").map(String::as_str), + Some(path), + "typing must reach the buffer the prompt names" + ); + + app.handle_key(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)) + .unwrap(); + assert!( + app.view_state.status_msg.starts_with("Save error:"), + "got {:?}", + app.view_state.status_msg + ); + assert!( + matches!(app.view_state.mode, AppMode::SavePrompt { .. }), + "a failed write must leave the prompt open, got {:?}", + app.view_state.mode + ); + assert_eq!( + app.view_state.buffers.get("save").map(String::as_str), + Some(path), + "the path the user typed must survive for them to correct" + ); + } + /// `ZZ` is save-and-quit. With no file path the save fails and aborts the /// chain (improvise-8tk), which also proves the second Z reached `wq`. #[test] diff --git a/crates/improvise-command/src/ui/effect.rs b/crates/improvise-command/src/ui/effect.rs index 73743287..50e44a86 100644 --- a/crates/improvise-command/src/ui/effect.rs +++ b/crates/improvise-command/src/ui/effect.rs @@ -788,8 +788,12 @@ impl Effect for SaveAs { app.model_state.dirty = false; app.view_state.status_msg = format!("Saved to {}", self.0.display()); } + // The same abort `Save` owes (improvise-8tk). `SaveAs` is the + // other arm of that rule and was left out of the original fix, + // so `:w /bad/path . q` quit on a failed write. Err(e) => { app.view_state.status_msg = format!("Save error: {e}"); + app.abort_effects = true; } } } @@ -1961,6 +1965,68 @@ mod tests { ); } + /// BUG (improvise-lae, found reviewing it): improvise-8tk fixed **one + /// arm** of a rule with two. `Save` aborts the batch when the write + /// fails; `SaveAs` reported the error to the status bar and let the + /// batch continue, which is the same silent discard 8tk closed — the + /// shape `doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)` + /// names, and the three tests above are evidence about `Save` only. + /// + /// It is reachable today: the `:` line composes with ` . `, so + /// `:w /bad/path . q` builds exactly this batch, and + /// `ExecuteCommand` concatenates the two commands' effects into one. + /// improvise-lae's new save prompt also commits to `SaveAs`, so the + /// arm was about to carry more traffic. + #[test] + fn failed_save_as_aborts_the_chain() { + let mut app = test_app(); + app.model_state.dirty = true; + app.apply_effects(vec![ + Box::new(SaveAs(PathBuf::from( + "/nonexistent-dir-improvise-lae/model.improv", + ))), + Box::new(ChangeMode(AppMode::Quit)), + ]); + assert_ne!( + app.view_state.mode, + AppMode::Quit, + "a failed save-as must not let the trailing Quit apply" + ); + assert!( + app.model_state.dirty, + "a failed save-as must leave the model dirty" + ); + assert!( + app.view_state.status_msg.starts_with("Save error:"), + "expected the save error to stay on screen, got: {}", + app.view_state.status_msg + ); + } + + /// The positive arm: a `SaveAs` that succeeds must not abort, or every + /// `:w ` would swallow whatever followed it. Without this, + /// "always abort" passes the test above. + #[test] + fn successful_save_as_lets_the_chain_continue() { + let dir = std::env::temp_dir().join("improvise-lae-save-as-ok"); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("model.improv"); + let mut app = test_app(); + app.model_state.dirty = true; + app.apply_effects(vec![ + Box::new(SaveAs(path.clone())), + Box::new(ChangeMode(AppMode::Quit)), + ]); + assert_eq!(app.view_state.mode, AppMode::Quit); + assert!(!app.model_state.dirty); + assert_eq!( + app.model_state.file_path.as_deref(), + Some(path.as_path()), + "a successful save-as adopts the path" + ); + let _ = std::fs::remove_file(&path); + } + /// A successful save must not abort — `wq` still quits. #[test] fn successful_save_lets_the_chain_continue() {