-
Notifications
You must be signed in to change notification settings - Fork 45
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
Fix generation of docs on array members #447
Conversation
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.
Looks all right, only a minor note about comment formatting!
gen/cheader.tmpl
Outdated
@@ -339,8 +339,15 @@ typedef struct WGPU{{.Name | PascalCase}}{{$.ExtSuffix}} { | |||
WGPUChainedStructOut chain; | |||
{{- end}} | |||
{{- range $memberIndex, $_ := .Members}} | |||
{{- if (.Type | IsArray)}} | |||
/** Array count for `{{.Name | CamelCase}}`. The `INIT` macro sets this to 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.
Shouldn't we use MComment
here rather, or at least format the comment on multiple lines like all other docstrings? Saying this for the sake of consistency, though on the other hand we know by construction that this comment will be single line while other member comment blocks probably won't!
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 think I prefer it this way because it's more straightforward. MComment
and friends seem to be mostly needed because they can deal with multiline inputs (that usually come from the yaml file) whereas in this case I think it's more straightforward to just do the templating inline, otherwise I think I'd need to either add a new MCommentArrayCount
, or otherwise have it be a bit messy like:
{{- MComment (print "Array count for " (.Name | CamelCase) ". The `INIT` macro sets this to 0.") 4 }}
which doesn't seem that helpful (though TIL I can use print
which makes it possible at all). I do have conflicting feelings about whether I should use a 1-line doc comment or 3-line /**
*
*/
. I used 1-line because the useful documentation is on the other field, OTOH it feels very out of place. I think I will change it to 3-line.
MCommentArrayCount
is also a bit of a special case, because the pattern of splitting arrays into count+pointer is somewhat C-target-specific, though I'm sure there are some other targets that would use it, like C++ without std::span
.
I have very much been doing Whatever Works in this code generator, because I am new to Go and Go templating. @rajveermalviya is free to feel that I have made a mess in their code generator; I'm certain I could be doing things better. :)
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.
Landing now but happy to follow up on anything.
Fixes #431