-
Notifications
You must be signed in to change notification settings - Fork 747
codegen: Make comments indentation-aware. #799
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
Conversation
r? @fitzgen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me with a couple nitpicks below
Thanks!
src/ir/item.rs
Outdated
ctx.options().conservative_inline_namespaces | ||
}) | ||
}) | ||
.fold(1, |i, _| i + 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this fold just <iterator>.count() + 1
? Better not to make readers verify that the fold is just counting, when there's a std method for counting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just moved code around, but sure :)
"/// hello\n/// world\n/// foo"); | ||
|
||
assert_eq!(preprocess("/**\nhello\n*world\n*foo\n*/".to_owned()), | ||
assert_eq!(preprocess("/**\nhello\n*world\n*foo\n*/", 0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to have a unit test with nonzero indent for comment preprocessing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a super-fan of unit tests, but will do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(err, for stuff that can be integration-tested, I mean)
This commit moves comment processing to a central place (well, two, because of field docs, but that's fine). Also, it makes comments indentation aware, so multiline comments don't appear garbled. Finally, it also fixes an out-of-bounds panic when processing an empty multiline comment.
@bors-servo r=fitzgen |
📌 Commit 96304f9 has been approved by |
codegen: Make comments indentation-aware. This commit moves comment processing to a central place (well, two, because of field docs, but that's fine). Also, it makes comments indentation aware, so multiline comments don't appear garbled. Finally, it also fixes an out-of-bounds panic when processing an empty multiline comment.
☀️ Test successful - status-travis |
This commit moves comment processing to a central place (well, two, because of
field docs, but that's fine).
Also, it makes comments indentation aware, so multiline comments don't appear
garbled.
Finally, it also fixes an out-of-bounds panic when processing an empty multiline
comment.