Added sizes and alignments to WASI docs #268
Conversation
There was a problem hiding this comment.
Hey @torch2424! Looks good! One thing only. Do you think we could get rid of the HTML links for union layout bullets?
tools/witx/src/docs/ast.rs
Outdated
| let sizes_heading = heading_from_node(&node, 1); | ||
| node.new_child(MdSection::new(sizes_heading, "Union Layout")); | ||
| let union_layout = &self.union_layout(); | ||
| node.new_child(MdNamedType::new( |
There was a problem hiding this comment.
If you used a generic MdSection instead of an MdNamedType, then it won鈥檛 be HTML tagged by default.
|
@kubkon Thank you very much for the review! 馃槃 馃憤 I went ahead and made your requested changes. This should be good to go 馃槃
|
There was a problem hiding this comment.
Awesome, thanks @torch2424!
Since this PR affects only the witx tooling, I'm gonna go ahead and merge this. cc @sunfishcode
|
On second thought, I'll wait and see what @sunfishcode thinks since it does affect the docs, so I'm not sure whether it requires chairs approval or not ;-) |
tools/witx/src/docs/ast.rs
Outdated
| @@ -26,7 +27,13 @@ impl ToMarkdown for Document { | |||
| heading.new_level_down(), | |||
| name, | |||
| name, | |||
| &d.docs, | |||
| format!( | |||
| "{}Size: {}\n Alignment: {}", | |||
There was a problem hiding this comment.
Is this space before the "Alignment:" intended? It seems to make the spacing inconsistent with "Size:" and "Offset:".
tools/witx/src/docs/ast.rs
Outdated
| @@ -184,7 +193,7 @@ impl ToMarkdown for StructDatatype { | |||
| MdHeading::new_bullet(), | |||
| id.as_str(), | |||
| name, | |||
| &member.docs, | |||
| format!("{}Offset: {}", &member.docs, &offset).as_str(), | |||
There was a problem hiding this comment.
Would it make sense to add a newline after the offset here? Before this change, it looks like there's a blank line at that point, which creates a nice visual separation of different documentation items.
tools/witx/src/docs/ast.rs
Outdated
| @@ -26,7 +27,13 @@ impl ToMarkdown for Document { | |||
| heading.new_level_down(), | |||
| name, | |||
| name, | |||
| &d.docs, | |||
| format!( | |||
| "{}Size: {}\n Alignment: {}", | |||
There was a problem hiding this comment.
Also, similar to the "Offset:", should there be a trailing newline here?
|
@sunfishcode @kubkon Thanks for the review / approval! 馃槃 @sunfishcode Nice catch on the whitespace stuff! My hack-y markdown to html thing made the Whitespace look a lot different than what Github was rendering. Fixing these now 馃槃 |
|
Found a whitespace I think that looks good in Github. Let me know what you think 馃槃
|
closes #266
This adds documentation for all the sizes, alignments, and offset of data types, structs, and unions for
.witxdocuments. 馃槃 馃憤cc @pchickey for the review 馃槃
Screenshot
*I was using a local md to html converter to view the doc, which is why it looks a little funny 馃槃 *