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: slice of structs not consistently formatted #34582

Open
smasher164 opened this issue Sep 27, 2019 · 7 comments
Open

cmd/gofmt: slice of structs not consistently formatted #34582

smasher164 opened this issue Sep 27, 2019 · 7 comments

Comments

@smasher164
Copy link
Member

@smasher164 smasher164 commented Sep 27, 2019

See https://play.golang.org/p/uwG_GFPiU13.
I expected the literal to be reformatted like this:

package main

func main() {
	_ = []struct{ a int }{
		{
			a: 1,
		},
		{
			a: 2,
		},
		{
			a: 3,
		},
	}
}

instead, it preserves the previous formatting.

package main

func main() {
	_ = []struct{ a int }{
		{
			a: 1,
		},
		{
			a: 2,
		}, {
			a: 3,
		},
	}
}

Is this intentional? Or does it make sense to keep the formatting of structs in the slice the same?

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Sep 27, 2019

@agnivade
Copy link
Contributor

@agnivade agnivade commented Sep 28, 2019

Both

},
{

and }, { are allowed.

To make different styles fall to a single one in a single declaration, we would have to keep track of braces and their positions and then possibly go backwards and change token positions.

Up to Robert to decide to do it or not.

@mvdan
Copy link
Member

@mvdan mvdan commented Sep 28, 2019

I personally think this would be nice to have, if it doesn't add a lot of complexity to gofmt.

@griesemer
Copy link
Contributor

@griesemer griesemer commented Sep 30, 2019

Nice to have, but complicated to get right, I suspect. In general, where multiple formats are accepted, we defer to the user.

@griesemer griesemer added this to the Unplanned milestone Sep 30, 2019
@mvdan
Copy link
Member

@mvdan mvdan commented Mar 14, 2020

I've implemented a prototype at mvdan/gofumpt@5d37481 if anyone wants to play with it. The solution is pretty straightforward there, but I already had some logic to determine if a composite literal had any newlines between its elements.

@smasher164
Copy link
Member Author

@smasher164 smasher164 commented Mar 15, 2020

Thank you for working on this!
I tried out mvdan/gofumpt@5d37481, but it enforces that

},
{

is applied in all cases. For example,

_ = []struct{ a int }{
    {a: 1}, {
        a: 2,
    }, {
        a: 3,
    },
}

It is not obvious to me that the above literal gets reformatted into

_ = []struct{ a int }{
    {a: 1},
    {
        a: 2,
    },
    {
        a: 3,
    },
}

What gets determined to be the consistent style? The only one I can think of is the style of the first element in the list gets applied to the subsequent elements.

@mvdan
Copy link
Member

@mvdan mvdan commented Mar 21, 2020

Thanks for the report, @smasher164. Let's take this to mvdan/gofumpt#36, since the bug doesn't concern gofmt.

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
5 participants
You can’t perform that action at this time.