eyre-rs / color-eyre

Custom hooks for colorful human oriented error reports via panics and the eyre crate
Other
958 stars 57 forks source link

owo_colors re-export makes code-completion hard to use (IntelliJ Rust) #109

Closed chklauser closed 2 years ago

chklauser commented 2 years ago

color-eyre lib.rs re-exports owo_colors. Because owo_colors defines a blanket implementation of its OwoColorize trait that covers pretty much all types, code completion will show the many many methods of OwoColorize on every code completion. This makes code completion much less useful for every crate that uses color-eyre 😒.

Example (IntelliJ Rust in CLion): image

(IntelliJ Rust intentionally offers methods for completion that aren't in scope yet. When such a method is used, the IDE automatically adds the necessary use. This is an incredibly useful IDE feature. )

Questions

  1. Is there a reason why color-eyre has to re-export owo_colors? As far as I can tell, no type of owo_colors shows up in the public API => users who wish to use owo_colors could define their own dependency.
  2. Would it maybe be possible to put the re-export behind a cargo feature? (Totally fine if it's enabled by default). πŸ‘‰ Sketch commit of what that could look like (excl. docs)

Possible workarounds

  1. Wrapper crate: Create a tiny "wrapper" crate in the project workspace that re-exports everything except owo_colors from color-eyre.

Thanks

I think that color-eyre is a really cool project πŸ˜„ It makes it easy to create CLI tools that communicate information effectively when it's arguably most useful: when things don't quite go according to plan πŸ˜‰ Thanks for sharing it with the world πŸ˜„πŸ‘

yaahc commented 2 years ago
  1. Is there a reason why color-eyre has to re-export owo_colors? As far as I can tell, no type of owo_colors shows up in the public API => users who wish to use owo_colors could define their own dependency.

the reasoning behind re-exporting owo-colors is because we use some of their types in our public API, and if you depend on multiple versions of the same crate you can end up with confusing errors around incompatible types. I try to avoid this by always re-exporting all dependencies which are part of my crate's public API so that users don't have to import those crates separately and can instead name the re-exported version and be 100% sure they're going to get the correct version that is used in our API.

2. Would it maybe be possible to put the re-export behind a cargo feature? (Totally fine if it's enabled by default). πŸ‘‰ Sketch commit of what that could look like (excl. docs)

I'm totally fine with merging this as a work around, though I do feel like the best fix would be upstream in IntelliJ Rust. I feel like maybe this is an indicator that certain cases of trait methods that aren't in scope shouldn't be completed, in this case because the trait impl has a blanket impl<T: Sized> OwoColorize for T {}. In practice owo-colors is not usable in all of these situations because the bounds that cause compilation errors are on the types that it produces, which will end up not implementing Display and thus doing nothing other than getting in the way. I'd be surprised if there are any traits that have a blanket impl for all T where it makes sense to complete them still even when the trait isn't in scope.

I think that color-eyre is a really cool project πŸ˜„ It makes it easy to create CLI tools that communicate information effectively when it's arguably most useful: when things don't quite go according to plan πŸ˜‰ Thanks for sharing it with the world πŸ˜„πŸ‘

Thank you 😊

chklauser commented 2 years ago

Hey, thanks for looking into this πŸ™‚

I totally agree that re-exporting makes sense when the types show up in the public API, and indeed they do (Theme). In that case, the optional feature should really include all of the APIs (e.g., custom themes) that expose the owo_colors types to avoid crate version mismatches.

But it seems awkward to carve out such a feature. I don't really want to impose changes on color-eyre just because some editor can't deal with a blanket impl. I only opened the issue because I mistakenly thought that owo_colors was not used in the public API πŸ˜“

=> I have filed an issue with IntelliJ Rust https://github.com/intellij-rust/intellij-rust/issues/8901