refactor: comprehensive code smells cleanup

[umputun]

Comprehensive code smells cleanup across all packages, addressing 53 findings from 8 smell analysis agents. All categories addressed except function length and runGit context propagation (deferred as separate tasks).

Repeated expressions → helper methods — treePaneHidden(), wrapWidth(), isCursorLine(), effectiveStatusFg() extracted to replace duplicated multi-line expressions in ui package

Standalone functions → struct methods — blameTargetRef, parseBlame, isHexString moved to Git methods; reconstructFiles, highlightFile, mapHighlightedLines moved to Highlighter methods; displayKeyName moved to Model method; parseUnifiedDiff unexported (internal to diff package)

Code deduplication — resolveFlagPath replaces near-identical config/keys path resolution in main.go; annotListBoxStyle extracts shared box construction; helpColors/helpLine deduplicates help overlay setup; ensureVisibleInList shared between fileTree and mdTOC; initThemes unifies InitBundled/InitAll/InitNames; shared file reading in fallback.go; renderTwoPaneLayout deduplicates TOC/tree layout; findFirstVisibleMatch deduplicates search wrap-around logic

Unicode truncation fix — truncateDirName and formatThemeEntry now use rune-based length/slicing instead of byte-based, fixing incorrect truncation of multi-byte characters (CJK, emoji)

Parameter count reduction — wrappedLineCtx struct for renderWrappedCollapsedLine; renderCtx struct for renderFileEntry; blameGutter takes explicit time.Time instead of reading struct field; buildSearchMatchSet returns map instead of mutating

Over-exported symbols unexported — InstallFile → installFile, ChromaValidator removed (inline func(string) bool), IsLocalPath → isLocalPath, BundledNames → bundledNames (now returns error)

Minor fixes — stale comments fixed, slices.Sort/slices.SortFunc/slices.Insert replaces sort package, constants for magic strings, doc comments on loader functions, else if chains replaced with switch statements

34 files changed, 737 insertions, 490 deletions

[Copilot]

Pull request overview

Refactors multiple packages to address code-smell findings (duplication, over-exporting, receiver/method consistency), improves Unicode-safe truncation in UI rendering, and updates documentation/tests accordingly.

Changes:

• Extracts/centralizes repeated UI expressions and layout/rendering helpers; reduces parameter counts via small context structs.
• Converts several standalone helpers into struct methods (Git/Highlighter/Model) and unexports internal-only symbols/APIs.
• Fixes Unicode display-width truncation (CJK/emoji) using go-runewidth and adds/updates tests.

Reviewed changes

Copilot reviewed 35 out of 36 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
docs/plans/completed/20260408-code-smells-cleanup.md	Adds completed implementation plan/notes for the cleanup work.
CLAUDE.md	Updates project structure/data-flow docs to match refactors (e.g., parseUnifiedDiff, blame gutter signature).
app/ui/view.go	Deduplicates two-pane layout rendering; extracts status-fg fallback helper.
app/ui/model.go	Adds helper methods for repeated “tree hidden” and “cursor line” logic.
app/ui/diffview.go	Refactors wrapping width calc; introduces wrapWidth() helper; passes explicit time to blame gutter formatting.
app/ui/collapsed.go	Reduces wrapped-line rendering params via wrappedLineCtx; uses shared wrap width helper.
app/ui/search.go	Extracts findFirstVisibleMatch; makes buildSearchMatchSet return a map instead of mutating.
app/ui/search_test.go	Updates tests for buildSearchMatchSet return value.
app/ui/filetree.go	Extracts ensureVisibleInList; introduces Unicode-safe truncation using display-width calculations.
app/ui/filetree_test.go	Adds coverage for ensureVisibleInList and Unicode truncation behavior.
app/ui/themeselect.go	Uses display-width truncation for theme names; simplifies filter logic.
app/ui/themeselect_test.go	Adds Unicode truncation test for theme entries.
app/ui/mdtoc.go	Simplifies fence parsing and uses shared ensure-visible helper.
app/ui/handlers.go	Deduplicates help overlay colors/lines; uses keymap section constant; refactors some handlers.
app/ui/handlers_test.go	Adds tests for handleFileAnnotateKey.
app/ui/loaders.go	Adds doc comments; fixes TOC clearing behavior when switching files.
app/ui/loaders_test.go	Adds regression test ensuring TOC is cleared when switching to non-markdown.
app/ui/annotlist.go	Extracts shared annotation list overlay box style helper.
app/ui/annotate.go	Replaces magic strings with constants; minor control-flow simplifications.
app/ui/annotate_test.go	Updates tests to use the new file-annotation key constant.
app/ui/diffnav.go	Switches some if/else chains to switch; adjusts TOC jump return signature.
app/ui/configpatch.go	Uses slices.Insert for simpler config patch insertion.
app/keymap/keymap.go	Adds SectionPane constant and uses it in default help entries.
app/highlight/highlight.go	Converts internal helper functions into Highlighter methods.
app/highlight/highlight_test.go	Updates tests for Highlighter method receivers.
app/theme/theme.go	Deduplicates theme init flows; unexports internal helpers; replaces sort with slices.
app/theme/theme_test.go	Updates tests to match unexported/renamed theme helpers and new signatures.
app/main.go	Consolidates config/keys path resolution into resolveFlagPath.
app/main_test.go	Updates tests to cover resolveFlagPath and config/keys wiring.
app/diff/diff.go	Unexports unified diff parser; adds IsPlaceholder to DiffLine.
app/diff/diff_test.go	Updates tests to call parseUnifiedDiff.
app/diff/blame.go	Converts blame helpers to Git methods; minor control-flow simplification.
app/diff/blame_test.go	Updates tests to use Git receiver methods.
app/diff/fallback.go	Refactors file-reading/placeholder handling and marks non-content placeholders explicitly.
app/diff/fallback_test.go	Adds regression test around placeholder detection for content lines.
app/fsutil/fsutil_test.go	Minor formatting change in tests.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The plan refers to parseUnifiedDiff as an “unexported method”, but it’s an unexported package-level function in app/diff (not a method). Consider updating the wording to avoid confusion when cross-referencing the code/doc change in CLAUDE.md.

[Copilot]

strings.HasPrefix(rel, "..") is not a correct containment check for filepath.Rel results: it will treat a legitimate in-repo path like "..foo/bar" (a directory starting with "..") as outside the workdir. A safer check is rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) (and similarly for other call sites using the same pattern).