From 977459949f16ff88b3b4100a47ffc16899df2e70 Mon Sep 17 00:00:00 2001
From: Daniel Rainer <daniel.rainer@localhost>
Date: Wed, 18 Jun 2025 21:04:27 +0200
Subject: [PATCH] Obtain history file path in `HistoryImpl::save`

Eliminates some code duplication between the two different saving
implementations. These changes are based on
https://github.com/fish-shell/fish-shell/pull/11492#discussion_r2149438316.

One extra change is included here, namely the early return on an empty file
name, indicating private mode. Without this, `history_path.unwrap()` fails in
some tests. Returning early is probably what we want in such situations anyway.
---
 src/history.rs | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/src/history.rs b/src/history.rs
index add7932e4..383dbcb5e 100644
--- a/src/history.rs
+++ b/src/history.rs
@@ -623,11 +623,7 @@ fn rewrite_to_temporary_file(
     }
 
     /// Saves history by rewriting the file.
-    fn save_internal_via_rewrite(&mut self) -> std::io::Result<()> {
-        let Some(history_path) = self.history_file_path()? else {
-            return Ok(());
-        };
-
+    fn save_internal_via_rewrite(&mut self, history_path: &wstr) -> std::io::Result<()> {
         FLOGF!(
             history,
             "Saving %lu items via rewrite",
@@ -643,7 +639,7 @@ fn save_internal_via_rewrite(&mut self) -> std::io::Result<()> {
                 })
             };
 
-        let (file_id, _) = rewrite_via_temporary_file(&history_path, rewrite)?;
+        let (file_id, _) = rewrite_via_temporary_file(history_path, rewrite)?;
         self.history_file_id = file_id;
 
         // We've saved everything, so we have no more unsaved items.
@@ -660,11 +656,7 @@ fn save_internal_via_rewrite(&mut self) -> std::io::Result<()> {
     }
 
     /// Saves history by appending to the file.
-    fn save_internal_via_appending(&mut self) -> std::io::Result<()> {
-        let Some(history_path) = self.history_file_path()? else {
-            return Ok(());
-        };
-
+    fn save_internal_via_appending(&mut self, history_path: &wstr) -> std::io::Result<()> {
         FLOGF!(
             history,
             "Saving %lu items via appending",
@@ -674,7 +666,7 @@ fn save_internal_via_appending(&mut self) -> std::io::Result<()> {
         assert!(self.deleted_items.is_empty());
 
         let mut locked_history_file =
-            LockedFile::new(LockingMode::Exclusive(WriteMethod::Append), &history_path)?;
+            LockedFile::new(LockingMode::Exclusive(WriteMethod::Append), history_path)?;
 
         // Check if the file was modified since it was last read.
         let file_id = file_id_for_file(locked_history_file.get());
@@ -754,14 +746,23 @@ fn save(&mut self, vacuum: bool) {
             self.first_unwritten_new_item_index = self.new_items.len();
             self.deleted_items.clear();
             self.clear_file_state();
+            return;
         }
 
+        let history_path = match self.history_file_path() {
+            Ok(history_path) => history_path.unwrap(),
+            Err(e) => {
+                FLOG!(history, "Saving history failed:", e);
+                return;
+            }
+        };
+
         // Try saving. If we have items to delete, we have to rewrite the file. If we do not, we can
         // append to it.
         let mut ok = false;
         if !vacuum && self.deleted_items.is_empty() {
             // Try doing a fast append.
-            if let Err(e) = self.save_internal_via_appending() {
+            if let Err(e) = self.save_internal_via_appending(&history_path) {
                 FLOG!(history, "Appending to history failed:", e);
             } else {
                 ok = true;
@@ -769,7 +770,7 @@ fn save(&mut self, vacuum: bool) {
         }
         if !ok {
             // We did not or could not append; rewrite the file ("vacuum" it).
-            if let Err(e) = self.save_internal_via_rewrite() {
+            if let Err(e) = self.save_internal_via_rewrite(&history_path) {
                 FLOG!(history, "Rewriting history failed:", e)
             }
         }