Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(color-eyre): add export-owo-colors feature #188

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

0xAdk
Copy link

@0xAdk 0xAdk commented Jul 31, 2024

Reexporting owo-colors causes rust-analzyer's code completion to fill with many auto-use suggestions for the owo-colors::OwoColorize trait.

This feature allows users to opt-out of reexporting owo-colors.

see eyre-rs/color-eyre#109

@0xAdk
Copy link
Author

0xAdk commented Jul 31, 2024

Sorry since I don't use features often this didn't occur to me. Would this be a breaking change for anyone with default-features = false? Maybe a non-default no-export-owo-colors (I'm sure there is a better name) feature would be better.

@0xAdk 0xAdk changed the title feat(color-eyre): add default export-owo-colors feature feat(color-eyre): add no-export-owo-colors feature Jul 31, 2024
@HactarCE
Copy link

HactarCE commented Dec 4, 2024

Features are meant to be additive. Consider two crates, where the first one depends on those exports and the second one doesn't. If the second crate specifies no-export-owo-colors, then combining both crates into one project would cause the first crate to not compile because the no-export-owo-colors feature is active.

On the other hand, an export-owo-colors feature would work. If any crate in the dependency tree depends on it, it will be included in the final build.

Currently there are no default features, so there's no reason someone would have default_features = false for color_owo. It may still count as a breaking change, but it is unlikely to break code in practice.

I think export-owo-colors is definitely the way to go. For minimal breakage, add it as a default feature (although I think most users of this crate will not want it enabled).

Always reexporting `owo-colors` causes rust-analzyer's code completion to
fill with many auto-use suggestions for the `owo-colors::OwoColorize` trait.

This feature allows users to opt-out of reexporting `owo-colors`.
@0xAdk
Copy link
Author

0xAdk commented Dec 5, 2024

ok the feature is now additive.

Currently there are no default features, so there's no reason someone would have default_features = false for color_owo

I'm not sure what you mean. There is both track-caller and capture-spantrace already enabled by default.

@0xAdk 0xAdk changed the title feat(color-eyre): add no-export-owo-colors feature feat(color-eyre): add export-owo-colors feature Dec 5, 2024
@HactarCE
Copy link

HactarCE commented Dec 5, 2024

Ah, my bad. I guess I looked in the wrong place for feature documentation

@ten3roberts
Copy link
Contributor

I think export-owo-colors is definitely the way to go. For minimal breakage, add it as a default feature (although I think most users of this crate will not want it enabled).

Agreed,

Personally, I rarely export dependency crates unless they are extensively used in the external API, as the user can easily bring their own, rather than having two ways to import it. This is made somewhat easier with RA recent checks for [doc_hidden] attrs that prevents considering them for autoimport, but for many, it is not beneficial to always have access to, and accidentally auto-import red from color_eyre::owo_colors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants