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

cmd/doc: extra comment marker (//) included in terminal output for go doc cmd/compile #40992

Closed
inrick opened this issue Aug 23, 2020 · 5 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. Suggested Issues that may be good for new contributors looking for work to do.
Milestone

Comments

@inrick
Copy link

inrick commented Aug 23, 2020

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

$ go version
go version go1.15 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/me/local/go/bin"
GOCACHE="/home/me/.cache/go-build"
GOENV="/home/me/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/me/local/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/me/local/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build546021136=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.15 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.15
uname -sr: Linux 5.8.1-arch1-1
LSB Version:	1.4
Distributor ID:	Arch
Description:	Arch Linux
Release:	rolling
Codename:	n/a
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.31.
lldb --version: lldb version 10.0.1
gdb --version: GNU gdb (GDB) 9.2

What did you do?

Ran go doc cmd/compile.

What did you expect to see?

Output matching the web documentation (https://golang.org/pkg/cmd/compile/), that is:

...
In order to be recognized as a line directive, the comment must start with
//line or /*line followed by a space, and must contain at least one colon.
...

What did you see instead?

...
In order to be recognized as a line directive, the comment must start with
// //line or /*line followed by a space, and must contain at least one colon.
...

which was at first a bit confusing.

If it is any help, some previous versions of Go (e.g., 1.14) also included:

...
The line directive is an historical special case; all other directives are
of the form //go:name and must start at the beginning of a line, indicating
// that the directive is defined by the Go toolchain.
...

Now that the comment has been rewritten in 1.15 the extra // is no longer present:

...
The line directive is a historical special case; all other directives are of
the form //go:name, indicating that they are defined by the Go toolchain.
...
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 26, 2020
@cagedmantis cagedmantis added this to the Backlog milestone Aug 26, 2020
@cagedmantis
Copy link
Contributor

/cc @robpike @mvdan

@robpike
Copy link
Contributor

robpike commented Aug 27, 2020

This would be a bug in go/doc, I suspect. @griesemer

@griesemer
Copy link
Contributor

griesemer commented Aug 27, 2020

The output for https://golang.org/cmd/compile/ looks ok. I am guessing something goes wrong in the ToText function in go/doc/comment.go. Have not investigated yet.

This is a good issue to look into for somebody who wants to get their feet wet with contributing to the Go project.

@agnivade agnivade added the Suggested Issues that may be good for new contributors looking for work to do. label Aug 27, 2020
@gopherbot
Copy link

Change https://golang.org/cl/251237 mentions this issue: cmd/doc: adding validation before adding comment marker

@kemalelmizan
Copy link
Contributor

(Hi, new folk here first time trying to contribute to the project. Please go easy on me.)
The problem is lineWrapper.write introduces new comment marker on fix to this issue #20929 but does not check field if they already have // or not. This introduces inconsistency from

s := `
In order to be recognized as a line directive, the comment must start with
//line or /*line followed by a space, and must contain at least one colon.
`
doc.ToText(os.Stdout, s, "", "", 76)

that returns

In order to be recognized as a line directive, the comment must start with
// //line or /*line followed by a space, and must contain at least one colon.

and doc.ToHTML(os.Stdout, s, map[string]string{}) that returns correct output

<p>
In order to be recognized as a line directive, the comment must start with
//line or /*line followed by a space, and must contain at least one colon.
</p>

Snippet here https://goplay.tools/snippet/tEl0elooxM2
I've added additional test to the commit too.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 3, 2020
@dmitshur dmitshur modified the milestones: Backlog, Go1.16 Nov 3, 2020
@golang golang locked and limited conversation to collaborators Nov 3, 2021
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. Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

9 participants