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

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

Comments

@subsr97
Copy link

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

@agnivade agnivade commented Feb 20, 2019

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 this to the Unplanned milestone Feb 20, 2019
@josharian
Copy link
Contributor

@josharian josharian commented Feb 20, 2019

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 subsr97 commented Feb 20, 2019

@josharian Are you referring to #28057?

@josharian
Copy link
Contributor

@josharian josharian commented Feb 20, 2019

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

@griesemer
Copy link
Contributor

@griesemer griesemer commented Feb 20, 2019

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.

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.

None yet
4 participants
You can’t perform that action at this time.