Fix regression causing cursor shape commands to leak into noninteractive shell

As reported in
https://matrix.to/#/!YLTeaulxSDauOOxBoR:matrix.org/$CLuoHTdvcRj_8-HBBq0p-lmGWeix5khEtKEDxN2Ulfo

Running

	fish -C '
		fzf_key_bindings
		echo fish_vi_key_bindings >>~/.config/fish/config.fish
		fzf-history-widget
	'

and pressing "enter" will add escape sequences like "[2 q" (cursor shape)
to fish's command line.

This is because fzf-history-widget binds "enter" to a filter
that happens to be a fish script:

	set -lx FZF_DEFAULT_OPTS \
		... \
		"--bind='enter:become:string replace -a -- \n\t \n {2..} | string collect'" \
		'--with-shell='(status fish-path)\\ -c)

The above ~/.config/fish/config.fish (redundantly) runs "fish_vi_key_bindings"
even in *noninteractive* shells, then "fish_vi_cursor" will print cursor
sequences in its "fish_exit" handler.  The sequence is not printed to the
terminal but to fzf which doesn't parse CSI commands.

This is a regression introduced by a5dfa84f73 (fish_vi_cursor: skip if stdin
is not a tty, 2023-11-14). That commit wanted "fish -c read" to be able to
use Vi cursor.  This is a noninteractive shell, but inside "read" we are
"effectively interactive".  However "status is-interactive" does not tell
us that.

Let's use a more contained fix to make sure that we print escape sequences only
if either fish is interactive, or if we are evaluating an interactive read.

In general, "fish -c read" is prone to configuration errors, since we
recommend gating configuration (for bind etc) on "status is-interactive"
which will not run here.

doc_src/cmds/status.rst

@@ -11,6 +11,7 @@ Synopsis
     status
     status is-login
     status is-interactive
+    status is-interactive-read
     status is-block
     status is-breakpoint
     status is-command-substitution
@@ -50,6 +51,9 @@ The following operations (subcommands) are available:
 **is-interactive**, **-i** or **--is-interactive**
     Returns 0 if fish is interactive - that is, connected to a keyboard.
 
+**is-interactive-read** or **--is-interactive-read**
+    Returns 0 if fish is running an interactive :doc:`read <read>` builtin which is connected to a keyboard.
+
 **is-login**, **-l** or **--is-login**
     Returns 0 if fish is a login shell - that is, if fish should perform login tasks such as setting up :envvar:`PATH`.
 

share/completions/status.fish

@@ -18,6 +18,7 @@ set -l __fish_status_all_commands \
     is-full-job-control \
     is-interactive \
     is-interactive-job-control \
+    is-interactive-read \
     is-login \
     is-no-job-control \
     job-control \
@@ -32,6 +33,7 @@ complete -c status -s h -l help -d "Display help and exit"
 # The "is-something" subcommands.
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-login -d "Test if this is a login shell"
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-interactive -d "Test if this is an interactive shell"
+complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-interactive-read -d "Test if inside an interactive read builtin"
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-command-substitution -d "Test if a command substitution is currently evaluated"
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-block -d "Test if a code block is currently evaluated"
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-breakpoint -d "Test if a breakpoint is currently in effect"

share/functions/fish_vi_cursor.fish

@@ -1,13 +1,11 @@
 function fish_vi_cursor -d 'Set cursor shape for different vi modes'
-    # if stdin is not a tty, there is effectively no bind mode.
-    if not test -t 0
-        return
-    end
-
     set -q fish_cursor_unknown
     or set -g fish_cursor_unknown block
 
     function __fish_vi_cursor --argument-names varname
+        if not status is-interactive; and not status is-interactive-read
+            return
+        end
         if not set -q $varname
             switch $varname
                 case fish_cursor_insert

src/builtins/status.rs

@@ -6,6 +6,7 @@
 use crate::proc::{
     get_job_control_mode, get_login, is_interactive_session, set_job_control_mode, JobControl,
 };
+use crate::reader::reader_in_interactive_read;
 use crate::wutil::{waccess, wbasename, wdirname, wrealpath, Error};
 use libc::F_OK;
 use nix::errno::Errno;
@@ -50,6 +51,7 @@ enum StatusCmd {
     STATUS_IS_FULL_JOB_CTRL,
     STATUS_IS_INTERACTIVE,
     STATUS_IS_INTERACTIVE_JOB_CTRL,
+    STATUS_IS_INTERACTIVE_READ,
     STATUS_IS_LOGIN,
     STATUS_IS_NO_JOB_CTRL,
     STATUS_LINE_NUMBER,
@@ -82,6 +84,7 @@ enum StatusCmd {
     (STATUS_IS_FULL_JOB_CTRL, "is-full-job-control"),
     (STATUS_IS_INTERACTIVE, "is-interactive"),
     (STATUS_IS_INTERACTIVE_JOB_CTRL, "is-interactive-job-control"),
+    (STATUS_IS_INTERACTIVE_READ, "is-interactive-read"),
     (STATUS_IS_LOGIN, "is-login"),
     (STATUS_IS_NO_JOB_CTRL, "is-no-job-control"),
     (STATUS_SET_JOB_CONTROL, "job-control"),
@@ -141,6 +144,7 @@ fn default() -> Self {
 const IS_FULL_JOB_CTRL_SHORT: char = '\x02';
 const IS_INTERACTIVE_JOB_CTRL_SHORT: char = '\x03';
 const IS_NO_JOB_CTRL_SHORT: char = '\x04';
+const IS_INTERACTIVE_READ_SHORT: char = '\x05';
 
 const SHORT_OPTIONS: &wstr = L!(":L:cbilfnhj:t");
 const LONG_OPTIONS: &[WOption] = &[
@@ -162,6 +166,11 @@ fn default() -> Self {
         NoArgument,
         IS_INTERACTIVE_JOB_CTRL_SHORT,
     ),
+    wopt(
+        L!("is-interactive-read"),
+        NoArgument,
+        IS_INTERACTIVE_READ_SHORT,
+    ),
     wopt(L!("is-login"), NoArgument, 'l'),
     wopt(L!("is-no-job-control"), NoArgument, IS_NO_JOB_CTRL_SHORT),
     wopt(L!("job-control"), RequiredArgument, 'j'),
@@ -271,6 +280,11 @@ fn parse_cmd_opts(
                     return STATUS_CMD_ERROR;
                 }
             }
+            IS_INTERACTIVE_READ_SHORT => {
+                if !opts.try_set_status_cmd(STATUS_IS_INTERACTIVE_READ, streams) {
+                    return STATUS_CMD_ERROR;
+                }
+            }
             IS_NO_JOB_CTRL_SHORT => {
                 if !opts.try_set_status_cmd(STATUS_IS_NO_JOB_CTRL, streams) {
                     return STATUS_CMD_ERROR;
@@ -548,6 +562,13 @@ pub fn status(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> O
                         return STATUS_CMD_ERROR;
                     }
                 }
+                STATUS_IS_INTERACTIVE_READ => {
+                    if reader_in_interactive_read() {
+                        return STATUS_CMD_OK;
+                    } else {
+                        return STATUS_CMD_ERROR;
+                    }
+                }
                 STATUS_IS_NO_JOB_CTRL => {
                     if get_job_control_mode() == JobControl::none {
                         return STATUS_CMD_OK;

src/reader.rs

@@ -242,6 +242,13 @@ unsafe impl Sync for ReaderDataStack {}
     unsafe { &mut *READER_DATA_STACK.0.get() }
 }
 
+pub fn reader_in_interactive_read() -> bool {
+    reader_data_stack()
+        .iter()
+        .rev()
+        .any(|reader| reader.conf.exit_on_interrupt)
+}
+
 /// Access the top level reader data.
 pub fn current_data() -> Option<&'static mut ReaderData> {
     reader_data_stack()