Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

more handle errors with multiple files #4997

Merged
merged 6 commits into from
Feb 4, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 38 additions & 13 deletions src/uu/more/src/more.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,29 +105,51 @@
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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to suggest using the show! and map_err_context functionality:

I think those would make it unnecessary to check the number of files and to log the error messages in the buffer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well i was using the buffer so that i dont have to switch the raw mode for each message.. do i even have to do that, since the errormessages are printed to stderr and not stdout? sadly the documentation for the raw mode doesnt mention stderr..
(https://docs.rs/crossterm/0.27.0/crossterm/terminal/index.html#raw-mode)

terminal::disable_raw_mode().unwrap();
return Err(UUsageError::new(

Check warning on line 110 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L109-L110

Added lines #L109 - L110 were not covered by tests
Ludmuterol marked this conversation as resolved.
Show resolved Hide resolved
1,
format!("{} is a directory.", file.quote()),

Check warning on line 112 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L112

Added line #L112 was not covered by tests
));
}
buff.push_str(&format!("more: {} is a directory.\n", file.quote()));

Check warning on line 115 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L115

Added line #L115 was not covered by tests
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(

Check warning on line 121 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L120-L121

Added lines #L120 - L121 were not covered by tests
1,
format!("cannot open {}: No such file or directory", file.quote()),

Check warning on line 123 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L123

Added line #L123 was not covered by tests
));
}
buff.push_str(&format!(

Check warning on line 126 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L126

Added line #L126 was not covered by tests
"more: cannot open {}: No such file or directory\n",
file.quote()

Check warning on line 128 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L128

Added line #L128 was not covered by tests
));
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(

Check warning on line 136 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L135-L136

Added lines #L135 - L136 were not covered by tests
1,
format!("cannot open {}: {}", file.quote(), why.kind()),

Check warning on line 138 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L138

Added line #L138 was not covered by tests
));
}
buff.push_str(&format!(

Check warning on line 141 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L141

Added line #L141 was not covered by tests
"more: cannot open {}: {}\n",
file.quote(),
why.kind()

Check warning on line 144 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L143-L144

Added lines #L143 - L144 were not covered by tests
));
continue;
}
Ok(opened_file) => opened_file,
};
if length > 1 {
buff.push_str(&MULTI_FILE_TOP_PROMPT.replace("{}", file.to_str().unwrap()));

Check warning on line 151 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L151

Added line #L151 was not covered by tests
}
let mut reader = BufReader::new(opened_file);
reader.read_to_string(&mut buff).unwrap();
more(
Expand All @@ -141,6 +163,9 @@
buff.clear();
}
reset_term(&mut stdout);
if !buff.is_empty() {
print!("{}", buff);

Check warning on line 167 in src/uu/more/src/more.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/more/src/more.rs#L167

Added line #L167 was not covered by tests
}
} else if !std::io::stdin().is_terminal() {
stdin().read_to_string(&mut buff).unwrap();
let mut stdout = setup_term();
Expand Down
23 changes: 23 additions & 0 deletions tests/by-util/test_more.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,26 @@
.stderr_contains("permission denied");
}
}

#[test]
fn test_more_error_on_multiple_files() {
if std::io::stdout().is_terminal() {
let ts = TestScenario::new("more");
ts.fixtures.mkdir_all("folder");
ts.fixtures.make_file("file1");
ts.ucmd()

Check warning on line 123 in tests/by-util/test_more.rs

View check run for this annotation

Codecov / codecov/patch

tests/by-util/test_more.rs#L120-L123

Added lines #L120 - L123 were not covered by tests
.arg("folder")
.arg("file2")
.arg("file1")
.succeeds()
.stdout_contains("folder")
.stdout_contains("file2")
.stdout_contains("file1");
ts.ucmd()

Check warning on line 131 in tests/by-util/test_more.rs

View check run for this annotation

Codecov / codecov/patch

tests/by-util/test_more.rs#L130-L131

Added lines #L130 - L131 were not covered by tests
.arg("file2")
.arg("file3")
.succeeds()
.stdout_contains("file2")
.stdout_contains("file3");
}

Check warning on line 137 in tests/by-util/test_more.rs

View check run for this annotation

Codecov / codecov/patch

tests/by-util/test_more.rs#L136-L137

Added lines #L136 - L137 were not covered by tests
}
Loading