meithecatte/enumflags2

0.6 "roadmap"

Closed this issue · 9 comments

meithecatte commented

aka "things I want to do before releasing 0.6":

  • Merge all PRs, obviously.
  • Move the RawBitFlags trait to _internals.
  • Reexport the proc-macro from the main crate.
    • Update documentation to discourage direct dependencies on on enumflags2_derive.
    • Is there a way to warn about this? Perhaps put internal in the macro name and then remove it when reexporting? #[deprecated] and #[allow(deprecated)] when reexporting? (does that even work?)
    • Update examples.
  • Implement TryFrom as outlined below
arcnmx commented

A suggestion/question, since the minimum rustc version is being bumped with this release: any interest in implementing the TryFrom fallible conversion traits?

Is there a way to warn about this? Perhaps put internal in the macro name and then remove it when reexporting?

Hm not actually sure, can you rename with #[macro_export()]? pub use macro as something; doesn't even work in 2018 (yet?) :(

Seems overkill maybe but you could always deprecate the proc-macro crate and release the new one under a different name.

meithecatte commented

A suggestion/question, since the minimum rustc version is being bumped with this release: any interest in implementing the TryFrom fallible conversion traits?

I'm not sure what you're suggesting here? TryFrom<repr_int>? TryFrom<BitFlags<T>> for T?

arcnmx commented

TryFrom<T::Type> for BitFlags<T> in other words a Result variant of BitFlags::from_bits with a FromBitsError (which could also do any number of things like return (truncated, invalid) bits, tell you which bits were invalid, etc).

It helps to support the conversion traits where we can since we don't impl From (could have it truncate but that could be surprising behaviour so).

meithecatte commented

Seems overkill maybe but you could always deprecate the proc-macro crate and release the new one under a different name.

Actually, the crates.io name and the source code name don't have to be the same. The phenomenon is most commonly seen with hyphens vs underscores, but it's not limited to that.

This could be an opportunity/excuse to drop the 2 in enumflags2 from everywhere but crates.io (and users' Cargo.tomls).

Is there a way to warn about this? Perhaps put internal in the macro name and then remove it when reexporting?

Hm not actually sure, can you rename with #[macro_export()]? pub use macro as something; doesn't even work in 2018 (yet?) :(

It seems to work in 2015, though. See the reexport-derive-internal-in-name branch.

arcnmx commented

This could be an opportunity/excuse to drop the 2 in enumflags2 from everywhere but crates.io (and users' Cargo.tomls).

Ooh yeah that would be nice.

It seems to work in 2015, though. See the reexport-derive-internal-in-name branch.

Oh nice! I recall having issues with this in the past, but maybe that was for macro_rules! macros, and it maybe works for proc macros because the path also happens to resolve as a function?

meithecatte commented

This could be an opportunity/excuse to drop the 2 in enumflags2 from everywhere but crates.io (and users' Cargo.tomls).

Ooh yeah that would be nice.

What do you think about naming the macro itself BitFlags? I might do these two changes, but I'm not quite sure about the potential cons here.

arcnmx commented

What do you think about naming the macro itself BitFlags?

No real opinion here. Derive macros are usually named after the trait they impl, which makes RawBitFlags the appropriate choice? But since that's intentionally not a public API, shortening to BitFlags seems reasonable enough, particularly because of the interaction with the actual BitFlags struct. shrugs

I might do these two changes, but I'm not quite sure about the potential cons here.

The only cons I really see here are user confusion? Cargo isn't good at guiding people to release notes or migration guides, but... If you bump the version and everything breaks, we just need to make it not too hard to figure out what to change. I would suggest having a small migration guide of some sort, or at least mention of how to deal with the rename. README example/intro should probably include a small disclaimer highlighting the mismatch between the crate name in Cargo.toml and in source code.

Specifically for renaming the macro, we already have enumflags::BitFlags as a type so making the macro match that doesn't seem like a big deal. Despite the name not matching the crate, and let's imagine the bitflags crate one day switches to a proc-macro of the same name, it seems unlikely that one would run into the issue where they want to import a different BitFlags derive into the same module.

arcnmx commented

Also regarding TryFrom, I tried and failed. Although I like the idea and the error type (might be worth including regardless? at least then it's easy to wrap it for cases when you do actually need a TryFrom impl), I'm not sure if there's any way to get around needing to choose between providing only one of either From<T> or TryFrom<T::Type> as a blanket impl?

Nevermind I didn't think to just, use the numeric types manually since we already know the set of types that impl BitFlagNum. Blanket impls not working with TryFrom is just a general limitation of the trait (and I think is why it took so long to get stabilized?). Will PR shortly for feedback.

meithecatte commented