bincode-org / virtue

A sinless derive helper
MIT License
48 stars 10 forks source link

Give ParsedAttribute::Property a Token rather than a Literal? #24

Open mkj opened 2 years ago

mkj commented 2 years ago

Thanks for virtue, it works nicely for my purposes parsing SSH binary protocol. I'm using enum variant attributes that can have Idents in them such as #[sshwire(variant = SSH_NAME_ED25519)], where the SSH_NAME_ED25519 is a const &'static str. That means ParsedAttribute won't work since it only allows a Literal value.

Would it make sense to allow any TokenTree for Property, rather than just a Literal? (I guess Punct wouldn't make sense).

VictorKoenders commented 2 years ago

I'm up for changing ParsedAttribute ::Property. Some options I see:

I do feel like there is a better way that virtue could do derive attribute parsing, but I can't quite put my finger on it. If you have suggestions for a major overhaul please let me know

mkj commented 2 years ago

I wonder if it would make more sense for it to be something like parse_tagged_attributes(prefix: &str, atts: &[Attributes]) -> HashMap<Ident, Vec<ParsedAttributeValue>> rather than having to group and iterate over attributes? That would hide the relative ordering of attributes, I'm not sure if that matters?

Or it could even drop the returned Vec if repeated tags aren't expected?

For Property I think just a IdentProperty is probably fine - I've realised a TokenTree wouldn't be enough anyway if there were something like #[sshwire(variant = SSH_NAME_ED25519.to_uppercase()] unless it wrapped the value part in a Group

kythyria commented 1 year ago

The other missing combination is ParsedAttribute::LiteralTag(Literal). I found myself wanting to parse #[endpoint("/some/where", paginated=offset)].