exec-rs icon indicating copy to clipboard operation
exec-rs copied to clipboard

exec() -> Result<Infallible, Error> ?

Open danya02 opened this issue 1 year ago • 3 comments

It's not very common to encounter functions that return if there's an error, and diverge if there is no error. When using this crate, this wasn't very intuitive for me, and I had to check a couple times that unconditionally printing the function's result is actually what I wanted to do.

How about changing the signature of exec and friends so that they return a Result, where the Ok variant is impossible (like std::convert::Infallible, or the never type once it's stabilized)?

Then, the following use clearly indicates that the condition where we remain in the program is exceptional:

match Command::new("echo").args("hello").exec() {
    Ok(_) => unreachable!(),
    Err(why) => eprintln!("Error execing the process: {why}"),
}

or, even shorter:

if Err(why) = Command::new("echo").args("hello").exec() {
    eprintln!("Error execing the process: {why}");
}

Also, unwrap()/expect() allows you to panic at the spot with the details, rather than adding custom formatting for this.

danya02 avatar Feb 18 '24 22:02 danya02

This is an interesting idea! I'm a little cautious about touching this crate much, because it has been static for a long time and people may be using it on very old Rust versions. The last release was 7 years ago.

But we'd need to bump the semver version, so requiring a more recent Rust might be OK, too.

emk avatar May 04 '24 20:05 emk

If supporting ancient versions of Rust (before 1.34.0, which was where it was stabilized, which was released in 2019) is a concern, you can implement your own Infallible: while the library version has a bunch of trait implementations, but you need basically none of them in order for it to work in the way I described: just a Clone+Copy derive is fine.

Speaking of which, I noticed that your current error type doesn't implement Clone when it could have easily done so. Though since nobody seemed to complain, maybe that's not an issue anyway. I wouldn't even know what to do with the error, anyway: an execve failing sounds like one of those errors where you can't do much except alert the human that their system is broken.

danya02 avatar May 05 '24 21:05 danya02

Btw, I noticed from #6 that Rust's standard library already has a method that returns only if there's an error, which is what I'm complaining about here. It was stabilized in Rust 1.9, which was released in 2016, just like this library (though a bit later), so I don't know whether to take this as proof that this is alright, or, conversely, that it's an outdated approach that should be changed (but stdlib cannot because of the stability guarantees).

danya02 avatar May 05 '24 21:05 danya02