Debug impl changes

[arcnmx]

This is a common Debug implementation with a lot less codegen per enum required. You can now use format specifiers like {:02x?} to get BitFlags<T>(0x03, A | B), and custom Debug impls for the enum will be used in place of A or B if used.

The internal BitFlagsFmt trait has been removed and replaced with a bitflags_type_name() method to get the same old BitFlags<T>(...) behaviour. A consequence of this is that the enum types must now be explicitly annotated with #[derive(Debug)] in order for debug to work.

[arcnmx]

Yeah, that part is ugly and will need more explanation. The story is basically:

1. The relevant RFC that introduced debug hex formatters proposed an API for it
2. Implementation of that API was punted off with a fixme pointing to a now closed tracking issue
3. The PR implementing it states some of the reasons the API wasn't introduced

Which leaves the only decent stable way I know of determining the formatter used to be the flags() field with bits set by the discriminator of the FlagV1 enum.

[arcnmx]

Moved the strangeness into its own formatter struct so it's fairly isolated and documented. I wish it were possible to do things a bit better/neater, but the Formatter API is pretty inflexible. This is yet another case of std not dogfooding and just resorting to mutating private fields to achieve desired behaviour, which disadvantages external types since they obviously can't mimic the same approach.

Also implemented the binary/hex/etc. formatters for BitFlags, since it seems pretty reasonable to expect the bits to be explicitly formatted. I'm holding off on Display because I'm not sure there's much value in just formatting them as a decimal? I feel like this is actually a case where it'd be a good idea to bring back BitFlagsFmt as a public API to allow the downstream enum itself to impl Display however it wishes (which would just have a default impl to decimal formatting I guess).

[meithecatte]

I looked for a while, and I couldn't find any better way to implement this, so I guess the `#[allow(deprecated)]

I'm holding off on Display because I'm not sure there's much value in just formatting them as a decimal?

Not only is there little value in decimal output, but it's also most likely not the right choice for user-facing output.

I feel like this is actually a case where it'd be a good idea to bring back BitFlagsFmt as a public API to allow the downstream enum itself to impl Display however it wishes

I wouldn't really worry about this. You can't impl Display for Vec<MyType> and nobody complains.

Would you mind adding some unit tests for this functionality? (assert_eq!(format!(...), "...");) I'd put them as a mod tests right next to the actual formatting code.

[meithecatte]

The helper formatter structs really shouldn't be in this module. What it's for is only the BitFlagNum trait - it needs to be a part of public interface, but we don't want anyone implementing it on their own types.

[arcnmx]

Yeah, would you prefer...

1. their own private formatting module?
2. root of the crate above the Debug impl?
3. inside the debug fmt() function? (too indenty/messy imo)

Also possibly a public user-facing flag_fmt function or something on BitFlags? A | B may sometimes be useful to provide without the rest of the Debug output wrapping it.

[arcnmx]

Not only is there little value in decimal output, but it's also most likely not the right choice for user-facing output.

Likely not, though there are a few cases where having ToString (and a mirror FromStr) is useful for interacting with text interfaces/serialization? But yeah the context for what "user-facing" means is vague, for example one could imagine wanting a "r-x" encoding for file permission flags.

Another thought about BitFlagsFmt: we could also bring the debug_fmt function back with a default impl using FlagFormatter for the same reason. Though this PR enables customizing the individual Debug impl of each flag, it still bakes the " | " format in where there may be other more succinct developer-friendly representations. Supporting the {:#?} format specifier may be desired for the above rwx example etc.

I wouldn't really worry about this. You can't impl Display for Vec and nobody complains.

I'd argue the difference is that BitFlags<T> is effectively the public API for T (the singular flag type is only really for constructing or inspecting values), and thus it's reasonable to expect there to be a bit more interworking there. Many traits (like Binary/Hex/etc) have an obvious single implementation, but others like Display can depend on the type and it seems like it could be nice to at least enable customization where it's desired. Newtype wrappers are always an option, but cumbersome.

This is a general shortcoming of the design though; despite there often being want to, you can't really impl anything for BitFlags<T> when needed. We can offer common std traits, with obvious implementations or ugly BitFlagsX traits to fill in the gaps... But this approach doesn't scale well when combined with external types/traits, we can't have feature flags for every dependency in existence! (though I'd argue a serde flag would be a good addition, since there's an obvious single implementation for Serialize/Deserialize on BitFlags that doesn't care about T)

Would you mind adding some unit tests for this functionality? (assert_eq!(format!(...), "...");) I'd put them as a mod tests right next to the actual formatting code.

Yup!

[arcnmx]

Whee more changes:

• Changes to the debug binary formatter because I didn't realise {#08b} actually formats numbers with 6 zeroes, not 8 (the prefix counts as part of the length). This throws a bit of a wrench in the works in that there's no good way to pass along the Formatter configuration directly.
  • std impls get around this by mutating the width field before/after formatting something, which we obviously can't do.
  • I'm just branching on :x? vs :X? and supporting the width since format_args!() does allow it to be specified at runtime, and hoping no one cares about the other flags like alignment (does anyone really want space-padded middle-aligned numbers anyway??)
• Pretty-printing uses debug_struct instead of debug_tuple because pretty tuples are pretty ugly to read.
  • (I'm not opposed to just using structs in both cases, but I'm not convinced consistency is that important and we had the tuple before so)
• Formatting details moved to a private module with a few unit tests inside
• Tests against the actual BitFlags<T> fmt impls added to test suite

[meithecatte]

Good job! I don't see anything to improve here, making this perfect by at least one measure :)

By the way, would you like to add yourself to authors in Cargo.toml? Across the no_std support and this Debug implementation, you've done what feels like more work than I have since I started maintaining this crate.