diff --git a/docs/coding-cli.md b/docs/coding-cli.md index c68dd90a..e88c8f6f 100644 --- a/docs/coding-cli.md +++ b/docs/coding-cli.md @@ -757,7 +757,9 @@ IDs, grouped by family with the light variant first: - `catppuccin-latte`, `catppuccin-frappe`, `catppuccin-macchiato`, `catppuccin-mocha` — the Catppuccin flavours, light to darkest; - `dracula`; +- `everforest-light`, `everforest-dark`; - `gruvbox-light`, `gruvbox-dark`; +- `kanagawa`; - `modus-operandi`, `modus-vivendi` — the Modus pair, built for WCAG AAA contrast; - `nord`; @@ -766,10 +768,38 @@ IDs, grouped by family with the light variant first: - `solarized-light`, `solarized-dark`; - `tokyo-night-light`, `tokyo-night-storm`, `tokyo-night`. -Every built-in highlights its fenced code with the syntax style that belongs to -the same family, so the UI accents and the code block agree: `tokyo-night` -borrows Tokyo Night's `tokyonight-night` style, `gruvbox-dark` borrows Gruvbox's, -and a palette from a file follows the family its background resolves to. +A built-in whose family ships a syntax style of its own highlights fenced code +with that style, so the UI accents and the code block agree: `tokyo-night` +borrows Tokyo Night's `tokyonight-night` style and `gruvbox-dark` borrows +Gruvbox's. + +A family chroma ships no style for — Everforest, Kanagawa and the others — and +every palette loaded from a file get a complete style derived from their own 14 +palette roles instead, so a fence reads as part of its theme rather than as a +borrowed one. The derivation follows the palette's meaning rather than any one +family's syntax theme: + +- `workspace` is plain code text, names and identifiers; +- `muted` is everything to skim past: comments, punctuation, string delimiters + and subordinate headings; +- `model` is the primary accent (the one headings use): keywords, types, markup; +- `session` is the secondary accent (links and the cursor use it): calls, + builtins, prompts; +- `change` is the value accent: strings, added diff lines; +- `active` is the attention accent: numbers and constants; +- `warning` is the caution accent: escapes, decorators, entities; +- `error` is reserved for failures: exceptions, lexer errors, removed diff + lines. + +`separator`, `composer_prompt`, `code_background` and `diagnostic` are chrome, +surfaces and the status panel's own channel, so a fence never draws with them. +The full role-to-token table lives in `markdown_palette_style.go`. + +A fence paints no fill, so its tokens are read against your terminal's own +background rather than against the theme's `code_background`; the accents are +tuned the way each family tunes them, and the guard checks the two pairings that +do share `code_background` — the fence body and the inline-code pill — at 4.0:1 or +better for every built-in. Open the command picker with `/` and choose `/theme`. The picker lists `auto`, built-ins, and discovered user themes in stable order. Press Enter to apply a @@ -796,8 +826,8 @@ Markdown draws on the terminal's own canvas. No heading paints a background, a code fence paints no block background, and the inline-code fill is painted only when the theme's own background is the one the terminal reported, so a light theme inside a dark terminal drops the fill instead of drawing a bright pill. -A fence keeps only the token colors of its bundled syntax palette; a `diff` -fence therefore still shows its added and removed fills. +A fence never paints a token fill: a `diff` fence's added and removed lines +carry the theme's own `change` and `error` colors on the terminal's canvas. All TUI selections are stored together in the active configuration file under `[tui]`: @@ -831,7 +861,13 @@ window, or no reported prompt size — and a provider that reports no cache read shows an honest `0% cache`. `model` renders a short display name instead of `provider/model`: the provider prefix and any nested catalog path are dropped, so `xai/grok-4.5` reads as `Grok 4.5` and `cline-pass/deepseek-v4.1-flash` as -`DeepSeek V4.1 Flash`. The other fields are `workspace`, `session`, +`DeepSeek V4.1 Flash`. The name is followed by the reasoning level the next +request will carry, in parentheses — `Claude Opus 4.1 (high)`. That level comes +from the resolved model, so a configured `default` shows the level the model's own +metadata resolves it to; a model that declares reasoning levels but has none +selected shows `(default)`, which is the honest answer because the provider's own +default is unknown to Pips; a model with no reasoning levels shows no suffix at +all. The other fields are `workspace`, `session`, `task_progress` (`Tasks completed/total`), `phase`, `mode` (Plan mode), and `team`. `/statusline` toggles and reorders them. @@ -962,10 +998,13 @@ left click can move the selection on a full-area list (`/resume`, `/skills`, `/mcp`, `/tree`, `/agents` and its Runs/Library tabs) — a click selects a row and activates nothing, and motion events are ignored, so the pointer never repaints a frame by hovering. Text is selected in the viewport itself: drag over the -conversation and the addressed rows -are copied through the same clipboard path as `Ctrl+X` (an OSC 52 request plus a -fallback file) with a short confirmation on the status line, and `Esc` clears the -highlight. Rows are copied as they are painted, so a paragraph that wrapped is +conversation and the addressed rows go to the clipboard through an OSC 52 request +only — a drag is a transient reading gesture, so it never writes the fallback +file that `Ctrl+X`, `/copy` and `/export` use — and the reserved band under the +transcript confirms it with `copied 3 lines`. A confirmed copy clears the +highlight; a copy that could not be made keeps the selection and reports why. +`Esc` clears the selection too. Rows are copied as they are painted, so a +paragraph that wrapped is copied as the lines on screen; `/copy` and `/export` give the reflow-free text. With no route open, `Ctrl+R` releases the capture for the session (and takes it back), which hands selection, copy and link handling to the terminal; while the @@ -979,6 +1018,35 @@ otherwise). Neither mode ever clears the terminal's scrollback, and both restore terminal state before the session is released. +Between the activity line and the composer box the frame reserves one blank row: +the reserved band. It stays blank while there is nothing to say, and it carries +the immediate feedback that is not the status line's job. While the transcript is +not at the bottom — after PgUp/PgDn, Home/End or a wheel notch — it shows a +centred `▼`, and a left click anywhere on that row returns to the newest rows +exactly as `End` does; the status line no longer repeats that hint. The glyph +carries no label: the band is a signpost, and the status line is where keys are +named. When a drag copy is confirmed, the band reports `copied 3 lines` for a few +seconds, on the same clock as a status notice; with both present the confirmation +keeps the right edge and the signpost is centred to its left, and the +confirmation wins the row when the two cannot both be shown. The band is one row +on every frame, so the composer never moves when its contents change. + +The band's up counterpart is the turn indicator. While anything is above the +window, the transcript's first row carries a centred `▲`; a left click on that +one cell puts the newest prompt above the window at its top, so repeated clicks +walk back through the conversation one turn at a time — that is why the arrow +stays on screen after each jump, and why it only leaves once the window reaches +the beginning of the conversation. The two arrows bracket the reader: `▲` walks +back a turn, `▼` returns to the newest rows. Both are the filled triangles on +purpose: the hollow triangle is this UI's +attention marker (an awaiting approval, a compacting context, the theme picker's +polarity mark), so the scroll pair reads as movement instead of as a warning. The +indicator is drawn into the first row rather than into a band of its +own, because there is no free row above the transcript — reserving one would take +a row from the messages and would change the window height that decides whether +the indicator is needed at all, so showing it could hide it again on the next +frame. It costs no message row and disappears as soon as your prompt is on screen. + `show_thinking_blocks = true` (the default) renders the model's visible reasoning as a Thinking block in the fullscreen viewport: the whole thought is shown under a `Thinking` label, wrapped and dimmed in the active theme's muted color, and it diff --git a/internal/coding/cli/reasoning_report.go b/internal/coding/cli/reasoning_report.go index aeb9cfb4..70ec9685 100644 --- a/internal/coding/cli/reasoning_report.go +++ b/internal/coding/cli/reasoning_report.go @@ -23,11 +23,12 @@ func reasoningEncoding(resolved modelcatalog.ResolvedModel) (string, bool) { // reasoningSelection returns the level that will be sent, or "" when the // selection falls back to the provider default. func reasoningSelection(resolved modelcatalog.ResolvedModel) string { - if resolved.ReasoningLevel == nil { + level, ok := resolved.EffectiveReasoningLevel() + if !ok { return "" } - return string(*resolved.ReasoningLevel) + return string(level) } // writeResolvedReasoning prints the reasoning state that will actually reach diff --git a/internal/coding/modelcatalog/catalog.go b/internal/coding/modelcatalog/catalog.go index d1682273..17c47cd6 100644 --- a/internal/coding/modelcatalog/catalog.go +++ b/internal/coding/modelcatalog/catalog.go @@ -75,6 +75,23 @@ func (m ResolvedModel) Clone() ResolvedModel { return cloned } +// EffectiveReasoningLevel reports the reasoning level the request assembly sends +// for this model, and whether one is selected at all. +// +// [registry.resolve] folds the explicit selection, the variant's level and the +// model's DefaultReasoningLevel into ReasoningLevel, and the generation layer +// sends exactly that value. A false result means pips sends no level and the +// provider's own default applies, which pips cannot know: it ships no model +// capability database. A caller must therefore show that case as "default" or +// omit it rather than inventing a level. +func (m ResolvedModel) EffectiveReasoningLevel() (config.ReasoningLevel, bool) { + if m.ReasoningLevel == nil || *m.ReasoningLevel == "" { + return "", false + } + + return *m.ReasoningLevel, true +} + // Equal reports value equality without relying on pointer or map identity. func (m ResolvedModel) Equal(other ResolvedModel) bool { return reflect.DeepEqual(m, other) diff --git a/internal/coding/tui/band.go b/internal/coding/tui/band.go new file mode 100644 index 00000000..b10ec68e --- /dev/null +++ b/internal/coding/tui/band.go @@ -0,0 +1,155 @@ +//nolint:wsl_v5 // The band's two occupants and their alignment stay adjacent. +package tui + +import ( + "fmt" + "strings" + "time" + + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + "github.com/charmbracelet/x/ansi" +) + +// The reserved band is the row between the activity line and the Composer. The +// frame budget counts it on every frame (layoutBandRows), so the Composer never +// moves when the band's contents change: the band is one fixed row that is blank +// until there is something to say. +// +// Two things live there today. +// +// - A scroll hint, drawn only while the newest transcript rows are off screen. +// It is the affordance that replaces the status line's scroll hint, and a left +// click anywhere on the band row restores the newest rows, the way End does. +// - The confirmation of a drag copy ("copied 3 lines"). The drag path writes +// the clipboard alone, so it has no file to name; an explicit /copy, /export +// or path copy keeps its status-line notice, which does name the file. The +// two surfaces therefore never repeat each other. +const ( + // bandScrollIcon marks the direction the newest rows are in. It is the filled + // triangle rather than the hollow one, which this UI uses as its attention marker; + // its up counterpart is the turn indicator at the top of the transcript. The glyph + // carries no label: the band is a signpost, and the status line already names the + // keys a reader needs. + bandScrollIcon = "▼" +) + +// copiedNoticeExpiredMsg clears the band's copy confirmation only if it is still +// the current one, so a stale timer cannot erase a newer confirmation. +type copiedNoticeExpiredMsg struct { + generation uint64 +} + +// bandScrollHint is the marker the band shows while the newest output is off screen, +// or "" while the reader is already looking at it. +// +// The test is the scroll region's own "at the bottom" rather than its `follow` +// flag: a resize that grows the window clamps the offset onto the new maximum +// without restoring `follow`, and a hint driven by the flag would then point at +// rows that are already on screen. +func (m *Model) bandScrollHint() string { + if m.transcriptScroll.atBottom() { + return "" + } + + return bandScrollIcon +} + +// copiedNoticeText is the band's copy confirmation, or "" when none is current. +func (m *Model) copiedNoticeText() string { + return strings.TrimSpace(sanitizeInspectionText(m.copiedNotice)) +} + +// reservedBand renders the band's single row. The scroll hint is centred on the +// whole row, because it is a standing invitation to move rather than a field of +// the status line; the copy confirmation keeps the status line's inset and its +// right edge, which is where a transient outcome reads as an outcome. +// +// A narrow terminal truncates the hint's label rather than the outcome, and +// NO_COLOR drops the styling and keeps the text. +// +// The row is blank when there is nothing to show, which is what keeps a frame +// with no live feedback byte-identical to the blank separator row it replaced. +func (m *Model) reservedBand() string { + hint := m.bandScrollHint() + notice := m.copiedNoticeText() + if hint == "" && notice == "" { + return "" + } + if !m.options.NoColor { + muted := lipgloss.NewStyle().Foreground(paletteFor(m.theme).muted) + hint = styleWhenNotEmpty(muted, hint) + notice = styleWhenNotEmpty(muted, notice) + } + + width := max(1, m.width) + inset := timelineInset(width) + + switch { + case hint == "": + // A copy confirmation keeps the status line's inset and right edge, where a + // transient outcome reads as an outcome. + return strings.Repeat(" ", inset) + alignStatusLine("", notice, width-2*inset) + case notice == "": + // The scroll hint is centred on the whole row: it is a standing invitation to + // move rather than a field of the status line. + return centreLine(hint, width) + } + + // Both. The row is [hint][gap][confirmation][inset]; the confirmation wins it + // when the hint could not keep its glyphs. + noticeField := ansi.StringWidth(notice) + hintField := width - 1 - noticeField - inset + if hintField < ansi.StringWidth(bandScrollIcon) { + return strings.Repeat(" ", inset) + alignStatusLine("", notice, width-2*inset) + } + + return centreLine(hint, hintField) + " " + notice + strings.Repeat(" ", inset) +} + +// centreLine centres value in a field of width columns. The row it returns always +// measures exactly width, so the band stays aligned with the frame edge whatever +// it holds, and an odd remainder leaves the extra column on the right. +func centreLine(value string, width int) string { + if width <= 0 { + return "" + } + + value = ansi.Truncate(value, width, "…") + padding := width - ansi.StringWidth(value) + left := padding / 2 + + return strings.Repeat(" ", left) + value + strings.Repeat(" ", padding-left) +} + +func styleWhenNotEmpty(style lipgloss.Style, value string) string { + if value == "" { + return "" + } + + return style.Render(value) +} + +// copiedLinesLabel words how much text a copy took, so the reader can tell a +// one-line pick from a screenful without pasting. +func copiedLinesLabel(lines int) string { + if lines == 1 { + return "copied 1 line" + } + + return fmt.Sprintf("copied %d lines", lines) +} + +// showCopiedNotice puts a copy confirmation in the band for as long as a status +// notice lives, so both surfaces expire on the same clock. +func (m *Model) showCopiedNotice(lines int) tea.Cmd { + m.copiedNoticeSeq++ + m.copiedNotice = copiedLinesLabel(lines) + m.setLayout() + + generation := m.copiedNoticeSeq + + return tea.Tick(statusNoticeTime, func(time.Time) tea.Msg { + return copiedNoticeExpiredMsg{generation: generation} + }) +} diff --git a/internal/coding/tui/copy.go b/internal/coding/tui/copy.go index c9bc9497..c2f1e8b0 100644 --- a/internal/coding/tui/copy.go +++ b/internal/coding/tui/copy.go @@ -114,6 +114,74 @@ func (m *Model) assistantResponses() []string { return responses } +// clipboardResultMsg reports the outcome of one copy's clipboard half. The +// Program's own OSC 52 request has no completion signal, so it only ever carries +// an error from a caller-supplied writer; the drag gesture reads the outcome to +// decide whether the highlight it addressed still stands. +type clipboardResultMsg struct { + generation uint64 + lines int + // selection marks the drag gesture, whose success clears the highlight and + // whose line count the band reports. A path copy and an export report through + // the saver instead. + selection bool + err error +} + +// copyToClipboard hands content to the clipboard. It is the TUI's one clipboard +// write: the drag gesture and /copy both go through it, so a caller that supplies +// [Options.ClipboardWriter] replaces the OSC 52 request for every copy rather than +// for one of them. +func (m *Model) copyToClipboard(content string, lines int, selection bool) tea.Cmd { + m.clipboardCopySeq++ + + generation := m.clipboardCopySeq + write := m.options.ClipboardWriter + + commands := make([]tea.Cmd, 0, 2) + if write == nil { + commands = append(commands, tea.SetClipboard(content)) + } + commands = append(commands, func() tea.Msg { + var err error + if write != nil { + err = write(content) + } + + return clipboardResultMsg{ + generation: generation, lines: lines, selection: selection, err: err, + } + }) + + return tea.Batch(commands...) +} + +// copySelection is the drag gesture's copy: the clipboard alone, never the text +// saver. A selection is a transient reading gesture, so leaving a file behind for +// it would make the same gesture produce a different side effect depending on +// whether a saver happened to be configured. An explicit /copy, /export or a copy +// with a path keeps its saver, which is what makes those results durable. +// +// The selection is deliberately left alone here: the outcome message clears it on +// success and leaves it standing on failure, so the highlight always describes a +// copy that has not been confirmed yet. +func (m *Model) copySelection() tea.Cmd { + text := m.selectionText() + if strings.TrimSpace(text) == "" { + // Nothing was addressed, so nothing was copied. The highlight stays so the + // reader can see what the drag covered, and the notice says why nothing + // happened. + return m.setStatusError("nothing selected to copy") + } + + return m.copyToClipboard(text, copiedLineCount(text), true) +} + +// copiedLineCount counts the lines a copy took, which is what the band reports. +func copiedLineCount(text string) int { + return strings.Count(strings.TrimRight(text, "\n"), "\n") + 1 +} + // saveText runs one operator-requested text write. The clipboard half is an // OSC 52 request through the Program writer, so the single-writer contract // holds; the file half is the durable confirmation the terminal cannot give. @@ -125,7 +193,7 @@ func (m *Model) saveText(kind TextKind, path, content string, clipboard bool) te commands := make([]tea.Cmd, 0, 2) if clipboard { - commands = append(commands, tea.SetClipboard(content)) + commands = append(commands, m.copyToClipboard(content, 0, false)) } if saver != nil { m.textSaveSeq++ diff --git a/internal/coding/tui/copy_test.go b/internal/coding/tui/copy_test.go index e97df462..cd3883dc 100644 --- a/internal/coding/tui/copy_test.go +++ b/internal/coding/tui/copy_test.go @@ -118,6 +118,16 @@ func findTextSaved(messages []tea.Msg) (textSavedMsg, bool) { return textSavedMsg{}, false } +func findClipboardResult(messages []tea.Msg) (clipboardResultMsg, bool) { + for _, message := range messages { + if result, ok := message.(clipboardResultMsg); ok { + return result, true + } + } + + return clipboardResultMsg{}, false +} + func TestCopyTakesTheStreamingDraftAsTheNewestReply(t *testing.T) { t.Parallel() diff --git a/internal/coding/tui/hit.go b/internal/coding/tui/hit.go index f3b01096..14afdcdf 100644 --- a/internal/coding/tui/hit.go +++ b/internal/coding/tui/hit.go @@ -2,6 +2,7 @@ package tui import ( tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" ) // A click is resolved against the frame the user actually saw. readyView records @@ -18,6 +19,72 @@ type frameHitMap struct { transcript int // topDropped is how many transcript rows the frame clamp removed from the top. topDropped int + // band is the frame row the reserved band was painted at, or -1 when the + // frame drew none. A left click there restores the newest transcript rows, + // which is what the arrow the band draws promises. + band int + // turnIndicator is true when this frame painted the up indicator in its first + // row, so a click there can be answered without re-deciding the question + // against a scroll position the reader has already left. + turnIndicator bool +} + +// frameRowOf reports the frame row the first line of the band at index was +// painted at, or -1 when the height clamp pushed it off the frame. parts is the +// composed frame before the clamp, so the row is the height above the band minus +// the rows the clamp removed from the top. +// +// An empty prefix is zero rows, not one: lipgloss answers Height("") with 1, so +// the empty join has to be special-cased or a band at the top of the frame would +// record the row below it. +func frameRowOf(parts []string, index, dropped int) int { + if index < 0 || index >= len(parts) { + return -1 + } + + row := 0 + if index > 0 { + row = lipgloss.Height(lipgloss.JoinVertical(lipgloss.Left, parts[:index]...)) + } + + row -= dropped + if row < 0 { + return -1 + } + + return row +} + +// bandRowOf reports the frame row the reserved band afforded a click at, or -1 +// when the frame drew no arrow there. affords is the answer the frame was painted +// with rather than a fresh one: the height this frame settles on decides whether +// the newest rows are off screen, so asking again after the transcript window has +// measured itself could record a target the frame does not show. +func (m *Model) bandRowOf(parts []string, index, dropped int, affords bool) int { + if !affords { + return -1 + } + + return frameRowOf(parts, index, dropped) +} + +// bandMouse answers a left click on the reserved band by returning to the newest +// rows, which is the mouse form of End. It requires the ready frame to be the one +// on screen, because the recorded row belongs to that frame; a frame that drew no +// arrow affords nothing. +func (m *Model) bandMouse(message tea.MouseMsg) (tea.Cmd, bool) { + if !m.readyViewOwnsPointer() || m.frameHit.band < 0 { + return nil, false + } + + click, ok := message.(tea.MouseClickMsg) + if !ok || click.Button != tea.MouseLeft || click.Y != m.frameHit.band { + return nil, false + } + + m.transcriptScroll.gotoBottom() + + return redrawFrame(), true } // handleMouse is installed on every frame's View while mouse reporting is on. It @@ -46,6 +113,18 @@ func (m *Model) handleMouse(message tea.MouseMsg) tea.Cmd { return nil } + // The turn indicator and the reserved band are pointer targets of their own and + // need no hit test: the frame that painted each one recorded where. They are + // answered before the selection gate because neither is a selection gesture — + // the gesture that owns the transcript would otherwise drop a click on them. + if command, handled := m.turnIndicatorMouse(message); handled { + return command + } + + if command, handled := m.bandMouse(message); handled { + return command + } + // An overlay owns the keyboard, so it owns the pointer: a click that cannot be // aimed at a visible entry is dropped rather than forwarded to the transcript. if !m.viewportOwnsPointer() { diff --git a/internal/coding/tui/hit_test.go b/internal/coding/tui/hit_test.go index bddc0d3b..070a36bc 100644 --- a/internal/coding/tui/hit_test.go +++ b/internal/coding/tui/hit_test.go @@ -109,11 +109,26 @@ func TestFrameHitMapMatchesThePaintedFrame(t *testing.T) { "the band fills its allocation when the store is longer") // The band is the top of the frame, so the first painted row is the window's - // first row. + // first row. The turn indicator is the one thing the frame draws over that row, + // so its span is checked separately and the rest of the row must match. frameRows := strings.Split(view.Content, "\n") windowRows := strings.Split(model.transcriptScroll.visible(), "\n") require.NotEmpty(t, windowRows) - assert.Equal(t, windowRows[0], strings.TrimRight(frameRows[0], " "), + + withoutIndicator := func(row string) string { return row } + if model.frameHit.turnIndicator { + _, left, right, ok := model.turnIndicator() + require.True(t, ok) + assert.Equal(t, turnIndicatorIcon, ansi.Strip(ansi.Cut(frameRows[0], left, right)), + "the frame draws the indicator in the transcript's first row") + withoutIndicator = func(row string) string { + return ansi.Truncate(row, left, "") + ansi.TruncateLeft(row, right, "") + } + } + + assert.Equal(t, + strings.TrimRight(withoutIndicator(windowRows[0]), " "), + strings.TrimRight(withoutIndicator(frameRows[0]), " "), "the band is painted at the top of the frame") first, ok := model.entryRowAt(0) diff --git a/internal/coding/tui/launch_pty_test.go b/internal/coding/tui/launch_pty_test.go new file mode 100644 index 00000000..95238145 --- /dev/null +++ b/internal/coding/tui/launch_pty_test.go @@ -0,0 +1,132 @@ +//go:build darwin || linux + +//nolint:wsl_v5 // PTY launch, frame reads and teardown follow physical order. +package tui + +import ( + "context" + "fmt" + "os" + "strings" + "testing" + "time" + + "github.com/rsbin1178/pips/ai" + "github.com/rsbin1178/pips/internal/coding/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestPTYLaunchShowsTheStatusLineAndTheReservedBand is the end-to-end launch +// check: the real TUI entry runs under a PTY and its frame is read back, so the +// three surfaces this work touched are observed together on the path a user +// takes — the status line naming the model and its effective reasoning level, the +// reserved band under the transcript, and a non-empty transcript between them. +// +// It runs twice, because a frame that renders once can still be a frame that only +// renders once. +func TestPTYLaunchShowsTheStatusLineAndTheReservedBand(t *testing.T) { + if os.Getenv("PIPS_TUI_LAUNCH_PTY_HELPER") == "1" { + runLaunchHelper(t) + + return + } + t.Parallel() + + for round := range 2 { + t.Run(fmt.Sprintf("round-%d", round), func(t *testing.T) { + t.Parallel() + + harness := newPTYHarness(t, "PIPS_TUI_LAUNCH_PTY_HELPER", + "TestPTYLaunchShowsTheStatusLineAndTheReservedBand", 24, 80) + + require.Eventually(t, func() bool { + frame := harness.frame(80, 24) + + return containsAll(frame, launchTailMarker, "Test Model (high)") + }, 30*time.Second, 10*time.Millisecond, "helper output: %s", harness.output.String()) + + frame := harness.frame(80, 24) + assert.Contains(t, frame, launchTailMarker, "the transcript is on screen") + assert.Contains(t, frame, "Test Model (high)", + "the status line names the model and the level it will send") + t.Logf("ready frame:\n%s", frame) + + // The band is the row under the transcript: it is blank until the + // reader is off the bottom, and the arrow then appears in it without + // the status line repeating the hint. + assert.NotContains(t, frame, bandScrollIcon) + + harness.write(sgrMouse(64, 10, 10, false)) // one wheel notch up + require.Eventually(t, func() bool { + return containsAll(harness.frame(80, 24), bandScrollIcon) + }, 30*time.Second, 10*time.Millisecond, "helper output: %s", harness.output.String()) + + scrolled := harness.frame(80, 24) + assert.Contains(t, scrolled, bandScrollIcon) + assert.NotContains(t, scrolled, "End for latest", "the status line does not repeat the band's hint") + t.Logf("scrolled frame:\n%s", scrolled) + + harness.write("\x0bquit\r") + result := harness.waitForExit() + require.NotNil(t, result) + require.Equal(t, 0, result.ExitCode(), "helper output: %s", harness.output.String()) + }) + } +} + +// runLaunchHelper is the child side: it starts the real TUI over the PTY with a +// model that declares a reasoning level, so the status line has one to name. +func runLaunchHelper(t *testing.T) { + t.Helper() + + high := config.ReasoningLevel("high") + controller, ok := reasoningController(t, reasoningModelConfig(), &high).(stubController) + require.True(t, ok) + + state := readyState() + state.Transcript = launchTranscript() + controller.state = state + + err := Run(context.Background(), Options{ + Input: os.Stdin, Output: os.Stdout, Environment: os.Environ(), + Workspace: t.TempDir(), Trusted: true, NoColor: true, + PinPresentation: true, Screen: ScreenFullscreen, AltScreen: AltScreenAlways, + MouseReporting: true, + ExitOutput: config.ExitOutputResumeHint, + Bootstrap: func(context.Context, bool) (Controller, error) { + return controller, nil + }, + }) + if err != nil { + _, _ = os.Stderr.WriteString(err.Error()) + os.Exit(1) + } + + os.Exit(0) +} + +// launchTailMarker is the last message of the launch transcript, so the check does +// not depend on where the viewport happens to sit in a longer conversation. +const launchTailMarker = "LAUNCH-REPLY-39" + +// launchTranscript is long enough that the frame cannot show all of it, which is +// what gives the reserved band an arrow to draw. +func launchTranscript() []ai.Message { + messages := []ai.Message{ai.UserText("LAUNCH-MARKER")} + for index := range 40 { + messages = append(messages, ai.AssistantText(fmt.Sprintf("LAUNCH-REPLY-%02d", index))) + } + + return messages +} + +func containsAll(value string, needles ...string) bool { + for _, needle := range needles { + if !strings.Contains(value, needle) { + return false + } + } + + return true +} diff --git a/internal/coding/tui/layout.go b/internal/coding/tui/layout.go index f010f881..1f841c24 100644 --- a/internal/coding/tui/layout.go +++ b/internal/coding/tui/layout.go @@ -7,7 +7,7 @@ import ( // The ready chat frame is composed from vertical bands, top to bottom: // -// transcript | prompt | composer | status +// transcript | prompt | band | composer | status // // Every band receives an explicit height before composition, and the composed // frame is bounded to the terminal height by clampFrame. The transcript is the @@ -16,6 +16,12 @@ import ( const ( // layoutStatusRows is the status band height; the status line never wraps. layoutStatusRows = 1 + // layoutBandRows is the reserved band between the activity line and the + // Composer. The band is one row on every frame — the transcript's elastic + // height is computed after it — so the Composer never moves when the band's + // contents change. It is the same row the frame used to leave blank there, + // which is why reserving it explicitly moves no existing frame. + layoutBandRows = 1 // layoutComposerBorderRows is the rounded border contributed by composerBox. layoutComposerBorderRows = 2 // layoutComposerMinRows keeps one editable row even in a short window. diff --git a/internal/coding/tui/markdown.go b/internal/coding/tui/markdown.go index ff87f7de..91fe2e97 100644 --- a/internal/coding/tui/markdown.go +++ b/internal/coding/tui/markdown.go @@ -9,6 +9,7 @@ import ( "fmt" "image/color" "strings" + "time" "unicode" "unicode/utf8" @@ -45,6 +46,10 @@ type markdownRenderer struct { bytes int maxBytes int live map[string]markdownKey + // liveFrames holds the last frame each live slot produced, so a slot whose + // tail is expensive can be served from the previous frame instead of paying + // for the whole un-frozen tail again on every tick. See reuseLiveFrame. + liveFrames map[string]liveFrame // frozen holds the assembled prefix of each live slot, so a streaming frame // extends it instead of rendering the whole body again. thinking is the same // state for the one live Thinking section, which is rendered as prose rather @@ -96,23 +101,22 @@ func (r *markdownRenderer) render( // A growing body misses the content cache on every frame, so the render is // assembled from a frozen prefix whenever a boundary allows it; the work then // scales with the live tail instead of with the whole body. See -// [markdown_live.go] for the boundary contract. +// [markdown_live.go] for the boundary contract, and [markdownRenderer.renderLiveRows] +// for the bound on what one un-freezable tail may cost per frame. func (r *markdownRenderer) renderLive( slot, content string, width int, theme colorTheme, noColor bool, ) (string, error) { - if rendered, ok := r.renderLiveIncremental(slot, content, width, theme, noColor); ok { - // The assembled frame is never a reusable cache entry: it changes every - // frame. Dropping the previous version still keeps an obsolete document - // out of the settled LRU when a fallback render stored one. - r.evictLive(slot) - - return rendered, nil + // The inline path and the managed store differ only in the indent they apply + // to each row, so both go through the same frame and share its cost bound. + rows, err := r.renderLiveRows(slot, content, width, 0, theme, noColor) + if err != nil { + return content, err } - return r.renderCached(slot, content, width, theme, noColor) + return strings.Join(rows.all(), "\n"), nil } // renderLiveRows is [markdownRenderer.renderLive] for the managed transcript @@ -121,11 +125,37 @@ func (r *markdownRenderer) renderLive( // again, and it returns the frozen rows and the tail as two segments so the store // never copies the whole live body's row headers into one slice. inset is the // frame-edge indent the store wants, applied to each row once. +// +// A live body whose tail cannot be frozen re-renders that whole tail every frame, +// so this is also where the cost of one such frame is bounded: see +// [markdownRenderer.reuseLiveFrame]. func (r *markdownRenderer) renderLiveRows( slot, content string, width, inset int, theme colorTheme, noColor bool, +) (rowSegments, error) { + if rows, ok := r.reuseLiveFrame(slot, content, width, inset, theme, noColor); ok { + return rows, nil + } + + started := time.Now() + rows, err := r.renderLiveRowsNow(slot, content, width, inset, theme, noColor) + if err != nil { + return rows, err + } + r.rememberLiveFrame(slot, content, width, inset, theme, noColor, rows, time.Since(started)) + + return rows, nil +} + +// renderLiveRowsNow assembles the frame for the current content, paying for the +// whole un-frozen tail. +func (r *markdownRenderer) renderLiveRowsNow( + slot, content string, + width, inset int, + theme colorTheme, + noColor bool, ) (rowSegments, error) { if rows, ok := r.renderLiveIncrementalRows(slot, content, width, inset, theme, noColor); ok { r.evictLive(slot) @@ -141,21 +171,136 @@ func (r *markdownRenderer) renderLiveRows( return rowSegments{frozen: insetRowSlice(splitTranscriptRows(rendered), inset)}, nil } -// renderLiveIncremental renders a growing live body from its frozen prefix. ok -// is false when no boundary may be frozen or a render failed, so the caller -// renders the body whole. -func (r *markdownRenderer) renderLiveIncremental( +// A liveFrame is what the last render of one live slot produced, with what it +// cost. The un-frozen tail of a live body cannot be frozen while it sits inside a +// code fence, a list, a blockquote or a table, so a streaming block re-renders its +// whole tail on every frame: the cost grows with the tail and the work is O(body²) +// over one message. +type liveFrame struct { + content string + rows rowSegments + width int + inset int + theme themeFingerprint + noColor bool + cost time.Duration + rendered time.Time +} + +const ( + // liveFrameFactor is how much longer than its own cost a live frame stands + // before the renderer pays for the tail again. + liveFrameFactor = 2 + // liveFrameBytes is how large an un-frozen tail has to be before its frame may + // be reused at all. Below it a frame costs a small fraction of one render tick, + // so rendering it every tick is both cheap and exact — which is what keeps a + // body that small byte-identical to a whole render on every frame. Above it the + // cost grows with the tail (measured: 1.4-2.7 µs per byte for an open fence, plus + // a few milliseconds of renderer setup), the frame's own cost decides how long it + // stands, and a frame may therefore be one cooldown behind the content it was + // asked for. + // + // The staleness is bounded and never corrupt: a reused frame is the whole render + // of the content it was rendered from, never a mix. What it is not is a valid + // frame for newer content whose structure changed — a fence that closed and + // reopened, say. Exactness comes back the moment the renderer pays for the tail + // again, which is the first frame after the cooldown lapses: a body that stops + // growing is therefore exact within one cooldown of its last render, and stays + // exact because the frame then holds that content (reuse needs strict growth). + // Measured on a 98 KiB open fence at the TUI's own 33 ms cadence: 7.6 repaints a + // second, worst observed lag 270 ms and 15 KiB of content behind. + liveFrameBytes = 8 << 10 + // maxLiveFrames bounds the frames kept for reuse. One live body is the normal + // case; the bound only stops a long session's settled slots from pinning their + // last tail. + maxLiveFrames = 8 +) + +// cooldown is how long a live frame stands. It is never shorter than one render +// frame, so a tail that fits inside the frame budget renders exactly as often as +// it did before, and it grows with the frame's own cost, so an expensive tail +// takes at most liveFrameFactor frames' worth of wall time for one render. +// Bounding that share is what keeps a pathological block from owning the event +// loop. +func (frame liveFrame) cooldown() time.Duration { + return max(renderFrame, liveFrameFactor*frame.cost) +} + +// reusable reports whether the frame is large enough to be worth reusing. The +// test is on the frame's own size rather than on a clock, so a body small enough +// to render exactly every frame always does. +func (frame liveFrame) reusable() bool { + return len(frame.content) >= liveFrameBytes +} + +// reuseLiveFrame reports whether the last frame this slot produced still stands. +// A frame may only stand for a strictly longer document that extends the one it +// was rendered for, so a new message, a rewound draft, a resize or a theme change +// always renders. The rows it returns are the frame's own, so the composed view +// stays internally consistent even though its tail is one render behind. +func (r *markdownRenderer) reuseLiveFrame( slot, content string, - width int, + width, inset int, theme colorTheme, noColor bool, -) (string, bool) { - rows, ok := r.renderLiveIncrementalRows(slot, content, width, 0, theme, noColor) - if !ok { - return "", false +) (rowSegments, bool) { + if slot == "" { + return rowSegments{}, false + } + frame, ok := r.liveFrames[slot] + if !ok || !frame.reusable() { + return rowSegments{}, false + } + if frame.width != width || frame.inset != inset || frame.noColor != noColor || + frame.theme != themeFingerprint(theme.fingerprint) { + return rowSegments{}, false + } + if len(content) <= len(frame.content) || !strings.HasPrefix(content, frame.content) { + return rowSegments{}, false + } + if time.Since(frame.rendered) >= frame.cooldown() { + return rowSegments{}, false } - return strings.Join(rows.all(), "\n"), true + return frame.rows, true +} + +func (r *markdownRenderer) rememberLiveFrame( + slot, content string, + width, inset int, + theme colorTheme, + noColor bool, + rows rowSegments, + cost time.Duration, +) { + if slot == "" { + return + } + if r.liveFrames == nil { + r.liveFrames = make(map[string]liveFrame, maxLiveFrames) + } + r.evictStaleLiveFrames() + r.liveFrames[slot] = liveFrame{ + content: content, rows: rows, width: width, inset: inset, + theme: themeFingerprint(theme.fingerprint), noColor: noColor, + cost: cost, rendered: time.Now(), + } +} + +// evictStaleLiveFrames keeps the reuse map bounded. A settled slot never asks +// again, so the least recently rendered frame is the first to go. +func (r *markdownRenderer) evictStaleLiveFrames() { + if len(r.liveFrames) < maxLiveFrames { + return + } + oldestSlot := "" + var oldest time.Time + for slot, frame := range r.liveFrames { + if oldestSlot == "" || frame.rendered.Before(oldest) { + oldestSlot, oldest = slot, frame.rendered + } + } + delete(r.liveFrames, oldestSlot) } // renderLiveIncrementalRows assembles one frame's rows from the frozen prefix @@ -261,7 +406,15 @@ func (r *markdownRenderer) renderUncached(content string, width int, theme color r.engineBuilds++ } + // glamour resolves the fence style through chroma's process-global, unlocked + // registry, so the read has to be ordered against pips's derived-style + // registrations (which a runtime theme load performs). ensureChromaStyle + // registers this theme's own style if a path that skips loadThemeRegistry + // built the snapshot; the read lock below is what makes the pair race-free. + ensureChromaStyle(theme) + chromaStyleRegistryMu.RLock() rendered, err := r.engine.Render(content) + chromaStyleRegistryMu.RUnlock() // Glamour's block-stack backing array retains popped buffers. Reuse only // flat, bounded paragraph engines; a large/nested document must release it. if len(content) > markdownEngineBytes || len(rendered) > markdownEngineBytes || !independentMarkdownDocument(content) { @@ -437,38 +590,54 @@ const ( chromaStyleBW = "bw" ) -// bundledChromaStyle names the chroma style a theme's syntax is drawn from, so -// the tokens follow the theme instead of the first render in the process. A theme -// the switch does not name, such as one loaded from a file, follows the family its -// background was resolved for. -func bundledChromaStyle(theme colorTheme) string { +// builtinFamilyChromaStyles is the bundled chroma style behind every built-in +// whose family ships one. Most ids share their name with the chroma style built +// for them; the rest are spelled out. An id missing from this table is a family +// chroma ships no style for, and such a theme gets a complete style derived from +// its own palette instead (see markdown_palette_style.go). +var builtinFamilyChromaStyles = map[string]string{ + themeIDDefaultDark: chromaStyleGitHubDark, + themeIDDefaultLight: chromaStyleGitHub, + themeIDGruvboxDark: chromaStyleGruvbox, + themeIDOneDark: chromaStyleOneDark, + themeIDTokyoNight: chromaStyleTokyoNightNight, + themeIDTokyoNightStorm: chromaStyleTokyoNightStorm, + themeIDTokyoNightLight: chromaStyleTokyoNightDay, + themeIDDracula: themeIDDracula, + themeIDNord: themeIDNord, + themeIDSolarizedLight: themeIDSolarizedLight, + themeIDSolarizedDark: themeIDSolarizedDark, + themeIDGruvboxLight: themeIDGruvboxLight, + themeIDModusOperandi: themeIDModusOperandi, + themeIDModusVivendi: themeIDModusVivendi, + themeIDCatppuccinLatte: themeIDCatppuccinLatte, themeIDCatppuccinFrappe: themeIDCatppuccinFrappe, + themeIDCatppuccinMacchiato: themeIDCatppuccinMacchiato, themeIDCatppuccinMocha: themeIDCatppuccinMocha, + themeIDRosePine: themeIDRosePine, themeIDRosePineMoon: themeIDRosePineMoon, + themeIDRosePineDawn: themeIDRosePineDawn, +} + +// familyChromaStyle names the bundled style that carries a built-in theme's +// family syntax tokens, and reports false when chroma ships no style for that +// family — which is exactly the set of themes pips derives a complete style for. +func familyChromaStyle(theme colorTheme) (string, bool) { if theme.borrowsTerminalColors() { // A theme with no polarity of its own cannot hand chroma absolute token // colours, so it names the bundled style that carries none. - return chromaStyleBW + return chromaStyleBW, true } + name, ok := builtinFamilyChromaStyles[theme.id] - switch theme.id { - case themeIDDefaultDark: - return chromaStyleGitHubDark - case themeIDDefaultLight: - return chromaStyleGitHub - case themeIDGruvboxDark: - return chromaStyleGruvbox - case themeIDOneDark: - return chromaStyleOneDark - case themeIDTokyoNight: - return chromaStyleTokyoNightNight - case themeIDTokyoNightStorm: - return chromaStyleTokyoNightStorm - case themeIDTokyoNightLight: - return chromaStyleTokyoNightDay - // These four share their name with the chroma style built for them. - case themeIDDracula, themeIDNord, themeIDSolarizedLight, - themeIDCatppuccinLatte, themeIDCatppuccinFrappe, themeIDCatppuccinMacchiato, themeIDCatppuccinMocha, - themeIDRosePine, themeIDRosePineMoon, themeIDRosePineDawn, - themeIDGruvboxLight, themeIDSolarizedDark, themeIDModusOperandi, themeIDModusVivendi: - return theme.id + return name, ok +} + +// bundledChromaStyle names the chroma style a theme's syntax is drawn from, so +// the tokens follow the theme instead of the first render in the process. It is +// the render path's last resort: every built-in resolves through its own family +// style or through a derived style, and every theme loaded from a file resolves +// through the style derived from its own palette. +func bundledChromaStyle(theme colorTheme) string { + if name, ok := familyChromaStyle(theme); ok { + return name } if theme.isLight() { diff --git a/internal/coding/tui/markdown_chroma.go b/internal/coding/tui/markdown_chroma.go index a880dc82..b757f858 100644 --- a/internal/coding/tui/markdown_chroma.go +++ b/internal/coding/tui/markdown_chroma.go @@ -3,25 +3,55 @@ package tui import ( "image/color" + "sync" "github.com/alecthomas/chroma/v2" "github.com/alecthomas/chroma/v2/styles" ) -// pipsChromaPrefix marks the styles pips derives from a theme palette, so a -// derived name can never collide with a bundled one. +// pipsChromaPrefix marks the styles pips derives, so a derived name can never +// collide with a bundled one. const pipsChromaPrefix = "pips-" +// paletteStyleChars is how much of a theme's canonical digest names its derived +// style. Sixteen hex characters are 64 bits, so an accidental collision needs on +// the order of 2^32 distinct palettes by the birthday bound — far out of reach for +// a directory of theme files — while the name stays short enough to read in a +// debugger. +const paletteStyleChars = 16 + // paletteDiffStyles maps a built-in theme id to the style that highlights its // fences. It is built in init because chroma's style registry is a plain map with -// no lock: every write has to happen before a renderer reads it, and a derived -// style must never be registered from a render path. +// no lock: every write has to be ordered against a read, and a derived style must +// never be registered from a render path. var paletteDiffStyles map[string]string +// chromaStyleRegistryMu orders every access to chroma's process-global, unlocked +// style registry. Writers are pips's derived-style registrations; readers are +// glamour's fence highlighting, which resolves the style name through the same +// registry. Every render takes the read side (markdownRenderer.renderUncached) +// and every registration takes the write side, so the map is never written while +// another goroutine reads it — which is what keeps the mechanism race-free even +// though a user theme is loaded and registered at runtime. +var chromaStyleRegistryMu sync.RWMutex + func init() { paletteDiffStyles = buildPaletteDiffStyles() + registerBuiltinPaletteStyles() } +// buildPaletteDiffStyles derives the diff-accent style of every built-in whose +// family ships a bundled chroma style. Such a style inherits the family's syntax +// tokens and replaces only the diff entries: the syntax colours are worth taking +// from the family's own style, because that style is drawn from the same palette, +// while the added and removed lines are not, because a bundled style either +// carries no diff entry at all or paints one with a fill of its own choosing, +// which is how a light theme ended up with bright #ddffdd bars inside a dark +// terminal. +// +// A built-in whose family ships no bundled style is skipped here: there is no +// syntax to inherit, so it gets a complete style derived from its own palette +// instead (see registerBuiltinPaletteStyles). func buildPaletteDiffStyles() map[string]string { names := make(map[string]string) @@ -32,9 +62,13 @@ func buildPaletteDiffStyles() map[string]string { // fixed token values; its fences stay with the colourless style. continue } + family, ok := familyChromaStyle(theme) + if !ok { + continue + } name := pipsChromaPrefix + theme.id - style, err := deriveDiffStyle(name, theme) + style, err := deriveDiffStyle(name, theme, family) if err != nil { continue } @@ -45,19 +79,31 @@ func buildPaletteDiffStyles() map[string]string { return names } +// registerBuiltinPaletteStyles derives a complete style for every built-in whose +// family ships no bundled chroma style. It runs in init, before any renderer can +// read the registry, which is why it needs no lock beyond the one the +// registration helper takes. +func registerBuiltinPaletteStyles() { + for _, entry := range builtinThemeEntries() { + theme := entry.theme + if theme.borrowsTerminalColors() { + continue + } + if _, ok := paletteDiffStyles[theme.id]; ok { + continue + } + registerPaletteChromaStyle(theme) + } +} + // deriveDiffStyle copies the theme's bundled syntax style and replaces its diff -// tokens. The syntax colours are worth taking from the family's own bundled -// style, because that style is drawn from the same palette; the added and removed -// lines are not, because a bundled style either carries no diff entry at all or -// paints one with a fill of its own choosing, which is how a light theme ended up -// with bright #ddffdd bars inside a dark terminal. -func deriveDiffStyle(name string, theme colorTheme) (*chroma.Style, error) { +// tokens. The builder inherits the bundled entries, so only the diff tokens +// change. They are rebuilt with noinherit, because otherwise the bundled entry's +// own fill would be inherited straight back into the token. +func deriveDiffStyle(name string, theme colorTheme, family string) (*chroma.Style, error) { palette := paletteFor(theme) - base := styles.Get(bundledChromaStyle(theme)) + base := styles.Get(family) - // The builder inherits the bundled entries, so only the diff tokens change. They - // are rebuilt with noinherit, because otherwise the bundled entry's own fill - // would be inherited straight back into the token. builder := base.Builder() builder.Add(chroma.GenericInserted, chromaToken(palette.change)+" noinherit") builder.Add(chroma.GenericDeleted, chromaToken(palette.error)+" noinherit") @@ -72,15 +118,103 @@ func deriveDiffStyle(name string, theme colorTheme) (*chroma.Style, error) { return style, nil } +// paletteChromaStyleName is the name of the complete style derived from a theme's +// palette. The name is the theme's own canonical digest, so two themes that share +// a palette but differ in id or name keep separate styles, and a theme whose +// palette changes never resolves the previous palette's style. +func paletteChromaStyleName(theme colorTheme) string { + return pipsChromaPrefix + theme.paletteStyle +} + +// registerPaletteChromaStyle makes sure the style derived from theme's palette is +// in chroma's registry and returns the name to highlight with. It is idempotent, +// so the common path (a theme already registered when its registry was built) +// costs one map read under the read lock. +// +// chroma's registry is a plain map with no lock of its own, so this write has to +// be ordered against every read; the read side is glamour's fence highlighting, +// which renderUncached runs under the same mutex. Two goroutines may therefore +// load or switch themes while a third renders fences, and neither the map nor a +// theme's colours can cross. +func registerPaletteChromaStyle(theme colorTheme) string { + name := paletteChromaStyleName(theme) + + chromaStyleRegistryMu.RLock() + _, registered := styles.Registry[name] + chromaStyleRegistryMu.RUnlock() + if registered { + return name + } + + chromaStyleRegistryMu.Lock() + defer chromaStyleRegistryMu.Unlock() + + if _, registered := styles.Registry[name]; registered { + return name + } + style, err := derivePaletteStyle(name, theme) + if err != nil { + // The derivation only fails on a malformed table entry, which the table + // test rules out. Falling back keeps a render working rather than + // crashing the TUI; the guard test fails instead. + return bundledChromaStyle(theme) + } + styles.Register(style) + + return name +} + +// ensureChromaStyle registers the palette-derived style a theme's fences need. +// A canvas-proof palette needs none, and neither does a built-in whose family +// ships a bundled style; every other theme does, so a snapshot that never passed +// through loadThemeRegistry (a fixture, or a theme built directly) still renders +// with its own palette instead of chroma's fallback. +func ensureChromaStyle(theme colorTheme) { + if theme.borrowsTerminalColors() { + return + } + if _, ok := paletteDiffStyles[theme.id]; ok { + return + } + registerPaletteChromaStyle(theme) +} + +// lookupChromaStyle reads chroma's registry under the same lock every write +// takes. Production reads happen inside renderUncached, next to glamour's own +// lookup; this accessor is for the callers that need the style object itself, +// such as the guards that pin the derived table. +func lookupChromaStyle(name string) *chroma.Style { + chromaStyleRegistryMu.RLock() + defer chromaStyleRegistryMu.RUnlock() + + return styles.Get(name) +} + +// chromaStyleRegistrySize reports how many styles chroma's global registry holds. +// It is the instrument for the guard that a render never grows the registry; the +// read takes the same lock the registrations take, so it cannot race with a +// concurrent theme load. +func chromaStyleRegistrySize() int { + chromaStyleRegistryMu.RLock() + defer chromaStyleRegistryMu.RUnlock() + + return len(styles.Registry) +} + // markdownChromaTheme names the style that highlights a theme's fenced code: the -// derived style, which carries the theme's own diff accents, for a built-in, and -// the family's bundled style for a palette that arrived from a file. +// family-inheriting style for a built-in whose family ships one, the complete +// palette-derived style for a built-in whose family does not, and the complete +// palette-derived style for a palette that arrived from a file. A palette that +// borrows the terminal's own colours keeps the bundled style that carries none. func markdownChromaTheme(theme colorTheme) string { if name, ok := paletteDiffStyles[theme.id]; ok { return name } + if theme.borrowsTerminalColors() { + return chromaStyleBW + } - return bundledChromaStyle(theme) + return paletteChromaStyleName(theme) } // chromaToken renders a palette colour as a chroma style entry. It carries no diff --git a/internal/coding/tui/markdown_live_budget_test.go b/internal/coding/tui/markdown_live_budget_test.go new file mode 100644 index 00000000..d2abf4d9 --- /dev/null +++ b/internal/coding/tui/markdown_live_budget_test.go @@ -0,0 +1,251 @@ +//nolint:wsl_v5 // The bound's cases and their measurements stay adjacent. +package tui + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// liveFrameProbeBody is a block large enough to be worth reusing and shaped so a +// streamed prefix of it cannot be frozen: an open code fence, which is exactly +// the case that makes a live frame's cost grow with the tail. +func liveFrameProbeBody() string { + var body strings.Builder + body.WriteString("```go\n") + for line := range 700 { + fmt.Fprintf(&body, "value%d := %d\n", line, line) + } + + return body.String() +} + +// TestLiveFrameStandsForItsOwnCooldown pins the bound on what one un-freezable +// live tail may cost per frame. A frame whose render is expensive stands for a +// cooldown proportional to that cost, so a streaming block cannot own the event +// loop; the renderer pays for the tail again once the cooldown lapses. +func TestLiveFrameStandsForItsOwnCooldown(t *testing.T) { + t.Parallel() + + renderer := newMarkdownRenderer(4) + body := liveFrameProbeBody() + require.GreaterOrEqual(t, len(body), liveFrameBytes) + + first, err := renderer.renderLiveRows("draft", body, 80, 0, themeDark, false) + require.NoError(t, err) + require.NotEmpty(t, first.all()) + frame := renderer.liveFrames["draft"] + require.True(t, frame.reusable()) + require.Positive(t, frame.cost) + + rendered := renderer.renderedBytes + + // A strictly longer body that extends the frame's own content is served from + // the frame while the cooldown holds. + grown := body + "value700 := 700\n" + second, err := renderer.renderLiveRows("draft", grown, 80, 0, themeDark, false) + require.NoError(t, err) + assert.Equal(t, first.all(), second.all(), "the frame stands for its cooldown") + assert.Equal(t, rendered, renderer.renderedBytes, "the second frame paid for no render") + + // Once the cooldown has lapsed the renderer pays for the tail again, and the + // frame it produces is the current content's. + renderer.liveFrames["draft"] = liveFrame{ + content: body, rows: first, width: 80, inset: 0, + theme: themeFingerprint(themeDark.fingerprint), cost: frame.cost, + } + expired := renderer.liveFrames["draft"] + expired.rendered = frame.rendered.Add(-frame.cooldown() - 1) + renderer.liveFrames["draft"] = expired + + third, err := renderer.renderLiveRows("draft", grown, 80, 0, themeDark, false) + require.NoError(t, err) + assert.Greater(t, renderer.renderedBytes, rendered, "the lapsed cooldown pays for the tail again") + assert.Equal(t, wholeRenderRows(t, grown), third.all(), + "and the frame it produces is the current content's, exactly") + + // A body that has stopped growing is exact once its own frame's cooldown lapses, + // and stays exact: this is the half that keeps a turn's last rows from staying one + // render behind for good. + final := grown + "trailing := 1\n" + if _, err := renderer.renderLiveRows("draft", final, 80, 0, themeDark, false); err != nil { + t.Fatal(err) + } + settled := renderer.liveFrames["draft"] + settled.rendered = settled.rendered.Add(-settled.cooldown() - 1) + renderer.liveFrames["draft"] = settled + + exact, err := renderer.renderLiveRows("draft", final, 80, 0, themeDark, false) + require.NoError(t, err) + assert.Equal(t, wholeRenderRows(t, final), exact.all(), "a settled body is exact") +} + +// wholeRenderRows is the rows a renderer that holds no frame for the body produces, +// which is what a live frame has to equal once it pays for the tail. +func wholeRenderRows(t *testing.T, body string) []string { + t.Helper() + + rows, err := newMarkdownRenderer(1).renderLiveRows("draft", body, 80, 0, themeDark, false) + require.NoError(t, err) + + return rows.all() +} + +// TestLiveFrameRendersWhenItCannotStand pins the invalidation: a frame only +// stands for a strictly longer document that extends it, at the same geometry, +// theme and colour mode. Each case must both pay for a render and produce the +// current content's frame. +func TestLiveFrameRendersWhenItCannotStand(t *testing.T) { + t.Parallel() + + body := liveFrameProbeBody() + grown := body + "value700 := 700\n" + replaced := strings.Repeat("replacement paragraph\n\n", 500) + + cases := map[string]func(*markdownRenderer) (rowSegments, error){ + "a replaced body": func(r *markdownRenderer) (rowSegments, error) { + return r.renderLiveRows("draft", replaced, 80, 0, themeDark, false) + }, + "a rewound body": func(r *markdownRenderer) (rowSegments, error) { + return r.renderLiveRows("draft", body[:len(body)/2], 80, 0, themeDark, false) + }, + "another width": func(r *markdownRenderer) (rowSegments, error) { + return r.renderLiveRows("draft", grown, 100, 0, themeDark, false) + }, + "another inset": func(r *markdownRenderer) (rowSegments, error) { + return r.renderLiveRows("draft", grown, 80, 2, themeDark, false) + }, + "another theme": func(r *markdownRenderer) (rowSegments, error) { + return r.renderLiveRows("draft", grown, 80, 0, themeLight, false) + }, + "colour mode": func(r *markdownRenderer) (rowSegments, error) { + return r.renderLiveRows("draft", grown, 80, 0, themeDark, true) + }, + "another slot": func(r *markdownRenderer) (rowSegments, error) { + return r.renderLiveRows("other", grown, 80, 0, themeDark, false) + }, + } + + for name, render := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + renderer := newMarkdownRenderer(4) + _, err := renderer.renderLiveRows("draft", body, 80, 0, themeDark, false) + require.NoError(t, err) + + rendered := renderer.renderedBytes + got, err := render(renderer) + require.NoError(t, err) + assert.Greater(t, renderer.renderedBytes, rendered, "the frame could not stand") + + fresh, err := render(newMarkdownRenderer(4)) + require.NoError(t, err) + assert.Equal(t, fresh.all(), got.all(), "the frame is the current content's") + }) + } +} + +// TestLiveFrameNeedsAStrictExtension pins the two conditions a frame's content +// must meet: the new body has to be longer, and it has to start with the frame's +// own body. +func TestLiveFrameNeedsAStrictExtension(t *testing.T) { + t.Parallel() + + renderer := newMarkdownRenderer(4) + body := liveFrameProbeBody() + _, err := renderer.renderLiveRows("draft", body, 80, 0, themeDark, false) + require.NoError(t, err) + + _, unchanged := renderer.reuseLiveFrame("draft", body, 80, 0, themeDark, false) + assert.False(t, unchanged, "an unchanged body is not served from the frame") + + _, shorter := renderer.reuseLiveFrame("draft", body[:len(body)/2], 80, 0, themeDark, false) + assert.False(t, shorter, "a shorter body is not served from the frame") + + _, unrelated := renderer.reuseLiveFrame("draft", "different\n"+body, 80, 0, themeDark, false) + assert.False(t, unrelated, "a body that does not extend the frame's is not served from it") + + _, extended := renderer.reuseLiveFrame("draft", body+"value700 := 700\n", 80, 0, themeDark, false) + assert.True(t, extended, "a strictly longer body that extends the frame's is served from it") + + _, unnamed := renderer.reuseLiveFrame("", body+"value700 := 700\n", 80, 0, themeDark, false) + assert.False(t, unnamed, "a frame with no slot is never reused") +} + +// TestLiveFrameReuseIsBounded keeps the reuse map from pinning a long session's +// settled slots. +func TestLiveFrameReuseIsBounded(t *testing.T) { + t.Parallel() + + renderer := newMarkdownRenderer(4) + body := liveFrameProbeBody() + + for slot := range maxLiveFrames + 3 { + _, err := renderer.renderLiveRows(fmt.Sprintf("slot-%d", slot), body, 80, 0, themeDark, false) + require.NoError(t, err) + } + + assert.Len(t, renderer.liveFrames, maxLiveFrames) + _, oldest := renderer.liveFrames["slot-0"] + assert.False(t, oldest, "the least recently rendered frame is the first to go") +} + +// TestReusedLiveFrameIsAWholeRenderOfItsOwnContent pins what the bound actually +// promises. A frame the renderer stands on is the whole render of the content it +// was rendered from — never an old prefix spliced onto a new tail — so the reader +// sees a frame that is one render behind rather than a frame that never existed. +// The frame is NOT a valid frame for the newer content once the structure changed +// (here a fence that closes, prose, and a fence that reopens), which is why the +// settled frame is the one that has to be exact; TestLiveFrameNeedsAStrictExtension +// and TestSmallLiveBodiesRenderExactlyEveryFrame pin that half. +func TestReusedLiveFrameIsAWholeRenderOfItsOwnContent(t *testing.T) { + t.Parallel() + + renderer := newMarkdownRenderer(4) + body := liveFrameProbeBody() + first, err := renderer.renderLiveRows("draft", body, 80, 0, themeDark, false) + require.NoError(t, err) + + grown := body + "```\n\nprose after the fence.\n\n```go\nreopened := 1\n" + second, err := renderer.renderLiveRows("draft", grown, 80, 0, themeDark, false) + require.NoError(t, err) + require.Equal(t, first.all(), second.all(), "the frame stands") + + fresh, err := newMarkdownRenderer(4).renderLiveRows("draft", body, 80, 0, themeDark, false) + require.NoError(t, err) + assert.Equal(t, fresh.all(), second.all(), + "the frame is the old content's whole render, not a spliced mix") + + whole, err := newMarkdownRenderer(1).render(grown, 80, themeDark, false) + require.NoError(t, err) + assert.NotEqual(t, whole, strings.Join(second.all(), "\n"), + "the frame is one render behind the content it was asked for") +} + +// TestSmallLiveBodiesRenderExactlyEveryFrame pins the other side of the bound: a +// body below the reuse threshold is rendered every time, so the streamed output +// stays byte-identical to a whole render. +func TestSmallLiveBodiesRenderExactlyEveryFrame(t *testing.T) { + t.Parallel() + + document := "Intro paragraph.\n\nSecond paragraph with `code`.\n\nTail.\n" + streaming := newMarkdownRenderer(4) + + for size := range len(document) + 1 { + content := document[:size] + got, err := streaming.renderLive("draft", content, 40, themeDark, false) + require.NoError(t, err) + + want, err := newMarkdownRenderer(1).render(content, 40, themeDark, false) + require.NoError(t, err) + assert.Equal(t, want, got, "size %d", size) + } + + assert.Less(t, len(document), liveFrameBytes) + assert.False(t, streaming.liveFrames["draft"].reusable(), + "a body this small is never served from a previous frame") +} diff --git a/internal/coding/tui/markdown_palette_style.go b/internal/coding/tui/markdown_palette_style.go new file mode 100644 index 00000000..441b026f --- /dev/null +++ b/internal/coding/tui/markdown_palette_style.go @@ -0,0 +1,229 @@ +//nolint:wsl_v5 // The role table and the derivation that consumes it stay adjacent. +package tui + +import ( + "image/color" + + "github.com/alecthomas/chroma/v2" +) + +// A theme whose family ships no bundled chroma style, and every theme loaded +// from a file, gets a complete syntax style derived from its own 14-role +// palette. This file is that derivation: one table maps every chroma token class +// to exactly one palette role, and [derivePaletteStyle] turns the table into a +// self-contained chroma style (no parent, so nothing leaks in from a family +// style the theme does not belong to). +// +// Roles the table deliberately never uses: +// +// - codeBackground is a surface. A fence draws on the terminal's own canvas, +// so painting a fill would break the canvas contract the theme system pins +// (and would colour a light palette's fence inside a dark terminal). +// - separator and composerPrompt are chrome, not code. +// - diagnostic is the status panel's own channel, and reusing it in a fence +// would make a diagnostic colour mean two different things. +// +// Every entry carries its reason, and TestPaletteSyntaxTableIsCompleteAndJustified +// keeps the table complete against the token classes the derivation must cover. +// +// Readability is checked where a fence is actually painted. A fence draws on the +// terminal's own canvas and paints no fill, so a token's contrast is decided by +// the terminal's background, which pips does not know; the palette's accents are +// therefore tuned against the canvas each family was designed for. +// TestBuiltinThemeRepresentativeContrast checks the two pairings that do share +// `code_background` — the fence body and the inline-code pill, both of which are +// the `code`/`workspace` foreground. Measuring the accents against +// `code_background` instead flags most of the shipped catalogue (dracula's muted +// is 1.94:1 there, catppuccin-latte's active 1.70:1), which is a property of each +// family's own palette rather than of this derivation. + +// paletteSyntaxRole is the palette role a chroma token class draws its colour +// from. The zero value is the body-text role, so an entry that forgets a role +// still renders readable text rather than an unset colour. +type paletteSyntaxRole uint8 + +const ( + syntaxWorkspace paletteSyntaxRole = iota + syntaxMuted + syntaxModel + syntaxSession + syntaxChange + syntaxActive + syntaxWarning + syntaxError + syntaxCode +) + +func (role paletteSyntaxRole) colour(palette colorPalette) color.Color { + switch role { + case syntaxMuted: + return palette.muted + case syntaxModel: + return palette.model + case syntaxSession: + return palette.session + case syntaxChange: + return palette.change + case syntaxActive: + return palette.active + case syntaxWarning: + return palette.warning + case syntaxError: + return palette.error + case syntaxCode: + return palette.code + default: + return palette.workspace + } +} + +// paletteSyntaxToken is one row of the role -> token-class mapping. +type paletteSyntaxToken struct { + token chroma.TokenType + role paletteSyntaxRole + modifiers string + reason string +} + +// paletteSyntaxTokens is the complete mapping, ordered by token class so the +// table reads like the chroma token tree. The colour semantics it follows: +// +// - workspace is the transcript's own foreground, so plain code text, names +// and identifiers read as body text rather than as decoration. +// - muted carries everything the reader should skim past: comments, +// punctuation, string delimiters and subordinate headings. +// - model is the primary accent (headings already use it), so control-flow +// words, types and markup structure read as structure. +// - session is the secondary accent (links and the composer cursor already use +// it), so callables, builtins and prompts read as actions. +// - change is the value/positive accent, so strings and added diff lines read +// as values. +// - active is the attention accent, so numbers and constants read as literals. +// - warning is the caution accent, so escapes, decorators and entities pop out +// of the token they sit inside without reading as an error. +// - error is reserved for failures: exceptions, lexer errors, tracebacks and +// removed diff lines. +var paletteSyntaxTokens = []paletteSyntaxToken{ + {chroma.Background, syntaxWorkspace, "", "the fence inherits the terminal's canvas, so the style carries no fill"}, + {chroma.Text, syntaxWorkspace, "", "plain code text keeps the transcript's own foreground"}, + {chroma.Error, syntaxError, "", "input that could not be tokenised is a failure"}, + {chroma.Other, syntaxWorkspace, "", "an unclassified token is plain text"}, + + {chroma.Keyword, syntaxModel, "", "control-flow words are the primary accent, the one headings already use"}, + {chroma.KeywordConstant, syntaxModel, "", "a keyword-shaped literal stays with the other keywords"}, + {chroma.KeywordDeclaration, syntaxModel, "", "declaration keywords are keywords"}, + {chroma.KeywordNamespace, syntaxModel, "", "import and module keywords are keywords"}, + {chroma.KeywordPseudo, syntaxModel, "", "pseudo keywords are keywords"}, + {chroma.KeywordReserved, syntaxModel, "", "reserved words are keywords"}, + {chroma.KeywordType, syntaxModel, "", "type names read as structure, like the headings that share the accent"}, + + {chroma.Name, syntaxWorkspace, "", "a bare identifier is body text"}, + {chroma.NameAttribute, syntaxSession, "", "an attribute is reached through an action accent, like a call"}, + {chroma.NameBuiltin, syntaxSession, "", "a builtin is an action the language provides"}, + {chroma.NameBuiltinPseudo, syntaxSession, "", "self and cls are builtins"}, + {chroma.NameClass, syntaxModel, "", "a class is a type, and types read as structure"}, + {chroma.NameConstant, syntaxActive, "", "a named constant is a literal value"}, + {chroma.NameDecorator, syntaxWarning, "", "a decorator changes the meaning of what follows, so it must pop out"}, + {chroma.NameEntity, syntaxWarning, "", "an entity is a stand-in for a character, so it must pop out of its text"}, + {chroma.NameException, syntaxError, "", "an exception name is a failure"}, + {chroma.NameFunction, syntaxSession, "", "a call is an action"}, + {chroma.NameFunctionMagic, syntaxSession, "", "a magic call is still a call"}, + {chroma.NameKeyword, syntaxModel, "", "a name that acts as a keyword stays with the keywords"}, + {chroma.NameLabel, syntaxSession, "", "a label is a jump target, which reads as an action"}, + {chroma.NameNamespace, syntaxModel, "", "a namespace is structure"}, + {chroma.NameOperator, syntaxSession, "", "a word-shaped operator reads like the operator it stands for"}, + {chroma.NameOther, syntaxWorkspace, "", "an unclassified name is body text"}, + {chroma.NamePseudo, syntaxSession, "", "a pseudo name is a builtin-like action"}, + {chroma.NameProperty, syntaxWorkspace, "", "a property is a name, and names are body text"}, + {chroma.NameTag, syntaxModel, "", "a markup tag is structure"}, + {chroma.NameVariable, syntaxWorkspace, "", "a variable is a name, and names are body text"}, + {chroma.NameVariableAnonymous, syntaxWorkspace, "", "an anonymous variable is still a name"}, + {chroma.NameVariableClass, syntaxWorkspace, "", "a class variable is still a name"}, + {chroma.NameVariableGlobal, syntaxWorkspace, "", "a global variable is still a name"}, + {chroma.NameVariableInstance, syntaxWorkspace, "", "an instance variable is still a name"}, + {chroma.NameVariableMagic, syntaxSession, "", "a magic variable behaves like a builtin"}, + + {chroma.Literal, syntaxChange, "", "a bare literal is a value"}, + {chroma.LiteralDate, syntaxActive, "", "a date literal is a literal value"}, + {chroma.LiteralOther, syntaxChange, "", "an unclassified literal is a value"}, + + {chroma.LiteralString, syntaxChange, "", "a string is a value, which is what the positive accent means"}, + {chroma.LiteralStringAffix, syntaxChange, "", "a string affix belongs to its string"}, + {chroma.LiteralStringAtom, syntaxChange, "", "an atom is a string-like value"}, + {chroma.LiteralStringBacktick, syntaxChange, "", "a backtick string is a string"}, + {chroma.LiteralStringBoolean, syntaxActive, "", "a boolean literal is a literal value, not text"}, + {chroma.LiteralStringChar, syntaxChange, "", "a character literal is a string"}, + {chroma.LiteralStringDelimiter, syntaxMuted, "", "the quotes are the least informative part of a string"}, + {chroma.LiteralStringDoc, syntaxChange, "", "a docstring is a string, not a comment"}, + {chroma.LiteralStringDouble, syntaxChange, "", "a double-quoted string is a string"}, + {chroma.LiteralStringEscape, syntaxWarning, "", "an escape changes the meaning of the string, so it must pop out"}, + {chroma.LiteralStringHeredoc, syntaxChange, "", "a heredoc is a string"}, + {chroma.LiteralStringInterpol, syntaxWarning, "", "an interpolation is code inside a string, so it must pop out"}, + {chroma.LiteralStringName, syntaxChange, "", "a named string is a string"}, + {chroma.LiteralStringOther, syntaxChange, "", "an unclassified string is a string"}, + {chroma.LiteralStringRegex, syntaxActive, "", "a regex literal is a literal value with its own syntax"}, + {chroma.LiteralStringSingle, syntaxChange, "", "a single-quoted string is a string"}, + {chroma.LiteralStringSymbol, syntaxChange, "", "a symbol literal is a value"}, + + {chroma.LiteralNumber, syntaxActive, "", "a number is the literal the reader looks for"}, + {chroma.LiteralNumberBin, syntaxActive, "", "a binary number is a number"}, + {chroma.LiteralNumberFloat, syntaxActive, "", "a float is a number"}, + {chroma.LiteralNumberHex, syntaxActive, "", "a hex number is a number"}, + {chroma.LiteralNumberInteger, syntaxActive, "", "an integer is a number"}, + {chroma.LiteralNumberIntegerLong, syntaxActive, "", "a long integer is a number"}, + {chroma.LiteralNumberOct, syntaxActive, "", "an octal number is a number"}, + + {chroma.Operator, syntaxSession, "", "an operator joins values, which is the secondary accent's job"}, + {chroma.OperatorWord, syntaxModel, "", "a word-shaped operator reads as a keyword"}, + + {chroma.Punctuation, syntaxMuted, "", "separators carry little meaning, so they recede"}, + + {chroma.Comment, syntaxMuted, "", "a comment is deliberately de-emphasised"}, + {chroma.CommentHashbang, syntaxMuted, "", "a hashbang is a comment"}, + {chroma.CommentMultiline, syntaxMuted, "", "a block comment is a comment"}, + {chroma.CommentSingle, syntaxMuted, "", "a line comment is a comment"}, + {chroma.CommentSpecial, syntaxModel, "", "a special comment is a directive, so it reads as structure"}, + + {chroma.CommentPreproc, syntaxModel, "", "a preprocessor directive is structure, not prose"}, + {chroma.CommentPreprocFile, syntaxChange, "", "the file a directive names is a value"}, + + {chroma.Generic, syntaxWorkspace, "", "an unclassified generic token is body text"}, + {chroma.GenericDeleted, syntaxError, "", "a removed diff line is a failure to keep"}, + {chroma.GenericEmph, syntaxWorkspace, "italic", "emphasis is an attribute, so it keeps the body colour"}, + {chroma.GenericError, syntaxError, "", "a generic error is a failure"}, + {chroma.GenericHeading, syntaxModel, "bold", "a heading is structure and leads its section"}, + {chroma.GenericInserted, syntaxChange, "", "an added diff line is a value gained"}, + {chroma.GenericOutput, syntaxWorkspace, "", "program output is plain text"}, + {chroma.GenericPrompt, syntaxSession, "", "a prompt is the marker of an action"}, + {chroma.GenericStrong, syntaxWorkspace, "bold", "strong text is an attribute, so it keeps the body colour"}, + {chroma.GenericSubheading, syntaxMuted, "bold", "a subordinate heading is structure that should not compete with its parent"}, + {chroma.GenericTraceback, syntaxError, "", "a traceback is a failure"}, + {chroma.GenericUnderline, syntaxWorkspace, "underline", "underlined text is an attribute, so it keeps the body colour"}, + + {chroma.TextWhitespace, syntaxWorkspace, "", "whitespace is invisible, so its colour only has to be harmless"}, + {chroma.TextSymbol, syntaxWarning, "", "a symbol stands in for something else, so it must pop out"}, + {chroma.TextPunctuation, syntaxMuted, "", "text punctuation recedes like code punctuation"}, +} + +// derivePaletteStyle builds a complete, self-contained chroma style from a +// theme's palette. It is a pure function of the palette and the table above, so +// it can be tested without a renderer or a registry, and two calls for the same +// palette produce equal styles. +func derivePaletteStyle(name string, theme colorTheme) (*chroma.Style, error) { + palette := paletteFor(theme) + entries := make(chroma.StyleEntries, len(paletteSyntaxTokens)) + for _, row := range paletteSyntaxTokens { + entry := colorString(row.role.colour(palette)) + if row.modifiers != "" { + entry += " " + row.modifiers + } + entries[row.token] = entry + } + + style, err := chroma.NewStyle(name, entries) + if err != nil { + return nil, err + } + + return style, nil +} diff --git a/internal/coding/tui/markdown_palette_style_test.go b/internal/coding/tui/markdown_palette_style_test.go new file mode 100644 index 00000000..008a23b0 --- /dev/null +++ b/internal/coding/tui/markdown_palette_style_test.go @@ -0,0 +1,487 @@ +//nolint:wsl_v5 // Each guard states its expectation next to the call it drives. +package tui + +import ( + "image/color" + "os" + "regexp" + "slices" + "strings" + "sync" + "testing" + + "github.com/alecthomas/chroma/v2" + "github.com/alecthomas/chroma/v2/formatters" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestPaletteSyntaxTableIsCompleteAndJustified keeps the documented role -> +// token table honest. The derivation is only as complete as this table, and a +// row without a reason would make the mapping undocumented the moment it lands. +func TestPaletteSyntaxTableIsCompleteAndJustified(t *testing.T) { + t.Parallel() + + // The token classes the derivation must cover: the plan's representative set + // plus the categories the indexed formatter falls back to for anything else. + required := []chroma.TokenType{ + chroma.Background, chroma.Text, chroma.Error, + chroma.Keyword, chroma.Name, chroma.NameClass, chroma.NameFunction, chroma.NameBuiltin, + chroma.Literal, chroma.LiteralString, chroma.LiteralNumber, chroma.Operator, + chroma.Punctuation, chroma.Comment, chroma.CommentPreproc, chroma.Generic, + chroma.GenericInserted, chroma.GenericDeleted, chroma.GenericSubheading, + chroma.GenericHeading, chroma.GenericError, chroma.TextPunctuation, + } + + seen := make(map[chroma.TokenType]bool, len(paletteSyntaxTokens)) + roles := make(map[paletteSyntaxRole]bool) + for _, row := range paletteSyntaxTokens { + assert.False(t, seen[row.token], "%v is mapped once", row.token) + seen[row.token] = true + assert.NotEmpty(t, row.reason, "%v documents why it takes its role", row.token) + roles[row.role] = true + } + for _, token := range required { + assert.True(t, seen[token], "%v is covered by the derivation", token) + } + + // A fence draws with eight of the nine syntax roles; the chrome roles + // (separator, composerPrompt, codeBackground, diagnostic) stay out, so a + // fence never paints a surface or borrows the status panel's channel. + assert.Len(t, roles, 8, "the syntax table draws on eight roles and leaves the chrome ones alone") + for _, role := range []paletteSyntaxRole{ + syntaxMuted, syntaxModel, syntaxSession, + syntaxChange, syntaxActive, syntaxWarning, syntaxError, + } { + assert.True(t, roles[role], "role %d is used by the syntax table", role) + } + + // The zero role is the body-text role, so a row that forgets a role still + // renders readable text rather than an unset colour. + assert.Equal(t, syntaxWorkspace, paletteSyntaxRole(0)) + assert.Equal(t, syntaxCode, paletteSyntaxRole(8)) +} + +// TestMarkdownRenderRegistersNothingIntoTheChromaRegistry pins the timing claim: +// once the built-ins are registered at init, a render only reads. The test is +// deliberately not parallel, because it compares a process-global map's size +// across two moments. +// +//nolint:paralleltest // The size comparison is meaningless next to a concurrent registration. +func TestMarkdownRenderRegistersNothingIntoTheChromaRegistry(t *testing.T) { + before := chromaStyleRegistrySize() + + renderer := newMarkdownRenderer(4) + for _, entry := range builtinThemeEntries() { + _, err := renderer.render("```go\nfunc main() { x := 1 }\n```\n", 40, entry.theme, false) + require.NoError(t, err) + _, err = renderer.render("```diff\n+added\n-removed\n```\n", 40, entry.theme, false) + require.NoError(t, err) + } + + assert.Equal(t, before, chromaStyleRegistrySize(), "a render registers no style") + + // A theme built directly (no registry load) is registered on first render, + // and only once: the later renders find it already there. + direct := newResolvedTheme("direct", "Direct", themeBackgroundDark, themeDark.palette) + after := chromaStyleRegistrySize() + for range 3 { + _, err := renderer.render("```go\nfunc main() {}\n```\n", 40, direct, false) + require.NoError(t, err) + } + assert.Equal(t, after+1, chromaStyleRegistrySize(), + "a snapshot that skipped the registry load is registered exactly once") + assert.Equal(t, paletteChromaStyleName(direct), markdownChromaTheme(direct)) +} + +// TestMarkdownChromaRegistrationIsRaceFree renders fences in parallel while other +// goroutines build theme registries from files, which registers a derived style +// per theme. Two of the files carry identical palettes and different ids. Run +// under -race: the whole point of the mechanism is that chroma's unlocked +// registry is never written while it is read. +func TestMarkdownChromaRegistrationIsRaceFree(t *testing.T) { + t.Parallel() + + directory := t.TempDir() + require.NoError(t, os.Chmod(directory, 0o700)) //nolint:gosec // The loader only accepts a private theme directory. + shared := ` +[palette] +model = "#FF00AA" +change = "#00C8A0" +` + for _, id := range []string{"alpha", "beta"} { + writeThemeFile(t, directory, id, `schema = "pips.tui.theme/v1alpha1" +name = "`+strings.ToUpper(id)+`" +inherits = "nord" +`+shared) + } + writeThemeFile(t, directory, "gamma", `schema = "pips.tui.theme/v1alpha1" +name = "Gamma" +inherits = "solarized-light" + +[palette] +model = "#123456" +`) + + const fence = "```go\nfunc main() { x := 1 }\n```\n" + + themes := make(chan colorTheme, 256) + var loaders, renderers sync.WaitGroup + for range 4 { + loaders.Go(func() { + for range 12 { + registry := loadThemeRegistry(directory) + for _, entry := range registry.Entries() { + themes <- entry.theme + } + } + }) + } + for range 4 { + renderers.Go(func() { + renderer := newMarkdownRenderer(8) + for _, entry := range builtinThemeEntries() { + if entry.theme.borrowsTerminalColors() { + continue + } + for range 2 { + rendered, err := renderer.render(fence, 40, entry.theme, false) + //nolint:testifylint // require would fail from a goroutine; the assertions are collected by the test. + assert.NoError(t, err) + assert.Contains(t, rendered, "main") + } + } + for theme := range themes { + if theme.borrowsTerminalColors() { + continue + } + name := markdownChromaTheme(theme) + assert.True(t, strings.HasPrefix(name, pipsChromaPrefix), theme.id) + assert.NotNil(t, lookupChromaStyle(name), theme.id) + rendered, err := renderer.render(fence, 40, theme, false) + //nolint:testifylint // require would fail from a goroutine; the assertions are collected by the test. + assert.NoError(t, err) + assert.Contains(t, rendered, "main") + } + }) + } + loaders.Wait() + close(themes) + renderers.Wait() + + // Two files that share a palette still resolve their own style, and each + // carries its own palette rather than a family fallback. + registry := loadThemeRegistry(directory) + alpha := mustTheme(t, registry, "alpha") + beta := mustTheme(t, registry, "beta") + gamma := mustTheme(t, registry, "gamma") + assert.NotEqual(t, markdownChromaTheme(alpha), markdownChromaTheme(beta), + "same palette, different identity, separate styles") + for _, theme := range []colorTheme{alpha, beta} { + keyword := lookupChromaStyle(markdownChromaTheme(theme)).Get(chroma.Keyword) + assert.Equal(t, chroma.MustParseColour("#FF00AA"), keyword.Colour, theme.id) + } + assert.Equal(t, chroma.MustParseColour("#123456"), + lookupChromaStyle(markdownChromaTheme(gamma)).Get(chroma.Keyword).Colour) + assert.NotEqual(t, markdownChromaTheme(gamma), markdownChromaTheme(alpha)) +} + +// paletteEscapeProbe is one token class read back out of a rendered fence. +type paletteEscapeProbe struct { + name string + token chroma.TokenType + fence string + needle string +} + +// paletteEscapeProbes are the plan's representative token classes, each reached +// through a real lexer: Go covers the syntax classes, Python the class name, and +// a diff hunk header the subheading. +var paletteEscapeProbes = []paletteEscapeProbe{ + {name: "Keyword", token: chroma.Keyword, fence: paletteGoFence, needle: "func"}, + {name: "NameFunction", token: chroma.NameFunction, fence: paletteGoFence, needle: "main"}, + {name: "LiteralString", token: chroma.LiteralString, fence: paletteGoFence, needle: `"hi"`}, + {name: "LiteralNumber", token: chroma.LiteralNumber, fence: paletteGoFence, needle: "42"}, + {name: "Comment", token: chroma.Comment, fence: paletteGoFence, needle: "// note"}, + {name: "Operator", token: chroma.Operator, fence: paletteGoFence, needle: ":="}, + {name: "NameClass", token: chroma.NameClass, fence: palettePythonFence, needle: "Widget"}, + {name: "GenericSubheading", token: chroma.GenericSubheading, fence: paletteDiffFence, needle: "@@"}, + {name: "GenericInserted", token: chroma.GenericInserted, fence: paletteDiffFence, needle: "+added"}, + {name: "GenericDeleted", token: chroma.GenericDeleted, fence: paletteDiffFence, needle: "-removed"}, +} + +const ( + paletteGoFence = "```go\n// note\nfunc main() {\n\tvar n int = 42\n\tvar s string = \"hi\"\n\tn := 0\n\t_ = n\n}\n```\n" + palettePythonFence = "```python\nclass Widget:\n pass\n```\n" + paletteDiffFence = "```diff\n@@ -1,2 +1,2 @@\n+added\n-removed\n```\n" +) + +// TestBuiltinSyntaxColoursMapToASingleEscape pins a stability property every +// derived style depends on: chroma answers a palette colour with the nearest entry +// of its 256-colour table, and two entries can sit at exactly the same distance +// from a colour. Chroma then breaks the tie by map iteration order, so a fence +// would change shade between frames. Every built-in's syntax role must therefore +// map to exactly one escape. +// +// This is a property of the palette, not of the derivation: a user theme can still +// land on an ambiguous colour, which is why the escape-table test accepts any +// escape the derived style yields for a token. +func TestBuiltinSyntaxColoursMapToASingleEscape(t *testing.T) { + t.Parallel() + + for _, entry := range builtinThemeEntries() { + theme := entry.theme + if theme.borrowsTerminalColors() { + continue + } + if _, bundled := builtinFamilyChromaStyles[theme.id]; bundled { + // A family style's own tokens are chroma's, not pips's. + continue + } + palette := paletteFor(theme) + for _, role := range []paletteSyntaxRole{ + syntaxWorkspace, syntaxMuted, syntaxModel, syntaxSession, + syntaxChange, syntaxActive, syntaxWarning, syntaxError, syntaxCode, + } { + colour := role.colour(palette) + escapes := chromaEscapesForColour(colour) + assert.Len(t, escapes, 1, "%s role %d (%s) maps to one escape", theme.id, role, colorString(colour)) + } + } +} + +// tieProbeAttempts is how many times the tripwire puts one token through the +// formatter. A two-way tie is only visible across calls, because chroma breaks it +// by map iteration order; this many attempts surface one with probability 1 - 2^-15. +const tieProbeAttempts = 16 + +// tieSelfTestAttempts is how many times the probe is checked against the one tie +// this work documented by hand, before the sweep it gates runs. It is far larger +// than the sweep's count because a miss there would make the check itself a lie; at +// this count a miss would be 2^-255. +const tieSelfTestAttempts = 256 + +// TestFamilyStyledBuiltInsKeepTheKnownTokenTies is a tripwire for a property pips +// does not own. Several built-ins that keep a family style resolve a token onto two +// entries of chroma's 256-colour table at exactly the same distance, and chroma +// breaks that tie by map iteration order, so that token's shade can change between +// frames. The colours come from chroma's bundled style data and the terminal256 +// formatter's nearest-colour search rather than from pips's palette, and the +// existing 22 keep their family tokens by design. +// +// The instrument probes every mapped token. Grouping tokens by the colour +// `Style.Get` reports is not sound: the formatter resolves a token through its +// sub-category and category and can land on a different colour, and it also emits a +// background for some tokens. Measured across the catalogue, 20 tokens disagree +// with what `Style.Get` reports and two of them tie where that colour does not, so +// such a grouping would silently skip the tying token. +// +// Detection is probabilistic, and the assertion is one-sided for that reason: a +// detection is always a real tie, so a missed one must not fail the suite, while a +// tie that is not in the known set must. The probe itself is checked against the tie +// this work documented by hand, so a broken probe cannot pass silently. +// +// Measured set on chroma v2.14.0: catppuccin-mocha, default-light, gruvbox-dark, +// gruvbox-light, one-dark. +func TestFamilyStyledBuiltInsKeepTheKnownTokenTies(t *testing.T) { + t.Parallel() + + known := []string{ + themeIDCatppuccinMocha, themeIDDefaultLight, themeIDGruvboxDark, + themeIDGruvboxLight, themeIDOneDark, + } + + // The probe has to be able to see a tie at all, and this is the tie the kanagawa + // palette comment claims: carpYellow sits between two table entries. + require.Len(t, tokenEscapesForColour(chroma.MustParseColour(kanagawaCarpYellow), tieSelfTestAttempts), 2, + "the probe detects the documented carpYellow tie") + + tied := make([]string, 0, len(known)) + for _, entry := range builtinThemeEntries() { + theme := entry.theme + if theme.borrowsTerminalColors() { + continue + } + if _, bundled := builtinFamilyChromaStyles[theme.id]; !bundled { + continue + } + if familyStyleTies(theme) { + tied = append(tied, theme.id) + } + } + slices.Sort(tied) + + assert.Subset(t, known, tied, "a family style gained a 256-colour tie") +} + +// familyStyleTies reports whether any mapped token's escape can vary in this family +// style, which is what a fence would show as a changing shade. +func familyStyleTies(theme colorTheme) bool { + style := lookupChromaStyle(markdownChromaTheme(theme)) + for _, row := range paletteSyntaxTokens { + if len(tokenEscapes(style, row.token, tieProbeAttempts)) > 1 { + return true + } + } + + return false +} + +// tokenEscapesForColour is [tokenEscapes] for a bare colour, which is how the +// tripwire checks that its probe can see a tie at all. +func tokenEscapesForColour(colour chroma.Colour, attempts int) []string { + style := chroma.MustNewStyle("tie-probe", chroma.StyleEntries{chroma.Keyword: colour.String()}) + + return tokenEscapes(style, chroma.Keyword, attempts) +} + +// TestMarkdownPaletteEscapeTable renders the probe fences through the real +// renderer for every built-in and for a theme loaded from a file, and asserts +// each representative token class reaches the terminal with the escape the +// derived style defines for its palette role. It also logs the per-theme escape +// table, which is the evidence that a fence is readable and consistent with its +// theme. +func TestMarkdownPaletteEscapeTable(t *testing.T) { + t.Parallel() + + registry := loadThemeRegistryFromText(t, map[string]string{ + "probe": `schema = "pips.tui.theme/v1alpha1" +name = "Probe" +inherits = "nord" + +[palette] +model = "#FF00AA" +change = "#00C8A0" +active = "#123456" +`, + }) + themes := registry.Themes() + require.NotEmpty(t, themes) + + // Only the themes whose family ships no chroma style draw with the derived + // table; the ones with a family style keep that family's tokens, so for them + // the table is evidence rather than an expectation. + derivedThemes := 0 + + renderer := newMarkdownRenderer(4) + for _, theme := range themes { + if theme.borrowsTerminalColors() { + continue + } + name := markdownChromaTheme(theme) + style := lookupChromaStyle(name) + require.Equal(t, name, style.Name, theme.id) + derived := name == paletteChromaStyleName(theme) + if derived { + derivedThemes++ + } + + escapes := make(map[string]string, len(paletteEscapeProbes)) + for _, probe := range paletteEscapeProbes { + rendered, err := renderer.render(probe.fence, 60, theme, false) + require.NoError(t, err) + + got := markdownLineEscape(rendered, probe.needle) + escapes[probe.name] = got + if derived { + assert.Contains(t, chromaEscapesFor(t, style, probe.token), got, + "%s %s reaches the terminal with its palette role", theme.id, probe.name) + } + assert.Empty(t, markdownSurfaces(rendered), "%s %s: %q", theme.id, probe.name, rendered) + } + + t.Logf("%-18s kw=%s fn=%s str=%s num=%s com=%s op=%s cls=%s sub=%s ins=%s del=%s", + theme.id, escapes["Keyword"], escapes["NameFunction"], escapes["LiteralString"], + escapes["LiteralNumber"], escapes["Comment"], escapes["Operator"], escapes["NameClass"], + escapes["GenericSubheading"], escapes["GenericInserted"], escapes["GenericDeleted"]) + } + + // The user theme and the two derived built-in families must have been + // asserted against the table, not merely logged. + assert.GreaterOrEqual(t, derivedThemes, 4, "probe + everforest dark/light + kanagawa draw with the derived table") +} + +// markdownEscape matches one SGR sequence and captures its parameters. +var markdownEscape = regexp.MustCompile(`\x1b\[([0-9;]+)m`) + +// chromaEscapeAttempts is how many times a colour is put through the formatter +// before its answer is treated as settled. Chroma breaks a nearest-colour tie by +// map iteration order, so a colour that sits exactly between two table entries +// answers differently across calls; a handful of attempts surfaces that, and a +// single answer means there was nothing to break. +const chromaEscapeAttempts = 48 + +// chromaEscapesFor formats one token through the same chroma formatter glamour +// uses, so the expectation is the formatter's own answer for the derived style +// rather than a value this test computes for itself. +func chromaEscapesFor(t *testing.T, style *chroma.Style, token chroma.TokenType) []string { + t.Helper() + + escapes := tokenEscapes(style, token, chromaEscapeAttempts) + require.NotEmpty(t, escapes, "the formatter colours %v", token) + + return escapes +} + +// tokenEscapes collects the distinct escapes a style gives one token, tolerating a +// token the formatter leaves uncoloured: the terminal256 formatter clears the +// Background entry, so it has no escape that could vary. +func tokenEscapes(style *chroma.Style, token chroma.TokenType, attempts int) []string { + return chromaEscapes(func() string { + var out strings.Builder + _ = formatters.TTY256.Format(&out, style, chroma.Literator(chroma.Token{Type: token, Value: "X"})) + + before, _, ok := strings.Cut(out.String(), "X") + if !ok { + return "" + } + + matches := markdownEscape.FindAllStringSubmatch(before, -1) + if len(matches) == 0 { + return "" + } + + return matches[len(matches)-1][1] + }, attempts) +} + +// chromaEscapesForColour is chromaEscapesFor for a bare palette colour, which is +// how the ambiguity guard reads a role without building a style for it. +func chromaEscapesForColour(colour color.Color) []string { + style := chroma.MustNewStyle("palette-probe", chroma.StyleEntries{ + chroma.Keyword: colorString(colour), + }) + + return chromaEscapes(func() string { + var out strings.Builder + _ = formatters.TTY256.Format(&out, style, chroma.Literator(chroma.Token{Type: chroma.Keyword, Value: "X"})) + + before, _, ok := strings.Cut(out.String(), "X") + if !ok { + return "" + } + + matches := markdownEscape.FindAllStringSubmatch(before, -1) + if len(matches) == 0 { + return "" + } + + return matches[len(matches)-1][1] + }, chromaEscapeAttempts) +} + +// chromaEscapes collects the distinct answers one formatting call gives over the +// given number of runs, in a stable order. +func chromaEscapes(format func() string, attempts int) []string { + seen := make(map[string]bool, 2) + for range attempts { + seen[format()] = true + } + escapes := make([]string, 0, len(seen)) + for escape := range seen { + escapes = append(escapes, escape) + } + slices.Sort(escapes) + + return escapes +} diff --git a/internal/coding/tui/markdown_test.go b/internal/coding/tui/markdown_test.go index eb24eac2..d6f35db4 100644 --- a/internal/coding/tui/markdown_test.go +++ b/internal/coding/tui/markdown_test.go @@ -11,7 +11,6 @@ import ( "testing" "github.com/alecthomas/chroma/v2" - "github.com/alecthomas/chroma/v2/styles" "github.com/charmbracelet/x/ansi" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -636,53 +635,149 @@ func TestMarkdownSyntaxColoursFollowTheTheme(t *testing.T) { "the light and dark families highlight a fence differently") } -// TestMarkdownChromaThemesAreBundledStyles keeps the per-theme syntax styles +// TestMarkdownChromaThemesFollowTheirPalette keeps the per-theme syntax styles // resolvable. Chroma answers an unknown name with its own default style, so a // typo or a rename would silently drop the theme's palette instead of failing. func TestMarkdownChromaThemesFollowTheirPalette(t *testing.T) { t.Parallel() + // The syntax style behind each built-in whose family ships one is that + // family's: the id-sharing majority, the three spelled-out Tokyo Night ids, + // the two default themes and the colourless canvas-proof palette. A silent + // family fallback would fail here. + spelled := map[string]string{ + themeIDDefaultDark: chromaStyleGitHubDark, + themeIDDefaultLight: chromaStyleGitHub, + themeIDGruvboxDark: chromaStyleGruvbox, + themeIDOneDark: chromaStyleOneDark, + themeIDTokyoNight: chromaStyleTokyoNightNight, + themeIDTokyoNightStorm: chromaStyleTokyoNightStorm, + themeIDTokyoNightLight: chromaStyleTokyoNightDay, + themeIDTerminal: chromaStyleBW, + } + for _, entry := range builtinThemeEntries() { theme := entry.theme name := markdownChromaTheme(theme) - assert.Equal(t, name, styles.Get(name).Name, "theme %s names a style that resolves", theme.id) + assert.Equal(t, name, lookupChromaStyle(name).Name, "theme %s names a style that resolves", theme.id) if theme.borrowsTerminalColors() { assert.Equal(t, chromaStyleBW, name, "a borrowed palette keeps the colourless style") continue } - assert.Equal(t, pipsChromaPrefix+theme.id, name, - "a built-in names the style derived from its own palette") - } + if _, bundled := builtinFamilyChromaStyles[theme.id]; bundled { + assert.Equal(t, pipsChromaPrefix+theme.id, name, + "a built-in with a family style keeps the style derived from that family") + want := spelled[theme.id] + if want == "" { + want = theme.id + } + assert.Equal(t, want, bundledChromaStyle(theme), theme.id) - // The syntax style behind each built-in is its own family's: the id-sharing - // majority, the three spelled-out Tokyo Night ids, and the two default themes. - // A silent family fallback would fail here. - spelled := map[string]string{ - themeIDDefaultDark: chromaStyleGitHubDark, - themeIDDefaultLight: chromaStyleGitHub, - themeIDGruvboxDark: chromaStyleGruvbox, - themeIDOneDark: chromaStyleOneDark, - themeIDTokyoNight: chromaStyleTokyoNightNight, - themeIDTokyoNightStorm: chromaStyleTokyoNightStorm, - themeIDTokyoNightLight: chromaStyleTokyoNightDay, - themeIDTerminal: chromaStyleBW, + continue + } + // The family ships no bundled style, so there is nothing to inherit: the + // built-in carries a complete style derived from its own palette. + assert.Equal(t, pipsChromaPrefix+theme.paletteStyle, name, + "a built-in with no family style names the style derived from its own palette") + assert.NotContains(t, []string{chromaStyleGitHub, chromaStyleGitHubDark}, name, + "%s must not fall back to a family style", theme.id) } +} + +// TestMarkdownDerivedStylesCarryEveryMappedToken pins the complete derivation: +// for the built-ins whose family ships no chroma style, every row of the +// documented role -> token table is present in the registered style and carries +// that row's palette role. +func TestMarkdownDerivedStylesCarryEveryMappedToken(t *testing.T) { + t.Parallel() + + derived := 0 for _, entry := range builtinThemeEntries() { - want, ok := spelled[entry.theme.id] - if !ok { - want = entry.theme.id + theme := entry.theme + if theme.borrowsTerminalColors() { + continue + } + if _, bundled := builtinFamilyChromaStyles[theme.id]; bundled { + continue } - assert.Equal(t, want, bundledChromaStyle(entry.theme), entry.theme.id) + derived++ + + palette := paletteFor(theme) + style := lookupChromaStyle(markdownChromaTheme(theme)) + require.Equal(t, markdownChromaTheme(theme), style.Name, theme.id) + + for _, row := range paletteSyntaxTokens { + resolved := style.Get(row.token) + assert.Equal(t, chroma.MustParseColour(colorString(row.role.colour(palette))), resolved.Colour, + "%s %v takes its colour from its palette role", theme.id, row.token) + assert.False(t, resolved.Background.IsSet(), + "%s %v paints no fill: a fence draws on the terminal's canvas", theme.id, row.token) + } + } + assert.NotZero(t, derived, "the families chroma ships no style for are derived") +} + +// TestMarkdownUserThemesDeriveTheirOwnSyntaxStyle drives the runtime path: a +// theme file loaded through loadThemeRegistry resolves to a style derived from +// its own palette, not to the family style its background would have picked. +// Two files with the same palette keep separate styles, and a changed palette +// never reuses the previous palette's style. +func TestMarkdownUserThemesDeriveTheirOwnSyntaxStyle(t *testing.T) { + t.Parallel() + + const palette = ` +[palette] +model = "#FF00AA" +change = "#00C8A0" +` + registry := loadThemeRegistryFromText(t, map[string]string{ + "ocean": `schema = "pips.tui.theme/v1alpha1" +name = "Ocean" +inherits = "nord" +` + palette, + "tide": `schema = "pips.tui.theme/v1alpha1" +name = "Tide" +inherits = "nord" +` + palette, + "coral": `schema = "pips.tui.theme/v1alpha1" +name = "Coral" +inherits = "nord" + +[palette] +model = "#123456" +`, + }) + + ocean := mustTheme(t, registry, "ocean") + tide := mustTheme(t, registry, "tide") + coral := mustTheme(t, registry, "coral") + + for _, theme := range []colorTheme{ocean, tide, coral} { + name := markdownChromaTheme(theme) + assert.Equal(t, pipsChromaPrefix+theme.paletteStyle, name, theme.id) + assert.Equal(t, name, lookupChromaStyle(name).Name, "%s resolves", theme.id) + assert.NotContains(t, []string{chromaStyleGitHub, chromaStyleGitHubDark}, name, + "%s must not fall back to a family style", theme.id) + } + + // Same palette, different identity: separate styles, both carrying the + // palette's own model role in the keyword class. + assert.NotEqual(t, markdownChromaTheme(ocean), markdownChromaTheme(tide), + "two themes that share a palette still keep separate styles") + assert.Equal(t, "#FF00AA", colorString(ocean.palette.model)) + for _, theme := range []colorTheme{ocean, tide} { + keyword := lookupChromaStyle(markdownChromaTheme(theme)).Get(chroma.Keyword) + assert.Equal(t, chroma.MustParseColour(colorString(theme.palette.model)), keyword.Colour, + "%s keyword follows its own palette", theme.id) } - // A theme from a file follows the family its background resolves to, because - // nothing derives a style for a palette that arrives at runtime. - light := newResolvedTheme("acme-light", "Acme Light", themeBackgroundLight, themeLight.palette) - dark := newResolvedTheme("acme-dark", "Acme Dark", themeBackgroundDark, themeDark.palette) - assert.Equal(t, chromaStyleGitHub, markdownChromaTheme(light)) - assert.Equal(t, chromaStyleGitHubDark, markdownChromaTheme(dark)) + // A different palette resolves to a different style, so a theme can never + // pick up another palette's tokens. + assert.NotEqual(t, markdownChromaTheme(coral), markdownChromaTheme(ocean)) + coralKeyword := lookupChromaStyle(markdownChromaTheme(coral)).Get(chroma.Keyword) + assert.Equal(t, chroma.MustParseColour("#123456"), coralKeyword.Colour) } // TestMarkdownDiffFencesFollowThePalette pins where a diff fence gets its @@ -700,7 +795,7 @@ func TestMarkdownDiffFencesFollowThePalette(t *testing.T) { continue } palette := paletteFor(theme) - style := styles.Get(markdownChromaTheme(theme)) + style := lookupChromaStyle(markdownChromaTheme(theme)) for token, want := range map[chroma.TokenType]color.Color{ chroma.GenericInserted: palette.change, diff --git a/internal/coding/tui/model.go b/internal/coding/tui/model.go index 22319fd6..795e10dc 100644 --- a/internal/coding/tui/model.go +++ b/internal/coding/tui/model.go @@ -214,7 +214,22 @@ type Model struct { // permissionMode caches the effective sandbox mode the Composer's permission // row shows. Reading it while rendering would clone the Controller's whole // configuration, so it is refreshed when a control result can change it. - permissionMode config.SandboxMode + permissionMode config.SandboxMode + // reasoningLevel caches the effective reasoning level the status line shows + // beside the model name, for the same reason: Controller.Model() clones the + // resolved snapshot, and the status line is drawn on every frame. + // reasoningDeclared records whether the bound model declares any level, so a + // model with no reasoning knob shows no suffix. + reasoningLevel string + reasoningDeclared bool + // copiedNotice is the reserved band's copy confirmation. It is separate from + // statusNotice because a drag copy writes the clipboard alone: there is no + // file to name, so the status line has nothing to add. + copiedNotice string + copiedNoticeSeq uint64 + // clipboardCopySeq generations the clipboard-only copy's outcome, so a stale + // result cannot clear a newer selection. + clipboardCopySeq uint64 textSaveSeq uint64 statusNotice string statusNoticeErr bool @@ -388,6 +403,7 @@ func (m *Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { m.dropPending() m.state = message.controller.Snapshot() m.refreshPermissionMode() + m.refreshReasoningLevel() m.resetHistory() configuredTUI := message.controller.Config().TUI if configuredTUI.StatusLine == nil { @@ -846,8 +862,11 @@ func (m *Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { } // A control result can carry a permission change (the picker's apply, a // reload, a Full Access confirmation), so the Composer's cached row is - // refreshed with it instead of on every frame. + // refreshed with it instead of on every frame. The same result can have + // replaced the bound model, so the status line's cached reasoning level is + // refreshed here too. m.refreshPermissionMode() + m.refreshReasoningLevel() switch { case pickerControl: if m.picker.kind == pickerCommand { @@ -928,13 +947,18 @@ func (m *Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { } m.statusNotice = "" m.statusNoticeErr = false - // A copy confirmation and the selection it describes expire together, so - // the highlight lives exactly as long as the notice that explains it. - m.clearSelection() m.setLayout() return m, nil - case selectionRedrawMsg: + case copiedNoticeExpiredMsg: + if message.generation != m.copiedNoticeSeq { + return m, nil + } + m.copiedNotice = "" + m.setLayout() + + return m, nil + case frameRedrawMsg: return m, nil case textSavedMsg: if message.generation != m.textSaveSeq { @@ -949,6 +973,19 @@ func (m *Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { return m, m.setStatusNotice( textSavedNotice(message.kind, message.path, message.clipboard), ) + case clipboardResultMsg: + // An error is always worth reporting, whichever copy produced it; the + // success effects belong to the drag gesture the reader just made. + if message.err != nil { + return m, m.setStatusError("could not copy: " + safeError(message.err)) + } + if !message.selection || message.generation != m.clipboardCopySeq { + return m, nil + } + m.clearSelection() + m.setLayout() + + return m, m.showCopiedNotice(message.lines) case tea.MouseWheelMsg: return m, m.scrollWheel(message) case tea.MouseMsg: @@ -1281,7 +1318,7 @@ func (m *Model) updateReadyKey(message tea.KeyPressMsg) (tea.Model, tea.Cmd) { if key == keyEscape && m.selection.visible { m.clearSelection() - return m, redrawSelection() + return m, redrawFrame() } if key == keyEscape && m.directAgent != nil && !m.directAgent.running && m.composer.Value() == "" { @@ -1436,9 +1473,14 @@ func (m *Model) readyView() tea.View { if notice := m.directAgentNotice(); notice != "" { footer = append(footer, truncateHeight(notice, 1)) } - for range conversationGapHeight { - footer = append(footer, "") - } + // The reserved band occupies the row the frame used to leave blank here, so + // the Composer's position does not depend on whether the band has anything to + // say. It is part of the footer, which means it is already counted in the + // transcript's elastic height below. Whether the band affords a click is + // decided here, with the contents it draws, and recorded below. + bandHint := m.bandScrollHint() + bandFooterIndex := len(footer) + footer = append(footer, m.reservedBand()) composerFooterIndex := len(footer) footer = append(footer, m.composerBand()) if m.picker.kind != pickerNone { @@ -1484,6 +1526,7 @@ func (m *Model) readyView() tea.View { parts = append(parts, timeline) } composerIndex := len(parts) + composerFooterIndex + bandIndex := len(parts) + bandFooterIndex promptIndex := -1 if promptFooterIndex >= 0 { promptIndex = len(parts) + promptFooterIndex @@ -1497,11 +1540,19 @@ func (m *Model) readyView() tea.View { // An over-tall frame keeps its tail, so whatever is dropped off the top is // transcript rows and the hit map has to account for them. dropped := clampFrameTailOffset(content, m.height) - view := m.presentationView(clampFrameTail(content, m.height)) + transcriptVisible := max(0, transcriptRows-min(dropped, transcriptRows)) + // The turn indicator is drawn into the top row of the frame rather than into a + // band of its own; see turn_indicator.go for why that row is the only free one, + // and why a frame with no visible transcript row draws none. + indicator := transcriptVisible > 0 && m.turnIndicatorShown() + content = m.drawTurnIndicator(clampFrameTail(content, m.height), indicator) + view := m.presentationView(content) m.frameHit = frameHitMap{ - painted: m.sizeReady, - transcript: max(0, transcriptRows-min(dropped, transcriptRows)), - topDropped: min(dropped, transcriptRows), + painted: m.sizeReady, + transcript: transcriptVisible, + topDropped: min(dropped, transcriptRows), + band: m.bandRowOf(parts, bandIndex, dropped, bandHint != ""), + turnIndicator: indicator, } // presentationView installs the pointer handler for every frame, so the ready // view only has to record where its transcript band sat. @@ -1729,7 +1780,7 @@ func (m *Model) statusLineItem(item statusline.Item, width int) string { value = sessionLabel style = style.Bold(true).Foreground(palette.session) case statusline.Model: - value = modelDisplayName(m.state.Provider, m.state.ModelID) + value = m.modelStatusValue() style = style.Foreground(palette.model) case statusline.ContextUsed: window := m.state.ContextWindow @@ -1869,11 +1920,10 @@ func (m *Model) statusExtras() []string { case m.historyOffered() && !m.transcriptScroll.follow: extras = append(extras, "PgUp for older history") } - // A paused reader needs to know that the newest output is off screen and how - // to get back to it; without this the viewport looks stuck. - if !m.transcriptScroll.follow && m.transcriptScroll.maxOffset() > 0 { - extras = append(extras, "scrolled · End for latest") - } + // A paused reader learns that the newest output is off screen from the arrow + // the reserved band draws under the transcript, and returns to it with End or + // a click on that band. The status line no longer repeats the hint: two + // surfaces saying the same thing compete for a narrow line. if !m.options.NoColor { style := lipgloss.NewStyle().Foreground(paletteFor(m.theme).muted) for index := range extras { diff --git a/internal/coding/tui/model_reasoning.go b/internal/coding/tui/model_reasoning.go new file mode 100644 index 00000000..56ac38e8 --- /dev/null +++ b/internal/coding/tui/model_reasoning.go @@ -0,0 +1,64 @@ +package tui + +// The status line names the model and, beside it, the reasoning level the next +// request will carry. The level is not derived here: modelcatalog.resolve folds +// the explicit selection, the variant's level and the model's +// default_reasoning_level into the resolved snapshot, and the generation layer +// sends exactly that value, so the display reads the same single source of truth. +// +// The value is cached rather than read while rendering, for the same reason the +// Composer's permission mode is: Controller.Model() clones the resolved snapshot, +// and the status line is drawn on every frame. It is refreshed wherever the bound +// model can change — bootstrap and a control result. + +// refreshReasoningLevel caches the effective reasoning level of the bound model. +// reasoningDeclared records whether the model declares any level at all, so a +// model with no reasoning knob shows no suffix rather than a meaningless +// "default". +func (m *Model) refreshReasoningLevel() { + m.reasoningLevel, m.reasoningDeclared = "", false + if m.controller == nil { + return + } + + resolved := m.controller.Model().Resolved + m.reasoningDeclared = len(resolved.ReasoningLevels) > 0 + + if level, ok := resolved.EffectiveReasoningLevel(); ok { + m.reasoningLevel = string(level) + } +} + +// reasoningLabel is the effective level in the form the status line shows it. +// +// - A selected level is shown as itself, whatever it is. A configured +// "default" is never shown as "default" when the model metadata resolves it +// to a concrete level, which is the whole point of reading the resolved +// snapshot instead of the raw configuration. +// - A model that declares reasoning levels but has none selected is shown as +// "default": pips sends no level, so the provider's own default applies, and +// that value is unknown to pips — it ships no model capability database — so +// it must not be guessed. +// - A model that declares no reasoning level at all gets no label, because +// there is no knob to describe. +func (m *Model) reasoningLabel() string { + switch { + case m.reasoningLevel != "": + return m.reasoningLevel + case m.reasoningDeclared: + return defaultSelectionLabel + default: + return "" + } +} + +// modelStatusValue is the Model item's text: the display name, followed by the +// effective reasoning level when there is one to name. +func (m *Model) modelStatusValue() string { + value := modelDisplayName(m.state.Provider, m.state.ModelID) + if label := m.reasoningLabel(); label != "" { + value += " (" + label + ")" + } + + return value +} diff --git a/internal/coding/tui/model_reasoning_test.go b/internal/coding/tui/model_reasoning_test.go new file mode 100644 index 00000000..0f59441d --- /dev/null +++ b/internal/coding/tui/model_reasoning_test.go @@ -0,0 +1,179 @@ +//nolint:wsl_v5 // Each case builds its fixture next to the expectation it proves. +package tui + +import ( + "testing" + + "github.com/charmbracelet/x/ansi" + "github.com/rsbin1178/pips/ai" + "github.com/rsbin1178/pips/internal/coding/config" + "github.com/rsbin1178/pips/internal/coding/modelcatalog" + "github.com/rsbin1178/pips/internal/coding/runtimecontrol" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// reasoningModelConfig is the model every case below resolves through the real +// catalog, so the "default" cases exercise default_reasoning_level resolution +// rather than a level this test invented. +func reasoningModelConfig() config.ModelConfig { + return config.ModelConfig{ + Ref: config.ModelRef{Provider: ai.ProviderOpenAI, Model: "test-model"}, + ContextWindow: 200000, + ReasoningLevels: []config.ReasoningLevel{"low", "high"}, + Variants: map[string]config.VariantConfig{}, + } +} + +// reasoningController builds a Controller whose resolved snapshot comes out of +// modelcatalog.New + Resolve for the given model and configured override. +func reasoningController(t *testing.T, model config.ModelConfig, override *config.ReasoningLevel) Controller { + t.Helper() + + cfg := config.Config{ + Model: model.Ref, + Models: []config.ModelConfig{model}, + Providers: map[ai.Provider]config.ProviderConfig{}, + Sandbox: config.SandboxWorkspaceWrite, + Approval: config.ApprovalOnRequest, + Reasoning: override, + } + catalog, err := modelcatalog.New(cfg) + require.NoError(t, err) + + selection := modelcatalog.SelectionFromConfig(cfg) + resolved, err := catalog.Resolve(selection) + require.NoError(t, err) + + return stubController{ + state: readyState(), + modelState: &runtimecontrol.ModelState{Selection: selection, Resolved: resolved}, + } +} + +// TestStatusLineShowsTheEffectiveReasoningLevel drives the real status-line +// render over the five cases the display must get right: an explicit level, a +// configured "default" that the model metadata resolves to a concrete level, and +// a level that cannot be resolved at all. +// +// The suffix is read back from the rendered line, so a change that stops the +// status line from naming the level fails here rather than in a unit test of the +// helper. +func TestStatusLineShowsTheEffectiveReasoningLevel(t *testing.T) { + t.Parallel() + + low := config.ReasoningLevel("low") + high := config.ReasoningLevel("high") + + withDefault := func(level *config.ReasoningLevel) config.ModelConfig { + model := reasoningModelConfig() + model.DefaultReasoningLevel = level + + return model + } + withoutKnob := config.ModelConfig{ + Ref: config.ModelRef{Provider: ai.ProviderOpenAI, Model: "test-model"}, + ContextWindow: 200000, + Variants: map[string]config.VariantConfig{}, + } + + tests := []struct { + name string + model config.ModelConfig + override *config.ReasoningLevel + want string + }{ + {name: "explicit low", model: reasoningModelConfig(), override: &low, want: "Test Model (low)"}, + {name: "explicit high", model: reasoningModelConfig(), override: &high, want: "Test Model (high)"}, + {name: "default resolves to high", model: withDefault(&high), want: "Test Model (high)"}, + {name: "default resolves to low", model: withDefault(&low), want: "Test Model (low)"}, + { + name: "declared but unresolvable", model: reasoningModelConfig(), + want: "Test Model (default)", + }, + {name: "no reasoning knob", model: withoutKnob, want: "Test Model"}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + controller := reasoningController(t, test.model, test.override) + model := readyModelWithController(t, controller, true) + + status := ansi.Strip(model.statusLine()) + assert.Contains(t, status, test.want) + if test.want == "Test Model" { + assert.NotContains(t, status, "Test Model (", "a model with no knob gets no suffix") + } + }) + } +} + +// TestStatusLineRefreshesTheReasoningLevelAfterAModelChange pins the cache +// boundary: the level is read once when the bound model changes, not on every +// frame, so the control result that applies a model selection must refresh it. +func TestStatusLineRefreshesTheReasoningLevelAfterAModelChange(t *testing.T) { + t.Parallel() + + high := config.ReasoningLevel("high") + model := readyModelWithController(t, stubController{state: readyState()}, true) + require.NotContains(t, ansi.Strip(model.statusLine()), "(") + + model.controller = reasoningController(t, reasoningModelConfig(), &high) + _, _ = model.Update(controlResultMsg{operation: operationModel}) + + assert.Contains(t, ansi.Strip(model.statusLine()), "Test Model (high)") +} + +// TestResolvedModelReportsTheLevelItWillSend keeps the display read and the wire +// value in step: EffectiveReasoningLevel must answer with the value the request +// assembly sends, and must say "unknown" rather than guessing when the model +// declares no default. +func TestResolvedModelReportsTheLevelItWillSend(t *testing.T) { + t.Parallel() + + high := config.ReasoningLevel("high") + model := reasoningModelConfig() + model.DefaultReasoningLevel = &high + + resolved, err := resolveReasoningModel(t, model, nil) + require.NoError(t, err) + level, ok := resolved.EffectiveReasoningLevel() + require.True(t, ok) + assert.Equal(t, high, level) + + undeclared, err := resolveReasoningModel(t, reasoningModelConfig(), nil) + require.NoError(t, err) + _, ok = undeclared.EffectiveReasoningLevel() + assert.False(t, ok, "pips ships no capability database, so an absent level is unknown, not a guess") + + explicit, err := resolveReasoningModel(t, reasoningModelConfig(), new(config.ReasoningLevel("low"))) + require.NoError(t, err) + level, ok = explicit.EffectiveReasoningLevel() + require.True(t, ok) + assert.Equal(t, config.ReasoningLevel("low"), level) +} + +func resolveReasoningModel( + t *testing.T, + model config.ModelConfig, + override *config.ReasoningLevel, +) (modelcatalog.ResolvedModel, error) { + t.Helper() + + cfg := config.Config{ + Model: model.Ref, + Models: []config.ModelConfig{model}, + Providers: map[ai.Provider]config.ProviderConfig{}, + Sandbox: config.SandboxWorkspaceWrite, + Approval: config.ApprovalOnRequest, + Reasoning: override, + } + catalog, err := modelcatalog.New(cfg) + if err != nil { + return modelcatalog.ResolvedModel{}, err + } + + return catalog.Resolve(modelcatalog.SelectionFromConfig(cfg)) +} diff --git a/internal/coding/tui/model_test.go b/internal/coding/tui/model_test.go index f7be15af..7258d3a6 100644 --- a/internal/coding/tui/model_test.go +++ b/internal/coding/tui/model_test.go @@ -765,6 +765,9 @@ type stubController struct { Controller state coding.State configStatusLine []statusline.Item + // modelState, when set, is what Model() reports. It stands in for the + // resolved snapshot the status line reads its reasoning level from. + modelState *runtimecontrol.ModelState // tui, when set, overrides the presentation fields of the stub configuration. tui config.TUIConfig // cost, when set, replaces the stub configuration's empty price table. @@ -776,6 +779,17 @@ type stubController struct { func (c stubController) Snapshot() coding.State { return c.state.Clone() } +// Model defaults to the zero state, so a stub that does not script a resolved +// model reports no reasoning level rather than reaching the embedded nil +// Controller. +func (c stubController) Model() runtimecontrol.ModelState { + if c.modelState == nil { + return runtimecontrol.ModelState{} + } + + return *c.modelState +} + // History defaults to an empty page so the embedded nil Controller is never // reached; tests that exercise paging set historyPage. func (c stubController) History( diff --git a/internal/coding/tui/mouse.go b/internal/coding/tui/mouse.go index 10e15cd2..6ef07074 100644 --- a/internal/coding/tui/mouse.go +++ b/internal/coding/tui/mouse.go @@ -152,16 +152,27 @@ func (m *Model) wheelLines(message tea.MouseWheelMsg) (int, bool) { } } -// viewportOwnsPointer reports whether the managed transcript is the surface a -// pointer may act on: the ready viewport is showing, nothing else owns input, and -// the escape hatch is not holding the screen open for the terminal. -func (m *Model) viewportOwnsPointer() bool { - return m.lifecycle == lifecycleReady && m.sizeReady && m.fullscreen() && +// readyViewOwnsPointer reports whether the ready frame is the one on screen: the +// layout's own transcript band is what the pointer acts on, and nothing else +// (a route, a prompt, a picker, the transcript escape hatch, an inline Team stage) +// has taken the pointer or the keyboard. +func (m *Model) readyViewOwnsPointer() bool { + return m.lifecycle == lifecycleReady && m.sizeReady && m.route.kind == routeNone && !m.teamRouteIsInline() && m.prompt.kind == promptNone && m.picker.kind == pickerNone && !m.transcriptMode.active } +// viewportOwnsPointer reports whether the managed transcript is the surface a +// pointer may act on as a selection: the ready viewport is showing, nothing else +// owns input, and the escape hatch is not holding the screen open for the +// terminal. The reserved band's arrow needs less than this — it is answered +// wherever the ready frame is the one on screen — so it asks +// [readyViewOwnsPointer] instead. +func (m *Model) viewportOwnsPointer() bool { + return m.readyViewOwnsPointer() && m.fullscreen() +} + // mouseReportingEnabled reports whether this frame asked the terminal for mouse // events. It is the resolved policy rather than the negotiated mode: the terminal // either honours the request or ignores it, and the help text has to describe the diff --git a/internal/coding/tui/mouse_selection.go b/internal/coding/tui/mouse_selection.go index a1af7984..8fb00d31 100644 --- a/internal/coding/tui/mouse_selection.go +++ b/internal/coding/tui/mouse_selection.go @@ -34,20 +34,20 @@ type selectionState struct { visible bool } -// selectionRedrawMsg asks for one more frame after a pointer gesture changed the -// selection. Mouse handling happens outside Update, and the renderer only -// repaints when a message comes back from the handler. -type selectionRedrawMsg struct{} +// frameRedrawMsg asks for one more frame after a pointer gesture changed +// something the frame draws. Mouse handling happens outside Update, and the +// renderer only repaints when a message comes back from the handler. +type frameRedrawMsg struct{} // selectionStyle paints the selected cells. It is the same reverse-video // treatment the find box gives a match — both mean "this text is addressed" — and // NO_COLOR drops both. var selectionStyle = lipgloss.NewStyle().Reverse(true) -// redrawSelection returns the no-op command that makes the renderer paint the +// redrawFrame returns the no-op command that makes the renderer paint the // gesture's result. -func redrawSelection() tea.Cmd { - return func() tea.Msg { return selectionRedrawMsg{} } +func redrawFrame() tea.Cmd { + return func() tea.Msg { return frameRedrawMsg{} } } // handleSelectionMouse routes a pointer gesture that is not a wheel notch. A @@ -68,7 +68,7 @@ func (m *Model) handleSelectionMouse(message tea.MouseMsg) tea.Cmd { point := selectionPoint{row: row, column: max(0, message.X)} m.selection = selectionState{anchor: point, focus: point, dragging: true, visible: true} - return redrawSelection() + return redrawFrame() case tea.MouseMotionMsg: if !m.selection.dragging { return nil @@ -76,7 +76,7 @@ func (m *Model) handleSelectionMouse(message tea.MouseMsg) tea.Cmd { m.selection.focus = selectionPoint{row: m.selectionRowAt(message.Y), column: max(0, message.X)} - return redrawSelection() + return redrawFrame() case tea.MouseReleaseMsg: if !m.selection.dragging || message.Button != tea.MouseLeft { return nil @@ -102,7 +102,9 @@ func (m *Model) selectionRowAt(frameRow int) int { } // finishSelection ends a drag: a press and release on one cell is a click and -// activates the entry, anything longer is a selection and is copied. +// activates the entry, anything longer is a selection and is copied to the +// clipboard alone. The highlight is left standing until the copy reports back, so +// a copy that could not be made keeps the text it addressed visible. func (m *Model) finishSelection() tea.Cmd { selection := m.selection m.selection.dragging = false @@ -113,14 +115,7 @@ func (m *Model) finishSelection() tea.Cmd { return m.activateEntry(selection.anchor.row) } - text := m.selectionText() - if strings.TrimSpace(text) == "" { - m.selection = selectionState{} - - return redrawSelection() - } - - return tea.Batch(m.saveText(TextKindCopy, "", text, true), redrawSelection()) + return tea.Batch(m.copySelection(), redrawFrame()) } // clearSelection drops the highlight, which is what Escape and a capture change diff --git a/internal/coding/tui/mouse_selection_pty_test.go b/internal/coding/tui/mouse_selection_pty_test.go index cb7bf8f2..baefb096 100644 --- a/internal/coding/tui/mouse_selection_pty_test.go +++ b/internal/coding/tui/mouse_selection_pty_test.go @@ -7,6 +7,7 @@ import ( "context" "encoding/base64" "fmt" + "io/fs" "os" "path/filepath" "strings" @@ -16,6 +17,7 @@ import ( "github.com/charmbracelet/x/vt" "github.com/rsbin1178/pips/ai" "github.com/rsbin1178/pips/internal/coding/config" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -31,8 +33,9 @@ func sgrMouse(button, column, row int, release bool) string { } // TestPTYDragSelectCopiesTheRows is the end-to-end proof of the gesture: a real -// terminal sends press, drag and release, and the addressed rows reach both the -// OSC 52 clipboard request and the fallback file. +// terminal sends press, drag and release, and the addressed rows reach the OSC 52 +// clipboard request. The fallback saver must never run: a drag is a transient +// reading gesture, so it leaves no file behind even when one is configured. func TestPTYDragSelectCopiesTheRows(t *testing.T) { const testName = "TestPTYDragSelectCopiesTheRows" if os.Getenv("PIPS_TUI_SELECTION_PTY_HELPER") == "1" { @@ -111,15 +114,21 @@ func TestPTYDragSelectCopiesTheRows(t *testing.T) { return strings.Contains(h.output.String(), payload) }, 30*time.Second, 10*time.Millisecond, "helper output: %s", h.output.String()) + // The reserved band reports what the gesture took, and the highlight is gone: + // the copy succeeded, so the addressed text is no longer pending. require.Eventually(t, func() bool { - written, err := os.ReadFile(copyPath) - - return err == nil && string(written) == expected - }, 30*time.Second, 10*time.Millisecond) + return strings.Contains(h.frame(60, 24), "copied 3 lines") + }, 30*time.Second, 10*time.Millisecond, "helper output: %s", h.output.String()) + assert.NotContains(t, h.frame(60, 24), "\x1b[7m", "the selection highlight is cleared") h.write("\x0bquit\r") result := h.waitForExit() require.NotNil(t, result) require.Equal(t, 0, result.ExitCode(), "helper output: %s", h.output.String()) h.assertRestored() + + // The process has exited, so every command it started has finished: a saver + // that had been asked to write would have left its file behind by now. + _, statErr := os.Stat(copyPath) + assert.ErrorIs(t, statErr, fs.ErrNotExist, "a drag copy never reaches the text saver") } diff --git a/internal/coding/tui/mouse_selection_test.go b/internal/coding/tui/mouse_selection_test.go index 551ed62c..799643db 100644 --- a/internal/coding/tui/mouse_selection_test.go +++ b/internal/coding/tui/mouse_selection_test.go @@ -97,9 +97,9 @@ func TestDragSelectionMadeBackwardsCopiesTheSameText(t *testing.T) { command := model.handleMouse(tea.MouseReleaseMsg{X: 0, Y: first, Button: tea.MouseLeft}) require.NotNil(t, command) - runCommandTree(t, command) - require.Len(t, *calls, 1) - assert.Equal(t, "❯ MARKER-A\n\n❯ MARKER-B", (*calls)[0].Content) + messages := runCommandTree(t, command) + assert.Equal(t, []string{"❯ MARKER-A\n\n❯ MARKER-B"}, clipboardTexts(messages)) + assert.Empty(t, *calls, "the drag writes the clipboard alone") } func TestDragSelectionCopiesTheAddressedRows(t *testing.T) { @@ -118,23 +118,20 @@ func TestDragSelectionCopiesTheAddressedRows(t *testing.T) { require.NotNil(t, command, "the release copies what the drag addressed") messages := runCommandTree(t, command) - require.Len(t, *calls, 1) - assert.Equal(t, TextKindCopy, (*calls)[0].Kind) // The blank row that separates the two entries is on screen, so it is part of // what the drag addressed. - assert.Equal(t, "❯ MARKER-A\n\n❯ MARKER-B", (*calls)[0].Content) assert.Equal(t, []string{"❯ MARKER-A\n\n❯ MARKER-B"}, clipboardTexts(messages)) + assert.Empty(t, *calls, "a drag copy never reaches the text saver") + require.True(t, model.selection.visible, "the highlight lives until the copy reports back") - saved, ok := findTextSaved(messages) + result, ok := findClipboardResult(messages) require.True(t, ok) - _, notice := model.Update(saved) - require.NotNil(t, notice) - assert.Contains(t, strings.Join(model.statusExtras(), "\n"), "/resolved/copy") - - // The highlight stays until the confirmation that explains it expires. - assert.True(t, model.selection.visible) - model.Update(statusNoticeExpiredMsg{generation: model.statusNoticeSeq}) - assert.False(t, model.selection.visible, "the selection and its notice expire together") + _, notice := model.Update(result) + assert.False(t, model.selection.visible, "a confirmed copy clears the highlight") + assert.Equal(t, "copied 3 lines", model.copiedNotice) + assert.NotNil(t, notice, "the confirmation expires on its own timer") + assert.NotContains(t, strings.Join(model.statusExtras(), "\n"), "copied", + "the band owns the confirmation; the status line does not repeat it") } func TestDragSelectionCopiesOnlyTheAddressedCells(t *testing.T) { @@ -150,9 +147,9 @@ func TestDragSelectionCopiesOnlyTheAddressedCells(t *testing.T) { command := model.handleMouse(tea.MouseReleaseMsg{X: 10, Y: row, Button: tea.MouseLeft}) require.NotNil(t, command) - runCommandTree(t, command) - require.Len(t, *calls, 1) - assert.Equal(t, "MARKER", (*calls)[0].Content) + messages := runCommandTree(t, command) + assert.Equal(t, []string{"MARKER"}, clipboardTexts(messages)) + assert.Empty(t, *calls, "the drag writes the clipboard alone") } func TestDragSelectionSkipsASelectionThatIsOnlyPadding(t *testing.T) { @@ -162,15 +159,18 @@ func TestDragSelectionSkipsASelectionThatIsOnlyPadding(t *testing.T) { row := paintedRow(t, model, "MARKER-A") // The cells past the row's text are the renderer's padding, and a reader who - // drags only over them has addressed nothing. + // drags only over them has addressed nothing. The gesture did not copy, so the + // highlight stays and the notice says why nothing happened. The notice is set + // before the command returns, so there is no timer to wait out here. model.handleMouse(tea.MouseClickMsg{X: 38, Y: row, Button: tea.MouseLeft}) model.handleMouse(tea.MouseMotionMsg{X: 39, Y: row, Button: tea.MouseLeft}) command := model.handleMouse(tea.MouseReleaseMsg{X: 39, Y: row, Button: tea.MouseLeft}) require.NotNil(t, command) - runCommandTree(t, command) assert.Empty(t, *calls, "padding is not copied") - assert.False(t, model.selection.visible) + assert.True(t, model.selection.visible, "a copy that did not happen keeps the selection") + assert.True(t, model.statusNoticeErr) + assert.Contains(t, ansi.Strip(model.statusLine()), "nothing selected to copy") } func TestSelectionHighlightKeepsTheRowWidth(t *testing.T) { @@ -237,7 +237,7 @@ func TestEscapeClearsTheSelectionBeforeCanceling(t *testing.T) { require.NotNil(t, command) messages := runCommandTree(t, command) require.Len(t, messages, 1) - assert.IsType(t, selectionRedrawMsg{}, messages[0], "Escape closes the selection, not the session") + assert.IsType(t, frameRedrawMsg{}, messages[0], "Escape closes the selection, not the session") assert.False(t, model.selection.visible) } diff --git a/internal/coding/tui/mouse_test.go b/internal/coding/tui/mouse_test.go index e372860c..d690a271 100644 --- a/internal/coding/tui/mouse_test.go +++ b/internal/coding/tui/mouse_test.go @@ -8,6 +8,7 @@ import ( "time" tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" "github.com/charmbracelet/x/ansi" "github.com/rsbin1178/pips/agent/team" "github.com/rsbin1178/pips/ai" @@ -58,6 +59,153 @@ func TestFullscreenWheelScrollsTheManagedTranscript(t *testing.T) { assert.Equal(t, bottom, model.transcriptScroll.offset) } +// TestPointerHandlerLeavesTheWheelToUpdate pins the message contract a frame that +// captures the pointer has to respect: the renderer forwards a mouse message to +// the installed handler and then to Update, so the wheel is answered exactly once, +// in Update. Answering it in the handler too would scroll twice per notch. +func TestPointerHandlerLeavesTheWheelToUpdate(t *testing.T) { + t.Parallel() + + model := wheelModel(t, nil) + _ = model.View() + require.True(t, model.transcriptScroll.follow) + bottom := model.transcriptScroll.offset + require.Positive(t, bottom, "the fixture must overflow the window") + + assert.Nil(t, model.handleMouse(tea.MouseWheelMsg{Button: tea.MouseWheelUp, Y: 4}), + "the pointer handler does not answer the wheel") + assert.Equal(t, bottom, model.transcriptScroll.offset, + "the pointer handler leaves the notch to Update") + + model.Update(tea.MouseWheelMsg{Button: tea.MouseWheelUp, Y: 4}) + assert.Equal(t, bottom-wheelLinesDefault, model.transcriptScroll.offset, + "one notch moves the transcript once") +} + +// TestReservedBandClickIsDroppedBehindAnOverlay keeps the recorded band row from +// outliving the frame that painted it: once a picker owns the screen, a click at +// that row is not the band's. +func TestReservedBandClickIsDroppedBehindAnOverlay(t *testing.T) { + t.Parallel() + + model := wheelModel(t, nil) + model.Update(tea.MouseWheelMsg{Button: tea.MouseWheelUp}) + _ = model.View() + require.GreaterOrEqual(t, model.frameHit.band, 0) + band := model.frameHit.band + + // A picker owns the screen while the recorded row is still the band's. + model.openCommandPicker() + + _, handled := model.bandMouse(tea.MouseClickMsg{X: 0, Y: band, Button: tea.MouseLeft}) + assert.False(t, handled, "the band does not answer behind an overlay") + assert.False(t, model.transcriptScroll.follow) +} + +// TestReservedBandClickTargetMatchesThePaintedRow pins the row arithmetic at the +// two edges a tall fixture hides: a band at the top of the frame (no transcript +// rows above it) and a frame too short to keep the band. lipgloss answers +// Height("") with 1, so an empty prefix used to shift the recorded row down one +// and the click landed on the composer border instead of the arrow. +func TestReservedBandClickTargetMatchesThePaintedRow(t *testing.T) { + t.Parallel() + + for _, height := range []int{2, 3, 4, 5, 10, 24} { + model := wheelModel(t, nil) + model.Update(tea.WindowSizeMsg{Width: 40, Height: height}) + model.Update(tea.MouseWheelMsg{Button: tea.MouseWheelUp}) + + content := model.View().Content + rows := strings.Split(content, "\n") + band := model.frameHit.band + + if band < 0 { + assert.NotContains(t, content, bandScrollIcon, + "height %d: a frame with no arrow records no target", height) + + continue + } + require.Less(t, band, len(rows), "height %d: the target is inside the frame", height) + assert.Contains(t, rows[band], bandScrollIcon, + "height %d: the recorded row is the one the arrow was painted at", height) + + command := model.handleMouse(tea.MouseClickMsg{X: 0, Y: band, Button: tea.MouseLeft}) + assert.NotNil(t, command, "height %d: the arrow's own row answers", height) + assert.True(t, model.transcriptScroll.follow, "height %d", height) + } +} + +// TestReservedBandArrowHidesWhenAResizeReachesTheBottom pins the hint's gate. A +// window that grows clamps the offset onto the new maximum without restoring the +// region's follow flag, so a hint driven by that flag would point at rows that are +// already on screen. +func TestReservedBandArrowHidesWhenAResizeReachesTheBottom(t *testing.T) { + t.Parallel() + + model := wheelModel(t, nil) + model.Update(tea.MouseWheelMsg{Button: tea.MouseWheelUp}) + require.False(t, model.transcriptScroll.follow) + require.Contains(t, ansi.Strip(model.View().Content), bandScrollIcon) + + // Grow the window by less than the scrollback, so the offset is clamped onto + // the new maximum and the flag is left behind. + model.Update(tea.WindowSizeMsg{Width: 40, Height: 20}) + require.True(t, model.transcriptScroll.atBottom(), "the clamp reached the new maximum") + require.False(t, model.transcriptScroll.follow, "the fixture still leaves the flag behind") + + assert.NotContains(t, ansi.Strip(model.View().Content), bandScrollIcon, + "the arrow points at nothing once the newest rows are on screen") + assert.Equal(t, -1, model.frameHit.band, "and it is not a click target either") +} + +// TestReservedBandHintIsCentred pins the band's layout: the scroll hint sits in the +// middle of the row rather than at the text column, because the band is a standing +// invitation to move rather than a field of the status line. The two margins differ +// by at most one column, which is what an odd remainder leaves. +func TestReservedBandHintIsCentred(t *testing.T) { + t.Parallel() + + model := wheelModel(t, nil) + + for _, width := range []int{24, 40, 60, 80, 121} { + model.Update(tea.WindowSizeMsg{Width: width, Height: 20}) + model.Update(tea.MouseWheelMsg{Button: tea.MouseWheelUp}) + + band := ansi.Strip(model.reservedBand()) + require.NotEmpty(t, band, "width %d", width) + require.Contains(t, band, bandScrollIcon, "width %d", width) + + trimmed := strings.TrimSpace(band) + left := strings.Index(band, trimmed) + right := ansi.StringWidth(band) - left - ansi.StringWidth(trimmed) + assert.LessOrEqual(t, max(left, right)-min(left, right), 1, + "width %d: margins %d and %d around %q", width, left, right, trimmed) + assert.Equal(t, width, ansi.StringWidth(band), "the band still fills the row at width %d", width) + } +} + +// TestReservedBandHintAndCopyConfirmationShareTheRow pins what happens when both +// occupants are present: the confirmation keeps the status line's right edge and +// the hint is centred in what is left of it, so neither covers the other. +func TestReservedBandHintAndCopyConfirmationShareTheRow(t *testing.T) { + t.Parallel() + + model := wheelModel(t, nil) + model.Update(tea.WindowSizeMsg{Width: 80, Height: 20}) + model.Update(tea.MouseWheelMsg{Button: tea.MouseWheelUp}) + // The confirmation is raised through its own setter, so this drives the shipped + // path rather than poking the field it writes. + require.NotNil(t, model.showCopiedNotice(3)) + require.Equal(t, "copied 3 lines", model.copiedNotice) + + band := ansi.Strip(model.reservedBand()) + assert.Contains(t, band, bandScrollIcon, "the hint is there beside the confirmation") + assert.Contains(t, band, "copied 3 lines", "and so is the confirmation") + assert.Less(t, strings.Index(band, bandScrollIcon), strings.Index(band, "copied 3 lines"), + "the hint sits to the left of the confirmation") + assert.Equal(t, 80, ansi.StringWidth(band)) +} + // TestWheelDeltaIsConservativeUnderAMultiplexer pins the per-terminal policy: one // line inside TMUX/Zellij/screen, three elsewhere. func TestWheelDeltaIsConservativeUnderAMultiplexer(t *testing.T) { @@ -133,22 +281,118 @@ func TestWheelIsRoutedToTheSurfaceThatOwnsTheScreen(t *testing.T) { }) } -// TestStatusLineReportsAPausedReader is the visible half of R4: a reader who is -// not following the tail can see that the newest output is off screen. -func TestStatusLineReportsAPausedReader(t *testing.T) { +// TestReservedBandReportsAPausedReader is the visible half of R4: a reader who is +// not following the tail can see that the newest output is off screen, from the +// arrow the reserved band draws under the transcript. The status line no longer +// repeats the hint. +func TestReservedBandReportsAPausedReader(t *testing.T) { t.Parallel() model := wheelModel(t, nil) - assert.NotContains(t, ansi.Strip(model.statusLine()), "scrolled ·") + assert.NotContains(t, ansi.Strip(model.View().Content), bandScrollIcon, + "a reader who is following the tail sees no arrow") + assert.NotContains(t, ansi.Strip(model.statusLine()), "End for latest") model.Update(tea.MouseWheelMsg{Button: tea.MouseWheelUp}) - assert.Contains(t, ansi.Strip(model.statusLine()), "scrolled · End for latest") + assert.Contains(t, ansi.Strip(model.View().Content), bandScrollIcon, + "a paused reader is told where the newest rows are") + assert.NotContains(t, ansi.Strip(strings.Join(model.statusExtras(), "\n")), "End for latest", + "the status line's scroll hint is gone: the band owns it") + assert.NotContains(t, ansi.Strip(model.statusLine()), "End for latest") model.Update(tea.KeyPressMsg{Code: tea.KeyEnd}) - assert.NotContains(t, ansi.Strip(model.statusLine()), "scrolled ·") + assert.NotContains(t, ansi.Strip(model.View().Content), bandScrollIcon) assert.True(t, model.transcriptScroll.follow) } +// TestReservedBandClickReturnsToTheLatest pins the pointer half of the arrow: +// a left click on the band row the arrow was painted at restores the newest rows, +// and the band records that affordance with the frame it painted. +func TestReservedBandClickReturnsToTheLatest(t *testing.T) { + t.Parallel() + + model := wheelModel(t, nil) + model.Update(tea.MouseWheelMsg{Button: tea.MouseWheelUp}) + require.False(t, model.transcriptScroll.follow) + + // Compose the frame the click will be resolved against. + content := model.View().Content + require.GreaterOrEqual(t, model.frameHit.band, 0, "the frame records the band row") + + // The band is the row between the transcript and the Composer border, so the + // border is directly below it. This is the integration half of the layout + // claim: a unit test of reservedBand() alone would pass with the band unwired. + rows := strings.Split(content, "\n") + border := -1 + for index, row := range rows { + // Rounded under colour, normal under NO_COLOR, which this fixture uses. + if strings.ContainsAny(row, "╭┌") { + border = index + + break + } + } + require.Positive(t, border, "the Composer box is on screen") + assert.Equal(t, border-1, model.frameHit.band, "the band sits directly above the Composer border") + + command := model.handleMouse(tea.MouseClickMsg{X: 0, Y: model.frameHit.band, Button: tea.MouseLeft}) + assert.NotNil(t, command) + assert.True(t, model.transcriptScroll.follow, "the click returns to the newest rows") + assert.NotContains(t, ansi.Strip(model.View().Content), bandScrollIcon) +} + +// TestReservedBandClickIsDroppedWhenTheFrameDrewNoArrow keeps the hit map honest: +// a band row that afforded nothing answers nothing, so a click on a blank band +// cannot move a reader who is already at the bottom. +func TestReservedBandClickIsDroppedWhenTheFrameDrewNoArrow(t *testing.T) { + t.Parallel() + + model := wheelModel(t, nil) + _ = model.View() + assert.Equal(t, -1, model.frameHit.band, "a frame with no arrow records no band target") + + _, handled := model.bandMouse(tea.MouseClickMsg{X: 0, Y: 0, Button: tea.MouseLeft}) + assert.False(t, handled) +} + +// TestReservedBandIsOneRowAtEveryWidth pins the layout contract: the band is a +// single reserved row between the activity text and the Composer border, and it +// is the row the frame used to leave blank there. +func TestReservedBandIsOneRowAtEveryWidth(t *testing.T) { + t.Parallel() + + assert.Equal(t, conversationGapHeight, layoutBandRows, + "the band is the row the pre-Composer gap already reserved") + + model := wheelModel(t, nil) + for _, width := range []int{12, 24, 40, 80, 200} { + model.Update(tea.WindowSizeMsg{Width: width, Height: defaultHeight}) + band := model.reservedBand() + assert.LessOrEqual(t, lipgloss.Height(band), layoutBandRows, "width %d", width) + assert.Empty(t, ansi.Strip(band), "a following reader sees a blank band at width %d", width) + } +} + +// TestReservedBandArrowFitsANarrowNoColorFrame pins the readable degrade: the +// arrow is plain text at every width, never wider than the frame, and carries no +// colour escape in NO_COLOR (wheelModel is the colour-free fixture). +func TestReservedBandArrowFitsANarrowNoColorFrame(t *testing.T) { + t.Parallel() + + model := wheelModel(t, nil) + for _, width := range []int{12, 20, 40, 80} { + model.Update(tea.WindowSizeMsg{Width: width, Height: defaultHeight}) + model.Update(tea.MouseWheelMsg{Button: tea.MouseWheelUp, Y: 4}) + require.False(t, model.transcriptScroll.follow, "width %d", width) + + band := model.reservedBand() + assert.Equal(t, 1, lipgloss.Height(band), "the band stays one row at width %d", width) + assert.LessOrEqual(t, ansi.StringWidth(band), width, "the band fits the frame at width %d", width) + assert.NotContains(t, band, "\x1b[", "NO_COLOR keeps the band free of escapes at width %d", width) + assert.Contains(t, band, bandScrollIcon, "width %d", width) + } +} + // TestHelpStatesTheModifierDragFallback records the user-visible cost of mouse // capture where a user can read it. func TestHelpStatesTheModifierDragFallback(t *testing.T) { diff --git a/internal/coding/tui/route_list_hit.go b/internal/coding/tui/route_list_hit.go index 824ff7ce..cdc618ad 100644 --- a/internal/coding/tui/route_list_hit.go +++ b/internal/coding/tui/route_list_hit.go @@ -143,13 +143,13 @@ func (m *Model) routeListMouse(message tea.MouseMsg) (tea.Cmd, bool) { if tab, ok := m.agentsRouteTabAt(click.Y, click.X); ok { m.selectAgentsRouteTab(tab) - return redrawSelection(), true + return redrawFrame(), true } } if index, ok := m.routeListRowAt(click.Y, click.X); ok { m.selectRouteListRow(index) - return redrawSelection(), true + return redrawFrame(), true } return nil, true diff --git a/internal/coding/tui/run.go b/internal/coding/tui/run.go index 32d6d065..85995625 100644 --- a/internal/coding/tui/run.go +++ b/internal/coding/tui/run.go @@ -224,6 +224,17 @@ type TextSaveRequest struct { // reports back whatever the saver resolved. type TextSaver func(context.Context, TextSaveRequest) (string, error) +// TextClipboard writes copied text to the terminal's clipboard and reports +// whether the write reached it. +// +// The default is nil, which sends an OSC 52 request through the Program — the +// only writer allowed to touch the terminal. A caller that owns another route to +// the clipboard, such as a remote session bridge, supplies its own; it replaces +// the request for every copy the TUI makes (the drag gesture, Ctrl+X, /copy), and +// its error is reported instead of being assumed away. An explicit /copy or +// /export with a path keeps its saver, which is the durable half of that result. +type TextClipboard func(content string) error + // Options contain the CLI-owned resources used by one TUI Program. // BuildStamp is the build identity the TUI reports, so a display surface never // has to import the CLI that stamped the binary. @@ -273,6 +284,9 @@ type Options struct { // SaveText persists Ctrl+X, /copy and /export output. A nil saver keeps the // clipboard path and reports that no file fallback is available. SaveText TextSaver + // ClipboardWriter overrides the OSC 52 write a copy performs. A nil writer + // keeps the terminal path. + ClipboardWriter TextClipboard } // Run owns the terminal Program and closes any acquired Controller after the diff --git a/internal/coding/tui/search_pty_test.go b/internal/coding/tui/search_pty_test.go index 95b6c346..1c7d7597 100644 --- a/internal/coding/tui/search_pty_test.go +++ b/internal/coding/tui/search_pty_test.go @@ -151,7 +151,8 @@ func TestPTYSearchAndListWheel(t *testing.T) { scrolled := frame() assert.NotContains(t, scrolled, "MARKER-199", "the jump left the tail") - assert.Contains(t, scrolled, "scrolled · End for latest", "the reader is paused") + assert.Contains(t, scrolled, bandScrollIcon, + "the reserved band tells the paused reader where the newest rows are") // Escape closes the box and hands the composer band back. _, err = master.Write([]byte{0x1b}) diff --git a/internal/coding/tui/selection_copy_test.go b/internal/coding/tui/selection_copy_test.go new file mode 100644 index 00000000..2f058ca9 --- /dev/null +++ b/internal/coding/tui/selection_copy_test.go @@ -0,0 +1,194 @@ +package tui + +import ( + "errors" + "testing" + + tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/x/ansi" + "github.com/rsbin1178/pips/ai" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// dragAcross drags from the first marker's row to the second one's and returns +// the release command, so every guard below starts from the same real gesture. +func dragAcross(t *testing.T, model *Model, from, to string) tea.Cmd { + t.Helper() + + first := paintedRow(t, model, from) + second := paintedRow(t, model, to) + require.Less(t, first, second) + + model.handleMouse(tea.MouseClickMsg{X: 0, Y: first, Button: tea.MouseLeft}) + model.handleMouse(tea.MouseMotionMsg{X: 40, Y: second, Button: tea.MouseLeft}) + + command := model.handleMouse(tea.MouseReleaseMsg{X: 40, Y: second, Button: tea.MouseLeft}) + require.NotNil(t, command) + + return command +} + +// TestDragSelectionNeverReachesTheTextSaver is the load-bearing half of the +// auto-copy contract: a spy saver is installed and the drag must not call it, +// while the clipboard write must be requested. +func TestDragSelectionNeverReachesTheTextSaver(t *testing.T) { + t.Parallel() + + model, calls := selectionModel(t, true, "MARKER-A", "MARKER-B") + + written := &[]string{} + model.options.ClipboardWriter = func(content string) error { + *written = append(*written, content) + + return nil + } + + messages := runCommandTree(t, dragAcross(t, model, "MARKER-A", "MARKER-B")) + + assert.Empty(t, *calls, "the drag path never reaches the text saver") + assert.Equal(t, []string{"❯ MARKER-A\n\n❯ MARKER-B"}, *written, + "the injected clipboard writer is the one asked to write") + assert.Empty(t, clipboardTexts(messages), + "an injected writer replaces the Program's OSC 52 request") + assert.Empty(t, model.copiedNotice, "no confirmation before the outcome arrives") +} + +// TestDragSelectionClearsTheHighlightOnASuccessfulCopy pins the visible half: the +// confirmation clears the selection, so no highlight is left behind once the text +// is on the clipboard. +func TestDragSelectionClearsTheHighlightOnASuccessfulCopy(t *testing.T) { + t.Parallel() + + model, _ := selectionModel(t, false, "MARKER-A", "MARKER-B") + command := dragAcross(t, model, "MARKER-A", "MARKER-B") + require.True(t, model.selection.visible) + + messages := runCommandTree(t, command) + result, ok := findClipboardResult(messages) + require.True(t, ok) + require.NoError(t, result.err) + + _, notice := model.Update(result) + assert.NotNil(t, notice) + assert.False(t, model.selection.visible, "a confirmed copy leaves no highlight") + assert.Equal(t, "copied 3 lines", model.copiedNotice) + assert.Contains(t, ansi.Strip(model.reservedBand()), "copied 3 lines") + assert.NotContains(t, ansi.Strip(model.View().Content), "\x1b[7m", + "the painted frame carries no selection highlight") +} + +// TestDragSelectionKeepsTheHighlightWhenTheCopyFails pins the other half: a copy +// that could not be made leaves the addressed text standing and says so. +func TestDragSelectionKeepsTheHighlightWhenTheCopyFails(t *testing.T) { + t.Parallel() + + model, calls := selectionModel(t, false, "MARKER-A", "MARKER-B") + model.options.ClipboardWriter = func(string) error { return errors.New("clipboard unavailable") } + + messages := runCommandTree(t, dragAcross(t, model, "MARKER-A", "MARKER-B")) + result, ok := findClipboardResult(messages) + require.True(t, ok) + require.Error(t, result.err) + assert.Empty(t, *calls) + + _, notice := model.Update(result) + assert.NotNil(t, notice) + assert.True(t, model.selection.visible, "a failed copy keeps what it addressed") + assert.Empty(t, model.copiedNotice, "a failed copy shows no confirmation") + assert.True(t, model.statusNoticeErr) + assert.Contains(t, ansi.Strip(model.statusLine()), "could not copy") +} + +// TestDragSelectionWithNothingAddressedKeepsTheSelection covers the other failure +// the gesture can report: a drag that covered only the renderer's padding. The +// notice is set before the command returns, so there is no timer to wait out. +func TestDragSelectionWithNothingAddressedKeepsTheSelection(t *testing.T) { + t.Parallel() + + model, calls := selectionModel(t, true, "MARKER-A") + row := paintedRow(t, model, "MARKER-A") + + model.handleMouse(tea.MouseClickMsg{X: 38, Y: row, Button: tea.MouseLeft}) + model.handleMouse(tea.MouseMotionMsg{X: 39, Y: row, Button: tea.MouseLeft}) + command := model.handleMouse(tea.MouseReleaseMsg{X: 39, Y: row, Button: tea.MouseLeft}) + require.NotNil(t, command) + + assert.Empty(t, *calls) + assert.True(t, model.selection.visible) + assert.True(t, model.statusNoticeErr) + assert.Contains(t, ansi.Strip(model.statusLine()), "nothing selected to copy") + assert.Empty(t, model.copiedNotice) +} + +// TestExplicitCopiesStillReachTheTextSaver is the regression guard for the paths +// the drag gesture must not change: /copy, a path copy and /export keep their +// saver, and a bare /copy keeps the Program's own clipboard request. +func TestExplicitCopiesStillReachTheTextSaver(t *testing.T) { + t.Parallel() + + model, calls := copyModel(t, ai.AssistantText("REPLY")) + + assert.Equal(t, []string{"REPLY"}, clipboardTexts(runCommandTree(t, model.copyAssistant(1, "")))) + assert.Empty(t, clipboardTexts(runCommandTree(t, model.copyAssistant(1, "/out/reply.txt"))), + "an explicit destination is the whole delivery") + assert.Empty(t, clipboardTexts(runCommandTree(t, model.exportConversation("")))) + + require.Len(t, *calls, 3) + assert.Equal(t, TextKindCopy, (*calls)[0].Kind) + assert.Empty(t, (*calls)[0].Path, "a bare /copy asks the saver for its default destination") + assert.Equal(t, "/out/reply.txt", (*calls)[1].Path) + assert.Equal(t, TextKindExport, (*calls)[2].Kind) +} + +// TestInjectedClipboardWriterReplacesEveryRequest pins the option's scope: a +// caller that owns another route to the clipboard replaces the OSC 52 request for +// every copy, not only for the drag gesture, so the two cannot disagree about +// where the text went. +func TestInjectedClipboardWriterReplacesEveryRequest(t *testing.T) { + t.Parallel() + + model, _ := copyModel(t, ai.AssistantText("REPLY")) + + written := &[]string{} + model.options.ClipboardWriter = func(content string) error { + *written = append(*written, content) + + return nil + } + + messages := runCommandTree(t, model.copyAssistant(1, "")) + assert.Empty(t, clipboardTexts(messages), "the injected writer replaces the OSC 52 request") + assert.Equal(t, []string{"REPLY"}, *written, "/copy uses the caller's route too") + + // A failing writer is reported for the explicit path as well, and leaves the + // drag gesture's own state alone. + model.options.ClipboardWriter = func(string) error { return errors.New("clipboard unavailable") } + messages = runCommandTree(t, model.copyAssistant(1, "")) + result, ok := findClipboardResult(messages) + require.True(t, ok) + require.Error(t, result.err) + assert.False(t, result.selection, "an explicit copy is not the drag gesture") + _, command := model.Update(result) + assert.NotNil(t, command) + assert.Contains(t, ansi.Strip(model.statusLine()), "could not copy") +} + +// TestCopiedNoticeExpiresOnItsOwn keeps the band's confirmation transient and +// independent of the status line's notice. +func TestCopiedNoticeExpiresOnItsOwn(t *testing.T) { + t.Parallel() + + model, _ := selectionModel(t, true, "MARKER-A", "MARKER-B") + messages := runCommandTree(t, dragAcross(t, model, "MARKER-A", "MARKER-B")) + result, ok := findClipboardResult(messages) + require.True(t, ok) + + _, expiry := model.Update(result) + require.NotNil(t, expiry) + require.Equal(t, "copied 3 lines", model.copiedNotice) + + model.Update(copiedNoticeExpiredMsg{generation: model.copiedNoticeSeq}) + assert.Empty(t, model.copiedNotice) + assert.Empty(t, ansi.Strip(model.reservedBand())) +} diff --git a/internal/coding/tui/theme.go b/internal/coding/tui/theme.go index a3de1197..a9ab12ef 100644 --- a/internal/coding/tui/theme.go +++ b/internal/coding/tui/theme.go @@ -24,6 +24,9 @@ const ( themeIDDefaultDark = "default-dark" themeIDDefaultLight = "default-light" themeIDDracula = "dracula" + themeIDEverforestDark = "everforest-dark" + themeIDEverforestLight = "everforest-light" + themeIDKanagawa = "kanagawa" themeIDNord = "nord" themeIDGruvboxDark = "gruvbox-dark" themeIDCatppuccinLatte = "catppuccin-latte" @@ -84,6 +87,11 @@ type colorTheme struct { background themeBackground palette colorPalette fingerprint string + // paletteStyle is the digest that names the complete chroma style derived + // from this palette. It is computed once, from the same canonical form as the + // fingerprint, so it is stable when the snapshot is stamped for a canvas and + // two themes with the same palette but different ids keep separate styles. + paletteStyle string // codeSurface reports whether Markdown may paint the inline-code background // on the canvas this snapshot will be drawn on. A surface fill is a claim // about the canvas, so it is stamped from the detected terminal background diff --git a/internal/coding/tui/theme_registry.go b/internal/coding/tui/theme_registry.go index e456d588..30c73a52 100644 --- a/internal/coding/tui/theme_registry.go +++ b/internal/coding/tui/theme_registry.go @@ -308,6 +308,10 @@ func loadThemeRegistry(directory string) themeRegistry { } slices.Sort(ids) for _, id := range ids { + // A theme loaded from a file has no family style to inherit, so its + // complete fence style is derived from its palette here, at the one point + // that builds a registry — before the model can render with it. + registerPaletteChromaStyle(resolved[id]) registry.entries = append(registry.entries, themeEntry{ theme: resolved[id], source: themeSourceUser, @@ -715,10 +719,12 @@ func newResolvedTheme(id, name string, background themeBackground, palette color colorString(palette.codeBackground), colorString(palette.diagnostic), ) digest := sha256.Sum256([]byte(canonical)) + fingerprint := hex.EncodeToString(digest[:]) return colorTheme{ id: id, name: name, background: background, palette: palette, - fingerprint: hex.EncodeToString(digest[:]), + fingerprint: fingerprint, + paletteStyle: fingerprint[:paletteStyleChars], } } @@ -750,7 +756,10 @@ func builtinThemeEntries() []themeEntry { ids := []string{ themeIDDefaultDark, themeIDDefaultLight, themeIDTerminal, themeIDCatppuccinLatte, themeIDCatppuccinFrappe, themeIDCatppuccinMacchiato, themeIDCatppuccinMocha, - themeIDDracula, themeIDGruvboxLight, themeIDGruvboxDark, + themeIDDracula, + themeIDEverforestLight, themeIDEverforestDark, + themeIDGruvboxLight, themeIDGruvboxDark, + themeIDKanagawa, themeIDModusOperandi, themeIDModusVivendi, themeIDNord, themeIDOneDark, themeIDRosePineDawn, themeIDRosePineMoon, themeIDRosePine, @@ -778,6 +787,18 @@ const ( gruvboxGray = "#928374" // gruvbox neutral gray, both variants solarizedBase0 = "#839496" // Solarized base0, light chrome and dark canvas + + // Everforest and Kanagawa reuse a tone on more than one role, as their + // official palettes name it once and their own themes reuse it. + everforestDarkGrey0 = "#7A8478" // Everforest dark grey0 + everforestDarkGrey1 = "#859289" // Everforest dark grey1 + everforestLightGrey0 = "#A6B0A0" // Everforest light grey0 + everforestLightGrey1 = "#939F91" // Everforest light grey1 + kanagawaFujiGray = "#727169" // Kanagawa fujiGray, the comment tone + // kanagawaCarpYellow is the tone the Kanagawa family uses for identifiers. The + // palette below deliberately does not use it for a role, and + // TestFamilyStyledBuiltInsKeepTheKnownTokenTies pins why. + kanagawaCarpYellow = "#E6C384" ) // builtinThemePalettes and builtinThemeMetadataByID hold every built-in that is @@ -800,6 +821,32 @@ var builtinThemePalettes = map[string]themePaletteStrings{ session: "#8BE9FD", model: "#BD93F9", idle: "#50FA7B", active: "#F1FA8C", warning: "#FFB86C", error: "#FF5555", change: "#FF79C6", code: "#F8F8F2", codeBackground: "#44475A", diagnostic: "#6272A4", }, + // Everforest, from sainnhe/everforest autoload/everforest.vim (medium + // background). Roles reuse the family's own names: fg for text, grey0/grey1 + // for chrome, and the six accents for session/model/idle/active/warning/error. + themeIDEverforestDark: { + separator: "#475258", composerPrompt: everforestDarkGrey1, muted: everforestDarkGrey1, workspace: "#D3C6AA", + session: "#7FBBB3", model: "#D699B6", idle: "#A7C080", active: "#DBBC7F", warning: "#E69875", + error: "#E67E80", change: "#83C092", code: "#D3C6AA", codeBackground: "#2D353B", diagnostic: everforestDarkGrey0, + }, + themeIDEverforestLight: { + separator: "#E6E2CC", composerPrompt: everforestLightGrey1, muted: everforestLightGrey1, workspace: "#5C6A72", + session: "#3A94C5", model: "#DF69BA", idle: "#8DA101", active: "#DFA000", warning: "#F57D26", + error: "#F85552", change: "#35A77C", code: "#5C6A72", codeBackground: "#FDF6E3", diagnostic: everforestLightGrey0, + }, + // Kanagawa, from rebelot/kanagawa.nvim lua/kanagawa/colors.lua and the "wave" + // theme in lua/kanagawa/themes.lua: fujiWhite on sumiInk3 for text, fujiGray + // for comments and chrome, and the theme's own syn.* accents for the rest. + // active is autumnYellow rather than the carpYellow the family uses for + // identifiers: carpYellow sits at exactly the same distance from two entries of + // the 256-colour cube, so chroma's nearest-colour search would return either + // one depending on map order and a fence's numbers would change shade between + // frames. TestBuiltinSyntaxColoursMapToASingleEscape keeps that from returning. + themeIDKanagawa: { + separator: "#54546D", composerPrompt: kanagawaFujiGray, muted: kanagawaFujiGray, workspace: "#DCD7BA", + session: "#7E9CD8", model: "#957FB8", idle: "#98BB6C", active: "#DCA561", warning: "#FFA066", + error: "#E82424", change: "#7AA89F", code: "#DCD7BA", codeBackground: "#2A2A37", diagnostic: kanagawaFujiGray, + }, themeIDModusOperandi: { separator: "#9F9F9F", composerPrompt: "#595959", muted: "#595959", workspace: "#000000", session: "#0031A9", model: "#721045", idle: "#006800", active: "#6F5500", warning: "#8A290F", @@ -902,6 +949,9 @@ var builtinThemeMetadataByID = map[string]builtinThemeMetadata{ themeIDDefaultDark: {name: "Default Dark", background: themeBackgroundDark}, themeIDDefaultLight: {name: "Default Light", background: themeBackgroundLight}, themeIDDracula: {name: "Dracula", background: themeBackgroundDark}, + themeIDEverforestDark: {name: "Everforest Dark", background: themeBackgroundDark}, + themeIDEverforestLight: {name: "Everforest Light", background: themeBackgroundLight}, + themeIDKanagawa: {name: "Kanagawa", background: themeBackgroundDark}, themeIDNord: {name: "Nord", background: themeBackgroundDark}, themeIDGruvboxDark: {name: "Gruvbox Dark", background: themeBackgroundDark}, themeIDGruvboxLight: {name: "Gruvbox Light", background: themeBackgroundLight}, diff --git a/internal/coding/tui/theme_test.go b/internal/coding/tui/theme_test.go index 09d73efc..4e2a4744 100644 --- a/internal/coding/tui/theme_test.go +++ b/internal/coding/tui/theme_test.go @@ -118,11 +118,11 @@ func TestBuiltinThemeRegistryIsStableAndComplete(t *testing.T) { registry := loadThemeRegistry("") options := registry.Options() - require.Len(t, options, 23) + require.Len(t, options, 26) assert.Equal(t, "auto", options[0].id) assert.True(t, options[0].automatic) entries := registry.Entries() - require.Len(t, entries, 22) + require.Len(t, entries, 25) ids := make([]string, 0, len(entries)) for _, entry := range entries { @@ -149,7 +149,8 @@ func TestBuiltinThemeRegistryIsStableAndComplete(t *testing.T) { assert.Equal(t, []string{ "default-dark", "default-light", "terminal", "catppuccin-latte", "catppuccin-frappe", "catppuccin-macchiato", "catppuccin-mocha", - "dracula", "gruvbox-light", "gruvbox-dark", "modus-operandi", "modus-vivendi", + "dracula", "everforest-light", "everforest-dark", + "gruvbox-light", "gruvbox-dark", "kanagawa", "modus-operandi", "modus-vivendi", "nord", "one-dark", "rose-pine-dawn", "rose-pine-moon", "rose-pine", "solarized-light", "solarized-dark", "tokyo-night-light", "tokyo-night-storm", "tokyo-night", }, ids) @@ -163,19 +164,41 @@ func TestBuiltinThemeRegistryIsStableAndComplete(t *testing.T) { assert.Equal(t, themeBackgroundLight, mustTheme(t, registry, themeIDGruvboxLight).background) assert.Equal(t, themeBackgroundDark, mustTheme(t, registry, themeIDSolarizedDark).background) assert.Equal(t, themeBackgroundLight, mustTheme(t, registry, themeIDModusOperandi).background) + assert.Equal(t, themeBackgroundDark, mustTheme(t, registry, themeIDEverforestDark).background) + assert.Equal(t, themeBackgroundLight, mustTheme(t, registry, themeIDEverforestLight).background) + assert.Equal(t, themeBackgroundDark, mustTheme(t, registry, themeIDKanagawa).background) + assert.Equal(t, "Everforest Dark", mustTheme(t, registry, themeIDEverforestDark).name) + assert.Equal(t, "Everforest Light", mustTheme(t, registry, themeIDEverforestLight).name) + assert.Equal(t, "Kanagawa", mustTheme(t, registry, themeIDKanagawa).name) } +// TestBuiltinThemeRepresentativeContrast pins the two pairings a palette actually +// paints on `code_background`: the inline-code pill draws the `code` role on it, +// and the fence body is the same foreground, so both must clear 4.0. +// +// The syntax accents are deliberately not checked against `code_background`: a +// fence paints them on the terminal's own canvas, which pips does not know, and +// the catalogue's accents are tuned against that canvas rather than against the +// pill's surface. Measured across the catalogue, several built-ins already sit +// below 3:1 there (dracula's muted is 1.94:1, catppuccin-latte's active 1.70:1), +// which is a property of each family's own palette and not of this guard. func TestBuiltinThemeRepresentativeContrast(t *testing.T) { t.Parallel() for _, entry := range loadThemeRegistry("").Entries() { - if entry.theme.borrowsTerminalColors() { + theme := entry.theme + if theme.borrowsTerminalColors() { // The code fill is never painted for a theme that borrows the terminal's - // own colours, so workspace-versus-code_background says nothing about it. + // own colours, so code_background says nothing about it. continue } - ratio := themeContrast(entry.theme.palette.workspace, entry.theme.palette.codeBackground) - assert.GreaterOrEqual(t, ratio, 4.0, entry.theme.id) + body := themeContrast(theme.palette.workspace, theme.palette.codeBackground) + pill := themeContrast(theme.palette.code, theme.palette.codeBackground) + t.Logf("%-18s workspace %s and code %s on code_background %s = %.2f:1 / %.2f:1", + theme.id, colorString(theme.palette.workspace), colorString(theme.palette.code), + colorString(theme.palette.codeBackground), body, pill) + assert.GreaterOrEqual(t, body, 4.0, "%s fence body", theme.id) + assert.GreaterOrEqual(t, pill, 4.0, "%s inline-code pill", theme.id) } } diff --git a/internal/coding/tui/transcript.go b/internal/coding/tui/transcript.go index 218a3ee7..8c9244db 100644 --- a/internal/coding/tui/transcript.go +++ b/internal/coding/tui/transcript.go @@ -2,6 +2,7 @@ package tui import ( + "slices" "strconv" "strings" @@ -337,6 +338,24 @@ func (s *transcriptStore) reindex() { s.indexDirty = false } +// rowOfPreviousUserTurn reports the row the newest user entry above `before` starts +// at, which is where the turn the reader is walking back to began. ok is false when +// no user entry sits above that row. +func (s *transcriptStore) rowOfPreviousUserTurn(before int) (int, bool) { + s.reindex() + + for index := range slices.Backward(s.records) { + if s.records[index].block.kind != blockUser { + continue + } + if s.starts[index] < before { + return s.starts[index], true + } + } + + return 0, false +} + // updateRevision fingerprints the record set, its geometry and the live // record's height, so a search scan can skip a frame in which nothing it reads // changed. It deliberately does not read the live record's text: only a search diff --git a/internal/coding/tui/turn_indicator.go b/internal/coding/tui/turn_indicator.go new file mode 100644 index 00000000..eadf4501 --- /dev/null +++ b/internal/coding/tui/turn_indicator.go @@ -0,0 +1,126 @@ +package tui + +import ( + "strings" + + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + "github.com/charmbracelet/x/ansi" +) + +// The turn indicator is the top counterpart of the reserved band. While there are +// rows above the window it draws a small up triangle in the transcript's first row; +// a left click puts the newest user entry above the window at its top, so repeated +// clicks walk back through the conversation one turn at a time. The band's own hint +// then offers the way forward again, so the two arrows bracket the reader. +// +// It is drawn into the first row rather than into a band of its own because there is +// no free row above the transcript: reserving one would take a row from the messages +// and, worse, would change the very window height that decides whether the indicator +// is needed, so showing it could hide it again on the next frame. +// +// The glyphs are solid on purpose: the hollow triangle is this UI's attention marker +// (an awaiting approval, a compacting context, the picker's polarity mark), so the +// scroll pair uses the filled pair to read as movement rather than as a warning. +const turnIndicatorIcon = "▲" + +// turnIndicatorShown reports whether the up indicator belongs on this frame. It marks +// that the conversation continues above the window, so it stays for every turn the +// reader walks back through and only leaves once the window reaches the beginning of +// the conversation. Only the managed fullscreen viewport owns such a window: the +// inline layout hands history to the terminal and keeps only the tail, and the +// transcript escape hatch gives the screen to the terminal. +func (m *Model) turnIndicatorShown() bool { + if !m.fullscreen() || m.transcriptMode.active || !m.readyViewOwnsPointer() { + return false + } + + return m.transcriptScroll.offset > 0 +} + +// turnIndicator reports the frame row the indicator is painted at and the columns it +// covers, or ok false while it is hidden. +func (m *Model) turnIndicator() (row, left, right int, ok bool) { + if !m.turnIndicatorShown() { + return 0, 0, 0, false + } + + size := ansi.StringWidth(turnIndicatorIcon) + left = max(0, (max(1, m.width)-size)/2) + + return 0, left, left + size, true +} + +// drawTurnIndicator writes the indicator into the first row of a composed frame, +// centred, replacing the cells it covers. The row is padded to the frame width first, +// because the overlay needs a cell to replace even when the row it lands on is blank +// or short. +// +// shown is the decision the frame made, which the caller takes from the frame it +// composed: the indicator marks a row of the transcript, so a frame too short to show +// any transcript row draws none. Without that test the overlay would land on the +// reserved band's own row in a very short frame and cover the band's marker with its +// own. +func (m *Model) drawTurnIndicator(content string, shown bool) string { + _, left, right, ok := m.turnIndicator() + if !ok || !shown || content == "" { + return content + } + + newline := strings.Index(content, "\n") + row, tail := content, "" + + if newline >= 0 { + row, tail = content[:newline], content[newline:] + } + + // The overlay needs a cell to replace, so a blank or short row is padded to the + // span it covers first. + if width := ansi.StringWidth(row); width < right { + row += strings.Repeat(" ", right-width) + } + + icon := turnIndicatorIcon + if !m.options.NoColor { + icon = lipgloss.NewStyle().Foreground(paletteFor(m.theme).muted).Render(icon) + } + + return ansi.Truncate(row, left, "") + icon + ansi.TruncateLeft(row, right, "") + tail +} + +// turnIndicatorMouse answers a left click on the indicator by walking back one turn. +// Any other button, and any column outside the indicator's own span, belongs to the +// transcript underneath it. +func (m *Model) turnIndicatorMouse(message tea.MouseMsg) (tea.Cmd, bool) { + if !m.frameHit.turnIndicator { + return nil, false + } + + row, left, right, ok := m.turnIndicator() + if !ok { + return nil, false + } + + click, isClick := message.(tea.MouseClickMsg) + if !isClick || click.Button != tea.MouseLeft || click.Y != row || + click.X < left || click.X >= right { + return nil, false + } + + return m.jumpToPreviousTurn(), true +} + +// jumpToPreviousTurn puts the newest user entry above the window at its top, so +// repeated clicks walk back through the conversation one turn at a time. A viewport +// whose remaining rows hold no user entry goes to the beginning of the conversation +// instead, which is the last place the indicator can take the reader. +func (m *Model) jumpToPreviousTurn() tea.Cmd { + row, ok := m.transcript.rowOfPreviousUserTurn(m.transcriptScroll.offset) + if !ok { + row = 0 + } + + m.transcriptScroll.scrollTo(row) + + return redrawFrame() +} diff --git a/internal/coding/tui/turn_indicator_test.go b/internal/coding/tui/turn_indicator_test.go new file mode 100644 index 00000000..8182a318 --- /dev/null +++ b/internal/coding/tui/turn_indicator_test.go @@ -0,0 +1,171 @@ +//nolint:wsl_v5 // Each guard drives the indicator and states its outcome next to it. +package tui + +import ( + "strings" + "testing" + + tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/x/ansi" + "github.com/rsbin1178/pips/ai" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// turnIndicatorModel is a fullscreen viewport holding three long turns, so the +// indicator has several turns to walk back through. +func turnIndicatorModel(t *testing.T) *Model { + t.Helper() + + state := readyState() + state.Transcript = []ai.Message{ + ai.UserText("TURN-ONE"), ai.AssistantText(strings.Repeat("one answer\n\n", 12)), + ai.UserText("TURN-TWO"), ai.AssistantText(strings.Repeat("two answer\n\n", 12)), + ai.UserText("TURN-THREE"), ai.AssistantText(strings.Repeat("three answer\n\n", 12)), + } + + model := fullscreenModel(t, stubController{state: state}, true) + model.Update(tea.WindowSizeMsg{Width: 60, Height: 20}) + model.rerenderTranscript(true) + _ = model.View() + + return model +} + +// turnIndicatorClick clicks the arrow's own cell and returns its command. +func turnIndicatorClick(t *testing.T, model *Model) tea.Cmd { + t.Helper() + + _, left, _, shown := model.turnIndicator() + require.True(t, shown, "the arrow has to be on the frame") + + command := model.handleMouse(tea.MouseClickMsg{X: left, Y: 0, Button: tea.MouseLeft}) + require.NotNil(t, command) + + return command +} + +// TestTurnIndicatorShowsWhileAnythingIsAbove pins the marker's reach: it reports that +// the conversation continues above the window rather than that one particular turn is +// cut off, so it is there for every turn and only leaves at the beginning. +func TestTurnIndicatorShowsWhileAnythingIsAbove(t *testing.T) { + t.Parallel() + + model := turnIndicatorModel(t) + require.Positive(t, model.transcriptScroll.offset, "the fixture is at the bottom of a long conversation") + require.True(t, model.turnIndicatorShown()) + require.True(t, model.frameHit.turnIndicator, "the frame recorded the indicator") + + rows := strings.Split(model.View().Content, "\n") + _, left, right, shown := model.turnIndicator() + require.True(t, shown) + cell := ansi.Strip(ansi.TruncateLeft(rows[0], left, "")) + assert.True(t, strings.HasPrefix(cell, turnIndicatorIcon), + "the indicator is centred in the first row, found %q", cell) + assert.Equal(t, model.width, ansi.StringWidth(rows[0]), "the row keeps the frame width") + assert.Equal(t, 1, right-left) + + // The beginning of the conversation is the one place it leaves. + model.Update(tea.KeyPressMsg{Code: tea.KeyHome}) + require.Equal(t, 0, model.transcriptScroll.offset) + assert.False(t, model.turnIndicatorShown()) + _ = model.View() + assert.False(t, model.frameHit.turnIndicator) + assert.NotContains(t, strings.Split(model.View().Content, "\n")[0], turnIndicatorIcon) +} + +// TestTurnIndicatorWalksBackOneTurnAtATime pins the pointer half: each click puts the +// newest prompt above the window at its top, so repeated clicks walk back through the +// conversation, and the arrow stays until the window reaches the beginning. +func TestTurnIndicatorWalksBackOneTurnAtATime(t *testing.T) { + t.Parallel() + + model := turnIndicatorModel(t) + + for _, want := range []string{"TURN-THREE", "TURN-TWO", "TURN-ONE"} { + before := model.transcriptScroll.offset + row, ok := model.transcript.rowOfPreviousUserTurn(before) + require.True(t, ok, "a turn sits above offset %d", before) + + turnIndicatorClick(t, model) + assert.Equal(t, row, model.transcriptScroll.offset, "the window moved to %s", want) + assert.Contains(t, strings.Split(model.View().Content, "\n")[0], want, + "the prompt the reader walked back to is the first row") + + _ = model.View() + assert.True(t, model.frameHit.turnIndicator, + "the arrow stays for the next turn the reader may walk back to") + } + + // The last click reaches the beginning of the conversation, where there is no + // user entry left to walk back to. + row, ok := model.transcript.rowOfPreviousUserTurn(model.transcriptScroll.offset) + assert.False(t, ok) + turnIndicatorClick(t, model) + assert.Equal(t, 0, row, "the fallback is the beginning of the conversation") + require.Equal(t, 0, model.transcriptScroll.offset) + + _ = model.View() + assert.False(t, model.frameHit.turnIndicator, "and there the arrow is gone") + assert.False(t, model.turnIndicatorShown()) +} + +// TestTurnIndicatorIgnoresOtherCells pins the hit test: only the cells the frame +// painted the arrow in answer, so a click beside it is still the transcript's. +func TestTurnIndicatorIgnoresOtherCells(t *testing.T) { + t.Parallel() + + model := turnIndicatorModel(t) + _, left, right, shown := model.turnIndicator() + require.True(t, shown) + offset := model.transcriptScroll.offset + + for _, click := range []tea.MouseMsg{ + tea.MouseClickMsg{X: left - 1, Y: 0, Button: tea.MouseLeft}, + tea.MouseClickMsg{X: right, Y: 0, Button: tea.MouseLeft}, + tea.MouseClickMsg{X: left, Y: 1, Button: tea.MouseLeft}, + tea.MouseClickMsg{X: left, Y: 0, Button: tea.MouseRight}, + } { + _, handled := model.turnIndicatorMouse(click) + assert.False(t, handled, "%v is not the arrow's", click) + } + assert.Equal(t, offset, model.transcriptScroll.offset, "the window did not move") +} + +// TestTurnIndicatorIsNotAnsweredWithoutTheFrame pins the discipline the band also +// follows: a click resolves against the frame the reader saw, so an arrow the last +// painted frame did not draw answers nothing. +func TestTurnIndicatorIsNotAnsweredWithoutTheFrame(t *testing.T) { + t.Parallel() + + model := turnIndicatorModel(t) + _, left, _, shown := model.turnIndicator() + require.True(t, shown) + + // Compose a frame at the beginning, then click where the arrow used to be. + model.Update(tea.KeyPressMsg{Code: tea.KeyHome}) + _ = model.View() + require.False(t, model.frameHit.turnIndicator) + + _, handled := model.turnIndicatorMouse(tea.MouseClickMsg{X: left, Y: 0, Button: tea.MouseLeft}) + assert.False(t, handled) + assert.Equal(t, 0, model.transcriptScroll.offset, "nothing moved") +} + +// TestTurnIndicatorNeedsSomethingAbove keeps the arrow off a conversation that fits +// the window, where there is nothing above to walk back to. +func TestTurnIndicatorNeedsSomethingAbove(t *testing.T) { + t.Parallel() + + state := readyState() + state.Transcript = []ai.Message{ai.UserText("ONLY"), ai.AssistantText("a short reply")} + + model := fullscreenModel(t, stubController{state: state}, true) + model.Update(tea.WindowSizeMsg{Width: 60, Height: 20}) + model.rerenderTranscript(true) + _ = model.View() + + require.Equal(t, 0, model.transcriptScroll.offset) + assert.False(t, model.turnIndicatorShown()) + assert.False(t, model.frameHit.turnIndicator) +}