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

Add Termination support for the E side of Result<_, E> #1156

Closed
wants to merge 1 commit into from
Closed

Add Termination support for the E side of Result<_, E> #1156

wants to merge 1 commit into from

Conversation

SUPERCILEX
Copy link

🌟 What is the purpose of this PR?

The fact that Report implements Termination and is in the Err variant of Result is very confusing because the stdlib completely ignores the Err variant's termination, leading to the exit code being dropped. This PR adds a new type Exit that will return the ExitCode provided from the Report Termination. Without this PR, one must write:

        match result {
            Ok(()) => ExitCode::SUCCESS,
            Err(err) => {
                // Ignore error if the write fails, for example because stderr is
                // already closed. There is not much point panicking at this point.
                drop(writeln!(io::stderr(), "Error: {err:?}"));
                err.report()
            }
        }

🔗 Related links

rust-lang/rust#48711 (comment)

TODO

I need to write docs if this moves forward.

@CLAassistant
Copy link

CLAassistant commented Oct 4, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the area/libs > error-stack Affects the `error-stack` crate (library) label Oct 4, 2022
@Alfred-Mountfield
Copy link
Contributor

Thanks for the PR!
This is quite an intriguing one and we're not sure how to handle it if I'm honest. We definitely share the frustration, it greatly affects the ergonomics (and indeed even the value of Termination for error-stack). It seems that this is much more of an upstream problem though, and patching it here feels.. hacky.

We're not entirely against the proposal, but at the moment I think we'd lean towards not including it.

How would you feel about opening a GitHub discussion, and we can gauge if this is something other people feel strongly about, and in the mean time I think we'd close this PR off.

@SUPERCILEX
Copy link
Author

Yeah I just thought of this which seems better than having every library add a hack: rust-lang/rust#48711 (comment)

@SUPERCILEX SUPERCILEX closed this Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/libs > error-stack Affects the `error-stack` crate (library)
Development

Successfully merging this pull request may close these issues.

3 participants