Skip to content

Add decorator syntax extensions on trait and impl items #25024

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 19 commits into from
May 13, 2015
Merged

Add decorator syntax extensions on trait and impl items #25024

merged 19 commits into from
May 13, 2015

Conversation

nrc
Copy link
Member

@nrc nrc commented May 1, 2015

No description provided.

aochagavia and others added 15 commits January 31, 2015 11:38
@aochagavia
Deprecate ItemModifier in favor of MultiItemModifier
53579e6 
[breaking-change]
@aochagavia
Deprecate ItemDecorator in favor of MultiItemDecorator
5769039 
[breaking-change]
@aochagavia
Add MultiItemDecorator
77d5921
@aochagavia
Add MultiDecorator variant to SyntaxExtension enum
f97cff9
@aochagavia
Deprecate SyntaxExtension::Decorator
6f233aa 
It has been replaced by SyntaxExtension::MultiDecorator

[breaking-change]
@aochagavia
Deprecate SyntaxExtension::Modifier
2e07b0d 
Replaced by SyntaxExtension::MultiModifier

[breaking-change]
@aochagavia
Add tests for MultiItemDecorator
01172ee
@aochagavia
Replace uses of Decorator and Modifier
aa3fe20
@aochagavia
Fix pretty printer
5af4df4
@aochagavia
Fix rebase issues
143f2db
@nrc
Merge branch 'syntax' of https://github.com/aochagavia/rust into muli…
0a62a05 
…t-decor

Conflicts:
	src/librustc/plugin/registry.rs
	src/libsyntax/ext/base.rs
	src/libsyntax/ext/cfg_attr.rs
	src/libsyntax/ext/deriving/mod.rs
	src/libsyntax/ext/expand.rs
	src/libsyntax/print/pprust.rs
	src/test/auxiliary/macro_crate_test.rs
@nrc
Rebasing and making MulitDecorators work
0a4f9a2
@nrc
WIP refactor expansion of decorators and move derive to MultiDecorator
c0a42ae
@nrc
Merge branch 'master' into mulit-decor
b2ddd93
@nrc
Get tests passing
aa5ca28
@rust-highfive
Copy link
Contributor

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@nrc nrc mentioned this pull request May 1, 2015
Closed
@alexcrichton
Copy link
Member

r? @sfackler or @huonw (also cc both)

@bors
Copy link
Collaborator

bors commented May 2, 2015

☔ The latest upstream changes (presumably #25027) made this pull request unmergeable. Please resolve the merge conflicts.

@nrc
Copy link
Member Author

nrc commented May 6, 2015

@sfackler review ping

@sfackler
Copy link
Member

sfackler commented May 7, 2015

I would not be opposed to just removing Modifier and Decorator to cut down on the duplicated code for the old/new cases, but r+ other than that.

@nrc
Copy link
Member Author

nrc commented May 7, 2015

@sfackler thanks for the review. My plan is to remove them after a month or so deprecation period - don't want to blind-side users too much.

@nrc
Copy link
Member Author

nrc commented May 12, 2015

@bors r=@sfackler 5d16772

@bors
Copy link
Collaborator

bors commented May 12, 2015

⌛ Testing commit 5d16772 with merge d1528da...

@bors
Copy link
Collaborator

bors commented May 12, 2015

💔 Test failed - auto-mac-64-opt

@nrc
Copy link
Member Author

nrc commented May 13, 2015

@bors r=sfackler c544e83

bors added a commit that referenced this pull request May 13, 2015
@bors
Auto merge of #25024 - nrc:mulit-decor, r=sfackler
dd9dcc1
@bors
Copy link
Collaborator

bors commented May 13, 2015

⌛ Testing commit c544e83 with merge dd9dcc1...

@bors
Copy link
Collaborator

bors commented May 13, 2015

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

@bors bors merged commit c544e83 into rust-lang:master May 13, 2015
@gsingh93
Copy link
Contributor

Would it be a good idea to just rename MultiItemModifer to ItemModifier instead of removing ItemModifier? I'm not really a fan of the name MultiItemModifier.

@nrc
Copy link
Member Author

nrc commented May 20, 2015

Probably. That would require another deprecation stage, but that's OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@sfackler sfackler

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@nrc @rust-highfive @alexcrichton @bors @sfackler @gsingh93 @aochagavia