From 75b3cbcfd97ba45df9bd41d6da0d8c7ef34312eb Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Wed, 23 Feb 2022 21:56:24 +0100 Subject: [PATCH 1/7] pr: move from getopts to clap fixes --- Cargo.lock | 10 - src/uu/pr/Cargo.toml | 1 - src/uu/pr/src/pr.rs | 631 ++++++++++++++++++++----------------------- 3 files changed, 298 insertions(+), 344 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fd064dc76..805c1c83a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -890,15 +890,6 @@ dependencies = [ "version_check", ] -[[package]] -name = "getopts" -version = "0.2.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "14dbbfd5c71d70241ecf9e6f13737f7b5ce823821063188d7e46c41d371eebd5" -dependencies = [ - "unicode-width", -] - [[package]] name = "getrandom" version = "0.2.4" @@ -2905,7 +2896,6 @@ version = "0.0.12" dependencies = [ "chrono", "clap 3.0.10", - "getopts", "itertools", "quick-error", "regex", diff --git a/src/uu/pr/Cargo.toml b/src/uu/pr/Cargo.toml index a1fdae4a2..8e9029daa 100644 --- a/src/uu/pr/Cargo.toml +++ b/src/uu/pr/Cargo.toml @@ -17,7 +17,6 @@ path = "src/pr.rs" [dependencies] clap = { version = "3.0", features = ["wrap_help", "cargo"] } uucore = { version=">=0.0.7", package="uucore", path="../../uucore", features=["entries"] } -getopts = "0.2.21" chrono = "0.4.19" quick-error = "2.0.1" itertools = "0.10.0" diff --git a/src/uu/pr/src/pr.rs b/src/uu/pr/src/pr.rs index c167770c0..8ba3872f5 100644 --- a/src/uu/pr/src/pr.rs +++ b/src/uu/pr/src/pr.rs @@ -1,5 +1,3 @@ -#![crate_name = "uu_pr"] - // This file is part of the uutils coreutils package. // // For the full copyright and license information, please view the LICENSE file @@ -13,9 +11,7 @@ extern crate quick_error; use chrono::offset::Local; use chrono::DateTime; -use clap::{App, AppSettings}; -use getopts::Matches; -use getopts::{HasArg, Occur}; +use clap::{App, AppSettings, Arg, ArgMatches}; use itertools::Itertools; use quick_error::ResultExt; use regex::Regex; @@ -26,14 +22,35 @@ use std::io::{stdin, stdout, BufRead, BufReader, Lines, Read, Write}; use std::os::unix::fs::FileTypeExt; use uucore::display::Quotable; -use uucore::error::UResult; +use uucore::error::{set_exit_code, UResult}; -type IOError = std::io::Error; - -const NAME: &str = "pr"; const VERSION: &str = env!("CARGO_PKG_VERSION"); const ABOUT: &str = "Write content of given file or standard input to standard output with pagination filter"; +const AFTER_HELP: &str = + " +PAGE\n Begin output at page number page of the formatted input. + -COLUMN\n Produce multi-column output. See --column + +The pr utility is a printing and pagination filter +for text files. When multiple input files are specified, +each is read, formatted, and written to standard +output. By default, the input is separated +into 66-line pages, each with + +o A 5-line header with the page number, date, + time, and the pathname of the file. + +o A 5-line trailer consisting of blank lines. + +If standard output is associated with a terminal, +diagnostic messages are suppressed until the pr +utility has completed processing. + +When multiple column output is specified, text columns +are of equal width. By default text columns +are separated by at least one . Input lines +that do not fit into a text column are truncated. +Lines are not truncated under single column output."; const TAB: char = '\t'; const LINES_PER_PAGE: usize = 66; const LINES_PER_PAGE_FOR_FORM_FEED: usize = 63; @@ -47,25 +64,27 @@ const DEFAULT_COLUMN_SEPARATOR: &char = &TAB; const FF: u8 = 0x0C_u8; mod options { - pub const STRING_HEADER_OPTION: &str = "h"; - pub const DOUBLE_SPACE_OPTION: &str = "d"; - pub const NUMBERING_MODE_OPTION: &str = "n"; - pub const FIRST_LINE_NUMBER_OPTION: &str = "N"; - pub const PAGE_RANGE_OPTION: &str = "pages"; - pub const NO_HEADER_TRAILER_OPTION: &str = "t"; - pub const PAGE_LENGTH_OPTION: &str = "l"; - pub const SUPPRESS_PRINTING_ERROR: &str = "r"; - pub const FORM_FEED_OPTION: &str = "F"; - pub const FORM_FEED_OPTION_SMALL: &str = "f"; - pub const COLUMN_WIDTH_OPTION: &str = "w"; - pub const PAGE_WIDTH_OPTION: &str = "W"; - pub const ACROSS_OPTION: &str = "a"; - pub const COLUMN_OPTION: &str = "column"; - pub const COLUMN_CHAR_SEPARATOR_OPTION: &str = "s"; - pub const COLUMN_STRING_SEPARATOR_OPTION: &str = "S"; - pub const MERGE_FILES_PRINT: &str = "m"; - pub const OFFSET_SPACES_OPTION: &str = "o"; - pub const JOIN_LINES_OPTION: &str = "J"; + pub const HEADER: &str = "header"; + pub const DOUBLE_SPACE: &str = "double-space"; + pub const NUMBER_LINES: &str = "number-lines"; + pub const FIRST_LINE_NUMBER: &str = "first-line-number"; + pub const PAGES: &str = "pages"; + pub const OMIT_HEADER: &str = "omit-header"; + pub const PAGE_LENGTH: &str = "length"; + pub const NO_FILE_WARNINGS: &str = "no-file-warnings"; + pub const FORM_FEED: &str = "form-feed"; + pub const COLUMN_WIDTH: &str = "width"; + pub const PAGE_WIDTH: &str = "page-width"; + pub const ACROSS: &str = "across"; + pub const COLUMN: &str = "column"; + pub const COLUMN_CHAR_SEPARATOR: &str = "separator"; + pub const COLUMN_STRING_SEPARATOR: &str = "sep-string"; + pub const MERGE: &str = "merge"; + pub const INDENT: &str = "indent"; + pub const JOIN_LINES: &str = "join-lines"; + pub const HELP: &str = "help"; + pub const VERSION: &str = "version"; + pub const FILES: &str = "files"; } struct OutputOptions { @@ -95,7 +114,7 @@ struct FileLine { line_number: usize, page_number: usize, group_key: usize, - line_content: Result, + line_content: Result, form_feeds_after: usize, } @@ -136,8 +155,8 @@ impl Default for FileLine { } } -impl From for PrError { - fn from(err: IOError) -> Self { +impl From for PrError { + fn from(err: std::io::Error) -> Self { Self::EncounteredErrors(err.to_string()) } } @@ -145,8 +164,8 @@ impl From for PrError { quick_error! { #[derive(Debug)] enum PrError { - Input(err: IOError, path: String) { - context(path: &'a str, err: IOError) -> (err, path.to_owned()) + Input(err: std::io::Error, path: String) { + context(path: &'a str, err: std::io::Error) -> (err, path.to_owned()) display("pr: Reading from input {0} gave error", path) source(err) } @@ -177,7 +196,182 @@ pub fn uu_app<'a>() -> App<'a> { App::new(uucore::util_name()) .version(VERSION) .about(ABOUT) + .after_help(AFTER_HELP) .setting(AppSettings::InferLongArgs) + .setting(AppSettings::AllArgsOverrideSelf) + .setting(AppSettings::NoAutoHelp) + .setting(AppSettings::NoAutoVersion) + .arg( + Arg::new(options::PAGES) + .long(options::PAGES) + .help("Begin and stop printing with page FIRST_PAGE[:LAST_PAGE]") + .takes_value(true) + .value_name("FIRST_PAGE[:LAST_PAGE]"), + ) + .arg( + Arg::new(options::HEADER) + .short('h') + .long(options::HEADER) + .help( + "Use the string header to replace the file name \ + in the header line.", + ) + .takes_value(true) + .value_name("STRING"), + ) + .arg( + Arg::new(options::DOUBLE_SPACE) + .short('d') + .long(options::DOUBLE_SPACE) + .help("Produce output that is double spaced. An extra character is output following every + found in the input.") + ) + .arg( + Arg::new(options::NUMBER_LINES) + .short('n') + .long(options::NUMBER_LINES) + .help("Provide width digit line numbering. The default for width, if not specified, is 5. The number occupies + the first width column positions of each text column or each line of -m output. If char (any non-digit + character) is given, it is appended to the line number to separate it from whatever follows. The default + for char is a . Line numbers longer than width columns are truncated.") + .takes_value(true) + .value_name("[char][width]") + ) + .arg( + Arg::new(options::FIRST_LINE_NUMBER) + .short('N') + .long(options::FIRST_LINE_NUMBER) + .help("start counting with NUMBER at 1st line of first page printed") + .takes_value(true) + .value_name("NUMBER") + ) + .arg( + Arg::new(options::OMIT_HEADER) + .short('t') + .long(options::OMIT_HEADER) + .help("Write neither the five-line identifying header nor the five-line trailer usually supplied for each page. Quit + writing after the last line of each file without spacing to the end of the page.") + ) + .arg( + Arg::new(options::PAGE_LENGTH) + .short('l') + .long(options::PAGE_LENGTH) + .help("Override the 66-line default (default number of lines of text 56, and with -F 63) and reset the page length to lines. If lines is not greater than the sum of both + the header and trailer depths (in lines), the pr utility shall suppress both the header and trailer, as if the + -t option were in effect. ") + .takes_value(true) + .value_name("PAGE_LENGTH") + ) + .arg( + Arg::new(options::NO_FILE_WARNINGS) + .short('r') + .long(options::NO_FILE_WARNINGS) + .help("omit warning when a file cannot be opened") + ) + .arg( + Arg::new(options::FORM_FEED) + .short('F') + .short_alias('f') + .long(options::FORM_FEED) + .help("Use a for new pages, instead of the default behavior that uses a sequence of s.") + ) + .arg( + Arg::new(options::COLUMN_WIDTH) + .short('w') + .long(options::COLUMN_WIDTH) + .help("Set the width of the line to width column positions for multiple text-column output only. If the -w option is + not specified and the -s option is not specified, the default width shall be 72. If the -w option is not specified + and the -s option is specified, the default width shall be 512.") + .takes_value(true) + .value_name("width") + ) + .arg( + Arg::new(options::PAGE_WIDTH) + .short('W') + .long(options::PAGE_WIDTH) + .help("set page width to PAGE_WIDTH (72) characters always, + truncate lines, except -J option is set, no interference + with -S or -s") + .takes_value(true) + .value_name("width") + ) + .arg( + Arg::new(options::ACROSS) + .short('a') + .long(options::ACROSS) + .help("Modify the effect of the - column option so that the columns are filled across the page in a round-robin order + (for example, when column is 2, the first input line heads column 1, the second heads column 2, the third is the + second line in column 1, and so on).") + ) + .arg( + Arg::new(options::COLUMN) + .long(options::COLUMN) + .help("Produce multi-column output that is arranged in column columns (the default shall be 1) and is written down each + column in the order in which the text is received from the input file. This option should not be used with -m. + The options -e and -i shall be assumed for multiple text-column output. Whether or not text columns are produced + with identical vertical lengths is unspecified, but a text column shall never exceed the length of the + page (see the -l option). When used with -t, use the minimum number of lines to write the output.") + .takes_value(true) + .value_name("column") + ) + .arg( + Arg::new(options::COLUMN_CHAR_SEPARATOR) + .short('s') + .long(options::COLUMN_CHAR_SEPARATOR) + .help("Separate text columns by the single character char instead of by the appropriate number of s + (default for char is the character).") + .takes_value(true) + .value_name("char") + ) + .arg( + Arg::new(options::COLUMN_STRING_SEPARATOR) + .short('S') + .long(options::COLUMN_STRING_SEPARATOR) + .help("separate columns by STRING, + without -S: Default separator with -J and + otherwise (same as -S\" \"), no effect on column options") + .takes_value(true) + .value_name("string") + ) + .arg( + Arg::new(options::MERGE) + .short('m') + .long(options::MERGE) + .help("Merge files. Standard output shall be formatted so the pr utility writes one line from each file specified by a + file operand, side by side into text columns of equal fixed widths, in terms of the number of column positions. + Implementations shall support merging of at least nine file operands.") + ) + .arg( + Arg::new(options::INDENT) + .short('o') + .long(options::INDENT) + .help("Each line of output shall be preceded by offset s. If the -o option is not specified, the default offset + shall be zero. The space taken is in addition to the output line width (see the -w option below).") + .takes_value(true) + .value_name("margin") + ) + .arg( + Arg::new(options::JOIN_LINES) + .short('J') + .help("merge full lines, turns off -W line truncation, no column + alignment, --sep-string[=STRING] sets separators") + ) + .arg( + Arg::new(options::HELP) + .long(options::HELP) + .help("Show this help message") + ) + .arg( + Arg::new(options::VERSION) + .short('V') + .long(options::VERSION) + .help("Show version information") + ) + .arg( + Arg::new(options::FILES) + .multiple_occurrences(true) + .multiple_values(true) + ) } #[uucore::main] @@ -185,229 +379,39 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args = args .collect_str(uucore::InvalidEncodingHandling::Ignore) .accept_any(); - let mut opts = getopts::Options::new(); - - opts.opt( - "", - options::PAGE_RANGE_OPTION, - "Begin and stop printing with page FIRST_PAGE[:LAST_PAGE]", - "FIRST_PAGE[:LAST_PAGE]", - HasArg::Yes, - Occur::Optional, - ); - - opts.opt( - options::STRING_HEADER_OPTION, - "header", - "Use the string header to replace the file name \ - in the header line.", - "STRING", - HasArg::Yes, - Occur::Optional, - ); - - opts.opt( - options::DOUBLE_SPACE_OPTION, - "double-space", - "Produce output that is double spaced. An extra character is output following every - found in the input.", - "", - HasArg::No, - Occur::Optional, - ); - - opts.opt( - options::NUMBERING_MODE_OPTION, - "number-lines", - "Provide width digit line numbering. The default for width, if not specified, is 5. The number occupies - the first width column positions of each text column or each line of -m output. If char (any non-digit - character) is given, it is appended to the line number to separate it from whatever follows. The default - for char is a . Line numbers longer than width columns are truncated.", - "[char][width]", - HasArg::Maybe, - Occur::Optional, - ); - - opts.opt( - options::FIRST_LINE_NUMBER_OPTION, - "first-line-number", - "start counting with NUMBER at 1st line of first page printed", - "NUMBER", - HasArg::Yes, - Occur::Optional, - ); - - opts.opt( - options::NO_HEADER_TRAILER_OPTION, - "omit-header", - "Write neither the five-line identifying header nor the five-line trailer usually supplied for each page. Quit - writing after the last line of each file without spacing to the end of the page.", - "", - HasArg::No, - Occur::Optional, - ); - - opts.opt( - options::PAGE_LENGTH_OPTION, - "length", - "Override the 66-line default (default number of lines of text 56, and with -F 63) and reset the page length to lines. If lines is not greater than the sum of both - the header and trailer depths (in lines), the pr utility shall suppress both the header and trailer, as if the - -t option were in effect. ", - "lines", - HasArg::Yes, - Occur::Optional, - ); - - opts.opt( - options::SUPPRESS_PRINTING_ERROR, - "no-file-warnings", - "omit warning when a file cannot be opened", - "", - HasArg::No, - Occur::Optional, - ); - - opts.opt( - options::FORM_FEED_OPTION, - "form-feed", - "Use a for new pages, instead of the default behavior that uses a sequence of s.", - "", - HasArg::No, - Occur::Optional, - ); - opts.opt( - options::FORM_FEED_OPTION_SMALL, - "form-feed", - "Same as -F but pause before beginning the first page if standard output is a - terminal.", - "", - HasArg::No, - Occur::Optional, - ); - - opts.opt( - "", - options::COLUMN_OPTION, - "Produce multi-column output that is arranged in column columns (the default shall be 1) and is written down each - column in the order in which the text is received from the input file. This option should not be used with -m. - The options -e and -i shall be assumed for multiple text-column output. Whether or not text columns are produced - with identical vertical lengths is unspecified, but a text column shall never exceed the length of the - page (see the -l option). When used with -t, use the minimum number of lines to write the output.", - "[column]", - HasArg::Yes, - Occur::Optional, - ); - - opts.opt( - options::COLUMN_WIDTH_OPTION, - "width", - "Set the width of the line to width column positions for multiple text-column output only. If the -w option is - not specified and the -s option is not specified, the default width shall be 72. If the -w option is not specified - and the -s option is specified, the default width shall be 512.", - "[width]", - HasArg::Yes, - Occur::Optional, - ); - - opts.opt( - options::PAGE_WIDTH_OPTION, - "page-width", - "set page width to PAGE_WIDTH (72) characters always, - truncate lines, except -J option is set, no interference - with -S or -s", - "[width]", - HasArg::Yes, - Occur::Optional, - ); - - opts.opt( - options::ACROSS_OPTION, - "across", - "Modify the effect of the - column option so that the columns are filled across the page in a round-robin order - (for example, when column is 2, the first input line heads column 1, the second heads column 2, the third is the - second line in column 1, and so on).", - "", - HasArg::No, - Occur::Optional, - ); - - opts.opt( - options::COLUMN_CHAR_SEPARATOR_OPTION, - "separator", - "Separate text columns by the single character char instead of by the appropriate number of s - (default for char is the character).", - "char", - HasArg::Yes, - Occur::Optional, - ); - - opts.opt( - options::COLUMN_STRING_SEPARATOR_OPTION, - "sep-string", - "separate columns by STRING, - without -S: Default separator with -J and - otherwise (same as -S\" \"), no effect on column options", - "string", - HasArg::Yes, - Occur::Optional, - ); - - opts.opt( - options::MERGE_FILES_PRINT, - "merge", - "Merge files. Standard output shall be formatted so the pr utility writes one line from each file specified by a - file operand, side by side into text columns of equal fixed widths, in terms of the number of column positions. - Implementations shall support merging of at least nine file operands.", - "", - HasArg::No, - Occur::Optional, - ); - - opts.opt( - options::OFFSET_SPACES_OPTION, - "indent", - "Each line of output shall be preceded by offset s. If the -o option is not specified, the default offset - shall be zero. The space taken is in addition to the output line width (see the -w option below).", - "offset", - HasArg::Yes, - Occur::Optional, - ); - - opts.opt( - options::JOIN_LINES_OPTION, - "join-lines", - "merge full lines, turns off -W line truncation, no column - alignment, --sep-string[=STRING] sets separators", - "offset", - HasArg::No, - Occur::Optional, - ); - - opts.optflag("", "help", "display this help and exit"); - opts.optflag("V", "version", "output version information and exit"); let opt_args = recreate_arguments(&args); - let matches = match opts.parse(&opt_args[1..]) { + let mut app = uu_app(); + let matches = match app.try_get_matches_from_mut(opt_args) { Ok(m) => m, - Err(e) => panic!("Invalid options\n{}", e), + Err(e) => { + e.print()?; + set_exit_code(1); + return Ok(()); + } }; - if matches.opt_present("version") { - println!("{} {}", NAME, VERSION); + if matches.is_present(options::VERSION) { + println!("{}", app.render_long_version()); return Ok(()); } - let mut files = matches.free.clone(); + if matches.is_present(options::HELP) { + app.print_long_help()?; + return Ok(()); + } + + let mut files = matches + .values_of(options::FILES) + .map(|v| v.collect::>()) + .unwrap_or_default() + .clone(); if files.is_empty() { - files.insert(0, FILE_STDIN.to_owned()); + files.insert(0, FILE_STDIN); } - if matches.opt_present("help") { - return print_usage(&mut opts, &matches); - } - - let file_groups: Vec<_> = if matches.opt_present(options::MERGE_FILES_PRINT) { + let file_groups: Vec<_> = if matches.is_present(options::MERGE) { vec![files] } else { files.into_iter().map(|i| vec![i]).collect() @@ -470,56 +474,13 @@ fn recreate_arguments(args: &[String]) -> Vec { .collect() } -fn print_error(matches: &Matches, err: &PrError) { - if !matches.opt_present(options::SUPPRESS_PRINTING_ERROR) { +fn print_error(matches: &ArgMatches, err: &PrError) { + if !matches.is_present(options::NO_FILE_WARNINGS) { eprintln!("{}", err); } } -fn print_usage(opts: &mut getopts::Options, matches: &Matches) -> UResult<()> { - println!("{} {} -- print files", NAME, VERSION); - println!(); - println!( - "Usage: {} [+page] [-column] [-adFfmprt] [[-e] [char] [gap]] - [-L locale] [-h header] [[-i] [char] [gap]] - [-l lines] [-o offset] [[-s] [char]] [[-n] [char] - [width]] [-w width] [-] [file ...].", - NAME - ); - println!(); - let usage: &str = "The pr utility is a printing and pagination filter - for text files. When multiple input files are specified, - each is read, formatted, and written to standard - output. By default, the input is separated - into 66-line pages, each with - - o A 5-line header with the page number, date, - time, and the pathname of the file. - - o A 5-line trailer consisting of blank lines. - - If standard output is associated with a terminal, - diagnostic messages are suppressed until the pr - utility has completed processing. - - When multiple column output is specified, text columns - are of equal width. By default text columns - are separated by at least one . Input lines - that do not fit into a text column are truncated. - Lines are not truncated under single column output."; - println!("{}", opts.usage(usage)); - println!(" +page \t\tBegin output at page number page of the formatted input."); - println!( - " -column \t\tProduce multi-column output. Refer --{}", - options::COLUMN_OPTION - ); - if matches.free.is_empty() { - return Err(1.into()); - } - Ok(()) -} - -fn parse_usize(matches: &Matches, opt: &str) -> Option> { +fn parse_usize(matches: &ArgMatches, opt: &str) -> Option> { let from_parse_error_to_pr_error = |value_to_parse: (String, String)| { let i = value_to_parse.0; let option = value_to_parse.1; @@ -528,51 +489,51 @@ fn parse_usize(matches: &Matches, opt: &str) -> Option> { }) }; matches - .opt_str(opt) - .map(|i| (i, format!("-{}", opt))) + .value_of(opt) + .map(|i| (i.to_string(), format!("-{}", opt))) .map(from_parse_error_to_pr_error) } fn build_options( - matches: &Matches, - paths: &[String], + matches: &ArgMatches, + paths: &[&str], free_args: &str, ) -> Result { - let form_feed_used = matches.opt_present(options::FORM_FEED_OPTION) - || matches.opt_present(options::FORM_FEED_OPTION_SMALL); + let form_feed_used = matches.is_present(options::FORM_FEED); - let is_merge_mode = matches.opt_present(options::MERGE_FILES_PRINT); + let is_merge_mode = matches.is_present(options::MERGE); - if is_merge_mode && matches.opt_present(options::COLUMN_OPTION) { + if is_merge_mode && matches.is_present(options::COLUMN) { let err_msg = String::from("cannot specify number of columns when printing in parallel"); return Err(PrError::EncounteredErrors(err_msg)); } - if is_merge_mode && matches.opt_present(options::ACROSS_OPTION) { + if is_merge_mode && matches.is_present(options::ACROSS) { let err_msg = String::from("cannot specify both printing across and printing in parallel"); return Err(PrError::EncounteredErrors(err_msg)); } - let merge_files_print = if matches.opt_present(options::MERGE_FILES_PRINT) { + let merge_files_print = if matches.is_present(options::MERGE) { Some(paths.len()) } else { None }; - let header = matches.opt_str(options::STRING_HEADER_OPTION).unwrap_or( - if is_merge_mode || paths[0] == FILE_STDIN { - String::new() + let header = matches + .value_of(options::HEADER) + .unwrap_or(if is_merge_mode || paths[0] == FILE_STDIN { + "" } else { - paths[0].to_string() - }, - ); + paths[0] + }) + .to_string(); let default_first_number = NumberingMode::default().first_number; - let first_number = parse_usize(matches, options::FIRST_LINE_NUMBER_OPTION) - .unwrap_or(Ok(default_first_number))?; + let first_number = + parse_usize(matches, options::FIRST_LINE_NUMBER).unwrap_or(Ok(default_first_number))?; let number = matches - .opt_str(options::NUMBERING_MODE_OPTION) + .value_of(options::NUMBER_LINES) .map(|i| { let parse_result = i.parse::(); @@ -596,14 +557,14 @@ fn build_options( } }) .or_else(|| { - if matches.opt_present(options::NUMBERING_MODE_OPTION) { + if matches.is_present(options::NUMBER_LINES) { Some(NumberingMode::default()) } else { None } }); - let double_space = matches.opt_present(options::DOUBLE_SPACE_OPTION); + let double_space = matches.is_present(options::DOUBLE_SPACE); let content_line_separator = if double_space { "\n".repeat(2) @@ -652,7 +613,7 @@ fn build_options( }; let invalid_pages_map = |i: String| { - let unparsed_value = matches.opt_str(options::PAGE_RANGE_OPTION).unwrap(); + let unparsed_value = matches.value_of(options::PAGES).unwrap(); i.parse::().map_err(|_e| { PrError::EncounteredErrors(format!( "invalid --pages argument {}", @@ -662,7 +623,7 @@ fn build_options( }; let start_page = match matches - .opt_str(options::PAGE_RANGE_OPTION) + .value_of(options::PAGES) .map(|i| { let x: Vec<_> = i.split(':').collect(); x[0].to_string() @@ -674,7 +635,7 @@ fn build_options( }; let end_page = match matches - .opt_str(options::PAGE_RANGE_OPTION) + .value_of(options::PAGES) .filter(|i| i.contains(':')) .map(|i| { let x: Vec<_> = i.split(':').collect(); @@ -702,12 +663,12 @@ fn build_options( }; let page_length = - parse_usize(matches, options::PAGE_LENGTH_OPTION).unwrap_or(Ok(default_lines_per_page))?; + parse_usize(matches, options::PAGE_LENGTH).unwrap_or(Ok(default_lines_per_page))?; let page_length_le_ht = page_length < (HEADER_LINES_PER_PAGE + TRAILER_LINES_PER_PAGE); let display_header_and_trailer = - !(page_length_le_ht) && !matches.opt_present(options::NO_HEADER_TRAILER_OPTION); + !(page_length_le_ht) && !matches.is_present(options::OMIT_HEADER); let content_lines_per_page = if page_length_le_ht { page_length @@ -715,23 +676,24 @@ fn build_options( page_length - (HEADER_LINES_PER_PAGE + TRAILER_LINES_PER_PAGE) }; - let page_separator_char = if matches.opt_present(options::FORM_FEED_OPTION) { + let page_separator_char = if matches.is_present(options::FORM_FEED) { let bytes = vec![FF]; String::from_utf8(bytes).unwrap() } else { "\n".to_string() }; - let across_mode = matches.opt_present(options::ACROSS_OPTION); + let across_mode = matches.is_present(options::ACROSS); - let column_separator = match matches.opt_str(options::COLUMN_STRING_SEPARATOR_OPTION) { + let column_separator = match matches.value_of(options::COLUMN_STRING_SEPARATOR) { Some(x) => Some(x), - None => matches.opt_str(options::COLUMN_CHAR_SEPARATOR_OPTION), + None => matches.value_of(options::COLUMN_CHAR_SEPARATOR), } + .map(ToString::to_string) .unwrap_or_else(|| DEFAULT_COLUMN_SEPARATOR.to_string()); - let default_column_width = if matches.opt_present(options::COLUMN_WIDTH_OPTION) - && matches.opt_present(options::COLUMN_CHAR_SEPARATOR_OPTION) + let default_column_width = if matches.is_present(options::COLUMN_WIDTH) + && matches.is_present(options::COLUMN_CHAR_SEPARATOR) { DEFAULT_COLUMN_WIDTH_WITH_S_OPTION } else { @@ -739,12 +701,12 @@ fn build_options( }; let column_width = - parse_usize(matches, options::COLUMN_WIDTH_OPTION).unwrap_or(Ok(default_column_width))?; + parse_usize(matches, options::COLUMN_WIDTH).unwrap_or(Ok(default_column_width))?; - let page_width = if matches.opt_present(options::JOIN_LINES_OPTION) { + let page_width = if matches.is_present(options::JOIN_LINES) { None } else { - match parse_usize(matches, options::PAGE_WIDTH_OPTION) { + match parse_usize(matches, options::PAGE_WIDTH) { Some(res) => Some(res?), None => None, } @@ -764,7 +726,7 @@ fn build_options( // --column has more priority than -column - let column_option_value = match parse_usize(matches, options::COLUMN_OPTION) { + let column_option_value = match parse_usize(matches, options::COLUMN) { Some(res) => Some(res?), None => start_column_option, }; @@ -776,9 +738,8 @@ fn build_options( across_mode, }); - let offset_spaces = - " ".repeat(parse_usize(matches, options::OFFSET_SPACES_OPTION).unwrap_or(Ok(0))?); - let join_lines = matches.opt_present(options::JOIN_LINES_OPTION); + let offset_spaces = " ".repeat(parse_usize(matches, options::INDENT).unwrap_or(Ok(0))?); + let join_lines = matches.is_present(options::JOIN_LINES); let col_sep_for_printing = column_mode_options .as_ref() @@ -855,7 +816,7 @@ fn open(path: &str) -> Result, PrError> { .unwrap_or_else(|_| Err(PrError::NotExists(path.to_string()))) } -fn split_lines_if_form_feed(file_content: Result) -> Vec { +fn split_lines_if_form_feed(file_content: Result) -> Vec { file_content .map(|content| { let mut lines = Vec::new(); @@ -972,7 +933,7 @@ fn read_stream_and_create_pages( ) } -fn mpr(paths: &[String], options: &OutputOptions) -> Result { +fn mpr(paths: &[&str], options: &OutputOptions) -> Result { let n_files = paths.len(); // Check if files exists @@ -1032,7 +993,11 @@ fn mpr(paths: &[String], options: &OutputOptions) -> Result { Ok(0) } -fn print_page(lines: &[FileLine], options: &OutputOptions, page: usize) -> Result { +fn print_page( + lines: &[FileLine], + options: &OutputOptions, + page: usize, +) -> Result { let line_separator = options.line_separator.as_bytes(); let page_separator = options.page_separator_char.as_bytes(); @@ -1064,7 +1029,7 @@ fn write_columns( lines: &[FileLine], options: &OutputOptions, out: &mut impl Write, -) -> Result { +) -> Result { let line_separator = options.content_line_separator.as_bytes(); let content_lines_per_page = if options.double_space { From bb379b53841f652fc582aa1b3c9a56e9c04024e4 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Wed, 23 Feb 2022 23:12:05 +0100 Subject: [PATCH 2/7] pr: fix heuristic for number-lines argument (#3109) --- src/uu/pr/src/pr.rs | 3 ++- tests/by-util/test_pr.rs | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/uu/pr/src/pr.rs b/src/uu/pr/src/pr.rs index 8ba3872f5..79baf72c9 100644 --- a/src/uu/pr/src/pr.rs +++ b/src/uu/pr/src/pr.rs @@ -235,6 +235,7 @@ pub fn uu_app<'a>() -> App<'a> { character) is given, it is appended to the line number to separate it from whatever follows. The default for char is a . Line numbers longer than width columns are truncated.") .takes_value(true) + .allow_hyphen_values(true) .value_name("[char][width]") ) .arg( @@ -453,7 +454,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { /// * `args` - Command line arguments fn recreate_arguments(args: &[String]) -> Vec { let column_page_option = Regex::new(r"^[-+]\d+.*").unwrap(); - let num_regex = Regex::new(r"(.\d+)|(\d+)|^[^-]$").unwrap(); + let num_regex = Regex::new(r"^[^-]\d*$").unwrap(); //let a_file: Regex = Regex::new(r"^[^-+].*").unwrap(); let n_regex = Regex::new(r"^-n\s*$").unwrap(); let mut arguments = args.to_owned(); diff --git a/tests/by-util/test_pr.rs b/tests/by-util/test_pr.rs index 5c16e7acc..2e72aaed7 100644 --- a/tests/by-util/test_pr.rs +++ b/tests/by-util/test_pr.rs @@ -448,3 +448,16 @@ fn test_with_join_lines_option() { &valid_last_modified_template_vars(start), ); } + +#[test] +fn test_value_for_number_lines() { + // *5 is of the form [SEP[NUMBER]] so is accepted and succeeds + new_ucmd!().args(&["-n", "*5", "test.log"]).succeeds(); + + // a is of the form [SEP[NUMBER]] so is accepted and succeeds + new_ucmd!().args(&["-n", "a", "test.log"]).succeeds(); + + // foo5.txt is of not the form [SEP[NUMBER]] so is not used as value. + // Therefore, pr tries to access the file, which does not exist. + new_ucmd!().args(&["-n", "foo5.txt", "test.log"]).fails(); +} From ee36dea1a93a67a9b93af510e0d2d75dd5f84aa1 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Wed, 5 Jan 2022 21:06:04 -0500 Subject: [PATCH 3/7] split: implement outputting kth chunk of file Implement `-n l/k/N` option, where the `k`th chunk of the input file is written to stdout. For example, $ seq -w 0 99 > f; split -n l/3/10 f 20 21 22 23 24 25 26 27 28 29 --- src/uu/split/src/split.rs | 72 ++++++++++++++++ tests/by-util/test_split.rs | 8 ++ tests/fixtures/split/onehundredlines.txt | 100 +++++++++++++++++++++++ 3 files changed, 180 insertions(+) create mode 100644 tests/fixtures/split/onehundredlines.txt diff --git a/src/uu/split/src/split.rs b/src/uu/split/src/split.rs index e2504f305..090d89d4e 100644 --- a/src/uu/split/src/split.rs +++ b/src/uu/split/src/split.rs @@ -859,6 +859,11 @@ where /// /// This function returns an error if there is a problem reading from /// `reader` or writing to one of the output files. +/// +/// # See also +/// +/// * [`kth_chunk_by_line`], which splits its input in the same way, +/// but writes only one specified chunk to stdout. fn split_into_n_chunks_by_line( settings: &Settings, reader: &mut R, @@ -915,6 +920,67 @@ where Ok(()) } +/// Print the k-th chunk of a file, splitting by line. +/// +/// This function is like [`split_into_n_chunks_by_line`], but instead +/// of writing each chunk to its own file, it only writes to stdout +/// the contents of the chunk identified by `chunk_number`. +/// +/// # Errors +/// +/// This function returns an error if there is a problem reading from +/// `reader` or writing to one of the output files. +/// +/// # See also +/// +/// * [`split_into_n_chunks_by_line`], which splits its input in the +/// same way, but writes each chunk to its own file. +fn kth_chunk_by_line( + settings: &Settings, + reader: &mut R, + chunk_number: u64, + num_chunks: u64, +) -> UResult<()> +where + R: BufRead, +{ + // Get the size of the input file in bytes and compute the number + // of bytes per chunk. + let metadata = metadata(&settings.input).unwrap(); + let num_bytes = metadata.len(); + let chunk_size = (num_bytes / (num_chunks as u64)) as usize; + + // Write to stdout instead of to a file. + let stdout = std::io::stdout(); + let mut writer = stdout.lock(); + + let mut num_bytes_remaining_in_current_chunk = chunk_size; + let mut i = 0; + for line_result in reader.lines() { + let line = line_result?; + let bytes = line.as_bytes(); + if i == chunk_number { + writer.write_all(bytes)?; + writer.write_all(b"\n")?; + } + + // Add one byte for the newline character. + let num_bytes = bytes.len() + 1; + if num_bytes >= num_bytes_remaining_in_current_chunk { + num_bytes_remaining_in_current_chunk = chunk_size; + i += 1; + } else { + num_bytes_remaining_in_current_chunk -= num_bytes; + } + + if i > chunk_number { + break; + } + } + + Ok(()) +} + fn split(settings: &Settings) -> UResult<()> { let mut reader = BufReader::new(if settings.input == "-" { Box::new(stdin()) as Box @@ -935,6 +1001,12 @@ fn split(settings: &Settings) -> UResult<()> { Strategy::Number(NumberType::Lines(num_chunks)) => { split_into_n_chunks_by_line(settings, &mut reader, num_chunks) } + Strategy::Number(NumberType::KthLines(chunk_number, num_chunks)) => { + // The chunk number is given as a 1-indexed number, but it + // is a little easier to deal with a 0-indexed number. + let chunk_number = chunk_number - 1; + kth_chunk_by_line(settings, &mut reader, chunk_number, num_chunks) + } Strategy::Number(_) => Err(USimpleError::new(1, "-n mode not yet fully implemented")), Strategy::Lines(chunk_size) => { let mut writer = LineChunkWriter::new(chunk_size, settings) diff --git a/tests/by-util/test_split.rs b/tests/by-util/test_split.rs index ab59a573a..06aa9ea61 100644 --- a/tests/by-util/test_split.rs +++ b/tests/by-util/test_split.rs @@ -587,3 +587,11 @@ fn test_lines() { assert_eq!(file_read("xaa"), "1\n2\n3\n"); assert_eq!(file_read("xab"), "4\n5\n"); } + +#[test] +fn test_lines_kth() { + new_ucmd!() + .args(&["-n", "l/3/10", "onehundredlines.txt"]) + .succeeds() + .stdout_only("20\n21\n22\n23\n24\n25\n26\n27\n28\n29\n"); +} diff --git a/tests/fixtures/split/onehundredlines.txt b/tests/fixtures/split/onehundredlines.txt new file mode 100644 index 000000000..f2abdb403 --- /dev/null +++ b/tests/fixtures/split/onehundredlines.txt @@ -0,0 +1,100 @@ +00 +01 +02 +03 +04 +05 +06 +07 +08 +09 +10 +11 +12 +13 +14 +15 +16 +17 +18 +19 +20 +21 +22 +23 +24 +25 +26 +27 +28 +29 +30 +31 +32 +33 +34 +35 +36 +37 +38 +39 +40 +41 +42 +43 +44 +45 +46 +47 +48 +49 +50 +51 +52 +53 +54 +55 +56 +57 +58 +59 +60 +61 +62 +63 +64 +65 +66 +67 +68 +69 +70 +71 +72 +73 +74 +75 +76 +77 +78 +79 +80 +81 +82 +83 +84 +85 +86 +87 +88 +89 +90 +91 +92 +93 +94 +95 +96 +97 +98 +99 From f3bd1f302047502cafc24e4b467fcc92488ed65a Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 4 Mar 2022 22:22:19 +0100 Subject: [PATCH 4/7] Add onehundredlines in the spell ignore --- tests/by-util/test_split.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-util/test_split.rs b/tests/by-util/test_split.rs index 06aa9ea61..08431a8f6 100644 --- a/tests/by-util/test_split.rs +++ b/tests/by-util/test_split.rs @@ -2,7 +2,7 @@ // * // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -// spell-checker:ignore xzaaa sixhundredfiftyonebytes ninetyonebytes threebytes asciilowercase fghij klmno pqrst uvwxyz fivelines twohundredfortyonebytes +// spell-checker:ignore xzaaa sixhundredfiftyonebytes ninetyonebytes threebytes asciilowercase fghij klmno pqrst uvwxyz fivelines twohundredfortyonebytes onehundredlines extern crate rand; extern crate regex; From 3fb36d02e3c1816265b6e6be82586c40937efe84 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 19 Feb 2022 19:14:21 -0500 Subject: [PATCH 5/7] df: add unit tests for internal helper functions Add unit tests in the `df.rs` file for internal helper functions `mount_info_lt()`, `is_best()`, `is_included()`, and `filter_mount_list()`. --- src/uu/df/src/df.rs | 321 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 320 insertions(+), 1 deletion(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index b7d27e245..6d480662e 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -5,7 +5,7 @@ // // For the full copyright and license information, please view the LICENSE file // that was distributed with this source code. -// spell-checker:ignore itotal iused iavail ipcent pcent +// spell-checker:ignore itotal iused iavail ipcent pcent tmpfs squashfs mod table; #[cfg(unix)] @@ -431,3 +431,322 @@ pub fn uu_app<'a>() -> App<'a> { ) .arg(Arg::new(OPT_PATHS).multiple_occurrences(true)) } + +#[cfg(test)] +mod tests { + + mod mount_info_lt { + + use crate::mount_info_lt; + use uucore::fsext::MountInfo; + + /// Instantiate a [`MountInfo`] with the given fields. + fn mount_info(dev_name: &str, mount_root: &str, mount_dir: &str) -> MountInfo { + MountInfo { + dev_id: String::new(), + dev_name: String::from(dev_name), + fs_type: String::new(), + mount_dir: String::from(mount_dir), + mount_option: String::new(), + mount_root: String::from(mount_root), + remote: false, + dummy: false, + } + } + + #[test] + fn test_absolute() { + // Prefer device name "/dev/foo" over "dev_foo". + let m1 = mount_info("/dev/foo", "/", "/mnt/bar"); + let m2 = mount_info("dev_foo", "/", "/mnt/bar"); + assert!(!mount_info_lt(&m1, &m2)); + } + + #[test] + fn test_shorter() { + // Prefer mount directory "/mnt/bar" over "/mnt/bar/baz"... + let m1 = mount_info("/dev/foo", "/", "/mnt/bar"); + let m2 = mount_info("/dev/foo", "/", "/mnt/bar/baz"); + assert!(!mount_info_lt(&m1, &m2)); + + // ..but prefer mount root "/root" over "/". + let m1 = mount_info("/dev/foo", "/root", "/mnt/bar"); + let m2 = mount_info("/dev/foo", "/", "/mnt/bar/baz"); + assert!(mount_info_lt(&m1, &m2)); + } + + #[test] + fn test_over_mounted() { + // Prefer the earlier entry if the devices are different but + // the mount directory is the same. + let m1 = mount_info("/dev/foo", "/", "/mnt/baz"); + let m2 = mount_info("/dev/bar", "/", "/mnt/baz"); + assert!(!mount_info_lt(&m1, &m2)); + } + } + + mod is_best { + + use crate::is_best; + use uucore::fsext::MountInfo; + + /// Instantiate a [`MountInfo`] with the given fields. + fn mount_info(dev_id: &str, mount_dir: &str) -> MountInfo { + MountInfo { + dev_id: String::from(dev_id), + dev_name: String::new(), + fs_type: String::new(), + mount_dir: String::from(mount_dir), + mount_option: String::new(), + mount_root: String::new(), + remote: false, + dummy: false, + } + } + + #[test] + fn test_empty() { + let m = mount_info("0", "/mnt/bar"); + assert!(is_best(&[], &m)); + } + + #[test] + fn test_different_dev_id() { + let m1 = mount_info("0", "/mnt/bar"); + let m2 = mount_info("1", "/mnt/bar"); + assert!(is_best(&[m1.clone()], &m2)); + assert!(is_best(&[m2], &m1)); + } + + #[test] + fn test_same_dev_id() { + // There are several conditions under which a `MountInfo` is + // considered "better" than the others, we're just checking + // one condition in this test. + let m1 = mount_info("0", "/mnt/bar"); + let m2 = mount_info("0", "/mnt/bar/baz"); + assert!(!is_best(&[m1.clone()], &m2)); + assert!(is_best(&[m2], &m1)); + } + } + + mod is_included { + + use crate::{is_included, FsSelector, Options}; + use std::collections::HashSet; + use uucore::fsext::MountInfo; + + /// Instantiate a [`MountInfo`] with the given fields. + fn mount_info(fs_type: &str, mount_dir: &str, remote: bool, dummy: bool) -> MountInfo { + MountInfo { + dev_id: String::new(), + dev_name: String::new(), + fs_type: String::from(fs_type), + mount_dir: String::from(mount_dir), + mount_option: String::new(), + mount_root: String::new(), + remote, + dummy, + } + } + + #[test] + fn test_remote_included() { + let opt = Default::default(); + let paths = []; + let m = mount_info("ext4", "/mnt/foo", true, false); + assert!(is_included(&m, &paths, &opt)); + } + + #[test] + fn test_remote_excluded() { + let opt = Options { + show_local_fs: true, + ..Default::default() + }; + let paths = []; + let m = mount_info("ext4", "/mnt/foo", true, false); + assert!(!is_included(&m, &paths, &opt)); + } + + #[test] + fn test_dummy_included() { + let opt = Options { + show_all_fs: true, + show_listed_fs: true, + ..Default::default() + }; + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, true); + assert!(is_included(&m, &paths, &opt)); + } + + #[test] + fn test_dummy_excluded() { + let opt = Default::default(); + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, true); + assert!(!is_included(&m, &paths, &opt)); + } + + #[test] + fn test_exclude_match() { + let exclude = HashSet::from([String::from("ext4")]); + let fs_selector = FsSelector { + exclude, + ..Default::default() + }; + let opt = Options { + fs_selector, + ..Default::default() + }; + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, false); + assert!(!is_included(&m, &paths, &opt)); + } + + #[test] + fn test_exclude_no_match() { + let exclude = HashSet::from([String::from("tmpfs")]); + let fs_selector = FsSelector { + exclude, + ..Default::default() + }; + let opt = Options { + fs_selector, + ..Default::default() + }; + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, false); + assert!(is_included(&m, &paths, &opt)); + } + + #[test] + fn test_include_match() { + let include = HashSet::from([String::from("ext4")]); + let fs_selector = FsSelector { + include, + ..Default::default() + }; + let opt = Options { + fs_selector, + ..Default::default() + }; + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, false); + assert!(is_included(&m, &paths, &opt)); + } + + #[test] + fn test_include_no_match() { + let include = HashSet::from([String::from("tmpfs")]); + let fs_selector = FsSelector { + include, + ..Default::default() + }; + let opt = Options { + fs_selector, + ..Default::default() + }; + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, false); + assert!(!is_included(&m, &paths, &opt)); + } + + #[test] + fn test_include_and_exclude_match_neither() { + let include = HashSet::from([String::from("tmpfs")]); + let exclude = HashSet::from([String::from("squashfs")]); + let fs_selector = FsSelector { include, exclude }; + let opt = Options { + fs_selector, + ..Default::default() + }; + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, false); + assert!(!is_included(&m, &paths, &opt)); + } + + #[test] + fn test_include_and_exclude_match_exclude() { + let include = HashSet::from([String::from("tmpfs")]); + let exclude = HashSet::from([String::from("ext4")]); + let fs_selector = FsSelector { include, exclude }; + let opt = Options { + fs_selector, + ..Default::default() + }; + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, false); + assert!(!is_included(&m, &paths, &opt)); + } + + #[test] + fn test_include_and_exclude_match_include() { + let include = HashSet::from([String::from("ext4")]); + let exclude = HashSet::from([String::from("squashfs")]); + let fs_selector = FsSelector { include, exclude }; + let opt = Options { + fs_selector, + ..Default::default() + }; + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, false); + assert!(is_included(&m, &paths, &opt)); + } + + #[test] + fn test_include_and_exclude_match_both() { + // TODO The same filesystem type in both `include` and + // `exclude` should cause an error, but currently does + // not. + let include = HashSet::from([String::from("ext4")]); + let exclude = HashSet::from([String::from("ext4")]); + let fs_selector = FsSelector { include, exclude }; + let opt = Options { + fs_selector, + ..Default::default() + }; + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, false); + assert!(!is_included(&m, &paths, &opt)); + } + + #[test] + fn test_paths_empty() { + let opt = Default::default(); + let paths = []; + let m = mount_info("ext4", "/mnt/foo", false, false); + assert!(is_included(&m, &paths, &opt)); + } + + #[test] + fn test_not_in_paths() { + let opt = Default::default(); + let paths = [String::from("/mnt/foo")]; + let m = mount_info("ext4", "/mnt/bar", false, false); + assert!(!is_included(&m, &paths, &opt)); + } + + #[test] + fn test_in_paths() { + let opt = Default::default(); + let paths = [String::from("/mnt/foo")]; + let m = mount_info("ext4", "/mnt/foo", false, false); + assert!(is_included(&m, &paths, &opt)); + } + } + + mod filter_mount_list { + + use crate::filter_mount_list; + + #[test] + fn test_empty() { + let opt = Default::default(); + let paths = []; + let mount_infos = vec![]; + assert!(filter_mount_list(mount_infos, &paths, &opt).is_empty()); + } + } +} From 41acdb5471784940fbeb359e3b32d573b5562429 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 27 Feb 2022 11:18:45 -0500 Subject: [PATCH 6/7] df: borrow Row in DisplayRow::new() Change the signature of `DisplayRow::new()` to borrow `Row` instead of consuming it, so that the `Row` can be used after it is displayed. --- src/uu/df/src/df.rs | 2 +- src/uu/df/src/table.rs | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 6d480662e..436c19247 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -309,7 +309,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .collect(); println!("{}", Header::new(&opt)); for row in data { - println!("{}", DisplayRow::new(row, &opt)); + println!("{}", DisplayRow::new(&row, &opt)); } Ok(()) diff --git a/src/uu/df/src/table.rs b/src/uu/df/src/table.rs index 2f7fba456..682d7836b 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -120,7 +120,7 @@ impl From for Row { /// The `options` control how the information in the row gets displayed. pub(crate) struct DisplayRow<'a> { /// The data in this row. - row: Row, + row: &'a Row, /// Options that control how to display the data. options: &'a Options, @@ -135,7 +135,7 @@ pub(crate) struct DisplayRow<'a> { impl<'a> DisplayRow<'a> { /// Instantiate this struct. - pub(crate) fn new(row: Row, options: &'a Options) -> Self { + pub(crate) fn new(row: &'a Row, options: &'a Options) -> Self { Self { row, options } } @@ -354,7 +354,7 @@ mod tests { inodes_usage: Some(0.2), }; assert_eq!( - DisplayRow::new(row, &options).to_string(), + DisplayRow::new(&row, &options).to_string(), "my_device 100 25 75 25% my_mount " ); } @@ -385,7 +385,7 @@ mod tests { inodes_usage: Some(0.2), }; assert_eq!( - DisplayRow::new(row, &options).to_string(), + DisplayRow::new(&row, &options).to_string(), "my_device my_type 100 25 75 25% my_mount " ); } @@ -416,7 +416,7 @@ mod tests { inodes_usage: Some(0.2), }; assert_eq!( - DisplayRow::new(row, &options).to_string(), + DisplayRow::new(&row, &options).to_string(), "my_device 10 2 8 20% my_mount " ); } @@ -447,7 +447,7 @@ mod tests { inodes_usage: Some(0.2), }; assert_eq!( - DisplayRow::new(row, &options).to_string(), + DisplayRow::new(&row, &options).to_string(), "my_device my_type 4.0k 1.0k 3.0k 25% my_mount " ); } @@ -478,7 +478,7 @@ mod tests { inodes_usage: Some(0.2), }; assert_eq!( - DisplayRow::new(row, &options).to_string(), + DisplayRow::new(&row, &options).to_string(), "my_device my_type 4.0Ki 1.0Ki 3.0Ki 25% my_mount " ); } From d0ebd1c9d06dd67e47207fc43fb571b818cf1ccb Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sun, 27 Feb 2022 11:20:27 -0500 Subject: [PATCH 7/7] df: add support for --total option Add support for the `--total` option to `df`, which displays the total of each numeric column. For example, $ df --total Filesystem 1K-blocks Used Available Use% Mounted on udev 3858016 0 3858016 0% /dev ... /dev/loop14 63488 63488 0 100% /snap/core20/1361 total 258775268 98099712 148220200 40% - --- src/uu/df/src/df.rs | 15 +++++++++++ src/uu/df/src/table.rs | 58 ++++++++++++++++++++++++++++++++++++++++ tests/by-util/test_df.rs | 44 ++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 436c19247..6371bb0be 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -106,6 +106,11 @@ impl From<&ArgMatches> for BlockSize { } } +/// Parameters that control the behavior of `df`. +/// +/// Most of these parameters control which rows and which columns are +/// displayed. The `block_size` determines the units to use when +/// displaying numbers of bytes or inodes. #[derive(Default)] struct Options { show_local_fs: bool, @@ -115,6 +120,9 @@ struct Options { show_inode_instead: bool, block_size: BlockSize, fs_selector: FsSelector, + + /// Whether to show a final row comprising the totals for each column. + show_total: bool, } impl Options { @@ -128,6 +136,7 @@ impl Options { show_inode_instead: matches.is_present(OPT_INODES), block_size: BlockSize::from(matches), fs_selector: FsSelector::from(matches), + show_total: matches.is_present(OPT_TOTAL), } } } @@ -307,9 +316,15 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .filter(|fs| fs.usage.blocks != 0 || opt.show_all_fs || opt.show_listed_fs) .map(Into::into) .collect(); + println!("{}", Header::new(&opt)); + let mut total = Row::new("total"); for row in data { println!("{}", DisplayRow::new(&row, &opt)); + total += row; + } + if opt.show_total { + println!("{}", DisplayRow::new(&total, &opt)); } Ok(()) diff --git a/src/uu/df/src/table.rs b/src/uu/df/src/table.rs index 682d7836b..1b1c61d1f 100644 --- a/src/uu/df/src/table.rs +++ b/src/uu/df/src/table.rs @@ -15,6 +15,7 @@ use crate::{BlockSize, Filesystem, Options}; use uucore::fsext::{FsUsage, MountInfo}; use std::fmt; +use std::ops::AddAssign; /// A row in the filesystem usage data table. /// @@ -67,6 +68,63 @@ pub(crate) struct Row { inodes_usage: Option, } +impl Row { + pub(crate) fn new(source: &str) -> Self { + Self { + fs_device: source.into(), + fs_type: "-".into(), + fs_mount: "-".into(), + bytes: 0, + bytes_used: 0, + bytes_free: 0, + bytes_usage: None, + #[cfg(target_os = "macos")] + bytes_capacity: None, + inodes: 0, + inodes_used: 0, + inodes_free: 0, + inodes_usage: None, + } + } +} + +impl AddAssign for Row { + /// Sum the numeric values of two rows. + /// + /// The `Row::fs_device` field is set to `"total"` and the + /// remaining `String` fields are set to `"-"`. + fn add_assign(&mut self, rhs: Self) { + let bytes = self.bytes + rhs.bytes; + let bytes_used = self.bytes_used + rhs.bytes_used; + let inodes = self.inodes + rhs.inodes; + let inodes_used = self.inodes_used + rhs.inodes_used; + *self = Self { + fs_device: "total".into(), + fs_type: "-".into(), + fs_mount: "-".into(), + bytes, + bytes_used, + bytes_free: self.bytes_free + rhs.bytes_free, + bytes_usage: if bytes == 0 { + None + } else { + Some(bytes_used as f64 / bytes as f64) + }, + // TODO Figure out how to compute this. + #[cfg(target_os = "macos")] + bytes_capacity: None, + inodes, + inodes_used, + inodes_free: self.inodes_free + rhs.inodes_free, + inodes_usage: if inodes == 0 { + None + } else { + Some(inodes_used as f64 / inodes as f64) + }, + } + } +} + impl From for Row { fn from(fs: Filesystem) -> Self { let MountInfo { diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index 3af02428e..513423766 100644 --- a/tests/by-util/test_df.rs +++ b/tests/by-util/test_df.rs @@ -1,3 +1,4 @@ +// spell-checker:ignore udev use crate::common::util::*; #[test] @@ -77,4 +78,47 @@ fn test_type_option() { new_ucmd!().args(&["-t", "ext4", "-t", "ext3"]).succeeds(); } +#[test] +fn test_total() { + // Example output: + // + // Filesystem 1K-blocks Used Available Use% Mounted on + // udev 3858016 0 3858016 0% /dev + // ... + // /dev/loop14 63488 63488 0 100% /snap/core20/1361 + // total 258775268 98099712 148220200 40% - + let output = new_ucmd!().arg("--total").succeeds().stdout_move_str(); + + // Skip the header line. + let lines: Vec<&str> = output.lines().skip(1).collect(); + + // Parse the values from the last row. + let last_line = lines.last().unwrap(); + let mut iter = last_line.split_whitespace(); + assert_eq!(iter.next().unwrap(), "total"); + let reported_total_size = iter.next().unwrap().parse().unwrap(); + let reported_total_used = iter.next().unwrap().parse().unwrap(); + let reported_total_avail = iter.next().unwrap().parse().unwrap(); + + // Loop over each row except the last, computing the sum of each column. + let mut computed_total_size = 0; + let mut computed_total_used = 0; + let mut computed_total_avail = 0; + let n = lines.len(); + for line in &lines[..n - 1] { + let mut iter = line.split_whitespace(); + iter.next().unwrap(); + computed_total_size += iter.next().unwrap().parse::().unwrap(); + computed_total_used += iter.next().unwrap().parse::().unwrap(); + computed_total_avail += iter.next().unwrap().parse::().unwrap(); + } + + // Check that the sum of each column matches the reported value in + // the last row. + assert_eq!(computed_total_size, reported_total_size); + assert_eq!(computed_total_used, reported_total_used); + assert_eq!(computed_total_avail, reported_total_avail); + // TODO We could also check here that the use percentage matches. +} + // ToDO: more tests...