diff --git a/docs/CLI.md b/docs/CLI.md index 7090b17c0a..d83b9eaf4c 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -58,6 +58,29 @@ notice. Legacy `[cli].update_channel` values are ignored and removed the next time Reasonix saves the configuration. The `reasonix update` alias behaves the same way. +`[cli].diff_fences = true` renders a fenced ` ```diff ` / ` ```patch ` block +through the colourised diff renderer (add/remove backgrounds, `+`/`-` gutter, +line numbers) instead of the plain code rail. It is **off by default**: a model +writes headerless diff fences often, and the plain rail is the lossless default. +A fence section with no `--- `/`+++ ` file header falls back to the plain rail so +no line is dropped. Like `[cli].diff_formatter` it is user/global only — a +project-local `reasonix.toml` cannot set it. + +`[cli].diff_formatter` names an optional external command that formats a diff +before the CLI/TUI emits it — a fenced ` ```diff ` / ` ```patch ` block, a writer +tool's diff card, and a shell result whose whole output is a diff (see +`[agent].embedded_diff_detection` below). + +It is an argv line run without a shell, e.g. `delta --color-only --paging=never`. + +The whole diff is written to the command's stdin; its stdout is re-emitted with +non-SGR control sequences stripped, so the formatter's colours survive but a +cursor or clipboard escape cannot. + +On any failure, timeout, or empty output the built-in renderer is used. Like +`[cli].update_channel` it is user/global only — a project-local `reasonix.toml` +cannot set it. + ## Configure providers ```sh diff --git a/docs/CLI.zh-CN.md b/docs/CLI.zh-CN.md index fe9a8d59ea..fcc71985fd 100644 --- a/docs/CLI.zh-CN.md +++ b/docs/CLI.zh-CN.md @@ -52,6 +52,25 @@ reasonix upgrade --force # 重新安装当前正式版 `[cli].update_channel` 值不再影响更新,并会在 Reasonix 下次保存配置时移除。别名 `reasonix update` 的行为完全相同。 +`[cli].diff_fences = true` 会把围栏 ` ```diff ` / ` ```patch ` 块交给彩色 diff +渲染器(增删背景、`+`/`-` 侧栏、行号),而不是普通代码栏。它**默认关闭**:模型经常 +写出没有文件头的 diff 围栏,普通代码栏才是无损的默认。没有 `--- `/`+++ ` 文件头的 +分段会回退到普通代码栏,不会丢行。与 `[cli].diff_formatter` 一样,它仅属于用户/全局 +配置,项目内的 `reasonix.toml` 无法设置。 + +`[cli].diff_formatter` 指定一个可选的外部命令,在 CLI/TUI 写入 diff 之前对其格式化: +既包括回答流中的围栏 ` ```diff ` / ` ```patch ` 块,也包括 transcript 中写文件工具的 +diff 卡片,以及整段输出就是 diff 的 shell 结果(见下方 +`[agent].embedded_diff_detection`)。 + +它是 argv 形式、不经过 shell,例如 `delta --color-only --paging=never`。 + +整个 diff 写入该命令的 stdin,其 stdout 在去掉非 SGR 控制序列后回写,因此格式化器的 +颜色得以保留,而光标或剪贴板转义无法生效。 + +命令失败、超时或输出为空时回退到内置渲染器。与 `[cli].update_channel` 一样,它仅属于 +用户/全局配置,项目内的 `reasonix.toml` 无法设置。 + ## 配置供应商 ```sh diff --git a/docs/CONFIG_PATHS.md b/docs/CONFIG_PATHS.md index 138b8736a8..bb12dca9e1 100644 --- a/docs/CONFIG_PATHS.md +++ b/docs/CONFIG_PATHS.md @@ -91,6 +91,10 @@ theme = "auto" cursor_shape = "bar" # CLI/TUI text cursor: underline|block|bar show_turn_usage = false # hide per-request token/cost receipts in the TUI; default true +[cli] +diff_fences = true # render a fenced diff/patch block through the colourised diff renderer; default false; user/global only +diff_formatter = "delta --color-only --paging=never" # external formatter for a fenced diff block, a writer diff card, and a whole-diff shell result; user/global only + [desktop] provider_access = ["deepseek"] @@ -120,6 +124,25 @@ visible without covering double-width CJK characters; use `block` or TUI transcript after each model request. Accounting and live status updates remain active. The default is `true`. +`[cli].diff_formatter` names an optional external command that formats a diff +for the CLI/TUI to render — a fenced ` ```diff ` / ` ```patch ` block, a writer +tool's diff card, and a shell result whose whole output is a diff (see +`[agent].embedded_diff_detection` below). + +It is an argv line run without a shell, e.g. `delta --color-only --paging=never`. + +The whole diff is written to the command's stdin; its stdout is re-emitted with +non-SGR control sequences stripped, so the formatter's colours survive but a +cursor or clipboard escape cannot. + +In the full-screen TUI it runs off the render path: the built-in rows are drawn +first and replaced once its output is ready, so a slow formatter cannot freeze +the UI. + +On any failure, timeout, or empty output the built-in renderer is kept. Like +`[cli].update_channel` it is user/global only — a project-local `reasonix.toml` +cannot set it. + ### Custom provider `api_key_env` names When a custom provider is added from the desktop settings or `reasonix setup`, diff --git a/internal/contract/config/cli_diff_formatter_test.go b/internal/contract/config/cli_diff_formatter_test.go new file mode 100644 index 0000000000..1405a94639 --- /dev/null +++ b/internal/contract/config/cli_diff_formatter_test.go @@ -0,0 +1,103 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/BurntSushi/toml" + + "reasonix/internal/base/testenv" +) + +// TestDiffFormatterIsUserGlobal proves a repository-local reasonix.toml cannot run +// an external diff formatter on the user's machine. +func TestDiffFormatterIsUserGlobal(t *testing.T) { + home := testenv.TempDir(t) + root := testenv.TempDir(t) + t.Setenv("REASONIX_HOME", home) + if err := os.WriteFile(filepath.Join(home, "config.toml"), []byte("[cli]\ndiff_formatter = \"cat\"\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "reasonix.toml"), []byte("[cli]\ndiff_formatter = \"rm -rf /\"\n"), 0o600); err != nil { + t.Fatal(err) + } + cfg, err := LoadForRootReadOnly(root) + if err != nil { + t.Fatal(err) + } + if got := cfg.CLI.DiffFormatter; got != "cat" { + t.Fatalf("project diff_formatter leaked: got %q, want the user-global %q", got, "cat") + } +} + +// TestDiffFormatterRendering proves the user-global [cli] section renders when set +// and never appears in a project-scoped render. +func TestDiffFormatterRendering(t *testing.T) { + cfg := Default() + cfg.CLI.DiffFormatter = "delta --color-only --paging=never" + + user := RenderTOMLForScope(cfg, RenderScopeUser) + if !strings.Contains(user, "[cli]") || !strings.Contains(user, `diff_formatter = "delta --color-only --paging=never"`) { + t.Fatalf("user render missing diff_formatter:\n%s", user) + } + project := RenderTOMLForScope(cfg, RenderScopeProject) + if strings.Contains(project, "[cli]") || strings.Contains(project, "diff_formatter") { + t.Fatalf("project render contains user-global CLI config:\n%s", project) + } +} + +// TestDiffFormatterRoundTrips proves the rendered key reloads into the same field. +func TestDiffFormatterRoundTrips(t *testing.T) { + cfg := Default() + cfg.CLI.DiffFormatter = "delta --paging=never" + + var got Config + if _, err := toml.Decode(RenderTOMLForScope(cfg, RenderScopeUser), &got); err != nil { + t.Fatalf("rendered TOML does not parse: %v", err) + } + if got.CLI.DiffFormatter != cfg.CLI.DiffFormatter { + t.Fatalf("round-trip diff_formatter = %q, want %q", got.CLI.DiffFormatter, cfg.CLI.DiffFormatter) + } +} + +// TestDiffFencesDefaultsOff proves fenced diff rendering is opt-in. +func TestDiffFencesDefaultsOff(t *testing.T) { + if Default().CLI.DiffFences { + t.Fatal("diff_fences is on by default") + } +} + +// TestDiffFencesIsUserGlobal proves a repository-local reasonix.toml cannot turn +// on diff-fence rendering, and that the user-global render emits it only when set. +func TestDiffFencesIsUserGlobal(t *testing.T) { + home := testenv.TempDir(t) + root := testenv.TempDir(t) + t.Setenv("REASONIX_HOME", home) + if err := os.WriteFile(filepath.Join(home, "config.toml"), []byte("[cli]\ndiff_fences = true\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "reasonix.toml"), []byte("[cli]\ndiff_fences = false\n"), 0o600); err != nil { + t.Fatal(err) + } + cfg, err := LoadForRootReadOnly(root) + if err != nil { + t.Fatal(err) + } + if !cfg.CLI.DiffFences { + t.Fatal("project diff_fences leaked over the user-global value") + } + user := RenderTOMLForScope(cfg, RenderScopeUser) + if !strings.Contains(user, "[cli]") || !strings.Contains(user, "diff_fences = true") { + t.Fatalf("user render missing diff_fences:\n%s", user) + } + project := RenderTOMLForScope(cfg, RenderScopeProject) + if strings.Contains(project, "diff_fences") { + t.Fatalf("project render contains user-global CLI config:\n%s", project) + } + // Off is the default, so an unset field renders no [cli] section at all. + if off := RenderTOMLForScope(Default(), RenderScopeUser); strings.Contains(off, "diff_fences") { + t.Fatalf("default render emitted diff_fences:\n%s", off) + } +} diff --git a/internal/contract/config/config.go b/internal/contract/config/config.go index 9f20a4267a..a3200d1689 100644 --- a/internal/contract/config/config.go +++ b/internal/contract/config/config.go @@ -262,12 +262,22 @@ type UIConfig struct { // CLIConfig controls user-global native CLI behavior. It is separate from // project runtime settings so a repository cannot change the installed -// binary's update channel. +// binary's update channel, run an external diff formatter, or switch the +// diff renderer. type CLIConfig struct { // UpdateChannel is decoded for compatibility with pre-single-channel // configurations. Runtime behavior is always the official release channel, // and the canonical renderer intentionally drops this field. UpdateChannel string `toml:"update_channel"` + // DiffFormatter is an optional external command (argv, no shell) that formats + // a fenced ```diff/```patch block and a writer diff card, e.g. "delta". Empty + // uses the built-in renderer. User-global only; ./reasonix.toml cannot set it. + DiffFormatter string `toml:"diff_formatter"` + // DiffFences opts into rendering a fenced ```diff/```patch block through the + // colourised diff renderer instead of the plain code rail. Default false: + // a model writes headerless diff fences often, and the plain rail is the + // lossless default. User-global only; ./reasonix.toml cannot set it. + DiffFences bool `toml:"diff_fences"` } // DesktopExternalOpener returns the selected opener id; unavailable ids fall diff --git a/internal/contract/config/load.go b/internal/contract/config/load.go index 6bae91fe50..cbb627c180 100644 --- a/internal/contract/config/load.go +++ b/internal/contract/config/load.go @@ -145,8 +145,8 @@ func (r Roots) loadForRoot(root string, migrateOnDisk bool) (*Config, error) { } else if projectMeta.IsDefined("agent", "system_prompt_file") { cfg.systemPromptFileSource = promptFileSourceProject } - // The native CLI update channel controls the one user-installed binary. - // A repository-local reasonix.toml must never switch that global choice. + // The whole [cli] table is user-global: the update channel picks the one + // installed binary, and a repository must not run a formatter or redraw diffs. cfg.CLI = globalCLI // Secret protection is a user-global security control: a cloned repo's // reasonix.toml must not be able to flip on the workflow-breaking env/path diff --git a/internal/contract/config/render.go b/internal/contract/config/render.go index f5071edf8a..bb29c25222 100644 --- a/internal/contract/config/render.go +++ b/internal/contract/config/render.go @@ -86,6 +86,7 @@ func RenderTOMLForScope(c *Config, scope RenderScope) string { } if scope != RenderScopeProject { + renderCLIConfig(&b, c, scope) renderDesktopSection(&b, c) } else if c.Desktop.ProviderAccess != nil { // provider_access is intentionally mergeable across user and project diff --git a/internal/contract/config/render_cli.go b/internal/contract/config/render_cli.go new file mode 100644 index 0000000000..dd2714cfe1 --- /dev/null +++ b/internal/contract/config/render_cli.go @@ -0,0 +1,28 @@ +package config + +import ( + "fmt" + "strings" +) + +// renderCLIConfig writes the [cli] section, which is user-global only: a +// repo-local reasonix.toml must never run an external diff formatter. The +// section is emitted only when set, so the retired update_channel key never +// reappears. +func renderCLIConfig(b *strings.Builder, c *Config, scope RenderScope) { + if scope == RenderScopeProject { + return + } + cmd := strings.TrimSpace(c.CLI.DiffFormatter) + if cmd == "" && !c.CLI.DiffFences { + return + } + b.WriteString("[cli]\n") + if cmd != "" { + fmt.Fprintf(b, "diff_formatter = %q # external argv (no shell) that formats a fenced diff block and a writer diff card; the diff is piped on stdin and its stdout re-emitted with non-SGR escapes stripped; user-global only\n", cmd) + } + if c.CLI.DiffFences { + b.WriteString("diff_fences = true # render a fenced ```diff/```patch block through the colourised diff renderer instead of the plain code rail; default false; user-global only\n") + } + b.WriteString("\n") +} diff --git a/internal/frontend/termrender/diffformatter.go b/internal/frontend/termrender/diffformatter.go new file mode 100644 index 0000000000..a47f9b468b --- /dev/null +++ b/internal/frontend/termrender/diffformatter.go @@ -0,0 +1,284 @@ +// This file implements the optional external diff formatter: a user-global +// command (e.g. delta) that formats a diff before the CLI emits it — both a +// fenced ```diff / ```patch block in the conversation answer stream and a writer +// tool's diff card in the transcript. Configured via [cli].diff_formatter as an +// argv line and exec'd directly — no shell. +package termrender + +import ( + "bytes" + "context" + "os" + "os/exec" + "strconv" + "strings" + "sync" + "time" + + "reasonix/internal/contract/config" +) + +const ( + // diffFormatTimeout bounds the external formatter so a hung command cannot + // stall a render (inline) or hold a background run open; on timeout the + // built-in path is used. + diffFormatTimeout = 3 * time.Second + // diffFormatWaitDelay bounds the wait for a formatter's pipes to close after + // the process exits or the timeout fires. Without it a formatter that leaves + // a child holding stdout (sh -c 'sleep 12 & cat') blocks Wait for the + // child's lifetime, so the timeout does not actually bound the run. + diffFormatWaitDelay = 500 * time.Millisecond + // diffFormatMaxBytes skips the external formatter for very large blocks, + // where the subprocess round-trip costs more than it saves and the built-in + // renderer (which folds) is the better default. + diffFormatMaxBytes = 1 << 20 // 1 MiB +) + +// activeDiffFormatter is the argv of the configured [cli].diff_formatter; empty +// when unset. Resolved once at CLI startup, like activeTheme. +var activeDiffFormatter []string + +// configureDiffFormatter resolves [cli].diff_formatter into activeDiffFormatter. +// It is user-global only: a repo-local reasonix.toml cannot run a command on the +// user's machine. +func configureDiffFormatter(cfg *config.Config) { + var argv []string + if cfg != nil { + argv = splitDiffFormatter(cfg.CLI.DiffFormatter) + } + diffFormatMu.Lock() + activeDiffFormatter = argv + diffFormatCache, diffFormatOrder = map[diffFormatKey]diffFormatEntry{}, nil + diffFormatInflight = map[diffFormatKey]bool{} + diffFormatDisabled = false + diffFormatMu.Unlock() +} + +// SetDiffFormatterForTest installs the [cli].diff_formatter argv for a test and +// clears the memo so a case never reads another's result. Restore with the +// returned function; production configures it from the user's config. +func SetDiffFormatterForTest(argv []string) (restore func()) { + diffFormatMu.Lock() + prev := activeDiffFormatter + activeDiffFormatter = argv + diffFormatCache, diffFormatOrder = map[diffFormatKey]diffFormatEntry{}, nil + diffFormatInflight = map[diffFormatKey]bool{} + diffFormatMu.Unlock() + return func() { SetDiffFormatterForTest(prev) } +} + +// SetDiffFormatNotify registers fn as the re-render hook: the formatter calls it +// from its goroutine once a background run lands, so the frontend can repaint and +// pick up the result. Pass nil to run the formatter inline instead — a renderer +// that cannot re-render then still shows the output on the frame it draws. +func SetDiffFormatNotify(fn func()) { + diffFormatMu.Lock() + diffFormatNotify = fn + diffFormatMu.Unlock() +} + +// splitDiffFormatter splits a configured command line into argv, honouring single +// and double quotes so a path with spaces survives. It performs no shell +// expansion — the argv is exec'd directly. +func splitDiffFormatter(s string) []string { + var argv []string + var cur strings.Builder + haveToken := false + var quote byte // 0 = none, '\'' or '"' + for i := range len(s) { + c := s[i] + switch { + case quote != 0: + if c == quote { + quote = 0 + } else { + cur.WriteByte(c) + } + case c == '\'' || c == '"': + quote = c + haveToken = true + case c == ' ' || c == '\t': + if haveToken { + argv = append(argv, cur.String()) + cur.Reset() + haveToken = false + } + default: + cur.WriteByte(c) + haveToken = true + } + } + if haveToken { + argv = append(argv, cur.String()) + } + return argv +} + +// renderDiffExternal pipes the unified diff through argv (exec, no shell) and +// returns its stdout clamped to width. The formatter is told the terminal width +// through COLUMNS so a width-aware one (e.g. delta) can wrap; clamping is the +// backstop for one that ignores it. ok is false when the command is empty, the +// diff is too large, or the run fails or times out — callers keep the built-in. +func renderDiffExternal(argv []string, diff string, width int) (string, bool) { + if len(argv) == 0 || diff == "" || len(diff) > diffFormatMaxBytes { + return "", false + } + ctx, cancel := context.WithTimeout(context.Background(), diffFormatTimeout) + defer cancel() + cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) + cmd.WaitDelay = diffFormatWaitDelay + cmd.Stdin = strings.NewReader(diff) + if width > 0 { + cmd.Env = append(os.Environ(), "COLUMNS="+strconv.Itoa(width)) + } + var out bytes.Buffer + cmd.Stdout = &out + err := cmd.Run() + if ctx.Err() == context.DeadlineExceeded { + diffFormatMu.Lock() + diffFormatDisabled = true + diffFormatMu.Unlock() + } + if err != nil || out.Len() == 0 { + return "", false + } + return clampLines(out.String(), width), true +} + +// clampLines truncates each line to width columns (ANSI-aware, tabs expanded), +// so a formatter's over-wide output cannot overflow the viewport. +func clampLines(s string, width int) string { + if width <= 0 { + return s + } + lines := strings.Split(s, "\n") + for i, ln := range lines { + lines[i] = clampPlain(ln, width) + } + return strings.Join(lines, "\n") +} + +// formatToolDiff pipes a writer call's unified diff through the configured +// [cli].diff_formatter and returns its stdout as transcript rows, indented to +// the card's body column. ok is false when no formatter is configured, a +// background run is still in flight, or it fails, times out, or overshoots the +// size cap — the caller then keeps the built-in renderer. +func formatToolDiff(diff string, width int) ([]string, bool) { + // The card indents the body by two columns, so the formatter must target + // width-2: clamping to the full width and then indenting pushes an over-wide + // line past the viewport, where it wraps back to column 0. + body := width + if width > 0 { + body = max(width-2, 1) + } + out, ok := formatDiffCached(diff, body) + if !ok { + return nil, false + } + lines := strings.Split(strings.TrimRight(out, "\n"), "\n") + rows := make([]string, 0, len(lines)) + for _, ln := range lines { + rows = append(rows, " "+ln) + } + return rows, true +} + +const diffFormatCacheMax = 32 + +type diffFormatKey struct { + content string + width int +} + +type diffFormatEntry struct { + text string + ok bool +} + +var ( + diffFormatMu sync.Mutex + diffFormatCache = map[diffFormatKey]diffFormatEntry{} + diffFormatOrder []diffFormatKey + diffFormatInflight = map[diffFormatKey]bool{} + // diffFormatDisabled latches once a run hits diffFormatTimeout. A formatter + // that hangs once hangs again, and each retry costs a full timeout — worst on + // a resize, which changes the width key and so misses the per-(content,width) + // cache. Once latched the built-in renderer is used. + diffFormatDisabled bool + // diffFormatNotify, when set, makes the run asynchronous and is called from + // the formatter goroutine once a result lands, so the frontend repaints and + // picks it up. Nil runs the formatter inline. + diffFormatNotify func() +) + +// formatDiffCached returns the formatter's output for (diff, width), memoized +// per key. With a re-render hook set (SetDiffFormatNotify) the run goes to a +// background goroutine and this returns ok=false at once, so the caller draws +// the built-in rows and the frontend repaints when the result lands; without a +// hook the run is inline, so a renderer that cannot re-render still shows it. +func formatDiffCached(diff string, width int) (string, bool) { + if len(activeDiffFormatter) == 0 { + return "", false + } + key := diffFormatKey{content: diff, width: width} + diffFormatMu.Lock() + if diffFormatDisabled { + diffFormatMu.Unlock() + return "", false + } + if got, hit := diffFormatCache[key]; hit { + diffFormatMu.Unlock() + return got.text, got.ok + } + async := diffFormatNotify != nil + if async { + if diffFormatInflight[key] { + diffFormatMu.Unlock() + return "", false + } + diffFormatInflight[key] = true + } + argv := activeDiffFormatter + diffFormatMu.Unlock() + + if async { + go func() { + raw, ok := renderDiffExternal(argv, diff, width) + if ok { + raw = sgrOnly(raw) + } + diffFormatMu.Lock() + delete(diffFormatInflight, key) + storeDiffFormat(key, raw, ok) + notify := diffFormatNotify + diffFormatMu.Unlock() + if ok && notify != nil { + notify() + } + }() + return "", false + } + + raw, ok := renderDiffExternal(argv, diff, width) + if ok { + raw = sgrOnly(raw) + } + diffFormatMu.Lock() + storeDiffFormat(key, raw, ok) + diffFormatMu.Unlock() + return raw, ok +} + +// storeDiffFormat records a result under key, evicting the oldest entry when the +// memo is full. Callers hold diffFormatMu. +func storeDiffFormat(key diffFormatKey, text string, ok bool) { + if _, seen := diffFormatCache[key]; seen { + return + } + if len(diffFormatOrder) >= diffFormatCacheMax { + delete(diffFormatCache, diffFormatOrder[0]) + diffFormatOrder = diffFormatOrder[1:] + } + diffFormatCache[key] = diffFormatEntry{text: text, ok: ok} + diffFormatOrder = append(diffFormatOrder, key) +} diff --git a/internal/frontend/termrender/diffformatter_test.go b/internal/frontend/termrender/diffformatter_test.go new file mode 100644 index 0000000000..1d741685b5 --- /dev/null +++ b/internal/frontend/termrender/diffformatter_test.go @@ -0,0 +1,358 @@ +package termrender + +import ( + "os" + "path/filepath" + "reflect" + "runtime" + "strings" + "testing" + "time" + + "github.com/charmbracelet/colorprofile" + + "reasonix/internal/contract/event" +) + +func TestSplitDiffFormatter(t *testing.T) { + cases := []struct { + in string + want []string + }{ + {"", nil}, + {" ", nil}, + {"delta --color-only --paging=never", []string{"delta", "--color-only", "--paging=never"}}, + {`delta --syntax-theme "Monokai Extended"`, []string{"delta", "--syntax-theme", "Monokai Extended"}}, + {`my tool 'a b' c`, []string{"my", "tool", "a b", "c"}}, + {`x ""`, []string{"x", ""}}, + } + for _, tc := range cases { + if got := splitDiffFormatter(tc.in); !reflect.DeepEqual(got, tc.want) { + t.Errorf("splitDiffFormatter(%q) = %#v, want %#v", tc.in, got, tc.want) + } + } +} + +func TestRenderDiffExternalPassesStdinToStdout(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no portable stdin→stdout filter") + } + out, ok := renderDiffExternal([]string{"cat"}, "--- a/x\n+++ b/x\n", 0) + if !ok { + t.Fatal("renderDiffExternal(cat) failed") + } + if out != "--- a/x\n+++ b/x\n" { + t.Fatalf("stdout = %q", out) + } +} + +// A width-aware formatter sees COLUMNS, and its output is clamped to width so a +// formatter that ignores COLUMNS cannot overflow the viewport. +func TestRenderDiffExternalPassesWidthAndClamps(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no portable stdin→stdout filter") + } + // `cat` ignores COLUMNS; the over-wide line is clamped to 10 columns. + out, ok := renderDiffExternal([]string{"cat"}, strings.Repeat("x", 200)+"\n", 10) + if !ok { + t.Fatal("renderDiffExternal(cat) failed") + } + if got := VisibleWidth(strings.TrimRight(out, "\n")); got != 10 { + t.Fatalf("over-wide output not clamped: width %d (%q)", got, out) + } +} + +// A formatter that exits but leaves a child holding stdout must not block Wait +// for the child's lifetime: cmd.WaitDelay bounds the run (and the misbehaving +// formatter is rejected, so the caller falls back to the built-in renderer). +func TestRenderDiffExternalBoundsAChildHoldingStdout(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no portable shell") + } + start := time.Now() + _, _ = renderDiffExternal([]string{"sh", "-c", "sleep 5 & echo done"}, "diff\n", 80) + if elapsed := time.Since(start); elapsed > 3*time.Second { + t.Fatalf("renderDiffExternal blocked %v on a child holding stdout", elapsed) + } +} + +func TestRenderDiffExternalFallsBackOnError(t *testing.T) { + if _, ok := renderDiffExternal([]string{"reasonix-no-such-command-xyz"}, "diff", 80); ok { + t.Fatal("expected ok=false for a missing command") + } + if _, ok := renderDiffExternal(nil, "diff", 80); ok { + t.Fatal("expected ok=false for empty argv") + } + if _, ok := renderDiffExternal([]string{"cat"}, "", 80); ok { + t.Fatal("expected ok=false for empty diff") + } + if _, ok := renderDiffExternal([]string{"cat"}, strings.Repeat("x", diffFormatMaxBytes+1), 80); ok { + t.Fatal("expected ok=false for an oversized diff") + } +} + +func TestDiffBodyVerbatimWhenColourised(t *testing.T) { + // SGR-coloured input is shown as-is (no line-number gutter added). + d := event.FileDiff{Diff: "\x1b[32m+added\x1b[0m\n\x1b[31m-removed\x1b[0m\n"} + rows := diffBody(d, "x.go", 80, 0) + if len(rows) != 2 { + t.Fatalf("want 2 verbatim rows, got %d: %#v", len(rows), rows) + } + for _, r := range rows { + if !strings.Contains(r, "\x1b[") { + t.Fatalf("verbatim row lost its colour: %q", r) + } + if strings.Contains(r, "\x1b[48;5;") { + t.Fatalf("verbatim row should not carry a background bar: %q", r) + } + } +} + +func TestSGROnlyKeepsColourDropsHijack(t *testing.T) { + in := "\x1b[32mgreen\x1b[0m\x1b]52;c;aGk=\x07\x1b[2Jtext" + got := sgrOnly(in) + if !strings.Contains(got, "\x1b[32m") || !strings.Contains(got, "\x1b[0m") { + t.Fatalf("SGR dropped: %q", got) + } + if strings.Contains(got, "\x1b]") || strings.Contains(got, "\x1b[2J") { + t.Fatalf("non-SGR control sequence leaked: %q", got) + } + if !strings.Contains(got, "green") || !strings.Contains(got, "text") { + t.Fatalf("visible text lost: %q", got) + } +} + +func TestRenderDiffFenceVerbatimWhenColourised(t *testing.T) { + defer func(prev colorprofile.Profile) { activeColorProfile = prev }(activeColorProfile) + activeColorProfile = colorprofile.ANSI256 + defer enableDiffFences(t)() + r := NewMarkdownRenderer(80) + out := r.Render("```diff\n\x1b[32m+added\x1b[0m\n\x1b[31m-removed\x1b[0m\n```\n") + if !strings.Contains(out, "\x1b[32m+added") || !strings.Contains(out, "\x1b[31m-removed") { + t.Fatalf("coloured diff fence lost its SGR:\n%q", out) + } + if strings.Contains(out, "\x1b[48;5;") { + t.Fatalf("coloured diff fence should not be re-parsed into background bars:\n%q", out) + } +} + +// A configured [cli].diff_formatter runs the whole fence body through the +// command and its stdout is re-emitted under the fence — the formatter's SGR +// survives, but a non-SGR control sequence it emitted does not. +func TestRenderDiffFenceSanitisesConfiguredFormatter(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no portable stdin→stdout filter") + } + defer setDiffFormatter(nil) + setDiffFormatter([]string{"cat"}) + defer enableDiffFences(t)() + + r := NewMarkdownRenderer(80) + body := "--- a/x\n+++ b/x\n\x1b]52;c;aGk=\x07\x1b[31m-red\x1b[0m\n" + out := r.Render("```diff\n" + body + "```\n") + if strings.Contains(out, "\x1b]52;c;aGk=\x07") { + t.Fatalf("formatter's OSC was not filtered:\n%q", out) + } + if !strings.Contains(out, "\x1b[31m-red\x1b[0m") { + t.Fatalf("formatter output lost its SGR:\n%q", out) + } + if strings.Contains(out, "\x1b[48;5;") { + t.Fatalf("formatted fence should not be re-rendered with background bars:\n%q", out) + } +} + +// An open fence (no closing ```) must not run the external formatter: every +// streamed delta would otherwise spawn a subprocess that can never hit the +// memo. The built-in rows are drawn until the closing fence arrives. +func TestRenderDiffFenceOpenFenceSkipsFormatter(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no portable stdin→stdout filter") + } + defer setDiffFormatter(nil) + // A formatter that marks every line; the marker only appears once closed. + setDiffFormatter([]string{"sed", "s/^/MARK:/"}) + defer enableDiffFences(t)() + + r := NewMarkdownRenderer(80) + open := r.Render("```diff\n--- a/x\n+++ b/x\n@@ -1 +1 @@\n-old\n+new\n") + if strings.Contains(open, "MARK:") { + t.Fatalf("open fence ran the external formatter:\n%q", open) + } + if !strings.Contains(open, "old") || !strings.Contains(open, "new") { + t.Fatalf("open fence dropped content:\n%q", open) + } + closed := r.Render("```diff\n--- a/x\n+++ b/x\n@@ -1 +1 @@\n-old\n+new\n```\n") + if !strings.Contains(closed, "MARK:") { + t.Fatalf("closed fence skipped the external formatter:\n%q", closed) + } +} + +// A streamed answer re-renders the growing fence on every delta; the formatter +// must run once, on the closed fence, not once per delta. +func TestRenderDiffFenceStreamingRunsFormatterOnce(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no portable shell") + } + countFile := filepath.Join(t.TempDir(), "runs") + t.Setenv("COUNTFILE", countFile) + defer setDiffFormatter(nil) + setDiffFormatter([]string{"sh", "-c", `echo run >> "$COUNTFILE"; cat`}) + defer enableDiffFences(t)() + + r := NewMarkdownRenderer(80) + full := "```diff\n--- a/x\n+++ b/x\n@@ -1 +1 @@\n-old\n+new\n" + for i := 1; i <= len(full); i++ { + r.Render(full[:i]) + } + if data, err := os.ReadFile(countFile); err == nil && len(data) != 0 { + t.Fatalf("formatter ran on open fences:\n%s", data) + } + r.Render(full + "```\n") + data, err := os.ReadFile(countFile) + if err != nil { + t.Fatalf("formatter never ran: %v", err) + } + if got := strings.Count(string(data), "\n"); got != 1 { + t.Fatalf("formatter ran %d times, want 1", got) + } +} + +// Without a configured formatter the fence keeps the built-in renderer. +func TestRenderDiffFenceFallsBackWithoutFormatter(t *testing.T) { + defer func(prev []string) { activeDiffFormatter = prev }(activeDiffFormatter) + activeDiffFormatter = nil + defer enableDiffFences(t)() + + r := NewMarkdownRenderer(80) + out := r.Render("```diff\n--- a/x\n+++ b/x\n@@ -1 +1 @@\n-old\n+new\n```\n") + if !strings.Contains(out, "new") || !strings.Contains(out, "old") { + t.Fatalf("built-in renderer lost the diff body:\n%q", out) + } +} + +// A configured [cli].diff_formatter also formats a writer tool's diff card: the +// whole diff is piped through the command and its stdout is re-emitted under +// the card header, SGR kept and other escapes stripped, with no fold (maxLines +// is ignored), mirroring the fenced path. +func TestDiffBlockSanitisesConfiguredFormatter(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no portable stdin→stdout filter") + } + defer setDiffFormatter(nil) + setDiffFormatter([]string{"cat"}) + + d := event.FileDiff{ + Diff: "--- a/x.go\n+++ b/x.go\n@@ -1 +1 @@\n\x1b]52;c;aGk=\x07\x1b[31m-old\x1b[0m\n+new\n", + Added: 1, + Removed: 1, + } + block := DiffBlock("edit_file", `{"path":"pkg/x.go"}`, d, 80, 1) + if len(block) == 0 || !strings.Contains(block[0], "pkg/x.go") { + t.Fatalf("header should name the path, got %q", block[0]) + } + body := strings.Join(block[1:], "\n") + if strings.Contains(body, "\x1b]52;c;aGk=\x07") { + t.Fatalf("formatter's OSC was not filtered:\n%q", body) + } + // The raw "--- /+++ " pair the built-in path drops must not reach the + // formatter either, or the card re-emits a host path it already names. + if strings.Contains(body, "--- a/x.go") || strings.Contains(body, "+++ b/x.go") { + t.Fatalf("formatter path should drop the raw file headers:\n%q", body) + } + if !strings.Contains(body, "\x1b[31m-old\x1b[0m") { + t.Fatalf("formatter output lost its body:\n%q", body) + } + if strings.Contains(body, "\x1b[48;5;") { + t.Fatalf("formatted card should not be re-rendered with background bars:\n%q", body) + } + if strings.Contains(body, "more lines") { + t.Fatalf("formatted card should not fold even past maxLines:\n%q", body) + } + for _, row := range block[1:] { + if !strings.HasPrefix(row, " ") { + t.Fatalf("body row should be indented to the card column, got %q", row) + } + } +} + +// Without a configured formatter the writer diff card keeps the built-in +// renderer (headers dropped, line-number gutter added). +func TestDiffBlockFallsBackWithoutFormatter(t *testing.T) { + defer func(prev []string) { activeDiffFormatter = prev }(activeDiffFormatter) + activeDiffFormatter = nil + + d := event.FileDiff{Diff: "--- a/x.go\n+++ b/x.go\n@@ -1 +1 @@\n-old\n+new\n", Added: 1, Removed: 1} + body := strings.Join(DiffBlock("edit_file", `{"path":"x.go"}`, d, 80, 40)[1:], "\n") + if !strings.Contains(body, "old") || !strings.Contains(body, "new") { + t.Fatalf("built-in renderer lost the diff body:\n%q", body) + } + if strings.Contains(body, "--- a/x.go") || strings.Contains(body, "+++ b/x.go") { + t.Fatalf("built-in renderer should drop the file headers:\n%q", body) + } +} + +// A formatter that fails to run leaves the writer diff card on the built-in +// renderer. +func TestDiffBlockFallsBackWhenFormatterFails(t *testing.T) { + defer setDiffFormatter(nil) + setDiffFormatter([]string{"reasonix-no-such-command-xyz"}) + + d := event.FileDiff{Diff: "--- a/x.go\n+++ b/x.go\n@@ -1 +1 @@\n-old\n+new\n", Added: 1, Removed: 1} + body := strings.Join(DiffBlock("edit_file", `{"path":"x.go"}`, d, 80, 40)[1:], "\n") + if !strings.Contains(body, "old") || !strings.Contains(body, "new") { + t.Fatalf("built-in renderer lost the diff body:\n%q", body) + } +} + +// setDiffFormatter installs a formatter for a test and drops the memo so a case +// never reads a result another case cached. +func setDiffFormatter(argv []string) { SetDiffFormatterForTest(argv) } + +// With a re-render hook registered the run is asynchronous: the call returns +// not-ok at once (so the caller draws the built-in rows), the hook fires when +// the result lands, and the memo then serves it. +func TestFormatDiffCachedAsyncWhenNotifyHookSet(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no portable stdin→stdout filter") + } + defer setDiffFormatter(nil) + setDiffFormatter([]string{"cat"}) + + done := make(chan struct{}, 1) + SetDiffFormatNotify(func() { done <- struct{}{} }) + defer SetDiffFormatNotify(nil) + + if _, ok := formatDiffCached("body", 80); ok { + t.Fatal("async call should report not-ok so the caller draws built-in rows") + } + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("notify hook never fired") + } + if got, ok := formatDiffCached("body", 80); !ok || got != "body" { + t.Fatalf("memo not populated after the run landed: %q, %v", got, ok) + } +} + +// The memo answers a repeated (content, width) without spawning the command +// again, and a new width is a new key. +func TestFormatDiffCachedMemoisesByContentAndWidth(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("no portable stdin→stdout filter") + } + defer setDiffFormatter(nil) + setDiffFormatter([]string{"cat"}) + + if got, ok := formatDiffCached("body", 80); !ok || got != "body" { + t.Fatalf("formatDiffCached = %q, %v", got, ok) + } + // A command that cannot run: a cache hit must not consult it. + activeDiffFormatter = []string{"reasonix-no-such-command-xyz"} + if got, ok := formatDiffCached("body", 80); !ok || got != "body" { + t.Fatalf("cache miss re-ran the formatter: %q, %v", got, ok) + } + if _, ok := formatDiffCached("body", 100); ok { + t.Fatal("a new width should not reuse another width's entry") + } +} diff --git a/internal/frontend/termrender/diffview.go b/internal/frontend/termrender/diffview.go index 61c1b1619e..4eb9949289 100644 --- a/internal/frontend/termrender/diffview.go +++ b/internal/frontend/termrender/diffview.go @@ -64,6 +64,9 @@ func diffPath(args string) string { // DiffBlock renders a writer call as a header line ("✎ name path +A -B") plus // the highlighted, folded diff body. Returns nil when there's no textual diff. +// A configured [cli].diff_formatter (e.g. delta) takes the whole diff on stdin +// and its stdout is re-emitted under the header with non-SGR escapes stripped, +// mirroring the fenced-diff path — the built-in body (and its fold) is the fallback. func DiffBlock(name, args string, d event.FileDiff, width, maxLines int) []string { if d.Diff == "" { return nil @@ -73,9 +76,24 @@ func DiffBlock(name, args string, d event.FileDiff, width, maxLines int) []strin if stat := DiffStat(d); stat != "" { header += " " + stat } + if rows, ok := formatToolDiff(dropFileHeaderPair(d.Diff), width); ok { + return append([]string{header}, rows...) + } return append([]string{header}, diffBody(d, path, width, maxLines)...) } +// dropFileHeaderPair removes a leading "--- "/"+++ " pair, the same pair the +// built-in body drops positionally. The external formatter would otherwise +// re-emit that raw header — including an absolute host path in the label — which +// the card header already names. +func dropFileHeaderPair(diff string) string { + lines := strings.SplitN(diff, "\n", 3) + if len(lines) == 3 && strings.HasPrefix(lines[0], "--- ") && strings.HasPrefix(lines[1], "+++ ") { + return lines[2] + } + return diff +} + // diffBody renders the hunks with a line-number gutter, dropping the file and // "@@" headers (a dim "⋮" marks each hunk jump) and folding past maxLines to a // "+N more" footer. path selects the syntax lexer. diff --git a/internal/frontend/termrender/md.go b/internal/frontend/termrender/md.go index 3529eefaa6..ba937b7d0c 100644 --- a/internal/frontend/termrender/md.go +++ b/internal/frontend/termrender/md.go @@ -14,6 +14,7 @@ import ( "github.com/yuin/goldmark/text" "github.com/yuin/goldmark/util" + "reasonix/internal/contract/config" "reasonix/internal/contract/event" ) @@ -344,6 +345,12 @@ func (r *MarkdownRenderer) renderFenced(buf *strings.Builder, n ast.Node, src [] // startup from [cli].diff_fences. var activeDiffFences bool +// configureDiffFences resolves [cli].diff_fences into activeDiffFences. It is +// user-global only, mirroring [cli].diff_formatter. +func configureDiffFences(cfg *config.Config) { + activeDiffFences = cfg != nil && cfg.CLI.DiffFences +} + // isDiffFence reports whether a fence's info string asks for a unified diff. // Copy mode keeps the plain rail path so the copied text stays byte-identical // to the fence source (the diff rows carry render-only gutter/line numbers). @@ -362,6 +369,20 @@ func (r *MarkdownRenderer) renderDiffFence(buf *strings.Builder, fc *ast.FencedC prefix := strings.Repeat(" ", indent) width := max(r.width-indent, 8) text := diffFenceText(fc, src) + // Only run the external formatter once the source actually closed the + // fence. While the fence is still streaming in, every delta changes the + // body, so each redraw would spawn a fresh subprocess that can never hit + // the memo; the built-in rows are drawn until the closing fence arrives. + if fenceClosed(fc, src) { + if out, ok := formatDiffCached(text, width); ok { + buf.WriteString(out) + if !strings.HasSuffix(out, "\n") { + buf.WriteString("\n") + } + buf.WriteString("\n") + return + } + } // A fence that already carries ANSI (e.g. pasted delta output) is shown // verbatim — splitting/counting it would mis-read the SGR-prefixed lines. if hasSGR(text) { @@ -389,6 +410,19 @@ func (r *MarkdownRenderer) renderDiffFence(buf *strings.Builder, fc *ast.FencedC buf.WriteString("\n") } +// fenceClosed reports whether src actually carried the closing fence line for +// fc. goldmark still builds a FencedCodeBlock for a fence left open at EOF, so +// its Lines() run to the end of the source; a closed fence's last body line +// stops before its closing line. A block with no body lines (an empty or +// still-open fence) reads as open, which keeps the external formatter off it. +func fenceClosed(fc *ast.FencedCodeBlock, src []byte) bool { + lines := fc.Lines() + if lines.Len() == 0 { + return false + } + return lines.At(lines.Len()-1).Stop < len(src) +} + func diffFenceText(fc *ast.FencedCodeBlock, src []byte) string { var b strings.Builder for i := range fc.Lines().Len() { diff --git a/internal/frontend/termrender/theme_config.go b/internal/frontend/termrender/theme_config.go index 74659c7fa3..0da0e570b9 100644 --- a/internal/frontend/termrender/theme_config.go +++ b/internal/frontend/termrender/theme_config.go @@ -14,9 +14,13 @@ func ConfigureThemeFromConfig() { if cfg, err := config.Load(); err == nil { configureThemeWithStyle(cfg.UITheme(), cfg.UIThemeStyle()) cursorShape = cfg.UICursorShape() + configureDiffFormatter(cfg) + configureDiffFences(cfg) } else { ConfigureTheme("auto") cursorShape = "bar" + configureDiffFormatter(nil) + configureDiffFences(nil) } } diff --git a/internal/frontend/tui/program.go b/internal/frontend/tui/program.go index 7939e0783b..0961169102 100644 --- a/internal/frontend/tui/program.go +++ b/internal/frontend/tui/program.go @@ -55,6 +55,13 @@ func Run(ctx context.Context, opts Options) error { if clusters, ok := clustersEmoji(); ok && clusters { go p.Send(clusterReport) } + // Full screen repaints every frame, so the diff formatter runs off the render + // path: built-in rows now, formatted ones when the run lands. Inline writes to + // the scrollback, where a row cannot be repainted, so it stays inline. + if !opts.Inline { + termrender.SetDiffFormatNotify(func() { p.Send(diffFormattedMsg{}) }) + defer termrender.SetDiffFormatNotify(nil) + } _, err := p.Run() return err } @@ -139,6 +146,10 @@ type ( reprint bool } statusTickMsg struct{} + // diffFormattedMsg is the diff formatter's background run reporting a + // result; the model repaints on it so the formatted rows replace the + // built-in ones drawn while the run was in flight. + diffFormattedMsg struct{} ) func newModel(ctx context.Context, opts Options) *model { @@ -283,6 +294,8 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, m.onSpin() case statusTickMsg: return m, tea.Batch(m.fetchStatus(), tickStatus()) + case diffFormattedMsg: + return m, m.commit() case actionMsg: if msg.err != nil { m.tr.AddNotice("error", msg.what+": "+msg.err.Error()) diff --git a/internal/frontend/tui/view.go b/internal/frontend/tui/view.go index c0ef8149d9..07768de6ab 100644 --- a/internal/frontend/tui/view.go +++ b/internal/frontend/tui/view.go @@ -91,14 +91,11 @@ func (m *model) frame() tea.View { func (m *model) liveLines() []string { return m.liveLinesRail(m.scrollbarHidden()) } -// liveLinesRail draws what is still changing at the width it will occupy: -// full screen that is the transcript's, beside the scrollbar column, so a row -// is not wrapped once more when it joins the transcript. +// liveLinesRail draws what is still changing at the width it will occupy: the +// transcript's beside the scrollbar column full screen, and the live frame's +// otherwise, so a row is not wrapped once more when it joins the transcript. func (m *model) liveLinesRail(hideRail bool) []string { - width := m.width - if m.scr != nil { - width = m.contentWidth() - } + width := m.contentWidth() var out []string for i := range m.tr.Items { it := &m.tr.Items[i] diff --git a/reasonix.example.toml b/reasonix.example.toml index 89600231bc..f65215cf64 100644 --- a/reasonix.example.toml +++ b/reasonix.example.toml @@ -15,6 +15,10 @@ theme = "auto" # auto|dark|light; controls CLI colors only; REASONIX_THEME can # commandmode = "vi" # ""|vi; "vi" gives the CLI prompt a vi command mode (Esc enters command mode, ^C interrupts a stream, ^D exits) show_turn_usage = true # CLI/TUI: show per-request token and cost receipts in transcript scrollback +[cli] # user/global only; ./reasonix.toml cannot override +# diff_fences = true # render a fenced ```diff/```patch block through the colourised diff renderer instead of the plain code rail; default false +# diff_formatter = "delta --color-only --paging=never" # optional external argv (no shell) that formats a fenced ```diff/```patch block and a writer tool's diff card; the diff is piped on stdin and its stdout is re-emitted with non-SGR escapes stripped + [desktop] # language = "auto" # auto|en|zh; auto follows the browser/OS locale # currency = "auto" # legacy display currency; prefer [billing].display_currency