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

Handle pub(restricted) #1013

Merged
merged 3 commits into from
May 27, 2016

Conversation

kamalmarhubi
Copy link
Contributor

This commit properly handles pub(restricted) as introduced in RFC 1422
[0]. The syntax support was added in #971, but they were not correctly
formatted.

[0] https://github.com/rust-lang/rfcs/blob/master/text/1422-pub-restricted.md

Fixes #970

This commit properly handles pub(restricted) as introduced in RFC 1422
[0]. The syntax support was added in rust-lang#971, but they were not correctly
formatted.

[0] https://github.com/rust-lang/rfcs/blob/master/text/1422-pub-restricted.md

Fixes rust-lang#970
@@ -1817,7 +1814,7 @@ fn rewrite_where_clause(context: &RewriteContext,
}

fn format_header(item_name: &str, ident: ast::Ident, vis: &ast::Visibility) -> Option<String> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can no longer fail, right? I guess it should return a String then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, and propagated up one more call to format_unit_struct :-)

@marcusklaas
Copy link
Contributor

This is a great pull request. Solid code, good test coverage and a nice application of Cow! I think we could use itertools' join in a few more spots, so the inclusion of that crate is useful too. I'll create an issue for that.

r+ with the two nits addressed

Thanks!

The change to `format_visibiilty` means that `format_header` and
`format_unit_struct` can no longer fail. Their return type is updated to
reflect that.
@kamalmarhubi
Copy link
Contributor Author

Thanks for the kind words! Re itertools, I am bringing it in in #1007 so I wasn't too bothered about using it here. Also they're all generic methods, so they won't increase code size except when used. It's a super handy crate!

@marcusklaas marcusklaas merged commit 66cac1f into rust-lang:master May 27, 2016
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.

2 participants