Do not use `Error::source` in `fmt::Display` implementation

[woojiq]

What I mean by this title is that if I want to manually print the entire error stack using the Error::source method, I get an error message twice. Example:

cannot parse argument "213": The length of hex string must be 6
The length of hex string must be 6

Code I use to unwind an error:

let args = args::parse_cli_args().unwrap_or_else(|err| {
    let mut err: Box<&(dyn std::error::Error)> = Box::new(&err);
    loop {
        eprintln!("{}", err);
        if let Some(source) = err.source() {
            err = Box::new(source);
        } else {
            break;
        }
    }
    std::process::exit(1);
});

I understand that the idea behind "lexopt" is to be as simple as possible with no fuss. Therefore, I suggest not to change the current behavior much. We can move the current behavior to the "{:#}" formatter and use "{}" to print a single error message (to be able to print the full error manually). That is how anyhow does this¹. But it will be breaking changes (or not?). What do you think? If you want to see it too, I can create a PR.

rust-lang/project-error-handling#23

For now, the recommendation is to always do one or the other. If you're going to print your sources, you should not return them via the source function and vice versa.

Cheers.

Footnotes

1. https://docs.rs/anyhow/latest/anyhow/struct.Error.html#display-representations ↩

[blyxxyz]

I'm worried that this would make error messages worse for currently existing applications that only use Display. And it would do it in such a way that they might not realize it when upgrading. What are your thoughts on removing the source implementation instead?

I'll have to do a survey of existing code as well as some general research.

[woojiq]

And it would do it in such a way that they might not realize it when upgrading.

Agree. Although bump version (0.3.0 -> 0.4.0) and mention it in the changelog and I think it will be ok.

What are your thoughts on removing the source implementation instead?

It's not the worst option in the short term, but overall it doesn't look good because we could have an already composite error that lexopt takes and we lose information.