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

go/doc: doc package converts smart quotes in preformatted blocks #29730

Closed
segevfiner opened this issue Jan 14, 2019 · 7 comments

Comments

@segevfiner
Copy link
Contributor

commented Jan 14, 2019

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

$ go version
go version devel +7cbfa55b5d Sat Jan 12 00:27:48 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Git master.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="<snip>/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="<snip>/go"
GOPROXY=""
GORACE=""
GOROOT="<snip>/prj/go"
GOTMPDIR=""
GOTOOLDIR="<snip>/prj/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build019133103=/tmp/go-build -gno-record-gcc-switches"

What did you do?

  1. Save as smartquotes.go
package godoctest

// Bar does ``stuff''
//
//     Bar(`` + '')
func Bar(s string) {

}
  1. Run go doc Bar in that package directory.

What did you expect to see?

~/prj/go/bin/go doc Bar
func Bar(s string)
    Bar does “stuff”

    Bar(`` + '')

What did you see instead?

~/prj/go/bin/go doc Bar
func Bar(s string)
    Bar does “stuff”

    Bar(“ + ”)

The preformatted block has smart quotes converted.

Analysis

Smart quotes are converted here:

line = convertQuotes(line)

But it's explicitly not done in HTML output here:

emphasize(w, line, nil, false)

By the final false argument. So it probably shouldn't be done for the text format too.

@agnivade

This comment has been minimized.

Copy link
Member

commented Jan 16, 2019

@robpike

This comment has been minimized.

Copy link
Contributor

commented Jan 16, 2019

It's go/doc. @griesemer?

@robpike robpike changed the title cmd/doc: Converts smart quotes in preformatted blocks go/doc: doc package converts smart quotes in preformatted blocks Jan 16, 2019

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Jan 17, 2019

I haven't investigated in detail but probably happens in go/doc/comment.go somewhere. This sounds like a good starter problem for somebody to fix.

@griesemer griesemer added the Suggested label Jan 17, 2019

@griesemer griesemer added this to the Go1.13 milestone Jan 17, 2019

@agnivade

This comment has been minimized.

Copy link
Member

commented Jan 17, 2019

I made the convertQuotes change, which converts these special quotes to unicode quotes. But we were never checking for preformatted blocks.

For 1.11, it will be -

func Bar(s string)
    Bar does ``stuff''

    Bar(`` + '')

For 1.12, it will be -

func Bar(s string)
    Bar does “stuff”

    Bar(” + ”)

@griesemer - Are you fine with ignoring these conversions in preformatted blocks ? We will then need to add the logic to check for preformatted blocks.

@griesemer

This comment has been minimized.

Copy link
Contributor

commented Jan 17, 2019

@agnivade It makes sense to leave preformatted blocks alone, but I don't have the full picture in my head at the moment. Probably worthwhile making sure we have a plan that doesn't require this changing back and forth in each release.

@segevfiner

This comment has been minimized.

Copy link
Contributor Author

commented Jan 17, 2019

Note that the code already doesn't convert quotes in the HTML output.

@gopherbot

This comment has been minimized.

Copy link

commented Feb 18, 2019

Change https://golang.org/cl/162922 mentions this issue: go/doc: skip escaping comments in pre-formatted blocks

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