search

rust-cli / human-panic

Panic messages for humans.
https://docs.rs/human-panic
Apache License 2.0
1.66k stars 65 forks source link

Only Metadata<'static> works, remove generic #17

Closed remram44 closed 6 years ago

remram44 commented 6 years ago

Choose one: is this a 🐛 bug fix

Metadata<'a> is wrong, since only a==static will compile. This changes it for clarity.

Checklist

Context

N/A

Semver Changes

Changed Metadata is technically pub, though users of this library are unlikely to provide their own.

killercup commented 6 years ago

I mentioned this in @spacekookie's latest PR (#15).

@yoshuawuyts, let's talk about this a bit. Are all these fields actually always present? I'd make these fields private (of some type) and provide getters that return Option<&str> (and never ever return Some("")). (Also, I'd probably sprinkle Cows all over the code base, but that's another story.)

yoshuawuyts commented 6 years ago

Are all these fields actually always present?

Yeah, probably not. Good call to make it optional. Also allows us to create better reports, cleaning up #3.

I'd make these fields private (of some type) and provide getters that return Option<&str> (and never ever return Some("")).

Sounds reasonable. Most of the stringly typed API stuff was just to get something working. Agree this is cleaner.

(Also, I'd probably sprinkle Cows all over the code base, but that's another story.)

Cool! - never used it before; curious to learn more!

remram44 commented 6 years ago

Are all these fields actually always present? I'd make these fields private (of some type) and provide getters that return Option<&str> (and never ever return Some("")). (Also, I'd probably sprinkle Cows all over the code base, but that's another story.)

Funny you should mention this. I tried some of that too (aee0194) but it ended up kind of complicated.

I don't think there is much point for the user to override some of those fields. It is simply more convenient and more powerful to provide a new message for the terminal as a whole...

Re: making fields private, remember that the Metadata struct is built from the user's crate via the macro (though a constructor could be provided).

spacekookie commented 6 years ago

This was implemented in the end by #32