-
Notifications
You must be signed in to change notification settings - Fork 70
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
Make ContextCompat
consistent and non-ambiguous with WrapErr
#150
Changes from 5 commits
a039517
2b32acd
90645ef
39ec865
4d6e73e
231090d
0eabf81
c98bc8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,12 +100,13 @@ fn test_option_ok_or_eyre() { | |
#[cfg(feature = "anyhow")] | ||
#[test] | ||
fn test_context() { | ||
use eyre::ContextCompat; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency with the other tests, this Or the existing Examples of |
||
|
||
let _ = eyre::set_hook(Box::new(|_e| { | ||
let expected_location = file!(); | ||
Box::new(LocationHandler::new(expected_location)) | ||
})); | ||
|
||
use eyre::WrapErr; | ||
let err = read_path("totally_fake_path") | ||
.context("oopsie") | ||
.unwrap_err(); | ||
|
@@ -117,12 +118,13 @@ fn test_context() { | |
#[cfg(feature = "anyhow")] | ||
#[test] | ||
fn test_with_context() { | ||
use eyre::ContextCompat; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above:
|
||
|
||
let _ = eyre::set_hook(Box::new(|_e| { | ||
let expected_location = file!(); | ||
Box::new(LocationHandler::new(expected_location)) | ||
})); | ||
|
||
use eyre::WrapErr; | ||
let err = read_path("totally_fake_path") | ||
.with_context(|| "oopsie") | ||
.unwrap_err(); | ||
|
@@ -140,7 +142,7 @@ fn test_option_compat_wrap_err() { | |
})); | ||
|
||
use eyre::ContextCompat; | ||
let err = None::<()>.wrap_err("oopsie").unwrap_err(); | ||
let err = None::<()>.context("oopsie").unwrap_err(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The encosing test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this is why consistent naming is so good, cause now that yields a duplicate definition error. Thanks a lot for your suggestions 😊 |
||
|
||
// should panic if the location isn't in our crate | ||
println!("{:?}", err); | ||
|
@@ -155,7 +157,7 @@ fn test_option_compat_wrap_err_with() { | |
})); | ||
|
||
use eyre::ContextCompat; | ||
let err = None::<()>.wrap_err_with(|| "oopsie").unwrap_err(); | ||
let err = None::<()>.with_context(|| "oopsie").unwrap_err(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This correctly adheres to our model now as you can not "wrap an error" over an option. You now either use the anyhow compatibility flag, or as the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The encosing test |
||
|
||
// should panic if the location isn't in our crate | ||
println!("{:?}", err); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit "concrete"