Skip to content

Fix file-level annotation input overflow and Enter-to-edit#27

Merged
umputun merged 4 commits intomasterfrom
fix-file-annotation-bugs
Apr 5, 2026
Merged

Fix file-level annotation input overflow and Enter-to-edit#27
umputun merged 4 commits intomasterfrom
fix-file-annotation-bugs

Conversation

@umputun
Copy link
Copy Markdown
Owner

@umputun umputun commented Apr 5, 2026

Fixes #26

Two bugs where file-level annotations (A key) behaved differently from line-level annotations:

1. Input width overflow - newAnnotationInput subtracted a fixed 6 chars for the prefix, but file-level annotations render a wider "💬 file: " prefix. Changed newAnnotationInput to accept a prefixWidth parameter so each caller passes the correct offset.

2. Enter key didn't edit file annotations - handleEnterKey always called startAnnotation() which returned nil when diffCursor == -1 (file annotation line). Added a cursorOnFileAnnotationLine() check to dispatch to startFileAnnotation() instead.

umputun added 3 commits April 5, 2026 13:00
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
In handleEnterKey, check cursorOnFileAnnotationLine() before falling
through to startAnnotation(). When on a file annotation line
(diffCursor == -1), dispatch to startFileAnnotation() instead.

Related to #26
Copilot AI review requested due to automatic review settings April 5, 2026 18:18
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes inconsistencies between file-level (A) and line-level (a) annotations by correcting the annotation input width calculation and enabling Enter-to-edit on the file-annotation header line.

Changes:

  • Parameterized annotation input width calculation to account for different rendered prefixes (line vs file).
  • Updated Enter-key handling to start file-level annotation editing when the cursor is on the file-annotation line.
  • Added regression tests covering width differences and Enter-to-edit behavior for file annotations.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
ui/model.go Routes Enter on the file-annotation header line to startFileAnnotation() so Enter edits file annotations.
ui/annotate.go Updates newAnnotationInput to accept a caller-provided prefix width and applies distinct offsets for line vs file annotations.
ui/model_test.go Adds tests for file-vs-line input widths and Enter-to-edit behavior on file annotation lines.
docs/plans/completed/20260405-fix-file-annotation-bugs.md Documents the investigation and implemented fix steps for the two bugs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ui/model.go Outdated
Comment on lines 1157 to 1162
if m.cursorOnFileAnnotationLine() {
cmd := m.startFileAnnotation()
m.viewport.SetContent(m.renderDiff())
return m, cmd
}
cmd := m.startAnnotation()
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleEnterKey now duplicates the same SetContent(renderDiff()) + return in both branches. Consider selecting the command (startFileAnnotation vs startAnnotation) first and then calling SetContent once to keep this handler easier to maintain.

Suggested change
if m.cursorOnFileAnnotationLine() {
cmd := m.startFileAnnotation()
m.viewport.SetContent(m.renderDiff())
return m, cmd
}
cmd := m.startAnnotation()
var cmd tea.Cmd
if m.cursorOnFileAnnotationLine() {
cmd = m.startFileAnnotation()
} else {
cmd = m.startAnnotation()
}

Copilot uses AI. Check for mistakes.

**Files:**
- Modify: `ui/annotate.go`
- Modify: `ui/diffview.go`
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This completed plan claims ui/diffview.go was modified in Task 1’s file list, but this PR doesn’t actually change that file. Consider updating the file list (or adjusting the text) so the plan accurately reflects what was changed.

Suggested change
- Modify: `ui/diffview.go`

Copilot uses AI. Check for mistakes.
Comment on lines 14 to +21
// newAnnotationInput creates and focuses a text input for annotation editing.
func (m *Model) newAnnotationInput(placeholder string) (textinput.Model, tea.Cmd) {
// prefixWidth accounts for the visible prefix characters (cursor col + emoji + label + margin).
func (m *Model) newAnnotationInput(placeholder string, prefixWidth int) (textinput.Model, tea.Cmd) {
ti := textinput.New()
ti.Placeholder = placeholder
cmd := ti.Focus()
ti.CharLimit = 500
ti.Width = max(10, m.diffContentWidth()-6) // cursor col + emoji prefix "💬 " + border margin
ti.Width = max(10, m.diffContentWidth()-prefixWidth)
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefixWidth values are still hard-coded (6, 12), which can drift from the actual rendered prefixes if the label/emoji changes again. Consider computing it from the exact prefix strings via lipgloss.Width(...) (optionally plus a named padding constant) and clarifying the comment since diffContentWidth() already excludes the cursor bar.

Copilot uses AI. Check for mistakes.
@umputun umputun merged commit 7527936 into master Apr 5, 2026
2 checks passed
@umputun umputun deleted the fix-file-annotation-bugs branch April 5, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File-level annotations behave differently from line-level annotations

2 participants