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.

Sign up for GitHub

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

Jump to bottom

go/doc/comment: failure to recognize initial code block #61373

Closed
ghost opened this issue Jul 15, 2023 · 7 comments
Closed

go/doc/comment: failure to recognize initial code block #61373

ghost opened this issue Jul 15, 2023 · 7 comments
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@ghost
Copy link

ghost commented Jul 15, 2023

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

go version go1.20 windows/amd64

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

go env Output
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Steven\AppData\Local\go-build
set GOENV=C:\Users\Steven\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\Steven\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Steven\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=D:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=D:\go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.20
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=C:\Windows\TEMP\go-build2119281587=/tmp/go-build -gno-record-gcc-switches

What did you do?

if I run this code:

package main

import (
   "go/doc/comment"
   "os"
)

const pass = `one
 two
 three`

const fail = ` two
 three`

func main() {
   {
      d := new(comment.Parser).Parse(pass)
      b := new(comment.Printer).HTML(d)
      os.Stdout.Write(b)
   }
   os.Stdout.WriteString("\n")
   {
      d := new(comment.Parser).Parse(fail)
      b := new(comment.Printer).HTML(d)
      os.Stdout.Write(b)
   }
}

What did you expect to see?

<p>one
<pre>two
three
</pre>

<pre>two
three
</pre>

What did you see instead?

<p>one
<pre>two
three
</pre>

<p>two
three

it seems the first input gives expected output, but the second does not. from what I can tell, both inputs match the definition of a code block:

A code block is a span of indented or blank lines not starting with a bullet list marker or numbered list marker.

https://go.dev/doc/comment#code
@AndrewHarrisSPU
Copy link

Interesting - subtly, I think the implementation has a notion of common indentation. By tweaking the relative amount of whitespace it's possible to get different parses (https://go.dev/play/p/-MM0d5hDbYk).

Is this a documentation thing? I couldn't pick out this behavior from the documentation directly or infer it from subtle details it does cover. The package doc for Parser.Parse says:

Parse parses the doc comment text and returns the *Doc form. Comment markers (/* // and */) in the text must have already been removed.

Would "Comment markers and indentation common to all lines in text" be enough of a tweak?

@ghost
Copy link
Author

ghost commented Jul 15, 2023

Is this a documentation thing?

I would prefer if the current behavior was an error, because I would like the freedom to start a comment with a code block, which seems is not currently possible. but if I had to guess, I would say the current behavior is correct, and that the documentation needs to be improved.

Would "Comment markers and indentation common to all lines in text" be enough of a tweak?

I dont think so. I think this behavior is unexpected enough that it should be special cased in the documentation, assuming the current behavior is correct. so something like:

it is not possible to start a comment with a code block.

@AndrewHarrisSPU
Copy link

I would like the freedom to start a comment with a code block

I think this is possible? In https://go.dev/play/p/-MM0d5hDbYk just having the opening line have more spaces in front seemed to set it into a <pre> block

@ghost
Copy link
Author

ghost commented Jul 15, 2023

yes, I just noticed that as well. it seems the current restriction would more accurately be:

it is not possible to have only a code block as a comment. a code block must be accompanied by at least one leading or trailing line

@seankhliao seankhliao added Documentation Issues describing a change to documentation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 17, 2023
@bcmills bcmills added this to the Backlog milestone Jul 17, 2023
@ghost ghost closed this as completed Aug 27, 2023
@ianlancetaylor
Copy link
Member

This is happening because the go/doc/comment parser first strips any indentation that appears consistently in every line. If you change your example to

const fail = ` two
 three
four`

then the initial code block is recognized.

Stripping consistent indentation is a heuristic that seems to work reasonably well in practice.

@ghost ghost reopened this Aug 27, 2023
@ghost
Copy link
Author

ghost commented Aug 27, 2023

ideally I would like to be able to have a code block only comment, which is not currently possible. however failing that, I think it would be helpful to at least document that its a known shortcoming that you cannot do that.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/524039 mentions this issue: _content/doc/comment: document removal of common prefix

willpoint pushed a commit to orijtech/website that referenced this issue Oct 17, 2023
@ianlancetaylor @willpoint
_content/doc/comment: document removal of common prefix

Verified

This commit was signed with the committer’s verified signature.
a1346054
GPG key ID: D149AD21DC40440C
Verified
Learn about vigilant mode
88e235d 
Fixes golang/go#61373

Change-Id: I60fa1be13cb7e32af5169e303f5e1c40d207edc4
Reviewed-on: https://go-review.googlesource.com/c/website/+/524039
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
@golang golang locked and limited conversation to collaborators Aug 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Documentation Issues describing a change to documentation. FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@ianlancetaylor @bcmills @gopherbot @seankhliao @AndrewHarrisSPU