detached afb8c1a

fix(theme): resolve accent render review findings

commit
afb8c1abf2c3ed977851d9f7795380d50b0691db
author
brice · · 2026-07-18 11:40 UTC
parent
c5fa06a
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
8 files · +69 -26
Unified
UnifiedSplit
M internal/usecase/daemon/copymode.go +1 -1
@@ -416,7 +416,7 @@ func composeCopyClientFrame(mode *scopy.Mode, target domain.Rect, frame renderer
416416 if mode == nil || target.Width <= 0 || target.Height <= 0 || frame.Width <= 0 || frame.Height <= 0 {
417417 return frame, nil
418418 }
419- copyFrame := mode.Render(styles.SurfaceBar, styles.Selection)
419+ copyFrame := mode.Render(styles.CopyStatus, styles.Selection)
420420 bodyRows := max(copyFrame.Height-1, 0)
421421 for y := 0; y < target.Height && y < bodyRows && target.Y+y < frame.Height-1; y++ {
422422 dstX := max(target.X, 0)
@@ -416,7 +416,7 @@ func composeCopyClientFrame(mode *scopy.Mode, target domain.Rect, frame renderer
416 if mode == nil || target.Width <= 0 || target.Height <= 0 || frame.Width <= 0 || frame.Height <= 0 {416 if mode == nil || target.Width <= 0 || target.Height <= 0 || frame.Width <= 0 || frame.Height <= 0 {
417 return frame, nil417 return frame, nil
418 }418 }
419- copyFrame := mode.Render(styles.SurfaceBar, styles.Selection)419+ copyFrame := mode.Render(styles.CopyStatus, styles.Selection)
420 bodyRows := max(copyFrame.Height-1, 0)420 bodyRows := max(copyFrame.Height-1, 0)
421 for y := 0; y < target.Height && y < bodyRows && target.Y+y < frame.Height-1; y++ {421 for y := 0; y < target.Height && y < bodyRows && target.Y+y < frame.Height-1; y++ {
422 dstX := max(target.X, 0)422 dstX := max(target.X, 0)
M internal/usecase/daemon/copymode_test.go +31 -13
@@ -881,18 +881,36 @@ func TestMouseDragCopyEntryCapturesSourceForYank(t *testing.T) {
881881 require.Equal(t, want, string(msg.Data))
882882 }
883883
884-func TestComposeCopyClientFrameFillsFullStatusRowForSplitPane(t *testing.T) {
885- base := renderer.NewFrame(20, 8)
886- pane := newPane("split", nil, domain.Size{Cols: 12, Rows: 2})
887- pane.screen.Write([]byte("copy"))
888- mode := scopy.NewMode(scopy.NewDocument(scopy.NewSnapshot(pane.history, pane.screen.Frame), domain.DefaultWordSeparators))
889- styles := resolveStyles(nil)
890- styles.SurfaceBar = renderer.Style{Foreground: 7, Background: 3}
891-
892- frame, _ := composeCopyClientFrame(mode, domain.Rect{X: 0, Y: 1, Width: 12, Height: 2}, base, styles)
893-
894- for x, cell := range frame.Row(frame.Height - 1) {
895- require.Truef(t, cell.Style.Equal(styles.SurfaceBar), "status cell %d must retain the full-width SurfaceBar role", x)
884+func TestComposeCopyClientFrameStylesStatusContentAndSurround(t *testing.T) {
885+ for _, tt := range []struct {
886+ name string
887+ selectMode bool
888+ want string
889+ }{
890+ {name: "scroll", want: "[SCROLL]"},
891+ {name: "selection", selectMode: true, want: "[SELECT]"},
892+ } {
893+ t.Run(tt.name, func(t *testing.T) {
894+ base := renderer.NewFrame(20, 8)
895+ pane := newPane("split", nil, domain.Size{Cols: 12, Rows: 2})
896+ pane.screen.Write([]byte("copy"))
897+ mode := scopy.NewMode(scopy.NewDocument(scopy.NewSnapshot(pane.history, pane.screen.Frame), domain.DefaultWordSeparators))
898+ if tt.selectMode {
899+ mode.ToggleLineSelection()
900+ }
901+ styles := resolveStyles(nil)
902+ styles.SurfaceBar = renderer.Style{Foreground: 7, Background: 3}
903+ styles.CopyStatus = renderer.Style{Foreground: 5, Background: 4}
904+
905+ frame, _ := composeCopyClientFrame(mode, domain.Rect{X: 0, Y: 1, Width: 12, Height: 2}, base, styles)
906+ status := frame.Row(frame.Height - 1)
907+ for x := range 12 {
908+ require.Truef(t, status[x].Style.Equal(styles.CopyStatus), "status content cell %d must use CopyStatus", x)
909+ }
910+ for x := 12; x < len(status); x++ {
911+ require.Truef(t, status[x].Style.Equal(styles.SurfaceBar), "surrounding status cell %d must use SurfaceBar", x)
912+ }
913+ require.Contains(t, rowText(status), tt.want)
914+ })
896915 }
897- require.Contains(t, rowText(frame.Row(frame.Height-1)), "[SCROLL]")
898916 }
@@ -881,18 +881,36 @@ func TestMouseDragCopyEntryCapturesSourceForYank(t *testing.T) {
881 require.Equal(t, want, string(msg.Data))881 require.Equal(t, want, string(msg.Data))
882 }882 }
883 883
884-func TestComposeCopyClientFrameFillsFullStatusRowForSplitPane(t *testing.T) {884+func TestComposeCopyClientFrameStylesStatusContentAndSurround(t *testing.T) {
885- base := renderer.NewFrame(20, 8)885+ for _, tt := range []struct {
886- pane := newPane("split", nil, domain.Size{Cols: 12, Rows: 2})886+ name string
887- pane.screen.Write([]byte("copy"))887+ selectMode bool
888- mode := scopy.NewMode(scopy.NewDocument(scopy.NewSnapshot(pane.history, pane.screen.Frame), domain.DefaultWordSeparators))888+ want string
889- styles := resolveStyles(nil)889+ }{
890- styles.SurfaceBar = renderer.Style{Foreground: 7, Background: 3}890+ {name: "scroll", want: "[SCROLL]"},
891-891+ {name: "selection", selectMode: true, want: "[SELECT]"},
892- frame, _ := composeCopyClientFrame(mode, domain.Rect{X: 0, Y: 1, Width: 12, Height: 2}, base, styles)892+ } {
893-893+ t.Run(tt.name, func(t *testing.T) {
894- for x, cell := range frame.Row(frame.Height - 1) {894+ base := renderer.NewFrame(20, 8)
895- require.Truef(t, cell.Style.Equal(styles.SurfaceBar), "status cell %d must retain the full-width SurfaceBar role", x)895+ pane := newPane("split", nil, domain.Size{Cols: 12, Rows: 2})
896+ pane.screen.Write([]byte("copy"))
897+ mode := scopy.NewMode(scopy.NewDocument(scopy.NewSnapshot(pane.history, pane.screen.Frame), domain.DefaultWordSeparators))
898+ if tt.selectMode {
899+ mode.ToggleLineSelection()
900+ }
901+ styles := resolveStyles(nil)
902+ styles.SurfaceBar = renderer.Style{Foreground: 7, Background: 3}
903+ styles.CopyStatus = renderer.Style{Foreground: 5, Background: 4}
904+
905+ frame, _ := composeCopyClientFrame(mode, domain.Rect{X: 0, Y: 1, Width: 12, Height: 2}, base, styles)
906+ status := frame.Row(frame.Height - 1)
907+ for x := range 12 {
908+ require.Truef(t, status[x].Style.Equal(styles.CopyStatus), "status content cell %d must use CopyStatus", x)
909+ }
910+ for x := 12; x < len(status); x++ {
911+ require.Truef(t, status[x].Style.Equal(styles.SurfaceBar), "surrounding status cell %d must use SurfaceBar", x)
912+ }
913+ require.Contains(t, rowText(status), tt.want)
914+ })
896 }915 }
897- require.Contains(t, rowText(frame.Row(frame.Height-1)), "[SCROLL]")
898 }916 }
M internal/usecase/daemon/picker_test.go +1 -1
@@ -774,7 +774,7 @@ func TestCaptureOverlayLayersResizeRecomposesPickerWithoutStalePreview(t *testin
774774 for x := layout.Separator.X; x < layout.Separator.X+layout.Separator.Width; x++ {
775775 cell := inner.At(x, y)
776776 require.Equal(t, tt.wantSep, cell.Rune)
777- require.True(t, cell.Style.Equal(themeui.MutedTextStyle(state.theme)))
777+ require.True(t, cell.Style.Equal(state.styles.PickerSeparator), "separator must use the cached PickerSeparator role")
778778 }
779779 }
780780 })
@@ -774,7 +774,7 @@ func TestCaptureOverlayLayersResizeRecomposesPickerWithoutStalePreview(t *testin
774 for x := layout.Separator.X; x < layout.Separator.X+layout.Separator.Width; x++ {774 for x := layout.Separator.X; x < layout.Separator.X+layout.Separator.Width; x++ {
775 cell := inner.At(x, y)775 cell := inner.At(x, y)
776 require.Equal(t, tt.wantSep, cell.Rune)776 require.Equal(t, tt.wantSep, cell.Rune)
777- require.True(t, cell.Style.Equal(themeui.MutedTextStyle(state.theme)))777+ require.True(t, cell.Style.Equal(state.styles.PickerSeparator), "separator must use the cached PickerSeparator role")
778 }778 }
779 }779 }
780 })780 })
M internal/usecase/daemon/render_pipeline.go +2 -4
@@ -171,10 +171,8 @@ func composeFrame(state capturedRenderState, in composeCacheInput, scratchIn ...
171171 }
172172
173173 func drawCapturedPaneTitleBar(frame renderer.Frame, pl layout.Placement, title string, focused bool, styles themeui.Styles, neutralBorder renderer.Style, dimmer themeui.Dimmer) {
174- style := neutralBorder
175- if focused {
176- style = styles.StatusBar
177- } else {
174+ style := styles.StatusBar
175+ if !focused {
178176 style = dimmer.Dim(neutralBorder)
179177 }
180178 for x := pl.TitleBar.X; x < pl.TitleBar.X+pl.TitleBar.Width && x < frame.Width; x++ {
@@ -171,10 +171,8 @@ func composeFrame(state capturedRenderState, in composeCacheInput, scratchIn ...
171 }171 }
172 172
173 func drawCapturedPaneTitleBar(frame renderer.Frame, pl layout.Placement, title string, focused bool, styles themeui.Styles, neutralBorder renderer.Style, dimmer themeui.Dimmer) {173 func drawCapturedPaneTitleBar(frame renderer.Frame, pl layout.Placement, title string, focused bool, styles themeui.Styles, neutralBorder renderer.Style, dimmer themeui.Dimmer) {
174- style := neutralBorder174+ style := styles.StatusBar
175- if focused {175+ if !focused {
176- style = styles.StatusBar
177- } else {
178 style = dimmer.Dim(neutralBorder)176 style = dimmer.Dim(neutralBorder)
179 }177 }
180 for x := pl.TitleBar.X; x < pl.TitleBar.X+pl.TitleBar.Width && x < frame.Width; x++ {178 for x := pl.TitleBar.X; x < pl.TitleBar.X+pl.TitleBar.Width && x < frame.Width; x++ {
M internal/usecase/theme/resolved_styles.go +1 -0
@@ -100,6 +100,7 @@ func indexedStyles(styles Styles, slot uint8) Styles {
100100 func indexedForeground(style renderer.Style, slot uint8) renderer.Style {
101101 style.HasForegroundRGB = false
102102 style.Foreground = int(slot)
103+ style.Inverse = false
103104 return style
104105 }
105106
@@ -100,6 +100,7 @@ func indexedStyles(styles Styles, slot uint8) Styles {
100 func indexedForeground(style renderer.Style, slot uint8) renderer.Style {100 func indexedForeground(style renderer.Style, slot uint8) renderer.Style {
101 style.HasForegroundRGB = false101 style.HasForegroundRGB = false
102 style.Foreground = int(slot)102 style.Foreground = int(slot)
103+ style.Inverse = false
103 return style104 return style
104 }105 }
105 106
M internal/usecase/theme/styles_test.go +15 -0
@@ -139,6 +139,21 @@ func TestPulseColor(t *testing.T) {
139139 }
140140 }
141141
142+func TestIndexedForegroundClearsInverseForNonTrueColorFallback(t *testing.T) {
143+ fallback := indexedForeground(renderer.Style{
144+ Bold: true,
145+ Inverse: true,
146+ Foreground: -1,
147+ HasForegroundRGB: true,
148+ ForegroundRGB: renderer.RGB{R: 1, G: 2, B: 3},
149+ }, 2)
150+
151+ require.False(t, fallback.Inverse, "an indexed foreground must not become an effective background")
152+ require.False(t, fallback.HasForegroundRGB)
153+ require.Equal(t, 2, fallback.Foreground)
154+ require.True(t, fallback.Bold)
155+}
156+
142157 func TestMRUFade(t *testing.T) {
143158 base := renderer.Style{
144159 Bold: true,
@@ -139,6 +139,21 @@ func TestPulseColor(t *testing.T) {
139 }139 }
140 }140 }
141 141
142+func TestIndexedForegroundClearsInverseForNonTrueColorFallback(t *testing.T) {
143+ fallback := indexedForeground(renderer.Style{
144+ Bold: true,
145+ Inverse: true,
146+ Foreground: -1,
147+ HasForegroundRGB: true,
148+ ForegroundRGB: renderer.RGB{R: 1, G: 2, B: 3},
149+ }, 2)
150+
151+ require.False(t, fallback.Inverse, "an indexed foreground must not become an effective background")
152+ require.False(t, fallback.HasForegroundRGB)
153+ require.Equal(t, 2, fallback.Foreground)
154+ require.True(t, fallback.Bold)
155+}
156+
142 func TestMRUFade(t *testing.T) {157 func TestMRUFade(t *testing.T) {
143 base := renderer.Style{158 base := renderer.Style{
144 Bold: true,159 Bold: true,
M internal/usecase/visualsearch/model_test.go +17 -7
@@ -187,13 +187,23 @@ func rowText(row []renderer.Cell) string {
187187 return b.String()
188188 }
189189
190-func TestVisualSearchRenderStyledFillsBaseAndSelection(t *testing.T) {
191- m := New(testSnapshot("alpha"))
192- m.Insert('a')
190+func TestVisualSearchRenderStyledUsesSemanticRoles(t *testing.T) {
193191 base := renderer.Style{Foreground: 1, Background: 2}
194192 selection := renderer.Style{Foreground: 3, Background: 4}
195- frame := m.RenderStyled(domain.Size{Cols: 20, Rows: 4}, RenderStyles{Base: base, Selection: selection})
196-
197- require.True(t, frame.At(19, 3).Style.Equal(base), "blank filler keeps base surface")
198- require.True(t, frame.At(2, 0).Style.Equal(selection), "input caret keeps selection surface")
193+ for _, tt := range []struct {
194+ name string
195+ x, y int
196+ want renderer.Style
197+ why string
198+ }{
199+ {name: "blank filler", x: 19, y: 3, want: base, why: "blank filler keeps base surface"},
200+ {name: "input caret", x: 2, y: 0, want: selection, why: "input caret keeps selection surface"},
201+ } {
202+ t.Run(tt.name, func(t *testing.T) {
203+ m := New(testSnapshot("alpha"))
204+ m.Insert('a')
205+ frame := m.RenderStyled(domain.Size{Cols: 20, Rows: 4}, RenderStyles{Base: base, Selection: selection})
206+ require.True(t, frame.At(tt.x, tt.y).Style.Equal(tt.want), tt.why)
207+ })
208+ }
199209 }
@@ -187,13 +187,23 @@ func rowText(row []renderer.Cell) string {
187 return b.String()187 return b.String()
188 }188 }
189 189
190-func TestVisualSearchRenderStyledFillsBaseAndSelection(t *testing.T) {190+func TestVisualSearchRenderStyledUsesSemanticRoles(t *testing.T) {
191- m := New(testSnapshot("alpha"))
192- m.Insert('a')
193 base := renderer.Style{Foreground: 1, Background: 2}191 base := renderer.Style{Foreground: 1, Background: 2}
194 selection := renderer.Style{Foreground: 3, Background: 4}192 selection := renderer.Style{Foreground: 3, Background: 4}
195- frame := m.RenderStyled(domain.Size{Cols: 20, Rows: 4}, RenderStyles{Base: base, Selection: selection})193+ for _, tt := range []struct {
196-194+ name string
197- require.True(t, frame.At(19, 3).Style.Equal(base), "blank filler keeps base surface")195+ x, y int
198- require.True(t, frame.At(2, 0).Style.Equal(selection), "input caret keeps selection surface")196+ want renderer.Style
197+ why string
198+ }{
199+ {name: "blank filler", x: 19, y: 3, want: base, why: "blank filler keeps base surface"},
200+ {name: "input caret", x: 2, y: 0, want: selection, why: "input caret keeps selection surface"},
201+ } {
202+ t.Run(tt.name, func(t *testing.T) {
203+ m := New(testSnapshot("alpha"))
204+ m.Insert('a')
205+ frame := m.RenderStyled(domain.Size{Cols: 20, Rows: 4}, RenderStyles{Base: base, Selection: selection})
206+ require.True(t, frame.At(tt.x, tt.y).Style.Equal(tt.want), tt.why)
207+ })
208+ }
199 }209 }
M transport_replay_integration_test.go +1 -0
@@ -113,6 +113,7 @@ func TestThemeGenerationTransportSequences(t *testing.T) {
113113 frame, err := receiver.Recv()
114114 require.NoError(t, err)
115115 require.Equal(t, ports.MsgTheme, frame.Type)
116+ require.Equal(t, ports.MarshalTheme(want), frame.Payload, "theme payload must be preserved byte-for-byte before decode")
116117 got, err := ports.UnmarshalTheme(frame.Payload)
117118 require.NoError(t, err)
118119 require.Equal(t, want, got)
@@ -113,6 +113,7 @@ func TestThemeGenerationTransportSequences(t *testing.T) {
113 frame, err := receiver.Recv()113 frame, err := receiver.Recv()
114 require.NoError(t, err)114 require.NoError(t, err)
115 require.Equal(t, ports.MsgTheme, frame.Type)115 require.Equal(t, ports.MsgTheme, frame.Type)
116+ require.Equal(t, ports.MarshalTheme(want), frame.Payload, "theme payload must be preserved byte-for-byte before decode")
116 got, err := ports.UnmarshalTheme(frame.Payload)117 got, err := ports.UnmarshalTheme(frame.Payload)
117 require.NoError(t, err)118 require.NoError(t, err)
118 require.Equal(t, want, got)119 require.Equal(t, want, got)

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close