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 support for nested comments #9936

Merged
merged 1 commit into from
Oct 21, 2013
Merged

Add support for nested comments #9936

merged 1 commit into from
Oct 21, 2013

Conversation

madjar
Copy link
Contributor

@madjar madjar commented Oct 18, 2013

This should close #9468.

I removed the test stating that nested comments should not be implemented.

I had a little chicken-and-egg problem because a comment of the std contains "/*", and adding support for nested comment creates a backward incompatibility in that case, so I had to use a dirty hack to get stage1 and stage2 to compile. This part should be revert when this commit lands in a snapshot.

This is my first non-typo contribution, so I'm open to any comment.

@alexcrichton
Copy link
Member

Would you mind squashing these commits into one? Also feel free to ping reviewers on the pull request when you update it because sadly github doesn't notify anyone on a push to the branch.

@madjar
Copy link
Contributor Author

madjar commented Oct 20, 2013

@alexcrichton I wasn't sure I should do that, thanks for the instructions.

@brson Fixed what you said.

bors added a commit that referenced this pull request Oct 21, 2013
This should close #9468.

I removed the test stating that nested comments should not be implemented.

I had a little chicken-and-egg problem because a comment of the std contains "/*", and adding support for nested comment creates a backward incompatibility in that case, so I had to use a dirty hack to get stage1 and stage2 to compile. This part should be revert when this commit lands in a snapshot.

This is my first non-typo contribution, so I'm open to any comment.
@brson
Copy link
Contributor

brson commented Oct 21, 2013

@madjar the main function of the test case you moved to run-pass needs to be pub because of the way all tests are concatenated together on the windows bots.

@madjar
Copy link
Contributor Author

madjar commented Oct 21, 2013

@brson Thanks, I wouldn't have guessed it myself. This should be fixed (and squashed).

@alexcrichton
Copy link
Member

Hm, it looks like something mahy have gone awry, the test is still just fn main() {}

@madjar
Copy link
Contributor Author

madjar commented Oct 21, 2013

Silly me having troubles with git. This is now (really fixed).

bors added a commit that referenced this pull request Oct 21, 2013
This should close #9468.

I removed the test stating that nested comments should not be implemented.

I had a little chicken-and-egg problem because a comment of the std contains "/*", and adding support for nested comment creates a backward incompatibility in that case, so I had to use a dirty hack to get stage1 and stage2 to compile. This part should be revert when this commit lands in a snapshot.

This is my first non-typo contribution, so I'm open to any comment.
@bors bors closed this Oct 21, 2013
@bors bors merged commit 1dc3d0b into rust-lang:master Oct 21, 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.

Lex nested block comments
4 participants