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/website: inconsistent tabs in code snippets #52255

Closed
looshch opened this issue Apr 9, 2022 · 11 comments
Closed

x/website: inconsistent tabs in code snippets #52255

looshch opened this issue Apr 9, 2022 · 11 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. website
Milestone

Comments

@looshch
Copy link
Contributor

looshch commented Apr 9, 2022

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

$ go version
go version go1.18 darwin/arm64

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="arm64"
GOBIN=""
GOCACHE="/Users/l/Library/Caches/go-build"
GOENV="/Users/l/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/l/Projects/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/l/Projects/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.18/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.18/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.18"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/l/Projects/go/src/website/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/_8/v1rjtbwx7f1dk3kq2qzykk2m0000gn/T/go-build198020044=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

visited go.dev/doc/articles/wiki, go.dev/doc/effective_go, go.dev/doc/code, go.dev/doc/faq, go.dev/ref/mem, and https://go.dev/ref/spec. In some places, real tabs are used instead of 4 spaces. The problem is that the former case is equal to 8 spaces in browsers, but the majority of code on go.dev are rendered using the latter representation, so indentation is not consistent. The following screenshot is taken from go.dev/doc/articles/wiki and contains the first Go code snippet. You can copy the first tab to search for it on other pages
Screenshot 2022-04-09 at 11 24 30 p m

I made 2 PRs because some pages are not in x/website but in this repo instead. To prevent this in future, maybe I can create a test checking contents inside every <pre> tag for tabs. I could suggest to create a replacer instead, but there are some cases when it will result in messed indentation, e.g. in the spec. What do you think?

What did you expect to see?

consistent indentation

What did you see instead?

inconsistent one

@gopherbot gopherbot added this to the Unreleased milestone Apr 9, 2022
@gopherbot
Copy link

Change https://go.dev/cl/399374 mentions this issue: _content/doc: fix inconsistent tabs in code snippets

@gopherbot
Copy link

Change https://go.dev/cl/399394 mentions this issue: doc: fix inconsistent tabs in code snippets

@ianlancetaylor
Copy link
Contributor

Go code uses real tabs, so perhaps the web pages should as well. I admit that I don't know the advantages or disadvantages of either choice.

@looshch
Copy link
Contributor Author

looshch commented Apr 10, 2022

@ianlancetaylor most of the code on the website is written with tabs which are equal to 4 real spaces, that’s why I assumed it as the correct choice. The reason behind this likely that all major modern browsers render tab 8 characters wide. In editors and IDEs one can choose whatever width she wants. Since the only purpose of tab indentation is readability, I think 4-characters width performs its role well because code snippets have both small height and nesting. But I would happily turn them into real tabs, consistency is the only concern I have

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 10, 2022
@looshch
Copy link
Contributor Author

looshch commented Apr 24, 2022

any updates?

@ianlancetaylor
Copy link
Contributor

CC @jamalc

@looshch
Copy link
Contributor Author

looshch commented May 13, 2022

any updates?

@jamalc
Copy link

jamalc commented May 23, 2022

Let's stick with tabs for code snippets. This keeps the snippets consistent with actual Go code and the Go playground. Doc authors won't have to worry about converting snippets and readers can copy correct code from docs.

@looshch
Copy link
Contributor Author

looshch commented May 23, 2022

This keeps the snippets consistent with actual Go code and the Go playground

readers can copy correct code from docs

anyway gofmt turns spaces into tabs where appropriate

Doc authors won't have to worry about converting snippets

only until authors decide to use markdown instead of HTML as a file extension. Consider this case in _content/doc/tutorial/workspaces.md on lines 80–81. It has 3-space indentation because it’s a part of a list item

   import (
       "fmt"

and rendered like this

import (
    "fmt"

If one puts 3 spaces and then adds a tab, he will see in browser

import (
 "fmt"


also just discovered that in internal/web/render.go on lines 232–238 Russ wrote

// In Markdown, tabs used for indentation are required to be interpreted as
// 4-space tab stops. See https://spec.commonmark.org/0.30/#tabs.
// Go also renders nicely and more compactly on the screen with 4-space
// tab stops, while browsers often use 8-space.
// And Goldmark crashes in some inputs that mix spaces and tabs.
// Fix the crashes and make the Go code consistently compact across browsers,
// all while staying Markdown-compatible, by expanding to 4-space tab stops.

edit: grammar
edit2: typo

@jamalc
Copy link

jamalc commented May 23, 2022

Fair enough. Spaces then.

@looshch
Copy link
Contributor Author

looshch commented May 23, 2022

thank you! I abandoned my another PR included in this issue after your reply because it rendered the PR irrelevant. Now it is restored and located at https://go.dev/cl/399394 Can I kindly ask you to review that one as well, please?

gopherbot pushed a commit that referenced this issue May 26, 2022
Fixes #52255

Change-Id: Ibb518cf2f6bac9e1ffc426a014afe80cc4c0df5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/399394
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
@golang golang locked and limited conversation to collaborators May 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. website
Projects
None yet
Development

No branches or pull requests

5 participants