Remove extra "ipnet" and "mac" features. Use implicit optional dependency feature instead

[faern]

Fixes the first thing that blocks #6.

I suggest adopting a changelog with the keepachangelog format, to record changes in. Will be easier for users to discover what they need to do when making subtle breaking changes like these.

[faern]

The tests (cargo test) do not pass for me locally. But they also don't pass on the main branch, and this repository has no CI so I ignored that for now. I don't think my code change broke anything new.

EDIT: This PR does actually break the tests. It's just that tests need to run with cargo test --all-features to pass on main. And they don't on this branch.. I have not used trybuild before. Have to figure out how to fix the tests I guess.

[faern]

If I add the features:

ipnetwork = ["dep:ipnetwork"]
macaddr = ["dep:macaddr"]

... Then it magically works. I don't know what's going on here. Is there some magic going on in trybuild?

[faern]

The magical documentation of features also broke with my first solution. This crates uses a lot of automagic I have not encountered before :D Well, now I think everything should work again!

[sophacles]

Oh wow, thanks for putting in a bunch of legwork here!

Re: magic - yes theres a lot of automagic involved, which I'm happy to start stripping out if it gets in the way of making the core functionality better.

For context: this project started out as a way to scratch an exercise in learning about macros by scratching an itch. At some point I realized someone else might find it useful and decided to clean up docs and publish the crate - but I wanted to have better docs and figured a test suite wouldn't hurt either, those magic crates came out of google-oriented-programming rather than a better "understand the things" approach 🫤 .

document-fatures crate: This is mostly just fluff and if it is getting in the way of using implicit dependencies rather than having to manually declare each, it can go away just fine.

trybuild: this one is a bit harder to drop as there needs to be something that can verify the results of good and bad macro invocation. Since all that happens during a compilation of other code it's a bit tricky and trybuild has all the machinery for that already.

feature declaration: The features were not deeply thought out, just the idea "some people may not want to have macaddr or ipnetwork pulled in and built since they aren't using them in their project", combined with the notion that there may be other cases of wanting to turn a string into a type with compile time error checking amplifying the first thought (e.g. parsing a url into the Url type, or alternate IP handling libraries, and so on). If implicit dependencies can be used to accomplish this and it makes everything simpler in the long run, I'm on board :).

---

Next steps here:

• I just learned about the implicit feature thing from your comment on Support both ipnet and ipnetwork. #6 so I'm going to spend some time wrapping my head around it.

• I'm going to play with this branch and spend some time with docs to understand the relationship between try-build, cfg-if and implicit/explicit feature declarations.

@faern If you have any thoughts on the above and suggestions for approaches that simplify future work while keeping user's lives simple as well, I'm all ears.

[faern]

I have now rebased this on latest main, added this change to the newly merged changelog, and fixed the commit message style!

[faern]

At some point I realized someone else might find it useful and decided to clean up docs and publish the crate

This is awesome! Thanks for contributing to the ecosystem ❤️

trybuild: this one is a bit harder to drop as there needs to be something that can verify the results

Of course. There is nothing wrong with having some more advanced custom test runner. I just don't understand why it won't work with optional dependency features 🤷

feature declaration: The features were not deeply thought out, just the idea "some people may not want to have macaddr or ipnetwork pulled in and built since they aren't using them in their project",

I 100% support this! I love when features are used to strip down the size of both the code in a library as well as the dependencies it pulls in!

[faern]

Rebased again and updated to work with the fix in #13.

[sophacles]

Looking into the implicit features thing some more, it seems like rust is moving towards getting rid of those anyway, in favor of explicit features defined like done here ( feat = [dep:feat]).

See: rust-lang/rfcs#3491 and the associated tracking issue: rust-lang/cargo#12826 .

I wonder if the trybuild folks were aware of this and decided to just no worry about implicit feature compat as a result (thus causing your difficulties above).

[faern]

I had no idea! TIL! But apparent it did not make it into Rust 2024 edition. So at least three years until this becomes a real thing. But since ipnetwork = ["dep:ipnetwork"] works, and is due to the accepted RFC maybe more future proof, I think we can merge this PR with the current solution.

[faern]

I see no benefit in naming the features anything other than the name of the crate they enable support for. It's the most direct and clear to users IMHO. Since a user must directly depend on ipnetwork/macaddr themselves it's easier to se the relationship if this crate has features named accordingly.

[sophacles]

I see no benefit in naming the features anything other than the name of the crate they enable support for. It's the most direct and clear to users IMHO.

My comment was just about why the implicit feature seems to have some odd behaviors over the explicitly defined ones. I completely agree that they should be named after the crate they enable.

I think we can merge this PR with the current solution.

I agree :)

[sophacles]

LGTM, merged