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

Question: is it possible to use paren inside a prefix? #2

Open
ajuvercr opened this issue Apr 3, 2021 · 2 comments
Open

Question: is it possible to use paren inside a prefix? #2

ajuvercr opened this issue Apr 3, 2021 · 2 comments

Comments

@ajuvercr
Copy link

ajuvercr commented Apr 3, 2021

This crate looks super nice! But I have a question

I'm trying to do this

#[derive(Parse, Debug)]
struct Findable {
    #[prefix(kwd::findable)]
    #[paren]
    arg_paren: syn::token::Paren,
    #[prefix(kwd::ty in arg_paren)]
    #[prefix(syn::Token![=] in arg_paren)]
    #[inside(arg_paren)]
    ty: syn::Type,
}

It would seem logical to make it work like this

#[derive(Parse, Debug)]
struct Findable {
    #[prefix(kwd::findable)]
    #[prefix(syn::token::Paren as arg_paren)]
    #[prefix(kwd::ty in arg_paren)]
    #[prefix(syn::Token![=] in arg_paren)]
    #[inside(arg_paren)]
    ty: syn::Type,
}

Just because I don't want useless fields inside my struct :)

@sharnoff
Copy link
Owner

sharnoff commented Apr 5, 2021

Hey! Glad to hear you like it :)

I definitely get what you're saying with trying to avoid useless fields inside your struct - I've run into this particular case a couple times already.

This currently isn't a supported feature with #[prefix] -- it's certainly possible in theory to add special support, but isn't currently available because of the additional complication from the special parsing requirements of parentheses (and brackets / braces). The complications here are specifically because Paren and others don't implement Parse (because they have somewhat different semantics). Essentially: without adding new traits in this crate, it's unfortunately not possible to make it "just work".

That being said, it's definitely possible to add and I'd be happy to look at ways to do so if there's interest. A personal concern I have about adding additional features to #[prefix] is that it may make these sorts of structs more difficult to interpret - this is just my opinion though, so I'd love to hear what you think.


In terms of possible implementations if this functionality were to be added, there's multiple options. The most straightforward and flexible sort of way would be to allow the "normal" field attributes inside of #[prefix] -- something like #[prefix( #[paren] token::Paren as arg_paren )]. This somewhat conflicts with the #[prefix(Foo in bar)] syntax, but perhaps that's fine.

Another option would be to introduce some attribute like #[parenthesized], which includes all prefixes in it. So for your case, we'd write:

#[derive(Parse)]
struct Findable {
    #[prefix(kwd::findable)]
    #[parenthesized] // <- this indicates that the rest of the prefixes AND the field itself are inside an anonymous `Paren` token
    #[prefix(kwd::ty)]
    #[prefix(syn::Token![=])]
    ty: syn::Type,
}

I think this way of doing it would be a little less clear (I've previously avoided allowing earlier attributes to change the behavior of later ones), and it also duplicates the functionality already provided for prefixes inside a token tree, but I'm open to hearing other opinions.

For now, I'm currently ok with having some left-over fields in some cases - for the most part, I find it more readible, and I don't think it has too many negatives, especially because it's then less work to use those would-be-prefixed fields later!

@ajuvercr
Copy link
Author

ajuvercr commented Apr 6, 2021

I hear you, you have many valid points.
The #[parenthesized] looks really cool though, but it doesn't really work with your existing code, it look like. (Because it would have to be a NeighborAttr which is ugly)

Maybe another option, but less clear, is to add syntax to in to support something like #[prefix(kwd::ty in new paren arg_paren)], and later you can just use arg_paren normally.
I don't think you'd approve entirely :D

I'll give it a shot tonight and create a pr, which you then, without hard feelings, can reject.

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

No branches or pull requests

2 participants