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 · 5 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 · 5 comments · May be fixed by golang/tools#325
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.

Comments

@gudvinr
Copy link

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 Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Feb 28, 2021
@gopherbot gopherbot added this to the Unreleased 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

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

@stamblerre stamblerre added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 21, 2021
@stamblerre
Copy link
Contributor

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

@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 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.

@hyangah
Copy link
Contributor

hyangah commented May 10, 2022

@gopherbot remove Documentation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants