Skip to content

Commit 7527936

Browse files
authored
Fix file-level annotation input overflow and Enter-to-edit (#27)
* add plan: fix-file-annotation-bugs * feat: fix file-level annotation input width overflow Add prefixWidth parameter to newAnnotationInput so file-level annotations use the correct width subtraction (12) matching the wider "💬 file: " prefix, instead of the line-level value (6) which caused terminal overflow. Related to #26 * feat: fix Enter key to edit existing file-level annotations In handleEnterKey, check cursorOnFileAnnotationLine() before falling through to startAnnotation(). When on a file annotation line (diffCursor == -1), dispatch to startFileAnnotation() instead. Related to #26 * fix: deduplicate handleEnterKey annotation branches
1 parent d3ae81b commit 7527936

4 files changed

Lines changed: 180 additions & 6 deletions

File tree

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Fix File-Level Annotation Bugs
2+
3+
## Overview
4+
Fix two bugs where file-level annotations (`A` key) behave differently from line-level annotations (`a` key).
5+
Related to #26.
6+
7+
**Bug 1**: file-level annotation input overflows terminal width because `newAnnotationInput`
8+
subtracts 6 chars (for `"💬 "` prefix) but `renderFileAnnotationHeader` renders the wider
9+
`"💬 file: "` prefix (6 extra visible chars).
10+
11+
**Bug 2**: pressing Enter on a file annotation line does nothing. `handleEnterKey` always
12+
calls `startAnnotation()` which returns nil when `diffCursor == -1`. It never dispatches
13+
to `startFileAnnotation()`.
14+
15+
## Context (from discovery)
16+
- `ui/annotate.go:15-22`: `newAnnotationInput` — width calc subtracts 6 for prefix
17+
- `ui/annotate.go:57-62`: `startFileAnnotation` — calls `newAnnotationInput` with same width
18+
- `ui/diffview.go:55-70`: `renderFileAnnotationHeader` — renders `"💬 file: "` (wider prefix)
19+
- `ui/model.go:1138-1162`: `handleEnterKey` — always calls `startAnnotation()` in diff pane
20+
- `ui/annotate.go:212-213`: `cursorOnFileAnnotationLine()` — already exists, checks `diffCursor == -1`
21+
22+
## Development Approach
23+
- **Testing approach**: regular (code first, then tests)
24+
- Both fixes are small and isolated
25+
- **CRITICAL: every task MUST include new/updated tests**
26+
- **CRITICAL: all tests must pass before starting next task**
27+
28+
## Solution Overview
29+
- **Bug 1**: pass a width offset parameter to `newAnnotationInput` so file annotations use a larger
30+
subtraction. The file prefix `"💬 file: "` is ~12 visible chars vs line prefix `"💬 "` ~6 chars.
31+
- **Bug 2**: in `handleEnterKey`, check `cursorOnFileAnnotationLine()` before falling through
32+
to `startAnnotation()`. If true, call `startFileAnnotation()` instead.
33+
34+
## Implementation Steps
35+
36+
### Task 1: Fix file-level annotation input width overflow
37+
38+
**Files:**
39+
- Modify: `ui/annotate.go`
40+
- Modify: `ui/diffview.go`
41+
- Modify: `ui/model_test.go`
42+
43+
- [x] change `newAnnotationInput` to accept a `prefixWidth int` parameter instead of hardcoded 6
44+
- [x] update `startAnnotation` call to pass 6 (line-level prefix width)
45+
- [x] update `startFileAnnotation` call to pass 12 (file-level prefix width: cursor col + `"💬 file: "` + margin)
46+
- [x] verify the rendered prefix in `renderFileAnnotationHeader` (`"💬 file: "`) matches the offset
47+
- [x] write test: file-level annotation input width is narrower than line-level to account for wider prefix
48+
- [x] run tests — must pass before task 2
49+
50+
### Task 2: Fix Enter key to edit existing file annotation
51+
52+
**Files:**
53+
- Modify: `ui/model.go`
54+
- Modify: `ui/model_test.go`
55+
56+
- [x] in `handleEnterKey`, add branch before `startAnnotation()`: when `m.cursorOnFileAnnotationLine()`, call `startFileAnnotation()` instead
57+
- [x] write test: Enter on file annotation line (diffCursor == -1) triggers file annotation edit
58+
- [x] write test: Enter on file annotation line pre-fills existing annotation text
59+
- [x] write test: Enter on regular diff line still triggers line annotation (regression check)
60+
- [x] run tests — must pass before task 3
61+
62+
### Task 3: Verify acceptance criteria
63+
64+
- [x] verify: file-level annotation input stays within terminal width
65+
- [x] verify: Enter on existing file annotation opens edit with pre-filled text
66+
- [x] verify: line-level annotations still work identically
67+
- [x] run full test suite: `make test`
68+
- [x] run linter: `make lint`
69+
- [x] run formatters: `make fmt`
70+
71+
### Task 4: [Final] Update documentation
72+
73+
- [x] update CLAUDE.md if new patterns discovered
74+
- [x] move this plan to `docs/plans/completed/`
75+
76+
## Post-Completion
77+
78+
**Manual verification:**
79+
- create a file-level annotation with a long text, verify it doesn't overflow
80+
- navigate to file annotation line, press Enter, verify edit mode opens with pre-filled text
81+
- verify line-level annotations still work as before

ui/annotate.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,13 @@ import (
1212
)
1313

1414
// newAnnotationInput creates and focuses a text input for annotation editing.
15-
func (m *Model) newAnnotationInput(placeholder string) (textinput.Model, tea.Cmd) {
15+
// prefixWidth accounts for the visible prefix characters (cursor col + emoji + label + margin).
16+
func (m *Model) newAnnotationInput(placeholder string, prefixWidth int) (textinput.Model, tea.Cmd) {
1617
ti := textinput.New()
1718
ti.Placeholder = placeholder
1819
cmd := ti.Focus()
1920
ti.CharLimit = 500
20-
ti.Width = max(10, m.diffContentWidth()-6) // cursor col + emoji prefix "💬 " + border margin
21+
ti.Width = max(10, m.diffContentWidth()-prefixWidth)
2122
return ti, cmd
2223
}
2324

@@ -36,7 +37,7 @@ func (m *Model) startAnnotation() tea.Cmd {
3637
return nil
3738
}
3839

39-
ti, cmd := m.newAnnotationInput("annotation...")
40+
ti, cmd := m.newAnnotationInput("annotation...", 6) // cursor col + emoji prefix "💬 " + border margin
4041

4142
// pre-fill with existing annotation if one exists
4243
lineNum := m.diffLineNum(dl)
@@ -59,7 +60,7 @@ func (m *Model) startFileAnnotation() tea.Cmd {
5960
return nil
6061
}
6162

62-
ti, cmd := m.newAnnotationInput("file-level annotation...")
63+
ti, cmd := m.newAnnotationInput("file-level annotation...", 12) // cursor col + "💬 file: " prefix + border margin
6364

6465
// pre-fill with existing file-level annotation if one exists
6566
for _, a := range m.store.Get(m.currFile) {
@@ -74,7 +75,6 @@ func (m *Model) startFileAnnotation() tea.Cmd {
7475
m.fileAnnotating = true
7576
m.diffCursor = -1 // position cursor on the file annotation line
7677
m.viewport.GotoTop()
77-
m.viewport.SetContent(m.renderDiff())
7878
return cmd
7979
}
8080

ui/model.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1154,7 +1154,12 @@ func (m Model) handleEnterKey() (tea.Model, tea.Cmd) {
11541154
}
11551155
return m, nil
11561156
case paneDiff:
1157-
cmd := m.startAnnotation()
1157+
var cmd tea.Cmd
1158+
if m.cursorOnFileAnnotationLine() {
1159+
cmd = m.startFileAnnotation()
1160+
} else {
1161+
cmd = m.startAnnotation()
1162+
}
11581163
m.viewport.SetContent(m.renderDiff())
11591164
return m, cmd
11601165
}

ui/model_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2241,6 +2241,34 @@ func TestModel_AnnotationInputWidthNarrowTerminal(t *testing.T) {
22412241
assert.GreaterOrEqual(t, m.annotateInput.Width, 10, "file text input width should be at least 10")
22422242
}
22432243

2244+
func TestModel_FileAnnotationInputWidthNarrowerThanLineLevel(t *testing.T) {
2245+
// file-level annotation has wider prefix ("💬 file: " vs "💬 "), so input should be narrower
2246+
lines := []diff.DiffLine{
2247+
{NewNum: 1, Content: "line1", ChangeType: diff.ChangeAdd},
2248+
}
2249+
m := testModel([]string{"a.go"}, nil)
2250+
m.tree = newFileTree([]string{"a.go"})
2251+
m.currFile = "a.go"
2252+
m.diffLines = lines
2253+
m.diffCursor = 0
2254+
m.focus = paneDiff
2255+
m.width = 120
2256+
m.treeWidth = 30
2257+
m.treeHidden = false
2258+
2259+
// line-level annotation
2260+
m.startAnnotation()
2261+
lineWidth := m.annotateInput.Width
2262+
2263+
// file-level annotation
2264+
m.annotating = false
2265+
m.startFileAnnotation()
2266+
fileWidth := m.annotateInput.Width
2267+
2268+
assert.Greater(t, lineWidth, fileWidth, "file-level input should be narrower than line-level due to wider prefix")
2269+
assert.Equal(t, 6, lineWidth-fileWidth, "width difference should match prefix width difference (12-6=6)")
2270+
}
2271+
22442272
func TestModel_FileAnnotationSavesWithLineZero(t *testing.T) {
22452273
lines := []diff.DiffLine{
22462274
{NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext},
@@ -2330,6 +2358,66 @@ func TestModel_FileAnnotationCursorHighlighted(t *testing.T) {
23302358
assert.Contains(t, rendered, "file: file note", "file annotation should be rendered")
23312359
}
23322360

2361+
func TestModel_EnterOnFileAnnotationLineTriggersFileAnnotation(t *testing.T) {
2362+
lines := []diff.DiffLine{
2363+
{NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext},
2364+
}
2365+
m := testModel([]string{"a.go"}, nil)
2366+
m.tree = newFileTree([]string{"a.go"})
2367+
m.currFile = "a.go"
2368+
m.diffLines = lines
2369+
m.focus = paneDiff
2370+
m.diffCursor = -1 // on file annotation line
2371+
m.store.Add(annotation.Annotation{File: "a.go", Line: 0, Type: "", Comment: "existing note"})
2372+
2373+
// press enter on file annotation line - should start file annotation mode
2374+
result, cmd := m.Update(tea.KeyMsg{Type: tea.KeyEnter})
2375+
model := result.(Model)
2376+
assert.True(t, model.annotating, "enter on file annotation line should start annotation mode")
2377+
assert.True(t, model.fileAnnotating, "enter on file annotation line should set fileAnnotating")
2378+
assert.NotNil(t, cmd, "should return textinput blink command")
2379+
}
2380+
2381+
func TestModel_EnterOnFileAnnotationLinePreFillsText(t *testing.T) {
2382+
lines := []diff.DiffLine{
2383+
{NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext},
2384+
}
2385+
m := testModel([]string{"a.go"}, nil)
2386+
m.tree = newFileTree([]string{"a.go"})
2387+
m.currFile = "a.go"
2388+
m.diffLines = lines
2389+
m.focus = paneDiff
2390+
m.diffCursor = -1 // on file annotation line
2391+
m.store.Add(annotation.Annotation{File: "a.go", Line: 0, Type: "", Comment: "pre-existing comment"})
2392+
2393+
// press enter - should pre-fill with existing annotation text
2394+
result, _ := m.Update(tea.KeyMsg{Type: tea.KeyEnter})
2395+
model := result.(Model)
2396+
assert.Equal(t, "pre-existing comment", model.annotateInput.Value(), "should pre-fill with existing file annotation")
2397+
}
2398+
2399+
func TestModel_EnterOnRegularDiffLineStillTriggersLineAnnotation(t *testing.T) {
2400+
lines := []diff.DiffLine{
2401+
{NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext},
2402+
{NewNum: 2, Content: "added", ChangeType: diff.ChangeAdd},
2403+
}
2404+
m := testModel([]string{"a.go"}, nil)
2405+
m.tree = newFileTree([]string{"a.go"})
2406+
m.currFile = "a.go"
2407+
m.diffLines = lines
2408+
m.focus = paneDiff
2409+
m.diffCursor = 1 // on regular diff line, not file annotation
2410+
// add a file annotation to ensure it doesn't interfere
2411+
m.store.Add(annotation.Annotation{File: "a.go", Line: 0, Type: "", Comment: "file note"})
2412+
2413+
// press enter on regular line - should start line annotation, not file annotation
2414+
result, cmd := m.Update(tea.KeyMsg{Type: tea.KeyEnter})
2415+
model := result.(Model)
2416+
assert.True(t, model.annotating, "enter on regular line should start annotation mode")
2417+
assert.False(t, model.fileAnnotating, "enter on regular line should not set fileAnnotating")
2418+
assert.NotNil(t, cmd, "should return textinput blink command")
2419+
}
2420+
23332421
func TestModel_DeleteFileAnnotationViaD(t *testing.T) {
23342422
lines := []diff.DiffLine{
23352423
{NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext},

0 commit comments

Comments
 (0)