fix(picker): keep the status dot uninverted on the selected row and drop empty host counts
- commit
- 6978461a5cfc525f5f9bc03d24b5cec2a8713eec
- author
- brice · · 2026-09-23 20:29 UTC
- parent
- 7eed007
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +75 -14
M internal/usecase/client/picker_catalogue.go +3 -1
| @@ -908,7 +908,9 @@ func pickerHostDetail(observation ports.BrokerDaemonObservation) string { | |||
| 908 | 908 | if !observation.InventoryKnown { | |
| 909 | 909 | return "…" | |
| 910 | 910 | } | |
| 911 | - | return strconv.Itoa(len(observation.Sessions)) | |
| 911 | + | // A host row is only published when it has no sessions: the status dot | |
| 912 | + | // already says why, so no count is shown. | |
| 913 | + | return "" | |
| 912 | 914 | } | |
| 913 | 915 | ||
| 914 | 916 | func pickerSessionStatus(session catalogue.RemoteCatalogSession) protocol.PickerLineStatus { | |
| @@ -908,7 +908,9 @@ func pickerHostDetail(observation ports.BrokerDaemonObservation) string { | |||||
| 908 | if !observation.InventoryKnown { | 908 | if !observation.InventoryKnown { | ||
| 909 | return "…" | 909 | return "…" | ||
| 910 | } | 910 | } | ||
| 911 | - | return strconv.Itoa(len(observation.Sessions)) | 911 | + | // A host row is only published when it has no sessions: the status dot |
| 912 | + | // already says why, so no count is shown. | |||
| 913 | + | return "" | |||
| 912 | } | 914 | } | ||
| 913 | 915 | ||||
| 914 | func pickerSessionStatus(session catalogue.RemoteCatalogSession) protocol.PickerLineStatus { | 916 | func pickerSessionStatus(session catalogue.RemoteCatalogSession) protocol.PickerLineStatus { | ||
A internal/usecase/client/picker_detail_test.go +32 -0
| @@ -0,0 +1,32 @@ | |||
| 1 | + | package client | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/stretchr/testify/require" | |
| 7 | + | ||
| 8 | + | "github.com/bnema/vev/internal/domain" | |
| 9 | + | "github.com/bnema/vev/internal/ports" | |
| 10 | + | "github.com/bnema/vev/internal/protocol/catalogue" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | func TestPickerShortDetails(t *testing.T) { | |
| 14 | + | tabs := func(n int) []catalogue.RemoteCatalogTab { return make([]catalogue.RemoteCatalogTab, n) } | |
| 15 | + | sessions := []struct { | |
| 16 | + | name string | |
| 17 | + | session catalogue.RemoteCatalogSession | |
| 18 | + | want string | |
| 19 | + | }{ | |
| 20 | + | {name: "no tabs", session: catalogue.RemoteCatalogSession{}, want: ""}, | |
| 21 | + | {name: "tab count", session: catalogue.RemoteCatalogSession{Tabs: tabs(2)}, want: "2"}, | |
| 22 | + | {name: "attached with tabs", session: catalogue.RemoteCatalogSession{Attached: true, Tabs: tabs(2)}, want: "* 2"}, | |
| 23 | + | {name: "attached without tabs", session: catalogue.RemoteCatalogSession{Attached: true}, want: "*"}, | |
| 24 | + | } | |
| 25 | + | for _, tt := range sessions { | |
| 26 | + | t.Run(tt.name, func(t *testing.T) { | |
| 27 | + | require.Equal(t, tt.want, pickerSessionDetail(tt.session)) | |
| 28 | + | }) | |
| 29 | + | } | |
| 30 | + | require.Equal(t, "…", pickerHostDetail(ports.BrokerDaemonObservation{Availability: domain.RemoteAvailabilityUnknown})) | |
| 31 | + | require.Equal(t, "", pickerHostDetail(ports.BrokerDaemonObservation{InventoryKnown: true})) | |
| 32 | + | } | |
| @@ -0,0 +1,32 @@ | |||||
| 1 | + | package client | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/stretchr/testify/require" | |||
| 7 | + | ||||
| 8 | + | "github.com/bnema/vev/internal/domain" | |||
| 9 | + | "github.com/bnema/vev/internal/ports" | |||
| 10 | + | "github.com/bnema/vev/internal/protocol/catalogue" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | func TestPickerShortDetails(t *testing.T) { | |||
| 14 | + | tabs := func(n int) []catalogue.RemoteCatalogTab { return make([]catalogue.RemoteCatalogTab, n) } | |||
| 15 | + | sessions := []struct { | |||
| 16 | + | name string | |||
| 17 | + | session catalogue.RemoteCatalogSession | |||
| 18 | + | want string | |||
| 19 | + | }{ | |||
| 20 | + | {name: "no tabs", session: catalogue.RemoteCatalogSession{}, want: ""}, | |||
| 21 | + | {name: "tab count", session: catalogue.RemoteCatalogSession{Tabs: tabs(2)}, want: "2"}, | |||
| 22 | + | {name: "attached with tabs", session: catalogue.RemoteCatalogSession{Attached: true, Tabs: tabs(2)}, want: "* 2"}, | |||
| 23 | + | {name: "attached without tabs", session: catalogue.RemoteCatalogSession{Attached: true}, want: "*"}, | |||
| 24 | + | } | |||
| 25 | + | for _, tt := range sessions { | |||
| 26 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 27 | + | require.Equal(t, tt.want, pickerSessionDetail(tt.session)) | |||
| 28 | + | }) | |||
| 29 | + | } | |||
| 30 | + | require.Equal(t, "…", pickerHostDetail(ports.BrokerDaemonObservation{Availability: domain.RemoteAvailabilityUnknown})) | |||
| 31 | + | require.Equal(t, "", pickerHostDetail(ports.BrokerDaemonObservation{InventoryKnown: true})) | |||
| 32 | + | } | |||
M internal/usecase/picker/model.go +18 -12
| @@ -174,7 +174,7 @@ func (m *Model) Intent() protocol.PickerIntent { | |||
| 174 | 174 | // SortMode reports the local ordering mode. | |
| 175 | 175 | func (m *Model) SortMode() SortMode { | |
| 176 | 176 | if m == nil { | |
| 177 | - | return SortRecent | |
| 177 | + | return SortGrouped | |
| 178 | 178 | } | |
| 179 | 179 | return m.sort | |
| 180 | 180 | } | |
| @@ -235,12 +235,16 @@ func (m *Model) keysAboveCursor() []string { | |||
| 235 | 235 | ||
| 236 | 236 | // selectFirstKey moves the cursor to the first key still present, if any. | |
| 237 | 237 | func (m *Model) selectFirstKey(keys []string) { | |
| 238 | + | index := make(map[string]int, len(m.rows)) | |
| 239 | + | for idx, candidate := range m.rows { | |
| 240 | + | if m.eligible(idx) { | |
| 241 | + | index[candidate.key()] = idx | |
| 242 | + | } | |
| 243 | + | } | |
| 238 | 244 | for _, key := range keys { | |
| 239 | - | for idx, candidate := range m.rows { | |
| 240 | - | if m.eligible(idx) && candidate.key() == key { | |
| 241 | - | m.selected = idx | |
| 242 | - | return | |
| 243 | - | } | |
| 245 | + | if idx, ok := index[key]; ok { | |
| 246 | + | m.selected = idx | |
| 247 | + | return | |
| 244 | 248 | } | |
| 245 | 249 | } | |
| 246 | 250 | } | |
| @@ -572,11 +576,11 @@ func lineStatusBadge(status protocol.PickerLineStatus) string { | |||
| 572 | 576 | return statusDot | |
| 573 | 577 | } | |
| 574 | 578 | ||
| 575 | - | // renderList draws each visible row as up to three segments: a name segment | |
| 576 | - | // (bold when styles came from a truecolor theme), a base-styled attention | |
| 577 | - | // marker right after the name, and a muted detail segment — or a base-styled | |
| 578 | - | // status badge for header rows. A tight width ellipsizes the detail segment | |
| 579 | - | // before eating into the name. | |
| 579 | + | // renderList draws each visible row: a dimmed tree prefix, the name (bold on | |
| 580 | + | // unselected live session and host headers), the attention marker, then for | |
| 581 | + | // headers a right-aligned muted detail (tab count, "*" when attached) and a | |
| 582 | + | // status dot in a fixed xterm-256 color. A tight width truncates the detail | |
| 583 | + | // before the name. | |
| 580 | 584 | func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles RenderStyles) { | |
| 581 | 585 | if m == nil || rect.Width <= 0 || rect.Height <= 0 { | |
| 582 | 586 | return | |
| @@ -617,7 +621,7 @@ func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles Render | |||
| 617 | 621 | if r.tree != "" { | |
| 618 | 622 | treeStyle := base | |
| 619 | 623 | treeStyle.Attrs |= renderer.AttrDim | |
| 620 | - | treeX = ui.DrawText(frame, rect.X, rect.Y+y, clipX, r.tree, treeStyle) | |
| 624 | + | treeX = ui.DrawText(frame, rect.X, rect.Y+y, max(rect.X, clipX-2), r.tree, treeStyle) | |
| 621 | 625 | } | |
| 622 | 626 | badge := "" | |
| 623 | 627 | if r.rendersAsHeader() { | |
| @@ -658,6 +662,8 @@ func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles Render | |||
| 658 | 662 | if badge != "" { | |
| 659 | 663 | badgeX := max(rect.X, clipX-badgeWidth) | |
| 660 | 664 | badgeStyle := base | |
| 665 | + | // The dot keeps its own color even on the inverse selected row. | |
| 666 | + | badgeStyle.Inverse = false | |
| 661 | 667 | badgeStyle.Foreground = lineStatusColor(r.line.Status) | |
| 662 | 668 | badgeStyle.HasForegroundRGB = false | |
| 663 | 669 | ui.DrawText(frame, badgeX, rect.Y+y, clipX, badge, badgeStyle) | |
| @@ -174,7 +174,7 @@ func (m *Model) Intent() protocol.PickerIntent { | |||||
| 174 | // SortMode reports the local ordering mode. | 174 | // SortMode reports the local ordering mode. | ||
| 175 | func (m *Model) SortMode() SortMode { | 175 | func (m *Model) SortMode() SortMode { | ||
| 176 | if m == nil { | 176 | if m == nil { | ||
| 177 | - | return SortRecent | 177 | + | return SortGrouped |
| 178 | } | 178 | } | ||
| 179 | return m.sort | 179 | return m.sort | ||
| 180 | } | 180 | } | ||
| @@ -235,12 +235,16 @@ func (m *Model) keysAboveCursor() []string { | |||||
| 235 | 235 | ||||
| 236 | // selectFirstKey moves the cursor to the first key still present, if any. | 236 | // selectFirstKey moves the cursor to the first key still present, if any. | ||
| 237 | func (m *Model) selectFirstKey(keys []string) { | 237 | func (m *Model) selectFirstKey(keys []string) { | ||
| 238 | + | index := make(map[string]int, len(m.rows)) | |||
| 239 | + | for idx, candidate := range m.rows { | |||
| 240 | + | if m.eligible(idx) { | |||
| 241 | + | index[candidate.key()] = idx | |||
| 242 | + | } | |||
| 243 | + | } | |||
| 238 | for _, key := range keys { | 244 | for _, key := range keys { | ||
| 239 | - | for idx, candidate := range m.rows { | 245 | + | if idx, ok := index[key]; ok { |
| 240 | - | if m.eligible(idx) && candidate.key() == key { | 246 | + | m.selected = idx |
| 241 | - | m.selected = idx | 247 | + | return |
| 242 | - | return | |||
| 243 | - | } | |||
| 244 | } | 248 | } | ||
| 245 | } | 249 | } | ||
| 246 | } | 250 | } | ||
| @@ -572,11 +576,11 @@ func lineStatusBadge(status protocol.PickerLineStatus) string { | |||||
| 572 | return statusDot | 576 | return statusDot | ||
| 573 | } | 577 | } | ||
| 574 | 578 | ||||
| 575 | - | // renderList draws each visible row as up to three segments: a name segment | 579 | + | // renderList draws each visible row: a dimmed tree prefix, the name (bold on |
| 576 | - | // (bold when styles came from a truecolor theme), a base-styled attention | 580 | + | // unselected live session and host headers), the attention marker, then for |
| 577 | - | // marker right after the name, and a muted detail segment — or a base-styled | 581 | + | // headers a right-aligned muted detail (tab count, "*" when attached) and a |
| 578 | - | // status badge for header rows. A tight width ellipsizes the detail segment | 582 | + | // status dot in a fixed xterm-256 color. A tight width truncates the detail |
| 579 | - | // before eating into the name. | 583 | + | // before the name. |
| 580 | func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles RenderStyles) { | 584 | func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles RenderStyles) { | ||
| 581 | if m == nil || rect.Width <= 0 || rect.Height <= 0 { | 585 | if m == nil || rect.Width <= 0 || rect.Height <= 0 { | ||
| 582 | return | 586 | return | ||
| @@ -617,7 +621,7 @@ func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles Render | |||||
| 617 | if r.tree != "" { | 621 | if r.tree != "" { | ||
| 618 | treeStyle := base | 622 | treeStyle := base | ||
| 619 | treeStyle.Attrs |= renderer.AttrDim | 623 | treeStyle.Attrs |= renderer.AttrDim | ||
| 620 | - | treeX = ui.DrawText(frame, rect.X, rect.Y+y, clipX, r.tree, treeStyle) | 624 | + | treeX = ui.DrawText(frame, rect.X, rect.Y+y, max(rect.X, clipX-2), r.tree, treeStyle) |
| 621 | } | 625 | } | ||
| 622 | badge := "" | 626 | badge := "" | ||
| 623 | if r.rendersAsHeader() { | 627 | if r.rendersAsHeader() { | ||
| @@ -658,6 +662,8 @@ func (m *Model) renderList(frame renderer.Frame, rect domain.Rect, styles Render | |||||
| 658 | if badge != "" { | 662 | if badge != "" { | ||
| 659 | badgeX := max(rect.X, clipX-badgeWidth) | 663 | badgeX := max(rect.X, clipX-badgeWidth) | ||
| 660 | badgeStyle := base | 664 | badgeStyle := base | ||
| 665 | + | // The dot keeps its own color even on the inverse selected row. | |||
| 666 | + | badgeStyle.Inverse = false | |||
| 661 | badgeStyle.Foreground = lineStatusColor(r.line.Status) | 667 | badgeStyle.Foreground = lineStatusColor(r.line.Status) | ||
| 662 | badgeStyle.HasForegroundRGB = false | 668 | badgeStyle.HasForegroundRGB = false | ||
| 663 | ui.DrawText(frame, badgeX, rect.Y+y, clipX, badge, badgeStyle) | 669 | ui.DrawText(frame, badgeX, rect.Y+y, clipX, badge, badgeStyle) | ||
M internal/usecase/picker/model_test.go +22 -1
| @@ -302,8 +302,11 @@ func TestRenderDrawsStatusBadgesAndStoppedRows(t *testing.T) { | |||
| 302 | 302 | for row, want := range map[int]int{0: 71, 1: 244} { | |
| 303 | 303 | text := []rune(strings.TrimRight(rowText(frame.Row(row)), " ")) | |
| 304 | 304 | require.Equal(t, '●', text[len(text)-1]) | |
| 305 | - | require.Equal(t, want, frame.At(len(text)-1, row).Style.Foreground, "row %d dot color", row) | |
| 305 | + | dot := frame.At(len(text)-1, row).Style | |
| 306 | + | require.Equal(t, want, dot.Foreground, "row %d dot color", row) | |
| 307 | + | require.False(t, dot.Inverse, "row %d dot is never inverted, even when selected", row) | |
| 306 | 308 | } | |
| 309 | + | require.True(t, frame.At(0, 0).Style.Inverse, "row 0 is the selected row") | |
| 307 | 310 | } | |
| 308 | 311 | ||
| 309 | 312 | func TestRenderShowsNoDaemonBadgeAndCreateHint(t *testing.T) { | |
| @@ -488,3 +491,21 @@ func TestTreePrefixesBranchSessionsAndTabs(t *testing.T) { | |||
| 488 | 491 | require.Equal(t, '├', cell.Rune) | |
| 489 | 492 | require.NotZero(t, cell.Style.Attrs&renderer.AttrDim, "tree lines are dimmed") | |
| 490 | 493 | } | |
| 494 | + | ||
| 495 | + | func TestReplaceLinesPreviousRowRespectsSearch(t *testing.T) { | |
| 496 | + | m := New([]protocol.PickerLine{navLine("a", "alpha"), navLine("b", "beta"), navLine("c", "alps")}, Config{Cursor: protocol.PickerCursor{Key: "c", Index: -1}}) | |
| 497 | + | m.EnterSearch() | |
| 498 | + | for _, r := range "al" { | |
| 499 | + | m.InsertSearch(r) | |
| 500 | + | } | |
| 501 | + | m.Down() // alpha -> alps: beta is hidden by the query | |
| 502 | + | selected, ok := m.Selected() | |
| 503 | + | require.True(t, ok) | |
| 504 | + | require.Equal(t, "c", selected.Key) | |
| 505 | + | ||
| 506 | + | m.ReplaceLines([]protocol.PickerLine{navLine("a", "alpha"), navLine("b", "beta")}, protocol.PickerCursor{Index: -1}) | |
| 507 | + | ||
| 508 | + | selected, ok = m.Selected() | |
| 509 | + | require.True(t, ok) | |
| 510 | + | require.Equal(t, "a", selected.Key, "the hidden row above is skipped for the nearest matching one") | |
| 511 | + | } | |
| @@ -302,8 +302,11 @@ func TestRenderDrawsStatusBadgesAndStoppedRows(t *testing.T) { | |||||
| 302 | for row, want := range map[int]int{0: 71, 1: 244} { | 302 | for row, want := range map[int]int{0: 71, 1: 244} { | ||
| 303 | text := []rune(strings.TrimRight(rowText(frame.Row(row)), " ")) | 303 | text := []rune(strings.TrimRight(rowText(frame.Row(row)), " ")) | ||
| 304 | require.Equal(t, '●', text[len(text)-1]) | 304 | require.Equal(t, '●', text[len(text)-1]) | ||
| 305 | - | require.Equal(t, want, frame.At(len(text)-1, row).Style.Foreground, "row %d dot color", row) | 305 | + | dot := frame.At(len(text)-1, row).Style |
| 306 | + | require.Equal(t, want, dot.Foreground, "row %d dot color", row) | |||
| 307 | + | require.False(t, dot.Inverse, "row %d dot is never inverted, even when selected", row) | |||
| 306 | } | 308 | } | ||
| 309 | + | require.True(t, frame.At(0, 0).Style.Inverse, "row 0 is the selected row") | |||
| 307 | } | 310 | } | ||
| 308 | 311 | ||||
| 309 | func TestRenderShowsNoDaemonBadgeAndCreateHint(t *testing.T) { | 312 | func TestRenderShowsNoDaemonBadgeAndCreateHint(t *testing.T) { | ||
| @@ -488,3 +491,21 @@ func TestTreePrefixesBranchSessionsAndTabs(t *testing.T) { | |||||
| 488 | require.Equal(t, '├', cell.Rune) | 491 | require.Equal(t, '├', cell.Rune) | ||
| 489 | require.NotZero(t, cell.Style.Attrs&renderer.AttrDim, "tree lines are dimmed") | 492 | require.NotZero(t, cell.Style.Attrs&renderer.AttrDim, "tree lines are dimmed") | ||
| 490 | } | 493 | } | ||
| 494 | + | ||||
| 495 | + | func TestReplaceLinesPreviousRowRespectsSearch(t *testing.T) { | |||
| 496 | + | m := New([]protocol.PickerLine{navLine("a", "alpha"), navLine("b", "beta"), navLine("c", "alps")}, Config{Cursor: protocol.PickerCursor{Key: "c", Index: -1}}) | |||
| 497 | + | m.EnterSearch() | |||
| 498 | + | for _, r := range "al" { | |||
| 499 | + | m.InsertSearch(r) | |||
| 500 | + | } | |||
| 501 | + | m.Down() // alpha -> alps: beta is hidden by the query | |||
| 502 | + | selected, ok := m.Selected() | |||
| 503 | + | require.True(t, ok) | |||
| 504 | + | require.Equal(t, "c", selected.Key) | |||
| 505 | + | ||||
| 506 | + | m.ReplaceLines([]protocol.PickerLine{navLine("a", "alpha"), navLine("b", "beta")}, protocol.PickerCursor{Index: -1}) | |||
| 507 | + | ||||
| 508 | + | selected, ok = m.Selected() | |||
| 509 | + | require.True(t, ok) | |||
| 510 | + | require.Equal(t, "a", selected.Key, "the hidden row above is skipped for the nearest matching one") | |||
| 511 | + | } | |||