make read honor FISH_HISTORY

The `read` command `-m` and `--mode-name` vars are now deprecated and do
nothing other than result in a warning message. The `read` command now
honors the `FISH_HISTORY` var that is used to control where commands are
read from and written to. You can set that var to the empty string to
suppress the use of both history files. Or you can set it to a history
session ID in which case that will limit the `read` history that is
available.

Fixes #1504

CHANGELOG.md

@@ -7,6 +7,7 @@
 - `string escape` has a new `--style=xxx` flag where `xxx` can be `script`, `var`, or `url` (#4150).
 - `string unescape` has been implemented to reverse the effects of `string escape` (#3543).
 - The history file can now be specified by setting the `FISH_HISTORY` variable (#102).
+- Read history is now controlled by the `FISH_HISTORY` variable rather than the `--mode-name` flag (#1504).
 
 ## Other significant changes
 

doc_src/read.txt

@@ -19,8 +19,6 @@ The following options are available:
 
 - `-l` or `--local` makes the variables local.
 
-- `-m NAME` or `--mode-name=NAME` specifies that the name NAME should be used to save/load the history file. If NAME is fish, the regular fish history will be available.
-
 - `-n NCHARS` or `--nchars=NCHARS` causes `read` to return after reading NCHARS characters rather than waiting for a complete line of input (either newline or null terminated).
 
 - `-p PROMPT_CMD` or `--prompt=PROMPT_CMD` uses the output of the shell command `PROMPT_CMD` as the prompt for the interactive mode. The default prompt command is <code>set_color green; echo read; set_color normal; echo "> "</code>.
@@ -51,6 +49,10 @@ When read reaches the end-of-file (EOF) instead of the separator, it sets `$stat
 
 Fish has a default limit of 10 MiB on the number of characters each `read` will consume. If you attempt to read more than that `$status` is set to 122 and the variable will be empty. You can modify that limit by setting the `FISH_READ_BYTE_LIMIT` variable at any time including in the environment before fish starts running. This is a safety mechanism to keep the shell from consuming an unreasonable amount of memory if the input is malformed.
 
+\subsection read-history Using another read history file
+
+The `read` command supported the `-m` and `--mode-name` flags in fish versions prior to 2.7.0 to specify an alternative read history file. Those flags are now deprecated and ignored. Instead, set the `FISH_HISTORY` variable to specify a history session ID. That will affect both the `read` history file and the fish command history file. You can set it to an empty string to specify that no history should be read or written. This is useful for presentations where you do not want possibly private or sensitive history to be exposed to the audience but do want history relevant to the presentation to be available.
+
 \subsection read-example Example
 
 The following code stores the value 'hello' in the shell variable `$foo`.

src/builtin.h

@@ -29,9 +29,6 @@ struct builtin_data_t {
 /// The default prompt for the read command.
 #define DEFAULT_READ_PROMPT L"set_color green; echo -n read; set_color normal; echo -n \"> \""
 
-/// The mode name to pass to history and input.
-#define READ_MODE_NAME L"fish_read"
-
 enum { COMMAND_NOT_BUILTIN, BUILTIN_REGULAR, BUILTIN_FUNCTION };
 
 /// Error message on missing argument.

src/builtin_read.cpp

@@ -22,6 +22,7 @@
 #include "expand.h"
 #include "fallback.h"  // IWYU pragma: keep
 #include "highlight.h"
+#include "history.h"
 #include "io.h"
 #include "proc.h"
 #include "reader.h"
@@ -41,7 +42,6 @@ struct read_cmd_opts_t {
     bool array = false;
     bool silent = false;
     bool split_null = false;
-    const wchar_t *mode_name = READ_MODE_NAME;
     int nchars = 0;
 };
 
@@ -108,7 +108,9 @@ static int parse_cmd_opts(read_cmd_opts_t &opts, int *optind,  //!OCLINT(high nc
                 break;
             }
             case L'm': {
-                opts.mode_name = w.woptarg;
+                streams.err.append_format(_(L"%ls: flags '--mode-name' / '-m' are now ignored. "
+                                            L"Set FISH_HISTORY instead.\n"),
+                                          cmd);
                 break;
             }
             case L'n': {
@@ -170,12 +172,13 @@ static int parse_cmd_opts(read_cmd_opts_t &opts, int *optind,  //!OCLINT(high nc
 /// Read from the tty. This is only valid when the stream is stdin and it is attached to a tty and
 /// we weren't asked to split on null characters.
 static int read_interactive(wcstring &buff, int nchars, bool shell, bool silent,
-                            const wchar_t *mode_name, const wchar_t *prompt,
+                            const wchar_t *prompt, const wchar_t *right_prompt,
-                            const wchar_t *right_prompt, const wchar_t *commandline) {
+                            const wchar_t *commandline) {
     int exit_res = STATUS_CMD_OK;
     const wchar_t *line;
 
-    reader_push(mode_name);
+    wcstring read_history_ID = history_session_id() + L"_read";
+    reader_push(read_history_ID.c_str());
     reader_set_left_prompt(prompt);
     reader_set_right_prompt(right_prompt);
     if (shell) {
@@ -381,8 +384,8 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     if (stream_stdin_is_a_tty && !opts.split_null) {
         // We should read interactively using reader_readline(). This does not support splitting on
         // null.
-        exit_res = read_interactive(buff, opts.nchars, opts.shell, opts.silent, opts.mode_name,
+        exit_res = read_interactive(buff, opts.nchars, opts.shell, opts.silent, opts.prompt,
-                                    opts.prompt, opts.right_prompt, opts.commandline);
+                                    opts.right_prompt, opts.commandline);
     } else if (!opts.nchars && !stream_stdin_is_a_tty &&
                lseek(streams.stdin_fd, 0, SEEK_CUR) != -1) {
         exit_res = read_in_chunks(streams.stdin_fd, buff, opts.split_null);

src/history.cpp

@@ -1143,20 +1143,17 @@ static void unescape_yaml(std::string *str) {
 }
 
 static wcstring history_filename(const wcstring &session_id, const wcstring &suffix) {
-    if (session_id.empty()) {
+    if (session_id.empty()) return L"";
-        return L"";
+
-    } else {
+    wcstring result;
-        wcstring path;
+    if (!path_get_data(result)) return L"";
-        if (!path_get_data(path)) return L"";
 
-        wcstring result = path;
     result.append(L"/");
     result.append(session_id);
     result.append(L"_history");
     result.append(suffix);
     return result;
 }
-}
 
 void history_t::clear_file_state() {
     ASSERT_IS_LOCKED(lock);
@@ -1804,6 +1801,7 @@ void history_sanity_check() {
     // No sanity checking implemented yet...
 }
 
+/// Return the prefix for the files to be used for command and read history.
 wcstring history_session_id() {
     wcstring result = DFLT_FISH_HISTORY_SESSION_ID;
 

src/history.h

@@ -340,24 +340,25 @@ class history_search_t {
         : history(), term(), search_type(HISTORY_SEARCH_TYPE_CONTAINS), case_sensitive(true) {}
 };
 
-// Init history library. The history file won't actually be loaded until the first time a history
+/// Init history library. The history file won't actually be loaded until the first time a history
-// search is performed.
+/// search is performed.
 void history_init();
 
-// Saves the new history to disk.
+/// Saves the new history to disk.
 void history_destroy();
 
-// Perform sanity checks.
+/// Perform sanity checks.
 void history_sanity_check();
 
+/// Return the prefix for the files to be used for command and read history.
 wcstring history_session_id();
 
-// Given a list of paths and a working directory, return the paths that are valid
+/// Given a list of paths and a working directory, return the paths that are valid
-// This does disk I/O and may only be called in a background thread
+/// This does disk I/O and may only be called in a background thread
 path_list_t valid_paths(const path_list_t &paths, const wcstring &working_directory);
 
-// Given a list of paths and a working directory,
+/// Given a list of paths and a working directory,
-// return true if all paths in the list are valid
+/// return true if all paths in the list are valid
-// Returns true for if paths is empty
+/// Returns true for if paths is empty
 bool all_paths_are_valid(const path_list_t &paths, const wcstring &working_directory);
 #endif