From c545ff77ade190bf06efe90417f6f2678df7c00c Mon Sep 17 00:00:00 2001 From: tpeters Date: Thu, 22 Jun 2023 18:08:05 +0200 Subject: [PATCH 1/4] more handle errors with multiple files --- src/uu/more/src/more.rs | 54 +++++++++++++++++++++++++++----------- tests/by-util/test_more.rs | 16 +++++++++++ 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/uu/more/src/more.rs b/src/uu/more/src/more.rs index a43489566c5..32afce9fcef 100644 --- a/src/uu/more/src/more.rs +++ b/src/uu/more/src/more.rs @@ -102,38 +102,60 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { while let (Some(file), next_file) = (files_iter.next(), files_iter.peek()) { let file = Path::new(file); if file.is_dir() { - terminal::disable_raw_mode().unwrap(); - return Err(UUsageError::new( - 1, - format!("{} is a directory.", file.quote()), - )); + if length == 1 { + terminal::disable_raw_mode().unwrap(); + return Err(UUsageError::new( + 1, + format!("{} is a directory.", file.quote()), + )); + } + buff.push_str(&format!("more: {} is a directory.\n", file.quote())); + continue; } if !file.exists() { - terminal::disable_raw_mode().unwrap(); - return Err(USimpleError::new( - 1, - format!("cannot open {}: No such file or directory", file.quote()), + if length == 1 { + terminal::disable_raw_mode().unwrap(); + return Err(USimpleError::new( + 1, + format!("cannot open {}: No such file or directory", file.quote()), + )); + } + buff.push_str(&format!( + "more: cannot open {}: No such file or directory\n", + file.quote() )); - } - if length > 1 { - buff.push_str(&MULTI_FILE_TOP_PROMPT.replace("{}", file.to_str().unwrap())); + continue; } let opened_file = match File::open(file) { Err(why) => { - terminal::disable_raw_mode().unwrap(); - return Err(USimpleError::new( - 1, - format!("cannot open {}: {}", file.quote(), why.kind()), + if length == 1 { + terminal::disable_raw_mode().unwrap(); + return Err(USimpleError::new( + 1, + format!("cannot open {}: {}", file.quote(), why.kind()), + )); + } + buff.push_str(&format!( + "more: cannot open {}: {}\n", + file.quote(), + why.kind() )); + continue; } Ok(opened_file) => opened_file, }; + if length > 1 { + buff.push_str(&MULTI_FILE_TOP_PROMPT.replace("{}", file.to_str().unwrap())); + } let mut reader = BufReader::new(opened_file); reader.read_to_string(&mut buff).unwrap(); more(&buff, &mut stdout, next_file.copied(), &options)?; buff.clear(); } reset_term(&mut stdout); + if !buff.is_empty() { + print!("{}", buff); + } } else if !std::io::stdin().is_terminal() { stdin().read_to_string(&mut buff).unwrap(); let mut stdout = setup_term(); diff --git a/tests/by-util/test_more.rs b/tests/by-util/test_more.rs index 95a4818b529..5a894d24775 100644 --- a/tests/by-util/test_more.rs +++ b/tests/by-util/test_more.rs @@ -67,3 +67,19 @@ fn test_more_invalid_file_perms() { .stderr_contains("permission denied"); } } + +#[test] +fn test_more_error_on_multiple_files() { + if std::io::stdout().is_terminal() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir_all("folder"); + at.make_file("file1"); + ucmd.arg("folder") + .arg("file2") + .arg("file1") + .succeeds() + .stdout_contains("folder") + .stdout_contains("file2") + .stdout_contains("file1"); + } +} From 41c1cc9f20e1b5afe0d0e710a5e8d87bc5a95df3 Mon Sep 17 00:00:00 2001 From: tpeters Date: Thu, 22 Jun 2023 18:36:09 +0200 Subject: [PATCH 2/4] tests/more test refactor and new case --- tests/by-util/test_more.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/by-util/test_more.rs b/tests/by-util/test_more.rs index 5a894d24775..3720af97c9c 100644 --- a/tests/by-util/test_more.rs +++ b/tests/by-util/test_more.rs @@ -71,15 +71,22 @@ fn test_more_invalid_file_perms() { #[test] fn test_more_error_on_multiple_files() { if std::io::stdout().is_terminal() { - let (at, mut ucmd) = at_and_ucmd!(); - at.mkdir_all("folder"); - at.make_file("file1"); - ucmd.arg("folder") + let ts = TestScenario::new("more"); + ts.fixtures.mkdir_all("folder"); + ts.fixtures.make_file("file1"); + ts.ucmd() + .arg("folder") .arg("file2") .arg("file1") .succeeds() .stdout_contains("folder") .stdout_contains("file2") .stdout_contains("file1"); + ts.ucmd() + .arg("file2") + .arg("file3") + .succeeds() + .stdout_contains("file2") + .stdout_contains("file3"); } } From afabed960717ac8e82ec9cc109bbef6a731caaf7 Mon Sep 17 00:00:00 2001 From: tpeters Date: Tue, 26 Sep 2023 23:33:27 +0200 Subject: [PATCH 3/4] tests/more new cases --- tests/by-util/test_more.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/by-util/test_more.rs b/tests/by-util/test_more.rs index 1b03b76ad49..978c2465f5c 100644 --- a/tests/by-util/test_more.rs +++ b/tests/by-util/test_more.rs @@ -114,6 +114,22 @@ fn test_more_invalid_file_perms() { } } +#[test] +fn test_more_error_on_single_arg() { + if std::io::stdout().is_terminal() { + let ts = TestScenario::new("more"); + ts.fixtures.mkdir_all("folder"); + ts.ucmd() + .arg("folder") + .fails() + .stderr_contains("is a directory"); + ts.ucmd() + .arg("file1") + .fails() + .stderr_contains("No such file or directory"); + } +} + #[test] fn test_more_error_on_multiple_files() { if std::io::stdout().is_terminal() { From afc207669d6d855a659283ef1bf63d1b57dda62c Mon Sep 17 00:00:00 2001 From: tpeters Date: Sun, 8 Oct 2023 07:07:32 +0200 Subject: [PATCH 4/4] more: use show! and change exitstatus and adjust tests to new exitvalue --- src/uu/more/src/more.rs | 53 ++++++++++++-------------------------- tests/by-util/test_more.rs | 19 +++++++------- 2 files changed, 26 insertions(+), 46 deletions(-) diff --git a/src/uu/more/src/more.rs b/src/uu/more/src/more.rs index 35a6b6ef954..9c3bef9f26f 100644 --- a/src/uu/more/src/more.rs +++ b/src/uu/more/src/more.rs @@ -24,8 +24,8 @@ use crossterm::{ use unicode_segmentation::UnicodeSegmentation; use unicode_width::UnicodeWidthStr; -use uucore::display::Quotable; use uucore::error::{UResult, USimpleError, UUsageError}; +use uucore::{display::Quotable, show}; use uucore::{format_usage, help_about, help_usage}; const ABOUT: &str = help_about!("more.md"); @@ -105,51 +105,35 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { while let (Some(file), next_file) = (files_iter.next(), files_iter.peek()) { let file = Path::new(file); if file.is_dir() { - if length == 1 { - terminal::disable_raw_mode().unwrap(); - return Err(UUsageError::new( - 1, - format!("{} is a directory.", file.quote()), - )); - } - buff.push_str(&format!("more: {} is a directory.\n", file.quote())); + terminal::disable_raw_mode().unwrap(); + show!(UUsageError::new( + 0, + format!("{} is a directory.", file.quote()), + )); + terminal::enable_raw_mode().unwrap(); continue; } if !file.exists() { - if length == 1 { - terminal::disable_raw_mode().unwrap(); - return Err(USimpleError::new( - 1, - format!("cannot open {}: No such file or directory", file.quote()), - )); - } - buff.push_str(&format!( - "more: cannot open {}: No such file or directory\n", - file.quote() + terminal::disable_raw_mode().unwrap(); + show!(USimpleError::new( + 0, + format!("cannot open {}: No such file or directory", file.quote()), )); + terminal::enable_raw_mode().unwrap(); continue; } let opened_file = match File::open(file) { Err(why) => { - if length == 1 { - terminal::disable_raw_mode().unwrap(); - return Err(USimpleError::new( - 1, - format!("cannot open {}: {}", file.quote(), why.kind()), - )); - } - buff.push_str(&format!( - "more: cannot open {}: {}\n", - file.quote(), - why.kind() + terminal::disable_raw_mode().unwrap(); + show!(USimpleError::new( + 0, + format!("cannot open {}: {}", file.quote(), why.kind()), )); + terminal::enable_raw_mode().unwrap(); continue; } Ok(opened_file) => opened_file, }; - if length > 1 { - buff.push_str(&MULTI_FILE_TOP_PROMPT.replace("{}", file.to_str().unwrap())); - } let mut reader = BufReader::new(opened_file); reader.read_to_string(&mut buff).unwrap(); more( @@ -163,9 +147,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { buff.clear(); } reset_term(&mut stdout); - if !buff.is_empty() { - print!("{}", buff); - } } else if !std::io::stdin().is_terminal() { stdin().read_to_string(&mut buff).unwrap(); let mut stdout = setup_term(); diff --git a/tests/by-util/test_more.rs b/tests/by-util/test_more.rs index 978c2465f5c..3e9c426ad67 100644 --- a/tests/by-util/test_more.rs +++ b/tests/by-util/test_more.rs @@ -91,8 +91,8 @@ fn test_more_dir_arg() { if std::io::stdout().is_terminal() { new_ucmd!() .arg(".") - .fails() - .usage_error("'.' is a directory."); + .succeeds() + .stderr_contains("'.' is a directory."); } } @@ -108,8 +108,7 @@ fn test_more_invalid_file_perms() { at.make_file("invalid-perms.txt"); set_permissions(at.plus("invalid-perms.txt"), permissions).unwrap(); ucmd.arg("invalid-perms.txt") - .fails() - .code_is(1) + .succeeds() .stderr_contains("permission denied"); } } @@ -121,11 +120,11 @@ fn test_more_error_on_single_arg() { ts.fixtures.mkdir_all("folder"); ts.ucmd() .arg("folder") - .fails() + .succeeds() .stderr_contains("is a directory"); ts.ucmd() .arg("file1") - .fails() + .succeeds() .stderr_contains("No such file or directory"); } } @@ -141,14 +140,14 @@ fn test_more_error_on_multiple_files() { .arg("file2") .arg("file1") .succeeds() - .stdout_contains("folder") - .stdout_contains("file2") + .stderr_contains("folder") + .stderr_contains("file2") .stdout_contains("file1"); ts.ucmd() .arg("file2") .arg("file3") .succeeds() - .stdout_contains("file2") - .stdout_contains("file3"); + .stderr_contains("file2") + .stderr_contains("file3"); } }