fix: remove anyhow feature flag from OptionExt location test

[LeoniePhiline]

1. Clarify trait usage in location test

8522f02

Both WrapErr and ContextCompat expose anyhow compatibility methods.

• ContextCompat is completely feature gated behind the anyhow flag.

• WrapErr, on the other hand, feature-gates individual functions behind the anyhow flag.

This has led to confusion in the past.

This change moves the use eyre::WrapErr statement from the top of the module into each individual test, as identically named functions (wrap_err.*) are exposed both by WrapErr and ContextCompat.

With use eyre::WrapErr moved directly into the tests, it becomes clear which trait-provided fn is being called.

2. Remove anyhow feature flag from OptionExt location test

68744f1

This bug was introduced (by me) in 34bd1d9#diff-1ff47dac6cf55e34ff587968c5b1f1ec6b6ae39d2668a66ecba3633163a21fc5R86 - partly due to the confusion fixed with the above commit.

Fixes #147

[LeoniePhiline]

I don't think this requires a changelog entry.

[ten3roberts]

I see, good catch.

I recently learnt that conditional trait methods are a backwards compatability nightmare (though WrapErr is sealed thankfully), as there was a recent wasmtime discussion regarding failure to compile because a trait implemented new methods another crate didn't implement under a certain feature flag combiniation.

A better option would be to move these to a supertrait or ContextCompat, and make trait implement for all errors to expose the same behavior as present, without footguns.

[LeoniePhiline]

That would be an entirely different changeset, though, right?

[ten3roberts]

Yes, just a proposition, as it would alleviate the confusion with two identically named methods from two different traits.

From what I see in the current source code ContextCompat is only implemented for Option, while WrapErr has the context method for errors.

If you are willing to do this, or want me to do this, just let me know.

It is a larger change, but would alleviate some of the confusion and import confusion for downstream users

[LeoniePhiline]

My intention was to work on #136 - I only discovered #147 when starting to look into #136.

If you have time, I'd be glad if you could implement your proposition. I am not entirely sure if I fully see your vision in my mind.

[ten3roberts]

Got my idea here: #150

I feel I need to clarify that this discussion is more about 8522f02 than the removal of the feature gate of the OptionExt trait.

Just doing what I can to clear out any confusion and make eyre as consistent as possible while we can.

In short, we move the context* methods to ContextCompat and implement it for Options and Results, to make all anyhow compatibility methods reside in one trait rather than two. This has the downside currently of wrap_err not being exposed for Options, but this just leans into the semantics of wrap an error with another higher level error, which does not make much sense for an Option.

When the anyhow feature drops from default features, errors and anyhow+options will be more self-contained and not lead to the same wrap_err in ContextCompat and WrapErr, and context from ContextCompat and WrapErr traits anymore.

Let me know what you think of this.

[thenorili]

LGTM -- good cleanup

[ten3roberts]

Thank you for your work Leonie.

Sorry for leaving this hanging, had a rough time lately, but I am back now 🎉

[LeoniePhiline]

Please do not worry at all! Your wellbeing is always more important. <3