3a57f9d fix(daemon): address attention review findings · vev

M internal/usecase/daemon/attention.go +2 -14


@@ -55,20 +55,8 @@ func (d Daemon) jumpAttention(sess session, ac *attachedClient) {
55	55		}
56	56
57	57		func oldestAttentionTab(sess *session) (int, bool) {
58		-	sess.mu.Lock()
59		-	defer sess.mu.Unlock()
60		-	idx := -1
61		-	var oldest time.Time
62		-	for i, tb := range sess.tabs {
63		-	if !tb.attention {
64		-	continue
65		-	}
66		-	if idx == -1 \|\| tb.attentionAt.Before(oldest) {
67		-	idx = i
68		-	oldest = tb.attentionAt
69		-	}
70		-	}
71		-	return idx, idx != -1
	58	+	idx, _, ok := oldestAttentionTabWithTime(sess)
	59	+	return idx, ok
72	60		}
73	61
74	62		type attentionTarget struct {


@@ -55,20 +55,8 @@ func (d Daemon) jumpAttention(sess session, ac *attachedClient) {
55		}	55		}
56			56
57		func oldestAttentionTab(sess *session) (int, bool) {	57		func oldestAttentionTab(sess *session) (int, bool) {
58	-	sess.mu.Lock()	58	+	idx, _, ok := oldestAttentionTabWithTime(sess)
59	-	defer sess.mu.Unlock()	59	+	return idx, ok
60	-	idx := -1
61	-	var oldest time.Time
62	-	for i, tb := range sess.tabs {
63	-	if !tb.attention {
64	-	continue
65	-	}
66	-	if idx == -1 \|\| tb.attentionAt.Before(oldest) {
67	-	idx = i
68	-	oldest = tb.attentionAt
69	-	}
70	-	}
71	-	return idx, idx != -1
72		}	60		}
73			61
74		type attentionTarget struct {	62		type attentionTarget struct {

M internal/usecase/daemon/copymode.go +10 -2


@@ -307,8 +307,16 @@ func isCopyEscapePrefix(data []byte) bool {
307	307		len(data) == 3 && data[0] == 0x1b && data[1] == '[' && (data[2] == '5' \|\| data[2] == '6')
308	308		}
309	309
310		-	func composeCopyClientFrame(mode scopy.Mode, tb tab, copyStatusStyle, selectionStyle renderer.Style) (renderer.Frame, []renderer.Damage) {
	310	+	func composeCopyClientFrame(mode scopy.Mode, tb tab, bars barState) (renderer.Frame, []renderer.Damage) {
	311	+	styles := newThemeStyles(bars.theme)
311	312		snap := scopy.NewSnapshot(tb.scrollback, tb.screen.Frame)
312		-	frame := mode.Render(snap, copyStatusStyle, selectionStyle)
	313	+	copyFrame := mode.Render(snap, styles.copyStatus, styles.selection)
	314	+	width, screenRows := tb.screen.Frame.Width, tb.screen.Frame.Height
	315	+	frame := renderer.NewFrame(width, screenRows+2)
	316	+	drawTopBarSnapshot(frame.Row(0), bars.status, bars.attentionFrame, styles)
	317	+	for y := range screenRows {
	318	+	copy(frame.Row(y+1), copyFrame.Row(y))
	319	+	}
	320	+	copy(frame.Row(screenRows+1), copyFrame.Row(screenRows))
313	321		return frame, []renderer.Damage{renderer.FullRedraw()}
314	322		}

 		len(data) == 3 && data[0] == 0x1b && data[1] == '[' && (data[2] == '5' || data[2] == '6')
 }
-func composeCopyClientFrame(mode *scopy.Mode, tb *tab, copyStatusStyle, selectionStyle renderer.Style) (renderer.Frame, []renderer.Damage) {
+func composeCopyClientFrame(mode *scopy.Mode, tb *tab, bars barState) (renderer.Frame, []renderer.Damage) {
+	styles := newThemeStyles(bars.theme)
 	snap := scopy.NewSnapshot(tb.scrollback, tb.screen.Frame)
-	frame := mode.Render(snap, copyStatusStyle, selectionStyle)
+	copyFrame := mode.Render(snap, styles.copyStatus, styles.selection)
+	width, screenRows := tb.screen.Frame.Width, tb.screen.Frame.Height
+	frame := renderer.NewFrame(width, screenRows+2)
+	drawTopBarSnapshot(frame.Row(0), bars.status, bars.attentionFrame, styles)
+	for y := range screenRows {
+		copy(frame.Row(y+1), copyFrame.Row(y))
+	}
+	copy(frame.Row(screenRows+1), copyFrame.Row(screenRows))
 	return frame, []renderer.Damage{renderer.FullRedraw()}
 }

M internal/usecase/daemon/copymode_test.go +21 -0


@@ -18,6 +18,7 @@ import (
18	18		portsmocks "github.com/bnema/vev/internal/ports/mocks"
19	19		scopy "github.com/bnema/vev/internal/usecase/copy"
20	20		"github.com/bnema/vev/pkg/renderer"
	21	+	"github.com/bnema/vev/pkg/vt"
21	22		)
22	23
23	24		// --- test doubles -----------------------------------------------------------

@@ -25,6 +26,26 @@ import (
25	26		// stubClock returns timers whose channel never fires, so a scheduler under it
26	27		// blocks in its debounce loop until the session context is cancelled. Used by
27	28
	29	+	func TestCopyModeFrameIncludesTopAndBottomChrome(t *testing.T) {
	30	+	p, release := newBlockingPTY(t)
	31	+	_, sess, _, _ := newManualSessionWithPTYs(t, p)
	32	+	defer release()
	33	+	sess.name = "work"
	34	+	tb := sess.activeTab()
	35	+	tb.screen = vt.NewScreen(12, 3)
	36	+	tb.screen.Write([]byte("live"))
	37	+	snap := scopy.NewSnapshot(tb.scrollback, tb.screen.Frame)
	38	+	mode := scopy.NewMode(snap)
	39	+
	40	+	frame, damage := composeCopyClientFrame(mode, tb, barState{status: sess.statusSegments()})
	41	+
	42	+	require.Equal(t, 12, frame.Width)
	43	+	require.Equal(t, 5, frame.Height)
	44	+	require.Equal(t, " 1 ", rowText(frame.Row(0)))
	45	+	require.Contains(t, rowText(frame.Row(4)), "[VISUAL]")
	46	+	require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, damage)
	47	+	}
	48	+
28	49		func TestCopyModePaletteCommandEntersAndDoesNotForward(t *testing.T) {
29	50		writes := make(chan []byte, 1)
30	51		p, _ := newBlockingPTYWithWrites(t, writes)


@@ -18,6 +18,7 @@ import (
18	portsmocks "github.com/bnema/vev/internal/ports/mocks"	18		portsmocks "github.com/bnema/vev/internal/ports/mocks"
19	scopy "github.com/bnema/vev/internal/usecase/copy"	19		scopy "github.com/bnema/vev/internal/usecase/copy"
20	"github.com/bnema/vev/pkg/renderer"	20		"github.com/bnema/vev/pkg/renderer"
		21	+	"github.com/bnema/vev/pkg/vt"
21	)	22		)
22		23
23	// --- test doubles -----------------------------------------------------------	24		// --- test doubles -----------------------------------------------------------

@@ -25,6 +26,26 @@ import (
25	// stubClock returns timers whose channel never fires, so a scheduler under it	26		// stubClock returns timers whose channel never fires, so a scheduler under it
26	// blocks in its debounce loop until the session context is cancelled. Used by	27		// blocks in its debounce loop until the session context is cancelled. Used by
27		28
		29	+	func TestCopyModeFrameIncludesTopAndBottomChrome(t *testing.T) {
		30	+	p, release := newBlockingPTY(t)
		31	+	_, sess, _, _ := newManualSessionWithPTYs(t, p)
		32	+	defer release()
		33	+	sess.name = "work"
		34	+	tb := sess.activeTab()
		35	+	tb.screen = vt.NewScreen(12, 3)
		36	+	tb.screen.Write([]byte("live"))
		37	+	snap := scopy.NewSnapshot(tb.scrollback, tb.screen.Frame)
		38	+	mode := scopy.NewMode(snap)
		39	+
		40	+	frame, damage := composeCopyClientFrame(mode, tb, barState{status: sess.statusSegments()})
		41	+
		42	+	require.Equal(t, 12, frame.Width)
		43	+	require.Equal(t, 5, frame.Height)
		44	+	require.Equal(t, " 1 ", rowText(frame.Row(0)))
		45	+	require.Contains(t, rowText(frame.Row(4)), "[VISUAL]")
		46	+	require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, damage)
		47	+	}
		48	+
28	func TestCopyModePaletteCommandEntersAndDoesNotForward(t *testing.T) {	49		func TestCopyModePaletteCommandEntersAndDoesNotForward(t *testing.T) {
29	writes := make(chan []byte, 1)	50		writes := make(chan []byte, 1)
30	p, _ := newBlockingPTYWithWrites(t, writes)	51		p, _ := newBlockingPTYWithWrites(t, writes)

M internal/usecase/daemon/input_test.go +2 -0


@@ -222,6 +222,8 @@ func TestSGRRowOffset(t *testing.T) {
222	222		{name: "leaves malformed fields unchanged", raw: []byte("\x1b[<0;2M"), delta: -1, want: []byte("\x1b[<0;2M")},
223	223		{name: "leaves non numeric row unchanged", raw: []byte("\x1b[<0;2;xM"), delta: -1, want: []byte("\x1b[<0;2;xM")},
224	224		{name: "leaves invalid shifted row unchanged", raw: []byte("\x1b[<0;2;1M"), delta: -1, want: []byte("\x1b[<0;2;1M")},
	225	+	{name: "handles digit width increase", raw: []byte("\x1b[<0;2;9M"), delta: 1, want: []byte("\x1b[<0;2;10M")},
	226	+	{name: "handles digit width decrease", raw: []byte("\x1b[<0;2;10m"), delta: -1, want: []byte("\x1b[<0;2;9m")},
225	227		}
226	228
227	229		for _, tc := range cases {


@@ -222,6 +222,8 @@ func TestSGRRowOffset(t *testing.T) {
222	{name: "leaves malformed fields unchanged", raw: []byte("\x1b[<0;2M"), delta: -1, want: []byte("\x1b[<0;2M")},	222		{name: "leaves malformed fields unchanged", raw: []byte("\x1b[<0;2M"), delta: -1, want: []byte("\x1b[<0;2M")},
223	{name: "leaves non numeric row unchanged", raw: []byte("\x1b[<0;2;xM"), delta: -1, want: []byte("\x1b[<0;2;xM")},	223		{name: "leaves non numeric row unchanged", raw: []byte("\x1b[<0;2;xM"), delta: -1, want: []byte("\x1b[<0;2;xM")},
224	{name: "leaves invalid shifted row unchanged", raw: []byte("\x1b[<0;2;1M"), delta: -1, want: []byte("\x1b[<0;2;1M")},	224		{name: "leaves invalid shifted row unchanged", raw: []byte("\x1b[<0;2;1M"), delta: -1, want: []byte("\x1b[<0;2;1M")},
		225	+	{name: "handles digit width increase", raw: []byte("\x1b[<0;2;9M"), delta: 1, want: []byte("\x1b[<0;2;10M")},
		226	+	{name: "handles digit width decrease", raw: []byte("\x1b[<0;2;10m"), delta: -1, want: []byte("\x1b[<0;2;9m")},
225	}	227		}
226		228
227	for _, tc := range cases {	229		for _, tc := range cases {

M internal/usecase/daemon/render.go +1 -1


@@ -299,7 +299,7 @@ func (d Daemon) paint(sess session, ac *attachedClient, reset bool) {
299	299		}
300	300		frame, damage := composeClientFrameWithState(bars, tb, reset, &ac.bars)
301	301		if copyActive {
302		-	frame, damage = composeCopyClientFrame(copyMode, tb, styles.copyStatus, styles.selection)
	302	+	frame, damage = composeCopyClientFrame(copyMode, tb, bars)
303	303		}
304	304		if pickerActive {
305	305		if previewTab == tb {


@@ -299,7 +299,7 @@ func (d Daemon) paint(sess session, ac *attachedClient, reset bool) {
299		}	299		}
300		frame, damage := composeClientFrameWithState(bars, tb, reset, &ac.bars)	300		frame, damage := composeClientFrameWithState(bars, tb, reset, &ac.bars)
301		if copyActive {	301		if copyActive {
302	-	frame, damage = composeCopyClientFrame(copyMode, tb, styles.copyStatus, styles.selection)	302	+	frame, damage = composeCopyClientFrame(copyMode, tb, bars)
303		}	303		}
304		if pickerActive {	304		if pickerActive {
305		if previewTab == tb {	305		if previewTab == tb {

M internal/usecase/daemon/status.go +5 -3


@@ -169,16 +169,18 @@ func (d Daemon) barStateFor(cur session, copyFeedback string) barState {
169	169		continue
170	170		}
171	171		sess.mu.Lock()
172		-	oldest := time.Time{}
	172	+	var oldest time.Time
	173	+	found := false
173	174		for _, tb := range sess.tabs {
174	175		if !tb.attention {
175	176		continue
176	177		}
177		-	if oldest.IsZero() \|\| tb.attentionAt.Before(oldest) {
	178	+	if !found \|\| tb.attentionAt.Before(oldest) {
178	179		oldest = tb.attentionAt
	180	+	found = true
179	181		}
180	182		}
181		-	if !oldest.IsZero() {
	183	+	if found {
182	184		attention = append(attention, attentionSession{name: sess.name, at: oldest})
183	185		}
184	186		sess.mu.Unlock()

 			continue
 		}
 		sess.mu.Lock()
-		oldest := time.Time{}
+		var oldest time.Time
+		found := false
 		for _, tb := range sess.tabs {
 			if !tb.attention {
 				continue
 			}
-			if oldest.IsZero() || tb.attentionAt.Before(oldest) {
+			if !found || tb.attentionAt.Before(oldest) {
 				oldest = tb.attentionAt
+				found = true
 			}
 		}
-		if !oldest.IsZero() {
+		if found {
 			attention = append(attention, attentionSession{name: sess.name, at: oldest})
 		}
 		sess.mu.Unlock()

M internal/usecase/daemon/status_test.go +11 -0


@@ -314,6 +314,17 @@ func TestStatusBarRendersOtherAttentionOldestFirstAndCollapses(t *testing.T) {
314	314		require.Equal(t, " current ok", rowText(narrow))
315	315		}
316	316
	317	+	func TestBarStateForIncludesZeroTimeAttention(t *testing.T) {
	318	+	p, releasePTY := newBlockingPTY(t)
	319	+	d, sess, _, _ := newManualSessionWithPTYs(t, p)
	320	+	defer releasePTY()
	321	+	d.sessions["zero"] = &session{id: "zero", name: "zero", tabs: []*tab{{attention: true}}}
	322	+
	323	+	state := d.barStateFor(sess, "")
	324	+
	325	+	require.Equal(t, []string{"zero"}, state.otherAttention)
	326	+	}
	327	+
317	328		func TestBarStateForExcludesCurrentSession(t *testing.T) {
318	329		p, releasePTY := newBlockingPTY(t)
319	330		d, sess, _, _ := newManualSessionWithPTYs(t, p)


@@ -314,6 +314,17 @@ func TestStatusBarRendersOtherAttentionOldestFirstAndCollapses(t *testing.T) {
314	require.Equal(t, " current ok", rowText(narrow))	314		require.Equal(t, " current ok", rowText(narrow))
315	}	315		}
316		316
		317	+	func TestBarStateForIncludesZeroTimeAttention(t *testing.T) {
		318	+	p, releasePTY := newBlockingPTY(t)
		319	+	d, sess, _, _ := newManualSessionWithPTYs(t, p)
		320	+	defer releasePTY()
		321	+	d.sessions["zero"] = &session{id: "zero", name: "zero", tabs: []*tab{{attention: true}}}
		322	+
		323	+	state := d.barStateFor(sess, "")
		324	+
		325	+	require.Equal(t, []string{"zero"}, state.otherAttention)
		326	+	}
		327	+
317	func TestBarStateForExcludesCurrentSession(t *testing.T) {	328		func TestBarStateForExcludesCurrentSession(t *testing.T) {
318	p, releasePTY := newBlockingPTY(t)	329		p, releasePTY := newBlockingPTY(t)
319	d, sess, _, _ := newManualSessionWithPTYs(t, p)	330		d, sess, _, _ := newManualSessionWithPTYs(t, p)

M pkg/vt/screen.go +3 -0


@@ -629,6 +629,9 @@ func (s *Screen) handleOSC(payload []byte) {
629	629		p := string(payload)
630	630		switch {
631	631		case strings.HasPrefix(p, "9;"):
	632	+	if strings.HasPrefix(p, "9;4;") {
	633	+	return
	634	+	}
632	635		s.OnNotify("", p[len("9;"):])
633	636		case strings.HasPrefix(p, "777;"):
634	637		parts := strings.SplitN(p[len("777;"):], ";", 3)


@@ -629,6 +629,9 @@ func (s *Screen) handleOSC(payload []byte) {
629	p := string(payload)	629		p := string(payload)
630	switch {	630		switch {
631	case strings.HasPrefix(p, "9;"):	631		case strings.HasPrefix(p, "9;"):
		632	+	if strings.HasPrefix(p, "9;4;") {
		633	+	return
		634	+	}
632	s.OnNotify("", p[len("9;"):])	635		s.OnNotify("", p[len("9;"):])
633	case strings.HasPrefix(p, "777;"):	636		case strings.HasPrefix(p, "777;"):
634	parts := strings.SplitN(p[len("777;"):], ";", 3)	637		parts := strings.SplitN(p[len("777;"):], ";", 3)

M pkg/vt/screen_test.go +8 -0


@@ -120,6 +120,14 @@ func TestOnNotifyOSC9(t *testing.T) {
120	120		require.Equal(t, "agent done", gotBody)
121	121		}
122	122
	123	+	func TestOnNotifyIgnoresOSC9Progress(t *testing.T) {
	124	+	s := NewScreen(10, 2)
	125	+	calls := 0
	126	+	s.OnNotify = func(string, string) { calls++ }
	127	+	s.Write([]byte("\x1b]9;4;1;50\x07"))
	128	+	require.Equal(t, 0, calls)
	129	+	}
	130	+
123	131		func TestOnNotifyOSC777STTerminated(t *testing.T) {
124	132		s := NewScreen(10, 2)
125	133		var gotTitle, gotBody string


@@ -120,6 +120,14 @@ func TestOnNotifyOSC9(t *testing.T) {
120	require.Equal(t, "agent done", gotBody)	120		require.Equal(t, "agent done", gotBody)
121	}	121		}
122		122
		123	+	func TestOnNotifyIgnoresOSC9Progress(t *testing.T) {
		124	+	s := NewScreen(10, 2)
		125	+	calls := 0
		126	+	s.OnNotify = func(string, string) { calls++ }
		127	+	s.Write([]byte("\x1b]9;4;1;50\x07"))
		128	+	require.Equal(t, 0, calls)
		129	+	}
		130	+
123	func TestOnNotifyOSC777STTerminated(t *testing.T) {	131		func TestOnNotifyOSC777STTerminated(t *testing.T) {
124	s := NewScreen(10, 2)	132		s := NewScreen(10, 2)
125	var gotTitle, gotBody string	133		var gotTitle, gotBody string