3 ms·

I think the `expect()` bad examples are something of a strawman. The `.expect()` for regex for example would say what the regex is matching for. I think it'd

I think the `expect()` bad examples are something of a strawman. The `.expect()` for regex for example would say what the regex is matching for. I think it'd be desirable to have a `.unwrap_with_context("Context: {}")`, and the you'd get `Context: Inner Panic Info`.

burntsushi 4y ago
So you're saying that the 'expect()' message when a regex compilation error occurs should be a translation from a terse domain specific language to bloviating prose? :-) What 'expect()' message would you write for this regex? https://github.com/BurntSushi/ucd-generate/blob/6d3aae3b8005bd707e683b6518a684ab6b74dc4e/ucd-parse/src/unicode_data.rs#L124 https://github.com/BurntSushi/ucd-generate/blob/6d3aae3b8005... I think 'unwrap()' there is perfectly appropriate. > I think it'd be desirable to have a `.unwrap_with_context("Context: {}")`, and the you'd get `Context: Inner Panic Info`. Why?
- richardwhiuk 4y ago
  .expect("UnicodeData::from_str regex failed to compile") With this, even with out backtrace, you can work out what happened. Without it, you just know that some regex somewhere is invalid.
  - burntsushi 4y ago
    Have you ever seen a Regex::new(..).unwrap() fail? It sounds like maybe not. It also sounds like you haven't seen an 'unwrap()' fail either. > Without it, you just know that some regex somewhere is invalid. That's bologna. As I discuss in the blog post, 'unwrap()' tells you the line number at which it panicked. There's even an example showing exactly this. There's even another example showing what happens when you call 'Regex::new(..).unwrap()' and it fails[1]: fn main() { regex::Regex::new(r"foo\p{glyph}bar").unwrap(); } And running it: $ cargo run Finished dev [unoptimized + debuginfo] target(s) in 0.00s Running `target/debug/rust-panic` thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Syntax( ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ regex parse error: foo\p{glyph}bar ^^^^^^^^^ error: Unicode property not found ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ )', main.rs:4:36 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrac So you get the line number (as is standard with any 'unwrap()') and you get the actual error message from the regex. No need to even enable the backtrace. You just don't need 'expect()' here. [1]: https://blog.burntsushi.net/unwrap/#why-not-use-expect-instead-of-unwrap https://blog.burntsushi.net/unwrap/#why-not-use-expect-inste...
- richardwhiuk 4y ago
  Just to be clear, I agree with everything else you've said, and you've produced some awesome Rust libraries.
  - burntsushi 4y ago
    :-)
xvedejas 4y ago
When you unwrap/expect on an error, the inner error value is already printed. Are you suggesting something more? Reference the implementation details from the article: impl<T, E: std::fmt::Debug> Result<T, E> { pub fn unwrap(self) -> T { match self { Ok(t) => t, Err(e) => panic!("called `Result::unwrap()` on an `Err` value: {:?}", e), } } }
vincenv 4y ago
You can get that with anyhow [1] by calling unwrap after `.with_context`. [1]: https://docs.rs/anyhow/latest/anyhow/ https://docs.rs/anyhow/latest/anyhow/

burntsushi 4y ago

So you're saying that the 'expect()' message when a regex compilation error occurs should be a translation from a terse domain specific language to bloviating prose? :-) What 'expect()' message would you write for this regex? https://github.com/BurntSushi/ucd-generate/blob/6d3aae3b8005bd707e683b6518a684ab6b74dc4e/ucd-parse/src/unicode_data.rs#L124 https://github.com/BurntSushi/ucd-generate/blob/6d3aae3b8005... I think 'unwrap()' there is perfectly appropriate. > I think it'd be desirable to have a `.unwrap_with_context("Context: {}")`, and the you'd get `Context: Inner Panic Info`. Why?

richardwhiuk 4y ago
.expect("UnicodeData::from_str regex failed to compile") With this, even with out backtrace, you can work out what happened. Without it, you just know that some regex somewhere is invalid.
- burntsushi 4y ago
  Have you ever seen a Regex::new(..).unwrap() fail? It sounds like maybe not. It also sounds like you haven't seen an 'unwrap()' fail either. > Without it, you just know that some regex somewhere is invalid. That's bologna. As I discuss in the blog post, 'unwrap()' tells you the line number at which it panicked. There's even an example showing exactly this. There's even another example showing what happens when you call 'Regex::new(..).unwrap()' and it fails[1]: fn main() { regex::Regex::new(r"foo\p{glyph}bar").unwrap(); } And running it: $ cargo run Finished dev [unoptimized + debuginfo] target(s) in 0.00s Running `target/debug/rust-panic` thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Syntax( ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ regex parse error: foo\p{glyph}bar ^^^^^^^^^ error: Unicode property not found ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ )', main.rs:4:36 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrac So you get the line number (as is standard with any 'unwrap()') and you get the actual error message from the regex. No need to even enable the backtrace. You just don't need 'expect()' here. [1]: https://blog.burntsushi.net/unwrap/#why-not-use-expect-instead-of-unwrap https://blog.burntsushi.net/unwrap/#why-not-use-expect-inste...
richardwhiuk 4y ago
Just to be clear, I agree with everything else you've said, and you've produced some awesome Rust libraries.
- burntsushi 4y ago
  :-)

xvedejas 4y ago

When you unwrap/expect on an error, the inner error value is already printed. Are you suggesting something more? Reference the implementation details from the article: impl<T, E: std::fmt::Debug> Result<T, E> { pub fn unwrap(self) -> T { match self { Ok(t) => t, Err(e) => panic!("called `Result::unwrap()` on an `Err` value: {:?}", e), } } }

vincenv 4y ago

You can get that with anyhow [1] by calling unwrap after `.with_context`. [1]: https://docs.rs/anyhow/latest/anyhow/ https://docs.rs/anyhow/latest/anyhow/