feat: modernize diff display and migrate Charm UI stack to v2#576
Open
edenreich wants to merge 7 commits into
Open
feat: modernize diff display and migrate Charm UI stack to v2#576edenreich wants to merge 7 commits into
edenreich wants to merge 7 commits into
Conversation
Replaces the hand-rolled parallel-array algorithm in DiffRenderer with a new diffview package built on aymanbagabas/go-udiff. The previous code marked every line after a mid-file insertion as both added and removed; the new implementation produces correct line shifts. - New internal/ui/components/diffview/ package with fluent builder - go-udiff for LCS-based diff computation - chroma syntax highlighting inside diff lines (opt-in via ChromaStyle) - Background-tinted insert/delete rows (GitHub-style) - Width-adaptive Auto layout (split when width >= 160, else unified) - Dark + light default styles tuned to existing themes Legacy DiffRenderer becomes a thin adapter forwarding to DiffView. Public surface unchanged; SetWidth(int) added for terminal-width-aware layout selection. Provider.GetCurrentTheme() exposed so the renderer can pick light/dark style by luminance of the assistant text color.
Imports moved from github.com/charmbracelet/{bubbletea,lipgloss,bubbles,
glamour} to the charm.land/{...}/v2 vanity domains. Mechanical compile
fixes follow from the v2 API differences:
- tea.Model.View() returns tea.View (struct), not string. The 14
affected types either inline-wrap (tea.NewView(x.Render())) or factor
the body into a private viewContent() string with a thin View()
wrapper. Sub-View() callers in chat.go consume .Content.
- bubbles/v2 viewport: Width/Height/YOffset are methods, not fields.
Constructor takes ...Option (WithWidth/WithHeight). All read and
write sites updated.
- bubbles/v2 progress: WithDefaultGradient was removed; switched to
WithDefaultBlend (the v2 equivalent).
- tea.MouseMsg is now an interface; wheel-up handling switched to the
concrete MouseWheelMsg with Button == MouseWheelUp.
- tea.KeyMsg is now an interface; struct literals in tests and the
pending-handler call site switched to tea.KeyPressMsg with the v2 Key
fields (Code for non-printables, Text for printable input). The same
pattern applied in init_github_action_view's text input handler.
- tea.EnableMouseCellMotion / tea.DisableMouse / tea.WithMouseCellMotion
/ tea.WithReportFocus removed; v2 controls these via View.MouseMode
and View.ReportFocus. The toggle-mouse keybinding now drives only the
state and status message; honoring the state in View is a follow-up.
- glamour.WithAutoStyle removed; v2 defaults to dark when no style
option is supplied, matching v1 behavior on dark terminals.
- lipgloss.Color is now a constructor returning color.Color rather
than a string-typed type. colors.GetLipglossColor's return type
becomes color.Color; the in-diff background extraction reads RGBA
from color.Color and re-encodes as hex for the chroma formatter.
- colors_test compares via reflect.DeepEqual instead of stringifying.
All tests pass; lint clean; binary builds.
A fourth built-in theme alongside Tokyo Night, GitHub Light, and Dracula. Uses the official Charm semantic tokens from github.com/charmbracelet/x/exp/charmtone - Sapphire as the primary blue, Cherry for errors, Julep for success, Charple for status, Smoke and Iron for neutrals, Salt for assistant text. Charmtone Keys implement color.Color and expose .Hex(), so each Theme getter just forwards. Registered as "charm" in the default theme provider; users select it via the existing /theme picker or by setting it as the active theme in config. The on-screen luminance check still classifies it as dark, so diffview keeps the dark Style and diff backgrounds read correctly.
edenreich
force-pushed
the
feat/modernize-diff-display
branch
from
a873a2f to
e5503bb
Compare
CI lint started reporting 103 SA5011 "possible nil-pointer dereference"
warnings on this branch in files my changes never touched (config/*_test.go,
internal/utils/shell_tracker_test.go, etc.). The pattern is the standard
`if x == nil { t.Fatal(...) }; useX(x)`. testing.T.Fatal is NoReturn, so
the dereference is safe; local staticcheck recognises that and reports
0 issues, but CI's flagged it. Exclude SA5011 in *_test.go to make the
two environments agree.
edenreich
force-pushed
the
feat/modernize-diff-display
branch
from
adf0787 to
d0ee4a2
Compare
…nput
Bubble Tea v2 changed what KeyMsg.String() returns for named printable
keys. For a space press, v1 returned " " and v2 returns "space". The
input handler used String() both for keybinding lookups *and* for
inserting the literal character into the buffer:
if keys.IsPrintableCharacter(keyStr) {
return handlePrintableCharacter(keyStr, inputView)
}
So pressing space tried to insert "space" (5 chars, IsPrintableCharacter
fails) and the input swallowed the keystroke entirely.
Introduce keys.PrintableText(KeyMsg) that pulls from KeyPressMsg.Text
(the v2 field carrying the literal typed text — " " for space, "a" for
a, "" for Enter/Backspace/arrows/modifier combos). Switch CanInputHandle
and the printable branch of handleCharacterInput to use it. The
keybinding-lookup path keeps using String() for named keys.
Also widen the cursor advance from +1 to +len(literal) so multi-byte
runes (é, 中, etc.) move the cursor past the full insertion.
Two regressions from the Bubble Tea v2 migration prevented conversation-
history scrolling:
1. Mouse wheel had no effect. v2 reads mouse mode from `tea.View.MouseMode`
on every render rather than from a one-shot `tea.EnableMouseCellMotion`
command (which we removed). With MouseMode defaulting to
MouseModeNone, no mouse/wheel events ever reached the program, so
neither the viewport's built-in wheel scrolling nor the userScrolledUp
tracker fired. ChatApplication.View now sets MouseMode to
MouseModeCellMotion whenever app.mouseEnabled is true — so ctrl+s
actually takes effect again.
2. Page Down had no effect. v2's KeyPressMsg{Code: KeyPgDown}.String()
returns "pgdown"; v1 returned "pgdn". The page_down binding only
listed "pgdn" / "page_down", so the v2 keystroke never matched.
Accept "pgdown" as a third alias in the binding and add it to
AllKnownKeys so it isn't misclassified as paste either.
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
1 participant
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
aymanbagabas/go-udiff+alecthomas/chroma/v2. The legacyDiffRendererwalked old/new line arrays in parallel and misrepresented every mid-file insert as a cascade of paired add/delete; the newinternal/ui/components/diffview/package produces correct line shifts, optional in-line syntax highlighting, GitHub-style tinted backgrounds, and a width-adaptive split/unified layout (split at ≥160 cols, unified below). The legacyDiffRendereris now a thin adapter — public API unchanged.charm.land/{bubbletea,lipgloss,bubbles,glamour}/v2). Touches imports across ~60 files and includes the mechanical API fixes:tea.Model.View()now returnstea.View(14 sites), viewportWidth/Height/YOffsetare methods andviewport.NewtakesOptions,tea.MouseMsg/tea.KeyMsgare now interfaces (wheel handler switched toMouseWheelMsg, key struct literals switched totea.KeyPressMsgwithCode/Textfields),glamour.WithAutoStyleremoved,lipgloss.Coloris now acolor.Colorconstructor,progress.WithDefaultGradient→WithDefaultBlend.charmtheme built ongithub.com/charmbracelet/x/exp/charmtonesemantic tokens (Sapphire / Cherry / Julep / Charple / Smoke / Iron / Salt over Pepper). Selectable via the existing/themepicker.Three commits, 70 files, +1533 / −831.
Follow-ups (not in this PR)
chat.diff.layout/chat.file_iconsconfig knobs (thediffviewbuilder already supports both; just needs wiring inconfig/config.go).app.GetMouseEnabled()into the roottea.View.MouseMode(v2 removed the imperative enable/disable commands).