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

Fix signature by adding parens when needed #42318

Merged
merged 1 commit into from
May 31, 2017

Conversation

GuillaumeGomez
Copy link
Member

Fixes #42299.

@rust-highfive
Copy link
Collaborator

r? @frewsxcv

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

@frewsxcv frewsxcv assigned nrc and unassigned frewsxcv May 30, 2017
@frewsxcv frewsxcv added the T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. label May 30, 2017
@QuietMisdreavus
Copy link
Member

Looks good, though I think you'll wind up racing #42286 to the merge since you both mess with the signature of fmt_type.

@frewsxcv
Copy link
Member

@bors delegate=QuietMisdreavus

@bors
Copy link
Contributor

bors commented May 30, 2017

✌️ @QuietMisdreavus can now approve this pull request

@QuietMisdreavus
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 30, 2017

📌 Commit 171d285 has been approved by QuietMisdreavus

@GuillaumeGomez
Copy link
Member Author

I'll improve the priority a bit then.

@bors: p=1

@@ -788,14 +797,14 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,
_ => {
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to add a case for ResolvedPath here to render the parenthesis rather than passing bool parameters everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure it'd be better. But don't hesitate to do it if you think it can be improved.

@@ -534,7 +535,7 @@ fn resolved_path(w: &mut fmt::Formatter, did: DefId, path: &clean::Path,
} else {
format!("{}", HRef::new(did, &last.name))
};
write!(w, "{}{}", path, last.params)?;
write!(w, "{}{}{}", if need_paren { "(" } else { "" }, path, last.params)?;
Copy link
Member

Choose a reason for hiding this comment

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

This is missing the print_all and w.alternate() cases above although that won't be an issue if you render the parenthesis in fmt_type itself like I suggest.

@bors
Copy link
Contributor

bors commented May 31, 2017

⌛ Testing commit 171d285 with merge fd7b44b...

bors added a commit that referenced this pull request May 31, 2017
…isdreavus

Fix signature by adding parens when needed

Fixes #42299.
@bors
Copy link
Contributor

bors commented May 31, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing fd7b44b to master...

@bors bors merged commit 171d285 into rust-lang:master May 31, 2017
@GuillaumeGomez GuillaumeGomez deleted the rustdoc-fix-signature branch May 31, 2017 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants