Merge pull request #2544 from dgageot/board/simplifying-custom-tui-component-5a41ea99

refactor(tui): simplify components, drop dead code, consolidate helpers

Author: dgageot

pkg/tui/components/markdown/fast_renderer_test.go

@@ -6,10 +6,13 @@ import (
 	"strings"
 	"testing"
 
+	"charm.land/glamour/v2"
 	"github.com/charmbracelet/x/ansi"
 	runewidth "github.com/mattn/go-runewidth"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/docker/docker-agent/pkg/tui/styles"
 )
 
 // stripANSI removes ANSI escape sequences from a string.
@@ -19,6 +22,19 @@ func stripANSI(s string) string {
 	return ansiRegex.ReplaceAllString(s, "")
 }
 
+// newGlamourRenderer creates a markdown renderer using glamour.
+// Used as a reference implementation to compare against the fast renderer.
+func newGlamourRenderer(width int) *glamour.TermRenderer {
+	style := styles.MarkdownStyle()
+
+	r, _ := glamour.NewTermRenderer(
+		glamour.WithWordWrap(width),
+		glamour.WithStyles(style),
+		glamour.WithPreservedNewLines(),
+	)
+	return r
+}
+
 func TestFastRendererBasicText(t *testing.T) {
 	t.Parallel()
 
@@ -909,7 +925,7 @@ func BenchmarkFastRenderer(b *testing.B) {
 }
 
 func BenchmarkGlamourRenderer(b *testing.B) {
-	r := NewGlamourRenderer(80)
+	r := newGlamourRenderer(80)
 	for b.Loop() {
 		_, _ = r.Render(benchmarkInput)
 	}
@@ -924,7 +940,7 @@ func BenchmarkFastRendererSmall(b *testing.B) {
 }
 
 func BenchmarkGlamourRendererSmall(b *testing.B) {
-	r := NewGlamourRenderer(80)
+	r := newGlamourRenderer(80)
 	input := "Hello **world**, this is a *test*."
 	for b.Loop() {
 		_, _ = r.Render(input)
@@ -940,7 +956,7 @@ func BenchmarkFastRendererCodeBlock(b *testing.B) {
 }
 
 func BenchmarkGlamourRendererCodeBlock(b *testing.B) {
-	r := NewGlamourRenderer(80)
+	r := newGlamourRenderer(80)
 	input := "```go\nfunc main() {\n\tfmt.Println(\"hello`\")\n}\n```"
 	for b.Loop() {
 		_, _ = r.Render(input)
@@ -962,7 +978,7 @@ func BenchmarkFastRendererTable(b *testing.B) {
 }
 
 func BenchmarkGlamourRendererTable(b *testing.B) {
-	r := NewGlamourRenderer(80)
+	r := newGlamourRenderer(80)
 	for b.Loop() {
 		_, _ = r.Render(benchmarkTableInput)
 	}
@@ -976,7 +992,7 @@ func BenchmarkFastRendererTableWidth20(b *testing.B) {
 }
 
 func BenchmarkGlamourRendererTableWidth20(b *testing.B) {
-	r := NewGlamourRenderer(20)
+	r := newGlamourRenderer(20)
 	for b.Loop() {
 		_, _ = r.Render(benchmarkTableInput)
 	}
@@ -990,7 +1006,7 @@ func BenchmarkFastRendererTableWidth200(b *testing.B) {
 }
 
 func BenchmarkGlamourRendererTableWidth200(b *testing.B) {
-	r := NewGlamourRenderer(200)
+	r := newGlamourRenderer(200)
 	for b.Loop() {
 		_, _ = r.Render(benchmarkTableInput)
 	}
@@ -1734,7 +1750,7 @@ func BenchmarkStreamingGlamourRenderer(b *testing.B) {
 
 	b.ResetTimer()
 	for b.Loop() {
-		r := NewGlamourRenderer(80)
+		r := newGlamourRenderer(80)
 		var accumulated strings.Builder
 		for _, chunk := range chunks {
 			accumulated.WriteString(chunk)

pkg/tui/components/markdown/renderer.go

@@ -1,11 +1,5 @@
 package markdown
 
-import (
-	"charm.land/glamour/v2"
-
-	"github.com/docker/docker-agent/pkg/tui/styles"
-)
-
 // Renderer is an interface for markdown renderers.
 type Renderer interface {
 	Render(input string) (string, error)
@@ -15,16 +9,3 @@ type Renderer interface {
 func NewRenderer(width int) Renderer {
 	return NewFastRenderer(width)
 }
-
-// NewGlamourRenderer creates a markdown renderer using glamour.
-// This is kept for compatibility and testing purposes.
-func NewGlamourRenderer(width int) *glamour.TermRenderer {
-	style := styles.MarkdownStyle()
-
-	r, _ := glamour.NewTermRenderer(
-		glamour.WithWordWrap(width),
-		glamour.WithStyles(style),
-		glamour.WithPreservedNewLines(),
-	)
-	return r
-}

pkg/tui/components/message/message.go

@@ -2,7 +2,6 @@ package message
 
 import (
 	"fmt"
-	"regexp"
 	"strings"
 
 	tea "charm.land/bubbletea/v2"
@@ -249,12 +248,6 @@ func (mv *messageModel) GetSize() (width, height int) {
 	return mv.width, mv.height
 }
 
-var ansiEscape = regexp.MustCompile("\x1b\\[[0-9;]*m")
-
-func stripANSI(s string) string {
-	return ansiEscape.ReplaceAllString(s, "")
-}
-
 // preserveLineBreaks preserves leading indentation by converting leading spaces
 // to non-breaking spaces (U+00A0) which won't be stripped by markdown parsers.
 // Line breaks are handled by glamour.WithPreservedNewLines().

pkg/tui/components/message/message_test.go

@@ -1,6 +1,7 @@
 package message
 
 import (
+	"regexp"
 	"strings"
 	"testing"
 
@@ -10,6 +11,12 @@ import (
 	"github.com/docker/docker-agent/pkg/tui/types"
 )
 
+var ansiEscape = regexp.MustCompile("\x1b\\[[0-9;]*m")
+
+func stripANSI(s string) string {
+	return ansiEscape.ReplaceAllString(s, "")
+}
+
 func TestErrorMessageWrapping(t *testing.T) {
 	t.Parallel()
 

pkg/tui/components/messages/messages.go

@@ -236,9 +236,6 @@ func (m *model) Update(msg tea.Msg) (layout.Model, tea.Cmd) {
 		}
 		return m, tea.Batch(cmds...)
 
-	case reasoningblock.BlockMsg:
-		return m.forwardToReasoningBlock(msg.GetBlockID(), msg)
-
 	case animation.TickMsg:
 		// Invalidate render cache if there's animated content that needs redrawing.
 		// This ensures fades, spinners, etc. actually update visually on each tick.
@@ -1142,21 +1139,6 @@ func (m *model) invalidateAllItems() {
 	m.renderDirty = true
 }
 
-// forwardToReasoningBlock finds the reasoning block with the given ID and forwards the message to it.
-func (m *model) forwardToReasoningBlock(blockID string, msg tea.Msg) (layout.Model, tea.Cmd) {
-	for i, tuiMsg := range m.messages {
-		if tuiMsg.Type == types.MessageTypeAssistantReasoningBlock {
-			if block, ok := m.views[i].(*reasoningblock.Model); ok && block.ID() == blockID {
-				updatedView, cmd := m.views[i].Update(msg)
-				m.views[i] = updatedView
-				m.invalidateItem(i)
-				return m, cmd
-			}
-		}
-	}
-	return m, nil
-}
-
 // Message management methods
 func (m *model) AddUserMessage(content string) tea.Cmd {
 	return m.addMessage(types.User(content))

pkg/tui/components/reasoningblock/reasoningblock.go

@@ -49,21 +49,6 @@ func fadeColor(progress float64) color.Color {
 // Tests can override this for deterministic behavior.
 var nowFunc = time.Now
 
-// BlockMsg is implemented by messages that target a specific reasoning block.
-type BlockMsg interface {
-	GetBlockID() string
-}
-
-// blockMsgBase is embedded in messages that target a specific reasoning block.
-type blockMsgBase struct {
-	BlockID string
-}
-
-func (m blockMsgBase) GetBlockID() string { return m.BlockID }
-
-// ToggleMsg is sent when the block should toggle expanded/collapsed state.
-type ToggleMsg struct{ blockMsgBase }
-
 // toolEntry holds a tool call message and its view.
 type toolEntry struct {
 	msg                   *types.Message

pkg/tui/components/scrollview/scrollview.go

@@ -53,9 +53,6 @@ func ReadOnlyScrollKeyMap() *ScrollKeyMap {
 
 type Option func(*Model)
 
-// WithGapWidth sets the space columns between content and scrollbar (default 1).
-func WithGapWidth(n int) Option { return func(m *Model) { m.gapWidth = n } }
-
 // WithReserveScrollbarSpace always reserves gap+scrollbar columns, preventing layout shifts.
 func WithReserveScrollbarSpace(v bool) Option {
 	return func(m *Model) { m.reserveScrollbarSpace = v }

pkg/tui/components/sidebar/sidebar.go

@@ -154,15 +154,8 @@ type model struct {
 	agentClickZones map[int]string // content line -> agent name
 }
 
-// Option is a functional option for configuring the sidebar.
-type Option func(*model)
-
-// WithLayoutConfig sets a custom layout configuration.
-func WithLayoutConfig(cfg LayoutConfig) Option {
-	return func(m *model) { m.layoutCfg = cfg }
-}
-
-func New(sessionState *service.SessionState, opts ...Option) Model {
+// New creates a new sidebar bound to the given session state.
+func New(sessionState *service.SessionState) Model {
 	ti := textinput.New()
 	ti.Placeholder = "Session title"
 	ti.CharLimit = 50
@@ -188,9 +181,6 @@ func New(sessionState *service.SessionState, opts ...Option) Model {
 		titleInput:       ti,
 		cacheDirty:       true, // Initial render needed
 	}
-	for _, opt := range opts {
-		opt(m)
-	}
 	return m
 }
 

pkg/tui/components/tool/directorytree/directorytree.go

@@ -1,7 +1,6 @@
 package directorytree
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/docker/docker-agent/pkg/tools/builtin"
@@ -27,18 +26,16 @@ func extractResult(msg *types.Message) string {
 		return ""
 	}
 
-	fileCount := meta.FileCount
-	dirCount := meta.DirCount
-	if fileCount+dirCount == 0 {
+	if meta.FileCount+meta.DirCount == 0 {
 		return "empty"
 	}
 
 	var parts []string
-	if fileCount > 0 {
-		parts = append(parts, formatCount(fileCount, "file", "files"))
+	if meta.FileCount > 0 {
+		parts = append(parts, toolcommon.Pluralize(meta.FileCount, "file", "files"))
 	}
-	if dirCount > 0 {
-		parts = append(parts, formatCount(dirCount, "dir", "dirs"))
+	if meta.DirCount > 0 {
+		parts = append(parts, toolcommon.Pluralize(meta.DirCount, "dir", "dirs"))
 	}
 
 	result := strings.Join(parts, ", ")
@@ -47,10 +44,3 @@ func extractResult(msg *types.Message) string {
 	}
 	return result
 }
-
-func formatCount(count int, singular, plural string) string {
-	if count == 1 {
-		return fmt.Sprintf("%d %s", count, singular)
-	}
-	return fmt.Sprintf("%d %s", count, plural)
-}