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

Writing multiple attributes in one line? #5

Closed
Boscop opened this issue Apr 20, 2018 · 9 comments · Fixed by #49
Closed

Writing multiple attributes in one line? #5

Boscop opened this issue Apr 20, 2018 · 9 comments · Fixed by #49

Comments

@Boscop
Copy link
Collaborator

Boscop commented Apr 20, 2018

It would be nice to be able to write multiple getset attributes in one line, to reduce verbosity, but when I do (e.g. #[get = "pub", set = "pub"]) the proc-macro panics..

Can you make it work, please? :)

@Hoverbear
Copy link
Collaborator

Yes I'd like this. :)

@Hoverbear
Copy link
Collaborator

According to The Reference Manual the grammer for attributes currently doesn't support #[a, b, c] to denote multiple attributes.

We could work around this possibly by using something like #[foobar(get, set, get_mut], but this would be a departure from the current style. So adding this feature would break compatability.

I'm also not sure if the resulting API would be an improvement.

I'm going to mark this help wanted as I/we need some help figuring out if, and how, this should be done.

@Boscop
Copy link
Collaborator Author

Boscop commented Apr 22, 2018

I think it would be desirable to allow listing all getters/setters for a field in one line :)

@Hoverbear
Copy link
Collaborator

Yeah, I totally agree. Just need to think of the best way to do it! 🤔

@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Jan 13, 2020

Current proposal is a no go

According to rust-lang/rust#55208, this is simple impossible since, even though attribute's syntax was relaxed a bit (a lot), it still requires attributes to correspond to this grammar:

PATH
PATH `(` TOKEN_STREAM `)`
PATH `[` TOKEN_STREAM `]`
PATH `{` TOKEN_STREAM `}`
PATH = TOKEN_TREE

In other words, the first path must be followed (optionally) by one of {, (, [, =.

Alternative proposal

Support both forms:

// old form
#[get]
#[set]
#[whatever]

// new form
#[getset(get, set, whatever)]

It's pretty easy to disambiguate them, no breaking change is needed. Not to mention that almost every other derive macro out there uses #[name(parameters)] form.

To be clear, unlike #32 and #35 I'm not proposing to remove or deprecate old style attrs, I'm proposing to add a new alternative syntax. Before you ask, this won't complicate the code much, I am full of resolve to prove it :)

I would be willing to send such a PR.

@Hoverbear
Copy link
Collaborator

@CreepySkeleton It sounds good to me. :) Please feel encouraged to open a PR, let me know if I can help with anything!

@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Jan 15, 2020

@Hoverbear could you please review #47 and #48 first so I can stack the upcoming work on top of them?

@Hoverbear
Copy link
Collaborator

Yes just give me a day or so, travelling from China. :)

@Hoverbear
Copy link
Collaborator

@CreepySkeleton Those looked great. :) Thanks so much for the stuff so far! Sorry about the delays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants