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 IntoInnerError::into_error() #304

Merged
merged 1 commit into from
Mar 6, 2023
Merged

Add IntoInnerError::into_error() #304

merged 1 commit into from
Mar 6, 2023

Conversation

Bravo555
Copy link
Contributor

@Bravo555 Bravo555 commented Mar 6, 2023

Currently, for code that doesn't attempt to recover from errors but just reports the error to the caller, it's forced to return the entire IntoInnerError, which is very big compared to io::Error which is where the actual error is.

This commit adds IntoInnerError::into_error() method that consumes IntoInnerError and returns contained io::Error. This mirrors the API of std::io::IntoInnerError in Rust stdlib, which provides such method.

Copy link
Owner

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This LGTM with a couple nits.

it's forced to return the entire IntoInnerError, which is very big compared to io::Error which is where the actual error is.

Note that an IntoInnerError is just a std::io::Error and a W. So if it's big, it's because your W is big. :-)

src/error.rs Outdated
@@ -312,6 +312,13 @@ impl<W> IntoInnerError<W> {
&self.err
}

/// Consumes the [`IntoInnerError`] and returns the error which caused the call to
/// [`Writer::into_inner()`] to fail. Unlike `error`, this can be used to
/// obtain ownership of the underlying error.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure the docs are wrapped to 79 columns, inclusive. (They don't currently appear to be.)

Also, please use Writer::into_inner and drop the parens.

And please use IntoInnerError::error instead of error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs wrapped, and I managed to link to Writer::into_iter() correctly this time.

Currently, for code that doesn't attempt to recover from errors but just
reports the error to the caller, it's forced to return the entire
`IntoInnerError`, which is very big compared to `io::Error` which is
where the actual error is.

This commit adds `IntoInnerError::into_error()` method that consumes
`IntoInnerError` and returns contained `io::Error`. This mirrors the API
of `std::io::IntoInnerError` in Rust stdlib, which provides such method.

Signed-off-by: Marcel Guzik <[email protected]>
Copy link
Owner

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@BurntSushi BurntSushi merged commit e3c663e into BurntSushi:master Mar 6, 2023
@BurntSushi
Copy link
Owner

This PR is in csv 1.2.1 on crates.io.

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

Successfully merging this pull request may close these issues.

None yet

2 participants