You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
With current gofmt rules slight changes to a struct declaration make for avalanche changes across declaration whitespace due to gofmt trying to layout struct declaration "pretty". Produced changeset size make diffs unreadable if any field identifier or type changes its length.
Fix relative indent position of the type of the field, then
Let code author hint gofmt where comments should line up.
Comment-start hint has a form of a /< digraph (a cs-mark hereafter) put into the comment that follows the opening brace of the block.
typeSomestruct { // /< cs-mark here counterint// midlen nceint64// shortlonglongnamestring// long
}
Note: cs-mark usually will be used on a struct declaration, but described feature will work for any kind of block.
Proposed new gofmt rules:
New behavior is turned on by a "/<" cs-mark presence
in a first comment of the block.
The tsPos and csPos below stand for type-start and comment-start
positions, respectively. cmPos stands for cs-mark position.
Tabwidth is fixed at 8. Measurements and adjusts are relative to the
block content start position, ie. are indent-independent.
NONE heuristics are used, gofmt just obey the recipe as set here.
Recipe:
R1. Set tsPos to the tabstop that follows the opening brace position.
R2. if cmPos <= tsPos {; cmPos += 8 } // make sure cmPos > tsPos
R3. If cmPos is not at tabstop now, advance it to the next tabstop.
R4. Set cs-mark position from the current (adjusted) cmPos.
R5. Rewrite cs-mark line.
R6. Adjust lines in block, shifting relevant parts using spaces.
R7. If text overflows one or the other "start" position, use single space.
Human can react to spurious overflows moving comment-mark a space or two.
R8. Estabilished tsPos and csPos are valid to the end of block.
Open question: should we recognize cs-mark in the inner scopes?
On Examples:
// Youri uses tw=8 and writes:
type Other struct { // /< author set marker here
counter int // midlen
nce int64 // short
longlongname string // long
}
// e Other struct { // /< author set comment-start position
// |---------#-----| R1 type starts at next ts
// |-------------------| given comment-start position
// |-------------------#---| R3 adjusted comment-start position
//......A.......B.......C.......D.......E.......F.......G.......H...
//
// Now Youri sees:
type Other struct { // /< adjusted cs-mark
counter int // midlen
nce int64 // short
longlongname string // long
}
// Bartek tw=4 sees:
type Other struct { // /< adjusted cs-mark
counter int // midlen
nce int64 // short
longlongname string // long
}
// Beata tw=2 sees:
type Other struct { // /< adjusted cs-mark
counter int // midlen
nce int64 // short
longlongname string // long
}
// Now Beata writes, she is using tw=2
type Other struct { // /< just near longlongname, as perceived
counter int // midlen
nce int64 // short
longlongname string // long
}
// e Other struct { // /< author set comment-start position
// |---------#-----| R1 type starts at next ts
// |--------------| given comment-start position
// |----------------------| R2 +8 shift
// |--------------#--------| R3 adjusted comment-start position
//......A.......B.......C.......D.......E.......F.......G.......H...
//
// Beata ts=2 now sees:
type Other struct { // /< adjusted
counter int // midlen
nce int64 // short
longlongname string // long
}
// Bartek ts=4 sees:
type Other struct { // /< adjusted
counter int // midlen
nce int64 // short
longlongname string // long
}
// Youri ts=8 sees:
type Other struct { // /< adjusted
counter int // midlen
nce int64 // short
longlongname string // long
}
//
// Excercise R7 rule.
//
// Beata ts=2 writes:
type sn struct { // /< Beata manually adjusted to longlongname
field int // midlen
n int64 // short
longlongname string // r
}
// e sn struct { // /< Beata manually adjusted to longlongname
// |------#| R1 type starts at next ts
// |-------------| given comment-start position
// |-------------#-| R3 adjusted comment-start position
//......A.......B.......C.......D.......E.......F.......G.......H...
//
// Beata ts=2 now sees:
type sn struct { // /< adjusted
field int // midlen
n int64 // short
longlongname string // R7
}
// Bartek ts=4 sees:
type sn struct { // /< adjusted
field int // midlen
n int64 // short
longlongname string // R7
}
// Youri ts=8 sees:
type sn struct { // /< adjusted
field int // midlen
n int64 // short
longlongname string // R7
}
// Beata moved mark by three spaces right:
//
// Beata ts=2 writes:
type sn struct { // /< three spaces more
field int // midlen
n int64 // short
longlongname string // r
}
// e sn struct { // /< three spaces more
// |------#| R1 type starts at next ts
// |----------------| given comment-start position
// |----------------#------| R3 adjusted comment-start position
//......A.......B.......C.......D.......E.......F.......G.......H...
// tw=2
type sn struct { // /< adjusted
field int // midlen
n int64 // short
longlongname string // R7 for type
}
// tw=4
type sn struct { // /< adjusted
field int // midlen
n int64 // short
longlongname string // R7 for type
}
// tw=8
type sn struct { // /< adjusted
field int // midlen
n int64 // short
longlongname string // R7 for type
}
Caveat: If the block opening brace changes position pre or past its current tabstop span (eg. if a struct was renamed), whole block will make to the changeset. These changes though are less likely than changes to the inner structure.
My first impression is that this proposal adds a fair bit of complexity to gofmt without a whole lot of benefit. I don't believe there exists any other mechanisms like this that allow you to influence gofmt's output, and I see that as mostly a good thing.
When I write a struct with many fields, I often find myself dividing them up into logical sections. Since sections of fields with a blank line or comment in between are grouped independently of each other, I don't end up with massive diffs every time I add or change a field name. This isn't something I do to appease gofmt, it's just a way I find helps me organize the fields.
I think it's also worth mentioning that it is possible in most cases to hide whitespace-only changes when diffing. For Git, that's git diff -w. GitHub has an option to hide whitespace changes under a settings gear icon in the "Files Changed" view.
I'm not saying there isn't a problem here. I've certainly seen my fair share of Go diffs that are messier than they need to be because of this. However, I think it will be difficult to find a change to gofmt whose complexity or disruptiveness can be justified to solve what I find to be no more than a minor annoyance. For me, this proposal doesn't meet that bar.
ohir
changed the title
cmd/gofmt: proposal: Minimize adjacent whitespace changes on struct layout edits.
cmd/gofmt: proposal: Minimize adjacent whitespace changes on struct layout edits (user controlled positions).
Feb 24, 2020
I think it's also worth mentioning that it is possible in most cases to hide whitespace-only changes when diffing.
Not only humans do diffs. This proposal was about making changesets smaller in the repo, so to have changeset truly reflecting changes made by a human author.
I'm not saying there isn't a problem here. [...]
I think it will be difficult to find a change to gofmt whose complexity or disruptiveness can be justified to solve what I find to be no more than a minor annoyance.
Proposal: Minimize adjacent whitespace changes on struct layout edits.
Author: Ohir Ripe [Wojciech S. Czarnecki]
Last updated: 2020/02/24
Discussion at https://golang.org/issues/37299
Abstract
I propose to add an opt-in go fmt solution meant to minimize whitespace changes to the block where a code author hinted at desired comments position.
Background
With current gofmt rules slight changes to a struct declaration make for avalanche changes across declaration whitespace due to gofmt trying to layout struct declaration "pretty". Produced changeset size make diffs unreadable if any field identifier or type changes its length.
Proposal
Comment-start hint has a form of a
/<
digraph (a cs-mark hereafter) put into the comment that follows the opening brace of the block.Note: cs-mark usually will be used on a struct declaration, but described feature will work for any kind of block.
Proposed new gofmt rules:
Caveat: If the block opening brace changes position pre or past its current tabstop span (eg. if a struct was renamed), whole block will make to the changeset. These changes though are less likely than changes to the inner structure.
Rationale
As explained in the background. This change will benefit Go programmers that use source control. Ie. likely all of them.
Compatibility
This is an opt-in feature. It does not affect already written code.
Implementation
None yet. It can successfully be implemented only by the core team.
(Because such a change makes sense if implemented across the whole Go ecosystem.)
Open issues
Edits
The text was updated successfully, but these errors were encountered: