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

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

Open
danya02 opened this issue Feb 18, 2024 · 3 comments
Open

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

danya02 opened this issue Feb 18, 2024 · 3 comments

Comments

@danya02
Copy link

danya02 commented Feb 18, 2024

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.

@emk
Copy link
Contributor

emk commented May 4, 2024

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.

@danya02
Copy link
Author

danya02 commented May 5, 2024

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
Copy link
Author

danya02 commented May 5, 2024

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants