From e958864bd9aaf2f90f8d78f09f8281b47519150a Mon Sep 17 00:00:00 2001 From: Yagiz Degirmenci <62724709+ycd@users.noreply.github.com> Date: Wed, 31 Mar 2021 22:21:10 +0300 Subject: [PATCH] tac: exit with proper code, move from getopts to clap, add test for invalid inputs (#1957) --- Cargo.lock | 6 +- src/uu/tac/Cargo.toml | 2 +- src/uu/tac/src/tac.rs | 136 ++++++++++++++++++++------------------ tests/by-util/test_tac.rs | 18 +++++ 4 files changed, 95 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1caa4750d..08a7bad43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2323,9 +2323,9 @@ dependencies = [ name = "uu_tac" version = "0.0.4" dependencies = [ - "getopts", - "uucore", - "uucore_procs", + "clap 2.33.3 (registry+https://github.com/rust-lang/crates.io-index)", + "uucore 0.0.7", + "uucore_procs 0.0.5", ] [[package]] diff --git a/src/uu/tac/Cargo.toml b/src/uu/tac/Cargo.toml index 15e743006..2e54c129e 100644 --- a/src/uu/tac/Cargo.toml +++ b/src/uu/tac/Cargo.toml @@ -15,7 +15,7 @@ edition = "2018" path = "src/tac.rs" [dependencies] -getopts = "0.2.18" +clap = "2.33" uucore = { version=">=0.0.7", package="uucore", path="../../uucore" } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 843babac9..68dae94e2 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -10,79 +10,77 @@ #[macro_use] extern crate uucore; -use std::fs::File; +use clap::{App, Arg}; use std::io::{stdin, stdout, BufReader, Read, Stdout, Write}; +use std::{fs::File, path::Path}; static NAME: &str = "tac"; static VERSION: &str = env!("CARGO_PKG_VERSION"); +static USAGE: &str = "[OPTION]... [FILE]..."; +static SUMMARY: &str = "Write each file to standard output, last line first."; + +mod options { + pub static BEFORE: &str = "before"; + pub static REGEX: &str = "regex"; + pub static SEPARATOR: &str = "separator"; + pub static FILE: &str = "file"; +} pub fn uumain(args: impl uucore::Args) -> i32 { let args = args.collect_str(); - let mut opts = getopts::Options::new(); + let matches = App::new(executable!()) + .name(NAME) + .version(VERSION) + .usage(USAGE) + .about(SUMMARY) + .arg( + Arg::with_name(options::BEFORE) + .short("b") + .long(options::BEFORE) + .help("attach the separator before instead of after") + .takes_value(false), + ) + .arg( + Arg::with_name(options::REGEX) + .short("r") + .long(options::REGEX) + .help("interpret the sequence as a regular expression (NOT IMPLEMENTED)") + .takes_value(false), + ) + .arg( + Arg::with_name(options::SEPARATOR) + .short("s") + .long(options::SEPARATOR) + .help("use STRING as the separator instead of newline") + .takes_value(true), + ) + .arg(Arg::with_name(options::FILE).hidden(true).multiple(true)) + .get_matches_from(args); - opts.optflag( - "b", - "before", - "attach the separator before instead of after", - ); - opts.optflag( - "r", - "regex", - "interpret the sequence as a regular expression (NOT IMPLEMENTED)", - ); - opts.optopt( - "s", - "separator", - "use STRING as the separator instead of newline", - "STRING", - ); - opts.optflag("h", "help", "display this help and exit"); - opts.optflag("V", "version", "output version information and exit"); - - let matches = match opts.parse(&args[1..]) { - Ok(m) => m, - Err(f) => crash!(1, "{}", f), - }; - if matches.opt_present("help") { - let msg = format!( - "{0} {1} - -Usage: - {0} [OPTION]... [FILE]... - -Write each file to standard output, last line first.", - NAME, VERSION - ); - - print!("{}", opts.usage(&msg)); - } else if matches.opt_present("version") { - println!("{} {}", NAME, VERSION); - } else { - let before = matches.opt_present("b"); - let regex = matches.opt_present("r"); - let separator = match matches.opt_str("s") { - Some(m) => { - if m.is_empty() { - crash!(1, "separator cannot be empty") - } else { - m - } + let before = matches.is_present(options::BEFORE); + let regex = matches.is_present(options::REGEX); + let separator = match matches.value_of(options::SEPARATOR) { + Some(m) => { + if m.is_empty() { + crash!(1, "separator cannot be empty") + } else { + m.to_owned() } - None => "\n".to_owned(), - }; - let files = if matches.free.is_empty() { - vec!["-".to_owned()] - } else { - matches.free - }; - tac(files, before, regex, &separator[..]); - } + } + None => "\n".to_owned(), + }; - 0 + let files: Vec = match matches.values_of(options::FILE) { + Some(v) => v.map(|v| v.to_owned()).collect(), + None => vec!["-".to_owned()], + }; + + tac(files, before, regex, &separator[..]) } -fn tac(filenames: Vec, before: bool, _: bool, separator: &str) { +fn tac(filenames: Vec, before: bool, _: bool, separator: &str) -> i32 { + let mut exit_code = 0; let mut out = stdout(); let sbytes = separator.as_bytes(); let slen = sbytes.len(); @@ -91,10 +89,19 @@ fn tac(filenames: Vec, before: bool, _: bool, separator: &str) { let mut file = BufReader::new(if filename == "-" { Box::new(stdin()) as Box } else { - match File::open(filename) { + let path = Path::new(filename); + if path.is_dir() || !path.metadata().is_ok() { + show_error!( + "failed to open '{}' for reading: No such file or directory", + filename + ); + continue; + } + match File::open(path) { Ok(f) => Box::new(f) as Box, Err(e) => { - show_warning!("failed to open '{}' for reading: {}", filename, e); + show_error!("failed to open '{}' for reading: {}", filename, e); + exit_code = 1; continue; } } @@ -102,7 +109,8 @@ fn tac(filenames: Vec, before: bool, _: bool, separator: &str) { let mut data = Vec::new(); if let Err(e) = file.read_to_end(&mut data) { - show_warning!("failed to read '{}': {}", filename, e); + show_error!("failed to read '{}': {}", filename, e); + exit_code = 1; continue; }; @@ -141,6 +149,8 @@ fn tac(filenames: Vec, before: bool, _: bool, separator: &str) { } show_line(&mut out, sbytes, &data[0..prev], before); } + + exit_code } fn show_line(out: &mut Stdout, sep: &[u8], dat: &[u8], before: bool) { diff --git a/tests/by-util/test_tac.rs b/tests/by-util/test_tac.rs index d46f9aec6..3733adbec 100644 --- a/tests/by-util/test_tac.rs +++ b/tests/by-util/test_tac.rs @@ -49,3 +49,21 @@ fn test_single_non_newline_separator_before() { .run() .stdout_is_fixture("delimited_primes_before.expected"); } + +#[test] +fn test_invalid_input() { + let (_, mut ucmd) = at_and_ucmd!(); + + ucmd.arg("b") + .run() + .stderr + .contains("tac: error: failed to open 'b' for reading"); + + let (at, mut ucmd) = at_and_ucmd!(); + + at.mkdir("a"); + ucmd.arg("a") + .run() + .stderr + .contains("tac: error: failed to read 'a'"); +}