Merge pull request #170 from azriel91/feature/169/remove-progress-bar…

…-rendering-artifact
azriel91 · Dec 29, 2023 · 6e58244 · 6e58244
2 parents 3c85023 + 8d5af4d
commit 6e58244
Show file tree

Hide file tree

Showing 11 changed files with 142 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@
 * Add interruptibility support using [`interruptible`] through `CmdCtxBuilder::with_interruptibility`. ([#141], [#163])
 * Add `ItemStreamOutcome` to track which `Item`s are processed or not processed. ([#164], [#165])
 * Suppress progress rendering for `StatesCurrentReadCmd`, `StatesGoalReadCmd`, and `DiffCmd`. ([#167], [#168])
+* Suppress control character echo on `stdin`, which removes progress bar rendering artifact. ([#169], [#170])
 
 
 [`interruptible`]: https://github.com/azriel91/interruptible
@@ -16,6 +17,8 @@
 [#165]: https://github.com/azriel91/peace/pull/165
 [#167]: https://github.com/azriel91/peace/issues/167
 [#168]: https://github.com/azriel91/peace/pull/168
+[#169]: https://github.com/azriel91/peace/issues/169
+[#170]: https://github.com/azriel91/peace/pull/170
 
 
 ## 0.0.11 (2023-06-27)

diff --git a/Cargo.toml b/Cargo.toml
@@ -124,10 +124,12 @@ heck = "0.4.1"
 indexmap = "2.1.0"
 indicatif = "0.17.7"
 interruptible = "0.0.3"
+libc = "0.2.151"
 miette = "5.10.0"
 pretty_assertions = "1.4.0"
 proc-macro2 = "1.0.71"
 quote = "1.0.33"
+raw_tty = "0.1.0"
 reqwest = "0.11.23"
 resman = "0.17.0"
 serde = "1.0.193"

diff --git a/crate/rt_model_native/Cargo.toml b/crate/rt_model_native/Cargo.toml
@@ -32,6 +32,10 @@ thiserror = { workspace = true }
 tokio = { workspace = true, features = ["fs", "io-std"] }
 tokio-util = { workspace = true, features = ["io", "io-util"] }
 
+[target.'cfg(unix)'.dependencies]
+libc = { workspace = true }
+raw_tty = { workspace = true }
+
 [features]
 default = []
 error_reporting = ["dep:miette", "peace_rt_model_core/error_reporting"]

diff --git a/crate/rt_model_native/src/output/cli_output.rs b/crate/rt_model_native/src/output/cli_output.rs
@@ -1,3 +1,5 @@
+use std::fmt::{self, Debug};
+
 use peace_fmt::Presentable;
 use peace_rt_model_core::{
     async_trait,
@@ -56,7 +58,7 @@ cfg_if::cfg_if! {
 /// [`with_colorized`]: CliOutputBuilder::with_colorized
 /// [`with_progress_format`]: CliOutputBuilder::with_progress_format
 /// [`with_progress_target`]: CliOutputBuilder::with_progress_target
-#[derive(Debug)]
+#[cfg_attr(not(unix), derive(Debug))]
 pub struct CliOutput<W> {
     /// Output stream to write the command outcome to.
     pub(crate) writer: W,
@@ -76,6 +78,46 @@ pub struct CliOutput<W> {
     /// Width of the item ID column for progress bars
     #[cfg(feature = "output_progress")]
     pub(crate) pb_item_id_width: Option<usize>,
+    /// The TTY guard that restores the terminal mode when `CliOutput` is
+    /// dropped.
+    ///
+    /// This is used to suppress control character echo, e.g. `SIGINT` rendering
+    /// `^C\n`.
+    #[cfg(unix)]
+    pub(crate) stdin_tty_with_guard: Option<raw_tty::TtyWithGuard<std::io::Stdin>>,
+}
+
+#[cfg(unix)]
+impl<W> Debug for CliOutput<W>
+where
+    W: Debug,
+{
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let mut debug_struct = f.debug_struct("CliOutput");
+        debug_struct
+            .field("writer", &self.writer)
+            .field("outcome_format", &self.outcome_format)
+            .field("colorize", &self.colorize);
+
+        #[cfg(feature = "output_progress")]
+        {
+            debug_struct
+                .field("progress_target", &self.progress_target)
+                .field("progress_format", &self.progress_format)
+                .field("pb_item_id_width", &self.pb_item_id_width);
+        }
+
+        debug_struct.field(
+            "stdin_tty_with_guard",
+            if self.stdin_tty_with_guard.is_some() {
+                &Some(..)
+            } else {
+                &None::<()>
+            },
+        );
+
+        debug_struct.finish()
+    }
 }
 
 impl CliOutput<Stdout> {

diff --git a/crate/rt_model_native/src/output/cli_output_builder.rs b/crate/rt_model_native/src/output/cli_output_builder.rs
@@ -237,6 +237,48 @@ where
             CliProgressFormatOpt::None => CliProgressFormat::None,
         };
 
+        // We need to suppress the `^C\n` characters that come through the terminal.
+        //
+        // This is a combination of reading the following:
+        //
+        // * <https://stackoverflow.com/questions/42563400/hide-c-pressing-ctrl-c-in-c>
+        // * <https://docs.rs/libc/latest/libc/constant.ECHOCTL.html>
+        // * <https://docs.rs/raw_tty/0.1.0/raw_tty/struct.TtyWithGuard.html>
+        //
+        // Also considered were:
+        //
+        // * [`uu_stty`](https://crates.io/crates/uu_stty)], but it doesn't look like it
+        //   supports modifying the terminal.
+        // * [`unix_tty`](https://docs.rs/unix-tty/0.3.4/unix_tty/), which looks like it
+        //   can do the job, but `raw_tty` has a drop guard to restore the original tty
+        //   settings.
+        #[cfg(unix)]
+        let stdin_tty_with_guard = {
+            use raw_tty::GuardMode;
+
+            match std::io::stdin().guard_mode() {
+                Ok(mut stdin_tty_with_guard) => {
+                    let mode_modify_result = stdin_tty_with_guard.modify_mode(|mut ios| {
+                        ios.c_lflag &= !libc::ECHOCTL;
+                        ios
+                    });
+                    match mode_modify_result {
+                        Ok(()) => {}
+                        Err(error) => {
+                            #[cfg(debug_assertions)]
+                            eprintln!("warn: Failed to modify termios mode:\n{error}");
+                        }
+                    }
+                    Some(stdin_tty_with_guard)
+                }
+                Err(error) => {
+                    #[cfg(debug_assertions)]
+                    eprintln!("warn: Failed to acquire stdin termios:\n{error}");
+                    None
+                }
+            }
+        };
+
         CliOutput {
             writer,
             outcome_format,
@@ -247,6 +289,8 @@ where
             progress_format,
             #[cfg(feature = "output_progress")]
             pb_item_id_width: None,
+            #[cfg(unix)]
+            stdin_tty_with_guard,
         }
     }
 }

diff --git a/examples/envman/Cargo.toml b/examples/envman/Cargo.toml
@@ -65,7 +65,7 @@ js-sys = "0.3.66"
 web-sys = "0.3.66"
 
 [features]
-default = ["cli"]
+default = []
 
 # === envman modes === #
 cli = [

diff --git a/examples/envman/src/cmds/env_clean_cmd.rs b/examples/envman/src/cmds/env_clean_cmd.rs
@@ -26,27 +26,28 @@ impl EnvCleanCmd {
     /// # Parameters
     ///
     /// * `output`: Output to write the execution outcome.
-    pub async fn run<O>(output: &mut O) -> Result<(), EnvManError>
+    /// * `debug`: Whether to print `CmdOutcome` debug info.
+    pub async fn run<O>(output: &mut O, debug: bool) -> Result<(), EnvManError>
     where
         O: OutputWrite<EnvManError> + Send,
     {
         let workspace = workspace()?;
         let env_man_flow = env_man_flow(output, &workspace).await?;
         match env_man_flow {
-            EnvManFlow::AppUpload => run!(output, AppUploadCmd, 14usize),
-            EnvManFlow::EnvDeploy => run!(output, EnvCmd, 18usize),
+            EnvManFlow::AppUpload => run!(output, AppUploadCmd, 14usize, debug),
+            EnvManFlow::EnvDeploy => run!(output, EnvCmd, 18usize, debug),
         };
 
         Ok(())
     }
 }
 
 macro_rules! run {
-    ($output:ident, $flow_cmd:ident, $padding:expr) => {{
+    ($output:ident, $flow_cmd:ident, $padding:expr, $debug:expr) => {{
         $flow_cmd::run(
             $output,
             CmdOpts::default().with_profile_print(false),
-            |cmd_ctx| run_with_ctx(cmd_ctx, $padding).boxed_local(),
+            |cmd_ctx| run_with_ctx(cmd_ctx, $padding, $debug).boxed_local(),
         )
         .await?;
     }};
@@ -55,6 +56,7 @@ macro_rules! run {
 async fn run_with_ctx<O>(
     cmd_ctx: &mut EnvManCmdCtx<'_, O, SetUp>,
     padding_count: usize,
+    debug: bool,
 ) -> Result<(), EnvManError>
 where
     O: OutputWrite<EnvManError>,
@@ -100,8 +102,10 @@ where
             .await?;
     }
 
-    crate::output::cmd_outcome_completion_present(output, resources, states_cleaned_outcome)
-        .await?;
+    if debug {
+        crate::output::cmd_outcome_completion_present(output, resources, states_cleaned_outcome)
+            .await?;
+    }
 
     Ok(())
 }

diff --git a/examples/envman/src/cmds/env_deploy_cmd.rs b/examples/envman/src/cmds/env_deploy_cmd.rs
@@ -26,27 +26,28 @@ impl EnvDeployCmd {
     /// # Parameters
     ///
     /// * `output`: Output to write the execution outcome.
-    pub async fn run<O>(output: &mut O) -> Result<(), EnvManError>
+    /// * `debug`: Whether to print `CmdOutcome` debug info.
+    pub async fn run<O>(output: &mut O, debug: bool) -> Result<(), EnvManError>
     where
         O: OutputWrite<EnvManError> + Send,
     {
         let workspace = workspace()?;
         let env_man_flow = env_man_flow(output, &workspace).await?;
         match env_man_flow {
-            EnvManFlow::AppUpload => run!(output, AppUploadCmd, 14usize),
-            EnvManFlow::EnvDeploy => run!(output, EnvCmd, 18usize),
+            EnvManFlow::AppUpload => run!(output, AppUploadCmd, 14usize, debug),
+            EnvManFlow::EnvDeploy => run!(output, EnvCmd, 18usize, debug),
         };
 
         Ok(())
     }
 }
 
 macro_rules! run {
-    ($output:ident, $flow_cmd:ident, $padding:expr) => {{
+    ($output:ident, $flow_cmd:ident, $padding:expr, $debug:expr) => {{
         $flow_cmd::run(
             $output,
             CmdOpts::default().with_profile_print(false),
-            |cmd_ctx| run_with_ctx(cmd_ctx, $padding).boxed_local(),
+            |cmd_ctx| run_with_ctx(cmd_ctx, $padding, $debug).boxed_local(),
         )
         .await?;
     }};
@@ -55,6 +56,7 @@ macro_rules! run {
 async fn run_with_ctx<O>(
     cmd_ctx: &mut EnvManCmdCtx<'_, O, SetUp>,
     padding_count: usize,
+    debug: bool,
 ) -> Result<(), EnvManError>
 where
     O: OutputWrite<EnvManError>,
@@ -100,8 +102,10 @@ where
             .await?;
     }
 
-    crate::output::cmd_outcome_completion_present(output, resources, states_ensured_outcome)
-        .await?;
+    if debug {
+        crate::output::cmd_outcome_completion_present(output, resources, states_ensured_outcome)
+            .await?;
+    }
 
     Ok(())
 }

diff --git a/examples/envman/src/cmds/env_discover_cmd.rs b/examples/envman/src/cmds/env_discover_cmd.rs
@@ -26,25 +26,26 @@ impl EnvDiscoverCmd {
     /// # Parameters
     ///
     /// * `output`: Output to write the execution outcome.
-    pub async fn run<O>(output: &mut O) -> Result<(), EnvManError>
+    /// * `debug`: Whether to print `CmdOutcome` debug info.
+    pub async fn run<O>(output: &mut O, debug: bool) -> Result<(), EnvManError>
     where
         O: OutputWrite<EnvManError> + Send,
     {
         let workspace = workspace()?;
         let env_man_flow = env_man_flow(output, &workspace).await?;
         match env_man_flow {
-            EnvManFlow::AppUpload => run!(output, AppUploadCmd, 14usize),
-            EnvManFlow::EnvDeploy => run!(output, EnvCmd, 18usize),
+            EnvManFlow::AppUpload => run!(output, AppUploadCmd, 14usize, debug),
+            EnvManFlow::EnvDeploy => run!(output, EnvCmd, 18usize, debug),
         }
 
         Ok(())
     }
 }
 
 macro_rules! run {
-    ($output:ident, $flow_cmd:ident, $padding:expr) => {{
+    ($output:ident, $flow_cmd:ident, $padding:expr, $debug:expr) => {{
         $flow_cmd::run($output, CmdOpts::default(), |cmd_ctx| {
-            run_with_ctx(cmd_ctx, $padding).boxed_local()
+            run_with_ctx(cmd_ctx, $padding, $debug).boxed_local()
         })
         .await?;
     }};
@@ -53,6 +54,7 @@ macro_rules! run {
 async fn run_with_ctx<O>(
     cmd_ctx: &mut EnvManCmdCtx<'_, O, SetUp>,
     padding_count: usize,
+    debug: bool,
 ) -> Result<(), EnvManError>
 where
     O: OutputWrite<EnvManError>,
@@ -120,8 +122,10 @@ where
             .await?;
     }
 
-    crate::output::cmd_outcome_completion_present(output, resources, states_discover_outcome)
-        .await?;
+    if debug {
+        crate::output::cmd_outcome_completion_present(output, resources, states_discover_outcome)
+            .await?;
+    }
 
     Ok(())
 }

diff --git a/examples/envman/src/main_cli.rs b/examples/envman/src/main_cli.rs
@@ -23,6 +23,7 @@ pub fn run() -> Result<(), EnvManError> {
         fast,
         format,
         color,
+        debug,
     } = CliArgs::parse();
 
     let runtime = if fast {
@@ -47,7 +48,7 @@ pub fn run() -> Result<(), EnvManError> {
             builder.build()
         };
 
-        match run_command(&mut cli_output, command).await {
+        match run_command(&mut cli_output, command, debug).await {
             Ok(()) => Ok(()),
             Err(error) => envman::output::errors_present(&mut cli_output, &[error]).await,
         }
@@ -57,6 +58,7 @@ pub fn run() -> Result<(), EnvManError> {
 async fn run_command(
     cli_output: &mut CliOutput<Stdout>,
     command: EnvManCommand,
+    debug: bool,
 ) -> Result<(), EnvManError> {
     match command {
         EnvManCommand::Init {
@@ -106,7 +108,7 @@ async fn run_command(
             };
             ProfileSwitchCmd::run(cli_output, profile_switch).await?
         }
-        EnvManCommand::Discover => EnvDiscoverCmd::run(cli_output).await?,
+        EnvManCommand::Discover => EnvDiscoverCmd::run(cli_output, debug).await?,
         EnvManCommand::Status => EnvStatusCmd::run(cli_output).await?,
         EnvManCommand::Goal => EnvGoalCmd::run(cli_output).await?,
         EnvManCommand::Diff {
@@ -125,8 +127,8 @@ async fn run_command(
 
             EnvDiffCmd::run(cli_output, env_diff_selection).await?
         }
-        EnvManCommand::Deploy => EnvDeployCmd::run(cli_output).await?,
-        EnvManCommand::Clean => EnvCleanCmd::run(cli_output).await?,
+        EnvManCommand::Deploy => EnvDeployCmd::run(cli_output, debug).await?,
+        EnvManCommand::Clean => EnvCleanCmd::run(cli_output, debug).await?,
         #[cfg(feature = "web_server")]
         EnvManCommand::Web { address, port } => {
             WebServer::start(Some(SocketAddr::from((address, port)))).await?