From d40d2b786f207be9942021abb7af0f3a5243d32d Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 18 May 2024 19:54:13 +0200 Subject: [PATCH] Work around wants_terminal not begin set inside eval On this binding we fail to disable CSI u bind c-t ' begin set -lx FZF_DEFAULT_OPTS --height 40% --bind=ctrl-z:ignore eval fzf | while read -l r; echo read $r; end end ' because for "fzf", ParseExecutionContext::setup_group() returns early with the parent process group (which should be fish's own) , hence "wants_terminal" is false. This seems questionable, I don't think the eval should make a difference here. For now, don't touch it; use the more accurate way of detecting whether a process may read keyboard input. In many of such cases "wants_terminal" is false, like echo (echo 1\n2\n3 | fzf) Fixes #10504 --- src/builtins/fg.rs | 16 ++++++++-------- src/exec.rs | 2 +- src/proc.rs | 4 ++++ tests/pexpects/history.py | 4 ++-- tests/pexpects/read.py | 2 +- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/builtins/fg.rs b/src/builtins/fg.rs index bb3a12057..49cc86de0 100644 --- a/src/builtins/fg.rs +++ b/src/builtins/fg.rs @@ -147,15 +147,15 @@ pub fn fg(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Optio { let job_group = job.group(); job_group.set_is_foreground(true); - let tmodes = job_group.tmodes.borrow(); - if job_group.wants_terminal() { + if job.entitled_to_terminal() { terminal_protocols_disable_ifn(); - if tmodes.is_some() { - let termios = tmodes.as_ref().unwrap(); - let res = unsafe { libc::tcsetattr(STDIN_FILENO, TCSADRAIN, termios) }; - if res < 0 { - perror("tcsetattr"); - } + } + let tmodes = job_group.tmodes.borrow(); + if job_group.wants_terminal() && tmodes.is_some() { + let termios = tmodes.as_ref().unwrap(); + let res = unsafe { libc::tcsetattr(STDIN_FILENO, TCSADRAIN, termios) }; + if res < 0 { + perror("tcsetattr"); } } } diff --git a/src/exec.rs b/src/exec.rs index 09a61e2ae..b294a971f 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -72,7 +72,7 @@ pub fn exec_job(parser: &Parser, job: &Job, block_io: IoChain) -> bool { return true; } - if job.group().wants_terminal() { + if job.entitled_to_terminal() { terminal_protocols_disable_ifn(); } diff --git a/src/proc.rs b/src/proc.rs index f685f80e2..7bc817771 100644 --- a/src/proc.rs +++ b/src/proc.rs @@ -886,6 +886,10 @@ impl Job { self.group().wants_job_control() } + pub fn entitled_to_terminal(&self) -> bool { + self.group().is_foreground() && self.processes().iter().any(|p| !p.is_internal()) + } + /// Return whether this job is initially going to run in the background, because & was /// specified. pub fn is_initially_background(&self) -> bool { diff --git a/tests/pexpects/history.py b/tests/pexpects/history.py index b559f519d..0e931b1f0 100644 --- a/tests/pexpects/history.py +++ b/tests/pexpects/history.py @@ -192,7 +192,7 @@ expect_prompt("nonephemeral! line") sendline("true") expect_prompt() sendline("echo a; history search '*ephemeral!*' | cat; echo b") -expect_prompt("a\r\necho nonephemeral! line\r\nb\r\n") +expect_prompt(r"a\r\n.*echo nonephemeral! line\r\nb\r\n") # If fish_should_add_to_history exists, it will completely take over, # so even lines with spaces are stored @@ -201,4 +201,4 @@ expect_prompt("spaced") sendline("true") expect_prompt() sendline("echo a; history search '*spaced*' | cat; echo b") -expect_prompt("a\r\n echo spaced\r\nb\r\n") +expect_prompt("a\r\n.* echo spaced\r\nb\r\n") diff --git a/tests/pexpects/read.py b/tests/pexpects/read.py index 17e20d189..b8adc04b4 100644 --- a/tests/pexpects/read.py +++ b/tests/pexpects/read.py @@ -13,7 +13,7 @@ send, sendline, sleep, expect_prompt, expect_re, expect_str = ( def expect_read_prompt(): - expect_re(r"\r\n?read> $") + expect_re(r"\r\n?read> .*$") def expect_marker(text):