From dc3da221dbfc3438a9469a8b98bc3a4819390060 Mon Sep 17 00:00:00 2001 From: Tiago Peczenyj Date: Tue, 26 May 2026 14:21:24 +0200 Subject: [PATCH 1/6] feat: add -sort to order diff findings by bytes saved -sort (default off) collects findings across all scanned packages and presents them largest-first by absolute bytes saved (OldSize-NewSize), using a stable sort so equal-savings structs keep source order and unknown-size findings sink to the bottom. Diff mode only for now; inspect support follows. Part of #30 Co-Authored-By: Claude Opus 4.7 --- README.md | 1 + internal/app/app.go | 8 +++++ internal/app/sort_test.go | 62 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 internal/app/sort_test.go diff --git a/README.md b/README.md index 5141b02..20d68a8 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,7 @@ structalign [flags] [packages] -verbose in -inspect mode, show padding on its own `_` line -tags preserve struct field tags in output (default: strip them) -summary in diff mode, print a one-line summary after the diffs + -sort in diff mode, present structs largest-first (by bytes saved) -type string only consider named structs matching these comma-separated glob patterns (e.g. "*Request,Config"); empty means all diff --git a/internal/app/app.go b/internal/app/app.go index af16598..4e69798 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -71,6 +71,7 @@ type options struct { generated bool skipCachePadded bool summary bool + sort bool } // savings is the absolute bytes a finding saves, or 0 when sizes are unknown or @@ -106,6 +107,7 @@ func (a *App) Run(args []string) int { fs.BoolVar(&opt.generated, "generated", false, "also analyze generated files (// Code generated ... DO NOT EDIT.)") fs.BoolVar(&opt.skipCachePadded, "skip-cache-padded", false, "skip structs containing a golang.org/x/sys/cpu.CacheLinePad field") fs.BoolVar(&opt.summary, "summary", false, "in diff mode, print a one-line summary after the diffs") + fs.BoolVar(&opt.sort, "sort", false, "present results largest-first (diff: by bytes saved)") fs.Usage = func() { fmt.Fprintf(a.Stderr, "structalign: print field-aligned struct reorderings (no file changes)\n\n") fmt.Fprintf(a.Stderr, "usage: structalign [flags] [packages]\n\n") @@ -195,6 +197,12 @@ func (a *App) Run(args []string) int { } } + if opt.sort && !opt.inspect { + sort.SliceStable(allFindings, func(i, j int) bool { + return savings(allFindings[i]) > savings(allFindings[j]) + }) + } + var total int if opt.inspect { total = printer.RenderLayouts(allLayouts, opt.verbose, opt.tags) diff --git a/internal/app/sort_test.go b/internal/app/sort_test.go new file mode 100644 index 0000000..d696b06 --- /dev/null +++ b/internal/app/sort_test.go @@ -0,0 +1,62 @@ +package app_test + +import ( + "bytes" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "github.com/peczenyj/structalign/internal/align" + "github.com/peczenyj/structalign/internal/app" + "github.com/peczenyj/structalign/internal/layout" + "github.com/peczenyj/structalign/internal/mocks" + "github.com/peczenyj/structalign/internal/testutil" + "github.com/peczenyj/structalign/pkg/common" +) + +// Small saves 8 bytes (24->16); Big saves 16 (40->24) — more interleaved fields. +const sortSrc = `package sample + +type Small struct { + A bool + B int64 + C bool +} + +type Big struct { + A bool + B int64 + C bool + D int64 + E bool +} +` + +func sortApp(t *testing.T, out, errb *bytes.Buffer) *app.App { + t.Helper() + tgt := testutil.Target(t, sortSrc) + ml := mocks.NewLoader(t) + ml.EXPECT().Load(mock.Anything).Return([]common.Target{tgt}, nil) + return &app.App{Loader: ml, Aligner: align.New(), Inspector: layout.New(), Stdout: out, Stderr: errb} +} + +func TestRunSortDiffOrdersBySavings(t *testing.T) { + var out, errb bytes.Buffer + a := sortApp(t, &out, &errb) + code := a.Run([]string{"-sort", "pkg"}) + assert.Equal(t, 1, code) + s := out.String() + assert.Less(t, strings.Index(s, "Big"), strings.Index(s, "Small"), + "with -sort, the bigger-saving struct must render first") +} + +func TestRunDiffDefaultOrderUnchanged(t *testing.T) { + var out, errb bytes.Buffer + a := sortApp(t, &out, &errb) + _ = a.Run([]string{"pkg"}) + s := out.String() + assert.Less(t, strings.Index(s, "Small"), strings.Index(s, "Big"), + "without -sort, source order (Small before Big) is preserved") +} From e7748d8027e88b809ed34c39d95fd729fd85daca Mon Sep 17 00:00:00 2001 From: Tiago Peczenyj Date: Tue, 26 May 2026 14:31:14 +0200 Subject: [PATCH 2/6] feat: extend -sort to order inspect layouts by struct size In -inspect mode, -sort now collects all layouts across packages and orders them largest-first by Layout.Total (stable sort). Updates the flag help to cover both modes. Diff behavior unchanged. Closes #30 Co-Authored-By: Claude Opus 4.7 --- README.md | 3 ++- internal/app/app.go | 16 ++++++++----- internal/app/sort_test.go | 47 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 20d68a8..1077063 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,8 @@ structalign [flags] [packages] -verbose in -inspect mode, show padding on its own `_` line -tags preserve struct field tags in output (default: strip them) -summary in diff mode, print a one-line summary after the diffs - -sort in diff mode, present structs largest-first (by bytes saved) + -sort present results largest-first (diff: by bytes saved; + inspect: by struct size) -type string only consider named structs matching these comma-separated glob patterns (e.g. "*Request,Config"); empty means all diff --git a/internal/app/app.go b/internal/app/app.go index 4e69798..2873d74 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -107,7 +107,7 @@ func (a *App) Run(args []string) int { fs.BoolVar(&opt.generated, "generated", false, "also analyze generated files (// Code generated ... DO NOT EDIT.)") fs.BoolVar(&opt.skipCachePadded, "skip-cache-padded", false, "skip structs containing a golang.org/x/sys/cpu.CacheLinePad field") fs.BoolVar(&opt.summary, "summary", false, "in diff mode, print a one-line summary after the diffs") - fs.BoolVar(&opt.sort, "sort", false, "present results largest-first (diff: by bytes saved)") + fs.BoolVar(&opt.sort, "sort", false, "present results largest-first (diff: by bytes saved; inspect: by struct size)") fs.Usage = func() { fmt.Fprintf(a.Stderr, "structalign: print field-aligned struct reorderings (no file changes)\n\n") fmt.Fprintf(a.Stderr, "usage: structalign [flags] [packages]\n\n") @@ -197,10 +197,16 @@ func (a *App) Run(args []string) int { } } - if opt.sort && !opt.inspect { - sort.SliceStable(allFindings, func(i, j int) bool { - return savings(allFindings[i]) > savings(allFindings[j]) - }) + if opt.sort { + if opt.inspect { + sort.SliceStable(allLayouts, func(i, j int) bool { + return allLayouts[i].Total > allLayouts[j].Total + }) + } else { + sort.SliceStable(allFindings, func(i, j int) bool { + return savings(allFindings[i]) > savings(allFindings[j]) + }) + } } var total int diff --git a/internal/app/sort_test.go b/internal/app/sort_test.go index d696b06..8858102 100644 --- a/internal/app/sort_test.go +++ b/internal/app/sort_test.go @@ -60,3 +60,50 @@ func TestRunDiffDefaultOrderUnchanged(t *testing.T) { assert.Less(t, strings.Index(s, "Small"), strings.Index(s, "Big"), "without -sort, source order (Small before Big) is preserved") } + +// inspectSortSrc names the small struct alphabetically first (Alpha, 24 bytes) +// and the large one last (Zeta, 40 bytes), so alphabetical order (the inspect +// default) and size-descending order genuinely differ. +const inspectSortSrc = `package sample + +type Alpha struct { + A bool + B int64 + C bool +} + +type Zeta struct { + A bool + B int64 + C bool + D int64 + E bool +} +` + +func inspectSortApp(t *testing.T, out, errb *bytes.Buffer) *app.App { + t.Helper() + tgt := testutil.Target(t, inspectSortSrc) + ml := mocks.NewLoader(t) + ml.EXPECT().Load(mock.Anything).Return([]common.Target{tgt}, nil) + return &app.App{Loader: ml, Aligner: align.New(), Inspector: layout.New(), Stdout: out, Stderr: errb} +} + +func TestRunSortInspectOrdersBySize(t *testing.T) { + var out, errb bytes.Buffer + a := inspectSortApp(t, &out, &errb) + code := a.Run([]string{"-inspect", "-sort", "pkg"}) + assert.Equal(t, 0, code) + s := out.String() + assert.Less(t, strings.Index(s, "type Zeta"), strings.Index(s, "type Alpha"), + "with -inspect -sort, the larger struct (Zeta, 40) renders before Alpha (24)") +} + +func TestRunInspectDefaultOrderUnchanged(t *testing.T) { + var out, errb bytes.Buffer + a := inspectSortApp(t, &out, &errb) + _ = a.Run([]string{"-inspect", "pkg"}) + s := out.String() + assert.Less(t, strings.Index(s, "type Alpha"), strings.Index(s, "type Zeta"), + "without -sort, inspect keeps its default (alphabetical) order: Alpha before Zeta") +} From 2a38ce5b12194eb775bebb1daca7c7056eca4fa6 Mon Sep 17 00:00:00 2001 From: Tiago Peczenyj Date: Tue, 26 May 2026 14:32:38 +0200 Subject: [PATCH 3/6] feat: add -threshold to filter diff findings by bytes saved -threshold N (int, default 0, diff-only) drops findings that save fewer than N bytes. Negative values are treated as 0 (no filtering). The filter runs before sort and summary, so -summary counts only the kept structs and the exit code is 0 when everything is filtered out. Closes #31 Co-Authored-By: Claude Opus 4.7 --- README.md | 2 ++ internal/app/app.go | 13 ++++++++ internal/app/threshold_test.go | 55 ++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 internal/app/threshold_test.go diff --git a/README.md b/README.md index 1077063..f0f1580 100644 --- a/README.md +++ b/README.md @@ -103,6 +103,8 @@ structalign [flags] [packages] -summary in diff mode, print a one-line summary after the diffs -sort present results largest-first (diff: by bytes saved; inspect: by struct size) + -threshold int in diff mode, only show structs that save at least N bytes + (default 0; negatives treated as 0) -type string only consider named structs matching these comma-separated glob patterns (e.g. "*Request,Config"); empty means all diff --git a/internal/app/app.go b/internal/app/app.go index 2873d74..e585b7a 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -72,6 +72,7 @@ type options struct { skipCachePadded bool summary bool sort bool + threshold int } // savings is the absolute bytes a finding saves, or 0 when sizes are unknown or @@ -108,6 +109,7 @@ func (a *App) Run(args []string) int { fs.BoolVar(&opt.skipCachePadded, "skip-cache-padded", false, "skip structs containing a golang.org/x/sys/cpu.CacheLinePad field") fs.BoolVar(&opt.summary, "summary", false, "in diff mode, print a one-line summary after the diffs") fs.BoolVar(&opt.sort, "sort", false, "present results largest-first (diff: by bytes saved; inspect: by struct size)") + fs.IntVar(&opt.threshold, "threshold", 0, "in diff mode, only show structs that save at least this many bytes") fs.Usage = func() { fmt.Fprintf(a.Stderr, "structalign: print field-aligned struct reorderings (no file changes)\n\n") fmt.Fprintf(a.Stderr, "usage: structalign [flags] [packages]\n\n") @@ -197,6 +199,17 @@ func (a *App) Run(args []string) int { } } + if !opt.inspect && opt.threshold > 0 { + min := int64(opt.threshold) + kept := allFindings[:0] + for _, f := range allFindings { + if savings(f) >= min { + kept = append(kept, f) + } + } + allFindings = kept + } + if opt.sort { if opt.inspect { sort.SliceStable(allLayouts, func(i, j int) bool { diff --git a/internal/app/threshold_test.go b/internal/app/threshold_test.go new file mode 100644 index 0000000..d6eb1c1 --- /dev/null +++ b/internal/app/threshold_test.go @@ -0,0 +1,55 @@ +package app_test + +import ( + "bytes" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +// These reuse sortApp / sortSrc from sort_test.go, where Small saves 8 bytes +// (24->16) and Big saves 16 (40->24). + +func TestRunThresholdFiltersSmallSavings(t *testing.T) { + var out, errb bytes.Buffer + a := sortApp(t, &out, &errb) + // 16 is inclusive: keeps Big (saves 16), drops Small (saves 8). + code := a.Run([]string{"-threshold=16", "pkg"}) + assert.Equal(t, 1, code) + s := out.String() + assert.Contains(t, s, "Big") + assert.NotContains(t, s, "Small") +} + +func TestRunThresholdZeroShowsAll(t *testing.T) { + var out, errb bytes.Buffer + a := sortApp(t, &out, &errb) + _ = a.Run([]string{"-threshold=0", "pkg"}) + s := out.String() + assert.Contains(t, s, "Big") + assert.Contains(t, s, "Small") +} + +func TestRunThresholdNegativeIsZero(t *testing.T) { + var out, errb bytes.Buffer + a := sortApp(t, &out, &errb) + _ = a.Run([]string{"-threshold=-5", "pkg"}) + assert.Contains(t, out.String(), "Small", "a negative threshold behaves like 0 (no filtering)") +} + +func TestRunThresholdExitsZeroWhenAllFiltered(t *testing.T) { + var out, errb bytes.Buffer + a := sortApp(t, &out, &errb) + code := a.Run([]string{"-threshold=10000", "pkg"}) + assert.Equal(t, 0, code, "all filtered out => no findings => exit 0") + assert.Equal(t, "", strings.TrimSpace(out.String())) +} + +func TestRunThresholdSummaryReflectsFiltered(t *testing.T) { + var out, errb bytes.Buffer + a := sortApp(t, &out, &errb) + _ = a.Run([]string{"-threshold=16", "-summary", "pkg"}) + assert.Contains(t, out.String(), "Summary: 1 struct affected, 16 bytes saved", + "summary counts only the structs that pass the threshold") +} From 5e04c325d131dc2d1ebfead71eb3fd9638bb4426 Mon Sep 17 00:00:00 2001 From: Tiago Peczenyj Date: Tue, 26 May 2026 14:42:25 +0200 Subject: [PATCH 4/6] feat: respect //nolint directives on struct types by default In diff mode, a struct whose type declaration carries a recognized //nolint directive is now suppressed by default, matching golangci-lint. Recognized: the named token "fieldalignment" (configurable via -nolint-linters) and a bare //nolint (always honored). -show-nolint reveals suppressed structs. Detection reads the type's doc comment (TypeSpec.Doc / grouped GenDecl.Doc) and any comment on the type's opening line (catching a trailing `type T struct { //nolint`, which the AST doesn't attach to TypeSpec.Comment). Inspect mode ignores these directives. New common.Options fields RespectNolint/NolintLinters carry the policy into internal/align. Closes #32 Co-Authored-By: Claude Opus 4.7 --- README.md | 12 ++++ internal/align/align.go | 10 ++- internal/align/nolint.go | 123 ++++++++++++++++++++++++++++++++++ internal/align/nolint_test.go | 85 +++++++++++++++++++++++ internal/app/app.go | 18 +++++ internal/app/nolint_test.go | 66 ++++++++++++++++++ pkg/common/options.go | 2 + 7 files changed, 314 insertions(+), 2 deletions(-) create mode 100644 internal/align/nolint.go create mode 100644 internal/align/nolint_test.go create mode 100644 internal/app/nolint_test.go diff --git a/README.md b/README.md index f0f1580..52ace60 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,11 @@ structalign [flags] [packages] -tests also analyze _test.go files (skipped by default) -skip-cache-padded skip structs with a golang.org/x/sys/cpu.CacheLinePad field + -show-nolint show structs even when their type carries a recognized + //nolint directive (directives are respected by default) + -nolint-linters string + //nolint tokens that suppress a finding (default + "fieldalignment"; a bare //nolint always counts) -version print version and exit ``` @@ -313,6 +318,13 @@ structalign -skip-cache-padded ./... # skip structs guarded by cpu.Cache - **`-skip-cache-padded`** leaves structs with a [`cpu.CacheLinePad`](https://pkg.go.dev/golang.org/x/sys/cpu#CacheLinePad) field alone, since reordering would move the pad and defeat its false-sharing guard. +- **`//nolint` directives are respected by default** (diff mode): a struct whose + type declaration carries a recognized `//nolint` — `//nolint:fieldalignment` or + a bare `//nolint` — is suppressed, matching golangci-lint. `-nolint-linters` + customizes which named tokens count (default `fieldalignment`; e.g. + `-nolint-linters=fieldalignment,betteralign`); a bare `//nolint` always counts. + `-show-nolint` reveals suppressed structs (audit mode). Inspect mode ignores + these directives. ### Field tags diff --git a/internal/align/align.go b/internal/align/align.go index 58252a0..c7c836a 100644 --- a/internal/align/align.go +++ b/internal/align/align.go @@ -43,6 +43,7 @@ func (a *Aligner) Findings(t common.Target, opts common.Options) ([]common.Findi return nil, nil } names := structNameIndex(t.Syntax) + nolints := nolintIndex(t.Syntax, t.Fset) insp := inspector.New(t.Syntax) var findings []common.Finding @@ -55,7 +56,7 @@ func (a *Aligner) Findings(t common.Target, opts common.Options) ([]common.Findi TypesSizes: t.Sizes, // common.Sizes satisfies types.Sizes ResultOf: map[*analysis.Analyzer]any{inspect.Analyzer: insp}, Report: func(d analysis.Diagnostic) { - f := buildFinding(t, d, names, opts) + f := buildFinding(t, d, names, nolints, opts) if f == nil { return } @@ -78,7 +79,7 @@ func (a *Aligner) Findings(t common.Target, opts common.Options) ([]common.Findi // buildFinding converts one analyzer diagnostic into a Finding, applying tag // stripping and all active filters. Returns nil when the finding should be // suppressed. -func buildFinding(t common.Target, d analysis.Diagnostic, names map[token.Pos]string, opts common.Options) *common.Finding { +func buildFinding(t common.Target, d analysis.Diagnostic, names map[token.Pos]string, nolints map[token.Pos]nolintInfo, opts common.Options) *common.Finding { f := common.Finding{Fset: t.Fset, Pos: d.Pos, Message: d.Message} if len(d.SuggestedFixes) > 0 && len(d.SuggestedFixes[0].TextEdits) > 0 { e := d.SuggestedFixes[0].TextEdits[0] @@ -107,6 +108,11 @@ func buildFinding(t common.Target, d analysis.Diagnostic, names map[token.Pos]st if opts.SkipCachePadded && isCachePadded(t, f.Name) { return nil } + if opts.RespectNolint { + if info, ok := nolints[f.Pos]; ok && info.suppressed(opts.NolintLinters) { + return nil + } + } return &f } diff --git a/internal/align/nolint.go b/internal/align/nolint.go new file mode 100644 index 0000000..073449a --- /dev/null +++ b/internal/align/nolint.go @@ -0,0 +1,123 @@ +package align + +import ( + "go/ast" + "go/token" + "strings" +) + +// nolintInfo records the //nolint directive found on a struct type declaration. +type nolintInfo struct { + bare bool // a bare //nolint (suppresses all linters) + tokens map[string]struct{} // named tokens, e.g. {"fieldalignment"} +} + +// suppressed reports whether this directive suppresses a finding given the set +// of honored named linters. A bare //nolint always suppresses. +func (info nolintInfo) suppressed(linters []string) bool { + if info.bare { + return true + } + for _, l := range linters { + if _, ok := info.tokens[l]; ok { + return true + } + } + return false +} + +// nolintIndex maps each named struct type's StructType.Pos() to the //nolint +// directive on its declaration. A directive is recognized from the type's doc +// comment (TypeSpec.Doc, or the enclosing GenDecl.Doc for grouped `type ( ... )` +// blocks) and from any comment on the type's opening line (e.g. a trailing +// `type T struct { //nolint`, which the AST does not attach to TypeSpec.Comment). +// The analyzer reports at StructType.Pos(), so this key matches a Finding's Pos. +func nolintIndex(files []*ast.File, fset *token.FileSet) map[token.Pos]nolintInfo { + index := make(map[token.Pos]nolintInfo) + for _, f := range files { + // Directives keyed by source line, from every comment in the file — + // used to catch a trailing directive on a struct's opening line. + byLine := make(map[int]nolintInfo) + for _, cg := range f.Comments { + for _, c := range cg.List { + line := fset.Position(c.Pos()).Line + info := byLine[line] + parseNolint(c.Text, &info) + byLine[line] = info + } + } + for _, decl := range f.Decls { + gd, ok := decl.(*ast.GenDecl) + if !ok || gd.Tok != token.TYPE { + continue + } + for _, spec := range gd.Specs { + ts, ok := spec.(*ast.TypeSpec) + if !ok { + continue + } + st, ok := ts.Type.(*ast.StructType) + if !ok { + continue + } + info := collectNolint(ts.Doc, gd.Doc, ts.Comment) + mergeNolint(&info, byLine[fset.Position(st.Pos()).Line]) + if info.bare || len(info.tokens) > 0 { + index[st.Pos()] = info + } + } + } + } + return index +} + +// collectNolint scans the given comment groups for //nolint directives. +func collectNolint(groups ...*ast.CommentGroup) nolintInfo { + var info nolintInfo + for _, g := range groups { + if g == nil { + continue + } + for _, c := range g.List { + parseNolint(c.Text, &info) + } + } + return info +} + +// mergeNolint folds src into dst (bare wins, tokens unioned). +func mergeNolint(dst *nolintInfo, src nolintInfo) { + if src.bare { + dst.bare = true + } + for tok := range src.tokens { + if dst.tokens == nil { + dst.tokens = make(map[string]struct{}) + } + dst.tokens[tok] = struct{}{} + } +} + +// parseNolint reads one "//nolint" or "//nolint:a,b,c" comment into info. +// "//nolint" (alone or followed by space) is bare; "//nolint:list" adds named +// tokens. A comment like "//nolintfoo" is not a directive. +func parseNolint(text string, info *nolintInfo) { + text = strings.TrimSpace(strings.TrimPrefix(text, "//")) + if !strings.HasPrefix(text, "nolint") { + return + } + rest := strings.TrimPrefix(text, "nolint") + switch { + case rest == "" || strings.HasPrefix(rest, " "): + info.bare = true + case strings.HasPrefix(rest, ":"): + for tok := range strings.SplitSeq(rest[1:], ",") { + if tok = strings.TrimSpace(tok); tok != "" { + if info.tokens == nil { + info.tokens = make(map[string]struct{}) + } + info.tokens[tok] = struct{}{} + } + } + } +} diff --git a/internal/align/nolint_test.go b/internal/align/nolint_test.go new file mode 100644 index 0000000..b242501 --- /dev/null +++ b/internal/align/nolint_test.go @@ -0,0 +1,85 @@ +package align_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/peczenyj/structalign/internal/align" + "github.com/peczenyj/structalign/internal/testutil" + "github.com/peczenyj/structalign/pkg/common" +) + +const nolintSrc = `package sample + +//nolint:fieldalignment +type Suppressed struct { + A bool + B int64 + C bool +} + +type Visible struct { + A bool + B int64 + C bool +} + +//nolint:errcheck +type Unrelated struct { + A bool + B int64 + C bool +} + +type Trailing struct { //nolint + A bool + B int64 + C bool +} +` + +func findingNames(fs []common.Finding) map[string]bool { + m := make(map[string]bool) + for _, f := range fs { + m[f.Name] = true + } + return m +} + +func TestFindingsRespectNolintByDefault(t *testing.T) { + tgt := testutil.Target(t, nolintSrc) + fs, err := align.New().Findings(tgt, common.Options{ + RespectNolint: true, + NolintLinters: []string{"fieldalignment"}, + }) + require.NoError(t, err) + names := findingNames(fs) + assert.False(t, names["Suppressed"], "//nolint:fieldalignment must suppress") + assert.True(t, names["Visible"], "unmarked struct stays") + assert.True(t, names["Unrelated"], "//nolint:errcheck must NOT suppress fieldalignment") + assert.False(t, names["Trailing"], "a bare //nolint (trailing) must suppress") +} + +func TestFindingsShowNolint(t *testing.T) { + tgt := testutil.Target(t, nolintSrc) + fs, err := align.New().Findings(tgt, common.Options{RespectNolint: false}) + require.NoError(t, err) + names := findingNames(fs) + assert.True(t, names["Suppressed"], "respect off => suppressed struct reappears") + assert.True(t, names["Trailing"], "respect off => bare-nolint struct reappears") +} + +func TestFindingsNolintLintersConfigurable(t *testing.T) { + tgt := testutil.Target(t, nolintSrc) + // Only honoring "betteralign" => :fieldalignment no longer suppresses. + fs, err := align.New().Findings(tgt, common.Options{ + RespectNolint: true, + NolintLinters: []string{"betteralign"}, + }) + require.NoError(t, err) + names := findingNames(fs) + assert.True(t, names["Suppressed"], "fieldalignment token not in the configured set => not suppressed") + assert.False(t, names["Trailing"], "bare //nolint is always honored regardless of the configured set") +} diff --git a/internal/app/app.go b/internal/app/app.go index e585b7a..51cbc23 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -10,6 +10,7 @@ import ( "regexp" "runtime/debug" "sort" + "strings" "github.com/peczenyj/structalign/internal/align" "github.com/peczenyj/structalign/internal/layout" @@ -73,6 +74,19 @@ type options struct { summary bool sort bool threshold int + showNolint bool + nolintLinters string +} + +// splitCSV splits a comma-separated list, trimming spaces and dropping empties. +func splitCSV(s string) []string { + var out []string + for p := range strings.SplitSeq(s, ",") { + if p = strings.TrimSpace(p); p != "" { + out = append(out, p) + } + } + return out } // savings is the absolute bytes a finding saves, or 0 when sizes are unknown or @@ -110,6 +124,8 @@ func (a *App) Run(args []string) int { fs.BoolVar(&opt.summary, "summary", false, "in diff mode, print a one-line summary after the diffs") fs.BoolVar(&opt.sort, "sort", false, "present results largest-first (diff: by bytes saved; inspect: by struct size)") fs.IntVar(&opt.threshold, "threshold", 0, "in diff mode, only show structs that save at least this many bytes") + fs.BoolVar(&opt.showNolint, "show-nolint", false, "show structs even when their type carries a recognized //nolint directive") + fs.StringVar(&opt.nolintLinters, "nolint-linters", "fieldalignment", "comma-separated //nolint tokens that suppress a finding (bare //nolint always counts)") fs.Usage = func() { fmt.Fprintf(a.Stderr, "structalign: print field-aligned struct reorderings (no file changes)\n\n") fmt.Fprintf(a.Stderr, "usage: structalign [flags] [packages]\n\n") @@ -186,6 +202,8 @@ func (a *App) Run(args []string) int { KeepTags: opt.tags, IncludeGenerated: opt.generated, SkipCachePadded: opt.skipCachePadded, + RespectNolint: !opt.showNolint, + NolintLinters: splitCSV(opt.nolintLinters), } if opt.inspect { allLayouts = append(allLayouts, a.Inspector.Layouts(t, o)...) diff --git a/internal/app/nolint_test.go b/internal/app/nolint_test.go new file mode 100644 index 0000000..c8cc91c --- /dev/null +++ b/internal/app/nolint_test.go @@ -0,0 +1,66 @@ +package app_test + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "github.com/peczenyj/structalign/internal/align" + "github.com/peczenyj/structalign/internal/app" + "github.com/peczenyj/structalign/internal/layout" + "github.com/peczenyj/structalign/internal/mocks" + "github.com/peczenyj/structalign/internal/testutil" + "github.com/peczenyj/structalign/pkg/common" +) + +const appNolintSrc = `package sample + +//nolint:fieldalignment +type Hidden struct { + A bool + B int64 + C bool +} +` + +func nolintApp(t *testing.T, out, errb *bytes.Buffer) *app.App { + t.Helper() + tgt := testutil.Target(t, appNolintSrc) + ml := mocks.NewLoader(t) + ml.EXPECT().Load(mock.Anything).Return([]common.Target{tgt}, nil) + return &app.App{Loader: ml, Aligner: align.New(), Inspector: layout.New(), Stdout: out, Stderr: errb} +} + +func TestRunHidesNolintByDefault(t *testing.T) { + var out, errb bytes.Buffer + a := nolintApp(t, &out, &errb) + code := a.Run([]string{"pkg"}) + assert.Equal(t, 0, code, "only struct is suppressed => no findings => exit 0") + assert.NotContains(t, out.String(), "Hidden") +} + +func TestRunShowNolintReveals(t *testing.T) { + var out, errb bytes.Buffer + a := nolintApp(t, &out, &errb) + code := a.Run([]string{"-show-nolint", "pkg"}) + assert.Equal(t, 1, code) + assert.Contains(t, out.String(), "Hidden") +} + +func TestRunNolintLintersOptOut(t *testing.T) { + var out, errb bytes.Buffer + a := nolintApp(t, &out, &errb) + // Honor only betteralign => :fieldalignment no longer suppresses Hidden. + code := a.Run([]string{"-nolint-linters=betteralign", "pkg"}) + assert.Equal(t, 1, code) + assert.Contains(t, out.String(), "Hidden") +} + +func TestRunInspectIgnoresNolint(t *testing.T) { + var out, errb bytes.Buffer + a := nolintApp(t, &out, &errb) + _ = a.Run([]string{"-inspect", "pkg"}) + assert.Contains(t, out.String(), "type Hidden struct", "inspect ignores //nolint") +} diff --git a/pkg/common/options.go b/pkg/common/options.go index 1e4e77b..41de051 100644 --- a/pkg/common/options.go +++ b/pkg/common/options.go @@ -6,4 +6,6 @@ type Options struct { KeepTags bool // preserve struct field tags in rendered text IncludeGenerated bool // analyze structs in generated files (default: skip them) SkipCachePadded bool // skip structs containing a golang.org/x/sys/cpu.CacheLinePad field + RespectNolint bool // suppress findings on types carrying a recognized //nolint directive + NolintLinters []string // named //nolint tokens that trigger suppression (bare //nolint always counts) } From ea7c7a285a0fd35508fc78c1398bcbbfff77076a Mon Sep 17 00:00:00 2001 From: Tiago Peczenyj Date: Tue, 26 May 2026 17:29:12 +0200 Subject: [PATCH 5/6] test(align): cover trailing named //nolint and non-struct type decls Add fixtures exercising mergeNolint's named-token union (a trailing //nolint:fieldalignment on the struct's opening line) and nolintIndex's skip of a non-struct type declaration, closing the patch-coverage gaps on the //nolint feature (mergeNolint 50%->100%, nolintIndex 92%->96%; the remaining branch is unreachable defensive code). Closes #32 Co-Authored-By: Claude Opus 4.7 --- internal/align/nolint_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/align/nolint_test.go b/internal/align/nolint_test.go index b242501..a8687fc 100644 --- a/internal/align/nolint_test.go +++ b/internal/align/nolint_test.go @@ -38,6 +38,15 @@ type Trailing struct { //nolint B int64 C bool } + +type TrailingNamed struct { //nolint:fieldalignment + A bool + B int64 + C bool +} + +// A non-struct type declaration the index must skip without panicking. +type Celsius float64 ` func findingNames(fs []common.Finding) map[string]bool { @@ -60,6 +69,7 @@ func TestFindingsRespectNolintByDefault(t *testing.T) { assert.True(t, names["Visible"], "unmarked struct stays") assert.True(t, names["Unrelated"], "//nolint:errcheck must NOT suppress fieldalignment") assert.False(t, names["Trailing"], "a bare //nolint (trailing) must suppress") + assert.False(t, names["TrailingNamed"], "a trailing //nolint:fieldalignment must suppress") } func TestFindingsShowNolint(t *testing.T) { From f3d3b573d4fb3739ed47648183a69f6c3d871ff2 Mon Sep 17 00:00:00 2001 From: Tiago Peczenyj Date: Tue, 26 May 2026 20:29:35 +0200 Subject: [PATCH 6/6] feat: themeable colors (cga/green/amber) (#40) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: themeable colors (cga/green/amber) via eggs and STRUCTALIGN_THEME Route ui's hardcoded ANSI constants through a Theme over semantic roles (Header/Added/Removed/Meta/Padding/Label); the default theme reproduces the historical palette byte-for-byte, so golden output is unchanged. Add built-in retro themes: cga (bright 16-color), green and amber (single-hue phosphor emulations distinguished by intensity + the +/- prefixes). Selection: hidden easter-egg flags -cga/-green/-amber (stripped pre-parse, so invisible in -help) take precedence over the documented STRUCTALIGN_THEME env var, else default. Unknown theme warns and falls back. Theme is orthogonal to -color: it only chooses which colors are used when color is on. Not a full theming system: a fixed set of built-ins and two ways to pick one. Closes #33 Co-Authored-By: Claude Opus 4.7 * test: cover non-default theme rendering and -green/-amber/-- egg paths Add a ui test that renders through a Printer with a theme set (covering Printer.theme()'s non-default branch, now 100%) and app tests for the -green/-amber eggs, a valid STRUCTALIGN_THEME env (no warning), and the "--" boundary in the egg-stripping loop — closing the themes patch-coverage gaps. Closes #33 Co-Authored-By: Claude Opus 4.7 --------- Co-authored-by: Claude Opus 4.7 --- README.md | 6 +++ internal/app/app.go | 42 ++++++++++++++++++++ internal/app/theme_test.go | 78 ++++++++++++++++++++++++++++++++++++++ internal/ui/printer.go | 67 ++++++++++++++++++++++++-------- internal/ui/theme_test.go | 56 +++++++++++++++++++++++++++ internal/ui/themes.go | 38 +++++++++++++++++++ 6 files changed, 271 insertions(+), 16 deletions(-) create mode 100644 internal/app/theme_test.go create mode 100644 internal/ui/theme_test.go create mode 100644 internal/ui/themes.go diff --git a/README.md b/README.md index 414dfdd..6a471e0 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,12 @@ In the default `-color=auto`, color is emitted only when stdout is a terminal an the [`NO_COLOR`](https://no-color.org) environment variable is unset. `NO_COLOR` (any non-empty value) disables color; an explicit `-color=always` overrides it. +The palette can be switched with the `STRUCTALIGN_THEME` environment variable — +`default` (the standard colors), `cga` (a bright 16-color CGA look), or `green` / +`amber` (single-hue phosphor-monitor emulations). It only affects *which* colors +are used when color is on; it does not turn color on by itself. An unknown value +warns and falls back to `default`. + Exit code is **1 when reorderings are found**, **0 when none** — so it drops into CI as a check. Inspect mode is informational and always exits 0. diff --git a/internal/app/app.go b/internal/app/app.go index 51cbc23..2f55bce 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -144,6 +144,33 @@ func (a *App) Run(args []string) int { return 2 } } + + // Easter-egg theme flags: -cga/-green/-amber select a retro palette. Like + // -fix, they are caught before parsing and stripped from args, so they stay + // invisible in -help and never trip "flag provided but not defined". Last + // one wins; anything after "--" is left untouched (positional args). + themeName := "" + filtered := args[:0:0] + afterDD := false + for _, arg := range args { + switch { + case afterDD: + filtered = append(filtered, arg) + case arg == "--": + afterDD = true + filtered = append(filtered, arg) + case arg == "-cga" || arg == "--cga": + themeName = "cga" + case arg == "-green" || arg == "--green": + themeName = "green" + case arg == "-amber" || arg == "--amber": + themeName = "amber" + default: + filtered = append(filtered, arg) + } + } + args = filtered + if err := fs.Parse(args); err != nil { return 2 } @@ -167,10 +194,25 @@ func (a *App) Run(args []string) int { width = ui.ResolveWidth(stdoutFile(a.Stdout)) } patterns := match.ParsePatterns(opt.typeFilter) + + // Resolve the theme: egg flag wins over STRUCTALIGN_THEME, else default. + if themeName == "" { + themeName = os.Getenv("STRUCTALIGN_THEME") + } + theme := ui.DefaultTheme() + if themeName != "" && themeName != "default" { + if th, ok := ui.ThemeByName(themeName); ok { + theme = th + } else { + fmt.Fprintf(a.Stderr, "structalign: unknown theme %q, using default\n", themeName) + } + } + printer := &ui.Printer{ Out: a.Stdout, Color: ui.WantColor(opt.colorize, stdoutFile(a.Stdout)), Width: width, + Theme: theme, } ld := a.Loader diff --git a/internal/app/theme_test.go b/internal/app/theme_test.go new file mode 100644 index 0000000..6e3c428 --- /dev/null +++ b/internal/app/theme_test.go @@ -0,0 +1,78 @@ +package app_test + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "github.com/peczenyj/structalign/internal/align" + "github.com/peczenyj/structalign/internal/app" + "github.com/peczenyj/structalign/internal/layout" + "github.com/peczenyj/structalign/internal/mocks" + "github.com/peczenyj/structalign/internal/testutil" + "github.com/peczenyj/structalign/pkg/common" +) + +func themeApp(t *testing.T, out, errb *bytes.Buffer) *app.App { + t.Helper() + tgt := testutil.Target(t, src) + ml := mocks.NewLoader(t) + ml.EXPECT().Load(mock.Anything).Return([]common.Target{tgt}, nil) + return &app.App{Loader: ml, Aligner: align.New(), Inspector: layout.New(), Stdout: out, Stderr: errb} +} + +func TestRunCgaEggAccepted(t *testing.T) { + var out, errb bytes.Buffer + a := themeApp(t, &out, &errb) + // The egg flag is stripped before flag parsing, so the run proceeds normally + // (exit 1 on findings) rather than failing with "flag provided but not defined". + code := a.Run([]string{"-cga", "-type=Mixed", "pkg"}) + assert.Equal(t, 1, code) + assert.NotContains(t, errb.String(), "not defined") +} + +func TestRunGreenAndAmberEggsAccepted(t *testing.T) { + for _, egg := range []string{"-green", "-amber"} { + var out, errb bytes.Buffer + a := themeApp(t, &out, &errb) + code := a.Run([]string{egg, "-type=Mixed", "pkg"}) + assert.Equal(t, 1, code, "%s should be stripped and the run proceed", egg) + assert.NotContains(t, errb.String(), "not defined") + } +} + +func TestRunValidThemeEnvNoWarning(t *testing.T) { + var out, errb bytes.Buffer + a := themeApp(t, &out, &errb) + t.Setenv("STRUCTALIGN_THEME", "green") + a.Run([]string{"-type=Mixed", "pkg"}) + assert.NotContains(t, errb.String(), "unknown theme", "a valid theme must not warn") +} + +func TestRunDoubleDashStopsEggStripping(t *testing.T) { + var out, errb bytes.Buffer + a := themeApp(t, &out, &errb) + // After "--", args are positional (package) args; the loop's afterDD branch + // passes them through untouched. + code := a.Run([]string{"-type=Mixed", "--", "pkg"}) + assert.Equal(t, 1, code) +} + +func TestRunUnknownThemeEnvWarns(t *testing.T) { + var out, errb bytes.Buffer + a := themeApp(t, &out, &errb) + t.Setenv("STRUCTALIGN_THEME", "bogus") + a.Run([]string{"-type=Mixed", "pkg"}) + assert.Contains(t, errb.String(), `unknown theme "bogus"`) +} + +func TestEggFlagNotInUsage(t *testing.T) { + var out, errb bytes.Buffer + a := &app.App{Stdout: &out, Stderr: &errb} + a.Run(nil) // no args => prints usage to stderr + assert.NotContains(t, errb.String(), "-cga", "easter-egg flags stay invisible in usage") + assert.NotContains(t, errb.String(), "-green") + assert.NotContains(t, errb.String(), "-amber") +} diff --git a/internal/ui/printer.go b/internal/ui/printer.go index ff88d9f..f99c53b 100644 --- a/internal/ui/printer.go +++ b/internal/ui/printer.go @@ -23,11 +23,43 @@ const ( cBold = "\x1b[1m" ) +// Theme maps semantic roles to ANSI SGR sequences. The zero value resolves to +// DefaultTheme (see Printer.theme), which reproduces the historical palette. +type Theme struct { + Header string // finding header / inspect "type X struct {" line + Added string // "+" diff lines / added side cells + Removed string // "-" diff lines / removed side cells + Meta string // column titles, divider, layout note, "-- assume" marker + Padding string // inspect padding comment / "_" padding line + Label string // the -summary "Summary:" label +} + +// DefaultTheme is the byte-for-byte historical palette. +func DefaultTheme() Theme { + return Theme{ + Header: cBold + cCyan, + Added: cGreen, + Removed: cRed, + Meta: cDim, + Padding: cRed, + Label: cBold, + } +} + // Printer renders to Out using the given color/width settings. type Printer struct { Out io.Writer Color bool - Width int // per-side column width for side-by-side diffs + Width int // per-side column width for side-by-side diffs + Theme Theme // zero value resolves to DefaultTheme +} + +// theme returns the configured theme, or DefaultTheme when unset. +func (p *Printer) theme() Theme { + if (p.Theme == Theme{}) { + return DefaultTheme() + } + return p.Theme } // RenderFindings renders each finding in the chosen diff style. Returns the count. @@ -50,7 +82,7 @@ func (p *Printer) RenderLayouts(layouts []common.Layout, verbose, keepTags bool) // label is bold when color is on; counts are pluralized. func (p *Printer) RenderSummary(structs int, bytesSaved int64) { fmt.Fprintf(p.Out, "%s %d %s affected, %d %s saved\n", //nolint:errcheck - paint(p.Color, cBold, "Summary:"), + paint(p.Color, p.theme().Label, "Summary:"), structs, plural(int64(structs), "struct", "structs"), bytesSaved, plural(bytesSaved, "byte", "bytes")) } @@ -76,7 +108,7 @@ func (p *Printer) renderFinding(f common.Finding, style common.DiffStyle) { pct := float64(f.OldSize-f.NewSize) / float64(f.OldSize) * 100 header += fmt.Sprintf(" (%02.2f%% smaller)", pct) } - fmt.Fprintln(p.Out, paint(p.Color, cBold+cCyan, header)) //nolint:errcheck + fmt.Fprintln(p.Out, paint(p.Color, p.theme().Header, header)) //nolint:errcheck if f.Original == "" || f.Proposed == "" { fmt.Fprintln(p.Out, " (no suggested fix produced)") //nolint:errcheck @@ -102,6 +134,7 @@ func (p *Printer) renderFinding(f common.Finding, style common.DiffStyle) { } func (p *Printer) renderUnified(a, b string) { + th := p.theme() al := strings.Split(a, "\n") bl := strings.Split(b, "\n") ops := textdiff.Lines(al, bl) @@ -110,14 +143,15 @@ func (p *Printer) renderUnified(a, b string) { case textdiff.Equal: fmt.Fprintf(p.Out, " %s\n", op.Text) //nolint:errcheck case textdiff.Del: - fmt.Fprintln(p.Out, paint(p.Color, cRed, "- "+op.Text)) //nolint:errcheck + fmt.Fprintln(p.Out, paint(p.Color, th.Removed, "- "+op.Text)) //nolint:errcheck case textdiff.Add: - fmt.Fprintln(p.Out, paint(p.Color, cGreen, "+ "+op.Text)) //nolint:errcheck + fmt.Fprintln(p.Out, paint(p.Color, th.Added, "+ "+op.Text)) //nolint:errcheck } } } func (p *Printer) renderSideBySide(a, b string) { + th := p.theme() al := strings.Split(a, "\n") bl := strings.Split(b, "\n") ops := textdiff.Lines(al, bl) @@ -156,15 +190,15 @@ func (p *Printer) renderSideBySide(a, b string) { var l, r string var lc, rc string if k < len(dels) { - l, lc = dels[k], cRed + l, lc = dels[k], th.Removed } if k < len(adds) { - r, rc = adds[k], cGreen + r, rc = adds[k], th.Added } rows = append(rows, row{l, r, lc, rc}) } case textdiff.Add: - rows = append(rows, row{"", op.Text, "", cGreen}) + rows = append(rows, row{"", op.Text, "", th.Added}) i++ } _ = pendDel @@ -177,10 +211,10 @@ func (p *Printer) renderSideBySide(a, b string) { // Pad the header text manually (not via %-*s) so it stays correct even // when paint() wraps it in ANSI escapes, which %-*s would miscount. fmt.Fprintf(p.Out, " %s%s%s\n", //nolint:errcheck - paint(p.Color, cDim, truncPad("current", p.Width)), + paint(p.Color, th.Meta, truncPad("current", p.Width)), sep, - paint(p.Color, cDim, "proposed")) - fmt.Fprintf(p.Out, " %s\n", paint(p.Color, cDim, //nolint:errcheck + paint(p.Color, th.Meta, "proposed")) + fmt.Fprintf(p.Out, " %s\n", paint(p.Color, th.Meta, //nolint:errcheck strings.Repeat("─", p.Width)+"─┼─"+strings.Repeat("─", p.Width))) for _, r := range rows { left := truncPad(r.l, p.Width) @@ -196,6 +230,7 @@ func (p *Printer) renderSideBySide(a, b string) { } func (p *Printer) renderLayout(l common.Layout, verbose, keepTags bool) { + th := p.theme() // Field declaration is " " (plus tag when -tags is set); align // all comments to the widest declaration. decls := make([]string, len(l.Fields)) @@ -213,13 +248,13 @@ func (p *Printer) renderLayout(l common.Layout, verbose, keepTags bool) { // Optional caveat (e.g. the generic-type disclaimer) above the declaration. if l.Note != "" { - fmt.Fprintln(p.Out, paint(p.Color, cDim, "// "+l.Note)) //nolint:errcheck + fmt.Fprintln(p.Out, paint(p.Color, th.Meta, "// "+l.Note)) //nolint:errcheck } // Header: the struct opening line carries size/align/padding. header := fmt.Sprintf("type %s%s struct { // size: %d, align: %d, padding: %d", l.Name, l.TypeParams, l.Total, l.Align, l.Padding) - fmt.Fprintln(p.Out, paint(p.Color, cBold+cCyan, header)) //nolint:errcheck + fmt.Fprintln(p.Out, paint(p.Color, th.Header, header)) //nolint:errcheck comments, commentWidth := layoutComments(l.Fields, verbose) @@ -227,18 +262,18 @@ func (p *Printer) renderLayout(l common.Layout, verbose, keepTags bool) { comment := comments[i] rendered := comment if !verbose && f.Padding > 0 { - rendered = paint(p.Color, cRed, comment) + rendered = paint(p.Color, th.Padding, comment) } line := fmt.Sprintf("\t%-*s // %s", declWidth, decls[i], rendered) if f.Assume != "" { pad := strings.Repeat(" ", commentWidth-len(comment)) - line += pad + " " + paint(p.Color, cDim, "-- assume "+f.Assume) + line += pad + " " + paint(p.Color, th.Meta, "-- assume "+f.Assume) } fmt.Fprintln(p.Out, line) //nolint:errcheck if verbose && f.Padding > 0 { // Field line carries no padding; padding gets its own `_` line. pad := fmt.Sprintf("\t%-*s // %d byte padding", declWidth, "_", f.Padding) - fmt.Fprintln(p.Out, paint(p.Color, cRed, pad)) //nolint:errcheck + fmt.Fprintln(p.Out, paint(p.Color, th.Padding, pad)) //nolint:errcheck } } fmt.Fprintln(p.Out, "}") //nolint:errcheck diff --git a/internal/ui/theme_test.go b/internal/ui/theme_test.go new file mode 100644 index 0000000..5740797 --- /dev/null +++ b/internal/ui/theme_test.go @@ -0,0 +1,56 @@ +package ui_test + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/peczenyj/structalign/internal/ui" +) + +// The default theme must reproduce the historical hardcoded ANSI palette, so +// existing golden output is byte-for-byte unchanged. +func TestDefaultThemeMatchesLegacyConstants(t *testing.T) { + th := ui.DefaultTheme() + assert.Equal(t, "\x1b[1m\x1b[36m", th.Header, "header was bold+cyan") + assert.Equal(t, "\x1b[32m", th.Added, "added was green") + assert.Equal(t, "\x1b[31m", th.Removed, "removed was red") + assert.Equal(t, "\x1b[2m", th.Meta, "meta was dim") + assert.Equal(t, "\x1b[31m", th.Padding, "padding was red") + assert.Equal(t, "\x1b[1m", th.Label, "label was bold") +} + +func TestThemeByNameKnown(t *testing.T) { + for _, name := range []string{"default", "cga", "green", "amber"} { + _, ok := ui.ThemeByName(name) + assert.True(t, ok, "theme %q should exist", name) + } +} + +func TestThemeByNameUnknown(t *testing.T) { + _, ok := ui.ThemeByName("nope") + assert.False(t, ok) +} + +// Rendering with a non-default theme set on the Printer must use that theme's +// codes (exercises Printer.theme()'s "theme is set" branch). +func TestPrinterUsesSetTheme(t *testing.T) { + cga, ok := ui.ThemeByName("cga") + assert.True(t, ok) + var buf bytes.Buffer + p := &ui.Printer{Out: &buf, Color: true, Theme: cga} + p.RenderSummary(1, 8) + assert.Contains(t, buf.String(), "\x1b[1m\x1b[97m", "cga Label (bold bright white) should be used") +} + +// The green (P1 phosphor) theme is monochrome: it must never use red (31), +// since add/removed are distinguished by intensity + the +/- prefixes. +func TestGreenThemeIsMonochrome(t *testing.T) { + th, ok := ui.ThemeByName("green") + assert.True(t, ok) + for _, sgr := range []string{th.Header, th.Added, th.Removed, th.Meta, th.Padding, th.Label} { + assert.NotContains(t, sgr, "31", "green theme must not use red") + assert.Contains(t, sgr, "32", "green theme uses the green family") + } +} diff --git a/internal/ui/themes.go b/internal/ui/themes.go new file mode 100644 index 0000000..05b1427 --- /dev/null +++ b/internal/ui/themes.go @@ -0,0 +1,38 @@ +package ui + +// builtinThemes holds the named palettes. "default" mirrors DefaultTheme(). +// The monochrome themes (green/amber) use a single hue and rely on intensity +// (bold vs dim) plus the +/- diff prefixes to distinguish added from removed. +var builtinThemes = map[string]Theme{ + "default": DefaultTheme(), + "cga": { + Header: "\x1b[1m\x1b[96m", // bright cyan + Added: "\x1b[92m", // bright green + Removed: "\x1b[91m", // bright red + Meta: "\x1b[90m", // bright black (gray) + Padding: "\x1b[91m", // bright red + Label: "\x1b[1m\x1b[97m", // bright white + }, + "green": { + Header: "\x1b[1m\x1b[32m", // bold green + Added: "\x1b[1m\x1b[32m", // bold green (distinguished by the "+" prefix) + Removed: "\x1b[2m\x1b[32m", // dim green (distinguished by the "-" prefix) + Meta: "\x1b[2m\x1b[32m", // dim green + Padding: "\x1b[2m\x1b[32m", // dim green + Label: "\x1b[1m\x1b[32m", // bold green + }, + "amber": { + Header: "\x1b[1m\x1b[38;5;214m", // bold amber (256-color) + Added: "\x1b[1m\x1b[38;5;214m", + Removed: "\x1b[2m\x1b[38;5;214m", // dim amber + Meta: "\x1b[2m\x1b[38;5;214m", + Padding: "\x1b[2m\x1b[38;5;214m", + Label: "\x1b[1m\x1b[38;5;214m", + }, +} + +// ThemeByName returns a built-in theme and whether it was found. +func ThemeByName(name string) (Theme, bool) { + th, ok := builtinThemes[name] + return th, ok +}