Skip to content

mute ↪ wrap continuation marker, add --wrap-indent#166

Merged
umputun merged 3 commits intomasterfrom
wrap-marker-style
May 1, 2026
Merged

mute ↪ wrap continuation marker, add --wrap-indent#166
umputun merged 3 commits intomasterfrom
wrap-marker-style

Conversation

@umputun
Copy link
Copy Markdown
Owner

@umputun umputun commented May 1, 2026

Wrap continuation marker color

The glyph used to inherit chroma syntax color or change-line foreground because the highlighted line styles set only background and let content fg fall through. It now renders with MutedFg via raw ANSI outside lipgloss, matching the muted treatment of «/» horizontal scroll indicators.

Applied at all three wrap callsites (renderWrappedDiffLine, renderWrappedCollapsedLine, renderDeletePlaceholder).

--wrap-indent N option

Default 0 (current behavior, byte-identical). When > 0, the marker is followed by N background-padded spaces so wrapped continuation content visually hangs under the first row's content rather than column-aligning with new bullets/items at the same column. Useful when reviewing markdown lists where unindented continuation can be misread as a new bullet.

wrapWidth() reserves the indent so total visual width stays consistent. Clamps so wrap width never drops below wrapMinContent (10) - wrap mode skips horizontal scroll, so a sub-zero return would let long lines overflow the pane silently. effectiveWrapIndent() mirrors the clamp so the marker padding stays in sync.

Trade-off: the indent reservation also shrinks the first row's wrap width even though the first row doesn't render the indent. Asymmetric per-row wrapping was rejected as too complex for the visual gain.

Search-match consistency

Two related search-match issues surfaced during code review and were fixed:

  1. Colored mode bg seam: in collapsed mode, search-matched rows used lineStyle = SearchMatch (with SearchBg) but bgColor stayed at add/modify/remove bg. The wrap marker (rendered outside lipgloss) and the right-side extendLineBg padding therefore showed a visible bg seam against the search-styled content. Fixed by flipping bgColor to SearchBg in the search-match branch.

  2. No-colors plain marker: in --no-colors mode, the SearchMatch lipgloss style renders as Reverse(true), but styledWrapMarker returned a plain " ↪ ". Search-matched continuation rows showed a plain marker against reverse-video content. Fixed by adding a searchMatch bool parameter to styledWrapMarker; in no-colors + search-match it emits \x1b[7m...\x1b[27m around the marker.

renderWrappedDiffLine (non-collapsed) passes searchMatch=false because its search highlight is per-word inside the existing change-type lipgloss style, not full-line Reverse.

Linter config

Drops goconst from .golangci.yml (config + enabled list + test exception). The 12 pre-existing goconst complaints in the codebase were noise more than signal at min-occurrences: 7.

Tests

  • TestModel_StyledWrapMarker: 9 subtests covering color modes, bg fallback, no-colors plain, no-colors search-match reverse-video, colored search-match parity, indent emission, oversized indent clamp.
  • TestModel_WrapWidthClampsLargeIndent: 6-row table at indent boundaries (0, 4, exactly at clamp, past clamp, equal to base, way past base).
  • TestModel_NewModelClampsNegativeWrapIndent: negative WrapIndent clamps to 0 in NewModel.
  • TestModel_WrappedLineCountReactsToIndent: wrappedLineCount reacts to wrapIndent so cursor/viewport math stays consistent.
  • TestModel_CollapsedWrapSearchMatchUsesSearchBg: end-to-end render asserts SearchBg on collapsed wrap continuations and on delete-placeholder continuations.
  • TestParseArgs_WrapIndent: 4 subtests (default, flag, env, config file).
  • TestDumpConfig: assertion for wrap-indent = 0 in the dumped config.

Docs

README.md, site/docs.html, .claude-plugin/skills/revdiff/references/config.md, and plugins/codex/skills/revdiff/references/config.md all updated with the new flag and a description that conveys the markdown-list-review use case rather than vague "bullets/items" wording.

umputun added 2 commits May 1, 2026 15:47
Render the wrap continuation marker (↪) with MutedFg via raw ANSI
outside the lipgloss style block, matching the muted treatment of
the «/» horizontal scroll indicators. Previously the marker inherited
chroma syntax color or change-line foreground because the highlighted
line styles only set background and let content fg fall through.
Applied at all three wrap callsites: renderWrappedDiffLine,
renderWrappedCollapsedLine, renderDeletePlaceholder.

Add --wrap-indent N option (env REVDIFF_WRAP_INDENT, ini wrap-indent,
default 0). When > 0, the marker is followed by N bg-padded spaces so
wrapped continuation content visually hangs under the first row's
content rather than column-aligning with new bullets/items at the
same column. Useful when reviewing markdown lists where unindented
continuation can be misread as a new bullet.

wrapWidth() reserves the indent so total visual width stays consistent.
Clamps the indent so wrapWidth never drops below wrapMinContent (10) —
wrap mode skips horizontal scroll, so a sub-zero return would let
long lines overflow the pane silently. effectiveWrapIndent() mirrors
the clamp so the marker's indent padding stays in sync.

Search-match handling: in collapsed mode, search-matched rows now flip
bgColor to SearchBg so the marker (rendered outside lipgloss) and the
right-side extendLineBg padding stay on the same bg as the lineStyle
content, eliminating a bg seam. In --no-colors mode, where the
SearchMatch lipgloss style renders as Reverse(true), styledWrapMarker
takes a searchMatch bool and emits \x1b[7m...\x1b[27m around the marker
so search-matched continuation rows stay visually consistent across
both color modes.

Tests cover: muted-color emission across change types and color modes,
no-colors search-match reverse-video fallback, wrap-indent clamp at
boundaries (0, 4, exactly at clamp, past clamp, equal to base, way
past base), negative WrapIndent clamps to 0 in NewModel, wrappedLineCount
reacts to indent (cursor/viewport math stays consistent), end-to-end
search-match render asserts SearchBg on collapsed wrap continuations
and on delete-placeholder continuations.
Copilot AI review requested due to automatic review settings May 1, 2026 21:16
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves wrap-mode rendering in the diff UI by ensuring wrap continuation markers render as “muted chrome” (not inheriting syntax/change foreground), adds a configurable hanging indent for wrapped continuation rows, and fixes a couple of search-match background consistency issues. It also updates docs/tests accordingly and removes goconst from golangci-lint configuration.

Changes:

  • Render the wrap continuation marker () using MutedFg via raw ANSI (and add a no-colors reverse-video fallback for search-match rows where appropriate).
  • Add --wrap-indent N / REVDIFF_WRAP_INDENT to visually indent wrapped continuation rows while keeping total width consistent via wrapWidth() reservation and clamping.
  • Fix collapsed-mode search-match background seams for wrap markers and right-side background padding; update tests and documentation.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
app/ui/diffview.go Adds styledWrapMarker, --wrap-indent width reservation/clamping, and uses muted marker in non-collapsed wrap rendering.
app/ui/collapsed.go Ensures collapsed wrap continuations use the muted marker and fixes search-match background seam behavior (marker + padding).
app/ui/model.go Wires WrapIndent into model configuration (clamped at construction).
app/main.go Passes CLI/config WrapIndent into ModelConfig.
app/config.go Adds --wrap-indent option (flag/env/ini) with description and default.
app/config_test.go Adds argument/config/env parsing tests for wrap-indent and asserts config dump includes it.
app/ui/diffview_test.go Adds unit tests for styledWrapMarker, wrap width clamping, and wrap row count behavior with indent.
app/ui/collapsed_render_test.go Adds end-to-end render assertions for search-match background consistency in collapsed wrap/placeholder continuations.
README.md Documents --wrap-indent and updates wrap feature bullet/table.
site/docs.html Adds --wrap-indent to the rendered options documentation.
.claude-plugin/skills/revdiff/references/config.md Documents --wrap-indent in config reference.
plugins/codex/skills/revdiff/references/config.md Documents --wrap-indent in config reference.
.golangci.yml Removes goconst from enabled linters and related config/exclusions.

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

Comment thread app/ui/diffview_test.go Outdated
Comment on lines +642 to +643
got := plain.styledWrapMarker("", false)
assert.Equal(t, " ↪ ", got, "without colors the marker should be plain ascii")
Comment thread app/ui/diffview_test.go Outdated
m.cfg.wrapIndent = 8
withIndentRows := m.wrappedLineCount(0)

assert.Greater(t, withIndentRows, noIndentRows, "non-zero wrapIndent should not reduce row count for an already-fitting line")
Comment thread app/ui/diffview.go Outdated
// background (chroma owns per-token fg for content), so the prefix would
// otherwise inherit the terminal default fg and may render invisibly on
// light theme backgrounds.
// light theme backgrounds. an empty prefix is returned unchanged so callers
Comment thread app/ui/diffview.go
Comment on lines +517 to +522
// continuation content visually hangs under the first row's content rather than
// aligning with new bullets/items at the same column. matches the muted treatment
// of «/» horizontal scroll indicators (see scrollIndicatorANSI) — both gutter-chrome
// glyphs use ColorKeyMutedFg via raw ANSI rather than going through lipgloss so
// they do not inherit chroma syntax color or change-line foreground. the two
// helpers stay separate because scrollIndicatorANSI also handles split bg semantics
- set cfg.noColors=true in the "no-colors mode" subtest so the test
  name matches the actual code path; update assertion message from
  "plain ascii" to "no ANSI sequences" since ↪ is Unicode
- update TestModel_WrappedLineCountReactsToIndent failure message to
  match the strict-increase assertion (assert.Greater) instead of the
  weaker "should not reduce" wording
- capitalize sentence starts after periods in the wrapPrefixForHighlight
  and styledWrapMarker comments to match Go comment conventions
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 1, 2026

Deploying revdiff with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2fdc94f
Status: ✅  Deploy successful!
Preview URL: https://0e35df88.revdiff.pages.dev
Branch Preview URL: https://wrap-marker-style.revdiff.pages.dev

View logs

@umputun umputun merged commit 1d6497f into master May 1, 2026
5 checks passed
@umputun umputun deleted the wrap-marker-style branch May 1, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants