From 28c21902c569fe718b50215256b7db3defecf691 Mon Sep 17 00:00:00 2001 From: Joseph Crail Date: Sun, 10 May 2015 18:57:24 -0400 Subject: [PATCH] Fix nl. Aside from the usual upgrades to sync with the nightly build, I fixed an unwrap() panic when reading lines with only a newline. I also refactored the repeated command calls to use helper functions. --- src/nl/helper.rs | 26 +++++++++--------- src/nl/nl.rs | 40 +++++++++++++-------------- test/nl.rs | 71 ++++++++++++++++++++++++++---------------------- 3 files changed, 71 insertions(+), 66 deletions(-) diff --git a/src/nl/helper.rs b/src/nl/helper.rs index c1d0fc584..a24de9dd0 100644 --- a/src/nl/helper.rs +++ b/src/nl/helper.rs @@ -8,8 +8,8 @@ fn parse_style(chars: &[char]) -> Result<::NumberingStyle, String> { ['t'] => { Ok(::NumberingStyle::NumberForNonEmpty) }, ['n'] => { Ok(::NumberingStyle::NumberForNone) }, ['p', rest..] => { - let s : String = rest.iter().map(|c| *c).collect(); - match regex::Regex::new(s.as_slice()) { + let s: String = rest.iter().map(|c| *c).collect(); + match regex::Regex::new(&s) { Ok(re) => Ok(::NumberingStyle::NumberForRegularExpression(re)), Err(_) => Err(String::from_str("Illegal regular expression")), } @@ -32,7 +32,7 @@ pub fn parse_options(settings: &mut ::Settings, opts: &getopts::Matches) -> Vec< } match opts.opt_str("n") { None => {}, - Some(val) => match val.as_slice() { + Some(val) => match val.as_ref() { "ln" => { settings.number_format = ::NumberFormat::Left; }, "rn" => { settings.number_format = ::NumberFormat::Right; }, "rz" => { settings.number_format = ::NumberFormat::RightZero; }, @@ -42,8 +42,8 @@ pub fn parse_options(settings: &mut ::Settings, opts: &getopts::Matches) -> Vec< match opts.opt_str("b") { None => {}, Some(val) => { - let chars: Vec = val.as_slice().chars().collect(); - match parse_style(chars.as_slice()) { + let chars: Vec = val.chars().collect(); + match parse_style(&chars) { Ok(s) => { settings.body_numbering = s; } Err(message) => { errs.push(message); } } @@ -52,8 +52,8 @@ pub fn parse_options(settings: &mut ::Settings, opts: &getopts::Matches) -> Vec< match opts.opt_str("f") { None => {}, Some(val) => { - let chars: Vec = val.as_slice().chars().collect(); - match parse_style(chars.as_slice()) { + let chars: Vec = val.chars().collect(); + match parse_style(&chars) { Ok(s) => { settings.footer_numbering = s; } Err(message) => { errs.push(message); } } @@ -62,8 +62,8 @@ pub fn parse_options(settings: &mut ::Settings, opts: &getopts::Matches) -> Vec< match opts.opt_str("h") { None => {}, Some(val) => { - let chars: Vec = val.as_slice().chars().collect(); - match parse_style(chars.as_slice()) { + let chars: Vec = val.chars().collect(); + match parse_style(&chars) { Ok(s) => { settings.header_numbering = s; } Err(message) => { errs.push(message); } } @@ -72,7 +72,7 @@ pub fn parse_options(settings: &mut ::Settings, opts: &getopts::Matches) -> Vec< match opts.opt_str("i") { None => {} Some(val) => { - let conv: Option = val.as_slice().parse().ok(); + let conv: Option = val.parse().ok(); match conv { None => { errs.push(String::from_str("Illegal value for -i")); @@ -84,7 +84,7 @@ pub fn parse_options(settings: &mut ::Settings, opts: &getopts::Matches) -> Vec< match opts.opt_str("w") { None => {} Some(val) => { - let conv: Option = val.as_slice().parse().ok(); + let conv: Option = val.parse().ok(); match conv { None => { errs.push(String::from_str("Illegal value for -w")); @@ -96,7 +96,7 @@ pub fn parse_options(settings: &mut ::Settings, opts: &getopts::Matches) -> Vec< match opts.opt_str("v") { None => {} Some(val) => { - let conv: Option = val.as_slice().parse().ok(); + let conv: Option = val.parse().ok(); match conv { None => { errs.push(String::from_str("Illegal value for -v")); @@ -108,7 +108,7 @@ pub fn parse_options(settings: &mut ::Settings, opts: &getopts::Matches) -> Vec< match opts.opt_str("l") { None => {} Some(val) => { - let conv: Option = val.as_slice().parse().ok(); + let conv: Option = val.parse().ok(); match conv { None => { errs.push(String::from_str("Illegal value for -l")); diff --git a/src/nl/nl.rs b/src/nl/nl.rs index 31f052dee..d80a048d9 100644 --- a/src/nl/nl.rs +++ b/src/nl/nl.rs @@ -1,5 +1,5 @@ #![crate_name = "nl"] -#![feature(collections, core, old_io, old_path, rustc_private)] +#![feature(collections, rustc_private, slice_patterns)] #![plugin(regex_macros)] /* @@ -16,13 +16,11 @@ extern crate regex; extern crate getopts; -use std::old_io::{stdin}; -use std::old_io::BufferedReader; -use std::old_io::fs::File; +use std::fs::File; +use std::io::{BufRead, BufReader, Read, stdin, Write}; use std::iter::repeat; -use std::num::Int; -use std::old_path::Path; -use getopts::{optopt, optflag, getopts, usage, OptGroup}; +use std::path::Path; +use getopts::{getopts, optflag, OptGroup, optopt, usage}; #[path="../common/util.rs"] #[macro_use] @@ -35,7 +33,7 @@ static USAGE: &'static str = "nl [OPTION]... [FILE]..."; static REGEX_DUMMY: &'static regex::Regex = ®ex!(r".?"); // Settings store options used by nl to produce its output. -struct Settings { +pub struct Settings { // The variables corresponding to the options -h, -b, and -f. header_numbering: NumberingStyle, body_numbering: NumberingStyle, @@ -109,7 +107,7 @@ pub fn uumain(args: Vec) -> i32 { number_separator: String::from_str("\t"), }; - let given_options = match getopts(args.tail(), &possible_options) { + let given_options = match getopts(&args[1..], &possible_options) { Ok (m) => { m } Err(f) => { show_error!("{}", f); @@ -130,7 +128,7 @@ pub fn uumain(args: Vec) -> i32 { if parse_errors.len() > 0 { show_error!("Invalid arguments supplied."); for message in parse_errors.iter() { - println!("{}", message.as_slice()); + println!("{}", message); } return 1; } @@ -139,27 +137,27 @@ pub fn uumain(args: Vec) -> i32 { let mut read_stdin = files.is_empty(); for file in files.iter() { - if file.as_slice() == "-" { + if file == "-" { // If both file names and '-' are specified, we choose to treat first all // regular files, and then read from stdin last. read_stdin = true; continue } - let path = Path::new(file.as_slice()); - let reader = File::open(&path).unwrap(); - let mut buffer = BufferedReader::new(reader); + let path = Path::new(file); + let reader = File::open(path).unwrap(); + let mut buffer = BufReader::new(reader); nl(&mut buffer, &settings); } if read_stdin { - let mut buffer = BufferedReader::new(stdin()); + let mut buffer = BufReader::new(stdin()); nl(&mut buffer, &settings); } 0 } // nl implements the main functionality for an individual buffer. -fn nl (reader: &mut BufferedReader, settings: &Settings) { +fn nl (reader: &mut BufReader, settings: &Settings) { let mut line_no = settings.starting_line_number; // The current line number's width as a string. Using to_string is inefficient // but since we only do it once, it should not hurt. @@ -167,7 +165,7 @@ fn nl (reader: &mut BufferedReader, settings: &Settings) { let line_no_width_initial = line_no_width; // Stores the smallest integer with one more digit than line_no, so that // when line_no >= line_no_threshold, we need to use one more digit. - let mut line_no_threshold = Int::pow(10u64, line_no_width as u32); + let mut line_no_threshold = 10u64.pow(line_no_width as u32); let mut empty_line_count: u64 = 0; let fill_char = match settings.number_format { NumberFormat::RightZero => '0', @@ -181,13 +179,13 @@ fn nl (reader: &mut BufferedReader, settings: &Settings) { let mut line_filter : fn(&str, ®ex::Regex) -> bool = pass_regex; for mut l in reader.lines().map(|r| r.unwrap()) { // Sanitize the string. We want to print the newline ourselves. - if l.as_slice().chars().rev().next().unwrap() == '\n' { + if l.len() > 0 && l.chars().rev().next().unwrap() == '\n' { l.pop(); } // Next we iterate through the individual chars to see if this // is one of the special lines starting a new "section" in the // document. - let line = l.as_slice(); + let line = l; let mut odd = false; // matched_group counts how many copies of section_delimiter // this string consists of (0 if there's anything else) @@ -229,7 +227,7 @@ fn nl (reader: &mut BufferedReader, settings: &Settings) { if settings.renumber { line_no = settings.starting_line_number; line_no_width = line_no_width_initial; - line_no_threshold = Int::pow(10u64, line_no_width as u32); + line_no_threshold = 10u64.pow(line_no_width as u32); } &settings.header_numbering }, @@ -268,7 +266,7 @@ fn nl (reader: &mut BufferedReader, settings: &Settings) { // in the next selector. empty_line_count = 0; } - if !line_filter(line, regex_filter) + if !line_filter(&line, regex_filter) || ( empty_line_count > 0 && empty_line_count < settings.join_blank_lines) { // No number is printed for this line. Either we did not // want to print one in the first place, or it is a blank diff --git a/test/nl.rs b/test/nl.rs index f27f4a4c2..3959fce3d 100644 --- a/test/nl.rs +++ b/test/nl.rs @@ -1,48 +1,57 @@ -#![allow(unstable)] - -use std::old_io::process::Command; +use std::io::Write; +use std::process::{Command, Stdio}; use std::str; static PROGNAME: &'static str = "./nl"; +fn run(args: &[&'static str]) -> String { + let po = Command::new(PROGNAME) + .args(args) + .output() + .unwrap_or_else(|e| panic!("{}", e)); + + str::from_utf8(&po.stdout).unwrap().to_string() +} + +fn run_with_stdin(input: &str, args: &[&'static str]) -> String { + let mut process = Command::new(PROGNAME) + .args(args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .unwrap_or_else(|e| panic!("{}", e)); + + process.stdin + .take() + .unwrap_or_else(|| panic!("Could not take child process stdin")) + .write_all(input.as_bytes()) + .unwrap_or_else(|e| panic!("{}", e)); + + let po = process.wait_with_output().unwrap_or_else(|e| panic!("{}", e)); + str::from_utf8(&po.stdout).unwrap().to_string() +} + #[test] fn test_stdin_nonewline() { - let mut process = Command::new(PROGNAME).spawn().unwrap(); - process.stdin.take().unwrap().write_all(b"No Newline").unwrap(); - let po = process.wait_with_output().unwrap(); - let out = str::from_utf8(po.output.as_slice()).unwrap(); - - assert_eq!(out, " 1\tNo Newline\n"); + let out = run_with_stdin("No Newline", &[]); + assert_eq!(&out, " 1\tNo Newline\n"); } #[test] fn test_stdin_newline() { - let mut process = Command::new(PROGNAME).arg("-s").arg("-") - .arg("-w").arg("1").spawn().unwrap(); - - process.stdin.take().unwrap().write_all(b"Line One\nLine Two\n").unwrap(); - let po = process.wait_with_output().unwrap(); - let out = str::from_utf8(po.output.as_slice()).unwrap(); - - assert_eq!(out, "1-Line One\n2-Line Two\n"); + let out = run_with_stdin("Line One\nLine Two\n", &["-s", "-", "-w", "1"]); + assert_eq!(&out, "1-Line One\n2-Line Two\n"); } #[test] fn test_padding_without_overflow() { - let po = Command::new(PROGNAME).arg("-i").arg("1000").arg("-s").arg("x") - .arg("-n").arg("rz").arg("simple.txt").output().unwrap(); - - let out = str::from_utf8(po.output.as_slice()).unwrap(); - assert_eq!(out, "000001xL1\n001001xL2\n002001xL3\n003001xL4\n004001xL5\n005001xL6\n006001xL7\n007001xL8\n008001xL9\n009001xL10\n010001xL11\n011001xL12\n012001xL13\n013001xL14\n014001xL15\n"); + let out = run(&["-i", "1000", "-s", "x", "-n", "rz", "simple.txt"]); + assert_eq!(&out, "000001xL1\n001001xL2\n002001xL3\n003001xL4\n004001xL5\n005001xL6\n006001xL7\n007001xL8\n008001xL9\n009001xL10\n010001xL11\n011001xL12\n012001xL13\n013001xL14\n014001xL15\n"); } #[test] fn test_padding_with_overflow() { - let po = Command::new(PROGNAME).arg("-i").arg("1000").arg("-s").arg("x") - .arg("-n").arg("rz").arg("-w").arg("4") - .arg("simple.txt").output().unwrap(); - - let out = str::from_utf8(po.output.as_slice()).unwrap(); - assert_eq!(out, "0001xL1\n1001xL2\n2001xL3\n3001xL4\n4001xL5\n5001xL6\n6001xL7\n7001xL8\n8001xL9\n9001xL10\n10001xL11\n11001xL12\n12001xL13\n13001xL14\n14001xL15\n"); + let out = run(&["-i", "1000", "-s", "x", "-n", "rz", "-w", "4", "simple.txt"]); + assert_eq!(&out, "0001xL1\n1001xL2\n2001xL3\n3001xL4\n4001xL5\n5001xL6\n6001xL7\n7001xL8\n8001xL9\n9001xL10\n10001xL11\n11001xL12\n12001xL13\n13001xL14\n14001xL15\n"); } #[test] @@ -57,9 +66,7 @@ fn test_sections_and_styles() { "1 |Nonempty\n2 |Nonempty\n3 |Followed by 10x empty\n\n\n\n\n4 |\n\n\n\n\n5 |\n6 |Followed by 5x empty\n\n\n\n\n7 |\n8 |Followed by 4x empty\n\n\n\n\n9 |Nonempty\n10 |Nonempty\n11 |Nonempty.\n" ), ].iter() { - let po = Command::new(PROGNAME).arg("-s").arg("|").arg("-n").arg("ln") - .arg("-w").arg("3").arg("-b").arg("a").arg("-l").arg("5") - .arg(fixture).output().unwrap(); - assert_eq!(str::from_utf8(po.output.as_slice()).unwrap(), output); + let out = run(&["-s", "|", "-n", "ln", "-w", "3", "-b", "a", "-l", "5", fixture]); + assert_eq!(&out, output); } }