Skip to content

Fix pretty-printing of empty impl items #9061

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

Merged
merged 2 commits into from Sep 12, 2013
Merged

Fix pretty-printing of empty impl items #9061

merged 2 commits into from Sep 12, 2013

Conversation

ghost
Copy link

@ghost ghost commented Sep 8, 2013

No description provided.


if methods.len() == 0 {
word(s.s, ";");
end(s); // end the head-ibox
end(s); // end the outer cbox
Copy link
Member

Choose a reason for hiding this comment

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

I'm personally very unfamiliar with this code, but looking at it, could you explain why there's two calls to end here? it looks like one of them is the complement of the above head call, and I was just wondering what the "outer cbox" was.

Copy link
Author

Choose a reason for hiding this comment

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

@alexcrichton My understanding of this is very limited too but I think head() opens two blocks (as can be seen in https://github.com/jakub-/rust/blob/7173b9d1b8475bf2b44e50b78501f45f98c76bc0/src/libsyntax/print/pprust.rs#L246).

impl Trait for T {
    fn foo() -> uint { 42 }
}

One spanning "impl Trait for T" (presumably so you can indent properly when you're wrapping a header that's too long?), and other one spanning the whole thing up to }.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me, thanks for the explanation!

@huonw
Copy link
Member

huonw commented Sep 10, 2013

I guess this will pretty-print impl Foo for Bar { } as impl Foo for Bar;? (It doesn't matter, just wondering.)

@ghost
Copy link
Author

ghost commented Sep 10, 2013

@huonw That is right, though that's what it used to before too. The problem was that in the case of an empty impl, it would end up at an incorrect indentation level.

@pnkfelix
Copy link
Member

The error flagged by bors here looks legitimate. Can you update your test case?

Use an existing type so that it compiles.
@ghost
Copy link
Author

ghost commented Sep 12, 2013

@pnkfelix Fixed, sorry about that! Didn't realize the pretty tests were compiled too.

@bors bors closed this Sep 12, 2013
@bors bors merged commit de79972 into rust-lang:master Sep 12, 2013
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