fix: pass non-unified-diff input through to stdout (closes #115)#132
Merged
Conversation
When diffnav is configured as `pager.diff` (per the README), git pipes the output of every `git diff` invocation through diffnav, including non-unified-diff variants like `git diff --stat`, `--shortstat`, `--name-only`, and `--name-status`. Those produce summary text with no `diff --git ` markers, which causes two latent bugs: 1. The `bluekeyes/go-gitdiff` parser returns zero files, the TUI tries to render a scrollbar over an empty viewport, and the program panics with `runtime error: integer divide by zero` in `RenderScrollbar` (`pkg/ui/common/scrollbar.go:18`). 2. Before the panic, bubbletea's terminal-capability handshake leaks `[?2026$p[?2027$p` query bytes to the user's stdout — the symptom in the bug report. Detect non-unified-diff input early in `cmd/root.go` (after stdin is fully read, before opening the TTY) and pass it through verbatim, so the caller sees the same output they would have without diffnav as the configured pager. Detection is a simple `strings.Contains(input, "diff --git ")` check, extracted into `isUnifiedDiff` for unit testing. Both `git diff` and `git show` always emit at least one `diff --git ` header in the unified form, so this matches the renderer's actual input contract. Eight unit tests cover the unified-diff happy path (`diff`, `show`), the four summary forms (`--stat`, `--shortstat`, `--name-only`, `--name-status`), patch-less `git log`, and empty input. Empirically reproduced on Windows (`go build` + piping the canonical `--stat` output). Pre-fix: terminal-capability bytes leak then divide- by-zero panic. Post-fix: stat output is printed verbatim and diffnav exits 0. Real unified-diff input still routes to the TUI as before. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dlvhdr
reviewed
May 15, 2026
Comment on lines
+205
to
+215
|
|
||
| // When configured as `pager.diff`, git pipes the output of every | ||
| // `git diff` invocation here, including non-unified-diff variants | ||
| // like `git diff --stat`, `--shortstat`, `--name-only`, and | ||
| // `--name-status`. Those produce summary text with no `diff --git` | ||
| // markers, which (a) gives the TUI zero files and panics with a | ||
| // divide-by-zero in the scrollbar renderer, and (b) leaks | ||
| // terminal-capability query bytes from bubbletea init back to the | ||
| // user's prompt. Pass such input straight through to stdout so the | ||
| // caller sees the same output they would have without diffnav as | ||
| // the configured pager. |
Contributor
Author
There was a problem hiding this comment.
Done — removed in 0b8101b. isUnifiedDiff's godoc already covers the intent.
The isUnifiedDiff helper already documents the intent; the explanation is repeated in the PR description for context.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When diffnav is configured as
pager.diff(per the README), git pipes the output of everygit diffinvocation through diffnav, including non-unified-diff variants likegit diff --stat,--shortstat,--name-only, and--name-status. Those produce summary text with nodiff --gitmarkers, which causes two latent bugs:bluekeyes/go-gitdiffparser returns zero files, the TUI tries to render a scrollbar over an empty viewport, and the program panics withruntime error: integer divide by zeroinRenderScrollbar(pkg/ui/common/scrollbar.go:18).[?2026\$p[?2027\$pquery bytes to the user's stdout — the symptom in the bug report.Fix
Detect non-unified-diff input early in
cmd/root.go(after stdin is fully read, before opening the TTY) and pass it through verbatim. The caller sees the same output they would have without diffnav as the configured pager — same behaviour ascat/the implicit pager-fallthrough that other TUI pagers use for non-diff input.Detection is a
strings.Contains(input, "diff --git ")check, extracted into a smallisUnifiedDiffhelper for unit testing. Bothgit diffandgit showalways emit at least onediff --githeader in the unified form, so the check matches the TUI's actual input contract.Test plan
cmd/input_test.gocovering:git diff(unified)git show(preamble + unified)git diff --statgit diff --shortstatgit diff --name-onlygit diff --name-statusgit log(no patch)--statoutput:[?2026\$p[?2027\$pbytes leak, thenruntime error: integer divide by zeropanic.gh_dash_pr.difftest fixture into the post-fix binary — TUI starts, hits the same pre-existing panic the upstream test environment hits, confirming the input is reaching the renderer).Notes
pkg/ui/panes/filetreetest failures onmain(TestNoLastPath,TestCollapseTree) are unrelated lipgloss-styling differences and not touched by this PR.git diff --statto not work #115. Should also fix thegit logsymptom from Does not work withgit log#28 (re-emerged after the recent v2 rewrite, sincegit logwithout-pproduces nodiff --gitline and would hit the same path).AI usage disclosure (per AI_POLICY.md)
Tool: Claude Code (Opus 4.7).
Extent: pattern analysis (root cause for the panic + escape leak), code drafting for the
isUnifiedDiffhelper and the early-return path, and the test cases. I reviewed every change manually, rango build,go test ./cmd/..., and reproduced the pre-/post-fix behaviour locally on Windows before pushing. Happy to walk through any line on request.