rename actions to ProcessActionsLines and remove unnecessary heap allocation

Author: zeegomo

src/cmd/actions.rs

@@ -1,16 +1,17 @@
 //! Command execution and sandboxing.
 
-mod actions;
+mod process_lines_actions;
 mod sandbox;
 
-pub use actions::Actions;
+pub use process_lines_actions::ProcessLinesActions;
 pub use sandbox::*;
 
 use crate::native;
 use crate::workspace::Workspace;
 use failure::{Error, Fail};
 use futures::{future, Future, Stream};
 use log::{error, info};
+use process_lines_actions::InnerState;
 use std::convert::AsRef;
 use std::env::consts::EXE_SUFFIX;
 use std::ffi::{OsStr, OsString};
@@ -127,7 +128,7 @@ pub struct Command<'w, 'pl> {
     binary: Binary,
     args: Vec<OsString>,
     env: Vec<(OsString, OsString)>,
-    process_lines: Option<&'pl mut dyn FnMut(&str, &mut Actions)>,
+    process_lines: Option<&'pl mut dyn FnMut(&str, &mut ProcessLinesActions)>,
     cd: Option<PathBuf>,
     timeout: Option<Duration>,
     no_output_timeout: Option<Duration>,
@@ -251,7 +252,7 @@ impl<'w, 'pl> Command<'w, 'pl> {
     /// # Ok(())
     /// # }
     /// ```
-    pub fn process_lines(mut self, f: &'pl mut dyn FnMut(&str, &mut Actions)) -> Self {
+    pub fn process_lines(mut self, f: &'pl mut dyn FnMut(&str, &mut ProcessLinesActions)) -> Self {
         self.process_lines = Some(f);
         self
     }
@@ -482,7 +483,7 @@ impl OutputKind {
 
 fn log_command(
     mut cmd: StdCommand,
-    mut process_lines: Option<&mut dyn FnMut(&str, &mut Actions)>,
+    mut process_lines: Option<&mut dyn FnMut(&str, &mut ProcessLinesActions)>,
     capture: bool,
     timeout: Option<Duration>,
     no_output_timeout: Option<Duration>,
@@ -514,7 +515,7 @@ fn log_command(
         .map(|line| (OutputKind::Stderr, line));
 
     let start = Instant::now();
-    let mut actions = Actions::new();
+    let mut actions = ProcessLinesActions::new();
 
     let output = stdout
         .select(stderr)
@@ -536,11 +537,15 @@ fn log_command(
                 return future::err(Error::from(CommandError::Timeout(timeout.as_secs())));
             }
 
-            actions.next_input(&line);
             if let Some(f) = &mut process_lines {
                 f(&line, &mut actions);
             }
-            let lines = actions.take_lines();
+            // this is done here to avoid duplicating the output line
+            let lines = match actions.take_lines() {
+                InnerState::Removed => Vec::new(),
+                InnerState::Original => vec![line],
+                InnerState::Replaced(new_lines) => new_lines,
+            };
 
             if log_output {
                 for line in &lines {

src/cmd/mod.rs

@@ -0,0 +1,89 @@
+use std::default::Default;
+
+#[cfg_attr(test, derive(Debug, PartialEq))]
+pub(super) enum InnerState {
+    Removed,
+    Original,
+    Replaced(Vec<String>),
+}
+
+impl Default for InnerState {
+    fn default() -> Self {
+        InnerState::Original
+    }
+}
+
+/// Represents actions that are available while reading live output from a process.
+///
+/// This will be available inside the function you provide to [`Command::process_lines`](struct.Command.html#method.process_lines)
+pub struct ProcessLinesActions {
+    state: InnerState,
+}
+
+impl<'a> ProcessLinesActions {
+    pub(super) fn new() -> Self {
+        ProcessLinesActions {
+            state: InnerState::default(),
+        }
+    }
+
+    pub(super) fn take_lines(&mut self) -> InnerState {
+        std::mem::take(&mut self.state)
+    }
+
+    /// Replace last read line from output with the lines provided.
+    ///
+    /// The new lines will be logged instead of the original line.
+    pub fn replace_with_lines(&mut self, new_lines: impl Iterator<Item = &'a str>) {
+        self.state = InnerState::Replaced(new_lines.map(|str| str.to_string()).collect());
+    }
+
+    /// Remove last read line from output.
+    ///
+    /// This means that the line will not be logged.
+    pub fn remove_line(&mut self) {
+        self.state = InnerState::Removed;
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::InnerState;
+    use super::ProcessLinesActions;
+    #[test]
+    fn test_replace() {
+        let mut actions = ProcessLinesActions::new();
+
+        actions.replace_with_lines("ipsum".split("\n"));
+        assert_eq!(
+            actions.take_lines(),
+            InnerState::Replaced(vec!["ipsum".to_string()])
+        );
+
+        actions.replace_with_lines("lorem ipsum dolor".split(" "));
+        assert_eq!(
+            actions.take_lines(),
+            InnerState::Replaced(vec![
+                "lorem".to_string(),
+                "ipsum".to_string(),
+                "dolor".to_string()
+            ])
+        );
+
+        // assert last input is discarded
+        assert_eq!(actions.take_lines(), InnerState::Original);
+    }
+
+    #[test]
+    fn test_remove() {
+        let mut actions = ProcessLinesActions::new();
+        actions.remove_line();
+        assert_eq!(actions.take_lines(), InnerState::Removed);
+    }
+
+    #[test]
+    fn test_no_actions() {
+        let mut actions = ProcessLinesActions::new();
+        assert_eq!(actions.take_lines(), InnerState::Original);
+    }
+}

src/cmd/process_lines_actions.rs

@@ -1,6 +1,5 @@
 use super::CrateTrait;
-use crate::cmd::Actions;
-use crate::cmd::Command;
+use crate::cmd::{Command, ProcessLinesActions};
 use crate::prepare::PrepareError;
 use crate::Workspace;
 use failure::{Error, ResultExt};
@@ -84,7 +83,7 @@ impl CrateTrait for GitRepo {
         //    fata: credential helper '{path}' told us to quit
         //
         let mut private_repository = false;
-        let mut detect_private_repositories = |line: &str, _actions: &mut Actions| {
+        let mut detect_private_repositories = |line: &str, _actions: &mut ProcessLinesActions| {
             if line.starts_with("fatal: credential helper") && line.ends_with("told us to quit") {
                 private_repository = true;
             }