Skip to content

Commit b0b1b12

Browse files
committed
refactor(tui): drop dead code and unused options across components
Remove unreachable functions, unused functional options and test-only helpers from the production code paths in pkg/tui. No behavior change; builds, lint, and the full test suite all pass. - scrollview: remove unused WithGapWidth option. - sidebar: remove unused WithLayoutConfig and Option type, drop the variadic opts ...Option from New (no caller passes any). - reasoningblock: remove the BlockMsg/blockMsgBase/ToggleMsg/ GetBlockID infrastructure; nothing constructed a ToggleMsg. - messages: drop the now-unreachable BlockMsg switch case in Update and the orphaned forwardToReasoningBlock helper. - tool/registry: collapse Register/RegisterAll into a single Register(registrations) entry point; the per-name overload was never called. - markdown: move newGlamourRenderer into fast_renderer_test.go (only tests used it); production renderer.go no longer needs the glamour or styles imports. - message: move stripANSI into message_test.go (only tests used it); production message.go no longer needs regexp. Assisted-By: docker-agent
1 parent b27e9d2 commit b0b1b12

10 files changed

Lines changed: 36 additions & 92 deletions

File tree

pkg/tui/components/markdown/fast_renderer_test.go

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@ import (
66
"strings"
77
"testing"
88

9+
"charm.land/glamour/v2"
910
"github.com/charmbracelet/x/ansi"
1011
runewidth "github.com/mattn/go-runewidth"
1112
"github.com/stretchr/testify/assert"
1213
"github.com/stretchr/testify/require"
14+
15+
"github.com/docker/docker-agent/pkg/tui/styles"
1316
)
1417

1518
// stripANSI removes ANSI escape sequences from a string.
@@ -19,6 +22,19 @@ func stripANSI(s string) string {
1922
return ansiRegex.ReplaceAllString(s, "")
2023
}
2124

25+
// newGlamourRenderer creates a markdown renderer using glamour.
26+
// Used as a reference implementation to compare against the fast renderer.
27+
func newGlamourRenderer(width int) *glamour.TermRenderer {
28+
style := styles.MarkdownStyle()
29+
30+
r, _ := glamour.NewTermRenderer(
31+
glamour.WithWordWrap(width),
32+
glamour.WithStyles(style),
33+
glamour.WithPreservedNewLines(),
34+
)
35+
return r
36+
}
37+
2238
func TestFastRendererBasicText(t *testing.T) {
2339
t.Parallel()
2440

@@ -909,7 +925,7 @@ func BenchmarkFastRenderer(b *testing.B) {
909925
}
910926

911927
func BenchmarkGlamourRenderer(b *testing.B) {
912-
r := NewGlamourRenderer(80)
928+
r := newGlamourRenderer(80)
913929
for b.Loop() {
914930
_, _ = r.Render(benchmarkInput)
915931
}
@@ -924,7 +940,7 @@ func BenchmarkFastRendererSmall(b *testing.B) {
924940
}
925941

926942
func BenchmarkGlamourRendererSmall(b *testing.B) {
927-
r := NewGlamourRenderer(80)
943+
r := newGlamourRenderer(80)
928944
input := "Hello **world**, this is a *test*."
929945
for b.Loop() {
930946
_, _ = r.Render(input)
@@ -940,7 +956,7 @@ func BenchmarkFastRendererCodeBlock(b *testing.B) {
940956
}
941957

942958
func BenchmarkGlamourRendererCodeBlock(b *testing.B) {
943-
r := NewGlamourRenderer(80)
959+
r := newGlamourRenderer(80)
944960
input := "```go\nfunc main() {\n\tfmt.Println(\"hello`\")\n}\n```"
945961
for b.Loop() {
946962
_, _ = r.Render(input)
@@ -962,7 +978,7 @@ func BenchmarkFastRendererTable(b *testing.B) {
962978
}
963979

964980
func BenchmarkGlamourRendererTable(b *testing.B) {
965-
r := NewGlamourRenderer(80)
981+
r := newGlamourRenderer(80)
966982
for b.Loop() {
967983
_, _ = r.Render(benchmarkTableInput)
968984
}
@@ -976,7 +992,7 @@ func BenchmarkFastRendererTableWidth20(b *testing.B) {
976992
}
977993

978994
func BenchmarkGlamourRendererTableWidth20(b *testing.B) {
979-
r := NewGlamourRenderer(20)
995+
r := newGlamourRenderer(20)
980996
for b.Loop() {
981997
_, _ = r.Render(benchmarkTableInput)
982998
}
@@ -990,7 +1006,7 @@ func BenchmarkFastRendererTableWidth200(b *testing.B) {
9901006
}
9911007

9921008
func BenchmarkGlamourRendererTableWidth200(b *testing.B) {
993-
r := NewGlamourRenderer(200)
1009+
r := newGlamourRenderer(200)
9941010
for b.Loop() {
9951011
_, _ = r.Render(benchmarkTableInput)
9961012
}
@@ -1734,7 +1750,7 @@ func BenchmarkStreamingGlamourRenderer(b *testing.B) {
17341750

17351751
b.ResetTimer()
17361752
for b.Loop() {
1737-
r := NewGlamourRenderer(80)
1753+
r := newGlamourRenderer(80)
17381754
var accumulated strings.Builder
17391755
for _, chunk := range chunks {
17401756
accumulated.WriteString(chunk)
Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
package markdown
22

3-
import (
4-
"charm.land/glamour/v2"
5-
6-
"github.com/docker/docker-agent/pkg/tui/styles"
7-
)
8-
93
// Renderer is an interface for markdown renderers.
104
type Renderer interface {
115
Render(input string) (string, error)
@@ -15,16 +9,3 @@ type Renderer interface {
159
func NewRenderer(width int) Renderer {
1610
return NewFastRenderer(width)
1711
}
18-
19-
// NewGlamourRenderer creates a markdown renderer using glamour.
20-
// This is kept for compatibility and testing purposes.
21-
func NewGlamourRenderer(width int) *glamour.TermRenderer {
22-
style := styles.MarkdownStyle()
23-
24-
r, _ := glamour.NewTermRenderer(
25-
glamour.WithWordWrap(width),
26-
glamour.WithStyles(style),
27-
glamour.WithPreservedNewLines(),
28-
)
29-
return r
30-
}

pkg/tui/components/message/message.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package message
22

33
import (
44
"fmt"
5-
"regexp"
65
"strings"
76

87
tea "charm.land/bubbletea/v2"
@@ -249,12 +248,6 @@ func (mv *messageModel) GetSize() (width, height int) {
249248
return mv.width, mv.height
250249
}
251250

252-
var ansiEscape = regexp.MustCompile("\x1b\\[[0-9;]*m")
253-
254-
func stripANSI(s string) string {
255-
return ansiEscape.ReplaceAllString(s, "")
256-
}
257-
258251
// preserveLineBreaks preserves leading indentation by converting leading spaces
259252
// to non-breaking spaces (U+00A0) which won't be stripped by markdown parsers.
260253
// Line breaks are handled by glamour.WithPreservedNewLines().

pkg/tui/components/message/message_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package message
22

33
import (
4+
"regexp"
45
"strings"
56
"testing"
67

@@ -10,6 +11,12 @@ import (
1011
"github.com/docker/docker-agent/pkg/tui/types"
1112
)
1213

14+
var ansiEscape = regexp.MustCompile("\x1b\\[[0-9;]*m")
15+
16+
func stripANSI(s string) string {
17+
return ansiEscape.ReplaceAllString(s, "")
18+
}
19+
1320
func TestErrorMessageWrapping(t *testing.T) {
1421
t.Parallel()
1522

pkg/tui/components/messages/messages.go

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -235,9 +235,6 @@ func (m *model) Update(msg tea.Msg) (layout.Model, tea.Cmd) {
235235
}
236236
return m, tea.Batch(cmds...)
237237

238-
case reasoningblock.BlockMsg:
239-
return m.forwardToReasoningBlock(msg.GetBlockID(), msg)
240-
241238
case animation.TickMsg:
242239
// Invalidate render cache if there's animated content that needs redrawing.
243240
// This ensures fades, spinners, etc. actually update visually on each tick.
@@ -1093,21 +1090,6 @@ func (m *model) invalidateAllItems() {
10931090
m.renderDirty = true
10941091
}
10951092

1096-
// forwardToReasoningBlock finds the reasoning block with the given ID and forwards the message to it.
1097-
func (m *model) forwardToReasoningBlock(blockID string, msg tea.Msg) (layout.Model, tea.Cmd) {
1098-
for i, tuiMsg := range m.messages {
1099-
if tuiMsg.Type == types.MessageTypeAssistantReasoningBlock {
1100-
if block, ok := m.views[i].(*reasoningblock.Model); ok && block.ID() == blockID {
1101-
updatedView, cmd := m.views[i].Update(msg)
1102-
m.views[i] = updatedView
1103-
m.invalidateItem(i)
1104-
return m, cmd
1105-
}
1106-
}
1107-
}
1108-
return m, nil
1109-
}
1110-
11111093
// Message management methods
11121094
func (m *model) AddUserMessage(content string) tea.Cmd {
11131095
return m.addMessage(types.User(content))

pkg/tui/components/reasoningblock/reasoningblock.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,21 +49,6 @@ func fadeColor(progress float64) color.Color {
4949
// Tests can override this for deterministic behavior.
5050
var nowFunc = time.Now
5151

52-
// BlockMsg is implemented by messages that target a specific reasoning block.
53-
type BlockMsg interface {
54-
GetBlockID() string
55-
}
56-
57-
// blockMsgBase is embedded in messages that target a specific reasoning block.
58-
type blockMsgBase struct {
59-
BlockID string
60-
}
61-
62-
func (m blockMsgBase) GetBlockID() string { return m.BlockID }
63-
64-
// ToggleMsg is sent when the block should toggle expanded/collapsed state.
65-
type ToggleMsg struct{ blockMsgBase }
66-
6752
// toolEntry holds a tool call message and its view.
6853
type toolEntry struct {
6954
msg *types.Message

pkg/tui/components/scrollview/scrollview.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ func ReadOnlyScrollKeyMap() *ScrollKeyMap {
5353

5454
type Option func(*Model)
5555

56-
// WithGapWidth sets the space columns between content and scrollbar (default 1).
57-
func WithGapWidth(n int) Option { return func(m *Model) { m.gapWidth = n } }
58-
5956
// WithReserveScrollbarSpace always reserves gap+scrollbar columns, preventing layout shifts.
6057
func WithReserveScrollbarSpace(v bool) Option {
6158
return func(m *Model) { m.reserveScrollbarSpace = v }

pkg/tui/components/sidebar/sidebar.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -154,15 +154,8 @@ type model struct {
154154
agentClickZones map[int]string // content line -> agent name
155155
}
156156

157-
// Option is a functional option for configuring the sidebar.
158-
type Option func(*model)
159-
160-
// WithLayoutConfig sets a custom layout configuration.
161-
func WithLayoutConfig(cfg LayoutConfig) Option {
162-
return func(m *model) { m.layoutCfg = cfg }
163-
}
164-
165-
func New(sessionState *service.SessionState, opts ...Option) Model {
157+
// New creates a new sidebar bound to the given session state.
158+
func New(sessionState *service.SessionState) Model {
166159
ti := textinput.New()
167160
ti.Placeholder = "Session title"
168161
ti.CharLimit = 50
@@ -188,9 +181,6 @@ func New(sessionState *service.SessionState, opts ...Option) Model {
188181
titleInput: ti,
189182
cacheDirty: true, // Initial render needed
190183
}
191-
for _, opt := range opts {
192-
opt(m)
193-
}
194184
return m
195185
}
196186

pkg/tui/components/tool/factory.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func newDefaultRegistry() *Registry {
6262

6363
// Define tool registrations declaratively.
6464
// Tools with the same visual representation share a builder.
65-
registry.RegisterAll([]Registration{
65+
registry.Register([]Registration{
6666
{[]string{builtin.ToolNameTransferTask}, transfertask.New},
6767
{[]string{builtin.ToolNameHandoff}, handoff.New},
6868
{[]string{builtin.ToolNameEditFile}, editfile.New},

pkg/tui/components/tool/registry.go

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,9 @@ func NewRegistry() *Registry {
3333
}
3434
}
3535

36-
// Register adds a single tool-to-builder mapping.
37-
func (r *Registry) Register(toolName string, builder ComponentBuilder) {
38-
r.mu.Lock()
39-
defer r.mu.Unlock()
40-
r.builders[toolName] = builder
41-
}
42-
43-
// RegisterAll adds multiple registrations at once.
44-
// This is the preferred way to set up the registry declaratively.
45-
func (r *Registry) RegisterAll(registrations []Registration) {
36+
// Register adds multiple registrations at once.
37+
// Tools with the same visual representation share a builder.
38+
func (r *Registry) Register(registrations []Registration) {
4639
r.mu.Lock()
4740
defer r.mu.Unlock()
4841
for _, reg := range registrations {

0 commit comments

Comments
 (0)