pathchk: use Results instead of writeln with stderr #5216
Conversation
|
Cool! I'm wondering whether these should be using our macros for printing with the util name though? Does GNU pathchk also write these without the util name? |
|
Good point, I will change it. |
|
@cakebaker ping ? :) |
cakebaker
force-pushed
the
pathchk_use_eprintln
branch
from
8876207
to
ac6b0f5
Compare
cakebaker
changed the title
|
Refactored it to use |
cakebaker
force-pushed
the
pathchk_use_eprintln
branch
from
ac6b0f5
to
6f2761b
Compare
src/uu/pathchk/src/pathchk.rs
Outdated
| for p in paths.unwrap() { | ||
| let mut path = Vec::new(); | ||
| for path_segment in p.split('/') { | ||
| path.push(path_segment.to_string()); | ||
| } | ||
| res &= check_path(&mode, &path); | ||
| check_path(&mode, &path)?; |
There was a problem hiding this comment.
This now short circuits if an error occurs, is that the intended behaviour? Because if I try multiple bad paths in GNU, I get:
❯ pathchk "" ""
pathchk: '': No such file or directory
pathchk: '': No such file or directory
so it looks like it should not short-circuit. I think we can use the show! macro here, which will set the error code.
Also we should open an issue for the string handling here. It should operate on Path and OsStr instead of String I think.
There was a problem hiding this comment.
Good catch, thanks. You also discovered a bug in our implementation because pathchk "" "" doesn't fail.
cakebaker
force-pushed
the
pathchk_use_eprintln
branch
from
6f2761b
to
e820ba4
Compare
sylvestre
force-pushed
the
pathchk_use_eprintln
branch
from
e820ba4
to
de5f8f2
Compare
|
@cakebaker doesn't build anymore, sorry |
|
no activity for a while, closing |
This PR uses
eprintln!instead of passingstderrtowriteln!. This makes it possible to remove#![allow(unused_must_use)].