394b69a fix(terminal): address truecolor review feedback · vev

M README.md +2 -2


@@ -133,9 +133,9 @@ Host *
133	133
134	134		## Terminal color
135	135
136		-	When the attaching client reports truecolor support, vev advertises panes as `TERM=xterm-direct` and exports `COLORTERM=truecolor` to child processes. Otherwise, panes use the conservative `TERM=xterm-256color` and do not receive `COLORTERM`. This works for local and remote attach because the client capability is carried inside vev's protocol rather than relying on SSH `SendEnv`/`AcceptEnv`.
	136	+	When the attaching client reports truecolor support, vev advertises newly spawned panes as `TERM=xterm-direct` and exports `COLORTERM=truecolor` to child processes. Otherwise, panes use the conservative `TERM=xterm-256color` and do not receive `COLORTERM`. This works for local and remote attach because the client capability is carried inside vev's protocol rather than relying on SSH `SendEnv`/`AcceptEnv`.
137	137
138		-	Applications that use terminfo can detect direct color from `xterm-direct`; applications that use the common environment convention can use `COLORTERM=truecolor` when it is present.
	138	+	Applications that use terminfo can detect direct color from `xterm-direct`; applications that use the common environment convention can use `COLORTERM=truecolor` when it is present. Panes restored from a saved snapshot start conservatively before any client attaches; after attach or resume, newly spawned panes use that client's reported capability.
139	139
140	140		## Keys
141	141


@@ -133,9 +133,9 @@ Host *
133			133
134		## Terminal color	134		## Terminal color
135			135
136	-	When the attaching client reports truecolor support, vev advertises panes as `TERM=xterm-direct` and exports `COLORTERM=truecolor` to child processes. Otherwise, panes use the conservative `TERM=xterm-256color` and do not receive `COLORTERM`. This works for local and remote attach because the client capability is carried inside vev's protocol rather than relying on SSH `SendEnv`/`AcceptEnv`.	136	+	When the attaching client reports truecolor support, vev advertises newly spawned panes as `TERM=xterm-direct` and exports `COLORTERM=truecolor` to child processes. Otherwise, panes use the conservative `TERM=xterm-256color` and do not receive `COLORTERM`. This works for local and remote attach because the client capability is carried inside vev's protocol rather than relying on SSH `SendEnv`/`AcceptEnv`.
137			137
138	-	Applications that use terminfo can detect direct color from `xterm-direct`; applications that use the common environment convention can use `COLORTERM=truecolor` when it is present.	138	+	Applications that use terminfo can detect direct color from `xterm-direct`; applications that use the common environment convention can use `COLORTERM=truecolor` when it is present. Panes restored from a saved snapshot start conservatively before any client attaches; after attach or resume, newly spawned panes use that client's reported capability.
139			139
140		## Keys	140		## Keys
141			141

M internal/app/run.go +14 -8


@@ -450,6 +450,19 @@ func (d localDaemonDialer) Dial(ctx context.Context) (ports.Transport, error) {
450	450		return ensureDaemon(ctx, d.dir, realDial, realSpawn, defaultBackoff)
451	451		}
452	452
	453	+	func detachedLocalHello(name, cwd string) ports.Hello {
	454	+	termEnv := os.Getenv("TERM")
	455	+	return ports.Hello{
	456	+	Version: ports.ProtocolVersion,
	457	+	Intent: ports.IntentNew,
	458	+	Name: name,
	459	+	Size: domain.Size{Cols: 80, Rows: 24},
	460	+	TermEnv: termEnv,
	461	+	Cwd: cwd,
	462	+	TrueColor: client.DetectTrueColor(termEnv, os.Getenv("COLORTERM")),
	463	+	}
	464	+	}
	465	+
453	466		func createDetachedLocalSession(ctx context.Context, name string) error {
454	467		transport, err := ensureDaemon(ctx, ipc.SocketDir(), realDial, realSpawn, defaultBackoff)
455	468		if err != nil {

@@ -461,14 +474,7 @@ func createDetachedLocalSession(ctx context.Context, name string) error {
461	474		if err != nil {
462	475		cwd = ""
463	476		}
464		-	hello := ports.Hello{
465		-	Version: ports.ProtocolVersion,
466		-	Intent: ports.IntentNew,
467		-	Name: name,
468		-	Size: domain.Size{Cols: 80, Rows: 24},
469		-	TermEnv: os.Getenv("TERM"),
470		-	Cwd: cwd,
471		-	}
	477	+	hello := detachedLocalHello(name, cwd)
472	478		if err := transport.Send(ports.Frame{Type: ports.MsgHello, Payload: ports.MarshalHello(hello)}); err != nil {
473	479		return fmt.Errorf("vev: creating detached session: %w", err)
474	480		}

 	return ensureDaemon(ctx, d.dir, realDial, realSpawn, defaultBackoff)
 }
+func detachedLocalHello(name, cwd string) ports.Hello {
+	termEnv := os.Getenv("TERM")
+	return ports.Hello{
+		Version:   ports.ProtocolVersion,
+		Intent:    ports.IntentNew,
+		Name:      name,
+		Size:      domain.Size{Cols: 80, Rows: 24},
+		TermEnv:   termEnv,
+		Cwd:       cwd,
+		TrueColor: client.DetectTrueColor(termEnv, os.Getenv("COLORTERM")),
+	}
+}
 func createDetachedLocalSession(ctx context.Context, name string) error {
 	transport, err := ensureDaemon(ctx, ipc.SocketDir(), realDial, realSpawn, defaultBackoff)
 	if err != nil {
 	if err != nil {
 		cwd = ""
 	}
-	hello := ports.Hello{
+	hello := detachedLocalHello(name, cwd)
-		Version: ports.ProtocolVersion,
-		Intent:  ports.IntentNew,
-		Name:    name,
-		Size:    domain.Size{Cols: 80, Rows: 24},
-		TermEnv: os.Getenv("TERM"),
-		Cwd:     cwd,
-	}
 	if err := transport.Send(ports.Frame{Type: ports.MsgHello, Payload: ports.MarshalHello(hello)}); err != nil {
 		return fmt.Errorf("vev: creating detached session: %w", err)
 	}

M internal/app/run_test.go +14 -0


@@ -18,6 +18,7 @@ import (
18	18		"github.com/bnema/vev/internal/ports"
19	19		"github.com/bnema/vev/internal/usecase/client"
20	20		"github.com/bnema/vev/internal/usecase/confirm"
	21	+	"github.com/stretchr/testify/require"
21	22		)
22	23
23	24		func TestParseArgs(t *testing.T) {

@@ -411,6 +412,19 @@ func TestRunAttachNestedNewCreatesDetachedSession(t *testing.T) {
411	412		}
412	413		}
413	414
	415	+	func TestDetachedLocalHelloIncludesTrueColor(t *testing.T) {
	416	+	t.Setenv("TERM", "xterm-direct")
	417	+	t.Setenv("COLORTERM", "")
	418	+
	419	+	hello := detachedLocalHello("scratch", "/tmp/work")
	420	+
	421	+	require.Equal(t, ports.IntentNew, hello.Intent)
	422	+	require.Equal(t, "scratch", hello.Name)
	423	+	require.Equal(t, "xterm-direct", hello.TermEnv)
	424	+	require.Equal(t, "/tmp/work", hello.Cwd)
	425	+	require.True(t, hello.TrueColor)
	426	+	}
	427	+
414	428		type namedDialer struct{ name string }
415	429
416	430		func (d namedDialer) Dial(context.Context) (ports.Transport, error) {


@@ -18,6 +18,7 @@ import (
18	"github.com/bnema/vev/internal/ports"	18		"github.com/bnema/vev/internal/ports"
19	"github.com/bnema/vev/internal/usecase/client"	19		"github.com/bnema/vev/internal/usecase/client"
20	"github.com/bnema/vev/internal/usecase/confirm"	20		"github.com/bnema/vev/internal/usecase/confirm"
		21	+	"github.com/stretchr/testify/require"
21	)	22		)
22		23
23	func TestParseArgs(t *testing.T) {	24		func TestParseArgs(t *testing.T) {

@@ -411,6 +412,19 @@ func TestRunAttachNestedNewCreatesDetachedSession(t *testing.T) {
411	}	412		}
412	}	413		}
413		414
		415	+	func TestDetachedLocalHelloIncludesTrueColor(t *testing.T) {
		416	+	t.Setenv("TERM", "xterm-direct")
		417	+	t.Setenv("COLORTERM", "")
		418	+
		419	+	hello := detachedLocalHello("scratch", "/tmp/work")
		420	+
		421	+	require.Equal(t, ports.IntentNew, hello.Intent)
		422	+	require.Equal(t, "scratch", hello.Name)
		423	+	require.Equal(t, "xterm-direct", hello.TermEnv)
		424	+	require.Equal(t, "/tmp/work", hello.Cwd)
		425	+	require.True(t, hello.TrueColor)
		426	+	}
		427	+
414	type namedDialer struct{ name string }	428		type namedDialer struct{ name string }
415		429
416	func (d namedDialer) Dial(context.Context) (ports.Transport, error) {	430		func (d namedDialer) Dial(context.Context) (ports.Transport, error) {

M internal/usecase/client/client.go +3 -2


@@ -332,7 +332,8 @@ type attachResult struct {
332	332		err error
333	333		}
334	334
335		-	func detectTrueColor(termEnv, colorTerm string) bool {
	335	+	// DetectTrueColor reports whether TERM/COLORTERM advertise direct color support.
	336	+	func DetectTrueColor(termEnv, colorTerm string) bool {
336	337		switch strings.ToLower(strings.TrimSpace(colorTerm)) {
337	338		case "truecolor", "24bit":
338	339		return true

@@ -354,7 +355,7 @@ func attachOnce(ctx context.Context, transport ports.Transport, term ports.Termi
354	355		}
355	356		termEnv := os.Getenv("TERM")
356	357		colorTerm := os.Getenv("COLORTERM")
357		-	trueColor := detectTrueColor(termEnv, colorTerm)
	358	+	trueColor := DetectTrueColor(termEnv, colorTerm)
358	359		hello := ports.Hello{
359	360		Version: ports.ProtocolVersion,
360	361		Intent: intent,

 	err         error
 }
-func detectTrueColor(termEnv, colorTerm string) bool {
+// DetectTrueColor reports whether TERM/COLORTERM advertise direct color support.
+func DetectTrueColor(termEnv, colorTerm string) bool {
 	switch strings.ToLower(strings.TrimSpace(colorTerm)) {
 	case "truecolor", "24bit":
 		return true
 	}
 	termEnv := os.Getenv("TERM")
 	colorTerm := os.Getenv("COLORTERM")
-	trueColor := detectTrueColor(termEnv, colorTerm)
+	trueColor := DetectTrueColor(termEnv, colorTerm)
 	hello := ports.Hello{
 		Version:     ports.ProtocolVersion,
 		Intent:      intent,

M internal/usecase/client/truecolor_test.go +1 -1


@@ -25,7 +25,7 @@ func TestDetectTrueColor(t *testing.T) {
25	25
26	26		for _, tt := range tests {
27	27		t.Run(tt.name, func(t *testing.T) {
28		-	require.Equal(t, tt.want, detectTrueColor(tt.termEnv, tt.colorTerm))
	28	+	require.Equal(t, tt.want, DetectTrueColor(tt.termEnv, tt.colorTerm))
29	29		})
30	30		}
31	31		}


@@ -25,7 +25,7 @@ func TestDetectTrueColor(t *testing.T) {
25			25
26		for _, tt := range tests {	26		for _, tt := range tests {
27		t.Run(tt.name, func(t *testing.T) {	27		t.Run(tt.name, func(t *testing.T) {
28	-	require.Equal(t, tt.want, detectTrueColor(tt.termEnv, tt.colorTerm))	28	+	require.Equal(t, tt.want, DetectTrueColor(tt.termEnv, tt.colorTerm))
29		})	29		})
30		}	30		}
31		}	31		}

M internal/usecase/daemon/session_test.go +43 -0


@@ -1091,6 +1091,49 @@ func TestAttachUpdatesFutureChildEnvTrueColor(t *testing.T) {
1091	1091		require.Contains(t, opens[1], "TERM_PROGRAM=vev")
1092	1092		}
1093	1093
	1094	+	func TestLiveAttachUpdatesFutureChildEnvTrueColor(t *testing.T) {
	1095	+	sz := domain.Size{Cols: 80, Rows: 24}
	1096	+	p1, release1 := newBlockingPTY(t)
	1097	+	defer release1()
	1098	+	p2, release2 := newBlockingPTY(t)
	1099	+	defer release2()
	1100	+
	1101	+	var opens [][]string
	1102	+	f := portsmocks.NewMockPTYFactory(t)
	1103	+	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
	1104	+	func(_ string, _ []string, env []string, _ string, _ domain.Size) (ports.PTY, error) {
	1105	+	opens = append(opens, append([]string(nil), env...))
	1106	+	if len(opens) == 1 {
	1107	+	return p1, nil
	1108	+	}
	1109	+	return p2, nil
	1110	+	},
	1111	+	).Twice()
	1112	+
	1113	+	d := newTestDaemon(t, f, stubClock{})
	1114	+	tr1 := portsmocks.NewMockTransport(t)
	1115	+	tr1.EXPECT().Send(mock.Anything).Return(nil).Maybe()
	1116	+	tr1.EXPECT().Close().Return(nil).Maybe()
	1117	+	tr2 := portsmocks.NewMockTransport(t)
	1118	+	tr2.EXPECT().Send(mock.Anything).Return(nil).Maybe()
	1119	+	tr2.EXPECT().Close().Return(nil).Maybe()
	1120	+	sess, _, err := d.route(ports.Hello{Version: ports.ProtocolVersion, Intent: ports.IntentNew, Name: "work", Size: sz, TrueColor: false}, tr1)
	1121	+	require.NoError(t, err)
	1122	+	_, ac, err := d.route(ports.Hello{Version: ports.ProtocolVersion, Intent: ports.IntentAttach, Name: "work", Size: sz, TrueColor: true}, tr2)
	1123	+	require.NoError(t, err)
	1124	+	defer func() {
	1125	+	_ = d.killSession(sess, ports.ReasonServerShutdown, false)
	1126	+	d.sessWg.Wait()
	1127	+	}()
	1128	+
	1129	+	require.Contains(t, opens[0], "TERM=xterm-256color")
	1130	+	require.NotContains(t, opens[0], "COLORTERM=truecolor")
	1131	+	require.NoError(t, d.createTab(sess, ac.size))
	1132	+	require.Contains(t, opens[1], "TERM=xterm-direct")
	1133	+	require.Contains(t, opens[1], "COLORTERM=truecolor")
	1134	+	require.Contains(t, opens[1], "TERM_PROGRAM=vev")
	1135	+	}
	1136	+
1094	1137		func TestCreateSessionAndSwitchInheritsTerminalEnv(t *testing.T) {
1095	1138		sz := domain.Size{Cols: 80, Rows: 24}
1096	1139		p1, release1 := newBlockingPTY(t)


@@ -1091,6 +1091,49 @@ func TestAttachUpdatesFutureChildEnvTrueColor(t *testing.T) {
1091	require.Contains(t, opens[1], "TERM_PROGRAM=vev")	1091		require.Contains(t, opens[1], "TERM_PROGRAM=vev")
1092	}	1092		}
1093		1093
		1094	+	func TestLiveAttachUpdatesFutureChildEnvTrueColor(t *testing.T) {
		1095	+	sz := domain.Size{Cols: 80, Rows: 24}
		1096	+	p1, release1 := newBlockingPTY(t)
		1097	+	defer release1()
		1098	+	p2, release2 := newBlockingPTY(t)
		1099	+	defer release2()
		1100	+
		1101	+	var opens [][]string
		1102	+	f := portsmocks.NewMockPTYFactory(t)
		1103	+	f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).RunAndReturn(
		1104	+	func(_ string, _ []string, env []string, _ string, _ domain.Size) (ports.PTY, error) {
		1105	+	opens = append(opens, append([]string(nil), env...))
		1106	+	if len(opens) == 1 {
		1107	+	return p1, nil
		1108	+	}
		1109	+	return p2, nil
		1110	+	},
		1111	+	).Twice()
		1112	+
		1113	+	d := newTestDaemon(t, f, stubClock{})
		1114	+	tr1 := portsmocks.NewMockTransport(t)
		1115	+	tr1.EXPECT().Send(mock.Anything).Return(nil).Maybe()
		1116	+	tr1.EXPECT().Close().Return(nil).Maybe()
		1117	+	tr2 := portsmocks.NewMockTransport(t)
		1118	+	tr2.EXPECT().Send(mock.Anything).Return(nil).Maybe()
		1119	+	tr2.EXPECT().Close().Return(nil).Maybe()
		1120	+	sess, _, err := d.route(ports.Hello{Version: ports.ProtocolVersion, Intent: ports.IntentNew, Name: "work", Size: sz, TrueColor: false}, tr1)
		1121	+	require.NoError(t, err)
		1122	+	_, ac, err := d.route(ports.Hello{Version: ports.ProtocolVersion, Intent: ports.IntentAttach, Name: "work", Size: sz, TrueColor: true}, tr2)
		1123	+	require.NoError(t, err)
		1124	+	defer func() {
		1125	+	_ = d.killSession(sess, ports.ReasonServerShutdown, false)
		1126	+	d.sessWg.Wait()
		1127	+	}()
		1128	+
		1129	+	require.Contains(t, opens[0], "TERM=xterm-256color")
		1130	+	require.NotContains(t, opens[0], "COLORTERM=truecolor")
		1131	+	require.NoError(t, d.createTab(sess, ac.size))
		1132	+	require.Contains(t, opens[1], "TERM=xterm-direct")
		1133	+	require.Contains(t, opens[1], "COLORTERM=truecolor")
		1134	+	require.Contains(t, opens[1], "TERM_PROGRAM=vev")
		1135	+	}
		1136	+
1094	func TestCreateSessionAndSwitchInheritsTerminalEnv(t *testing.T) {	1137		func TestCreateSessionAndSwitchInheritsTerminalEnv(t *testing.T) {
1095	sz := domain.Size{Cols: 80, Rows: 24}	1138		sz := domain.Size{Cols: 80, Rows: 24}
1096	p1, release1 := newBlockingPTY(t)	1139		p1, release1 := newBlockingPTY(t)