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/gopls: add an option to keep original line breaks in documentation comments #44684

Open
gudvinr opened this issue Feb 28, 2021 · 4 comments · May be fixed by golang/tools#325
Open

x/tools/gopls: add an option to keep original line breaks in documentation comments #44684

gudvinr opened this issue Feb 28, 2021 · 4 comments · May be fixed by golang/tools#325

Comments

@gudvinr
Copy link

@gudvinr gudvinr commented Feb 28, 2021

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

$ go version
go version go1.16 linux/amd64
$ gopls version
golang.org/x/tools/gopls v0.6.6

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=""
GOCACHE="~/.cache/go-build"
GOENV="~/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="~/tools/go/path/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="~/tools/go/path"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build1390191991=/tmp/go-build -gno-record-gcc-switches"

What did you do?

This is follow-up on issue for vscode-go which turned out to be an issue with gopls.

Documentation comments inside code look like this:
2021-02-25 18 13 16

It has line breaks so when you read comments while skimming yours or other peoples' code, you can expect comfortable and consistent text width. Comment width does not depend on viewport size, window size, font weight or system DPI.

Hover tooltips, however, changes its width and sometimes you may get very long documentation and sometimes it's too narrow.

In aforementioned issue, vscode-go developer stated that this behaviour is intended to make docs look like godoc html representation.
While it might sound like a good idea, it must be noted that godoc viewed inside browser as dedicated page.
Which means that:

  • you get consistent width for every documentation within a page
  • you get a look at documentation only
  • godoc presents documentation for package as a whole, not for single function at a time
  • author may change content layout using css to make width consistent and convenient to consume
  • you may change content layout using stylus and alike to make content look convenient for yourself

Consumption behaviour inside text editor differs from godoc.
It is easier to read short snippets when they narrow. There also was a link to upstream issue of vscode for making tooltip width that mitigates this issue.
However, while it solves this issue, there is another one. Code of the package still remains important source of truth when you want to figure out why it works how it works. That means representation of docs becomes inconsistent: while you see continuous line in tooltip, it still narrow multi-line text in comments.

What did you expect to see?

Documentation with predictable text size that looks like documentation inside code comments which code author originally wrote.

What did you see instead?

2021-02-25 18 13 30

@gopherbot gopherbot added this to the Unreleased milestone Feb 28, 2021
@stamblerre stamblerre removed this from the Unreleased milestone Feb 28, 2021
@stamblerre stamblerre added this to the gopls/unplanned milestone Feb 28, 2021
ShoshinNikita added a commit to ShoshinNikita/tools that referenced this issue Jun 20, 2021
…ks in hover

Add a new documentation option `KeepOriginalLineBreaks` to keep original
comment line breaks in hover. When the option is set to `true`, we add
2 spaces just before newline characters. This logic doesn't apply to
code blocks and headers.

Fixes golang/go#44684
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 20, 2021

Change https://golang.org/cl/329669 mentions this issue: internal/lsp/source: add an option to keep original comment line breaks in hover

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jun 21, 2021

Thank you for sending this CL, @ShoshinNikita, but I think we need to give this change a little more thought before we merge it. Adding a configuration option is a significant overhead, and I think we need a little more evidence that this change is worthwhile and would be widely used.

@ShoshinNikita, can you speak to why you're in support of this?
Can other users who would use this option weigh in or upvote the issue?

@ShoshinNikita
Copy link

@ShoshinNikita ShoshinNikita commented Jun 22, 2021

@stamblerre I thought the fact this issue is open and there're no any objections means it's ok to send a CL. Though I have no intention of using this feature myself, I believe the users should have an option to keep the original line breaks. I think it's a good argument:

...you can expect comfortable and consistent text width. Comment width does not depend on viewport size, window size, font weight or system DPI.

For example, I find it easier to read comments with the original line breaks:

Godoc Original line breaks
2021-06-22_20-48 2021-06-22_20-47
2021-06-22_20-52 2021-06-22_20-52_1

@stamblerre stamblerre removed their assignment Jun 28, 2021
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jun 28, 2021

@stamblerre I thought the fact this issue is open and there're no any objections means it's ok to send a CL.

Typically I would recommend leaving a comment on the issue before taking it on, or seeing if it has a "help wanted" label. We usually try to mark issues that need a decision, but sometimes we may forget, so I would suggest checking in before starting work.

I think we are still going to hold off on merging this until other folks weigh in. I see the argument, but the presentation of hover is very variable depending on your editor and how it shows the hover, so I think something like microsoft/vscode#14165 (which @hyangah linked on the original issue) really is a better bet. Adding a new configuration adds significant overhead and complexity to gopls (particularly for users, since it complicates the documentation), so I'd really like to see stronger support for this behavior before we add it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants