Allow opting-out of the macro-generated Display implementation

[korrat]

Thank you so much for this very useful library!

After using it for a while in some projects, I found one thing that bothers me a bit. For some errors, I would like to have the opportunity to implement Display myself. Currently, I get a compile error when trying that. Perhaps, the syntax could be #[snafu(display(false))].

As a motivating example, consider the following error, with the current display attribute.

#[derive(Debug, Snafu)]
#[snafu(display("failed to extract from record{}", if let Some(position) = position {
    format!(" {}", position.clone().line())
} else {
    String::new()
}))]
struct ExtractionError {
    position: Option<Position>,

    source: ExtractionErrorKind,
}

Since position is not guaranteed to exist, I cannot simply format the field. Instead, I have an intermediate allocation to format the field. With a custom Display implementation, formatting would be no problem.

Alternatives

I see two major alternatives, but I'm open to any other suggestions:

1. Avoid deriving Snafu for ExtractionError and implement the required traits myself. This would work, but goes against why I'm using Snafu in the first place.
2. Implement a newtype with a custom Display implementation for the field. Again, this would work, but is a bit of additional effort, which I would like to avoid. However, if custom display implementations remain unavailable with Snafu, I will probably go down that road.

[shepmaster]

I can see your point of view, and I think something akin to display(false) would make sense.

As a third avenue:

#[derive(Debug, Snafu)]
enum ExtractionError {
    #[snafu(display("failed to extract from record {position}"))]
    Thing1 {
        source: ExtractionErrorKind,
        position: i32,
    },

    #[snafu(display("failed to extract from record"))]
    Thing2 {
        source: ExtractionErrorKind,
    }
}

It's hard to tell exactly why position would be present or not though, so this may not be useful.

[8573]

I cannot simply format the field. Instead, I have an intermediate allocation to format the field.

In this case, can you use format_args! to avoid the allocation?

[shepmaster]

can you use format_args!

I had the same idea 😉. Unfortunately, the implementation of format_args is aggressive about references and you basically can never "return" it:

fn main() {
    let a_value = 42;

    let a = {
        format_args!("{a_value}")
    };

    println!("{a}");
}

error[E0716]: temporary value dropped while borrowed
 --> src/main.rs:5:9
  |
5 |         format_args!("{a_value}")
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^ creates a temporary which is freed while still in use
6 |     };
  |      - temporary value is freed at the end of this statement
7 |     
8 |     println!("{a}");
  |                - borrow later used here
  |
  = note: consider using a `let` binding to create a longer lived value
  = note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)```

[korrat]

can you use format_args!

I had the same idea 😉. Unfortunately, the implementation of format_args is aggressive about references and you basically can never "return" it:

I ran into the same issue when trying this at first. Sorry, I should have mentioned that.

It's hard to tell exactly why position would be present or not though, so this may not be useful.

Thanks, that nudged me in the right direction. Position here is from the csv crate. csv::StringRecord::position returns an Option<Position>, since you can create your own records in memory, which do not have a position in a file (yet). But in my case, I'm working on parsing CSV files, so the position should always exist. I'm unwrapping the position for now, so I will know if that expectation turns out to be false.

Nonetheless, I would vote to keep this issue open, since there are probably other cases where custom Display implementations would help.

I've also found another workaround: by implementing the expected format in Debug, you can use display("{:?}", self) to get the expected format. The biggest drawback is, that it ties your Display to your debug representations, which you might not want.

[shepmaster]

since there are probably other cases where custom Display implementations would help.

Certainly. In fact, there’s been low level chatter about having some kind of derive(Display) in the standard library. If that ever actually came to exist, I could see people preferring to use that over what SNAFU provides.

I'm unwrapping the position for now

I think that unwrapping is the correct decision for that particular piece of data, but amusingly you could also create another error variant for the position itself being missing and use that instead of unwrapping.

implementing the expected format in Debug

That’s an interesting idea, but I’d probably go with your shim type instead, just to avoid the confusion.

[felinira]

I am using gettext to translate errors to user facing error messages in their preferred language. It would be necessary to be able to implement Display for this.

[shepmaster]

to translate errors

would you mind chiming in on #254? If possible, I’d rather like to extend SNAFU to interact nicely with such functionality

[felinira]

As gettext can't parse attribute macros and I am also drilling very deeply into some errors this isn't really feasible. I might just work around it by implementing a separate method that creates a string.