From 2fe15d8618ad2ec3c39f22f69780968ef5e43ed9 Mon Sep 17 00:00:00 2001 From: Rene Leonhardt <65483435+reneleonhardt@users.noreply.github.com> Date: Sun, 6 Jul 2025 18:08:49 +0200 Subject: [PATCH 1/4] feat(commands): make :x behave more like in vim Closes #4087 --- helix-term/src/commands/typed.rs | 37 ++++++++++++++++++++++++++++++-- helix-view/src/document.rs | 7 ++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index e1bb8ee32..d6ba238e0 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -663,6 +663,28 @@ fn force_write_quit( force_quit(cx, Args::default(), event) } +/// exit command: Write only if named and modified (but not externally), then quit. +fn write_exit(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> anyhow::Result<()> { + if event != PromptEvent::Validate { + return Ok(()); + } + + let (_, doc) = current!(cx.editor); + + // Current document named and modified? + // Write changes if not externally modified already. + if doc.is_modified() { + if doc.path().is_some() && !doc.externally_overwritten() { + write_impl(cx, None, false)?; + cx.block_try_flush_writes()?; + } else { + doc.reset_modified(); + } + } + + quit(cx, Args::default(), event) +} + /// Results in an error if there are modified buffers remaining and sets editor /// error, otherwise returns `Ok(())`. If the current document is unmodified, /// and there are modified documents, switches focus to one of them. @@ -2816,7 +2838,7 @@ pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[ }, TypableCommand { name: "write-quit", - aliases: &["wq", "x"], + aliases: &["wq"], doc: "Write changes to disk and close the current view. Accepts an optional path (:wq some/path.txt)", fun: write_quit, completer: CommandCompleter::positional(&[completers::filename]), @@ -2827,7 +2849,7 @@ pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[ }, TypableCommand { name: "write-quit!", - aliases: &["wq!", "x!"], + aliases: &["wq!"], doc: "Write changes to disk and close the current view forcefully. Accepts an optional path (:wq! some/path.txt)", fun: force_write_quit, completer: CommandCompleter::positional(&[completers::filename]), @@ -2836,6 +2858,17 @@ pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[ ..Signature::DEFAULT }, }, + TypableCommand { + name: "exit", + aliases: &["xit", "x", "x!"], + doc: "Save the current view if named and modified (but not externally), then quit.", + fun: write_exit, + completer: CommandCompleter::none(), + signature: Signature { + positionals: (0, Some(0)), + ..Signature::DEFAULT + }, + }, TypableCommand { name: "write-all", aliases: &["wa"], diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index a0a56113c..874f84d6e 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1199,6 +1199,13 @@ impl Document { }; } + /// Path given and externally modified since `last_saved_time`? + pub fn externally_overwritten(&self) -> bool { + self.path() + .and_then(|p| p.metadata().ok().and_then(|m| m.modified().ok())) + .is_some_and(|mtime| mtime > self.last_saved_time) + } + // Detect if the file is readonly and change the readonly field if necessary (unix only) pub fn detect_readonly(&mut self) { // Allows setting the flag for files the user cannot modify, like root files From c4fb8bfafa20a7c8510efe92615c478a35f56ada Mon Sep 17 00:00:00 2001 From: Rene Leonhardt <65483435+reneleonhardt@users.noreply.github.com> Date: Sun, 6 Jul 2025 21:07:07 +0200 Subject: [PATCH 2/4] fix test and generate docs --- book/src/generated/typable-cmd.md | 5 +++-- helix-term/tests/test/commands/write.rs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/book/src/generated/typable-cmd.md b/book/src/generated/typable-cmd.md index e416e813f..7c81160dd 100644 --- a/book/src/generated/typable-cmd.md +++ b/book/src/generated/typable-cmd.md @@ -21,8 +21,9 @@ | `:line-ending` | Set the document's default line ending. Options: crlf, lf. | | `:earlier`, `:ear` | Jump back to an earlier point in edit history. Accepts a number of steps or a time span. | | `:later`, `:lat` | Jump to a later point in edit history. Accepts a number of steps or a time span. | -| `:write-quit`, `:wq`, `:x` | Write changes to disk and close the current view. Accepts an optional path (:wq some/path.txt) | -| `:write-quit!`, `:wq!`, `:x!` | Write changes to disk and close the current view forcefully. Accepts an optional path (:wq! some/path.txt) | +| `:write-quit`, `:wq` | Write changes to disk and close the current view. Accepts an optional path (:wq some/path.txt) | +| `:write-quit!`, `:wq!` | Write changes to disk and close the current view forcefully. Accepts an optional path (:wq! some/path.txt) | +| `:exit`, `:xit`, `:x`, `:x!` | Save the current view if named and modified (but not externally), then quit. | | `:write-all`, `:wa` | Write changes from all buffers to disk. | | `:write-all!`, `:wa!` | Forcefully write changes from all buffers to disk creating necessary subdirectories. | | `:write-quit-all`, `:wqa`, `:xa` | Write changes from all buffers to disk and close all views. | diff --git a/helix-term/tests/test/commands/write.rs b/helix-term/tests/test/commands/write.rs index 4b78e14c4..24f8435ee 100644 --- a/helix-term/tests/test/commands/write.rs +++ b/helix-term/tests/test/commands/write.rs @@ -144,7 +144,7 @@ async fn test_overwrite_protection() -> anyhow::Result<()> { file.as_file_mut().flush()?; file.as_file_mut().sync_all()?; - test_key_sequence(&mut app, Some(":x"), None, false).await?; + test_key_sequence(&mut app, Some(":x"), None, true).await?; reload_file(&mut file).unwrap(); let mut file_content = String::new(); From 45fb545ce9a77b4b5a7a5918debe93ad24c10952 Mon Sep 17 00:00:00 2001 From: Rene Leonhardt <65483435+reneleonhardt@users.noreply.github.com> Date: Sun, 6 Jul 2025 21:35:39 +0200 Subject: [PATCH 3/4] add more exit tests --- helix-term/tests/test/commands/write.rs | 42 +++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/helix-term/tests/test/commands/write.rs b/helix-term/tests/test/commands/write.rs index 24f8435ee..5979a6285 100644 --- a/helix-term/tests/test/commands/write.rs +++ b/helix-term/tests/test/commands/write.rs @@ -143,7 +143,9 @@ async fn test_overwrite_protection() -> anyhow::Result<()> { file.as_file_mut().flush()?; file.as_file_mut().sync_all()?; + let last_saved_time = file.path().metadata()?.modified()?; + // Exit empty unmodified view, externally changed file shouldn't be overwritten test_key_sequence(&mut app, Some(":x"), None, true).await?; reload_file(&mut file).unwrap(); @@ -152,6 +154,46 @@ async fn test_overwrite_protection() -> anyhow::Result<()> { assert_eq!("extremely important content", file_content); + let saved_time = file.path().metadata()?.modified()?; + assert_eq!(saved_time, last_saved_time); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_overwrite_if_not_modified_since() -> anyhow::Result<()> { + let mut file = tempfile::NamedTempFile::new()?; + file.as_file_mut().write_all("34".as_bytes())?; + file.as_file_mut().flush()?; + file.as_file_mut().sync_all()?; + + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; + + helpers::run_event_loop_until_idle(&mut app).await; + + // Modify and exit allowing saving because file hasn't been changed externally since opening + test_key_sequence(&mut app, Some("i12:x"), None, true).await?; + + reload_file(&mut file).unwrap(); + let mut file_content = String::new(); + file.read_to_string(&mut file_content)?; + + assert_eq!("1234", file_content); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_exit_if_not_named() -> anyhow::Result<()> { + let mut app = helpers::AppBuilder::new().build()?; + + helpers::run_event_loop_until_idle(&mut app).await; + + // Modify and exit without saving because file hasn't been named yet + test_key_sequence(&mut app, Some("i12:x"), None, true).await?; + Ok(()) } From 4647374449ea8b9ed28dacdf49b95aa6b6fd65a9 Mon Sep 17 00:00:00 2001 From: Rene Leonhardt <65483435+reneleonhardt@users.noreply.github.com> Date: Mon, 7 Jul 2025 10:36:42 +0200 Subject: [PATCH 4/4] simplify pickup_last_saved_time --- helix-view/src/document.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 874f84d6e..24003cd95 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1182,19 +1182,10 @@ impl Document { pub fn pickup_last_saved_time(&mut self) { self.last_saved_time = match self.path() { - Some(path) => match path.metadata() { - Ok(metadata) => match metadata.modified() { - Ok(mtime) => mtime, - Err(err) => { - log::debug!("Could not fetch file system's mtime, falling back to current system time: {}", err); - SystemTime::now() - } - }, - Err(err) => { - log::debug!("Could not fetch file system's mtime, falling back to current system time: {}", err); - SystemTime::now() - } - }, + Some(path) => path.metadata().and_then(|m| m.modified()).unwrap_or_else(|err| { + log::debug!("Could not fetch file system's mtime, falling back to current system time: {}", err); + SystemTime::now() + }), None => SystemTime::now(), }; }