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

witx: Parse comments into documentation in AST #140

Merged
merged 17 commits into from
Nov 12, 2019
Merged

Conversation

pchickey
Copy link
Contributor

@pchickey pchickey commented Nov 5, 2019

Proof of concept to show that https://github.com/alexcrichton/wat/pull/26/ is a viable approach

@pchickey
Copy link
Contributor Author

pchickey commented Nov 5, 2019

Todo:

  • Only parse doc comments (;;; line comments or (;; block comments ;;)) into docs; ignore other comments.
  • Trim whitespace
  • Parse interior of doc comments as Markdown?

@alexcrichton
Copy link
Contributor

👍

I whipped up a small render function which I think should do the trimming/doc comment parsing, and I think this looks like it might work well! I'll make a release of the wast crate with this support.

@pchickey pchickey marked this pull request as ready for review November 6, 2019 22:32
@pchickey
Copy link
Contributor Author

pchickey commented Nov 6, 2019

I've got this PR to a point where I think it should be reviewed and merged:

  • Doc comments are parsed at every node in the AST where they make sense to exist (imo). They are also output in the Render trait so that roundtrip testing still works. Unfortunately, whitespace doesn't roundtrip perfectly, and rather than come up with an elegant fix, I took the lazy way out and normalized all the whitespace in our witx files so that it will round trip.
  • A very rudimentary markdown output is implemented. This will need some polishing before we can replace the currently hand-written docs with automated docs. I'll spend more time on that in the future but I also wanted to get multi-module support done this week, so I want to move on from this PR without finishing that aspect. However, I think it is good enough to merge this code, in particular because the next point causes merge conflicts with essentially everything:
  • The existing comments in all our witx files have been converted to doc-comments. The sed script that does so is included in the commit message. A few places that mechanical conversion didn't work are fixed up manually.

@pchickey
Copy link
Contributor Author

pchickey commented Nov 7, 2019

Merge conflicts resolved. After we do a snapshot, I'll resolve the conflicts one more time and then merge.

@pchickey
Copy link
Contributor Author

Just rebased on #145 - will merge once that one is in

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

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

LGTM!

@pchickey pchickey merged commit acf3cf1 into master Nov 12, 2019
@pchickey pchickey deleted the pch/doc_comments branch November 12, 2019 00:54
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.

3 participants