From c9ae4819f50cf46d557ee4b69df48fdf4ca17981 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 4 Jan 2025 22:49:22 -0500 Subject: [PATCH 1/3] tsort: use TsortError for errors in tsort --- src/uu/tsort/src/tsort.rs | 68 ++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 23 deletions(-) diff --git a/src/uu/tsort/src/tsort.rs b/src/uu/tsort/src/tsort.rs index 4a61f2710..4b7e6b130 100644 --- a/src/uu/tsort/src/tsort.rs +++ b/src/uu/tsort/src/tsort.rs @@ -8,10 +8,11 @@ use std::collections::{HashMap, HashSet, VecDeque}; use std::fmt::Write; use std::fs::File; use std::io::{stdin, BufReader, Read}; +use std::fmt::Display; use std::path::Path; use uucore::display::Quotable; -use uucore::error::{FromIo, UResult, USimpleError}; -use uucore::{format_usage, help_about, help_usage}; +use uucore::error::{UError, UResult}; +use uucore::{format_usage, help_about, help_usage, show}; const ABOUT: &str = help_about!("tsort.md"); const USAGE: &str = help_usage!("tsort.md"); @@ -20,6 +21,43 @@ mod options { pub const FILE: &str = "file"; } +#[derive(Debug)] +enum TsortError { + /// The input file is actually a directory. + IsDir(String), + + /// The number of tokens in the input data is odd. + /// + /// The list of edges must be even because each edge has two + /// components: a source node and a target node. + NumTokensOdd(String), + + /// The graph contains a cycle. + Loop(String), + + /// A particular node in a cycle. (This is mainly used for printing.) + LoopNode(String), +} + +impl std::error::Error for TsortError {} + +impl UError for TsortError {} + +impl Display for TsortError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + Self::IsDir(d) => write!(f, "{d}: read error: Is a directory"), + Self::NumTokensOdd(i) => write!( + f, + "{}: input contains an odd number of tokens", + i.maybe_quote() + ), + Self::Loop(i) => write!(f, "{i}: input contains a loop:"), + Self::LoopNode(v) => write!(f, "{v}"), + } + } +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().try_get_matches_from(args)?; @@ -36,10 +74,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } else { let path = Path::new(&input); if path.is_dir() { - return Err(USimpleError::new( - 1, - format!("{input}: read error: Is a directory"), - )); + return Err(TsortError::IsDir(input.to_string()).into()); } file_buf = File::open(path).map_err_context(|| input.to_string())?; &mut file_buf as &mut dyn Read @@ -57,32 +92,19 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { for ab in tokens.chunks(2) { match ab.len() { 2 => g.add_edge(ab[0], ab[1]), - _ => { - return Err(USimpleError::new( - 1, - format!( - "{}: input contains an odd number of tokens", - input.maybe_quote() - ), - )) - } + _ => return Err(TsortError::NumTokensOdd(input.to_string()).into()), } } } match g.run_tsort() { Err(cycle) => { - let mut error_message = format!( - "{}: {}: input contains a loop:\n", - uucore::util_name(), - input - ); + show!(TsortError::Loop(input.to_string())); for node in &cycle { - writeln!(error_message, "{}: {}", uucore::util_name(), node).unwrap(); + show!(TsortError::LoopNode(node.to_string())); } - eprint!("{}", error_message); println!("{}", cycle.join("\n")); - Err(USimpleError::new(1, "")) + Ok(()) } Ok(ordering) => { println!("{}", ordering.join("\n")); From 5cf5acb4a28e6b4c68426cefdbf34da74f31b7bf Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 4 Jan 2025 22:51:08 -0500 Subject: [PATCH 2/3] tsort: use std::io::read_to_string to load data --- src/uu/tsort/src/tsort.rs | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/uu/tsort/src/tsort.rs b/src/uu/tsort/src/tsort.rs index 4b7e6b130..98d3c6f78 100644 --- a/src/uu/tsort/src/tsort.rs +++ b/src/uu/tsort/src/tsort.rs @@ -5,9 +5,6 @@ //spell-checker:ignore TAOCP use clap::{crate_version, Arg, Command}; use std::collections::{HashMap, HashSet, VecDeque}; -use std::fmt::Write; -use std::fs::File; -use std::io::{stdin, BufReader, Read}; use std::fmt::Display; use std::path::Path; use uucore::display::Quotable; @@ -66,25 +63,20 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .get_one::(options::FILE) .expect("Value is required by clap"); - let mut stdin_buf; - let mut file_buf; - let mut reader = BufReader::new(if input == "-" { - stdin_buf = stdin(); - &mut stdin_buf as &mut dyn Read + let data = if input == "-" { + let stdin = std::io::stdin(); + std::io::read_to_string(stdin)? } else { let path = Path::new(&input); if path.is_dir() { return Err(TsortError::IsDir(input.to_string()).into()); } - file_buf = File::open(path).map_err_context(|| input.to_string())?; - &mut file_buf as &mut dyn Read - }); + std::fs::read_to_string(path)? + }; - let mut input_buffer = String::new(); - reader.read_to_string(&mut input_buffer)?; let mut g = Graph::default(); - for line in input_buffer.lines() { + for line in data.lines() { let tokens: Vec<_> = line.split_whitespace().collect(); if tokens.is_empty() { break; From f703e88e9f82dc5d5ecfca89eb2dc4cc66bdd9e0 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 4 Jan 2025 22:51:36 -0500 Subject: [PATCH 3/3] tsort: split edge data on any whitespace chars Make `tsort` split on any whitespace character instead of just whitespace within a single line. This allows edge information to be split with the source node on one line and the target node on another. For example, after this commit $ printf "a\nb\n" | tsort a b whereas before this would print an error message. Closes #7077 --- src/uu/tsort/src/tsort.rs | 16 +++++----------- tests/by-util/test_tsort.rs | 8 ++++++++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/uu/tsort/src/tsort.rs b/src/uu/tsort/src/tsort.rs index 98d3c6f78..ea5084a34 100644 --- a/src/uu/tsort/src/tsort.rs +++ b/src/uu/tsort/src/tsort.rs @@ -74,18 +74,12 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { std::fs::read_to_string(path)? }; + // Create the directed graph from pairs of tokens in the input data. let mut g = Graph::default(); - - for line in data.lines() { - let tokens: Vec<_> = line.split_whitespace().collect(); - if tokens.is_empty() { - break; - } - for ab in tokens.chunks(2) { - match ab.len() { - 2 => g.add_edge(ab[0], ab[1]), - _ => return Err(TsortError::NumTokensOdd(input.to_string()).into()), - } + for ab in data.split_whitespace().collect::>().chunks(2) { + match ab { + [a, b] => g.add_edge(a, b), + _ => return Err(TsortError::NumTokensOdd(input.to_string()).into()), } } diff --git a/tests/by-util/test_tsort.rs b/tests/by-util/test_tsort.rs index 49809e0df..f86add294 100644 --- a/tests/by-util/test_tsort.rs +++ b/tests/by-util/test_tsort.rs @@ -75,3 +75,11 @@ fn test_error_on_dir() { .fails() .stderr_contains("tsort: tsort_test_dir: read error: Is a directory"); } + +#[test] +fn test_split_on_any_whitespace() { + new_ucmd!() + .pipe_in("a\nb\n") + .succeeds() + .stdout_only("a\nb\n"); +}