Allow fish_private_mode to change at runtime

Prior to this change, `fish_private_mode` worked by just suppressing
history outright. With this change, `fish_private_mode` can be toggled on
and off. Commands entered while `fish_private_mode` is set are stored but
in memory only; they are not written to disk.

Fixes #7590
Fixes #7589

doc_src/index.rst

@@ -2003,7 +2003,11 @@ If a function named :ref:`fish_greeting <cmd-fish_greeting>` exists, it will be
 Private mode
 -------------
 
-fish supports launching in private mode via ``fish --private`` (or ``fish -P`` for short). In private mode, old history is not available and any interactive commands you execute will not be appended to the global history file, making it useful both for avoiding inadvertently leaking personal information (e.g. for screencasts) and when dealing with sensitive information to prevent it being persisted to disk. You can query the global variable ``fish_private_mode`` (``if set -q fish_private_mode ...``) if you would like to respect the user's wish for privacy and alter the behavior of your own fish scripts.
+If ``$fish_private_mode`` is set to a non-empty value, commands will not be written to the history file on disk.
+
+You can also launch with ``fish --private`` (or ``fish -P`` for short). This both hides old history and prevents writing history to disk. This is useful to avoid leaking personal information (e.g. for screencasts) or when dealing with sensitive information.
+
+You can query the variable ``fish_private_mode`` (``if set -q fish_private_mode ...``) if you would like to respect the user's wish for privacy and alter the behavior of your own fish scripts.
 
 .. _event:
 

src/env.cpp

@@ -90,7 +90,6 @@ static const std::vector<electric_var_t> electric_variables{
     {L"_", electric_var_t::freadonly},
     {L"fish_kill_signal", electric_var_t::freadonly | electric_var_t::fcomputed},
     {L"fish_pid", electric_var_t::freadonly},
-    {L"fish_private_mode", electric_var_t::freadonly},
     {L"history", electric_var_t::freadonly | electric_var_t::fcomputed},
     {L"hostname", electric_var_t::freadonly},
     {L"pipestatus", electric_var_t::freadonly | electric_var_t::fcomputed},
@@ -119,8 +118,7 @@ static bool is_read_only(const wcstring &key) {
     if (auto ev = electric_var_t::for_name(key)) {
         return ev->readonly();
     }
-    // Hack.
-    return in_private_mode() && key == L"fish_history";
+    return false;
 }
 
 /// Return true if a variable should become a path variable by default. See #436.

src/history.cpp

@@ -1485,9 +1485,10 @@ history_t &history_t::history_with_name(const wcstring &name) {
 static relaxed_atomic_bool_t private_mode{false};
 
 void start_private_mode(env_stack_t &vars) {
-    private_mode = true;
     vars.set_one(L"fish_history", ENV_GLOBAL, L"");
     vars.set_one(L"fish_private_mode", ENV_GLOBAL, L"1");
 }
 
-bool in_private_mode() { return private_mode; }
+bool in_private_mode(const environment_t &vars) {
+    return !vars.get(L"fish_private_mode").missing_or_empty();
+}

src/history.h

@@ -312,7 +312,8 @@ bool all_paths_are_valid(const path_list_t &paths, const wcstring &working_direc
 
 /// Sets private mode on. Once in private mode, it cannot be turned off.
 void start_private_mode(env_stack_t &vars);
+
 /// Queries private mode status.
-bool in_private_mode();
+bool in_private_mode(const environment_t &vars);
 
 #endif

src/reader.cpp

@@ -3166,16 +3166,26 @@ void reader_data_t::handle_readline_command(readline_cmd_t c, readline_loop_stat
                 while (!text.empty() && text.back() == L' ') {
                     text.pop_back();
                 }
+
                 if (history && !conf.in_silent_mode) {
                     // Remove ephemeral items.
                     // Note we fall into this case if the user just types a space and hits return.
                     history->remove_ephemeral_items();
 
                     // Mark this item as ephemeral if there is a leading space (#615).
-                    auto mode = text.front() == L' ' ? history_persistence_mode_t::ephemeral
-                                                     : history_persistence_mode_t::disk;
+                    history_persistence_mode_t mode;
+                    if (text.front() == L' ') {
+                        // Leading spaces are ephemeral (#615).
+                        mode = history_persistence_mode_t::ephemeral;
+                    } else if (in_private_mode(vars)) {
+                        // Private mode means in-memory only.
+                        mode = history_persistence_mode_t::memory;
+                    } else {
+                        mode = history_persistence_mode_t::disk;
+                    }
                     history->add_pending_with_file_detection(text, vars.get_pwd_slash(), mode);
                 }
+
                 rls.finished = true;
                 update_buff_pos(&command_line, command_line.size());
             } else if (command_test_result == PARSER_TEST_INCOMPLETE) {

tests/pexpects/private_mode.py

@@ -0,0 +1,70 @@
+#!/usr/bin/env python3
+import os
+import time
+from pexpect_helper import SpawnedProc
+
+sp = SpawnedProc()
+sendline, sleep, expect_prompt, expect_str = (
+    sp.sendline,
+    sp.sleep,
+    sp.expect_prompt,
+    sp.expect_str,
+)
+
+# Helper to sendline and add to our view of history.
+recorded_history = []
+private_mode_active = False
+fish_path = os.environ.get("fish")
+
+# Send a line and record it in our history array if private mode is not active.
+def sendline_record(s):
+    sendline(s)
+    if not private_mode_active:
+        recorded_history.append(s)
+
+
+expect_prompt()
+
+# Start off with no history.
+sendline(r" builtin history clear; builtin history save")
+expect_prompt()
+
+# Ensure that fish_private_mode can be changed - see #7589.
+sendline_record(r"echo before_private_mode")
+expect_prompt("before_private_mode")
+sendline(r" builtin history save")
+expect_prompt()
+
+# Enter private mode.
+sendline_record(r"set -g fish_private_mode 1")
+expect_prompt()
+private_mode_active = True
+
+sendline_record(r"echo check2 $fish_private_mode")
+expect_prompt("check2 1")
+
+# Nothing else gets added.
+sendline_record(r"true")
+expect_prompt()
+sendline_record(r"false")
+expect_prompt()
+
+# Leave private mode. The command to leave it is still private.
+sendline_record(r"set -ge fish_private_mode")
+expect_prompt()
+private_mode_active = False
+
+# New commands get added.
+sendline_record(r"set alpha beta")
+expect_prompt()
+
+# Check our history is what we expect.
+# We have to wait for the time to tick over, else our item risks being discarded.
+now = time.time()
+start = int(now)
+while now - start < 1:
+    sleep(now - start)
+    now = time.time()
+
+sendline(r" builtin history save ; %s -c 'string join \n -- $history'" % fish_path)
+expect_prompt("\r\n".join(reversed(recorded_history)))