format: break long lines if it's easy to do so #70

mvdan · 2020-05-12T14:20:03Z

WORK IN PROGRESS.

See the test cases for what the intended effect is.

mvdan · 2020-05-12T14:27:50Z

To try it out, you can do the usual git clone plus git checkout and go install, or you can just:

GO111MODULE=on go get mvdan.cc/gofumpt@break-long-lines

Southclaws · 2020-05-12T14:46:16Z

Love it!

However, a couple of little things I ran into:

This case split a slice of long lines up but I think I would have preferred it to go all the way instead of splitting it just once, into this:

Because I think the staggerred nature of structures like this definitely makes it a bit awkward to read. There are three fields on one line, then one on the next line.

Other than that, it's working great! I ran it on a fairly large codebase and didn't find any other issues. Nice work!

remorses · 2020-05-12T18:14:19Z

Very cool 🔥

one thing i would really like is to transform one like structs like this

users, err := m.FindMany(mongoke.FindManyParams{Collection: collection, DatabaseUri: uri, Direction: mongoke.DESC, Pagination: mongoke.Pagination{First: 2}})

into a "one line per property" struct, where each nested property has one more indentation tab

users, err := m.FindMany(
	mongoke.FindManyParams{
		Collection:  collection,
		DatabaseUri: uri,
		Direction:   mongoke.DESC,
		Pagination: mongoke.Pagination{
			First: 2,
		},
	},
)

currently it does this, still better than go fmt

users, err := m.FindMany(mongoke.FindManyParams{Collection: collection,
	DatabaseUri: uri, Direction: mongoke.DESC, Pagination: mongoke.Pagination{First: 2}})

mvdan · 2020-05-12T22:06:39Z

Thanks both - I think that's a bug, and you should see a better format if you re-run gofumpt again. The fact that it needs two runs is the bug.

mvdan · 2020-05-12T22:48:54Z

OK, the bug above is now fixed. The tests also properly verify that the output does't change if the tool is re-run.

Southclaws · 2020-05-13T15:14:55Z

Ah, that explains why I ran it manually, then CTRL+S'd a file and it changed.

mvdan · 2020-05-13T15:16:48Z

Rebased to get the fix for #72.

blackb1rd · 2020-11-01T09:46:10Z

format/format.go

 const shortLineLimit = 60

+// Single-line nodes which take over this many bytes, and could easily be split
+// into two lines of at least its 40%, may be split.
+const longLineBase = 100


it would be better if User could be able to define how long the line.

See #2 (comment) - we can't and shouldn't add options.

Consider 80.

So first, something to know about me: I am an eighty column purist. For me, this has nothing to do with punchcards or whatnot, but rather with type readability, which tends to result in 50-100 characters per line — and generally about 70 or so. (I would redirect rebuttals to your bookshelf, where most any line of most any page of most any book will show this to be more or less correct.) So I personally embrace the “hard 80″, and have found that the rework that that can sometimes require results in more readable, better factored code.

http://dtrace.org/blogs/bmc/2020/10/11/rust-after-the-honeymoon/

I picked some random text books from my bookshelf and found characters per line of 83, 83, 86, 78, 82.

Prettier

Prettier uses 80: https://prettier.io/docs/en/options.html#print-width

Prettier is only partly relevant here, since:

Golang style should probably not be directly influenced by JavaScript/TypeScript style.

Prettier sometimes makes lines longer than 80 (even though it generally does keep lines below that).

With that said, JavaScript/TypeScript is the 2nd most popular language in the world, and Prettier is (probably?) the most popular code formatter in the world.

2 Files Side by Side

I think that one of the main practical reasons that people prefer lines of 80 characters is that you can open two files side by side on the same monitor and have all of the code fully visible on both windows. This is useful because you can directly compare two files side by side without having to do any horizontal scrolling. Historically, I know that people have used this setup with two VIM terminals, but it is also relevant with e.g. VSCode.

Excited to see this fork merged.

Maybe we'll go with 90 or even 80 if 100 works well and people want it to be even stricter. I definitely think that 80 would be too short to start with, especially given how Go has historically had no convention about line lengths.

80 is way short in "this day and age" imo. 90-120 is far nicer in my experience. Generally I go for 120 but that might be a bit long for side-by-side.

I use 100. For those arguing on what print media uses, keep in mind that code doesn't look the same as written language because you have indentation. Sure, my lines are 100 characters long but the first 4, 8, 16, etc columns are just tabs/spaces!

FWIW I also prefer 100

slimsag · 2021-01-01T20:43:13Z

I am noticing this branch is formatting this code:

package main

import "parse"

func main() {
	v := &parse.Error{Offset: 11, Parsing: parse.NodeType("OptionsCombyCode"), Message: ": valid options are (delimiter), found case"}
	_ = v
}

Into this:

package main

import "parse"

func main() {
	v := &parse.Error{
		Offset: 11, Parsing: parse.NodeType("OptionsCombyCode"),
		Message: ": valid options are (delimiter), found case",
	}
	_ = v
}

Note how Offset and Parsing fields are specified on the same line. I think it would be better formatted as one-field-per-line. Is this a bug or intended behavior?

mvdan · 2021-01-01T20:45:27Z

Sounds like a bug, thanks for pointing it out and providing a repro. I'll take a look after rebasing and updating this branch.

slimsag · 2021-01-01T21:03:37Z

Here is a slightly more complex test case in case it's helpful:

package main

type fooStruct struct {
	SomeLongField interface{}
}

func main() {
	v := &fooStruct{SomeLongField: &fooStruct{SomeLongField: &fooStruct{SomeLongField: &fooStruct{SomeLongField: "foobar"}}}}
	_ = v
}

Becomes:

	v := &fooStruct{SomeLongField: &fooStruct{SomeLongField: &fooStruct{
		SomeLongField: &fooStruct{SomeLongField: "foobar"},
	}}}

This is a hack (but a quite functional one) to split composite literals across multiple lines to avoid extra long struct values - which are a major problem with Valast today and e.g. block me from releasing the [autogold](https://github.com/hexops/autogold) package. We will defer this to gofumpt [once it can perform this functionality.](mvdan/gofumpt#70) Signed-off-by: Stephen Gutekanst <stephen@hexops.com>

mvdan · 2021-01-07T20:01:44Z

Alright, rebased. Here is a summary of the changes from master that this branch had been missing since... May 2020 🙈 abc0db2...7e21c17

I'll be putting some more work into this branch in the coming days. For now, please update to the latest version here and let me know if it still works for you.

mvdan/gofumpt#70 Signed-off-by: Stephen Gutekanst <stephen@hexops.com>

…ne formatting) * 66% performance improvement: hexops/valast#10 * Fixes performance issues on very large structures: #16 * Improves formatting of very long lines: mvdan/gofumpt#70 (comment) Signed-off-by: Stephen Gutekanst <stephen@hexops.com>

Note that this feature is guarded behind GOFUMPT_SPLIT_LONG_LINES=on, as this feature is still experimental and needs more testing and tweaking before it's turned on by default. In summary, if a line takes more than 100 characters, and can be easily split into two lines of at least about 40 characters, we add a newline. These numbers are a bit arbitrary and will likely be tweaked over time, as more testing happens. The basic idea is to split long lines which have a nice way of being split, but not the others, even if they are past any line length limit. There are also some special cases where the rules are bent to result in better formatting. For the time being, we won't document those, as they're all still fairly experimental and likely to change. Updates #2.

mvdan · 2021-03-11T16:43:46Z

I think I've thought about this for long enough, and it's time to merge and start testing it. Over the last few days I've also done some tweaks after testing the code on a bunch of my projects.

I'll be posting instructions on #2 for how to install the latest version and test the feature :)

mvdan changed the title ~~format: break long lines~~ WIP: format: break long lines May 12, 2020

mvdan mentioned this pull request May 12, 2020

break long lines when it's appropriate to do so #2

Open

mvdan force-pushed the break-long-lines branch from d21ad3b to 51d3b1b Compare May 12, 2020 22:48

mvdan force-pushed the break-long-lines branch from 51d3b1b to 8be3105 Compare May 13, 2020 15:16

mvdan mentioned this pull request May 16, 2020

proposal: add option to wrap lines to fit in N columns on a best-effort basis mvdan/sh#80

Open

blackb1rd reviewed Nov 1, 2020

View reviewed changes

Zamiell mentioned this pull request Dec 11, 2020

Single import declarations should be grouped with parentheses #97

Open

This was referenced Jan 2, 2021

use more newlines when formatting very complex composite literals #104

Open

use gofumpt to format String output hexops/valast#4

Merged

slimsag mentioned this pull request Jan 3, 2021

format composite literals more nicely hexops/valast#5

Merged

slimsag mentioned this pull request Jan 6, 2021

valast: slice got turns to different output than non-slice hexops/autogold#5

Open

mvdan force-pushed the break-long-lines branch from 8be3105 to d24d34e Compare January 7, 2021 20:00

slimsag mentioned this pull request Jan 11, 2021

search: parser tests use autogold snapshot values sourcegraph/sourcegraph#17176

Merged

slimsag added a commit to hexops/valast that referenced this pull request Feb 18, 2021

update to latest gofumpt (long line formatter improvements)

cd93bbd

mvdan/gofumpt#70 Signed-off-by: Stephen Gutekanst <stephen@hexops.com>

mvdan force-pushed the break-long-lines branch from d24d34e to c21a30b Compare March 11, 2021 10:37

mvdan changed the title ~~WIP: format: break long lines~~ format: break long lines if it's easy to do so Mar 11, 2021

mvdan force-pushed the break-long-lines branch from c21a30b to f6fc055 Compare March 11, 2021 16:42

mvdan merged commit 0c61c1e into master Mar 11, 2021

mvdan deleted the break-long-lines branch October 3, 2021 21:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

format: break long lines if it's easy to do so #70

format: break long lines if it's easy to do so #70

mvdan commented May 12, 2020

mvdan commented May 12, 2020

Southclaws commented May 12, 2020

remorses commented May 12, 2020

mvdan commented May 12, 2020

mvdan commented May 12, 2020

Southclaws commented May 13, 2020

mvdan commented May 13, 2020

blackb1rd Nov 1, 2020

mvdan Nov 1, 2020

steshaw Nov 3, 2020

Zamiell Nov 28, 2020

mvdan Nov 28, 2020

A-UNDERSCORE-D Dec 1, 2020

Southclaws Dec 1, 2020

slimsag Dec 31, 2020

slimsag commented Jan 1, 2021

mvdan commented Jan 1, 2021

slimsag commented Jan 1, 2021

mvdan commented Jan 7, 2021 •

edited

mvdan commented Mar 11, 2021

format: break long lines if it's easy to do so #70

format: break long lines if it's easy to do so #70

Conversation

mvdan commented May 12, 2020

mvdan commented May 12, 2020

Southclaws commented May 12, 2020

remorses commented May 12, 2020

mvdan commented May 12, 2020

mvdan commented May 12, 2020

Southclaws commented May 13, 2020

mvdan commented May 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Prettier

2 Files Side by Side

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slimsag commented Jan 1, 2021

mvdan commented Jan 1, 2021

slimsag commented Jan 1, 2021

mvdan commented Jan 7, 2021 • edited

mvdan commented Mar 11, 2021

mvdan commented Jan 7, 2021 •

edited