diff --git a/AGENTS.md b/AGENTS.md index c8c8db2d5b..47bbc21c19 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,7 +22,7 @@ This guide applies to this repository; `CLAUDE.md` points here. For changes to t - Keep public test imports on `vite-plus/test*`, which wraps upstream `vitest` and `@vitest/browser*`. Do not recreate `packages/test` or `@voidzero-dev/vite-plus-test`. - Vite Task crates are git dependencies in `Cargo.toml`; there is no local `crates/vt`. Do not run `cargo test -p vt` here. - Use `vp_shared::VpDirs` for Vite+ directory roots. See `crates/vp_shared/src/dirs.rs` and `crates/vp_shared/src/dirs/resolution.rs`. Call sites must not construct category paths or read `VP_HOME` or `XDG_*` directly. -- Follow [`.clippy.toml`](.clippy.toml) for Rust restrictions and replacements. Use `crates/vp_shared/src/output.rs` for user-facing output and `crates/vp_shared/src/env_config.rs` for test-scoped environment configuration. See `crates/vp_command/src/lib.rs` for `vt_path` usage. +- Follow [`.clippy.toml`](.clippy.toml) for Rust restrictions and replacements. Use `crates/vp_shared/src/output.rs` for user-facing output; command output that can be piped must use `print_and_flush` or another broken-pipe-safe helper instead of `print!` or `println!`. Enable `clippy::print_stdout` in migrated modules so CI prevents direct stdout macros from returning. Use `crates/vp_shared/src/env_config.rs` for test-scoped environment configuration. See `crates/vp_command/src/lib.rs` for `vt_path` usage. - For TypeScript CLI output, use `packages/cli/src/utils/terminal.ts` and match the surrounding command style. - Keep changes scoped to the task and leave unrelated tracked and untracked files alone. Prefer source references over duplicated instructions in this guide. diff --git a/crates/vp_cli_snapshots/src/bin/vpt/head_lines.rs b/crates/vp_cli_snapshots/src/bin/vpt/head_lines.rs new file mode 100644 index 0000000000..4e4e6bf89a --- /dev/null +++ b/crates/vp_cli_snapshots/src/bin/vpt/head_lines.rs @@ -0,0 +1,51 @@ +use std::{ + io::{Read, Write}, + process::{Command, Stdio}, +}; + +/// head-lines `` -- `` \[``...\] +/// +/// Reads and prints the first `` lines from the child's stdout, closes +/// the pipe, then exits with the child's exit code. +pub fn run(args: &[String]) -> Result<(), Box> { + let sep = args + .iter() + .position(|arg| arg == "--") + .ok_or("Usage: vpt head-lines -- [args...]")?; + if sep != 1 || args.len() <= sep + 1 { + return Err("Usage: vpt head-lines -- [args...]".into()); + } + + let count = args[0].parse::()?; + if count == 0 { + return Err("head-lines count must be greater than zero".into()); + } + + let cmd_args = &args[sep + 1..]; + let mut child = + Command::new(&cmd_args[0]).args(&cmd_args[1..]).stdout(Stdio::piped()).spawn()?; + let mut child_stdout = child.stdout.take().unwrap(); + let mut captured = Vec::new(); + let mut lines = 0; + + while lines < count { + let mut byte = [0]; + if child_stdout.read(&mut byte)? == 0 { + break; + } + captured.push(byte[0]); + if byte[0] == b'\n' { + lines += 1; + } + } + + // Closing the read end reproduces a consumer such as `head` exiting early. + drop(child_stdout); + let status = child.wait()?; + + let mut stdout = std::io::stdout().lock(); + stdout.write_all(&captured)?; + stdout.flush()?; + + std::process::exit(crate::exit_code::exit_code_from_status(status)); +} diff --git a/crates/vp_cli_snapshots/src/bin/vpt/main.rs b/crates/vp_cli_snapshots/src/bin/vpt/main.rs index 84a0660285..9502b97b44 100644 --- a/crates/vp_cli_snapshots/src/bin/vpt/main.rs +++ b/crates/vp_cli_snapshots/src/bin/vpt/main.rs @@ -19,6 +19,7 @@ mod exit; mod exit_code; mod exit_on_ctrlc; mod grep_file; +mod head_lines; mod json_edit; mod list_dir; mod mkdir; @@ -64,7 +65,7 @@ fn main() { if args.len() < 2 { eprintln!("Usage: vpt [args...]"); eprintln!( - "Subcommands: backpressure-run (Unix), barrier, check-tty, chmod, cp, exit, exit-on-ctrlc, grep-file, json-edit, list-dir, mkdir, pipe-stdin, print, print-color, print-cwd, print-env, print-file, print-native-path, probe, read-stdin, replace-file-content, rm, stat-file, touch-file, write-file" + "Subcommands: backpressure-run (Unix), barrier, check-tty, chmod, cp, exit, exit-on-ctrlc, grep-file, head-lines, json-edit, list-dir, mkdir, pipe-stdin, print, print-color, print-cwd, print-env, print-file, print-native-path, probe, read-stdin, replace-file-content, rm, stat-file, touch-file, write-file" ); std::process::exit(1); } @@ -85,6 +86,7 @@ fn main() { "exit" => exit::run(&args[2..]), "exit-on-ctrlc" => exit_on_ctrlc::run(), "grep-file" => grep_file::run(&args[2..]), + "head-lines" => head_lines::run(&args[2..]), "json-edit" => json_edit::run(&args[2..]), "list-dir" => list_dir::run(&args[2..]), "mkdir" => mkdir::run(&args[2..]), diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md index b3fc7abf0a..a9d0506e69 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/README.md +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/README.md @@ -214,7 +214,9 @@ is identical on every platform: `test -f x && cmd` guards keep their short-circuit), `vpt write-file`, `vpt touch-file`, `vpt replace-file-content`, `vpt list-dir`, `vpt mkdir`, `vpt rm`, `vpt cp`, `vpt chmod`, `vpt grep-file`, `vpt json-edit`, -`vpt pipe-stdin -- `, plus task payloads for `vp run` tests: +`vpt pipe-stdin -- `, +`vpt head-lines -- ` (closes the child's stdout after the selected +lines), plus task payloads for `vp run` tests: `vpt print`, `vpt print-color`, `vpt print-env`, `vpt print-cwd`, `vpt print-native-path` (prints OS-native separators, for redaction self-tests), `vpt check-tty`, `vpt read-stdin`, `vpt exit `, diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_list_remote/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_list_remote/snapshots.toml index ecbef22d4e..9277c0d854 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_list_remote/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_list_remote/snapshots.toml @@ -28,3 +28,12 @@ steps = [ { argv = ["vp", "env", "list", "node"], comment = "Installed Node.js versions retain their interactive formatting", formatted-snapshot = true }, { argv = ["vp", "env", "list", "pnpm"], comment = "Installed package-manager versions use the same interactive formatting", formatted-snapshot = true }, ] + +[[case]] +name = "command_env_list_remote_closed_pipe" +vp = "global" +skip-platforms = ["windows"] +seed-runtime = false +steps = [ + { argv = ["vpt", "head-lines", "1", "--", "vp", "env", "list-remote", "--lts"], tty = false, comment = "Closing stdout after the heading does not make remote version listing fail" }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_list_remote/snapshots/command_env_list_remote_closed_pipe.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_list_remote/snapshots/command_env_list_remote_closed_pipe.md new file mode 100644 index 0000000000..1cced8a2c6 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_env_list_remote/snapshots/command_env_list_remote_closed_pipe.md @@ -0,0 +1,10 @@ +# command_env_list_remote_closed_pipe + +## `vpt head-lines 1 -- vp env list-remote --lts` + +Closing stdout after the heading does not make remote version listing fail + +``` +Node.js +note: Run `vp env clean` to free disk space from unused managed runtimes and package manager caches. +``` diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_version/snapshots.toml b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_version/snapshots.toml index f2f179124d..d2fc96416f 100644 --- a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_version/snapshots.toml +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_version/snapshots.toml @@ -5,3 +5,11 @@ skip-platforms = ["windows"] steps = [ { argv = ["vp", "--version"], continue-on-failure = true }, ] + +[[case]] +name = "command_version_closed_pipe" +vp = "global" +skip-platforms = ["windows"] +steps = [ + { argv = ["vpt", "head-lines", "1", "--", "vp", "--version"], tty = false, comment = "Closing stdout after the first line does not make the version command fail" }, +] diff --git a/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_version/snapshots/command_version_closed_pipe.md b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_version/snapshots/command_version_closed_pipe.md new file mode 100644 index 0000000000..b66f448193 --- /dev/null +++ b/crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/command_version/snapshots/command_version_closed_pipe.md @@ -0,0 +1,9 @@ +# command_version_closed_pipe + +## `vpt head-lines 1 -- vp --version` + +Closing stdout after the first line does not make the version command fail + +``` +vp +``` diff --git a/crates/vp_global_cli/src/commands/env/list_remote.rs b/crates/vp_global_cli/src/commands/env/list_remote.rs index a3ef2b0caf..bf86eae4b9 100644 --- a/crates/vp_global_cli/src/commands/env/list_remote.rs +++ b/crates/vp_global_cli/src/commands/env/list_remote.rs @@ -1,4 +1,6 @@ -use std::{collections::BTreeMap, process::ExitStatus}; +#![deny(clippy::print_stdout)] + +use std::{collections::BTreeMap, io, process::ExitStatus}; use console::style; use futures::future::try_join_all; @@ -17,6 +19,10 @@ use crate::{cli::SortingMethod, error::Error}; const DEFAULT_MAJOR_VERSIONS: usize = 10; +fn print_stdout(message: &str) { + vp_shared::output::print_and_flush(&mut io::stdout().lock(), message); +} + #[derive(Serialize)] struct RemoteEnvironmentJson { #[serde(skip_serializing_if = "Option::is_none")] @@ -152,24 +158,25 @@ pub async fn execute( }); if json { - println!( - "{}", - serde_json::to_string_pretty(&RemoteEnvironmentJson { node, package_managers })? - ); + let output = + serde_json::to_string_pretty(&RemoteEnvironmentJson { node, package_managers })?; + print_stdout(&format!("{output}\n")); return Ok(ExitStatus::default()); } if let Some(node) = node { - println!("Node.js"); + print_stdout("Node.js\n"); print_node_versions(&node); } if let Some(mut package_managers) = package_managers { for kind in package_manager::selected(scope) { let name = kind.to_string(); let versions = package_managers.remove(&name).unwrap_or_default(); - println!(); - println!("{}", package_manager::title(kind)); + print_stdout(&format!("\n{}\n", package_manager::title(kind))); for entry in versions { - println!(" {}", format_package_manager_version(&entry, use_color())); + print_stdout(&format!( + " {}\n", + format_package_manager_version(&entry, use_color()) + )); } } } @@ -184,7 +191,7 @@ fn print_node_versions(versions: &[NodeVersionJson]) { let colorize = use_color(); for entry in versions { - println!(" {}", format_node_version(entry, colorize)); + print_stdout(&format!(" {}\n", format_node_version(entry, colorize))); } } diff --git a/crates/vp_global_cli/src/commands/env/mod.rs b/crates/vp_global_cli/src/commands/env/mod.rs index 3f568cc799..f270482900 100644 --- a/crates/vp_global_cli/src/commands/env/mod.rs +++ b/crates/vp_global_cli/src/commands/env/mod.rs @@ -24,7 +24,7 @@ mod unpin; mod r#use; mod which; -use std::process::ExitStatus; +use std::{io, process::ExitStatus}; #[cfg(windows)] pub(crate) use setup::{cleanup_legacy_windows_shim, get_trampoline_path, remove_or_rename_to_old}; @@ -40,8 +40,9 @@ fn print_env_header() { vp_shared::header::print_header(); } +#[deny(clippy::print_stdout)] fn print_env_clean_tip() { - vp_shared::output::raw(""); + vp_shared::output::print_and_flush(&mut io::stdout().lock(), "\n"); vp_shared::output::note( "Run `vp env clean` to free disk space from unused managed runtimes and package manager caches.", ); diff --git a/crates/vp_global_cli/src/commands/version.rs b/crates/vp_global_cli/src/commands/version.rs index 26cff89b7e..c35976392e 100644 --- a/crates/vp_global_cli/src/commands/version.rs +++ b/crates/vp_global_cli/src/commands/version.rs @@ -1,8 +1,10 @@ //! Version command. +#![deny(clippy::print_stdout)] + use std::{ collections::BTreeMap, - fs, + fs, io, path::{Path, PathBuf}, process::ExitStatus, }; @@ -121,12 +123,16 @@ fn resolve_tool_version( } } +fn print_stdout(message: &str) { + vp_shared::output::print_and_flush(&mut io::stdout().lock(), message); +} + fn print_rows(title: &str, rows: &[(&str, String)]) { - println!("{}", help::render_heading(title)); + print_stdout(&format!("{}\n", help::render_heading(title))); let label_width = rows.iter().map(|(label, _)| label.chars().count()).max().unwrap_or(0); for (label, value) in rows { let padding = " ".repeat(label_width.saturating_sub(label.chars().count())); - println!(" {}{} {value}", help::accent(label), padding); + print_stdout(&format!(" {}{} {value}\n", help::accent(label), padding)); } } @@ -169,8 +175,7 @@ fn detect_system_node_version() -> Option { pub async fn execute(cwd: AbsolutePathBuf) -> Result { vp_shared::header::print_header(); - println!("vp v{}", env!("CARGO_PKG_VERSION")); - println!(); + print_stdout(concat!("vp v", env!("CARGO_PKG_VERSION"), "\n\n")); // Local vite-plus and tools let local = find_local_vite_plus(cwd.as_path()); @@ -178,7 +183,7 @@ pub async fn execute(cwd: AbsolutePathBuf) -> Result { "Local vite-plus", &[("vite-plus", format_version(local.as_ref().map(|pkg| pkg.version.clone())))], ); - println!(); + print_stdout("\n"); let manifest = local.as_ref().and_then(read_toolchain_manifest); let tool_rows = TOOL_SPECS @@ -189,7 +194,7 @@ pub async fn execute(cwd: AbsolutePathBuf) -> Result { }) .collect::>(); print_rows("Tools", &tool_rows); - println!(); + print_stdout("\n"); // Environment info let package_manager_info = find_workspace_root(&cwd) diff --git a/crates/vp_shared/src/output.rs b/crates/vp_shared/src/output.rs index 30cf4c6a0d..3154f52d5b 100644 --- a/crates/vp_shared/src/output.rs +++ b/crates/vp_shared/src/output.rs @@ -4,10 +4,44 @@ //! consistent output across the entire CLI. Styling uses console's color detection //! for the stream receiving each message. -use std::sync::atomic::{AtomicBool, Ordering}; +use std::{ + io::{self, Write}, + sync::atomic::{AtomicBool, Ordering}, +}; use console::style; +/// Write a message and flush it without panicking for expected output-stream errors. +/// +/// Use this for command output that can be piped to a reader which exits early. +pub fn print_and_flush(writer: &mut dyn Write, message: &str) { + let mut remaining = message.as_bytes(); + while !remaining.is_empty() { + match writer.write(remaining) { + Ok(0) => fail_for_writer_error(io::ErrorKind::WriteZero.into()), + Ok(written) => remaining = &remaining[written..], + Err(error) if error.kind() == io::ErrorKind::BrokenPipe => return, + Err(error) if error.kind() == io::ErrorKind::Interrupted => {} + Err(error) if error.kind() == io::ErrorKind::WouldBlock => std::thread::yield_now(), + Err(error) => fail_for_writer_error(error), + } + } + + loop { + match writer.flush() { + Ok(()) => return, + Err(error) if error.kind() == io::ErrorKind::BrokenPipe => return, + Err(error) if error.kind() == io::ErrorKind::Interrupted => {} + Err(error) if error.kind() == io::ErrorKind::WouldBlock => std::thread::yield_now(), + Err(error) => fail_for_writer_error(error), + } + } +} + +fn fail_for_writer_error(error: io::Error) -> ! { + panic!("failed writing command output: {error}"); +} + /// When set, user-facing stdout output (info/pass/note/success/raw) is routed /// to stderr instead. Shim dispatch enables this once at entry: a shim's /// stdout belongs to the wrapped tool and must stay parseable. @@ -113,3 +147,58 @@ pub fn raw_inline(msg: &str) { pub fn raw_stderr(msg: &str) { eprintln!("{msg}"); } + +#[cfg(test)] +mod tests { + use super::*; + + #[derive(Default)] + struct RetryWriter { + output: Vec, + write_calls: usize, + flush_calls: usize, + } + + impl Write for RetryWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.write_calls += 1; + match self.write_calls { + 1 => { + let written = buf.len().min(2); + self.output.extend_from_slice(&buf[..written]); + Ok(written) + } + 2 => Err(io::ErrorKind::WouldBlock.into()), + _ => { + self.output.extend_from_slice(buf); + Ok(buf.len()) + } + } + } + + fn flush(&mut self) -> io::Result<()> { + self.flush_calls += 1; + match self.flush_calls { + 1 => Err(io::ErrorKind::Interrupted.into()), + 2 => Err(io::ErrorKind::WouldBlock.into()), + _ => Ok(()), + } + } + } + + #[test] + fn print_and_flush_retries_temporary_errors_without_losing_output() { + let mut writer = RetryWriter::default(); + print_and_flush(&mut writer, "output\n"); + assert_eq!(writer.output, b"output\n"); + assert_eq!(writer.flush_calls, 3); + } + + #[cfg(unix)] + #[test] + fn print_and_flush_tolerates_a_closed_pipe() { + let (reader, writer) = nix::unistd::pipe().unwrap(); + drop(reader); + print_and_flush(&mut std::fs::File::from(writer), "output\n"); + } +}