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

Jump to bottom

cmd/gofmt: inconsistent indentation for comments inside switch-case #30327

Open
subsr97 opened this issue Feb 20, 2019 · 5 comments
Open

cmd/gofmt: inconsistent indentation for comments inside switch-case #30327

subsr97 opened this issue Feb 20, 2019 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@subsr97
Copy link

subsr97 commented Feb 20, 2019

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

$ go version
go version go1.11.4 linux/amd64

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
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/local/ZOHOCORP/mani-pt2396/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/local/ZOHOCORP/mani-pt2396/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/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-build688297931=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Commented out the last case of switch with line or block comment(s).

https://play.golang.org/p/fkseigRrxYV

What did you expect to see?

The last case(s) to be in their place and not go inside the scope of the previous case.

	switch val {
	case foo1:
		foo()
	case foo2:
		foo()
	// case bar:
		// bar()
	}

	switch val {
	case foo1:
		foo()
	case foo2:
		foo()
	/* case bar:
		// bar()*/
	}

What did you see instead?

The last case(s) is aligned as if it is inside the scope of the previous case.

	switch val {
	case foo1:
		foo()
	case foo2:
		foo()
		// case bar:
		// bar()
	}

	switch val {
	case foo1:
		foo()
	case foo2:
		foo()
		/* case bar:
		// bar()*/
	}
@agnivade
Copy link
Contributor

Maybe good to fix, but the heuristics are complicated. How do we know whether we are not actually commenting out code inside the last case ?

Although I agree we should be consistent in the indentation whether we are in the middle of a switch-case or at the end. Currently, it seems there is an inconsistency.

package main

func main() {
	val := 1
	switch val {
	case 1:
		_ = 1
	// case 5: // gets aligned to the case
	// 	_ = 5
	case 2:
		_ = 2
		// case 3: // gets aligned to the code block inside the case
		// 	_ = 3
	}
}

@griesemer

@agnivade agnivade changed the title Unexpected line and block comment behaviour in last case of switch cmd/gofmt: inconsistent indentation for comments inside switch-case Feb 20, 2019
@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 20, 2019
@agnivade agnivade added this to the Unplanned milestone Feb 20, 2019
@josharian
Copy link
Contributor

I’m 95% confident this is a dup. On my phone and can’t find the original (apologies?), but there has been significant discussion of indent, gofmt, and switch cases on some other issue.

@subsr97
Copy link
Author

subsr97 commented Feb 20, 2019 •
edited

@josharian Are you referring to #28057?

@josharian
Copy link
Contributor

Yes. And in a related vein there's also #20681 and #9910. And perhaps others.

@griesemer
Copy link
Contributor

Leaving open for future consideration. Unfortunately, gofmt work is not currently a priority for us as long as code is not broken by it, so theses issues will linger a bit. The good news is that when we eventually get to addressing these, we have a lot of corner cases to test against. And once fixed, it's trivial to "fix" huge corpuses of code in one fell swoop.

@tommyknows tommyknows mentioned this issue May 1, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@josharian @agnivade @griesemer @subsr97