dominikh / go-tools

Staticcheck - The advanced Go linter
https://staticcheck.dev
MIT License
6.01k stars 361 forks source link

structlayout: align output #1540

Closed arp242 closed 4 weeks ago

arp242 commented 1 month ago

This aligns the output of structlayout. To my eyes at least, this is a lot more readable, including more readable than the structlayout-pretty output.

Before:

% structlayout bufio Reader
column.name string: 0-16 (size 16, align 8)
column.width int: 16-24 (size 8, align 8)
column.align int: 24-32 (size 8, align 8)
column.trim bool: 32-33 (size 1, align 1)
column.quote uint8: 33-34 (size 1, align 1)
padding: 34-40 (size 6, align 0)
column.quoteChar [2]string: 40-72 (size 32, align 8)
column.fill rune: 72-76 (size 4, align 4)
column.noHeader bool: 76-77 (size 1, align 1)
padding: 77-80 (size 3, align 0)

% structlayout zgo.at/uni/v2/unidata Emoji
Emoji.Codepoints []rune: 0-24 (size 24, align 8)
Emoji.Name string: 24-40 (size 16, align 8)
Emoji.group zgo.at/uni/v2/unidata.EmojiGroup: 40-41 (size 1, align 1)
padding: 41-42 (size 1, align 0)
Emoji.subgroup zgo.at/uni/v2/unidata.EmojiSubgroup: 42-44 (size 2, align 2)
padding: 44-48 (size 4, align 0)
Emoji.CLDR []string: 48-72 (size 24, align 8)
Emoji.skinTones bool: 72-73 (size 1, align 1)
padding: 73-80 (size 7, align 0)
Emoji.gender int: 80-88 (size 8, align 8)

And after:

% structlayout bufio Reader
Reader.buf           []byte      0-24  (size 24,  align  8)
Reader.rd            io.Reader  24-40  (size 16,  align  8)
Reader.r             int        40-48  (size  8,  align  8)
Reader.w             int        48-56  (size  8,  align  8)
Reader.err           error      56-72  (size 16,  align  8)
Reader.lastByte      int        72-80  (size  8,  align  8)
Reader.lastRuneSize  int        80-88  (size  8,  align  8)

% structlayout zgo.at/uni/v2/unidata Emoji
Emoji.Codepoints  []rune                                0-24  (size 24,  align  8)
Emoji.Name        string                               24-40  (size 16,  align  8)
Emoji.group       zgo.at/uni/v2/unidata.EmojiGroup     40-41  (size  1,  align  1)
  padding                                              41-42  (size  1,  align  0)
Emoji.subgroup    zgo.at/uni/v2/unidata.EmojiSubgroup  42-44  (size  2,  align  2)
  padding                                              44-48  (size  4,  align  0)
Emoji.CLDR        []string                             48-72  (size 24,  align  8)
Emoji.skinTones   bool                                 72-73  (size  1,  align  1)
  padding                                              73-80  (size  7,  align  0)
Emoji.gender      int                                  80-88  (size  8,  align  8)
dominikh commented 1 month ago

If we were to do this we should probably reorder the columns so that long type names don't completely ruin the layout. At the same time, I'm not a fan of putting the type last.

I don't think that padding should be indented, either. That suggests that it belongs to the preceding field when really it's for the next field, or in the case of trailing padding, for the first field.

I'll have to think about this.

Do note that you can achieve pretty similar output right now by piping through column -t -R3,5

arp242 commented 1 month ago

I put the two-space indent on the padding to make it stand out more; because padding is usually what you're interested in. I found this pretty helpful when using it. Alternatively, could align with the type maybe.

I'm not too fussed about the details like column order or indenting the padding; let me know if you want to change anything specific. Or just feel free to close it if you don't like the change; I got my version that works well for me and that's all I really wanted. This small patch is very easy to maintain out-of-tree because structcheck rarely sees changes. That said, personally I do think it's useful for others too, otherwise I wouldn't have sent a patch, but tastes about this sort of thing differs. So don't worry too much about it.

dominikh commented 4 weeks ago

I'll stick with the current output format for the sake of simplicity. Thanks!