From d0265b08457583fb446d2b89b0ad2471028e326e Mon Sep 17 00:00:00 2001 From: Jake Nelson Date: Sat, 9 May 2026 09:49:57 +1000 Subject: [PATCH 1/2] feat: configurable annotation marker --- .../skills/revdiff/references/config.md | 1 + README.md | 1 + app/config.go | 1 + app/config_test.go | 42 ++++++++++ app/main.go | 15 ++-- app/ui/annotate.go | 22 +++-- app/ui/annotate_test.go | 81 ++++++++++++++++++- app/ui/diffview.go | 20 ++--- app/ui/model.go | 11 ++- app/ui/model_test.go | 25 +++--- .../codex/skills/revdiff/references/config.md | 1 + site/docs.html | 1 + 12 files changed, 184 insertions(+), 37 deletions(-) diff --git a/.claude-plugin/skills/revdiff/references/config.md b/.claude-plugin/skills/revdiff/references/config.md index 40db3f34..b99f698d 100644 --- a/.claude-plugin/skills/revdiff/references/config.md +++ b/.claude-plugin/skills/revdiff/references/config.md @@ -32,6 +32,7 @@ Then uncomment and edit the values you want to change. | `--line-numbers` | `REVDIFF_LINE_NUMBERS` | Show line numbers in diff gutter | `false` | | `--blame` | `REVDIFF_BLAME` | Show blame gutter | `false` | | `--word-diff` | `REVDIFF_WORD_DIFF` | Highlight intra-line word-level changes in paired add/remove lines | `false` | +| `--annotation-marker` | `REVDIFF_ANNOTATION_MARKER` | Prefix shown before annotation lines | `💬` | | `--no-confirm-discard` | `REVDIFF_NO_CONFIRM_DISCARD` | Skip confirmation when discarding annotations with Q | `false` | | `--no-mouse` | `REVDIFF_NO_MOUSE` | Disable mouse support (scroll wheel, click) | `false` | | `--vim-motion` | `REVDIFF_VIM_MOTION` | Enable vim-style motion preset (counts, `gg`, `G`, `zz`/`zt`/`zb`, `ZZ`/`ZQ`) | `false` | diff --git a/README.md b/README.md index da44500e..83824e76 100644 --- a/README.md +++ b/README.md @@ -303,6 +303,7 @@ Positional arguments support several forms: | `--line-numbers` | Show line numbers in diff gutter, env: `REVDIFF_LINE_NUMBERS` | `false` | | `--blame` | Show blame gutter, env: `REVDIFF_BLAME` | `false` | | `--word-diff` | Highlight intra-line word-level changes in paired add/remove lines, env: `REVDIFF_WORD_DIFF` | `false` | +| `--annotation-marker` | Prefix shown before annotation lines, env: `REVDIFF_ANNOTATION_MARKER` | `💬` | | `--no-confirm-discard` | Skip confirmation when discarding annotations with Q, env: `REVDIFF_NO_CONFIRM_DISCARD` | `false` | | `--no-mouse` | Disable mouse support (scroll wheel, click), env: `REVDIFF_NO_MOUSE` | `false` | | `--vim-motion` | Enable vim-style motion preset (counts, `gg`, `G`, `zz`/`zt`/`zb`, `ZZ`/`ZQ`), env: `REVDIFF_VIM_MOTION` | `false` | diff --git a/app/config.go b/app/config.go index 41eecfcf..13ad3fde 100644 --- a/app/config.go +++ b/app/config.go @@ -34,6 +34,7 @@ type options struct { LineNumbers bool `long:"line-numbers" ini-name:"line-numbers" env:"REVDIFF_LINE_NUMBERS" description:"show line numbers in diff gutter"` Blame bool `long:"blame" ini-name:"blame" env:"REVDIFF_BLAME" description:"show blame gutter"` WordDiff bool `long:"word-diff" ini-name:"word-diff" env:"REVDIFF_WORD_DIFF" description:"highlight intra-line word-level changes in paired add/remove lines"` + AnnotationMarker string `long:"annotation-marker" ini-name:"annotation-marker" env:"REVDIFF_ANNOTATION_MARKER" default:"💬" description:"prefix shown before annotation lines"` VimMotion bool `long:"vim-motion" ini-name:"vim-motion" env:"REVDIFF_VIM_MOTION" description:"enable vim-style motion preset (counts, gg, G, zz/zt/zb, ZZ/ZQ)"` ChromaStyle string `long:"chroma-style" ini-name:"chroma-style" env:"REVDIFF_CHROMA_STYLE" default:"catppuccin-macchiato" description:"chroma style for syntax highlighting"` AllFiles bool `long:"all-files" short:"A" no-ini:"true" description:"browse all tracked files, not just diffs (git and jj only)"` diff --git a/app/config_test.go b/app/config_test.go index 2403fd26..306f03f9 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -23,6 +23,7 @@ func TestParseArgs_Defaults(t *testing.T) { assert.Equal(t, 2, opts.TreeWidth) assert.Equal(t, 4, opts.TabWidth) assert.Equal(t, "catppuccin-macchiato", opts.ChromaStyle) + assert.Equal(t, "💬", opts.AnnotationMarker) assert.False(t, opts.Staged) assert.False(t, opts.NoColors) assert.False(t, opts.NoStatusBar) @@ -413,6 +414,47 @@ func TestParseArgs_WordDiff(t *testing.T) { }) } +func TestParseArgs_AnnotationMarker(t *testing.T) { + t.Run("flag", func(t *testing.T) { + opts, err := parseArgs(append(noConfigArgs(t), "--annotation-marker=▸")) + require.NoError(t, err) + assert.Equal(t, "▸", opts.AnnotationMarker) + }) + + t.Run("empty flag", func(t *testing.T) { + opts, err := parseArgs(append(noConfigArgs(t), "--annotation-marker=")) + require.NoError(t, err) + assert.Empty(t, opts.AnnotationMarker) + }) + + t.Run("env", func(t *testing.T) { + t.Setenv("REVDIFF_ANNOTATION_MARKER", ">>>") + opts, err := parseArgs(noConfigArgs(t)) + require.NoError(t, err) + assert.Equal(t, ">>>", opts.AnnotationMarker) + }) + + t.Run("config file", func(t *testing.T) { + cfgDir := t.TempDir() + cfgPath := filepath.Join(cfgDir, "config") + err := os.WriteFile(cfgPath, []byte("[Application Options]\nannotation-marker = #\n"), 0o600) + require.NoError(t, err) + opts, err := parseArgs([]string{"--config", cfgPath}) + require.NoError(t, err) + assert.Equal(t, "#", opts.AnnotationMarker) + }) + + t.Run("config file empty", func(t *testing.T) { + cfgDir := t.TempDir() + cfgPath := filepath.Join(cfgDir, "config") + err := os.WriteFile(cfgPath, []byte("[Application Options]\nannotation-marker =\n"), 0o600) + require.NoError(t, err) + opts, err := parseArgs([]string{"--config", cfgPath}) + require.NoError(t, err) + assert.Empty(t, opts.AnnotationMarker) + }) +} + func TestParseArgs_VimMotion(t *testing.T) { t.Run("default", func(t *testing.T) { opts, err := parseArgs(noConfigArgs(t)) diff --git a/app/main.go b/app/main.go index e016e431..42723f0a 100644 --- a/app/main.go +++ b/app/main.go @@ -206,13 +206,14 @@ func run(opts options) error { vcsType: vcsType, description: description, }), - TabWidth: opts.TabWidth, - Ref: opts.ref(), - Staged: opts.Staged, - TreeWidthRatio: opts.TreeWidth, - Only: opts.Only, - WorkDir: workDir, - ActiveThemeName: themes.catalog.ActiveName(opts.Theme), + TabWidth: opts.TabWidth, + Ref: opts.ref(), + Staged: opts.Staged, + TreeWidthRatio: opts.TreeWidth, + Only: opts.Only, + WorkDir: workDir, + ActiveThemeName: themes.catalog.ActiveName(opts.Theme), + AnnotationMarker: opts.AnnotationMarker, NewFileTree: func(entries []diff.FileEntry) ui.FileTreeComponent { return sidepane.NewFileTree(entries) }, diff --git a/app/ui/annotate.go b/app/ui/annotate.go index 16d496ea..2583d058 100644 --- a/app/ui/annotate.go +++ b/app/ui/annotate.go @@ -18,6 +18,16 @@ import ( // annotKeyFile is the lookup key for file-level annotations in wrappedAnnotationLineCount. const annotKeyFile = "file" +// annotPrefix returns the cached annotation line prefix (marker + space). +func (m Model) annotPrefix() string { + return m.cfg.annotPrefix +} + +// annotFilePrefix returns the cached file-level annotation prefix (marker + " file: "). +func (m Model) annotFilePrefix() string { + return m.cfg.annotFilePrefix +} + // annotCharLimit caps annotation text length. sized for multi-item lists and // small pasted data slices, not for full-document content. const annotCharLimit = 8000 @@ -87,7 +97,7 @@ func (m *Model) startAnnotation() tea.Cmd { } } - ti, cmd := m.newAnnotationInput(placeholder, 6) // cursor col + emoji prefix "💬 " + border margin + ti, cmd := m.newAnnotationInput(placeholder, 3+lipgloss.Width(m.annotPrefix())) // cursor col + annotation prefix + border margin if preFill != "" { ti.SetValue(preFill) } @@ -147,7 +157,7 @@ func (m *Model) startFileAnnotation() tea.Cmd { } } - ti, cmd := m.newAnnotationInput(placeholder, 12) // cursor col + "💬 file: " prefix + border margin + ti, cmd := m.newAnnotationInput(placeholder, 3+lipgloss.Width(m.annotFilePrefix())) // cursor col + file annotation prefix + border margin if preFill != "" { ti.SetValue(preFill) } @@ -391,15 +401,15 @@ type annotCacheKey struct { // annotationPrefixBody resolves the (prefix, body) pair for the annotation // identified by key. file-level annotations (key == annotKeyFile) get the -// "💬 file: " prefix; line-level annotations get "💬 ". returns ("", "") when -// no annotation matches the key. +// file-level prefix; line-level annotations get the line prefix. returns ("", "") +// when no annotation matches the key. func (m Model) annotationPrefixBody(key string) (prefix, body string) { for _, a := range m.store.Get(m.file.name) { if key == annotKeyFile && a.Line == 0 { - return "\U0001f4ac file: ", a.Comment + return m.annotFilePrefix(), a.Comment } if key != annotKeyFile && m.annotationKey(a.Line, a.Type) == key { - return "\U0001f4ac ", a.Comment + return m.annotPrefix(), a.Comment } } return "", "" diff --git a/app/ui/annotate_test.go b/app/ui/annotate_test.go index d2a7ee34..de3d0eba 100644 --- a/app/ui/annotate_test.go +++ b/app/ui/annotate_test.go @@ -15,6 +15,7 @@ import ( "github.com/umputun/revdiff/app/ui/overlay" "github.com/umputun/revdiff/app/ui/sidepane" "github.com/umputun/revdiff/app/ui/style" + "github.com/umputun/revdiff/app/ui/worddiff" ) func TestModel_AnnotatedFilesMarker(t *testing.T) { @@ -501,6 +502,84 @@ func TestModel_RenderDiffWithAnnotations(t *testing.T) { assert.Contains(t, rendered, "\U0001f4ac") } +func TestModel_CustomAnnotationMarker(t *testing.T) { + res := style.PlainResolver() + m, err := NewModel(ModelConfig{ + Renderer: plainRenderer(), + Store: annotation.NewStore(), + Highlighter: noopHighlighter(), + StyleResolver: res, + StyleRenderer: style.NewRenderer(res), + SGR: style.SGR{}, + WordDiffer: worddiff.New(), + Overlay: overlay.NewManager(), + Themes: fakeThemeCatalog{}, + TreeWidthRatio: 3, + AnnotationMarker: "▸", + NewFileTree: testFileTreeFactory(), + ParseTOC: testParseTOCFactory(), + }) + require.NoError(t, err) + m.layout.width = 120 + m.layout.height = 40 + m.layout.treeWidth = m.layout.width * m.cfg.treeWidthRatio / 10 + m.ready = true + m.filesLoaded = true + + m.file.name = "a.go" + m.file.lines = []diff.DiffLine{ + {NewNum: 1, Content: "package main", ChangeType: diff.ChangeContext}, + {NewNum: 2, Content: "func foo() {}", ChangeType: diff.ChangeAdd}, + } + m.store.Add(annotation.Annotation{File: "a.go", Line: 2, Type: "+", Comment: "note"}) + m.store.Add(annotation.Annotation{File: "a.go", Line: 0, Type: "", Comment: "file note"}) + + rendered := m.renderDiff() + assert.Contains(t, rendered, "▸ note", "line annotation should use custom marker") + assert.Contains(t, rendered, "▸ file: file note", "file annotation should use custom marker") + assert.NotContains(t, rendered, "\U0001f4ac", "default emoji should not appear with custom marker") +} + +func TestModel_EmptyAnnotationMarkerExplicit(t *testing.T) { + res := style.PlainResolver() + m, err := NewModel(ModelConfig{ + Renderer: plainRenderer(), + Store: annotation.NewStore(), + Highlighter: noopHighlighter(), + StyleResolver: res, + StyleRenderer: style.NewRenderer(res), + SGR: style.SGR{}, + WordDiffer: worddiff.New(), + Overlay: overlay.NewManager(), + Themes: fakeThemeCatalog{}, + TreeWidthRatio: 3, + AnnotationMarker: "", + NewFileTree: testFileTreeFactory(), + ParseTOC: testParseTOCFactory(), + }) + require.NoError(t, err) + m.layout.width = 120 + m.layout.height = 40 + m.layout.treeWidth = m.layout.width * m.cfg.treeWidthRatio / 10 + m.ready = true + m.filesLoaded = true + + m.file.name = "a.go" + m.file.lines = []diff.DiffLine{ + {NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext}, + } + m.store.Add(annotation.Annotation{File: "a.go", Line: 1, Type: " ", Comment: "bare note"}) + m.store.Add(annotation.Annotation{File: "a.go", Line: 0, Type: "", Comment: "file note"}) + + rendered := m.renderDiff() + // verify empty marker produces bare space prefix, not emoji fallback + assert.NotContains(t, rendered, "\U0001f4ac", "empty marker should not produce emoji") + assert.NotContains(t, rendered, "\U0001f4ac bare note", "should not have emoji before line annotation") + assert.NotContains(t, rendered, "\U0001f4ac file:", "should not have emoji before file annotation") + assert.Contains(t, rendered, " bare note", "empty marker should render bare prefix for line annotation") + assert.Contains(t, rendered, " file: file note", "empty marker should render ' file: ' prefix for file annotation") +} + func TestModel_RenderDiffAnnotationInput(t *testing.T) { m := testModel(nil, nil) m.file.name = "a.go" @@ -1058,7 +1137,7 @@ func TestModel_FileAnnotationInputWidthNarrowerThanLineLevel(t *testing.T) { fileWidth := m.annot.input.Width assert.Greater(t, lineWidth, fileWidth, "file-level input should be narrower than line-level due to wider prefix") - assert.Equal(t, 6, lineWidth-fileWidth, "width difference should match prefix width difference (12-6=6)") + assert.Equal(t, 6, lineWidth-fileWidth, "width difference should match prefix width difference") } func TestModel_FileAnnotationSavesWithLineZero(t *testing.T) { diff --git a/app/ui/diffview.go b/app/ui/diffview.go index 6871d075..2f535348 100644 --- a/app/ui/diffview.go +++ b/app/ui/diffview.go @@ -313,7 +313,7 @@ func (m Model) buildAnnotationMap() (annotations map[string]string, fileComment func (m Model) renderFileAnnotationHeader(b *strings.Builder, fileComment string) { // when actively editing a file-level annotation, always show the input widget if m.annot.annotating && m.annot.fileAnnotating { - line := " " + m.renderer.AnnotationInline("\U0001f4ac file: ") + m.annot.input.View() + line := " " + m.renderer.AnnotationInline(m.annotFilePrefix()) + m.annot.input.View() // strip textinput's unstyled trailing padding so extendLineBg can re-pad with DiffBg line = strings.TrimRight(line, " ") b.WriteString(m.extendLineBg(line, m.resolver.Color(style.ColorKeyDiffPaneBg)) + "\n") @@ -330,7 +330,7 @@ func (m Model) renderFileAnnotationHeader(b *strings.Builder, fileComment string if m.nav.diffCursor == -1 && m.layout.focus == paneDiff { cursor = m.renderer.DiffCursor(m.cfg.noColors) } - m.renderWrappedAnnotation(b, cursor, "\U0001f4ac file: ", fileComment) + m.renderWrappedAnnotation(b, cursor, m.annotFilePrefix(), fileComment) } } @@ -655,7 +655,7 @@ func (m Model) extendLineBg(styled string, bg style.Color) string { // renderAnnotationOrInput writes the annotation input or existing annotation below a diff line. func (m Model) renderAnnotationOrInput(b *strings.Builder, idx int, annotationMap map[string]string) { if m.annot.annotating && !m.annot.fileAnnotating && idx == m.nav.diffCursor { - line := " " + m.renderer.AnnotationInline("\U0001f4ac ") + m.annot.input.View() + line := " " + m.renderer.AnnotationInline(m.annotPrefix()) + m.annot.input.View() // strip textinput's unstyled trailing padding so extendLineBg can re-pad with DiffBg line = strings.TrimRight(line, " ") b.WriteString(m.extendLineBg(line, m.resolver.Color(style.ColorKeyDiffPaneBg)) + "\n") @@ -669,7 +669,7 @@ func (m Model) renderAnnotationOrInput(b *strings.Builder, idx int, annotationMa if idx == m.nav.diffCursor && m.annot.cursorOnAnnotation && m.layout.focus == paneDiff { cursor = m.renderer.DiffCursor(m.cfg.noColors) } - m.renderWrappedAnnotation(b, cursor, "\U0001f4ac ", comment) + m.renderWrappedAnnotation(b, cursor, m.annotPrefix(), comment) } } } @@ -695,15 +695,15 @@ func (m Model) renderWrappedAnnotation(b *strings.Builder, cursor, prefix, body } } -// annotationContinuationIndent returns leading whitespace sized to match the emoji +// annotationContinuationIndent returns leading whitespace sized to match the marker // prefix on the first logical line of an annotation so continuation logical lines -// align under the body. Uses lipgloss.Width because the emoji is double-width. +// align under the body. Uses lipgloss.Width because markers may be wide glyphs. func (m Model) annotationContinuationIndent(firstLogicalLine string) string { switch { - case strings.HasPrefix(firstLogicalLine, "\U0001f4ac file: "): - return strings.Repeat(" ", lipgloss.Width("\U0001f4ac file: ")) - case strings.HasPrefix(firstLogicalLine, "\U0001f4ac "): - return strings.Repeat(" ", lipgloss.Width("\U0001f4ac ")) + case strings.HasPrefix(firstLogicalLine, m.annotFilePrefix()): + return strings.Repeat(" ", lipgloss.Width(m.annotFilePrefix())) + case strings.HasPrefix(firstLogicalLine, m.annotPrefix()): + return strings.Repeat(" ", lipgloss.Width(m.annotPrefix())) default: return "" } diff --git a/app/ui/model.go b/app/ui/model.go index 896a24e8..93846e95 100644 --- a/app/ui/model.go +++ b/app/ui/model.go @@ -264,7 +264,10 @@ type modelConfigState struct { crossFileHunks bool // allow [ and ] to jump across file boundaries treeWidthRatio int // 1-10 units for file tree panel tabSpaces string // spaces to replace tabs with - wrapIndent int // extra indent (in columns) for wrap continuation rows; 0 disables + wrapIndent int // extra indent (in columns) for wrap continuation rows; 0 disables + annotationMarker string // prefix shown before annotation lines + annotPrefix string // cached: marker + " " + annotFilePrefix string // cached: marker + " file: " } // layoutState holds viewport and layout concerns that change on resize and pane toggles. @@ -650,6 +653,9 @@ type ModelConfig struct { // the modal-key handler and keymap.Resolve. Copied into modes.vimMotion at // construction; the feature is gated on that field everywhere. VimMotion bool + // AnnotationMarker is the prefix shown before annotation lines. + // Empty is preserved so callers can intentionally render no marker. + AnnotationMarker string // ReviewInfo populates the review-info overlay with invocation scope, filters, and // aggregate file/line stats. Pass nil to preserve the legacy commit-only popup // behavior used by focused tests — every derived path (footer, rows, stats @@ -748,6 +754,9 @@ func NewModel(cfg ModelConfig) (Model, error) { treeWidthRatio: cfg.TreeWidthRatio, tabSpaces: strings.Repeat(" ", cfg.TabWidth), wrapIndent: max(0, cfg.WrapIndent), + annotationMarker: cfg.AnnotationMarker, + annotPrefix: cfg.AnnotationMarker + " ", + annotFilePrefix: cfg.AnnotationMarker + " file: ", }, layout: layoutState{ focus: paneTree, diff --git a/app/ui/model_test.go b/app/ui/model_test.go index 72945c43..7a01a880 100644 --- a/app/ui/model_test.go +++ b/app/ui/model_test.go @@ -146,18 +146,19 @@ func testModel(files []string, fileDiffs map[string][]diff.DiffLine) Model { } res := style.PlainResolver() m, err := NewModel(ModelConfig{ - Renderer: renderer, - Store: annotation.NewStore(), - Highlighter: noopHighlighter(), - StyleResolver: res, - StyleRenderer: style.NewRenderer(res), - SGR: style.SGR{}, - WordDiffer: worddiff.New(), - Overlay: overlay.NewManager(), - Themes: fakeThemeCatalog{}, - TreeWidthRatio: 3, - NewFileTree: testFileTreeFactory(), - ParseTOC: testParseTOCFactory(), + Renderer: renderer, + Store: annotation.NewStore(), + Highlighter: noopHighlighter(), + StyleResolver: res, + StyleRenderer: style.NewRenderer(res), + SGR: style.SGR{}, + WordDiffer: worddiff.New(), + Overlay: overlay.NewManager(), + Themes: fakeThemeCatalog{}, + TreeWidthRatio: 3, + AnnotationMarker: "\U0001f4ac", + NewFileTree: testFileTreeFactory(), + ParseTOC: testParseTOCFactory(), }) if err != nil { // testModel supplies all required deps — an error here is a bug in the helper, not the test. diff --git a/plugins/codex/skills/revdiff/references/config.md b/plugins/codex/skills/revdiff/references/config.md index 40db3f34..b99f698d 100644 --- a/plugins/codex/skills/revdiff/references/config.md +++ b/plugins/codex/skills/revdiff/references/config.md @@ -32,6 +32,7 @@ Then uncomment and edit the values you want to change. | `--line-numbers` | `REVDIFF_LINE_NUMBERS` | Show line numbers in diff gutter | `false` | | `--blame` | `REVDIFF_BLAME` | Show blame gutter | `false` | | `--word-diff` | `REVDIFF_WORD_DIFF` | Highlight intra-line word-level changes in paired add/remove lines | `false` | +| `--annotation-marker` | `REVDIFF_ANNOTATION_MARKER` | Prefix shown before annotation lines | `💬` | | `--no-confirm-discard` | `REVDIFF_NO_CONFIRM_DISCARD` | Skip confirmation when discarding annotations with Q | `false` | | `--no-mouse` | `REVDIFF_NO_MOUSE` | Disable mouse support (scroll wheel, click) | `false` | | `--vim-motion` | `REVDIFF_VIM_MOTION` | Enable vim-style motion preset (counts, `gg`, `G`, `zz`/`zt`/`zb`, `ZZ`/`ZQ`) | `false` | diff --git a/site/docs.html b/site/docs.html index 3b10d621..3794edf1 100644 --- a/site/docs.html +++ b/site/docs.html @@ -373,6 +373,7 @@

Options

--cross-file-hunksAllow [ and ] to continue into adjacent filesfalse --line-numbersShow line numbers in diff gutterfalse --word-diffHighlight intra-line word-level changes in paired add/remove linesfalse + --annotation-markerPrefix shown before annotation lines💬 --no-confirm-discardSkip discard confirmationfalse --no-mouseDisable mouse support (scroll wheel, click)false --vim-motionEnable vim-style motion preset (counts, gg, G, zz/zt/zb, ZZ/ZQ)false From 6fe46c3263cb860221c2277e4346cda975dd0a2f Mon Sep 17 00:00:00 2001 From: Jake Nelson Date: Wed, 13 May 2026 07:00:12 +1000 Subject: [PATCH 2/2] fix: restore border width, fix allocs & validation --- app/config.go | 4 ++++ app/config_test.go | 18 ++++++++++++++++++ app/ui/annotate.go | 2 +- app/ui/annotate_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ app/ui/diffview.go | 14 -------------- app/ui/diffview_test.go | 37 ++++++++++++------------------------- app/ui/model.go | 8 ++++---- 7 files changed, 79 insertions(+), 44 deletions(-) diff --git a/app/config.go b/app/config.go index 13ad3fde..80069903 100644 --- a/app/config.go +++ b/app/config.go @@ -157,6 +157,10 @@ func parseArgs(args []string) (options, error) { return options{}, errors.New("--compact-context must be >= 1") } + if strings.ContainsAny(opts.AnnotationMarker, "\n\r\t") { + return options{}, errors.New("--annotation-marker cannot contain newlines or tabs") + } + if opts.Description != "" && opts.DescriptionFile != "" { return options{}, errors.New("--description and --description-file are mutually exclusive") } diff --git a/app/config_test.go b/app/config_test.go index 306f03f9..80628c98 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -453,6 +453,24 @@ func TestParseArgs_AnnotationMarker(t *testing.T) { require.NoError(t, err) assert.Empty(t, opts.AnnotationMarker) }) + + t.Run("rejects newline", func(t *testing.T) { + _, err := parseArgs(append(noConfigArgs(t), "--annotation-marker=a\nb")) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot contain newlines or tabs") + }) + + t.Run("rejects tab", func(t *testing.T) { + _, err := parseArgs(append(noConfigArgs(t), "--annotation-marker=a\tb")) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot contain newlines or tabs") + }) + + t.Run("rejects carriage return", func(t *testing.T) { + _, err := parseArgs(append(noConfigArgs(t), "--annotation-marker=a\rb")) + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot contain newlines or tabs") + }) } func TestParseArgs_VimMotion(t *testing.T) { diff --git a/app/ui/annotate.go b/app/ui/annotate.go index 2583d058..bfbc23eb 100644 --- a/app/ui/annotate.go +++ b/app/ui/annotate.go @@ -449,7 +449,7 @@ func (m *Model) annotationVisualRows(prefix, body string) []string { func (m Model) composeAnnotationRows(prefix, body string, wrapW int) []string { first := prefix + body logical := strings.Split(first, "\n") - indent := m.annotationContinuationIndent(logical[0]) + indent := strings.Repeat(" ", lipgloss.Width(prefix)) var rows []string for i, segment := range logical { diff --git a/app/ui/annotate_test.go b/app/ui/annotate_test.go index de3d0eba..299ea08d 100644 --- a/app/ui/annotate_test.go +++ b/app/ui/annotate_test.go @@ -1140,6 +1140,46 @@ func TestModel_FileAnnotationInputWidthNarrowerThanLineLevel(t *testing.T) { assert.Equal(t, 6, lineWidth-fileWidth, "width difference should match prefix width difference") } +func TestModel_AnnotationInputWidthUsesMarkerWidth(t *testing.T) { + lines := []diff.DiffLine{{NewNum: 1, Content: "line1", ChangeType: diff.ChangeAdd}} + tests := []struct { + name string + marker string + wantLineWidth int + wantFileWidth int + }{ + {name: "default emoji marker", marker: "\U0001f4ac", wantLineWidth: 78, wantFileWidth: 72}, + {name: "wide emoji marker", marker: "\U0001f4ac\U0001f4ac", wantLineWidth: 76, wantFileWidth: 70}, + {name: "empty marker", marker: "", wantLineWidth: 80, wantFileWidth: 74}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := testModel([]string{"a.go"}, nil) + m.tree = testNewFileTree([]string{"a.go"}) + m.file.name = "a.go" + m.file.lines = lines + m.nav.diffCursor = 0 + m.layout.focus = paneDiff + m.layout.width = 120 + m.layout.treeWidth = 30 + m.layout.treeHidden = false + m.cfg.annotationMarker = tt.marker + m.cfg.annotPrefix = tt.marker + " " + m.cfg.annotFilePrefix = tt.marker + " file: " + + require.Equal(t, 84, m.diffContentWidth(), "test fixture pins absolute width") + + m.startAnnotation() + assert.Equal(t, tt.wantLineWidth, m.annot.input.Width, "line annotation input width") + + m.annot.annotating = false + m.startFileAnnotation() + assert.Equal(t, tt.wantFileWidth, m.annot.input.Width, "file annotation input width") + }) + } +} + func TestModel_FileAnnotationSavesWithLineZero(t *testing.T) { lines := []diff.DiffLine{ {NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext}, diff --git a/app/ui/diffview.go b/app/ui/diffview.go index 2f535348..f95e06e7 100644 --- a/app/ui/diffview.go +++ b/app/ui/diffview.go @@ -695,20 +695,6 @@ func (m Model) renderWrappedAnnotation(b *strings.Builder, cursor, prefix, body } } -// annotationContinuationIndent returns leading whitespace sized to match the marker -// prefix on the first logical line of an annotation so continuation logical lines -// align under the body. Uses lipgloss.Width because markers may be wide glyphs. -func (m Model) annotationContinuationIndent(firstLogicalLine string) string { - switch { - case strings.HasPrefix(firstLogicalLine, m.annotFilePrefix()): - return strings.Repeat(" ", lipgloss.Width(m.annotFilePrefix())) - case strings.HasPrefix(firstLogicalLine, m.annotPrefix()): - return strings.Repeat(" ", lipgloss.Width(m.annotPrefix())) - default: - return "" - } -} - const ( wrapGutterWidth = 3 // wrap gutter prefix width: " + ", " - ", " ", " ↪ " wrapMinContent = 10 // minimum content width per visual row when wrap-indent is active diff --git a/app/ui/diffview_test.go b/app/ui/diffview_test.go index db92def4..0397a476 100644 --- a/app/ui/diffview_test.go +++ b/app/ui/diffview_test.go @@ -1262,6 +1262,18 @@ func TestModel_RenderWrappedAnnotation_MultiLine(t *testing.T) { assert.Contains(t, rows[1], strings.Repeat(" ", 9)+"beta", "continuation indented to align past 💬 file: prefix") }) + t.Run("line-level body starting file colon uses line prefix indent", func(t *testing.T) { + m := newModel() + var b strings.Builder + cursor := m.renderer.DiffCursor(m.cfg.noColors) + m.renderWrappedAnnotation(&b, cursor, "\U0001f4ac ", "file: alpha\nbeta") + out := b.String() + rows := strings.Split(strings.TrimRight(out, "\n"), "\n") + require.Len(t, rows, 2) + assert.Contains(t, rows[1], strings.Repeat(" ", 3)+"beta", "line-level continuation should align past 💬 prefix") + assert.NotContains(t, rows[1], strings.Repeat(" ", 9)+"beta", "line-level body must not be treated as file-level prefix") + }) + t.Run("two logical lines each wrap, counts grow", func(t *testing.T) { m := newModel() m.layout.width = 40 // narrow pane to force wrap @@ -1280,28 +1292,3 @@ func TestModel_RenderWrappedAnnotation_MultiLine(t *testing.T) { assert.Contains(t, out, "bravo", "second logical line content present") }) } - -func TestModel_AnnotationContinuationIndent(t *testing.T) { - m := testModel(nil, nil) - - tests := []struct { - name string - first string - want int // number of indent spaces - }{ - {"line-level emoji prefix gets 3-space indent", "\U0001f4ac line note", 3}, - {"file-level emoji prefix gets 9-space indent", "\U0001f4ac file: note", 9}, - {"no emoji prefix yields no indent (default branch)", "plain text", 0}, - {"empty string yields no indent (default branch)", "", 0}, - {"lookalike prefix without emoji yields no indent", "file: note", 0}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - indent := m.annotationContinuationIndent(tt.first) - assert.Len(t, indent, tt.want) - for _, r := range indent { - assert.Equal(t, ' ', r, "indent must be spaces only") - } - }) - } -} diff --git a/app/ui/model.go b/app/ui/model.go index 93846e95..7fb92276 100644 --- a/app/ui/model.go +++ b/app/ui/model.go @@ -264,10 +264,10 @@ type modelConfigState struct { crossFileHunks bool // allow [ and ] to jump across file boundaries treeWidthRatio int // 1-10 units for file tree panel tabSpaces string // spaces to replace tabs with - wrapIndent int // extra indent (in columns) for wrap continuation rows; 0 disables - annotationMarker string // prefix shown before annotation lines - annotPrefix string // cached: marker + " " - annotFilePrefix string // cached: marker + " file: " + wrapIndent int // extra indent (in columns) for wrap continuation rows; 0 disables + annotationMarker string // prefix shown before annotation lines + annotPrefix string // cached: marker + " " + annotFilePrefix string // cached: marker + " file: " } // layoutState holds viewport and layout concerns that change on resize and pane toggles.