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

Add pub(crate) support #566

Merged
merged 1 commit into from
Oct 6, 2018
Merged

Add pub(crate) support #566

merged 1 commit into from
Oct 6, 2018

Conversation

richard-uk1
Copy link
Contributor

Adds pub(crate) support where pub support is present in macros.rs.

@richard-uk1
Copy link
Contributor Author

Currently there are no extra tests - I'll see if I can add some.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 88.051% when pulling 34b8739 on derekdreery:pub_crate into 460041a on Geal:master.

1 similar comment
@coveralls
Copy link

coveralls commented Aug 22, 2017

Coverage Status

Coverage decreased (-0.03%) to 88.051% when pulling 34b8739 on derekdreery:pub_crate into 460041a on Geal:master.

@coveralls
Copy link

coveralls commented Aug 22, 2017

Coverage Status

Coverage increased (+0.02%) to 88.1% when pulling 34b8739 on derekdreery:pub_crate into 460041a on Geal:master.

@Geal
Copy link
Collaborator

Geal commented Aug 23, 2017

hi! apparently the test cannot compile on current stable. IIRC that feature is still experimental, right?

@richard-uk1
Copy link
Contributor Author

Aah I thought it was already stable. You are correct. Can I mark this PR as blocked by upstream until the syntax becomes stable?

@richard-uk1 richard-uk1 changed the title Add pub(crate) support [blocked(unstable)] Add pub(crate) support Aug 23, 2017
@richard-uk1
Copy link
Contributor Author

I've tested and it works on beta - not stable.. I think it was stabilized in 1.19, but maybe there are some changes in macros to support it in 1.20. Anyway, all tests pass in 1.20, so when that's released this should be mergeable.

@richard-uk1 richard-uk1 changed the title [blocked(unstable)] Add pub(crate) support [blocked(requres 1.20)] Add pub(crate) support Aug 23, 2017
@richard-uk1 richard-uk1 changed the title [blocked(requres 1.20)] Add pub(crate) support [blocked(requires 1.20)] Add pub(crate) support Aug 23, 2017
@richard-uk1 richard-uk1 changed the title [blocked(requires 1.20)] Add pub(crate) support [blocked(requires rust 1.20)] Add pub(crate) support Aug 23, 2017
@richard-uk1
Copy link
Contributor Author

richard-uk1 commented Aug 24, 2017

1.20 has been released now, so this should work

EDIT 1.20 is in beta.

The reason this will not build with 1.19 is not because pub(restricted) is missing (it isn't), but because there are bugs in the macro code that are fixed in 1.20.

@richard-uk1 richard-uk1 changed the title [blocked(requires rust 1.20)] Add pub(crate) support Add pub(crate) support Sep 5, 2017
@richard-uk1
Copy link
Contributor Author

Version 1.20 is out, so this should no longer be blocked. I need to trigger a ci reload.

@coveralls
Copy link

coveralls commented Sep 5, 2017

Coverage Status

Coverage increased (+0.02%) to 88.1% when pulling 1172238 on derekdreery:pub_crate into 460041a on Geal:master.

@richard-uk1
Copy link
Contributor Author

I think there's an unstable 'vis' identifier, that would make all these variants unnecessary

@coveralls
Copy link

coveralls commented Sep 6, 2017

Coverage Status

Coverage decreased (-0.1%) to 87.931% when pulling d384fba on derekdreery:pub_crate into 460041a on Geal:master.

@CAD97
Copy link
Contributor

CAD97 commented Dec 18, 2017

Tracking issue for :vis macro matcher

Seems to be almost en route to stabilization.

@Geal Geal force-pushed the master branch 3 times, most recently from 6c99077 to e7ca818 Compare May 14, 2018 12:06
@Geal Geal merged commit c7a79a9 into rust-bakery:master Oct 6, 2018
@Geal
Copy link
Collaborator

Geal commented Oct 6, 2018

Thank you! This is getting in nom at last :)

@richard-uk1
Copy link
Contributor Author

Note that this PR doesn't actually use :vis, it duplicates the code for the different varieties. the code could be reduced considerably by using the now-stabilized :vis macro ident type.

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

Successfully merging this pull request may close these issues.

4 participants