From 4b9b6da4a0531b367947b436a4d5e6c43a0089b8 Mon Sep 17 00:00:00 2001 From: Booyaka101 Date: Sun, 10 May 2026 00:41:18 +0800 Subject: [PATCH 1/3] fix(diffviewer): wrap long lines through delta in side-by-side, mark clipped lines in unified (closes #99) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `diffFile` and `diffDir` were invoking delta with `--max-line-length=`, which is the truncation flag rather than a wrap hint. Combined with delta's default `--wrap-max-lines=2`, that capped any source line at roughly two viewport widths in side-by-side mode and silently dropped the rest. In unified mode, where delta does not wrap, the long line then hit the post- processing truncation in the `diffContentMsg` handler, which used an empty tail and clipped without any visible signal. This commit: - Replaces `--max-line-length=` with `--max-line-length=0` and adds `--wrap-max-lines=unlimited` for both invocations. Side-by-side now wraps long lines all the way to the end instead of truncating after two wraps. Verified locally: a 162-char line at `-w=80` previously stopped at "what flags delta is invoked with"; it now reaches "to see what happens here". - Replaces the empty truncation tail in the `diffContentMsg` handler with `…`, so unified-mode lines that exceed the viewport at least signal the cut-off rather than vanishing silently. The existing `diffviewer` test suite still passes. Pre-existing `TestSearchResultsRenderWhenFileTreeIsHidden`, `TestBuildFullFileTree`, and `TestCollapseTree` failures on `main` are unrelated to this change. Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/ui/panes/diffviewer/diffviewer.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/pkg/ui/panes/diffviewer/diffviewer.go b/pkg/ui/panes/diffviewer/diffviewer.go index c69c7bf..6a6cd4c 100644 --- a/pkg/ui/panes/diffviewer/diffviewer.go +++ b/pkg/ui/panes/diffviewer/diffviewer.go @@ -80,11 +80,15 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { } case diffContentMsg: - // Truncate lines to viewport width to prevent ANSI escape overflow. + // Clip lines that exceed the viewport width and use a visible tail + // marker so users can see content was cut off rather than failing + // silently. In side-by-side mode delta wraps long lines for us; in + // unified mode delta passes them through as-is, so this clip-with- + // marker is the only signal that there's more to the right. lines := strings.Split(msg.text, "\n") for i, line := range lines { if lipgloss.Width(line) > m.vp.Width() && m.vp.Width() > 0 { - lines[i] = ansi.Truncate(line, m.vp.Width(), "") + lines[i] = ansi.Truncate(line, m.vp.Width(), "…") } } diff := strings.Join(lines, "\n") @@ -304,7 +308,14 @@ func diffFile(node *cachedNode, width int, sideBySide bool) tea.Cmd { args := []string{ "--paging=never", fmt.Sprintf("-w=%d", width), - fmt.Sprintf("--max-line-length=%d", width), + // Disable hard truncation and let delta's own line-wrapping (active + // in side-by-side mode) carry the full line through. With + // `--max-line-length=` and the default `--wrap-max-lines=2`, + // long lines were being clipped at the viewport before we saw + // them. Anything still wider than the viewport gets clipped with + // a visible "…" marker in the `diffContentMsg` handler. + "--max-line-length=0", + "--wrap-max-lines=unlimited", } if useSideBySide { args = append(args, "--side-by-side") @@ -340,7 +351,9 @@ func diffDir(dir *cachedNode, width int, sideBySide bool, preamble string) tea.C fmt.Sprintf("--file-style='%s bold %s'", c, c), fmt.Sprintf("--file-decoration-style='%s box %s'", c, c), fmt.Sprintf("-w=%d", width), - fmt.Sprintf("--max-line-length=%d", width), + // See `diffFile` for why these are set this way. + "--max-line-length=0", + "--wrap-max-lines=unlimited", } if useSideBySide { args = append(args, "--side-by-side") From 51a116e08847133fc564024ba7d6fca079edeaf5 Mon Sep 17 00:00:00 2001 From: Booyaka101 Date: Sat, 16 May 2026 10:54:51 +0800 Subject: [PATCH 2/3] feat(diffviewer): horizontal scroll with arrow keys, conditional ellipsis markers Per #133 review: maintainer asked for horizontal scrolling instead of silent clipping, with the ellipsis markers gated on actual visible-vs- viewport content (and mirrored on the left side). Changes: - pkg/ui/keys.go: add ScrollLeft/ScrollRight bindings on left/right arrows (h/l are taken by file-tree expand/collapse). - pkg/ui/tui.go: forward arrow keys to diffViewer when the diff pane is active. - pkg/ui/panes/diffviewer: store the raw delta output and re-render the visible window on offset change. Cache now holds raw text so cache hits re-render through the same path. xOffset resets when a new file or dir is loaded. Ellipsis behaviour: - Right marker: only when lineWidth > xOffset + viewportWidth. - Left marker: only when xOffset > 0 AND lineWidth > xOffset. - Slice budget is reduced by 1 cell per active marker so the visible content + markers fit the viewport. --- pkg/ui/keys.go | 12 +++ pkg/ui/panes/diffviewer/diffviewer.go | 115 ++++++++++++++++++++++---- pkg/ui/tui.go | 8 ++ 3 files changed, 117 insertions(+), 18 deletions(-) diff --git a/pkg/ui/keys.go b/pkg/ui/keys.go index 32bb9be..0969e75 100644 --- a/pkg/ui/keys.go +++ b/pkg/ui/keys.go @@ -14,6 +14,8 @@ type KeyMap struct { PrevFile key.Binding CtrlD key.Binding CtrlU key.Binding + ScrollLeft key.Binding + ScrollRight key.Binding ToggleFileTree key.Binding Search key.Binding Quit key.Binding @@ -71,6 +73,14 @@ var keys = &KeyMap{ key.WithKeys("ctrl+u"), key.WithHelp("ctrl+u", "diff up"), ), + ScrollLeft: key.NewBinding( + key.WithKeys("left"), + key.WithHelp("←", "scroll left"), + ), + ScrollRight: key.NewBinding( + key.WithKeys("right"), + key.WithHelp("→", "scroll right"), + ), ToggleFileTree: key.NewBinding( key.WithKeys("e"), key.WithHelp("e", "toggle file tree"), @@ -124,6 +134,8 @@ func KeyGroups() [][]key.Binding { keys.PrevFile, keys.CtrlD, keys.CtrlU, + keys.ScrollLeft, + keys.ScrollRight, }, { keys.ToggleFileTree, keys.Search, diff --git a/pkg/ui/panes/diffviewer/diffviewer.go b/pkg/ui/panes/diffviewer/diffviewer.go index 6a6cd4c..2f18ed4 100644 --- a/pkg/ui/panes/diffviewer/diffviewer.go +++ b/pkg/ui/panes/diffviewer/diffviewer.go @@ -45,6 +45,8 @@ type Model struct { cache nodeCache sideBySide bool preamble string + rawText string + xOffset int } // SetPreamble stores the preamble text (e.g. commit metadata from git show). @@ -80,22 +82,12 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { } case diffContentMsg: - // Clip lines that exceed the viewport width and use a visible tail - // marker so users can see content was cut off rather than failing - // silently. In side-by-side mode delta wraps long lines for us; in - // unified mode delta passes them through as-is, so this clip-with- - // marker is the only signal that there's more to the right. - lines := strings.Split(msg.text, "\n") - for i, line := range lines { - if lipgloss.Width(line) > m.vp.Width() && m.vp.Width() > 0 { - lines[i] = ansi.Truncate(line, m.vp.Width(), "…") - } - } - diff := strings.Join(lines, "\n") if _, ok := m.cache[msg.cacheKey]; ok { - m.cache[msg.cacheKey].diff = diff + m.cache[msg.cacheKey].diff = msg.text } - m.vp.SetContent(diff) + m.rawText = msg.text + m.xOffset = 0 + m.renderViewport() } return m, tea.Batch(cmds...) @@ -130,7 +122,9 @@ func (m *Model) diff() tea.Cmd { key := cacheKey(m.file.path, m.sideBySide) if cached, ok := m.cache[key]; ok && cached.diff != "" { m.file = cached - m.vp.SetContent(cached.diff) + m.rawText = cached.diff + m.xOffset = 0 + m.renderViewport() return nil } node := &cachedNode{ @@ -146,7 +140,9 @@ func (m *Model) diff() tea.Cmd { key := cacheKey(m.dir.path, m.sideBySide) if cached, ok := m.cache[key]; ok && cached.diff != "" { m.dir = cached - m.vp.SetContent(cached.diff) + m.rawText = cached.diff + m.xOffset = 0 + m.renderViewport() return nil } node := &cachedNode{ @@ -217,7 +213,9 @@ func (m Model) SetFilePatch(file *gitdiff.File) (Model, tea.Cmd) { key := cacheKey(fname, m.sideBySide) if cached, ok := m.cache[key]; ok { m.file = cached - m.vp.SetContent(cached.diff) + m.rawText = cached.diff + m.xOffset = 0 + m.renderViewport() return m, nil } @@ -241,7 +239,9 @@ func (m Model) SetDirPatch(dirPath string, files []*gitdiff.File) (Model, tea.Cm key := cacheKey(dirPath, m.sideBySide) if cached, ok := m.cache[key]; ok { m.dir = cached - m.vp.SetContent(cached.diff) + m.rawText = cached.diff + m.xOffset = 0 + m.renderViewport() return m, nil } @@ -295,6 +295,85 @@ func (m *Model) ScrollTop() { m.vp.GotoTop() } +func (m *Model) scrollStep() int { + step := m.vp.Width() / 2 + if step < 1 { + step = 1 + } + return step +} + +// ScrollLeft scrolls the viewport horizontally toward column 0. +func (m *Model) ScrollLeft() { + if m.xOffset == 0 { + return + } + m.xOffset -= m.scrollStep() + if m.xOffset < 0 { + m.xOffset = 0 + } + m.renderViewport() +} + +// ScrollRight advances xOffset only if at least one line still has content +// past the visible window. +func (m *Model) ScrollRight() { + if m.rawText == "" { + return + } + vpW := m.vp.Width() + if vpW <= 0 { + return + } + limit := m.xOffset + vpW + for _, line := range strings.Split(m.rawText, "\n") { + if lipgloss.Width(line) > limit { + m.xOffset += m.scrollStep() + m.renderViewport() + return + } + } +} + +func (m *Model) renderViewport() { + if m.rawText == "" { + return + } + vpW := m.vp.Width() + if vpW <= 0 { + m.vp.SetContent(m.rawText) + return + } + lines := strings.Split(m.rawText, "\n") + for i, line := range lines { + lw := lipgloss.Width(line) + if m.xOffset == 0 && lw <= vpW { + continue + } + leftMark := m.xOffset > 0 && lw > m.xOffset + rightMark := lw > m.xOffset+vpW + budget := vpW + if leftMark { + budget-- + } + if rightMark { + budget-- + } + if budget < 0 { + budget = 0 + } + sliced := ansi.Cut(line, m.xOffset, m.xOffset+budget) + if leftMark { + sliced = "…" + sliced + } + if rightMark { + sliced = sliced + "…" + } + lines[i] = sliced + } + m.vp.SetContent(strings.Join(lines, "\n")) +} + func diffFile(node *cachedNode, width int, sideBySide bool) tea.Cmd { if width == 0 || node == nil || len(node.files) != 1 { return nil diff --git a/pkg/ui/tui.go b/pkg/ui/tui.go index feb7bda..35bc05c 100644 --- a/pkg/ui/tui.go +++ b/pkg/ui/tui.go @@ -303,6 +303,14 @@ func (m mainModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } else { m.diffViewer.ScrollTop() } + case key.Matches(msg, keys.ScrollLeft): + if m.activePanel != FileTreePanel { + m.diffViewer.ScrollLeft() + } + case key.Matches(msg, keys.ScrollRight): + if m.activePanel != FileTreePanel { + m.diffViewer.ScrollRight() + } case key.Matches(msg, keys.Copy): cmd = m.fileTree.CopyCurrNodePath() if cmd != nil { From d1bb8307397c49b16c820741da7afdd1c410a1de Mon Sep 17 00:00:00 2001 From: Booyaka101 Date: Sat, 16 May 2026 20:17:44 +0800 Subject: [PATCH 3/3] diffviewer: use viewport.ScrollLeft / ScrollRight directly Per #133 review: don't reimplement what the viewport already provides. bubbles/v2/viewport already exposes ScrollLeft(n) / ScrollRight(n) and manages xOffset / maxXOffset internally, so the local field and renderViewport pass were duplicating that work. ScrollLeft / ScrollRight on the diffviewer are now thin wrappers around the viewport, stepping one column per call (matches the j/k vertical-step convention in this codebase). The conditional ellipsis markers from the previous iteration are dropped: bubbles' viewport handles horizontal scroll by shifting the visible window and doesn't expose a per-visible-line render hook, so keeping the markers would mean reimplementing the slice path the review asked to remove. Happy to add them back in a follow-up via a View() wrapper, or by trying github.com/robinovitch61/viewport which the maintainer flagged as an alternative. The underlying bug fixes for #99 are unchanged: - delta is now invoked with --max-line-length=0 and --wrap-max-lines=unlimited in both diffFile and diffDir, so side-by-side carries the full line through. - the diffContentMsg handler no longer post-truncates the unified- mode output with an empty tail; SetContent gets the raw delta text and the viewport renders it directly. --- pkg/ui/panes/diffviewer/diffviewer.go | 100 +++----------------------- 1 file changed, 9 insertions(+), 91 deletions(-) diff --git a/pkg/ui/panes/diffviewer/diffviewer.go b/pkg/ui/panes/diffviewer/diffviewer.go index 2f18ed4..f7638a8 100644 --- a/pkg/ui/panes/diffviewer/diffviewer.go +++ b/pkg/ui/panes/diffviewer/diffviewer.go @@ -10,7 +10,6 @@ import ( tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" "github.com/bluekeyes/go-gitdiff/gitdiff" - "github.com/charmbracelet/x/ansi" "github.com/dlvhdr/diffnav/pkg/filenode" "github.com/dlvhdr/diffnav/pkg/icons" @@ -45,8 +44,6 @@ type Model struct { cache nodeCache sideBySide bool preamble string - rawText string - xOffset int } // SetPreamble stores the preamble text (e.g. commit metadata from git show). @@ -85,9 +82,7 @@ func (m Model) Update(msg tea.Msg) (Model, tea.Cmd) { if _, ok := m.cache[msg.cacheKey]; ok { m.cache[msg.cacheKey].diff = msg.text } - m.rawText = msg.text - m.xOffset = 0 - m.renderViewport() + m.vp.SetContent(msg.text) } return m, tea.Batch(cmds...) @@ -122,9 +117,7 @@ func (m *Model) diff() tea.Cmd { key := cacheKey(m.file.path, m.sideBySide) if cached, ok := m.cache[key]; ok && cached.diff != "" { m.file = cached - m.rawText = cached.diff - m.xOffset = 0 - m.renderViewport() + m.vp.SetContent(cached.diff) return nil } node := &cachedNode{ @@ -140,9 +133,7 @@ func (m *Model) diff() tea.Cmd { key := cacheKey(m.dir.path, m.sideBySide) if cached, ok := m.cache[key]; ok && cached.diff != "" { m.dir = cached - m.rawText = cached.diff - m.xOffset = 0 - m.renderViewport() + m.vp.SetContent(cached.diff) return nil } node := &cachedNode{ @@ -213,9 +204,7 @@ func (m Model) SetFilePatch(file *gitdiff.File) (Model, tea.Cmd) { key := cacheKey(fname, m.sideBySide) if cached, ok := m.cache[key]; ok { m.file = cached - m.rawText = cached.diff - m.xOffset = 0 - m.renderViewport() + m.vp.SetContent(cached.diff) return m, nil } @@ -239,9 +228,7 @@ func (m Model) SetDirPatch(dirPath string, files []*gitdiff.File) (Model, tea.Cm key := cacheKey(dirPath, m.sideBySide) if cached, ok := m.cache[key]; ok { m.dir = cached - m.rawText = cached.diff - m.xOffset = 0 - m.renderViewport() + m.vp.SetContent(cached.diff) return m, nil } @@ -295,83 +282,14 @@ func (m *Model) ScrollTop() { m.vp.GotoTop() } -func (m *Model) scrollStep() int { - step := m.vp.Width() / 2 - if step < 1 { - step = 1 - } - return step -} - -// ScrollLeft scrolls the viewport horizontally toward column 0. +// ScrollLeft scrolls the viewport one column toward column 0. func (m *Model) ScrollLeft() { - if m.xOffset == 0 { - return - } - m.xOffset -= m.scrollStep() - if m.xOffset < 0 { - m.xOffset = 0 - } - m.renderViewport() + m.vp.ScrollLeft(1) } -// ScrollRight advances xOffset only if at least one line still has content -// past the visible window. +// ScrollRight scrolls the viewport one column away from column 0. func (m *Model) ScrollRight() { - if m.rawText == "" { - return - } - vpW := m.vp.Width() - if vpW <= 0 { - return - } - limit := m.xOffset + vpW - for _, line := range strings.Split(m.rawText, "\n") { - if lipgloss.Width(line) > limit { - m.xOffset += m.scrollStep() - m.renderViewport() - return - } - } -} - -func (m *Model) renderViewport() { - if m.rawText == "" { - return - } - vpW := m.vp.Width() - if vpW <= 0 { - m.vp.SetContent(m.rawText) - return - } - lines := strings.Split(m.rawText, "\n") - for i, line := range lines { - lw := lipgloss.Width(line) - if m.xOffset == 0 && lw <= vpW { - continue - } - leftMark := m.xOffset > 0 && lw > m.xOffset - rightMark := lw > m.xOffset+vpW - budget := vpW - if leftMark { - budget-- - } - if rightMark { - budget-- - } - if budget < 0 { - budget = 0 - } - sliced := ansi.Cut(line, m.xOffset, m.xOffset+budget) - if leftMark { - sliced = "…" + sliced - } - if rightMark { - sliced = sliced + "…" - } - lines[i] = sliced - } - m.vp.SetContent(strings.Join(lines, "\n")) + m.vp.ScrollRight(1) } func diffFile(node *cachedNode, width int, sideBySide bool) tea.Cmd {