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
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur 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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 17, 2019
@dmitshur dmitshur added this to the Unreleased milestone May 17, 2019
@gopherbot
Copy link

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
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 This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

2 participants