Merge pull request #6 from bnema/feature/recent-command-palette
feat(palette): order commands by recent use
3 files · +108 -1
M internal/usecase/daemon/daemon.go +3 -0
| @@ -88,6 +88,9 @@ type Daemon struct { | |||
| 88 | 88 | animFrame int | |
| 89 | 89 | animWake chan struct{} | |
| 90 | 90 | ||
| 91 | + | paletteRecentMu sync.Mutex | |
| 92 | + | paletteRecent []string | |
| 93 | + | ||
| 91 | 94 | ptys ports.PTYFactory | |
| 92 | 95 | clock ports.Clock | |
| 93 | 96 | log *slog.Logger | |
| @@ -88,6 +88,9 @@ type Daemon struct { | |||||
| 88 | animFrame int | 88 | animFrame int | ||
| 89 | animWake chan struct{} | 89 | animWake chan struct{} | ||
| 90 | 90 | ||||
| 91 | + | paletteRecentMu sync.Mutex | |||
| 92 | + | paletteRecent []string | |||
| 93 | + | ||||
| 91 | ptys ports.PTYFactory | 94 | ptys ports.PTYFactory | ||
| 92 | clock ports.Clock | 95 | clock ports.Clock | ||
| 93 | log *slog.Logger | 96 | log *slog.Logger | ||
M internal/usecase/daemon/palette.go +45 -1
| @@ -13,7 +13,7 @@ var paletteModal = ui.Modal{WidthPct: 100, MinWidth: 32, FixedHeight: 11, Title: | |||
| 13 | 13 | func (d *Daemon) enterPalette(sess *session, ac *attachedClient) { | |
| 14 | 14 | d.closePalette(ac) | |
| 15 | 15 | ac.paletteMu.Lock() | |
| 16 | - | ac.palette = palette.NewRegistry() | |
| 16 | + | ac.palette = palette.New(d.paletteCommands()) | |
| 17 | 17 | ac.palettePending = nil | |
| 18 | 18 | ac.paletteMu.Unlock() | |
| 19 | 19 | d.paint(sess, ac, true) | |
| @@ -26,6 +26,48 @@ func (d *Daemon) closePalette(ac *attachedClient) { | |||
| 26 | 26 | ac.paletteMu.Unlock() | |
| 27 | 27 | } | |
| 28 | 28 | ||
| 29 | + | func (d *Daemon) paletteCommands() []command.Command { | |
| 30 | + | commands := command.Registry() | |
| 31 | + | d.paletteRecentMu.Lock() | |
| 32 | + | recent := append([]string(nil), d.paletteRecent...) | |
| 33 | + | d.paletteRecentMu.Unlock() | |
| 34 | + | ||
| 35 | + | byCode := make(map[string]command.Command, len(commands)) | |
| 36 | + | for _, cmd := range commands { | |
| 37 | + | byCode[cmd.Code] = cmd | |
| 38 | + | } | |
| 39 | + | out := make([]command.Command, 0, len(commands)) | |
| 40 | + | used := make(map[string]bool, len(recent)) | |
| 41 | + | for _, code := range recent { | |
| 42 | + | cmd, ok := byCode[code] | |
| 43 | + | if !ok || used[code] { | |
| 44 | + | continue | |
| 45 | + | } | |
| 46 | + | out = append(out, cmd) | |
| 47 | + | used[code] = true | |
| 48 | + | } | |
| 49 | + | for _, cmd := range commands { | |
| 50 | + | if !used[cmd.Code] { | |
| 51 | + | out = append(out, cmd) | |
| 52 | + | } | |
| 53 | + | } | |
| 54 | + | return out | |
| 55 | + | } | |
| 56 | + | ||
| 57 | + | func (d *Daemon) recordPaletteUse(code string) { | |
| 58 | + | d.paletteRecentMu.Lock() | |
| 59 | + | defer d.paletteRecentMu.Unlock() | |
| 60 | + | ||
| 61 | + | recent := make([]string, 0, len(d.paletteRecent)+1) | |
| 62 | + | recent = append(recent, code) | |
| 63 | + | for _, existing := range d.paletteRecent { | |
| 64 | + | if existing != code { | |
| 65 | + | recent = append(recent, existing) | |
| 66 | + | } | |
| 67 | + | } | |
| 68 | + | d.paletteRecent = recent | |
| 69 | + | } | |
| 70 | + | ||
| 29 | 71 | func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte) { | |
| 30 | 72 | sess := ac.currentSession() | |
| 31 | 73 | if sess == nil { | |
| @@ -82,6 +124,8 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte) { | |||
| 82 | 124 | name := sess.name | |
| 83 | 125 | sess.mu.Unlock() | |
| 84 | 126 | d.log.Error("palette command failed", "err", err, "session", name, "command", cmd.Code) | |
| 127 | + | } else { | |
| 128 | + | d.recordPaletteUse(cmd.Code) | |
| 85 | 129 | } | |
| 86 | 130 | return | |
| 87 | 131 | } | |
| @@ -13,7 +13,7 @@ var paletteModal = ui.Modal{WidthPct: 100, MinWidth: 32, FixedHeight: 11, Title: | |||||
| 13 | func (d *Daemon) enterPalette(sess *session, ac *attachedClient) { | 13 | func (d *Daemon) enterPalette(sess *session, ac *attachedClient) { | ||
| 14 | d.closePalette(ac) | 14 | d.closePalette(ac) | ||
| 15 | ac.paletteMu.Lock() | 15 | ac.paletteMu.Lock() | ||
| 16 | - | ac.palette = palette.NewRegistry() | 16 | + | ac.palette = palette.New(d.paletteCommands()) |
| 17 | ac.palettePending = nil | 17 | ac.palettePending = nil | ||
| 18 | ac.paletteMu.Unlock() | 18 | ac.paletteMu.Unlock() | ||
| 19 | d.paint(sess, ac, true) | 19 | d.paint(sess, ac, true) | ||
| @@ -26,6 +26,48 @@ func (d *Daemon) closePalette(ac *attachedClient) { | |||||
| 26 | ac.paletteMu.Unlock() | 26 | ac.paletteMu.Unlock() | ||
| 27 | } | 27 | } | ||
| 28 | 28 | ||||
| 29 | + | func (d *Daemon) paletteCommands() []command.Command { | |||
| 30 | + | commands := command.Registry() | |||
| 31 | + | d.paletteRecentMu.Lock() | |||
| 32 | + | recent := append([]string(nil), d.paletteRecent...) | |||
| 33 | + | d.paletteRecentMu.Unlock() | |||
| 34 | + | ||||
| 35 | + | byCode := make(map[string]command.Command, len(commands)) | |||
| 36 | + | for _, cmd := range commands { | |||
| 37 | + | byCode[cmd.Code] = cmd | |||
| 38 | + | } | |||
| 39 | + | out := make([]command.Command, 0, len(commands)) | |||
| 40 | + | used := make(map[string]bool, len(recent)) | |||
| 41 | + | for _, code := range recent { | |||
| 42 | + | cmd, ok := byCode[code] | |||
| 43 | + | if !ok || used[code] { | |||
| 44 | + | continue | |||
| 45 | + | } | |||
| 46 | + | out = append(out, cmd) | |||
| 47 | + | used[code] = true | |||
| 48 | + | } | |||
| 49 | + | for _, cmd := range commands { | |||
| 50 | + | if !used[cmd.Code] { | |||
| 51 | + | out = append(out, cmd) | |||
| 52 | + | } | |||
| 53 | + | } | |||
| 54 | + | return out | |||
| 55 | + | } | |||
| 56 | + | ||||
| 57 | + | func (d *Daemon) recordPaletteUse(code string) { | |||
| 58 | + | d.paletteRecentMu.Lock() | |||
| 59 | + | defer d.paletteRecentMu.Unlock() | |||
| 60 | + | ||||
| 61 | + | recent := make([]string, 0, len(d.paletteRecent)+1) | |||
| 62 | + | recent = append(recent, code) | |||
| 63 | + | for _, existing := range d.paletteRecent { | |||
| 64 | + | if existing != code { | |||
| 65 | + | recent = append(recent, existing) | |||
| 66 | + | } | |||
| 67 | + | } | |||
| 68 | + | d.paletteRecent = recent | |||
| 69 | + | } | |||
| 70 | + | ||||
| 29 | func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte) { | 71 | func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte) { | ||
| 30 | sess := ac.currentSession() | 72 | sess := ac.currentSession() | ||
| 31 | if sess == nil { | 73 | if sess == nil { | ||
| @@ -82,6 +124,8 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte) { | |||||
| 82 | name := sess.name | 124 | name := sess.name | ||
| 83 | sess.mu.Unlock() | 125 | sess.mu.Unlock() | ||
| 84 | d.log.Error("palette command failed", "err", err, "session", name, "command", cmd.Code) | 126 | d.log.Error("palette command failed", "err", err, "session", name, "command", cmd.Code) | ||
| 127 | + | } else { | |||
| 128 | + | d.recordPaletteUse(cmd.Code) | |||
| 85 | } | 129 | } | ||
| 86 | return | 130 | return | ||
| 87 | } | 131 | } | ||
M internal/usecase/daemon/palette_test.go +60 -0
| @@ -81,6 +81,66 @@ func TestPaletteCNSPromptsForSessionNameThenCreatesAndSwitches(t *testing.T) { | |||
| 81 | 81 | require.NotContains(t, string(finalOutput.Data), "Create session") | |
| 82 | 82 | } | |
| 83 | 83 | ||
| 84 | + | func TestPaletteReopensWithSuccessfulCommandFirst(t *testing.T) { | |
| 85 | + | d, sess, ac, sends, releases := newManualTabSession(t, 2) | |
| 86 | + | defer releases[0]() | |
| 87 | + | defer releases[1]() | |
| 88 | + | ||
| 89 | + | d.handleInput(sess, ac, []byte("\x1b ")) | |
| 90 | + | awaitFrame(t, sends, ports.MsgOutput) | |
| 91 | + | d.handleInput(sess, ac, []byte("NXT\r")) | |
| 92 | + | require.False(t, ac.paletteActive()) | |
| 93 | + | awaitFrame(t, sends, ports.MsgOutput) | |
| 94 | + | ||
| 95 | + | d.handleInput(sess, ac, []byte("\x1b ")) | |
| 96 | + | awaitFrame(t, sends, ports.MsgOutput) | |
| 97 | + | cmd, ok := ac.palette.Selected() | |
| 98 | + | require.True(t, ok) | |
| 99 | + | require.Equal(t, "NXT", cmd.Code) | |
| 100 | + | } | |
| 101 | + | ||
| 102 | + | func TestPaletteRecentCommandsNewestFirstThenRegistryOrder(t *testing.T) { | |
| 103 | + | d := &Daemon{} | |
| 104 | + | d.recordPaletteUse("SSP") | |
| 105 | + | d.recordPaletteUse("NXT") | |
| 106 | + | // STALE is not a registered command code; it must be dropped from output. | |
| 107 | + | d.recordPaletteUse("STALE") | |
| 108 | + | d.recordPaletteUse("SSP") | |
| 109 | + | ||
| 110 | + | commands := d.paletteCommands() | |
| 111 | + | codes := make([]string, len(commands)) | |
| 112 | + | for i, cmd := range commands { | |
| 113 | + | codes[i] = cmd.Code | |
| 114 | + | } | |
| 115 | + | require.Equal(t, []string{"SSP", "NXT", "CNT", "CNS", "CLT", "PVT", "VIS", "RNS", "DET"}, codes) | |
| 116 | + | } | |
| 117 | + | ||
| 118 | + | func TestPaletteRecencyCanBeUpdatedConcurrently(t *testing.T) { | |
| 119 | + | d := &Daemon{} | |
| 120 | + | codes := []string{"CNT", "CNS", "CLT", "NXT", "PVT", "SSP", "VIS", "RNS", "DET"} | |
| 121 | + | ||
| 122 | + | var wg sync.WaitGroup | |
| 123 | + | for range 50 { | |
| 124 | + | for _, code := range codes { | |
| 125 | + | wg.Add(1) | |
| 126 | + | go func(code string) { | |
| 127 | + | defer wg.Done() | |
| 128 | + | d.recordPaletteUse(code) | |
| 129 | + | _ = d.paletteCommands() | |
| 130 | + | }(code) | |
| 131 | + | } | |
| 132 | + | } | |
| 133 | + | wg.Wait() | |
| 134 | + | ||
| 135 | + | commands := d.paletteCommands() | |
| 136 | + | require.Len(t, commands, len(codes)) | |
| 137 | + | seen := map[string]bool{} | |
| 138 | + | for _, cmd := range commands { | |
| 139 | + | require.False(t, seen[cmd.Code], "duplicate command %s", cmd.Code) | |
| 140 | + | seen[cmd.Code] = true | |
| 141 | + | } | |
| 142 | + | } | |
| 143 | + | ||
| 84 | 144 | func TestPaletteCommandNoopRepaintsAfterClose(t *testing.T) { | |
| 85 | 145 | d, sess, ac, sends, releases := newManualTabSession(t, 1) | |
| 86 | 146 | defer releases[0]() | |
| @@ -81,6 +81,66 @@ func TestPaletteCNSPromptsForSessionNameThenCreatesAndSwitches(t *testing.T) { | |||||
| 81 | require.NotContains(t, string(finalOutput.Data), "Create session") | 81 | require.NotContains(t, string(finalOutput.Data), "Create session") | ||
| 82 | } | 82 | } | ||
| 83 | 83 | ||||
| 84 | + | func TestPaletteReopensWithSuccessfulCommandFirst(t *testing.T) { | |||
| 85 | + | d, sess, ac, sends, releases := newManualTabSession(t, 2) | |||
| 86 | + | defer releases[0]() | |||
| 87 | + | defer releases[1]() | |||
| 88 | + | ||||
| 89 | + | d.handleInput(sess, ac, []byte("\x1b ")) | |||
| 90 | + | awaitFrame(t, sends, ports.MsgOutput) | |||
| 91 | + | d.handleInput(sess, ac, []byte("NXT\r")) | |||
| 92 | + | require.False(t, ac.paletteActive()) | |||
| 93 | + | awaitFrame(t, sends, ports.MsgOutput) | |||
| 94 | + | ||||
| 95 | + | d.handleInput(sess, ac, []byte("\x1b ")) | |||
| 96 | + | awaitFrame(t, sends, ports.MsgOutput) | |||
| 97 | + | cmd, ok := ac.palette.Selected() | |||
| 98 | + | require.True(t, ok) | |||
| 99 | + | require.Equal(t, "NXT", cmd.Code) | |||
| 100 | + | } | |||
| 101 | + | ||||
| 102 | + | func TestPaletteRecentCommandsNewestFirstThenRegistryOrder(t *testing.T) { | |||
| 103 | + | d := &Daemon{} | |||
| 104 | + | d.recordPaletteUse("SSP") | |||
| 105 | + | d.recordPaletteUse("NXT") | |||
| 106 | + | // STALE is not a registered command code; it must be dropped from output. | |||
| 107 | + | d.recordPaletteUse("STALE") | |||
| 108 | + | d.recordPaletteUse("SSP") | |||
| 109 | + | ||||
| 110 | + | commands := d.paletteCommands() | |||
| 111 | + | codes := make([]string, len(commands)) | |||
| 112 | + | for i, cmd := range commands { | |||
| 113 | + | codes[i] = cmd.Code | |||
| 114 | + | } | |||
| 115 | + | require.Equal(t, []string{"SSP", "NXT", "CNT", "CNS", "CLT", "PVT", "VIS", "RNS", "DET"}, codes) | |||
| 116 | + | } | |||
| 117 | + | ||||
| 118 | + | func TestPaletteRecencyCanBeUpdatedConcurrently(t *testing.T) { | |||
| 119 | + | d := &Daemon{} | |||
| 120 | + | codes := []string{"CNT", "CNS", "CLT", "NXT", "PVT", "SSP", "VIS", "RNS", "DET"} | |||
| 121 | + | ||||
| 122 | + | var wg sync.WaitGroup | |||
| 123 | + | for range 50 { | |||
| 124 | + | for _, code := range codes { | |||
| 125 | + | wg.Add(1) | |||
| 126 | + | go func(code string) { | |||
| 127 | + | defer wg.Done() | |||
| 128 | + | d.recordPaletteUse(code) | |||
| 129 | + | _ = d.paletteCommands() | |||
| 130 | + | }(code) | |||
| 131 | + | } | |||
| 132 | + | } | |||
| 133 | + | wg.Wait() | |||
| 134 | + | ||||
| 135 | + | commands := d.paletteCommands() | |||
| 136 | + | require.Len(t, commands, len(codes)) | |||
| 137 | + | seen := map[string]bool{} | |||
| 138 | + | for _, cmd := range commands { | |||
| 139 | + | require.False(t, seen[cmd.Code], "duplicate command %s", cmd.Code) | |||
| 140 | + | seen[cmd.Code] = true | |||
| 141 | + | } | |||
| 142 | + | } | |||
| 143 | + | ||||
| 84 | func TestPaletteCommandNoopRepaintsAfterClose(t *testing.T) { | 144 | func TestPaletteCommandNoopRepaintsAfterClose(t *testing.T) { | ||
| 85 | d, sess, ac, sends, releases := newManualTabSession(t, 1) | 145 | d, sess, ac, sends, releases := newManualTabSession(t, 1) | ||
| 86 | defer releases[0]() | 146 | defer releases[0]() | ||