JelteF / derive_more

Some more derive(Trait) options
MIT License
1.73k stars 123 forks source link

Support _variant in outer level enum formatting for Display #377

Closed JelteF closed 3 months ago

JelteF commented 4 months ago

Resolves #142 Resolves #239

Synopsis

This adds back support for top-level format strings of the Display derive. It now includes the display of the variant whenever the _variant placeholder apears be found. It also supports using the field

Solution

This does not include the same support for Debug, since it is considered much less useful there and the differences between the Debug implementation and Display implementation make it a non-trivial port.

Only named arguments are supported in the format string, no positional ones. This made the implementation easier, maybe in a future PR positional support can be added.

This bumps MSRV to 1.70.0, on earlier versions the derived code throws the following error:

error: there is no argument named `_0`
    --> tests/display.rs:1373:26
     |
1373 |                 #[derive(Display)]
     |                          ^^^^^^^
     |
     = note: did you intend to capture a variable `_0` from the surrounding scope?
     = note: to avoid ambiguity, `format_args!` cannot capture variables when the format string is expanded from a macro
     = note: this error originates in the derive macro `Display` (in Nightly builds, run with -Z macro-backtrace for more info)

Checklist

JelteF commented 4 months ago

Still needs some cleanup, error-ing on edge cases, and updated docs, but the general design seems to work.

JelteF commented 4 months ago

Still needs some cleanup, error-ing on edge cases, and updated docs, but the general design seems to work.

Okay all done now, this is ready for review.

tyranron commented 3 months ago

@JelteF I did the re-implementation, now it seems fine. This restriction is not necessary, actually, and by doing some deeper inspection of the shared formatting attribute we may lift it in the future. This especially may be useful for the use-cases where we want to add some special modifiers (for octals, etc) for all the variants. I haven't done it to omit complicating this PR and to not slowing the 1.0 release, as it requires more time and very rich test suite.

The only thing I want to add before merging this PR is to extend tests suite a little bit more. Will do it shortly.

tyranron commented 3 months ago

@JelteF if you don't have any more suggestions on this, I'm ready to merge this one.