Skip to content

Commit 304f054

Browse files
authored
refactor: Make keep-sorted debug output a bit more useful for figuring out why things were sorted a certain way. (#88)
The debug output now reports all of the tokens that were actually used in comparisons. It also now reports all of the line groups in their sorted order instead of the line groups in their original order, but I think that should be fine. If you want to see it in action, run something like ```sh $ go run . -vv --id keep-sorted-test - < <(cat goldens/*.in) 1>/dev/null ```
1 parent 488401a commit 304f054

File tree

6 files changed

+202
-22
lines changed

6 files changed

+202
-22
lines changed

goldens/by_regex.err

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
WRN while parsing option "by_regex": error parsing regexp: missing argument to repetition operator: `*` line=99
2-
WRN by_regex cannot be used with ignore_prefixes (consider adding a non-capturing group to the start of your regex instead of ignore_prefixes: "(?:foo|bar)") line=106
1+
WRN while parsing option "by_regex": error parsing regexp: missing argument to repetition operator: `*` line=107
2+
WRN by_regex cannot be used with ignore_prefixes (consider adding a non-capturing group to the start of your regex instead of ignore_prefixes: "(?:foo|bar)") line=114
33
exit status 1

goldens/by_regex.in

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@ Multiple regexes
5757
int bar
5858
keep-sorted-test end
5959

60+
One regex, multiple capturing groups
61+
keep-sorted-test start by_regex=(.*)_(.*)
62+
foo_bar
63+
foo_baz
64+
bar_baz
65+
baz_qux
66+
keep-sorted-test end
67+
6068
Multiline blocks
6169
keep-sorted-test start block=yes newline_separated=yes by_regex=(\w+)\(\)\s+{
6270
bool func2() {

goldens/by_regex.out

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@ Multiple regexes
5757
long foo
5858
keep-sorted-test end
5959

60+
One regex, multiple capturing groups
61+
keep-sorted-test start by_regex=(.*)_(.*)
62+
bar_baz
63+
baz_qux
64+
foo_bar
65+
foo_baz
66+
keep-sorted-test end
67+
6068
Multiline blocks
6169
keep-sorted-test start block=yes newline_separated=yes by_regex=(\w+)\(\)\s+{
6270
List<SomeReallyLongTypeParameterThatWouldForceTheFunctionNameOntoTheNextLine>

keepsorted/block.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,8 +235,8 @@ func (b block) sorted() (sorted []string, alreadySorted bool) {
235235
}
236236
}
237237

238+
log.Printf("Creating line groups for block at index %d (options %v)", b.start, b.metadata.opts)
238239
groups := groupLines(lines, b.metadata)
239-
log.Printf("Previous %d groups were for block at index %d are (options %v)", len(groups), b.start, b.metadata.opts)
240240
trimTrailingComma := handleTrailingComma(groups)
241241

242242
numNewlines := int(b.metadata.opts.NewlineSeparated)
@@ -257,7 +257,7 @@ func (b block) sorted() (sorted []string, alreadySorted bool) {
257257
seen := map[string]bool{}
258258
var deduped []*lineGroup
259259
for _, lg := range groups {
260-
if s := lg.joinedLines() + "\n" + strings.Join(lg.comment, "\n"); !seen[s] {
260+
if s := lg.String(); !seen[s] {
261261
seen[s] = true
262262
deduped = append(deduped, lg)
263263
} else {
@@ -268,11 +268,22 @@ func (b block) sorted() (sorted []string, alreadySorted bool) {
268268
}
269269

270270
if alreadySorted && wasNewlineSeparated && !removedDuplicate && slices.IsSortedFunc(groups, compareLineGroups) {
271+
log.Printf("It was already sorted!")
272+
if log.Debug().Enabled() {
273+
for _, lg := range groups {
274+
log.Print(lg.DebugString())
275+
}
276+
}
271277
trimTrailingComma(groups)
272278
return lines, true
273279
}
274280

275281
slices.SortStableFunc(groups, compareLineGroups)
282+
if log.Debug().Enabled() {
283+
for _, lg := range groups {
284+
log.Print(lg.DebugString())
285+
}
286+
}
276287

277288
trimTrailingComma(groups)
278289

keepsorted/line_group.go

Lines changed: 143 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ type lineGroup struct {
3232

3333
// The actual content of the lineGroup.
3434
lineGroupContent
35+
36+
// Track which methods are used during sorting so we can filter debugging
37+
// output to just the parts that are relevant.
38+
access accessRecorder
3539
}
3640

3741
var compareLineGroups = comparingFunc((*lineGroup).commentOnly, falseFirst()).
@@ -50,6 +54,13 @@ type lineGroupContent struct {
5054
lines []string
5155
}
5256

57+
type accessRecorder struct {
58+
commentOnly bool
59+
regexTokens []regexTokenAccessRecorder
60+
joinedLines bool
61+
joinedComment bool
62+
}
63+
5364
// groupLines splits lines into one or more lineGroups based on the provided options.
5465
func groupLines(lines []string, metadata blockMetadata) []*lineGroup {
5566
var groups []*lineGroup
@@ -113,7 +124,6 @@ func groupLines(lines []string, metadata blockMetadata) []*lineGroup {
113124
commentRange = indexRange{}
114125
lineRange = indexRange{}
115126
block = codeBlock{}
116-
log.Printf("%#v", groups[len(groups)-1])
117127
}
118128
for i, l := range lines {
119129
if metadata.opts.Block && !lineRange.empty() && block.expectsContinuation() {
@@ -331,6 +341,7 @@ func findQuote(s string, i int) string {
331341
}
332342

333343
func (lg *lineGroup) append(s string) {
344+
lg.access = accessRecorder{}
334345
lg.lines[len(lg.lines)-1] = lg.lines[len(lg.lines)-1] + s
335346
}
336347

@@ -339,42 +350,53 @@ func (lg *lineGroup) hasSuffix(s string) bool {
339350
}
340351

341352
func (lg *lineGroup) trimSuffix(s string) {
353+
lg.access = accessRecorder{}
342354
lg.lines[len(lg.lines)-1] = strings.TrimSuffix(lg.lines[len(lg.lines)-1], s)
343355
}
344356

345357
func (lg *lineGroup) commentOnly() bool {
358+
lg.access.commentOnly = true
346359
return len(lg.lines) == 0
347360
}
348361

349362
func (lg *lineGroup) regexTokens() []regexToken {
350363
// TODO: jfaer - Should we match regexes on the original content?
351-
regexMatches := lg.opts.matchRegexes(lg.joinedLines())
364+
regexMatches := lg.opts.matchRegexes(lg.internalJoinedLines())
352365
ret := make([]regexToken, len(regexMatches))
366+
if lg.access.regexTokens == nil {
367+
lg.access.regexTokens = make([]regexTokenAccessRecorder, len(regexMatches))
368+
}
353369
for i, match := range regexMatches {
354370
if match == nil {
355371
// Regex did not match.
356372
continue
357373
}
358374

359375
ret[i] = make(regexToken, len(match))
376+
if lg.access.regexTokens[i] == nil {
377+
lg.access.regexTokens[i] = make(regexTokenAccessRecorder, len(match))
378+
}
360379
for j, s := range match {
361-
prefixOrder := lg.prefixOrder
380+
order := lg.prefixOrder
362381
if j != 0 {
363382
// Only try to match PrefixOrder on the first capture group in a regex.
364383
// TODO: jfaer - Should this just be the first capture group in the first regex match?
365-
prefixOrder = nil
384+
order = func() *prefixOrder { return nil }
366385
}
367386
ret[i][j] = &captureGroupToken{
368387
opts: &lg.opts,
369-
prefixOrder: prefixOrder,
388+
prefixOrder: order,
370389
raw: s,
390+
access: &lg.access.regexTokens[i][j],
371391
}
372392
}
373393
}
374394
return ret
375395
}
376396

377-
func (lg *lineGroup) joinedLines() string {
397+
// internalJoinedLines calculates the same thing as joinedLines, except it
398+
// doesn't record that it was used in the accessRecorder.
399+
func (lg *lineGroup) internalJoinedLines() string {
378400
if len(lg.lines) == 0 {
379401
return ""
380402
}
@@ -394,23 +416,58 @@ func (lg *lineGroup) joinedLines() string {
394416
return s.String()
395417
}
396418

419+
func (lg *lineGroup) joinedLines() string {
420+
lg.access.joinedLines = true
421+
return lg.internalJoinedLines()
422+
}
423+
397424
func (lg *lineGroup) joinedComment() string {
425+
lg.access.joinedComment = true
398426
if len(lg.comment) == 0 {
399427
return ""
400428
}
401429
return strings.Join(lg.comment, "\n")
402430
}
403431

404-
func (lg *lineGroup) GoString() string {
405-
var comment strings.Builder
406-
for _, c := range lg.comment {
407-
comment.WriteString(fmt.Sprintf(" %#v\n", c))
432+
func (lg *lineGroup) DebugString() string {
433+
var s strings.Builder
434+
s.WriteString("LineGroup{\n")
435+
if len(lg.comment) > 0 {
436+
s.WriteString("comment=\n")
437+
for _, c := range lg.comment {
438+
fmt.Fprintf(&s, " %#v\n", c)
439+
}
408440
}
409-
var lines strings.Builder
410-
for _, l := range lg.lines {
411-
lines.WriteString(fmt.Sprintf(" %#v\n", l))
441+
if len(lg.lines) > 0 {
442+
s.WriteString("lines=\n")
443+
for _, l := range lg.lines {
444+
fmt.Fprintf(&s, " %#v\n", l)
445+
}
412446
}
413-
return fmt.Sprintf("LineGroup{\ncomment=\n%slines=\n%s}", comment.String(), lines.String())
447+
if lg.access.commentOnly {
448+
fmt.Fprintf(&s, "commentOnly=%t\n", lg.commentOnly())
449+
}
450+
if lg.access.regexTokens != nil {
451+
for i, regex := range lg.regexTokens() {
452+
if regex.wasUsed() {
453+
fmt.Fprintf(&s, "regex[%d]=%s\n", i, regex.DebugString())
454+
}
455+
}
456+
}
457+
if lg.access.joinedLines {
458+
if len(lg.lines) > 1 {
459+
// Only print the joinedLines when they're meaningfully different from the
460+
// raw lines above.
461+
fmt.Fprintf(&s, "joinedLines=%#v\n", lg.joinedLines())
462+
} else if !lg.access.joinedComment {
463+
s.WriteString("linesTiebreaker=true\n")
464+
}
465+
}
466+
if lg.access.joinedComment {
467+
s.WriteString("commentTiebreaker=true\n")
468+
}
469+
s.WriteString("}")
470+
return s.String()
414471
}
415472

416473
func (lg *lineGroup) allLines() []string {
@@ -426,22 +483,66 @@ func (lg *lineGroup) String() string {
426483

427484
type regexToken []*captureGroupToken
428485

486+
type regexTokenAccessRecorder []captureGroupTokenAccessRecorder
487+
488+
func (t regexToken) wasUsed() bool {
489+
if t == nil {
490+
// Report that the regex didn't match.
491+
return true
492+
}
493+
for _, cg := range t {
494+
if cg.wasUsed() {
495+
return true
496+
}
497+
}
498+
return false
499+
}
500+
501+
func (t regexToken) DebugString() string {
502+
if t == nil {
503+
return "<did not match>"
504+
}
505+
506+
captureGroups := make([]string, len(t))
507+
for i, cg := range t {
508+
if cg.wasUsed() {
509+
captureGroups[i] = cg.DebugString()
510+
} else {
511+
captureGroups[i] = "<unused>"
512+
}
513+
}
514+
515+
if len(captureGroups) == 1 {
516+
return captureGroups[0]
517+
}
518+
return fmt.Sprintf("%v", captureGroups)
519+
}
520+
429521
type captureGroupToken struct {
430522
opts *blockOptions
431523
prefixOrder func() *prefixOrder
432524

433525
raw string
526+
527+
access *captureGroupTokenAccessRecorder
528+
}
529+
530+
type captureGroupTokenAccessRecorder struct {
531+
prefix bool
532+
transform bool
434533
}
435534

436535
func (t *captureGroupToken) prefix() orderedPrefix {
437-
if t.prefixOrder == nil {
536+
ord := t.prefixOrder()
537+
if ord == nil {
438538
return orderedPrefix{}
439539
}
440-
441-
return t.prefixOrder().match(t.raw)
540+
t.access.prefix = true
541+
return ord.match(t.raw)
442542
}
443543

444544
func (t *captureGroupToken) transform() numericTokens {
545+
t.access.transform = true
445546
// Combinations of switches (for example, case-insensitive and numeric
446547
// ordering) which must be applied to create a single comparison key,
447548
// otherwise a sub-ordering can preempt a total ordering:
@@ -466,3 +567,28 @@ func (t *captureGroupToken) transform() numericTokens {
466567
}
467568
return t.opts.maybeParseNumeric(s)
468569
}
570+
571+
func (t captureGroupToken) wasUsed() bool {
572+
return t.access.prefix || t.access.transform
573+
}
574+
575+
func (t captureGroupToken) DebugString() string {
576+
var s []string
577+
if t.access.prefix {
578+
s = append(s, fmt.Sprintf("prefix:%q", t.prefix().prefix))
579+
}
580+
if t.access.transform {
581+
var tokens strings.Builder
582+
if len(s) > 0 {
583+
tokens.WriteString("tokens:")
584+
}
585+
fmt.Fprintf(&tokens, "%s", t.transform().DebugString())
586+
s = append(s, tokens.String())
587+
}
588+
589+
ret := strings.Join(s, " ")
590+
if len(s) > 1 {
591+
ret = "[" + ret + "]"
592+
}
593+
return ret
594+
}

keepsorted/options.go

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,25 @@ type numericTokens struct {
473473
i []*big.Int
474474
}
475475

476+
func (t numericTokens) DebugString() string {
477+
s := make([]string, 0, t.len())
478+
for i := 0; i < t.len(); i++ {
479+
if i%2 == 0 {
480+
val := t.s[i/2]
481+
if i == 0 && val == "" {
482+
continue
483+
}
484+
s = append(s, fmt.Sprintf("%#v", t.s[i/2]))
485+
} else {
486+
s = append(s, fmt.Sprintf("%#v", t.i[i/2]))
487+
}
488+
}
489+
if len(s) == 1 {
490+
return s[0]
491+
}
492+
return fmt.Sprintf("%v", s)
493+
}
494+
476495
func (t numericTokens) len() int {
477496
return len(t.s) + len(t.i)
478497
}
@@ -502,6 +521,10 @@ type prefixOrder struct {
502521
}
503522

504523
func newPrefixOrder(opts blockOptions) *prefixOrder {
524+
if len(opts.PrefixOrder) == 0 {
525+
return nil
526+
}
527+
505528
// Assign a weight to each prefix so that they will be sorted into their
506529
// predetermined order.
507530
// Weights are negative so that entries with matching prefixes are put before
@@ -520,7 +543,11 @@ func newPrefixOrder(opts blockOptions) *prefixOrder {
520543
return &prefixOrder{opts, prefixWeights, prefixes}
521544
}
522545

523-
func (o prefixOrder) match(s string) orderedPrefix {
546+
func (o *prefixOrder) match(s string) orderedPrefix {
547+
if o == nil {
548+
return orderedPrefix{}
549+
}
550+
524551
pre, _ := o.opts.hasPrefix(s, slices.Values(o.prefixes))
525552
return orderedPrefix{pre, o.prefixWeights[pre]}
526553
}

0 commit comments

Comments
 (0)