search

PoignardAzur / venial

"A very small syn"
MIT License
196 stars 7 forks source link

Re-export missing type names #30

Closed Bromeon closed 2 years ago

Bromeon commented 2 years ago

Previously forgotten. Does it make sense to have

pub use types::*;

? This would have prevented that problem (and likely similar future ones). I would argue that if a type is public inside the types module, it's intended to be exported, otherwise we could use pub(crate). But maybe I'm missing something.

Bromeon commented 2 years ago

Btw, if you prefer if I directly merge PRs, I can also do that. I'll try to keep changes documented as good as possible, so you could always later reconstruct what happened and revert/amend if needed.

PoignardAzur commented 2 years ago

I'd prefer you leave PRs unmerged for a while. But if I don't give feedback for, say, a week, you can probably go ahead and merge.

PoignardAzur commented 2 years ago

Does it make sense to have

pub use types::*;

At this point, probably, yeah.

Bromeon commented 2 years ago

Uses now wildcard re-export.

This brought to light another missing symbol: InlineGenericArgs, which is referenced in docs.rs.

With it came also a missing panic documentation (see clippy run). I added it for now; there are likely quite a few more methods that could potentially fail to parse/convert tokens and thus panic.