more handle errors with multiple files #4997
Conversation
|
GNU testsuite comparison: |
There was a problem hiding this comment.
Hi! I've been a little absent from this project for a while, so that's why I didn't see this sooner.
I like the idea here! We should definitely improve this case.
If I try this patch, I get the following:
::::::::::::::
oranda.json
::::::::::::::
more: 'src' is a directory.
::::::::::::::
oranda.json
::::::::::::::
{
"project": {
"name": "uutils coreutils"
...
That seems a bit strange and like there's something going on with the printing. I've made a comment with some functions that might help here.
less and util-linux more both print the error message and then wait for user input. This makes sense because the next file might obscure the message. However, for a first version we don't have to do that yet.
But i have to say i stumbled upon something i need to ask about: is there a reason the more function returns an error with an exit code?
Nope, it'd be great if you could fix that! Thanks for finding that!
src/uu/more/src/more.rs
Outdated
| 1, | ||
| format!("{} is a directory.", file.quote()), | ||
| )); | ||
| if length == 1 { |
There was a problem hiding this comment.
I'd like to suggest using the show! and map_err_context functionality:
- https://docs.rs/uucore/latest/uucore/macro.show.html
- https://docs.rs/uucore/latest/uucore/error/trait.FromIo.html
I think those would make it unnecessary to check the number of files and to log the error messages in the buffer.
There was a problem hiding this comment.
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)
|
my terminal currently gives me the broken output i associate with the raw mode not being disabled. but i cant seem to find where the program could exit without disabling it first. and every test case i tried doesnt replicate the terminal not disabling the raw mode. that cargo test doesnt seem to execute the tests in a predictable order doesnt help with finding the case that leaves the terminal in raw mode... any keywords/ideas on how i can find the reason for that? |
|
GNU testsuite comparison: |
| 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) => { | ||
| terminal::disable_raw_mode().unwrap(); |
There was a problem hiding this comment.
I don't actually know why it stays in raw mode, but it seems unrelated to this change, so let's merge it!
* more handle errors with multiple files * tests/more test refactor and new case * tests/more new cases * more: use show! and change exitstatus and adjust tests to new exitvalue --------- Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>
added handling for case mentioned in (#4906) and added some tests as well.
But i have to say i stumbled upon something i need to ask about: is there a reason the more function returns an error with an exit code? cause when i compare:
is that an issue or smth that that doesn't need to be changed?
i tried to keep it this way but if multiple files are given it will always return no error. is this okay? or how should more handle one file failing and one working?