Skip to content

Polish nits surfaced by the cmd/render.go per-command split #324

@autholykos

Description

@autholykos

Five pre-existing code quirks that became visible during structural refactor #9 (PRs #322, #323). None were introduced by the split — the pure-move discipline just surfaced them. Bundling into one follow-up PR is the efficient shape.

Items

1. UTF-8-unsafe byte truncation in truncateRenderLine

Location: cmd/render_util.go:87-96

return value[:limit-3] + "..."

Byte slice on a Go string can split a multi-byte rune (CJK, accented Latin, math symbols). renderTableValue in cmd/render_search.go:61-77 already does this correctly via []rune. Same finding also appeared in the hardening review for excerpt() in internal/index/search.go — fix is the same shape.

Fix: convert to []rune, slice on rune count.

2. Hardcoded box-drawing char bypasses UTF-8 toggle

Location: cmd/render_freshness.go:57

indent := "│"
if i == len(item.Signals)-1 {
    indent = " "
}

The renderPresentation struct has a p.utf8 bool with ASCII fallbacks throughout (e.g. p.treeMid() returns "├─" or "|-"). This one spot ignores that toggle and will produce mojibake on non-UTF-8 terminals.

Fix: add a p.treeIndent() method in cmd/presentation.go that returns "│" when p.utf8 else "|", and use it in renderFreshnessResult.

3. truncateRenderLine and humanizeSymbol have only one call site each

Location: cmd/render_util.go

The render_util.go header comment says "helpers with 2+ consumers", but truncateRenderLine is only used by render_compliance.go and humanizeSymbol only by render_docdrift.go. Either move them to their respective command files, or update the doc comment to admit they're utility-shaped.

Fix: move both to their single-caller file. Removes two cross-file references.

4. renderOverlapRecommendation redundantly re-derives presentation

Location: cmd/render_overlap.go:65-77

func renderOverlapRecommendation(w io.Writer, recommendation string) {
    p := presentationForWriter(w)  // redundant: caller already has p
    ...
}

Its only caller renderOverlapResult already holds a p. Other helpers in the same file (e.g. overlapBlock) already take p as a parameter.

Fix: thread p renderPresentation through the signature.

5. minInt is redundant with Go 1.21 builtin min

Location: cmd/render_util.go:98-102

Go 1.21+ ships a generic min/max builtin. go.mod targets go 1.25.9, so this local wrapper is dead weight.

Fix: delete minInt, replace every call site with min(a, b).

Non-goals

  • No behavioral change to any rendered output — all fixes preserve user-visible text.
  • Not scope for this issue: the architectural sectionRenderer primitive the review originally proposed (Header/TreeList/KeyValueBlock/Badge). That's a separate design exercise tracked outside this issue.

Verification

  • make fmt, make vet, go test ./... all green after each item.
  • Existing cmd/render_test.go suite should pass unchanged.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:uxCLI, workflow, and user experience surfacesccd/priority:laterCCD: future worktype:enhancementIssues concerning code or feature improvement (performance, refactoring, etc)type:tech-debtthe issue is a tech debt that needs fixing

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions