Navigation Menu

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

x/pkgsite/internal/fetch/dochtml/internal/render: first paragraph should not be considered as a potential section heading #41616

Closed
hajimehoshi opened this issue Sep 24, 2020 · 9 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. pkgsite/dochtml Issues related to package documentation in pkgsite pkgsite

Comments

@hajimehoshi
Copy link
Member

What version of Go are you using (go version)?

n/a

Does this issue reproduce with the latest release?

n/a

What operating system and processor architecture are you using (go env)?

n/a

What did you do?

Open https://pkg.go.dev/github.com/hajimehoshi/ebiten@v1.12.0-rc.3#hdr-DrawRectShaderOptions_represents_options_for_DrawRectShader

What did you expect to see?

The text "DrawRectShaderOptions represents options for DrawRectShader" should be a normal text.

What did you see instead?

The text seems a header. Actually this is represented with a <h3> element.

@gopherbot gopherbot added this to the Unreleased milestone Sep 24, 2020
@julieqiu julieqiu added NeedsFix The path to resolution is known, but the work has not been done. help wanted labels Sep 24, 2020
@julieqiu
Copy link
Member

Thanks for the feedback! If anyone is interested in working on this, we'd love to accept a contribution for this fix.

@julieqiu julieqiu changed the title x/pkgsite: unexpected header x/pkgsite: comment in dochtml is represented as <h3> header, but should be normal text Sep 24, 2020
@trongbq
Copy link

trongbq commented Sep 25, 2020

@julieqiu I'd love to help, may I work on this one?

@julieqiu
Copy link
Member

@trongbq, please do!

@trongbq
Copy link

trongbq commented Sep 28, 2020

@hajimehoshi pkgsite's render recognizes that comment is a heading because the comment follows heading style. More details explanation bellow.

In the source code, the comment is written like this

// DrawRectShaderOptions represents options for DrawRectShader
//
// This API is experimental.

If you look at the first line, you can see that it doesn't have period mark at the end.
pkgsite's render uses following regex to identify heading, which return true for that line.

// Regexp for headings.
headingHead = `^[\p{Lu}]`                                  // any uppercase letter
headingBody = `([^,.;:!?+*/=()\[\]{}_^°&§~%#@<">\\]|'s )*` // any non-illegal character
headingTail = `([\p{L}\p{Nd}]|'s)?$`                       // any letter or digit

Is is okay for you that I will send a PR to add period mark at the end for comments like this? It will make comment style in your code be consistent.

@julieqiu I wonder should we keep existing check for header in comment. Do you think this is an issue need to be fixed in pkgsite? Personally, I think we can just keep pkgsite's render as it is.

P/s: I looked in godoc, that comment above is displayed normally without any headers. I didn't look deeper into the godoc source but I think that godoc doesn't have heading style.

@julieqiu
Copy link
Member

Is is okay for you that I will send a PR to add period mark at the end for comments like this? It will make comment style in your code be consistent.

SGTM, thanks!

/cc @dmitshur for input on this issue

@hajimehoshi
Copy link
Member Author

Is is okay for you that I will send a PR to add period mark at the end for comments like this?

Thanks, that'd be helpful!

@dmitshur dmitshur changed the title x/pkgsite: comment in dochtml is represented as <h3> header, but should be normal text x/pkgsite/internal/fetch/dochtml/internal/render: first paragraph should not be considered as a potential section heading Sep 29, 2020
@dmitshur
Copy link
Contributor

@trongbq Thank you for investigating this.

This godoc behavior is called a "section heading" and it is currently best documented in the second paragraph of "go/doc".ToHTML:

Each span of unindented non-blank lines is converted into a single paragraph. There is one exception to the rule: a span that consists of a single line, is followed by another paragraph span, begins with a capital letter, and contains no punctuation other than parentheses and commas is formatted as a heading.

It is also mentioned in the .../dochtml/interal/render package documentation, see here.

The go/doc implementation actually has additional conditions that need to be met for a line to be considered a heading: it must be immediately preceded by a blank line, and a non-heading paragraph before that.

The render package in pkgsite doesn't seem to check what came before the heading, so may consider the first line in the doc string as a heading. Given that comments for exported identifiers are expected to begin with the name of the identifier itself and be a complete sentence, not supporting headings in the first line (as godoc does) seems like the better behavior.

@trongbq
Copy link

trongbq commented Sep 29, 2020

@dmitshur thank you for such a clear explanation.

not supporting headings in the first line (as godoc does) seems like the better behavior.

I see, let's me try to implement this behavior in render package.

@gopherbot
Copy link

Change https://golang.org/cl/258398 mentions this issue: render: first paragraph should not be considered as a potential section heading.

@golang golang locked and limited conversation to collaborators Oct 7, 2021
@hyangah hyangah added the pkgsite/dochtml Issues related to package documentation in pkgsite label May 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. pkgsite/dochtml Issues related to package documentation in pkgsite pkgsite
Projects
None yet
Development

No branches or pull requests

6 participants