From 1ab6304c3eb31d184049968f7ba53c52b31995f2 Mon Sep 17 00:00:00 2001
From: Xavier L'Heureux <xavier.lheureux@icloud.com>
Date: Tue, 2 Jul 2019 17:00:36 -0400
Subject: [PATCH] Don't fork functions, prefer to execute in foreground
When the result is needed, use the .command method on Expander to
streamline the expansion
When the function is blocking (ex: COMMAND_NOT_FOUND), don't fork and
execute the function in foreground instead
---
src/binary/mod.rs | 18 +++++++++---------
src/binary/prompt.rs | 25 ++-----------------------
src/lib/builtins/mod.rs | 6 ++++--
src/lib/shell/fork.rs | 8 --------
src/lib/shell/fork_function.rs | 34 ----------------------------------
src/lib/shell/mod.rs | 4 +---
6 files changed, 16 insertions(+), 79 deletions(-)
delete mode 100644 src/lib/shell/fork_function.rs
diff --git a/src/binary/mod.rs b/src/binary/mod.rs
index d7e030c3..fa0cc5cd 100644
--- a/src/binary/mod.rs
+++ b/src/binary/mod.rs
@@ -11,7 +11,7 @@ use ion_shell::{
builtins::{man_pages, Status},
expansion::Expander,
parser::Terminator,
- types, Capture, IonError, PipelineError, Shell, Signal,
+ types, IonError, PipelineError, Shell, Signal, Value,
};
use itertools::Itertools;
use liner::{Buffer, Context, KeyBindings};
@@ -228,15 +228,15 @@ impl<'a> InteractiveShell<'a> {
Err(IonError::PipelineExecutionError(
PipelineError::CommandNotFound(command),
)) => {
- if shell
- .fork_function(
- Capture::None,
- |_| Ok(()),
- "COMMAND_NOT_FOUND",
- &["ion", &command],
- )
- .is_err()
+ if let Some(Value::Function(func)) =
+ shell.variables().get("COMMAND_NOT_FOUND").cloned()
{
+ if let Err(why) =
+ shell.execute_function(&func, &["ion", &command])
+ {
+ eprintln!("ion: command not found handler: {}", why);
+ }
+ } else {
eprintln!("ion: command not found: {}", command);
}
// Status::COULD_NOT_EXEC
diff --git a/src/binary/prompt.rs b/src/binary/prompt.rs
index e1252037..9758eb97 100644
--- a/src/binary/prompt.rs
+++ b/src/binary/prompt.rs
@@ -1,6 +1,5 @@
use super::InteractiveShell;
-use ion_shell::{expansion::Expander, Capture, Shell};
-use std::io::Read;
+use ion_shell::expansion::Expander;
impl<'a> InteractiveShell<'a> {
/// Generates the prompt that will be used by Liner.
@@ -9,7 +8,7 @@ impl<'a> InteractiveShell<'a> {
let blocks = shell.block_len() + if shell.unterminated { 1 } else { 0 };
if blocks == 0 {
- Self::prompt_fn(&shell).unwrap_or_else(|| {
+ shell.command("PROMPT").map(|res| res.to_string()).unwrap_or_else(|_| {
match shell.get_string(&shell.variables().get_str("PROMPT").unwrap_or_default()) {
Ok(prompt) => prompt.to_string(),
Err(why) => {
@@ -22,24 +21,4 @@ impl<'a> InteractiveShell<'a> {
" ".repeat(blocks)
}
}
-
- pub fn prompt_fn(shell: &Shell<'_>) -> Option<String> {
- shell
- .fork_function(
- Capture::StdoutThenIgnoreStderr,
- |result| {
- let mut string = String::with_capacity(1024);
- match result.stdout.ok_or(())?.read_to_string(&mut string) {
- Ok(_) => Ok(string),
- Err(why) => {
- eprintln!("ion: error reading stdout of child: {}", why);
- Err(())
- }
- }
- },
- "PROMPT",
- &["ion"],
- )
- .ok()
- }
}
diff --git a/src/lib/builtins/mod.rs b/src/lib/builtins/mod.rs
index 3a11d102..2a94989e 100644
--- a/src/lib/builtins/mod.rs
+++ b/src/lib/builtins/mod.rs
@@ -35,7 +35,7 @@ pub use self::{
};
use crate as ion_shell;
use crate::{
- shell::{Capture, Shell, Value},
+ shell::{Shell, Value},
types,
};
use builtins_proc::builtin;
@@ -308,7 +308,9 @@ pub fn cd(args: &[types::Str], shell: &mut Shell<'_>) -> Status {
match err {
Ok(()) => {
- let _ = shell.fork_function(Capture::None, |_| Ok(()), "CD_CHANGE", &["ion"]);
+ if let Some(Value::Function(function)) = shell.variables().get("CD_CHANGE").cloned() {
+ let _ = shell.execute_function(&function, &["ion"]);
+ }
Status::SUCCESS
}
Err(why) => Status::error(format!("{}", why)),
diff --git a/src/lib/shell/fork.rs b/src/lib/shell/fork.rs
index 39e3db60..84fbccf5 100644
--- a/src/lib/shell/fork.rs
+++ b/src/lib/shell/fork.rs
@@ -22,24 +22,16 @@ pub fn wait_for_child(pid: unistd::Pid) -> nix::Result<i32> {
///
/// A type that is utilized by the `Fork` structure.
pub enum Capture {
- /// Don't capture any streams at all.
- None = 0b0000,
/// Capture just the standard output stream.
Stdout = 0b0001,
/// Capture just the standard error stream.
Stderr = 0b0010,
- /// Capture both the standard output and error streams.
- Both = 0b0011,
/// Redirect just the stdandard output stream to /dev/null.
IgnoreStdout = 0b0100,
/// Redirect just the standard error stream to /dev/null.
IgnoreStderr = 0b1000,
- /// Redirect both the standard output and error streams to /dev/null.
- IgnoreBoth = 0b1100,
/// Capture standard output and ignore standard error.
StdoutThenIgnoreStderr = 0b1001,
- /// Capture standard error and ignore standard output.
- StderrThenIgnoreStdout = 0b0110,
}
/// Utilized by the shell for performing forks and capturing streams.
diff --git a/src/lib/shell/fork_function.rs b/src/lib/shell/fork_function.rs
deleted file mode 100644
index c3d9679d..00000000
--- a/src/lib/shell/fork_function.rs
+++ /dev/null
@@ -1,34 +0,0 @@
-use super::{fork::IonResult, variables::Value, Capture, Shell};
-use nix::unistd::{self, Pid};
-
-impl<'a> Shell<'a> {
- /// High-level function for executing a function programmatically.
- /// NOTE: Always add "ion" as a first argument in `args`.
- pub fn fork_function<S: AsRef<str>, T, F: FnOnce(IonResult) -> Result<T, ()>>(
- &self,
- capture: Capture,
- result: F,
- fn_name: &str,
- args: &[S],
- ) -> Result<T, ()> {
- if let Some(Value::Function(function)) = self.variables.get(fn_name) {
- let output = self
- .fork(capture, move |child| {
- if let Err(err) = function.execute(child, args) {
- if capture == Capture::None {
- eprintln!("ion: {} function call: {}", fn_name, err);
- }
- }
- Ok(())
- })
- .map_err(|err| eprintln!("ion: fork error: {}", err))
- .and_then(result);
-
- // Ensure that the parent retains ownership of the terminal before exiting.
- let _ = unistd::tcsetpgrp(nix::libc::STDIN_FILENO, Pid::this());
- output
- } else {
- Err(())
- }
- }
-}
diff --git a/src/lib/shell/mod.rs b/src/lib/shell/mod.rs
index c30efcb0..10b5e9d6 100644
--- a/src/lib/shell/mod.rs
+++ b/src/lib/shell/mod.rs
@@ -5,7 +5,6 @@ mod flow;
/// The various blocks
pub mod flow_control;
mod fork;
-mod fork_function;
mod job;
mod pipe_exec;
mod shell_expand;
@@ -18,13 +17,12 @@ pub(crate) use self::job::{Job, RefinedJob};
use self::{
directory_stack::DirectoryStack,
flow_control::{Block, Function, FunctionError, Statement},
- fork::{Fork, IonResult},
+ fork::{Capture, Fork, IonResult},
pipe_exec::foreground,
variables::Variables,
};
pub use self::{
flow::BlockError,
- fork::Capture,
pipe_exec::{
job_control::{BackgroundEvent, BackgroundProcess},
PipelineError,
--
GitLab