From 5e386a533876c32d912c7baf6d66a01f9a195fe6 Mon Sep 17 00:00:00 2001 From: Azriel Hoh Date: Fri, 29 Dec 2023 15:40:12 +1300 Subject: [PATCH 1/5] Remove `"cli"` from `envman` default features. --- examples/envman/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/envman/Cargo.toml b/examples/envman/Cargo.toml index 09b5c7e8f..196c2d357 100644 --- 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 = [ From 7466a54664c8e0c2d181a8fd4a05e0447c72b87f Mon Sep 17 00:00:00 2001 From: Azriel Hoh Date: Fri, 29 Dec 2023 15:44:56 +1300 Subject: [PATCH 2/5] Add `debug` flag to `envman` example to toggle `CmdOutcome` debug info. --- examples/envman/src/cmds/env_clean_cmd.rs | 18 +++++++++++------- examples/envman/src/cmds/env_deploy_cmd.rs | 18 +++++++++++------- examples/envman/src/cmds/env_discover_cmd.rs | 18 +++++++++++------- examples/envman/src/main_cli.rs | 10 ++++++---- examples/envman/src/model/cli_args.rs | 3 +++ 5 files changed, 42 insertions(+), 25 deletions(-) diff --git a/examples/envman/src/cmds/env_clean_cmd.rs b/examples/envman/src/cmds/env_clean_cmd.rs index 95532bb44..419f9aaa6 100644 --- a/examples/envman/src/cmds/env_clean_cmd.rs +++ b/examples/envman/src/cmds/env_clean_cmd.rs @@ -26,15 +26,16 @@ impl EnvCleanCmd { /// # Parameters /// /// * `output`: Output to write the execution outcome. - pub async fn run(output: &mut O) -> Result<(), EnvManError> + /// * `debug`: Whether to print `CmdOutcome` debug info. + pub async fn run(output: &mut O, debug: bool) -> Result<(), EnvManError> where O: OutputWrite + 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(()) @@ -42,11 +43,11 @@ impl EnvCleanCmd { } 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( cmd_ctx: &mut EnvManCmdCtx<'_, O, SetUp>, padding_count: usize, + debug: bool, ) -> Result<(), EnvManError> where O: OutputWrite, @@ -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 index b9edf68e9..6da2591d1 100644 --- a/examples/envman/src/cmds/env_deploy_cmd.rs +++ b/examples/envman/src/cmds/env_deploy_cmd.rs @@ -26,15 +26,16 @@ impl EnvDeployCmd { /// # Parameters /// /// * `output`: Output to write the execution outcome. - pub async fn run(output: &mut O) -> Result<(), EnvManError> + /// * `debug`: Whether to print `CmdOutcome` debug info. + pub async fn run(output: &mut O, debug: bool) -> Result<(), EnvManError> where O: OutputWrite + 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(()) @@ -42,11 +43,11 @@ impl EnvDeployCmd { } 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( cmd_ctx: &mut EnvManCmdCtx<'_, O, SetUp>, padding_count: usize, + debug: bool, ) -> Result<(), EnvManError> where O: OutputWrite, @@ -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 index 69169cb7f..4c7b3abd7 100644 --- a/examples/envman/src/cmds/env_discover_cmd.rs +++ b/examples/envman/src/cmds/env_discover_cmd.rs @@ -26,15 +26,16 @@ impl EnvDiscoverCmd { /// # Parameters /// /// * `output`: Output to write the execution outcome. - pub async fn run(output: &mut O) -> Result<(), EnvManError> + /// * `debug`: Whether to print `CmdOutcome` debug info. + pub async fn run(output: &mut O, debug: bool) -> Result<(), EnvManError> where O: OutputWrite + 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(()) @@ -42,9 +43,9 @@ impl EnvDiscoverCmd { } 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( cmd_ctx: &mut EnvManCmdCtx<'_, O, SetUp>, padding_count: usize, + debug: bool, ) -> Result<(), EnvManError> where O: OutputWrite, @@ -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 index 4b4f637e8..dcaa37203 100644 --- 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, 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? diff --git a/examples/envman/src/model/cli_args.rs b/examples/envman/src/model/cli_args.rs index 336028933..bb08fb2e6 100644 --- a/examples/envman/src/model/cli_args.rs +++ b/examples/envman/src/model/cli_args.rs @@ -39,6 +39,9 @@ pub struct CliArgs { /// * "never": Never colorize output. #[arg(long, default_value = "auto")] pub color: CliColorizeOpt, + /// Whether to show debug information. + #[arg(long, default_value = "false")] + pub debug: bool, } #[derive(Subcommand)] From 6aee78c3fb0e7cc6e8c0388b7afc23474c2864cf Mon Sep 17 00:00:00 2001 From: Azriel Hoh Date: Fri, 29 Dec 2023 19:35:39 +1300 Subject: [PATCH 3/5] Update `envman` `CliArgs` to be global, for easier CLI usage. --- examples/envman/src/model/cli_args.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/envman/src/model/cli_args.rs b/examples/envman/src/model/cli_args.rs index bb08fb2e6..f6757b4a6 100644 --- a/examples/envman/src/model/cli_args.rs +++ b/examples/envman/src/model/cli_args.rs @@ -23,24 +23,24 @@ pub struct CliArgs { #[command(subcommand)] pub command: EnvManCommand, /// Whether to run with multiple threads. - #[arg(long, default_value = "false")] + #[arg(long, default_value = "false", global(true))] pub fast: bool, /// The format of the command output. /// /// At this level, this needs to be specified before the subcommand. /// needs to be implemented /// for the argument to be passed in after the subcommand. - #[arg(long)] + #[arg(long, global(true))] pub format: Option, /// Whether output should be colorized. /// /// * "auto" (default): Colorize when used interactively. /// * "always": Always colorize output. /// * "never": Never colorize output. - #[arg(long, default_value = "auto")] + #[arg(long, default_value = "auto", global(true))] pub color: CliColorizeOpt, /// Whether to show debug information. - #[arg(long, default_value = "false")] + #[arg(long, default_value = "false", global(true))] pub debug: bool, } From bd2bf5174b1016c44e2c449be34dd48e0d79e540 Mon Sep 17 00:00:00 2001 From: Azriel Hoh Date: Fri, 29 Dec 2023 20:08:36 +1300 Subject: [PATCH 4/5] Suppress control characters e.g. `^C\n` in `CliOutput`. --- Cargo.toml | 2 + crate/rt_model_native/Cargo.toml | 4 ++ .../rt_model_native/src/output/cli_output.rs | 44 ++++++++++++++++++- .../src/output/cli_output_builder.rs | 44 +++++++++++++++++++ 4 files changed, 93 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 9c888b9d4..57328ddba 100644 --- 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 index ee3f009cf..2036d7aa6 100644 --- 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 index e31b3d027..287af4f07 100644 --- 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 { /// Output stream to write the command outcome to. pub(crate) writer: W, @@ -76,6 +78,46 @@ pub struct CliOutput { /// Width of the item ID column for progress bars #[cfg(feature = "output_progress")] pub(crate) pb_item_id_width: Option, + /// 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>, +} + +#[cfg(unix)] +impl Debug for CliOutput +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 { diff --git a/crate/rt_model_native/src/output/cli_output_builder.rs b/crate/rt_model_native/src/output/cli_output_builder.rs index 0caa43fb0..ec365986f 100644 --- 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: + // + // * + // * + // * + // + // 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, } } } From 8d5af4dd3f65cbce0afae0a0c8df384fdc7a9bfd Mon Sep 17 00:00:00 2001 From: Azriel Hoh Date: Fri, 29 Dec 2023 20:25:39 +1300 Subject: [PATCH 5/5] Update `CHANGELOG.md`. --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98b10c4dc..a4c5e078b 100644 --- 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)