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

use more newlines when formatting very complex composite literals #104

Open
slimsag opened this issue Jan 1, 2021 · 6 comments
Open

use more newlines when formatting very complex composite literals #104

slimsag opened this issue Jan 1, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@slimsag
Copy link

slimsag commented Jan 1, 2021

(huge thank you for gofumpt, BTW, it's awesome!)

I have lots of code similar to this which I am generating and then formatting with gofumpt:

package main

import "st"

func main() {
	v := [2]interface{}{&st.Query{DocTypes: &st.DocTypes{CST: &st.DocTypesCST{
		List: &st.DocTypesListCST{
			Name: "foo", Separator: ",",
			Next: &st.DocTypesListCST{Name: "bar"},
		},
	}}, Expr: &st.QueryExpr{Logical: &st.ExprLogical{Or: &st.ExprLogicalOr{
		Left:  st.QueryExpr{Default: &st.ExprDefault{Terms: "not implemented"}},
		Right: st.QueryExpr{Default: &st.ExprDefault{Terms: "not implemented"}},
	}}}}, nil}
	_ = v
}

My primary readability gripes here are:

  1. It's hard to tell what type the Expr: field is under (line }}, Expr: &st.QueryExpr{Logical: &st.ExprLogical{Or: &st.ExprLogicalOr{)
  2. It's hard to spot the 2nd nil value of the [2]interface{} array. When reviewing these where the 2nd array value is not nil but rather a &Foobar{} struct value, they blur together a bit for me.

I can think of a few improved formattings which seem more easily readable to me, including:

I understand gofumpt is more opinionated, though, and I'm not too attached to the exact formatting improvements here - I am just more broadly asking what improvements seem reasonable here such that gofumpt could make this code more readable potentially

@mvdan
Copy link
Owner

mvdan commented Jan 1, 2021

What was the original input to gofumpt? I definitely agree that the code as shown is pretty hard to read, but gofumpt also tries to obey the formatting choices in the original code in some cases.

@mvdan mvdan added the enhancement New feature or request label Jan 2, 2021
@slimsag
Copy link
Author

slimsag commented Jan 2, 2021

gofumpt also tries to obey the formatting choices in the original code in some cases.

Ah okay, that makes sense, I had a slight hunch this might be the case. I think it's reasonable to close this issue then.

The input in the issue description came after some other mutations I was trying, the most original (generated) input I had was this, which should be formatted nicely after #70:

package main

func main() {
	v := [2]interface {
}{&st.Query{DocTypes: &st.DocTypes{CST: &st.DocTypesCST{List: &st.DocTypesListCST{Name: "foo", Separator: ",", Next: &st.DocTypesListCST{Name: "bar"}}}}, Expr: &st.QueryExpr{Logical: &st.ExprLogical{Or: &st.ExprLogicalOr{Left: st.QueryExpr{Default: &st.ExprDefault{Terms: "not implemented"}}, Right: st.QueryExpr{Default: &st.ExprDefault{Terms: "not implemented"}}}}}}, nil}
}

@mvdan
Copy link
Owner

mvdan commented Jan 2, 2021

I think "generate all code in very few lines and let gofumpt pretty-print it however it wants" is a valid use case, so I'd leave this issue open. It does have a big overlap with breaking long lines, so I'd like to tackle that first, but once that's done then we can use this issue to see what else we could do for generated code.

@mvdan mvdan changed the title Can gofumpt format this code more nicely? use more newlines when formatting very complex composite literals Jan 2, 2021
@mvdan
Copy link
Owner

mvdan commented Jan 2, 2021

Thinking outloud a bit, I think we should enforce more newlines when there are many (e.g. more than 2) levels of composite literals starting or ending on a single line. I think }}}} is what jumps to me as the most confusing in your snippet.

However, if there is just a couple of levels, I think it's entirely reasonable. For example:

var _ = []T{{
    Field1: 0,
    Field2: 1,
}, {
    Field1: 2,
    Field2: 3,
}}

@slimsag
Copy link
Author

slimsag commented Jan 2, 2021

Cool, that sounds fair, I'll reword the issue title and I should be able to report back with a fair amount of data once #70 lands


I like your thought above, that sounds like a great reduction in the problem space here down to something tangible.

In fact, I think even if just "composite literals >2 deep ending on a single line" were the case, the code in the issue description would be much better (though I certainly have a strong preference for both starting and ending on a single line):

v := [2]interface{}{&st.Query{DocTypes: &st.DocTypes{CST: &st.DocTypesCST{
    List: &st.DocTypesListCST{
        Name: "foo", Separator: ",",
        Next: &st.DocTypesListCST{Name: "bar"},
    },
}}, Expr: &st.QueryExpr{Logical: &st.ExprLogical{Or: &st.ExprLogicalOr{
        Left:  st.QueryExpr{Default: &st.ExprDefault{Terms: "not implemented"}},
        Right: st.QueryExpr{Default: &st.ExprDefault{Terms: "not implemented"}},
    }},
}}, nil}

@randallmlough
Copy link

randallmlough commented Sep 1, 2023

Hoping to resurrect this request, since I really need this feature as well for generated code.

Example of before and (desired) after.
Code as generated:

func (Activity) Fields() []ent.Field {
	return []ent.Field{field.String("title").Optional(), field.String("dateRange").Optional(), field.String("location").Optional(), field.Int("groupId").Optional().Immutable()}
}

Desired formatting:

func (Activity) Fields() []ent.Field {
	return []ent.Field{
		field.String("title").Optional(),
		field.String("dateRange").Optional(),
		field.String("location").Optional(),
		field.Int("groupId").Optional().Immutable(),
	}
}

The second is way easier to read than the first.

UPDATE:
golines with gofumpt solves my formatting issue. It would still be nice to have it all be in one tool, but understand if it's out of scope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants