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/tools/godoc: remove replaceLinePrefixCommentsWithBlankLine in favor of better fix #32092

Open
dmitshur opened this issue May 17, 2019 · 1 comment

Comments

@dmitshur
Copy link
Member

commented May 17, 2019

Now that #7702 is resolved, it should be possible to remove the replaceLinePrefixCommentsWithBlankLine workaround with a better fix.

This issue is about tracking the progress on that (since all previous issues related to this were closed).

I've looked into this a bit and it's not clear to me if the fix belongs in golang.org/x/tools/godoc itself, or if perhaps it can be implemented in a lower level component: one of go/doc, go/parser, go/scanner, go/ast, etc. (The workaround would need to stay until it's possible to fully rely on the lower level fix in stdlib.)

I'm also not sure if it's expected behavior for //line comments to show up in ast.CommentGroup or not.

@griesemer Do you have suggestions on how to best deal with this issue, now that token.PositionFor API exists?

History

Issue #5247, reported in 2013, was about godoc then not handling the following input properly:

package p

//line file:2
// G doc.
func G()

It was showing "line file:2 G doc." as the documentation of function G, rather than just "G doc." as it should've been. (If you were to change the line number in the comment to //line file:10, it wouldn't show up anymore.)

At that time, there was no API to obtain the raw source positions from a token.Pos, which was the main blocker and a long-standing TODO.

As a result, the godoc issue (#5247) was fixed via a workaround in golang/tools@55ea531 with the comment:

// Temporary ad-hoc fix for issue 5247.
// TODO(gri) Remove this in favor of a better fix, eventually (see issue 7702).
replaceLinePrefixCommentsWithBlankLine(src)

Issue #7702 was filed about adding an API to access the original source lines. It has since been resolved via e66ff2b.

@dmitshur dmitshur added this to the Unreleased milestone May 17, 2019

@gopherbot

This comment has been minimized.

Copy link

commented May 17, 2019

Change https://golang.org/cl/177737 mentions this issue: godoc: re-add test for ignoring //line comments in source code

gopherbot pushed a commit to golang/tools that referenced this issue May 28, 2019
godoc: re-add test for ignoring //line comments in source code
The original CL 84050044 added a test case, and it happened to be
in between various CLI test cases. CLI support was removed from
x/tools/cmd/godoc in CL 141397, as part of golang/go#25443.
Re-add a test case for this behavior to prevent regressions.

Updates golang/go#32092
Updates golang/go#25443
Updates golang/go#5247

Change-Id: I0cea74cfe40d120e398a9005676134c5bad6136c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/177737
Reviewed-by: Robert Griesemer <gri@golang.org>

@gopherbot gopherbot added the Tools label Sep 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.