fix: allow interrupting read builtin, run pipeline cmds in subshell

brush-core/Cargo.toml

@@ -30,6 +30,8 @@ os_pipe = { version = "1.1.5", features = ["io_safety"] }
 rand = "0.8.5"
 thiserror = "1.0.61"
 tokio = { version = "1.37.0", features = [
+    "fs",
+    "io-std",
     "io-util",
     "macros",
     "process",

brush-core/src/builtins/read.rs

@@ -1,5 +1,6 @@
 use clap::Parser;
-use std::{collections::VecDeque, io::Read};
+use std::collections::VecDeque;
+use std::io::Read;
 
 use crate::{builtin, commands, env, error, openfiles, variables};
 
@@ -72,19 +73,17 @@ impl builtin::Command for ReadCommand {
         if self.raw_mode {
             tracing::debug!("read -r is not implemented");
         }
-        if self.silent {
-            return error::unimp("read -s");
-        }
         if self.timeout_in_seconds.is_some() {
             return error::unimp("read -t");
         }
         if self.fd_num_to_read.is_some() {
             return error::unimp("read -u");
         }
 
-        let input_line = read_line(context.stdin());
+        let input_line = self.read_line(context.stdin())?;
         if let Some(input_line) = input_line {
             let mut variable_names: VecDeque<String> = self.variable_names.clone().into();
+            let mut spillover: Option<String> = None;
             for field in input_line.split_ascii_whitespace() {
                 if let Some(variable_name) = variable_names.pop_front() {
                     context.shell.env.update_or_add(
@@ -95,36 +94,90 @@ impl builtin::Command for ReadCommand {
                         env::EnvironmentScope::Global,
                     )?;
                 } else {
-                    return error::unimp("too few variable names");
+                    match &mut spillover {
+                        Some(s) => {
+                            s.push(' ');
+                            s.push_str(field);
+                        }
+                        None => spillover = Some(field.to_owned()),
+                    }
                 }
             }
+
+            if let Some(spillover) = spillover {
+                context.shell.env.update_or_add(
+                    "REPLY",
+                    variables::ShellValueLiteral::Scalar(spillover),
+                    |_| Ok(()),
+                    env::EnvironmentLookup::Anywhere,
+                    env::EnvironmentScope::Global,
+                )?;
+            }
+
             Ok(crate::builtin::ExitCode::Success)
         } else {
             Ok(crate::builtin::ExitCode::Custom(1))
         }
     }
 }
 
-fn read_line(mut file: openfiles::OpenFile) -> Option<String> {
-    let mut line = String::new();
-    let mut buffer = [0; 1]; // 1-byte buffer
+impl ReadCommand {
+    fn read_line(&self, mut file: openfiles::OpenFile) -> Result<Option<String>, error::Error> {
+        let orig_term_attr = file.get_term_attr()?;
+        if let Some(orig_term_attr) = &orig_term_attr {
+            let mut updated_term_attr = orig_term_attr.to_owned();
+
+            updated_term_attr
+                .local_flags
+                .remove(nix::sys::termios::LocalFlags::ICANON);
+            updated_term_attr
+                .local_flags
+                .remove(nix::sys::termios::LocalFlags::ISIG);
+
+            if self.silent {
+                updated_term_attr
+                    .local_flags
+                    .remove(nix::sys::termios::LocalFlags::ECHO);
+            }
+
+            file.set_term_attr(nix::sys::termios::SetArg::TCSANOW, &updated_term_attr)?;
+        }
+
+        let mut line = String::new();
+        let mut buffer = [0; 1]; // 1-byte buffer
+
+        // TODO: Look at ignoring errors here.
+        while let Ok(n) = file.read(&mut buffer) {
+            if n == 0 {
+                break; // EOF reached.
+            }
+
+            let ch = buffer[0] as char;
+
+            if ch == '\x03' {
+                return Ok(None); // Ctrl+C aborts.
+            }
+
+            if ch == '\n' {
+                break; // End of line reached
+            }
+
+            // Ignore other control characters.
+            if ch.is_ascii_control() {
+                continue;
+            }
 
-    // TODO: Look at ignoring errors here.
-    while let Ok(n) = file.read(&mut buffer) {
-        if n == 0 {
-            break; // EOF reached
+            line.push(ch);
         }
-        let ch = buffer[0] as char;
 
-        if ch == '\n' {
-            break; // End of line reached
+        if let Some(orig_term_attr) = &orig_term_attr {
+            file.set_term_attr(nix::sys::termios::SetArg::TCSANOW, orig_term_attr)?;
         }
-        line.push(ch);
-    }
 
-    if line.is_empty() {
-        None
-    } else {
-        Some(line)
+        if line.is_empty() {
+            Ok(None)
+        } else {
+            Ok(Some(line))
+        }
     }
 }

brush-core/src/extendedtests.rs

@@ -115,7 +115,15 @@ pub(crate) fn apply_unary_predicate_to_str(
             }
         }
         ast::UnaryPredicate::FdIsOpenTerminal => {
-            error::unimp("unary extended test predicate: FdIsOpenTerminal")
+            if let Ok(fd) = operand.parse::<u32>() {
+                if let Some(open_file) = shell.open_files.files.get(&fd) {
+                    Ok(open_file.is_term())
+                } else {
+                    Ok(false)
+                }
+            } else {
+                Ok(false)
+            }
         }
         ast::UnaryPredicate::FileExistsAndIsSetuid => {
             let path = Path::new(operand);

brush-core/src/interp.rs

@@ -79,7 +79,7 @@ struct PipelineExecutionContext<'a> {
 
     current_pipeline_index: usize,
     pipeline_len: usize,
-    output_pipes: Vec<os_pipe::PipeReader>,
+    output_pipes: &'a mut Vec<os_pipe::PipeReader>,
 
     params: ExecutionParameters,
 }
@@ -301,20 +301,35 @@ impl Execute for ast::Pipeline {
         shell: &mut Shell,
         params: &ExecutionParameters,
     ) -> Result<ExecutionResult, error::Error> {
-        let mut pipeline_context = PipelineExecutionContext {
-            shell,
-            current_pipeline_index: 0,
-            pipeline_len: self.seq.len(),
-            output_pipes: vec![],
-            params: params.clone(),
-        };
-
+        let pipeline_len = self.seq.len();
+        let mut output_pipes = vec![];
         let mut spawn_results = VecDeque::new();
 
-        for command in &self.seq {
-            let spawn_result = command.execute_in_pipeline(&mut pipeline_context).await?;
+        for (current_pipeline_index, command) in self.seq.iter().enumerate() {
+            // If there's only one command in the pipeline, then we run directly in the current shell.
+            // Otherwise, we spawn a separate subshell for each command in the pipeline.
+            let spawn_result = if pipeline_len > 1 {
+                let mut subshell = shell.clone();
+                let mut pipeline_context = PipelineExecutionContext {
+                    shell: &mut subshell,
+                    current_pipeline_index,
+                    pipeline_len,
+                    output_pipes: &mut output_pipes,
+                    params: params.clone(),
+                };
+                command.execute_in_pipeline(&mut pipeline_context).await?
+            } else {
+                let mut pipeline_context = PipelineExecutionContext {
+                    shell,
+                    current_pipeline_index,
+                    pipeline_len,
+                    output_pipes: &mut output_pipes,
+                    params: params.clone(),
+                };
+                command.execute_in_pipeline(&mut pipeline_context).await?
+            };
+
             spawn_results.push_back(spawn_result);
-            pipeline_context.current_pipeline_index += 1;
         }
 
         let mut result = ExecutionResult::success();

brush-core/src/openfiles.rs

@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::io::IsTerminal;
 #[cfg(unix)]
 use std::os::fd::AsFd;
 #[cfg(unix)]
@@ -76,6 +77,55 @@ impl OpenFile {
             OpenFile::HereDocument(_) => error::unimp("as_raw_fd for here doc"),
         }
     }
+
+    pub(crate) fn is_term(&self) -> bool {
+        match self {
+            OpenFile::Stdin => std::io::stdin().is_terminal(),
+            OpenFile::Stdout => std::io::stdout().is_terminal(),
+            OpenFile::Stderr => std::io::stderr().is_terminal(),
+            OpenFile::Null => false,
+            OpenFile::File(f) => f.is_terminal(),
+            OpenFile::PipeReader(_) => false,
+            OpenFile::PipeWriter(_) => false,
+            OpenFile::HereDocument(_) => false,
+        }
+    }
+
+    pub(crate) fn get_term_attr(&self) -> Result<Option<nix::sys::termios::Termios>, error::Error> {
+        if !self.is_term() {
+            return Ok(None);
+        }
+
+        let result = match self {
+            OpenFile::Stdin => Some(nix::sys::termios::tcgetattr(std::io::stdin())?),
+            OpenFile::Stdout => Some(nix::sys::termios::tcgetattr(std::io::stdout())?),
+            OpenFile::Stderr => Some(nix::sys::termios::tcgetattr(std::io::stderr())?),
+            OpenFile::Null => None,
+            OpenFile::File(f) => Some(nix::sys::termios::tcgetattr(f)?),
+            OpenFile::PipeReader(_) => None,
+            OpenFile::PipeWriter(_) => None,
+            OpenFile::HereDocument(_) => None,
+        };
+        Ok(result)
+    }
+
+    pub(crate) fn set_term_attr(
+        &self,
+        action: nix::sys::termios::SetArg,
+        termios: &nix::sys::termios::Termios,
+    ) -> Result<(), error::Error> {
+        match self {
+            OpenFile::Stdin => nix::sys::termios::tcsetattr(std::io::stdin(), action, termios)?,
+            OpenFile::Stdout => nix::sys::termios::tcsetattr(std::io::stdout(), action, termios)?,
+            OpenFile::Stderr => nix::sys::termios::tcsetattr(std::io::stderr(), action, termios)?,
+            OpenFile::Null => (),
+            OpenFile::File(f) => nix::sys::termios::tcsetattr(f, action, termios)?,
+            OpenFile::PipeReader(_) => (),
+            OpenFile::PipeWriter(_) => (),
+            OpenFile::HereDocument(_) => (),
+        }
+        Ok(())
+    }
 }
 
 impl From<OpenFile> for Stdio {

brush-shell/tests/cases/builtins/read.yaml

@@ -17,3 +17,21 @@ cases:
           b
     stdin: |
       (echo a; echo b) | while read name; do echo "Hello, $name"; done
+
+  - name: "read from here string"
+    known_failure: true
+    stdin: |
+      read myvar <<< "hello"
+      echo "myvar: ${myvar}"
+
+  - name: "read from process substitution"
+    known_failure: true
+    stdin: |
+      read myvar < <(echo hello)
+      echo "myvar: ${myvar}"
+
+  - name: "read from command substitution"
+    known_failure: true
+    stdin: |
+      read myvar <<< "$(echo hello)"
+      echo "myvar: ${myvar}"

brush-shell/tests/cases/pipeline.yaml

@@ -25,3 +25,14 @@ cases:
           exit $1
     stdin: |
       ./script.sh 10 | ./script.sh 0 | ./script.sh 33
+
+  - name: "Side effects in pipe commands"
+    stdin: |
+      var=0
+      echo "var: ${var}"
+      { var=1; }
+      echo "var: ${var}"
+      { var=2; echo hi; } | cat
+      echo "var: ${var}"
+      echo hi | { var=3; cat; }
+      echo "var: ${var}"