From 2593b3f2e19ef933a61adea59878a29c911373d2 Mon Sep 17 00:00:00 2001 From: Arijit Dey Date: Sat, 24 Apr 2021 22:32:03 +0530 Subject: [PATCH] Rewrite the cli usage function Add crossterm as dependency Complete the paging portion Fixed tests cp: extract linux COW logic into function cp: add --reflink support for macOS Fixes #1773 Fix error in Cargo.lock Quit automatically if not much output is left Remove unnecessary redox and windows specific code Handle line wrapping Put everything according to uutils coding standards Add support for multiple files Fix failing test Use the args argument to get cli arguments Fix bug where text is repeated multiple times during printing Add a little prompt Add a top file prompt for multiple files Change println in loops to stdout.write and setup terminal only once Fix bug where all lines were printed in a single row Remove useless file and fix failing test Fix another test --- Cargo.lock | 121 ++++++++++ Cargo.toml | 1 + src/uu/more/Cargo.toml | 1 + src/uu/more/src/more.rs | 469 ++++++++++++++++++++++++++++--------- tests/by-util/test_more.rs | 21 +- 5 files changed, 494 insertions(+), 119 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index abdf482ee..fa7a13a5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -217,6 +217,7 @@ dependencies = [ name = "coreutils" version = "0.0.6" dependencies = [ + "atty", "conv", "filetime", "glob 0.3.0", @@ -507,6 +508,31 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "crossterm" +version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7c36c10130df424b2f3552fcc2ddcd9b28a27b1e54b358b45874f88d1ca6888c" +dependencies = [ + "bitflags", + "crossterm_winapi", + "lazy_static", + "libc", + "mio", + "parking_lot", + "signal-hook", + "winapi 0.3.9", +] + +[[package]] +name = "crossterm_winapi" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0da8964ace4d3e4a044fd027919b2237000b24315a37c916f61809f1ff2140b9" +dependencies = [ + "winapi 0.3.9", +] + [[package]] name = "csv" version = "1.1.6" @@ -732,6 +758,15 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "46dbcb333e86939721589d25a3557e180b52778cb33c7fdfe9e0158ff790d5ec" +[[package]] +name = "instant" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61124eeebbd69b8190558df225adf7e4caafce0d743919e5d6b19652314ec5ec" +dependencies = [ + "cfg-if 1.0.0", +] + [[package]] name = "ioctl-sys" version = "0.5.2" @@ -802,6 +837,15 @@ version = "0.2.85" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ccac4b00700875e6a07c6cde370d44d32fa01c5a65cdd2fca6858c479d28bb3" +[[package]] +name = "lock_api" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a3c91c24eae6777794bb1997ad98bbb87daf92890acab859f7eaa4320333176" +dependencies = [ + "scopeguard", +] + [[package]] name = "log" version = "0.4.14" @@ -862,6 +906,28 @@ dependencies = [ "autocfg", ] +[[package]] +name = "mio" +version = "0.7.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e50ae3f04d169fcc9bde0b547d1c205219b7157e07ded9c5aff03e0637cb3ed7" +dependencies = [ + "libc", + "log", + "miow", + "ntapi", + "winapi 0.3.9", +] + +[[package]] +name = "miow" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9f1c5b025cda876f66ef43a113f91ebc9f4ccef34843000e0adf6ebbab84e21" +dependencies = [ + "winapi 0.3.9", +] + [[package]] name = "nix" version = "0.13.1" @@ -893,6 +959,15 @@ version = "0.1.14" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72ef4a56884ca558e5ddb05a1d1e7e1bfd9a68d9ed024c21704cc98872dae1bb" +[[package]] +name = "ntapi" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f6bb902e437b6d86e03cce10a7e2af662292c5dfef23b65899ea3ac9354ad44" +dependencies = [ + "winapi 0.3.9", +] + [[package]] name = "num-integer" version = "0.1.44" @@ -977,6 +1052,31 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "parking_lot" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d7744ac029df22dca6284efe4e898991d28e3085c706c972bcd7da4a27a15eb" +dependencies = [ + "instant", + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa7a782938e745763fe6907fc6ba86946d72f49fe7e21de074e08128a99fb018" +dependencies = [ + "cfg-if 1.0.0", + "instant", + "libc", + "redox_syscall 0.2.6", + "smallvec 1.6.1", + "winapi 0.3.9", +] + [[package]] name = "paste" version = "0.1.18" @@ -1417,6 +1517,26 @@ dependencies = [ "generic-array", ] +[[package]] +name = "signal-hook" +version = "0.1.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e31d442c16f047a671b5a71e2161d6e68814012b7f5379d269ebd915fac2729" +dependencies = [ + "libc", + "mio", + "signal-hook-registry", +] + +[[package]] +name = "signal-hook-registry" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "16f1d0fef1604ba8f7a073c7e701f213e056707210e9020af4528e0101ce11a6" +dependencies = [ + "libc", +] + [[package]] name = "smallvec" version = "0.6.14" @@ -2112,6 +2232,7 @@ name = "uu_more" version = "0.0.6" dependencies = [ "clap", + "crossterm", "nix 0.13.1", "redox_syscall 0.1.57", "redox_termios", diff --git a/Cargo.toml b/Cargo.toml index 7c1a771fd..fdac87d7a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -347,6 +347,7 @@ unindent = "0.1" uucore = { version=">=0.0.8", package="uucore", path="src/uucore", features=["entries"] } walkdir = "2.2" tempdir = "0.3" +atty = "*" [target.'cfg(unix)'.dev-dependencies] rust-users = { version="0.10", package="users" } diff --git a/src/uu/more/Cargo.toml b/src/uu/more/Cargo.toml index 1f4bfed68..c79216ae5 100644 --- a/src/uu/more/Cargo.toml +++ b/src/uu/more/Cargo.toml @@ -18,6 +18,7 @@ path = "src/more.rs" clap = "2.33" uucore = { version = ">=0.0.7", package = "uucore", path = "../../uucore" } uucore_procs = { version = ">=0.0.5", package = "uucore_procs", path = "../../uucore_procs" } +crossterm = ">=0.19" [target.'cfg(target_os = "redox")'.dependencies] redox_termios = "0.1" diff --git a/src/uu/more/src/more.rs b/src/uu/more/src/more.rs index eabdbee85..45fe3ed81 100644 --- a/src/uu/more/src/more.rs +++ b/src/uu/more/src/more.rs @@ -10,150 +10,395 @@ #[macro_use] extern crate uucore; -use std::fs::File; -use std::io::{stdin, stdout, BufRead, BufReader, Read, Write}; +use std::{ + convert::TryInto, + fs::File, + io::{stdin, stdout, BufReader, Read, Stdout, Write}, + path::Path, + time::Duration, +}; #[cfg(all(unix, not(target_os = "fuchsia")))] extern crate nix; -#[cfg(all(unix, not(target_os = "fuchsia")))] -use nix::sys::termios::{self, LocalFlags, SetArg}; -use uucore::InvalidEncodingHandling; -#[cfg(target_os = "redox")] -extern crate redox_termios; -#[cfg(target_os = "redox")] -extern crate syscall; +use clap::{App, Arg}; +use crossterm::{ + event::{self, Event, KeyCode, KeyEvent, KeyModifiers}, + execute, + style::Attribute, + terminal, +}; -use clap::{App, Arg, ArgMatches}; - -static VERSION: &str = env!("CARGO_PKG_VERSION"); -static ABOUT: &str = "A file perusal filter for CRT viewing."; - -mod options { - pub const FILE: &str = "file"; +pub mod options { + pub const SILENT: &str = "silent"; + pub const LOGICAL: &str = "logical"; + pub const NO_PAUSE: &str = "no-pause"; + pub const PRINT_OVER: &str = "print-over"; + pub const CLEAN_PRINT: &str = "clean-print"; + pub const SQUEEZE: &str = "squeeze"; + pub const PLAIN: &str = "plain"; + pub const LINES: &str = "lines"; + pub const NUMBER: &str = "number"; + pub const PATTERN: &str = "pattern"; + pub const FROM_LINE: &str = "from-line"; + pub const FILES: &str = "files"; } -fn get_usage() -> String { - format!("{} [options] ...", executable!()) -} +const MULTI_FILE_TOP_PROMPT: &str = "::::::::::::::\n{}\n::::::::::::::\n"; pub fn uumain(args: impl uucore::Args) -> i32 { - let usage = get_usage(); - let args = args - .collect_str(InvalidEncodingHandling::ConvertLossy) - .accept_any(); - let matches = App::new(executable!()) - .version(VERSION) - .usage(usage.as_str()) - .about(ABOUT) + .about("A file perusal filter for CRT viewing.") + .version(env!("CARGO_PKG_VERSION")) .arg( - Arg::with_name(options::FILE) - .number_of_values(1) - .multiple(true), + Arg::with_name(options::SILENT) + .short("d") + .long(options::SILENT) + .help("Display help instead of ringing bell"), + ) + .arg( + Arg::with_name(options::LOGICAL) + .short("f") + .long(options::LOGICAL) + .help("Count logical rather than screen lines"), + ) + .arg( + Arg::with_name(options::NO_PAUSE) + .short("l") + .long(options::NO_PAUSE) + .help("Suppress pause after form feed"), + ) + .arg( + Arg::with_name(options::PRINT_OVER) + .short("c") + .long(options::PRINT_OVER) + .help("Do not scroll, display text and clean line ends"), + ) + .arg( + Arg::with_name(options::CLEAN_PRINT) + .short("p") + .long(options::CLEAN_PRINT) + .help("Do not scroll, clean screen and display text"), + ) + .arg( + Arg::with_name(options::SQUEEZE) + .short("s") + .long(options::SQUEEZE) + .help("Squeeze multiple blank lines into one"), + ) + .arg( + Arg::with_name(options::PLAIN) + .short("u") + .long(options::PLAIN) + .help("Suppress underlining and bold"), + ) + .arg( + Arg::with_name(options::LINES) + .short("n") + .long(options::LINES) + .value_name("number") + .takes_value(true) + .help("The number of lines per screenful"), + ) + .arg( + Arg::with_name(options::NUMBER) + .allow_hyphen_values(true) + .long(options::NUMBER) + .required(false) + .takes_value(true) + .help("Same as --lines"), + ) + .arg( + Arg::with_name(options::FROM_LINE) + .short("F") + .allow_hyphen_values(true) + .required(false) + .takes_value(true) + .value_name("number") + .help("Display file beginning from line number"), + ) + .arg( + Arg::with_name(options::PATTERN) + .short("P") + .allow_hyphen_values(true) + .required(false) + .takes_value(true) + .help("Display file beginning from pattern match"), + ) + .arg( + Arg::with_name(options::FILES) + .required(false) + .multiple(true) + .help("Path to the files to be read"), ) .get_matches_from(args); + let mut buff = String::new(); + let mut stdout = setup_term(); - // FixME: fail without panic for now; but `more` should work with no arguments (ie, for piped input) - if let None | Some("-") = matches.value_of(options::FILE) { - show_usage_error!("Reading from stdin isn't supported yet."); - return 1; - } - - if let Some(x) = matches.value_of(options::FILE) { - let path = std::path::Path::new(x); - if path.is_dir() { - show_usage_error!("'{}' is a directory.", x); - return 1; + if let Some(filenames) = matches.values_of(options::FILES) { + let length = filenames.len(); + for (idx, fname) in filenames.clone().enumerate() { + if Path::new(fname).is_dir() { + show_usage_error!("'{}' is a directory.", fname); + return 1; + } + if filenames.len() > 1 { + buff.push_str(&MULTI_FILE_TOP_PROMPT.replace("{}", fname)); + } + let mut reader = BufReader::new(File::open(fname).unwrap()); + reader.read_to_string(&mut buff).unwrap(); + let last = idx + 1 == length; + more(&buff, &mut stdout, last); + buff.clear(); } + } else { + stdin().read_to_string(&mut buff).unwrap(); + more(&buff, &mut stdout, true); } - - more(matches); - 0 } -#[cfg(all(unix, not(target_os = "fuchsia")))] -fn setup_term() -> termios::Termios { - let mut term = termios::tcgetattr(0).unwrap(); - // Unset canonical mode, so we get characters immediately - term.local_flags.remove(LocalFlags::ICANON); - // Disable local echo - term.local_flags.remove(LocalFlags::ECHO); - termios::tcsetattr(0, SetArg::TCSADRAIN, &term).unwrap(); - term +#[cfg(not(target_os = "fuchsia"))] +fn setup_term() -> std::io::Stdout { + let mut stdout = stdout(); + terminal::enable_raw_mode().unwrap(); + // Change this to a queue if more commands are executed to avoid too many writes to the terminal + execute!(stdout, terminal::Clear(terminal::ClearType::All)).unwrap(); + stdout } -#[cfg(any(windows, target_os = "fuchsia"))] +#[cfg(target_os = "fuchsia")] #[inline(always)] fn setup_term() -> usize { 0 } -#[cfg(target_os = "redox")] -fn setup_term() -> redox_termios::Termios { - let mut term = redox_termios::Termios::default(); - let fd = syscall::dup(0, b"termios").unwrap(); - syscall::read(fd, &mut term).unwrap(); - term.local_flags &= !redox_termios::ICANON; - term.local_flags &= !redox_termios::ECHO; - syscall::write(fd, &term).unwrap(); - let _ = syscall::close(fd); - term +#[cfg(not(target_os = "fuchsia"))] +fn reset_term(stdout: &mut std::io::Stdout) { + terminal::disable_raw_mode().unwrap(); + // Clear the prompt + execute!(stdout, terminal::Clear(terminal::ClearType::CurrentLine)).unwrap(); + println!("\r"); } -#[cfg(all(unix, not(target_os = "fuchsia")))] -fn reset_term(term: &mut termios::Termios) { - term.local_flags.insert(LocalFlags::ICANON); - term.local_flags.insert(LocalFlags::ECHO); - termios::tcsetattr(0, SetArg::TCSADRAIN, &term).unwrap(); -} - -#[cfg(any(windows, target_os = "fuchsia"))] +#[cfg(target_os = "fuchsia")] #[inline(always)] fn reset_term(_: &mut usize) {} -#[cfg(any(target_os = "redox"))] -fn reset_term(term: &mut redox_termios::Termios) { - let fd = syscall::dup(0, b"termios").unwrap(); - syscall::read(fd, term).unwrap(); - term.local_flags |= redox_termios::ICANON; - term.local_flags |= redox_termios::ECHO; - syscall::write(fd, &term).unwrap(); - let _ = syscall::close(fd); -} +fn more(buff: &str, mut stdout: &mut Stdout, is_last: bool) { + let (cols, rows) = terminal::size().unwrap(); + let lines = break_buff(buff, usize::from(cols)); + let line_count: u16 = lines.len().try_into().unwrap(); -fn more(matches: ArgMatches) { - let mut f: Box = match matches.value_of(options::FILE) { - None | Some("-") => Box::new(BufReader::new(stdin())), - Some(filename) => Box::new(BufReader::new(File::open(filename).unwrap())), - }; - let mut buffer = [0; 1024]; - - let mut term = setup_term(); - - let mut end = false; - while let Ok(sz) = f.read(&mut buffer) { - if sz == 0 { - break; - } - stdout().write_all(&buffer[0..sz]).unwrap(); - for byte in std::io::stdin().bytes() { - match byte.unwrap() { - b' ' => break, - b'q' | 27 => { - end = true; - break; - } - _ => (), - } - } - - if end { - break; - } + // Print everything and quit if line count is less than the availale rows + if line_count < rows { + println!("{}", buff); + return; } - reset_term(&mut term); - println!(); + let mut upper_mark = 0; + // Number of lines left + let mut lines_left = line_count - (upper_mark + rows); + // Are we on the very last page and the next down arrow will return this function + let mut to_be_done = false; + draw( + &mut upper_mark, + rows, + &mut stdout, + lines.clone(), + line_count, + ); + + loop { + if event::poll(Duration::from_millis(10)).unwrap() { + match event::read().unwrap() { + Event::Key(KeyEvent { + code: KeyCode::Char('q'), + modifiers: KeyModifiers::NONE, + }) + | Event::Key(KeyEvent { + code: KeyCode::Char('c'), + modifiers: KeyModifiers::CONTROL, + }) => { + reset_term(&mut stdout); + std::process::exit(0); + } + Event::Key(KeyEvent { + code: KeyCode::Down, + modifiers: KeyModifiers::NONE, + }) + | Event::Key(KeyEvent { + code: KeyCode::Char(' '), + modifiers: KeyModifiers::NONE, + }) => { + // If this is the last page but some text + // is still left that does not require a page of its own + // then this event will print the left lines + if lines_left < rows && !to_be_done { + for l in lines.iter().skip((line_count - upper_mark - rows).into()) { + stdout.write_all(format!("\r{}\n", l).as_bytes()).unwrap(); + } + make_prompt_and_flush(&mut stdout, line_count, line_count); + // If this is not the last input file + // do not return, but the next down arrow must return + // because we have printed everyhing + if !is_last { + to_be_done = true; + } else { + // Else quit + reset_term(&mut stdout); + return; + } + // This handles the next arrow key to quit + } else if lines_left < rows && to_be_done { + return; + } else { + // Print a normal page + upper_mark = upper_mark.saturating_add(rows.saturating_sub(1)); + lines_left = line_count.saturating_sub(upper_mark + rows); + draw( + &mut upper_mark, + rows, + &mut stdout, + lines.clone(), + line_count, + ); + } + } + Event::Key(KeyEvent { + code: KeyCode::Up, + modifiers: KeyModifiers::NONE, + }) => { + upper_mark = upper_mark.saturating_sub(rows.saturating_sub(1)); + draw( + &mut upper_mark, + rows, + &mut stdout, + lines.clone(), + line_count, + ); + } + _ => continue, + } + } + } +} + +fn draw( + upper_mark: &mut u16, + rows: u16, + mut stdout: &mut std::io::Stdout, + lines: Vec, + lc: u16, +) { + execute!(stdout, terminal::Clear(terminal::ClearType::CurrentLine)).unwrap(); + let (up_mark, lower_mark) = calc_range(*upper_mark, rows, lc); + // Reduce the row by 1 for the prompt + let displayed_lines = lines + .iter() + .skip(up_mark.into()) + .take(usize::from(rows.saturating_sub(1))); + + for line in displayed_lines { + stdout + .write_all(format!("\r{}\n", line).as_bytes()) + .unwrap(); + } + make_prompt_and_flush(&mut stdout, lower_mark, lc); + *upper_mark = up_mark; +} + +// Break the lines on the cols of the terminal +fn break_buff(buff: &str, cols: usize) -> Vec { + let mut lines = Vec::new(); + + for l in buff.lines() { + lines.append(&mut break_line(l, cols)); + } + lines +} + +fn break_line(mut line: &str, cols: usize) -> Vec { + let breaks = (line.len() / cols).saturating_add(1); + let mut lines = Vec::with_capacity(breaks); + if line.len() < cols { + lines.push(line.to_string()); + return lines; + } + + for _ in 1..=breaks { + let (line1, line2) = line.split_at(cols); + lines.push(line1.to_string()); + if line2.len() < cols { + lines.push(line2.to_string()); + break; + } + line = line2; + } + lines +} + +// Calculate upper_mark based on certain parameters +fn calc_range(mut upper_mark: u16, rows: u16, line_count: u16) -> (u16, u16) { + let mut lower_mark = upper_mark.saturating_add(rows); + + if lower_mark >= line_count { + upper_mark = line_count.saturating_sub(rows); + lower_mark = line_count; + } else { + lower_mark = lower_mark.saturating_sub(1) + } + (upper_mark, lower_mark) +} + +// Make a prompt similar to original more +fn make_prompt_and_flush(stdout: &mut Stdout, lower_mark: u16, lc: u16) { + write!( + stdout, + "\r{}--More--({}%){}", + Attribute::Reverse, + ((lower_mark as f64 / lc as f64) * 100.0).round() as u16, + Attribute::Reset + ) + .unwrap(); + stdout.flush().unwrap(); +} + +#[cfg(test)] +mod tests { + use super::{break_line, calc_range}; + + // It is good to test the above functions + #[test] + fn test_calc_range() { + assert_eq!((0, 24), calc_range(0, 25, 100)); + assert_eq!((50, 74), calc_range(50, 25, 100)); + assert_eq!((75, 100), calc_range(85, 25, 100)); + } + #[test] + fn test_break_lines_long() { + let mut test_string = String::with_capacity(100); + for _ in 0..200 { + test_string.push('#'); + } + + let lines = break_line(&test_string, 80); + + assert_eq!( + (80, 80, 40), + (lines[0].len(), lines[1].len(), lines[2].len()) + ); + } + + #[test] + fn test_break_lines_short() { + let mut test_string = String::with_capacity(100); + for _ in 0..20 { + test_string.push('#'); + } + + let lines = break_line(&test_string, 80); + + assert_eq!(20, lines[0].len()); + } } diff --git a/tests/by-util/test_more.rs b/tests/by-util/test_more.rs index 9245733ca..cc778f0b4 100644 --- a/tests/by-util/test_more.rs +++ b/tests/by-util/test_more.rs @@ -2,15 +2,22 @@ use crate::common::util::*; #[test] fn test_more_no_arg() { - // stderr = more: Reading from stdin isn't supported yet. - new_ucmd!().fails(); + // Reading from stdin is now supported, so this must succeed + if atty::is(atty::Stream::Stdout) { + new_ucmd!().succeeds(); + } else {} } #[test] fn test_more_dir_arg() { - let result = new_ucmd!().arg(".").run(); - result.failure(); - const EXPECTED_ERROR_MESSAGE: &str = - "more: '.' is a directory.\nTry 'more --help' for more information."; - assert_eq!(result.stderr_str().trim(), EXPECTED_ERROR_MESSAGE); + // Run the test only if there's a valud terminal, else do nothing + // Maybe we could capture the error, i.e. "Device not found" in that case + // but I am leaving this for later + if atty::is(atty::Stream::Stdout) { + let result = new_ucmd!().arg(".").run(); + result.failure(); + const EXPECTED_ERROR_MESSAGE: &str = + "more: '.' is a directory.\nTry 'more --help' for more information."; + assert_eq!(result.stderr_str().trim(), EXPECTED_ERROR_MESSAGE); + } else {} }