From b5cc51ab27fb82fb8a7f84d6c49ffd51abb291af Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Tue, 16 Dec 2025 23:34:19 +0900 Subject: [PATCH 1/2] feat: Add --no-prefix option to disable hostname prefix in output (pdsh -N compatibility) Add a new --no-prefix / -N option that disables the hostname prefix in command output lines. This is useful for programmatic parsing or cleaner display, and provides compatibility with pdsh's -N option. Changes: - Add --no-prefix / -N option to CLI struct in src/cli.rs - Add no_prefix field to ExecuteCommandParams - Modify OutputMode to track no_prefix setting for Stream and File modes - Update NodeOutputWriter to conditionally format output with/without prefix - Thread no_prefix option through ParallelExecutor streaming handlers - Add unit tests for no_prefix functionality Closes #92 --- src/app/dispatcher.rs | 1 + src/cli.rs | 7 +++ src/commands/exec.rs | 8 +++- src/executor/output_mode.rs | 93 ++++++++++++++++++++++++++++++++----- src/executor/output_sync.rs | 59 +++++++++++++++++------ src/executor/parallel.rs | 28 +++++++---- 6 files changed, 159 insertions(+), 37 deletions(-) diff --git a/src/app/dispatcher.rs b/src/app/dispatcher.rs index 9c2b6d4b..1ba026b3 100644 --- a/src/app/dispatcher.rs +++ b/src/app/dispatcher.rs @@ -390,6 +390,7 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu use_keychain, output_dir: cli.output_dir.as_deref(), stream: cli.stream, + no_prefix: cli.no_prefix, timeout, jump_hosts: cli.jump_hosts.as_deref(), port_forwards: if cli.has_port_forwards() { diff --git a/src/cli.rs b/src/cli.rs index 8f696aca..6658b6c6 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -125,6 +125,13 @@ pub struct Cli { )] pub stream: bool, + #[arg( + short = 'N', + long = "no-prefix", + help = "Disable hostname prefix in output lines (pdsh -N compatibility)\nUseful for programmatic parsing or cleaner display" + )] + pub no_prefix: bool, + #[arg( long, help = "Output directory for per-node command results\nCreates timestamped files:\n - hostname_TIMESTAMP.stdout (command output)\n - hostname_TIMESTAMP.stderr (error output)\n - hostname_TIMESTAMP.error (connection failures)\n - summary_TIMESTAMP.txt (execution summary)" diff --git a/src/commands/exec.rs b/src/commands/exec.rs index 69976179..830f520d 100644 --- a/src/commands/exec.rs +++ b/src/commands/exec.rs @@ -37,6 +37,7 @@ pub struct ExecuteCommandParams<'a> { pub use_keychain: bool, pub output_dir: Option<&'a Path>, pub stream: bool, + pub no_prefix: bool, pub timeout: Option, pub jump_hosts: Option<&'a str>, pub port_forwards: Option>, @@ -213,8 +214,11 @@ async fn execute_command_without_forwarding(params: ExecuteCommandParams<'_>) -> let executor = executor.with_keychain(params.use_keychain); // Determine output mode - let output_mode = - OutputMode::from_args(params.stream, params.output_dir.map(|p| p.to_path_buf())); + let output_mode = OutputMode::from_args_with_no_prefix( + params.stream, + params.output_dir.map(|p| p.to_path_buf()), + params.no_prefix, + ); // Execute with appropriate mode let results = if output_mode.is_normal() { diff --git a/src/executor/output_mode.rs b/src/executor/output_mode.rs index 4f2fd1ca..6f010651 100644 --- a/src/executor/output_mode.rs +++ b/src/executor/output_mode.rs @@ -36,13 +36,15 @@ pub enum OutputMode { /// Each line of output is prefixed with [hostname] and displayed /// in real-time as it arrives. This allows monitoring long-running /// commands across multiple nodes. - Stream, + /// The bool indicates whether to disable the prefix (no_prefix option). + Stream { no_prefix: bool }, /// File mode - save per-node output to separate files /// /// Each node's output is saved to a separate file in the specified /// directory. Files are named with hostname and timestamp. - File(PathBuf), + /// The bool indicates whether to disable the prefix in status messages. + File { path: PathBuf, no_prefix: bool }, /// TUI mode - interactive terminal UI with multiple view modes /// @@ -61,10 +63,28 @@ impl OutputMode { /// 3. Auto-detect TUI if TTY and no explicit mode /// 4. Default (Normal mode) pub fn from_args(stream: bool, output_dir: Option) -> Self { + Self::from_args_with_no_prefix(stream, output_dir, false) + } + + /// Create output mode from CLI arguments with no_prefix option + /// + /// Priority: + /// 1. --output-dir (File mode) + /// 2. --stream (Stream mode) + /// 3. Auto-detect TUI if TTY and no explicit mode + /// 4. Default (Normal mode) + pub fn from_args_with_no_prefix( + stream: bool, + output_dir: Option, + no_prefix: bool, + ) -> Self { if let Some(dir) = output_dir { - OutputMode::File(dir) + OutputMode::File { + path: dir, + no_prefix, + } } else if stream { - OutputMode::Stream + OutputMode::Stream { no_prefix } } else if is_tty() { // Auto-enable TUI mode for interactive terminals OutputMode::Tui @@ -77,10 +97,25 @@ impl OutputMode { /// /// Used when --no-tui or similar flags are present pub fn from_args_explicit(stream: bool, output_dir: Option, enable_tui: bool) -> Self { + Self::from_args_explicit_with_no_prefix(stream, output_dir, enable_tui, false) + } + + /// Create output mode with explicit TUI disable option and no_prefix + /// + /// Used when --no-tui or similar flags are present + pub fn from_args_explicit_with_no_prefix( + stream: bool, + output_dir: Option, + enable_tui: bool, + no_prefix: bool, + ) -> Self { if let Some(dir) = output_dir { - OutputMode::File(dir) + OutputMode::File { + path: dir, + no_prefix, + } } else if stream { - OutputMode::Stream + OutputMode::Stream { no_prefix } } else if enable_tui && is_tty() { OutputMode::Tui } else { @@ -95,12 +130,12 @@ impl OutputMode { /// Check if this is stream mode pub fn is_stream(&self) -> bool { - matches!(self, OutputMode::Stream) + matches!(self, OutputMode::Stream { .. }) } /// Check if this is file mode pub fn is_file(&self) -> bool { - matches!(self, OutputMode::File(_)) + matches!(self, OutputMode::File { .. }) } /// Check if this is TUI mode @@ -111,10 +146,19 @@ impl OutputMode { /// Get output directory if in file mode pub fn output_dir(&self) -> Option<&PathBuf> { match self { - OutputMode::File(dir) => Some(dir), + OutputMode::File { path, .. } => Some(path), _ => None, } } + + /// Check if prefix is disabled for this output mode + pub fn is_no_prefix(&self) -> bool { + match self { + OutputMode::Stream { no_prefix } => *no_prefix, + OutputMode::File { no_prefix, .. } => *no_prefix, + _ => false, + } + } } /// Check if stdout is a TTY @@ -191,12 +235,15 @@ mod tests { assert!(!normal.is_stream()); assert!(!normal.is_file()); - let stream = OutputMode::Stream; + let stream = OutputMode::Stream { no_prefix: false }; assert!(!stream.is_normal()); assert!(stream.is_stream()); assert!(!stream.is_file()); - let file = OutputMode::File(PathBuf::from("/tmp")); + let file = OutputMode::File { + path: PathBuf::from("/tmp"), + no_prefix: false, + }; assert!(!file.is_normal()); assert!(!file.is_stream()); assert!(file.is_file()); @@ -207,4 +254,28 @@ mod tests { let mode = OutputMode::default(); assert!(mode.is_normal()); } + + #[test] + fn test_no_prefix_option() { + // Stream mode with no_prefix + let mode = OutputMode::from_args_with_no_prefix(true, None, true); + assert!(mode.is_stream()); + assert!(mode.is_no_prefix()); + + // Stream mode without no_prefix + let mode = OutputMode::from_args_with_no_prefix(true, None, false); + assert!(mode.is_stream()); + assert!(!mode.is_no_prefix()); + + // File mode with no_prefix + let dir = PathBuf::from("/tmp/output"); + let mode = OutputMode::from_args_with_no_prefix(false, Some(dir.clone()), true); + assert!(mode.is_file()); + assert!(mode.is_no_prefix()); + assert_eq!(mode.output_dir(), Some(&dir)); + + // Normal mode (no_prefix doesn't apply) + let mode = OutputMode::Normal; + assert!(!mode.is_no_prefix()); + } } diff --git a/src/executor/output_sync.rs b/src/executor/output_sync.rs index 2e3d7d6f..939e4554 100644 --- a/src/executor/output_sync.rs +++ b/src/executor/output_sync.rs @@ -87,22 +87,36 @@ where /// Synchronized output writer for node prefixed output pub struct NodeOutputWriter { node_prefix: String, + no_prefix: bool, } impl NodeOutputWriter { - /// Create a new writer with a node prefix + /// Create a new writer with a node prefix (prefix enabled by default) + #[allow(dead_code)] pub fn new(node_host: &str) -> Self { + Self::new_with_no_prefix(node_host, false) + } + + /// Create a new writer with optional prefix disabled + pub fn new_with_no_prefix(node_host: &str, no_prefix: bool) -> Self { Self { node_prefix: format!("[{node_host}]"), + no_prefix, + } + } + + /// Format a line with or without prefix based on configuration + fn format_line(&self, line: &str) -> String { + if self.no_prefix { + line.to_string() + } else { + format!("{} {}", self.node_prefix, line) } } - /// Write stdout lines with node prefix atomically + /// Write stdout lines with optional node prefix atomically pub fn write_stdout_lines(&self, text: &str) -> io::Result<()> { - let lines: Vec = text - .lines() - .map(|line| format!("{} {}", self.node_prefix, line)) - .collect(); + let lines: Vec = text.lines().map(|line| self.format_line(line)).collect(); if !lines.is_empty() { let mut stdout = STDOUT_MUTEX.lock().unwrap(); @@ -114,12 +128,9 @@ impl NodeOutputWriter { Ok(()) } - /// Write stderr lines with node prefix atomically + /// Write stderr lines with optional node prefix atomically pub fn write_stderr_lines(&self, text: &str) -> io::Result<()> { - let lines: Vec = text - .lines() - .map(|line| format!("{} {}", self.node_prefix, line)) - .collect(); + let lines: Vec = text.lines().map(|line| self.format_line(line)).collect(); if !lines.is_empty() { let mut stderr = STDERR_MUTEX.lock().unwrap(); @@ -131,15 +142,15 @@ impl NodeOutputWriter { Ok(()) } - /// Write a single stdout line with node prefix + /// Write a single stdout line with optional node prefix pub fn write_stdout(&self, line: &str) -> io::Result<()> { - synchronized_println(&format!("{} {}", self.node_prefix, line)) + synchronized_println(&self.format_line(line)) } - /// Write a single stderr line with node prefix + /// Write a single stderr line with optional node prefix #[allow(dead_code)] pub fn write_stderr(&self, line: &str) -> io::Result<()> { - synchronized_eprintln(&format!("{} {}", self.node_prefix, line)) + synchronized_eprintln(&self.format_line(line)) } } @@ -151,6 +162,24 @@ mod tests { fn test_node_output_writer() { let writer = NodeOutputWriter::new("test-host"); assert_eq!(writer.node_prefix, "[test-host]"); + assert!(!writer.no_prefix); + } + + #[test] + fn test_node_output_writer_with_no_prefix() { + let writer = NodeOutputWriter::new_with_no_prefix("test-host", true); + assert_eq!(writer.node_prefix, "[test-host]"); + assert!(writer.no_prefix); + + // Test format_line with no_prefix enabled + assert_eq!(writer.format_line("test output"), "test output"); + + // Test with no_prefix disabled + let writer_with_prefix = NodeOutputWriter::new_with_no_prefix("test-host", false); + assert_eq!( + writer_with_prefix.format_line("test output"), + "[test-host] test output" + ); } #[test] diff --git a/src/executor/parallel.rs b/src/executor/parallel.rs index 4d027718..0270c6f6 100644 --- a/src/executor/parallel.rs +++ b/src/executor/parallel.rs @@ -616,15 +616,17 @@ impl ParallelExecutor { } // Execute based on mode and ensure cleanup + let no_prefix = output_mode.is_no_prefix(); let result = if output_mode.is_tui() { // TUI mode: interactive terminal UI self.handle_tui_mode(&mut manager, handles, command).await } else if output_mode.is_stream() { - // Stream mode: output in real-time with [node] prefixes - self.handle_stream_mode(&mut manager, handles).await + // Stream mode: output in real-time with optional [node] prefixes + self.handle_stream_mode(&mut manager, handles, no_prefix) + .await } else if let Some(output_dir) = output_mode.output_dir() { // File mode: save to per-node files - self.handle_file_mode(&mut manager, handles, output_dir) + self.handle_file_mode(&mut manager, handles, output_dir, no_prefix) .await } else { // Fallback to normal mode @@ -634,11 +636,12 @@ impl ParallelExecutor { result } - /// Handle stream mode output with [node] prefixes + /// Handle stream mode output with optional [node] prefixes async fn handle_stream_mode( &self, manager: &mut super::stream_manager::MultiNodeStreamManager, handles: Vec)>>, + no_prefix: bool, ) -> Result> { use super::output_sync::NodeOutputWriter; use std::time::Duration; @@ -651,7 +654,7 @@ impl ParallelExecutor { // Poll all streams for new output manager.poll_all(); - // Output any new data with [node] prefixes using synchronized writes + // Output any new data with optional [node] prefixes using synchronized writes for stream in manager.streams_mut() { let stdout = stream.take_stdout(); let stderr = stream.take_stderr(); @@ -659,7 +662,7 @@ impl ParallelExecutor { if !stdout.is_empty() { // Use lossy conversion to handle non-UTF8 data gracefully let text = String::from_utf8_lossy(&stdout); - let writer = NodeOutputWriter::new(&stream.node.host); + let writer = NodeOutputWriter::new_with_no_prefix(&stream.node.host, no_prefix); if let Err(e) = writer.write_stdout_lines(&text) { tracing::error!("Failed to write stdout for {}: {}", stream.node.host, e); } @@ -668,7 +671,7 @@ impl ParallelExecutor { if !stderr.is_empty() { // Use lossy conversion to handle non-UTF8 data gracefully let text = String::from_utf8_lossy(&stderr); - let writer = NodeOutputWriter::new(&stream.node.host); + let writer = NodeOutputWriter::new_with_no_prefix(&stream.node.host, no_prefix); if let Err(e) = writer.write_stderr_lines(&text) { tracing::error!("Failed to write stderr for {}: {}", stream.node.host, e); } @@ -817,6 +820,7 @@ impl ParallelExecutor { manager: &mut super::stream_manager::MultiNodeStreamManager, handles: Vec)>>, output_dir: &Path, + no_prefix: bool, ) -> Result> { use std::time::Duration; use tokio::fs; @@ -898,7 +902,10 @@ impl ParallelExecutor { match fs::write(&stdout_path, stream.stdout()).await { Ok(_) => { // Use synchronized output to prevent interleaving - let writer = super::output_sync::NodeOutputWriter::new(&stream.node.host); + let writer = super::output_sync::NodeOutputWriter::new_with_no_prefix( + &stream.node.host, + no_prefix, + ); if let Err(e) = writer .write_stdout(&format!("Output saved to {}", stdout_path.display())) { @@ -926,7 +933,10 @@ impl ParallelExecutor { match fs::write(&stderr_path, stream.stderr()).await { Ok(_) => { // Use synchronized output to prevent interleaving - let writer = super::output_sync::NodeOutputWriter::new(&stream.node.host); + let writer = super::output_sync::NodeOutputWriter::new_with_no_prefix( + &stream.node.host, + no_prefix, + ); if let Err(e) = writer .write_stdout(&format!("Errors saved to {}", stderr_path.display())) { From 2af67f1b338ff2e38dac04c53370ced727dded64 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Tue, 16 Dec 2025 23:53:22 +0900 Subject: [PATCH 2/2] docs: Add documentation and tests for --no-prefix option - Add CLI parsing tests for --no-prefix/-N option (10 test cases) - Update README.md with --no-prefix in Command-Line Options section - Update README.md Stream Mode section with --no-prefix usage example - Update manpage (bssh.1) with --no-prefix/-N option documentation Tests cover: - Long and short option parsing - Default value (false) - Combination with --stream mode - Combination with --output-dir mode - Combination with cluster option - Combination with other CLI options - OutputMode is_no_prefix() behavior for all modes --- README.md | 7 ++ docs/man/bssh.1 | 8 ++ tests/no_prefix_test.rs | 174 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 189 insertions(+) create mode 100644 tests/no_prefix_test.rs diff --git a/README.md b/README.md index 0199777b..dc2ecf96 100644 --- a/README.md +++ b/README.md @@ -290,6 +290,12 @@ bssh -C production --stream "tail -f /var/log/syslog" # [node1] Oct 30 10:15:23 systemd[1]: Started nginx.service # [node2] Oct 30 10:15:24 kernel: [UFW BLOCK] IN=eth0 OUT= # [node1] Oct 30 10:15:25 nginx: Configuration test successful + +# Stream mode without hostname prefix (pdsh -N compatibility) +bssh -C production --stream --no-prefix "uname -a" +# Output (no [node] prefixes): +# Linux node1 5.15.0-generic +# Linux node2 5.15.0-generic ``` #### File Mode (Save to Per-Node Files) @@ -930,6 +936,7 @@ Options: -p, --parallel Maximum parallel connections [default: 10] --timeout Command timeout in seconds (0 for unlimited) [default: 300] --output-dir Output directory for command results + -N, --no-prefix Disable hostname prefix in output (pdsh -N compatibility) -v, --verbose Increase verbosity (-v, -vv, -vvv) -h, --help Print help -V, --version Print version diff --git a/docs/man/bssh.1 b/docs/man/bssh.1 index 3fcc793e..67420286 100644 --- a/docs/man/bssh.1 +++ b/docs/man/bssh.1 @@ -193,6 +193,14 @@ monitoring long-running commands across multiple nodes. Automatically disabled when output is piped or in CI environments. This disables the interactive TUI mode. +.TP +.BR \-N ", " \-\-no\-prefix +Disable hostname prefix in output lines (pdsh -N compatibility). When +specified, output lines are displayed without the [hostname] prefix, +which is useful for programmatic parsing or cleaner display. Works +with both stream mode (--stream) and file mode (--output-dir). +Example: bssh -H host1,host2 --stream -N "uname -a" + .TP .BR \-v ", " \-\-verbose Increase verbosity (can be used multiple times: -v, -vv, -vvv) diff --git a/tests/no_prefix_test.rs b/tests/no_prefix_test.rs new file mode 100644 index 00000000..44c091c2 --- /dev/null +++ b/tests/no_prefix_test.rs @@ -0,0 +1,174 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for --no-prefix / -N option functionality + +use bssh::cli::Cli; +use bssh::executor::OutputMode; +use clap::Parser; +use std::path::PathBuf; + +/// Test CLI parsing with --no-prefix long option +#[test] +fn test_no_prefix_long_option() { + let args = vec!["bssh", "-H", "host1,host2", "--no-prefix", "uptime"]; + + let cli = Cli::parse_from(args); + + assert!(cli.no_prefix, "--no-prefix should set no_prefix to true"); + // Hosts are parsed as comma-separated values + assert_eq!( + cli.hosts, + Some(vec!["host1".to_string(), "host2".to_string()]) + ); +} + +/// Test CLI parsing with -N short option +#[test] +fn test_no_prefix_short_option() { + let args = vec!["bssh", "-H", "host1,host2", "-N", "uptime"]; + + let cli = Cli::parse_from(args); + + assert!(cli.no_prefix, "-N should set no_prefix to true"); +} + +/// Test CLI parsing without no_prefix option (default should be false) +#[test] +fn test_no_prefix_default_false() { + let args = vec!["bssh", "-H", "host1,host2", "uptime"]; + + let cli = Cli::parse_from(args); + + assert!(!cli.no_prefix, "no_prefix should be false by default"); +} + +/// Test --no-prefix with --stream mode +#[test] +fn test_no_prefix_with_stream_mode() { + let args = vec![ + "bssh", + "-H", + "host1,host2", + "--stream", + "--no-prefix", + "uptime", + ]; + + let cli = Cli::parse_from(args); + + assert!(cli.no_prefix, "--no-prefix should be set"); + assert!(cli.stream, "--stream should be set"); + + // OutputMode should respect both flags + let mode = OutputMode::from_args_with_no_prefix(cli.stream, None, cli.no_prefix); + assert!(mode.is_stream()); + assert!(mode.is_no_prefix()); +} + +/// Test --no-prefix with --output-dir mode +#[test] +fn test_no_prefix_with_output_dir() { + let args = vec![ + "bssh", + "-H", + "host1,host2", + "--output-dir", + "/tmp/output", + "-N", + "uptime", + ]; + + let cli = Cli::parse_from(args); + + assert!(cli.no_prefix, "-N should be set"); + assert_eq!(cli.output_dir, Some(PathBuf::from("/tmp/output"))); + + // OutputMode should respect both flags + let mode = + OutputMode::from_args_with_no_prefix(cli.stream, cli.output_dir.clone(), cli.no_prefix); + assert!(mode.is_file()); + assert!(mode.is_no_prefix()); +} + +/// Test --no-prefix with cluster option +#[test] +fn test_no_prefix_with_cluster() { + let args = vec!["bssh", "-C", "production", "--no-prefix", "df -h"]; + + let cli = Cli::parse_from(args); + + assert!(cli.no_prefix, "--no-prefix should be set"); + assert_eq!(cli.cluster, Some("production".to_string())); +} + +/// Test -N does not conflict with other short options +#[test] +fn test_no_prefix_with_other_options() { + let args = vec![ + "bssh", "-H", "host1", "-N", "-A", // use-agent + "-v", // verbose + "uptime", + ]; + + let cli = Cli::parse_from(args); + + assert!(cli.no_prefix, "-N should be set"); + assert!(cli.use_agent, "-A should be set"); + assert_eq!(cli.verbose, 1, "-v should increase verbosity"); +} + +/// Test OutputMode::is_no_prefix for Normal mode (should be false) +#[test] +fn test_output_mode_is_no_prefix_normal() { + let mode = OutputMode::Normal; + assert!( + !mode.is_no_prefix(), + "Normal mode should not report no_prefix as true" + ); +} + +/// Test OutputMode::is_no_prefix for Tui mode (should be false) +#[test] +fn test_output_mode_is_no_prefix_tui() { + let mode = OutputMode::Tui; + assert!( + !mode.is_no_prefix(), + "Tui mode should not report no_prefix as true" + ); +} + +/// Test OutputMode construction with explicit no_prefix +#[test] +fn test_output_mode_explicit_construction() { + // Stream mode with no_prefix enabled + let stream_with_prefix = OutputMode::Stream { no_prefix: false }; + assert!(!stream_with_prefix.is_no_prefix()); + + let stream_without_prefix = OutputMode::Stream { no_prefix: true }; + assert!(stream_without_prefix.is_no_prefix()); + + // File mode with no_prefix enabled + let file_with_prefix = OutputMode::File { + path: PathBuf::from("/tmp"), + no_prefix: false, + }; + assert!(!file_with_prefix.is_no_prefix()); + + let file_without_prefix = OutputMode::File { + path: PathBuf::from("/tmp"), + no_prefix: true, + }; + assert!(file_without_prefix.is_no_prefix()); +}