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: address formatting oddities of composite literals with irregularly sized entries #22852
Comments
This is by design. Search the issue tracker where @griesemer has explained it a few times. In general, the best strategy to being happy about gofmt's choices is to not worry about it. Maybe this needs a FAQ entry. I'll let @griesemer decide whether to keep this bug open as a documentation bug. |
Fine to leave this open as documentation issue. |
Looking at this again, I admit that the formatting looks pretty ugly in this case. Also, given that this very issue has come up so many times, it may be worthwhile re-investigating the formatting of literals of this form. There's a few solutions I see:
|
In my opinion, the align all option would be easier for programmers to reason with and understand. 👍 |
Change https://golang.org/cl/104755 mentions this issue: |
Change https://golang.org/cl/104836 mentions this issue: |
Issue golang/go#22852 modifies formatting for Go tables. If the code stays as is, users running Go 1.9 will format the file one way and users running tip will format the file a different way. This is problematic when git-codereview attempts to check the file is gofmt'ed correctly, and does not let you mail commits that aren't. Adding another newline makes the file compatible with tip and the stable release branch and avoids this problem. Change-Id: I0dbfe01db6e07578993e793e3710702ff2518436 Reviewed-on: https://go-review.googlesource.com/104836 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The go/printer (and thus gofmt) uses a heuristic to determine whether to break alignment between elements of an expression list which is spread across multiple lines. The heuristic only kicked in if the entry sizes (character length) was above a certain threshold (20) and the ratio between the previous and current entry size was above a certain value (4). This heuristic worked reasonably most of the time, but also led to unfortunate breaks in many cases where a single entry was suddenly much smaller (or larger) then the previous one. The behavior of gofmt was sufficiently mysterious in some of these situations that many issues were filed against it. The simplest solution to address this problem is to remove the heuristic altogether and have a programmer introduce empty lines to force different alignments if it improves readability. The problem with that approach is that the places where it really matters, very long tables with many (hundreds, or more) entries, may be machine-generated and not "post-processed" by a human (e.g., unicode/utf8/tables.go). If a single one of those entries is overlong, the result would be that the alignment would force all comments or values in key:value pairs to be adjusted to that overlong value, making the table hard to read (e.g., that entry may not even be visible on screen and all other entries seem spaced out too wide). Instead, we opted for a slightly improved heuristic that behaves much better for "normal", human-written code. 1) The threshold is increased from 20 to 40. This disables the heuristic for many common cases yet even if the alignment is not "ideal", 40 is not that many characters per line with todays screens, making it very likely that the entire line remains "visible" in an editor. 2) Changed the heuristic to not simply look at the size ratio between current and previous line, but instead considering the geometric mean of the sizes of the previous (aligned) lines. This emphasizes the "overall picture" of the previous lines, rather than a single one (which might be an outlier). 3) Changed the ratio from 4 to 2.5. Now that we ignore sizes below 40, a ratio of 4 would mean that a new entry would have to be 4 times bigger (160) or smaller (10) before alignment would be broken. A ratio of 2.5 seems more sensible. Applied updated gofmt to all of src and misc. Also tested against several former issues that complained about this and verified that the output for the given examples is satisfactory (added respective test cases). Some of the files changed because they were not gofmt-ed in the first place. For golang#644. For golang#7335. For golang#10392. (and probably more related issues) Fixes golang#22852. Change-Id: I5e48b3d3b157a5cf2d649833b7297b33f43a6f6e
The go/printer (and thus gofmt) uses a heuristic to determine whether to break alignment between elements of an expression list which is spread across multiple lines. The heuristic only kicked in if the entry sizes (character length) was above a certain threshold (20) and the ratio between the previous and current entry size was above a certain value (4). This heuristic worked reasonably most of the time, but also led to unfortunate breaks in many cases where a single entry was suddenly much smaller (or larger) then the previous one. The behavior of gofmt was sufficiently mysterious in some of these situations that many issues were filed against it. The simplest solution to address this problem is to remove the heuristic altogether and have a programmer introduce empty lines to force different alignments if it improves readability. The problem with that approach is that the places where it really matters, very long tables with many (hundreds, or more) entries, may be machine-generated and not "post-processed" by a human (e.g., unicode/utf8/tables.go). If a single one of those entries is overlong, the result would be that the alignment would force all comments or values in key:value pairs to be adjusted to that overlong value, making the table hard to read (e.g., that entry may not even be visible on screen and all other entries seem spaced out too wide). Instead, we opted for a slightly improved heuristic that behaves much better for "normal", human-written code. 1) The threshold is increased from 20 to 40. This disables the heuristic for many common cases yet even if the alignment is not "ideal", 40 is not that many characters per line with todays screens, making it very likely that the entire line remains "visible" in an editor. 2) Changed the heuristic to not simply look at the size ratio between current and previous line, but instead considering the geometric mean of the sizes of the previous (aligned) lines. This emphasizes the "overall picture" of the previous lines, rather than a single one (which might be an outlier). 3) Changed the ratio from 4 to 2.5. Now that we ignore sizes below 40, a ratio of 4 would mean that a new entry would have to be 4 times bigger (160) or smaller (10) before alignment would be broken. A ratio of 2.5 seems more sensible. Applied updated gofmt to all of src and misc. Also tested against several former issues that complained about this and verified that the output for the given examples is satisfactory (added respective test cases). Some of the files changed because they were not gofmt-ed in the first place. For golang#644. For golang#7335. For golang#10392. (and probably more related issues) Fixes golang#22852. Change-Id: I5e48b3d3b157a5cf2d649833b7297b33f43a6f6e
What version of Go are you using (
go version
)?1.9.2
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?Windows 10 (1709) 64-bit
What did you do?
Created maps with long keys.
^ Hit the playground's
format
button to reproduce the issueWhat did you expect to see?
Blocks formatted consistently
What did you see instead?
Broken Indentation blocks when running Go Fmt.
^ Hit the playground's
format
button to reproduce the issueThe text was updated successfully, but these errors were encountered: