fix(picker): draw tree lines in a fixed grey and trim doubled detail spaces
- commit
- c549dd31699fbc9bfa42b43fc6188ca94c4a4f11
- author
- brice · · 2026-09-24 03:56 UTC
- parent
- 3a14a3c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +75 -5
M internal/usecase/client/picker_interaction_test.go +2 -2
| @@ -44,7 +44,7 @@ func TestPickerRendererPreservesANSI256ProfileAcrossResize(t *testing.T) { | |||
| 44 | 44 | styles := picker.RenderStyles{ | |
| 45 | 45 | Selection: rgbPickerStyle(), SelectionName: rgbPickerStyle(), SelectionMuted: rgbPickerStyle(), | |
| 46 | 46 | Name: rgbPickerStyle(), Detail: rgbPickerStyle(), Background: rgbPickerStyle(), Base: rgbPickerStyle(), | |
| 47 | - | Stopped: rgbPickerStyle(), Separator: rgbPickerStyle(), Status: rgbPickerStyle(), | |
| 47 | + | Stopped: rgbPickerStyle(), Separator: rgbPickerStyle(), Tree: rgbPickerStyle(), Status: rgbPickerStyle(), | |
| 48 | 48 | SearchMatch: rgbPickerStyle(), SelectionMatch: rgbPickerStyle(), | |
| 49 | 49 | } | |
| 50 | 50 | ||
| @@ -70,7 +70,7 @@ func TestPickerRenderersFollowTerminalColorProfile(t *testing.T) { | |||
| 70 | 70 | styles := picker.RenderStyles{ | |
| 71 | 71 | Selection: rgbPickerStyle(), SelectionName: rgbPickerStyle(), SelectionMuted: rgbPickerStyle(), | |
| 72 | 72 | Name: rgbPickerStyle(), Detail: rgbPickerStyle(), Background: rgbPickerStyle(), Base: rgbPickerStyle(), | |
| 73 | - | Stopped: rgbPickerStyle(), Separator: rgbPickerStyle(), Status: rgbPickerStyle(), | |
| 73 | + | Stopped: rgbPickerStyle(), Separator: rgbPickerStyle(), Tree: rgbPickerStyle(), Status: rgbPickerStyle(), | |
| 74 | 74 | SearchMatch: rgbPickerStyle(), SelectionMatch: rgbPickerStyle(), | |
| 75 | 75 | } | |
| 76 | 76 | renderers := map[string]func(trueColor bool) *pickerRenderer{ | |
| @@ -44,7 +44,7 @@ func TestPickerRendererPreservesANSI256ProfileAcrossResize(t *testing.T) { | |||||
| 44 | styles := picker.RenderStyles{ | 44 | styles := picker.RenderStyles{ | ||
| 45 | Selection: rgbPickerStyle(), SelectionName: rgbPickerStyle(), SelectionMuted: rgbPickerStyle(), | 45 | Selection: rgbPickerStyle(), SelectionName: rgbPickerStyle(), SelectionMuted: rgbPickerStyle(), | ||
| 46 | Name: rgbPickerStyle(), Detail: rgbPickerStyle(), Background: rgbPickerStyle(), Base: rgbPickerStyle(), | 46 | Name: rgbPickerStyle(), Detail: rgbPickerStyle(), Background: rgbPickerStyle(), Base: rgbPickerStyle(), | ||
| 47 | - | Stopped: rgbPickerStyle(), Separator: rgbPickerStyle(), Status: rgbPickerStyle(), | 47 | + | Stopped: rgbPickerStyle(), Separator: rgbPickerStyle(), Tree: rgbPickerStyle(), Status: rgbPickerStyle(), |
| 48 | SearchMatch: rgbPickerStyle(), SelectionMatch: rgbPickerStyle(), | 48 | SearchMatch: rgbPickerStyle(), SelectionMatch: rgbPickerStyle(), | ||
| 49 | } | 49 | } | ||
| 50 | 50 | ||||
| @@ -70,7 +70,7 @@ func TestPickerRenderersFollowTerminalColorProfile(t *testing.T) { | |||||
| 70 | styles := picker.RenderStyles{ | 70 | styles := picker.RenderStyles{ | ||
| 71 | Selection: rgbPickerStyle(), SelectionName: rgbPickerStyle(), SelectionMuted: rgbPickerStyle(), | 71 | Selection: rgbPickerStyle(), SelectionName: rgbPickerStyle(), SelectionMuted: rgbPickerStyle(), | ||
| 72 | Name: rgbPickerStyle(), Detail: rgbPickerStyle(), Background: rgbPickerStyle(), Base: rgbPickerStyle(), | 72 | Name: rgbPickerStyle(), Detail: rgbPickerStyle(), Background: rgbPickerStyle(), Base: rgbPickerStyle(), | ||
| 73 | - | Stopped: rgbPickerStyle(), Separator: rgbPickerStyle(), Status: rgbPickerStyle(), | 73 | + | Stopped: rgbPickerStyle(), Separator: rgbPickerStyle(), Tree: rgbPickerStyle(), Status: rgbPickerStyle(), |
| 74 | SearchMatch: rgbPickerStyle(), SelectionMatch: rgbPickerStyle(), | 74 | SearchMatch: rgbPickerStyle(), SelectionMatch: rgbPickerStyle(), | ||
| 75 | } | 75 | } | ||
| 76 | renderers := map[string]func(trueColor bool) *pickerRenderer{ | 76 | renderers := map[string]func(trueColor bool) *pickerRenderer{ | ||
M internal/usecase/picker/model.go +22 -2
| @@ -57,6 +57,7 @@ type RenderStyles struct { | |||
| 57 | 57 | Base renderer.Style // non-selected row fill + suffixes | |
| 58 | 58 | Stopped renderer.Style // full row style for non-selected stopped rows (fill, name, detail, suffix) | |
| 59 | 59 | Separator renderer.Style // preview separator | |
| 60 | + | Tree renderer.Style // foreground of tree prefixes on non-selected rows; zero keeps them dimmed | |
| 60 | 61 | Status renderer.Style // one-row picker-local contextual help | |
| 61 | 62 | SearchMatch renderer.Style // matched runes on ordinary rows | |
| 62 | 63 | SelectionMatch renderer.Style // matched runes on the selected row | |
| @@ -75,7 +76,12 @@ func defaultRenderStyles() RenderStyles { | |||
| 75 | 76 | searchMatch.Bold = true | |
| 76 | 77 | selectionMatch := selection | |
| 77 | 78 | selectionMatch.Bold = true | |
| 78 | - | return RenderStyles{Selection: selection, SelectionName: selection, SelectionMuted: selection, Name: base, Detail: base, Background: base, Base: base, Separator: separator, Stopped: stopped, Status: separator, SearchMatch: searchMatch, SelectionMatch: selectionMatch} | |
| 79 | + | // Tree lines use the palette's bright black (the theme's grey) rather | |
| 80 | + | // than SGR dim: terminals such as kitty draw box-drawing glyphs | |
| 81 | + | // themselves and barely dim them, so dim lines stayed near full white. | |
| 82 | + | tree := renderer.DefaultStyle() | |
| 83 | + | tree.Foreground = 244 // fixed mid grey: readable on dark and light themes | |
| 84 | + | return RenderStyles{Selection: selection, SelectionName: selection, SelectionMuted: selection, Name: base, Detail: base, Background: base, Base: base, Separator: separator, Tree: tree, Stopped: stopped, Status: separator, SearchMatch: searchMatch, SelectionMatch: selectionMatch} | |
| 79 | 85 | } | |
| 80 | 86 | ||
| 81 | 87 | // Preview is a bounded view of the selected pane's visible frame. | |
| @@ -270,6 +276,10 @@ func (m *Model) rebuild(key string, hadKey bool, fallbackIndex int) { | |||
| 270 | 276 | return | |
| 271 | 277 | } | |
| 272 | 278 | for _, line := range m.sortRun(run) { | |
| 279 | + | // The picker draws its own gap before a detail. A source such as a | |
| 280 | + | // tab title (" (fish)") may already lead with a space, which would | |
| 281 | + | // double it; every source, local or remote, is trimmed here. | |
| 282 | + | line.Detail = strings.TrimSpace(line.Detail) | |
| 273 | 283 | m.rows = append(m.rows, row{line: line, foldedLabel: strings.ToLower(line.Label), foldedDetail: strings.ToLower(line.Detail)}) | |
| 274 | 284 | } | |
| 275 | 285 | run = run[:0] | |
| @@ -620,7 +630,17 @@ func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles Render | |||
| 620 | 630 | treeX := rect.X | |
| 621 | 631 | if r.tree != "" { | |
| 622 | 632 | treeStyle := base | |
| 623 | - | treeStyle.Attrs |= renderer.AttrDim | |
| 633 | + | // A caller style set without its own Tree style keeps the row's | |
| 634 | + | // colors dimmed; a zero Tree style would be black on black. | |
| 635 | + | if idx == m.selected || r.line.Stopped || styles.Tree == (renderer.Style{}) { | |
| 636 | + | treeStyle.Attrs |= renderer.AttrDim | |
| 637 | + | } else { | |
| 638 | + | // Only the foreground comes from Tree: the row's fill and its | |
| 639 | + | // dimming (a Dim line or a search miss) stay as they are. | |
| 640 | + | treeStyle.Foreground = styles.Tree.Foreground | |
| 641 | + | treeStyle.HasForegroundRGB = styles.Tree.HasForegroundRGB | |
| 642 | + | treeStyle.ForegroundRGB = styles.Tree.ForegroundRGB | |
| 643 | + | } | |
| 624 | 644 | treeX = ui.DrawText(frame, rect.X, rect.Y+y, max(rect.X, clipX-2), r.tree, treeStyle) | |
| 625 | 645 | } | |
| 626 | 646 | badge := "" | |
| @@ -57,6 +57,7 @@ type RenderStyles struct { | |||||
| 57 | Base renderer.Style // non-selected row fill + suffixes | 57 | Base renderer.Style // non-selected row fill + suffixes | ||
| 58 | Stopped renderer.Style // full row style for non-selected stopped rows (fill, name, detail, suffix) | 58 | Stopped renderer.Style // full row style for non-selected stopped rows (fill, name, detail, suffix) | ||
| 59 | Separator renderer.Style // preview separator | 59 | Separator renderer.Style // preview separator | ||
| 60 | + | Tree renderer.Style // foreground of tree prefixes on non-selected rows; zero keeps them dimmed | |||
| 60 | Status renderer.Style // one-row picker-local contextual help | 61 | Status renderer.Style // one-row picker-local contextual help | ||
| 61 | SearchMatch renderer.Style // matched runes on ordinary rows | 62 | SearchMatch renderer.Style // matched runes on ordinary rows | ||
| 62 | SelectionMatch renderer.Style // matched runes on the selected row | 63 | SelectionMatch renderer.Style // matched runes on the selected row | ||
| @@ -75,7 +76,12 @@ func defaultRenderStyles() RenderStyles { | |||||
| 75 | searchMatch.Bold = true | 76 | searchMatch.Bold = true | ||
| 76 | selectionMatch := selection | 77 | selectionMatch := selection | ||
| 77 | selectionMatch.Bold = true | 78 | selectionMatch.Bold = true | ||
| 78 | - | return RenderStyles{Selection: selection, SelectionName: selection, SelectionMuted: selection, Name: base, Detail: base, Background: base, Base: base, Separator: separator, Stopped: stopped, Status: separator, SearchMatch: searchMatch, SelectionMatch: selectionMatch} | 79 | + | // Tree lines use the palette's bright black (the theme's grey) rather |
| 80 | + | // than SGR dim: terminals such as kitty draw box-drawing glyphs | |||
| 81 | + | // themselves and barely dim them, so dim lines stayed near full white. | |||
| 82 | + | tree := renderer.DefaultStyle() | |||
| 83 | + | tree.Foreground = 244 // fixed mid grey: readable on dark and light themes | |||
| 84 | + | return RenderStyles{Selection: selection, SelectionName: selection, SelectionMuted: selection, Name: base, Detail: base, Background: base, Base: base, Separator: separator, Tree: tree, Stopped: stopped, Status: separator, SearchMatch: searchMatch, SelectionMatch: selectionMatch} | |||
| 79 | } | 85 | } | ||
| 80 | 86 | ||||
| 81 | // Preview is a bounded view of the selected pane's visible frame. | 87 | // Preview is a bounded view of the selected pane's visible frame. | ||
| @@ -270,6 +276,10 @@ func (m *Model) rebuild(key string, hadKey bool, fallbackIndex int) { | |||||
| 270 | return | 276 | return | ||
| 271 | } | 277 | } | ||
| 272 | for _, line := range m.sortRun(run) { | 278 | for _, line := range m.sortRun(run) { | ||
| 279 | + | // The picker draws its own gap before a detail. A source such as a | |||
| 280 | + | // tab title (" (fish)") may already lead with a space, which would | |||
| 281 | + | // double it; every source, local or remote, is trimmed here. | |||
| 282 | + | line.Detail = strings.TrimSpace(line.Detail) | |||
| 273 | m.rows = append(m.rows, row{line: line, foldedLabel: strings.ToLower(line.Label), foldedDetail: strings.ToLower(line.Detail)}) | 283 | m.rows = append(m.rows, row{line: line, foldedLabel: strings.ToLower(line.Label), foldedDetail: strings.ToLower(line.Detail)}) | ||
| 274 | } | 284 | } | ||
| 275 | run = run[:0] | 285 | run = run[:0] | ||
| @@ -620,7 +630,17 @@ func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles Render | |||||
| 620 | treeX := rect.X | 630 | treeX := rect.X | ||
| 621 | if r.tree != "" { | 631 | if r.tree != "" { | ||
| 622 | treeStyle := base | 632 | treeStyle := base | ||
| 623 | - | treeStyle.Attrs |= renderer.AttrDim | 633 | + | // A caller style set without its own Tree style keeps the row's |
| 634 | + | // colors dimmed; a zero Tree style would be black on black. | |||
| 635 | + | if idx == m.selected || r.line.Stopped || styles.Tree == (renderer.Style{}) { | |||
| 636 | + | treeStyle.Attrs |= renderer.AttrDim | |||
| 637 | + | } else { | |||
| 638 | + | // Only the foreground comes from Tree: the row's fill and its | |||
| 639 | + | // dimming (a Dim line or a search miss) stay as they are. | |||
| 640 | + | treeStyle.Foreground = styles.Tree.Foreground | |||
| 641 | + | treeStyle.HasForegroundRGB = styles.Tree.HasForegroundRGB | |||
| 642 | + | treeStyle.ForegroundRGB = styles.Tree.ForegroundRGB | |||
| 643 | + | } | |||
| 624 | treeX = ui.DrawText(frame, rect.X, rect.Y+y, max(rect.X, clipX-2), r.tree, treeStyle) | 644 | treeX = ui.DrawText(frame, rect.X, rect.Y+y, max(rect.X, clipX-2), r.tree, treeStyle) | ||
| 625 | } | 645 | } | ||
| 626 | badge := "" | 646 | badge := "" | ||
M internal/usecase/picker/model_test.go +51 -1
| @@ -489,7 +489,57 @@ func TestTreePrefixesBranchSessionsAndTabs(t *testing.T) { | |||
| 489 | 489 | frame := m.Render(domain.Size{Cols: 40, Rows: 30}, Preview{}) | |
| 490 | 490 | cell := frame.At(0, 1) | |
| 491 | 491 | require.Equal(t, '├', cell.Rune) | |
| 492 | - | require.NotZero(t, cell.Style.Attrs&renderer.AttrDim, "tree lines are dimmed") | |
| 492 | + | require.NotZero(t, cell.Style.Attrs&renderer.AttrDim, "the selected row keeps its dimmed tree") | |
| 493 | + | ||
| 494 | + | cell = frame.At(0, 4) // "└─ beta", not selected | |
| 495 | + | require.Equal(t, '└', cell.Rune) | |
| 496 | + | // A fixed grey, not SGR dim: some terminals barely dim box drawing. | |
| 497 | + | require.Equal(t, 244, cell.Style.Foreground, "tree lines use a readable grey") | |
| 498 | + | require.False(t, cell.Style.HasForegroundRGB) | |
| 499 | + | require.Zero(t, cell.Style.Attrs&renderer.AttrDim) | |
| 500 | + | } | |
| 501 | + | ||
| 502 | + | func TestTreeStyleKeepsRowDimming(t *testing.T) { | |
| 503 | + | tests := []struct { | |
| 504 | + | name string | |
| 505 | + | lines []protocol.PickerLine | |
| 506 | + | query string | |
| 507 | + | }{ | |
| 508 | + | {name: "dim line", lines: func() []protocol.PickerLine { | |
| 509 | + | dim := navLine("b", "beta") | |
| 510 | + | dim.Dim = true | |
| 511 | + | return []protocol.PickerLine{section("local"), navLine("a", "alpha"), dim} | |
| 512 | + | }()}, | |
| 513 | + | {name: "search miss", lines: []protocol.PickerLine{section("local"), navLine("a", "alpha"), navLine("b", "beta")}, query: "alp"}, | |
| 514 | + | } | |
| 515 | + | for _, tt := range tests { | |
| 516 | + | t.Run(tt.name, func(t *testing.T) { | |
| 517 | + | m := New(tt.lines, Config{}) | |
| 518 | + | if tt.query != "" { | |
| 519 | + | m.EnterSearch() | |
| 520 | + | for _, r := range tt.query { | |
| 521 | + | m.InsertSearch(r) | |
| 522 | + | } | |
| 523 | + | } | |
| 524 | + | frame := m.Render(domain.Size{Cols: 40, Rows: 30}, Preview{}) | |
| 525 | + | cell := frame.At(0, 2) // "└─ beta", not selected | |
| 526 | + | require.Equal(t, '└', cell.Rune) | |
| 527 | + | require.Equal(t, 244, cell.Style.Foreground) | |
| 528 | + | require.NotZero(t, cell.Style.Attrs&renderer.AttrDim, "a dimmed row keeps its tree dimmed") | |
| 529 | + | }) | |
| 530 | + | } | |
| 531 | + | } | |
| 532 | + | ||
| 533 | + | func TestDetailLeadingSpaceIsTrimmed(t *testing.T) { | |
| 534 | + | tab := tabLine("a1", "1", protocol.PickerCanNavigate) | |
| 535 | + | tab.Detail = " (fish)" | |
| 536 | + | m := New([]protocol.PickerLine{section("local"), navLine("a", "alpha"), tab}, Config{}) | |
| 537 | + | frame := m.Render(domain.Size{Cols: 40, Rows: 30}, Preview{}) | |
| 538 | + | row := "" | |
| 539 | + | for x := range 14 { | |
| 540 | + | row += string(frame.At(x, 2).Rune) | |
| 541 | + | } | |
| 542 | + | require.Equal(t, " └─ 1 (fish)", row, "one gap, not two") | |
| 493 | 543 | } | |
| 494 | 544 | ||
| 495 | 545 | func TestReplaceLinesPreviousRowRespectsSearch(t *testing.T) { | |
| @@ -489,7 +489,57 @@ func TestTreePrefixesBranchSessionsAndTabs(t *testing.T) { | |||||
| 489 | frame := m.Render(domain.Size{Cols: 40, Rows: 30}, Preview{}) | 489 | frame := m.Render(domain.Size{Cols: 40, Rows: 30}, Preview{}) | ||
| 490 | cell := frame.At(0, 1) | 490 | cell := frame.At(0, 1) | ||
| 491 | require.Equal(t, '├', cell.Rune) | 491 | require.Equal(t, '├', cell.Rune) | ||
| 492 | - | require.NotZero(t, cell.Style.Attrs&renderer.AttrDim, "tree lines are dimmed") | 492 | + | require.NotZero(t, cell.Style.Attrs&renderer.AttrDim, "the selected row keeps its dimmed tree") |
| 493 | + | ||||
| 494 | + | cell = frame.At(0, 4) // "└─ beta", not selected | |||
| 495 | + | require.Equal(t, '└', cell.Rune) | |||
| 496 | + | // A fixed grey, not SGR dim: some terminals barely dim box drawing. | |||
| 497 | + | require.Equal(t, 244, cell.Style.Foreground, "tree lines use a readable grey") | |||
| 498 | + | require.False(t, cell.Style.HasForegroundRGB) | |||
| 499 | + | require.Zero(t, cell.Style.Attrs&renderer.AttrDim) | |||
| 500 | + | } | |||
| 501 | + | ||||
| 502 | + | func TestTreeStyleKeepsRowDimming(t *testing.T) { | |||
| 503 | + | tests := []struct { | |||
| 504 | + | name string | |||
| 505 | + | lines []protocol.PickerLine | |||
| 506 | + | query string | |||
| 507 | + | }{ | |||
| 508 | + | {name: "dim line", lines: func() []protocol.PickerLine { | |||
| 509 | + | dim := navLine("b", "beta") | |||
| 510 | + | dim.Dim = true | |||
| 511 | + | return []protocol.PickerLine{section("local"), navLine("a", "alpha"), dim} | |||
| 512 | + | }()}, | |||
| 513 | + | {name: "search miss", lines: []protocol.PickerLine{section("local"), navLine("a", "alpha"), navLine("b", "beta")}, query: "alp"}, | |||
| 514 | + | } | |||
| 515 | + | for _, tt := range tests { | |||
| 516 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 517 | + | m := New(tt.lines, Config{}) | |||
| 518 | + | if tt.query != "" { | |||
| 519 | + | m.EnterSearch() | |||
| 520 | + | for _, r := range tt.query { | |||
| 521 | + | m.InsertSearch(r) | |||
| 522 | + | } | |||
| 523 | + | } | |||
| 524 | + | frame := m.Render(domain.Size{Cols: 40, Rows: 30}, Preview{}) | |||
| 525 | + | cell := frame.At(0, 2) // "└─ beta", not selected | |||
| 526 | + | require.Equal(t, '└', cell.Rune) | |||
| 527 | + | require.Equal(t, 244, cell.Style.Foreground) | |||
| 528 | + | require.NotZero(t, cell.Style.Attrs&renderer.AttrDim, "a dimmed row keeps its tree dimmed") | |||
| 529 | + | }) | |||
| 530 | + | } | |||
| 531 | + | } | |||
| 532 | + | ||||
| 533 | + | func TestDetailLeadingSpaceIsTrimmed(t *testing.T) { | |||
| 534 | + | tab := tabLine("a1", "1", protocol.PickerCanNavigate) | |||
| 535 | + | tab.Detail = " (fish)" | |||
| 536 | + | m := New([]protocol.PickerLine{section("local"), navLine("a", "alpha"), tab}, Config{}) | |||
| 537 | + | frame := m.Render(domain.Size{Cols: 40, Rows: 30}, Preview{}) | |||
| 538 | + | row := "" | |||
| 539 | + | for x := range 14 { | |||
| 540 | + | row += string(frame.At(x, 2).Rune) | |||
| 541 | + | } | |||
| 542 | + | require.Equal(t, " └─ 1 (fish)", row, "one gap, not two") | |||
| 493 | } | 543 | } | ||
| 494 | 544 | ||||
| 495 | func TestReplaceLinesPreviousRowRespectsSearch(t *testing.T) { | 545 | func TestReplaceLinesPreviousRowRespectsSearch(t *testing.T) { | ||