From 6bfe248388edc2e478afbb93eb37abee2ab1a14d Mon Sep 17 00:00:00 2001 From: Pablu Date: Mon, 27 Jul 2026 14:04:41 +0200 Subject: [PATCH] Make code suggestions work aswell --- README.md | 3 +- suggestions.go | 225 ++++++++++++++++++++++++++++++++++++++++++++ suggestions_test.go | 178 +++++++++++++++++++++++++++++++++++ tui.go | 141 ++++++++++++++++++++++----- 4 files changed, 521 insertions(+), 26 deletions(-) create mode 100644 suggestions.go create mode 100644 suggestions_test.go diff --git a/README.md b/README.md index 4bca5d9..1da2d3e 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,8 @@ A read-only terminal UI for people receiving GitHub pull-request reviews. It shows open PRs, review threads with highlighted diff hunks and comment authors, reviewer/assignee state, and the latest commit's check rollup. Resolved threads -start folded. The current PR is refreshed in the background. +start folded. GitHub suggestion blocks are shown as syntax-highlighted +remove/add previews. The current PR is refreshed in the background. ## Install and run diff --git a/suggestions.go b/suggestions.go new file mode 100644 index 0000000..0618845 --- /dev/null +++ b/suggestions.go @@ -0,0 +1,225 @@ +package main + +import ( + "strings" + + "github.com/charmbracelet/x/ansi" +) + +type parsedCommentBody struct { + Prose string + Suggestions []string +} + +type codeRange struct { + Start int + End int +} + +func parseCommentBody(body string) parsedCommentBody { + var ( + result parsedCommentBody + prose []string + suggestion []string + openingLine string + inSuggestion bool + ) + for _, line := range strings.Split(body, "\n") { + trimmed := strings.TrimSpace(line) + if !inSuggestion && isSuggestionFence(trimmed) { + inSuggestion = true + openingLine = line + suggestion = nil + continue + } + if inSuggestion && trimmed == "```" { + result.Suggestions = append(result.Suggestions, strings.Join(suggestion, "\n")) + inSuggestion = false + openingLine = "" + continue + } + if inSuggestion { + suggestion = append(suggestion, line) + } else { + prose = append(prose, line) + } + } + if inSuggestion { + prose = append(prose, openingLine) + prose = append(prose, suggestion...) + } + result.Prose = strings.TrimSpace(strings.Join(prose, "\n")) + return result +} + +func isSuggestionFence(line string) bool { + if !strings.HasPrefix(line, "```suggestion") { + return false + } + suffix := strings.TrimPrefix(line, "```suggestion") + return suffix == "" || strings.HasPrefix(suffix, ":") +} + +func reviewedSourceLines(thread ReviewThread, comment ReviewComment) []string { + start, end := commentReviewAnchor(thread, comment) + parsed := parseDiff(comment.DiffHunk, start, end, thread.DiffSide) + lines := make([]string, 0, end-start+1) + for _, line := range parsed { + if !line.selected { + continue + } + lines = append(lines, sourceFromDiffLine(line.raw)) + } + return lines +} + +func sourceFromDiffLine(raw string) string { + if strings.HasPrefix(raw, "+") || strings.HasPrefix(raw, "-") || strings.HasPrefix(raw, " ") { + raw = raw[1:] + } + return strings.ReplaceAll(raw, "\t", " ") +} + +func normalizeSuggestion(removed []string, replacement string) ([]string, []string) { + added := []string(nil) + if replacement != "" { + added = strings.Split(replacement, "\n") + for i := range added { + added[i] = strings.ReplaceAll(added[i], "\t", " ") + } + } + padding := commonSourceIndent(removed, added) + return trimSourceIndent(removed, padding), trimSourceIndent(added, padding) +} + +func commonSourceIndent(groups ...[]string) int { + padding := -1 + for _, lines := range groups { + for _, line := range lines { + if strings.TrimSpace(line) == "" { + continue + } + indent := len(line) - len(strings.TrimLeft(line, " ")) + if padding == -1 || indent < padding { + padding = indent + } + } + } + return max(0, padding) +} + +func trimSourceIndent(lines []string, padding int) []string { + result := make([]string, len(lines)) + for i, line := range lines { + result[i] = trimIndent(line, padding) + } + return result +} + +func suggestionChangedRanges(removed, added []string) ([]codeRange, []codeRange) { + removedRanges := make([]codeRange, len(removed)) + addedRanges := make([]codeRange, len(added)) + matches := matchingLinePairs(removed, added) + matches = append(matches, [2]int{len(removed), len(added)}) + + oldStart, newStart := 0, 0 + for _, match := range matches { + oldEnd, newEnd := match[0], match[1] + blockSize := max(oldEnd-oldStart, newEnd-newStart) + for offset := 0; offset < blockSize; offset++ { + oldIndex, newIndex := oldStart+offset, newStart+offset + switch { + case oldIndex < oldEnd && newIndex < newEnd: + removedRanges[oldIndex], addedRanges[newIndex] = changedRange( + removed[oldIndex], added[newIndex], + ) + case oldIndex < oldEnd: + removedRanges[oldIndex] = fullCodeRange(removed[oldIndex]) + case newIndex < newEnd: + addedRanges[newIndex] = fullCodeRange(added[newIndex]) + } + } + if oldEnd < len(removed) && newEnd < len(added) { + oldStart, newStart = oldEnd+1, newEnd+1 + } else { + oldStart, newStart = oldEnd, newEnd + } + } + return removedRanges, addedRanges +} + +func matchingLinePairs(removed, added []string) [][2]int { + dp := make([][]int, len(removed)+1) + for i := range dp { + dp[i] = make([]int, len(added)+1) + } + for i := len(removed) - 1; i >= 0; i-- { + for j := len(added) - 1; j >= 0; j-- { + if removed[i] == added[j] { + dp[i][j] = dp[i+1][j+1] + 1 + } else { + dp[i][j] = max(dp[i+1][j], dp[i][j+1]) + } + } + } + + var matches [][2]int + for i, j := 0, 0; i < len(removed) && j < len(added); { + switch { + case removed[i] == added[j]: + matches = append(matches, [2]int{i, j}) + i++ + j++ + case dp[i+1][j] >= dp[i][j+1]: + i++ + default: + j++ + } + } + return matches +} + +func changedRange(oldLine, newLine string) (codeRange, codeRange) { + oldRunes, newRunes := []rune(oldLine), []rune(newLine) + prefix := 0 + for prefix < len(oldRunes) && prefix < len(newRunes) && oldRunes[prefix] == newRunes[prefix] { + prefix++ + } + oldSuffix, newSuffix := len(oldRunes), len(newRunes) + for oldSuffix > prefix && newSuffix > prefix && + oldRunes[oldSuffix-1] == newRunes[newSuffix-1] { + oldSuffix-- + newSuffix-- + } + return codeRange{ + Start: ansi.StringWidth(string(oldRunes[:prefix])), + End: ansi.StringWidth(string(oldRunes[:oldSuffix])), + }, codeRange{ + Start: ansi.StringWidth(string(newRunes[:prefix])), + End: ansi.StringWidth(string(newRunes[:newSuffix])), + } +} + +func fullCodeRange(line string) codeRange { + return codeRange{End: ansi.StringWidth(line)} +} + +func commentReviewAnchor(thread ReviewThread, comment ReviewComment) (int, int) { + end := comment.OriginalLine + start := comment.OriginalStartLine + if end == 0 { + end = comment.Line + start = comment.StartLine + } + if end > 0 { + if start == 0 { + start = end + } + return start, end + } + start = thread.StartLine + if start == 0 { + start = thread.Line + } + return start, thread.Line +} diff --git a/suggestions_test.go b/suggestions_test.go new file mode 100644 index 0000000..6ffc140 --- /dev/null +++ b/suggestions_test.go @@ -0,0 +1,178 @@ +package main + +import ( + "strings" + "testing" + + "github.com/charmbracelet/x/ansi" +) + +func TestParseCommentBodyExtractsSuggestionFences(t *testing.T) { + body := "Use the clearer name:\n\n```suggestion:-0+0\n better_name = value\n```\n\nThis also matches the API." + got := parseCommentBody(body) + if len(got.Suggestions) != 1 || got.Suggestions[0] != " better_name = value" { + t.Fatalf("suggestions = %#v", got.Suggestions) + } + if strings.Contains(got.Prose, "```") || !strings.Contains(got.Prose, "Use the clearer name") || + !strings.Contains(got.Prose, "matches the API") { + t.Fatalf("prose = %q", got.Prose) + } +} + +func TestReviewedSourceLinesUseHistoricalSelection(t *testing.T) { + comment := ReviewComment{ + DiffHunk: "@@ -38,5 +38,5 @@\n context\n old_name = value\n return old_name\n context\n context", + OriginalStartLine: 39, + OriginalLine: 40, + } + got := reviewedSourceLines(ReviewThread{DiffSide: "RIGHT"}, comment) + want := []string{"old_name = value", "return old_name"} + if strings.Join(got, "\n") != strings.Join(want, "\n") { + t.Fatalf("reviewed source = %#v, want %#v", got, want) + } +} + +func TestNormalizeSuggestionPreservesRelativeIndent(t *testing.T) { + removed, added := normalizeSuggestion( + []string{" if old:", " run_old()"}, + " if new:\n run_new()", + ) + if strings.Join(removed, "\n") != "if old:\n run_old()" { + t.Fatalf("removed = %#v", removed) + } + if strings.Join(added, "\n") != "if new:\n run_new()" { + t.Fatalf("added = %#v", added) + } +} + +func TestDetailRendersSuggestionAsRemovalAndAddition(t *testing.T) { + m := NewApp(nil, "o", "r", false, 50, 10) + m.width = 100 + m.details = PRDetails{Threads: []ReviewThread{{ + Path: "example.py", + Line: 10, + DiffSide: "RIGHT", + Comments: []ReviewComment{{ + Author: "reviewer", + Body: "Use this instead:\n```suggestion\nnew_value = compute()\nreturn new_value\n```", + DiffHunk: "@@ -10,2 +10,2 @@\nold_value = compute()\nreturn old_value", + OriginalLine: 11, + OriginalStartLine: 10, + }}, + }}} + + var ( + removed, added int + rendered strings.Builder + ) + for _, line := range m.detailLines(60) { + rendered.WriteString(ansi.Strip(line.rail + line.fixed + line.text)) + rendered.WriteByte('\n') + switch line.suggestionChange { + case '-': + removed++ + case '+': + added++ + } + } + if removed != 2 || added != 2 { + t.Fatalf("suggestion rows: removed=%d added=%d\n%s", removed, added, rendered.String()) + } + if strings.Contains(rendered.String(), "```suggestion") || + !strings.Contains(rendered.String(), "│ @reviewer") || + !strings.Contains(rendered.String(), "│ suggested change") { + t.Fatalf("suggestion was not rendered structurally:\n%s", rendered.String()) + } +} + +func TestSuggestionBackgroundIsDirectionalWithoutTextUnderline(t *testing.T) { + removed := suggestionHighlight(" - ", "old", 12, '-') + added := suggestionHighlight(" + ", "new", 12, '+') + if !strings.Contains(removed, "\x1b[48;5;52m") || + !strings.Contains(added, "\x1b[48;5;22m") { + t.Fatalf("suggestion decorations missing: removed=%q added=%q", removed, added) + } + if strings.Contains(removed, "\x1b[4m") || strings.Contains(added, "\x1b[4m") || + strings.Contains(removed, "\x1b[4;") || strings.Contains(added, "\x1b[4;") || + strings.Contains(removed, "\x1b[58;") || strings.Contains(added, "\x1b[58;") { + t.Fatal("suggestion rendering still enables terminal underlining") + } + if ansi.StringWidth(removed) != 12 || ansi.StringWidth(added) != 12 { + t.Fatal("suggestion backgrounds do not fill the row") + } + if !strings.Contains(removed, "\x1b[0m\x1b[48;5;52m ") || + !strings.Contains(added, "\x1b[0m\x1b[48;5;22m ") { + t.Fatal("padded row remainder is still underlined") + } +} + +func TestMultipleSuggestionsStayInsideCommentBlock(t *testing.T) { + m := NewApp(nil, "o", "r", false, 50, 10) + m.width = 100 + m.details = PRDetails{Threads: []ReviewThread{{ + Path: "main.go", Line: 1, DiffSide: "RIGHT", + Comments: []ReviewComment{{ + Author: "alice", OriginalLine: 1, + DiffHunk: "@@ -1 +1 @@\nold", + Body: "```suggestion\nfirst\n```\n```suggestion\nsecond\n```", + }}, + }}} + var rendered strings.Builder + for _, line := range m.detailLines(60) { + rendered.WriteString(ansi.Strip(line.rail + line.fixed + line.text)) + rendered.WriteByte('\n') + } + if !strings.Contains(rendered.String(), "│ @alice") || + !strings.Contains(rendered.String(), "│ suggested change 1/2") || + !strings.Contains(rendered.String(), "│ suggested change 2/2") { + t.Fatalf("suggestions escaped the comment block:\n%s", rendered.String()) + } +} + +func TestSuggestionGutterHasNoLeadingPadding(t *testing.T) { + lines := wrapSuggestionLine("main.go", "replacement()", '+', fullCodeRange("replacement()"), 30) + if len(lines) == 0 || ansi.Strip(lines[0].fixed) != "+ " { + t.Fatalf("suggestion gutter = %q, want compact marker", ansi.Strip(lines[0].fixed)) + } +} + +func TestSuggestionChangedRangesAlignInsertedLines(t *testing.T) { + removed := []string{"def setup():", " if ready:", " run()"} + added := []string{"def setup():", " super().setup()", " if ready:", " run()"} + removedRanges, addedRanges := suggestionChangedRanges(removed, added) + + for i, changed := range removedRanges { + if changed.End != changed.Start { + t.Fatalf("unchanged removed line %d marked as changed: %#v", i, changed) + } + } + for _, index := range []int{0, 2, 3} { + if changed := addedRanges[index]; changed.End != changed.Start { + t.Fatalf("unchanged added line %d marked as changed: %#v", index, changed) + } + } + if addedRanges[1] != fullCodeRange(added[1]) { + t.Fatalf("inserted line range = %#v", addedRanges[1]) + } +} + +func TestSuggestionChangedRangeIsCharacterPrecise(t *testing.T) { + removed, added := changedRange("value = old_name()", "value = new_name()") + if got := "value = old_name()"[removed.Start:removed.End]; got != "old" { + t.Fatalf("removed range selected %q", got) + } + if got := "value = new_name()"[added.Start:added.End]; got != "new" { + t.Fatalf("added range selected %q", got) + } +} + +func TestChangedSnippetGetsDarkerBackground(t *testing.T) { + code := highlightedSource("go", "value := oldName()") + decorated := highlightCodeRange(code, codeRange{Start: 9, End: 16}, '-') + if !strings.Contains(decorated, "\x1b[48;2;55;0;0m") { + t.Fatalf("darker intra-line background missing: %q", decorated) + } + if ansi.Strip(decorated) != "value := oldName()" { + t.Fatalf("highlight altered source: %q", ansi.Strip(decorated)) + } +} diff --git a/tui.go b/tui.go index 10133dc..5cbe27b 100644 --- a/tui.go +++ b/tui.go @@ -437,7 +437,15 @@ func (m App) threadDetail(width, height int) string { rendered := make([]string, 0, len(visible)) innerWidth := max(1, width-2) for _, line := range visible { - renderedLine := ansi.Truncate(line.fixed+line.text, innerWidth, "…") + railWidth := ansi.StringWidth(line.rail) + contentWidth := max(1, innerWidth-railWidth) + var renderedLine string + if line.suggestionChange != 0 { + renderedLine = suggestionHighlight(line.fixed, line.text, contentWidth, line.suggestionChange) + } else { + renderedLine = ansi.Truncate(line.fixed+line.text, contentWidth, "…") + } + renderedLine = line.rail + renderedLine if line.selected { renderedLine = selectedBackground(renderedLine, innerWidth) } @@ -447,9 +455,11 @@ func (m App) threadDetail(width, height int) string { } type detailLine struct { - text string - fixed string - selected bool + text string + fixed string + rail string + selected bool + suggestionChange byte } func (m App) detailLines(width int) []detailLine { @@ -481,9 +491,40 @@ func (m App) detailLines(width int) []detailLine { } } for _, comment := range thread.Comments { - lines = append(lines, detailLine{}, detailLine{text: authorStyle(comment.Author).Render("@"+comment.Author) + " " + dimStyle.Render(comment.CreatedAt.Local().Format("2006-01-02 15:04"))}) - for _, commentLine := range strings.Split(wrap(comment.Body, max(10, width-4)), "\n") { - lines = append(lines, detailLine{text: commentLine}) + content := parseCommentBody(comment.Body) + rail := lipgloss.NewStyle().Foreground(authorColor(comment.Author)).Render("│ ") + lines = append(lines, detailLine{}, detailLine{ + rail: rail, + text: authorStyle(comment.Author).Render("@"+comment.Author) + " " + + dimStyle.Render(comment.CreatedAt.Local().Format("2006-01-02 15:04")), + }) + if content.Prose != "" { + for _, commentLine := range strings.Split(wrap(content.Prose, max(10, width-4)), "\n") { + lines = append(lines, detailLine{rail: rail, text: commentLine}) + } + } + for suggestionIndex, suggestion := range content.Suggestions { + removed, added := normalizeSuggestion(reviewedSourceLines(thread, comment), suggestion) + removedRanges, addedRanges := suggestionChangedRanges(removed, added) + label := "suggested change" + if len(content.Suggestions) > 1 { + label = fmt.Sprintf("suggested change %d/%d", suggestionIndex+1, len(content.Suggestions)) + } + lines = append(lines, detailLine{rail: rail}, detailLine{rail: rail, text: dimStyle.Render(label)}) + for sourceIndex, source := range removed { + lines = append(lines, addCommentRail( + wrapSuggestionLine( + thread.Path, source, '-', removedRanges[sourceIndex], max(1, width-4), + ), rail, + )...) + } + for sourceIndex, source := range added { + lines = append(lines, addCommentRail( + wrapSuggestionLine( + thread.Path, source, '+', addedRanges[sourceIndex], max(1, width-4), + ), rail, + )...) + } } } if thread.IsTruncated { @@ -520,6 +561,53 @@ func wrapDiffLine(line highlightedDiffLine, width int) []detailLine { return result } +func wrapSuggestionLine(path, source string, change byte, changed codeRange, width int) []detailLine { + marker := badStyle.Render(string(change)) + if change == '+' { + marker = okStyle.Render(string(change)) + } + code := highlightedSource(lexerForPath(path), source) + code = highlightCodeRange(code, changed, change) + lines := wrapDiffLine(highlightedDiffLine{ + gutter: marker + " ", + code: code, + }, width) + for i := range lines { + lines[i].suggestionChange = change + } + return lines +} + +func highlightCodeRange(code string, changed codeRange, change byte) string { + if changed.End <= changed.Start { + return code + } + totalWidth := ansi.StringWidth(code) + start := clamp(changed.Start, 0, totalWidth) + end := clamp(changed.End, start, totalWidth) + if end <= start { + return code + } + + background := "\x1b[48;2;55;0;0m" + if change == '+' { + background = "\x1b[48;2;0;55;0m" + } + const reset = "\x1b[0m" + before := ansi.Cut(code, 0, start) + middle := ansi.Cut(code, start, end) + after := ansi.Cut(code, end, totalWidth) + middle = strings.ReplaceAll(middle, reset, reset+background) + return before + background + middle + reset + after +} + +func addCommentRail(lines []detailLine, rail string) []detailLine { + for i := range lines { + lines[i].rail = rail + } + return lines +} + func wrapCodeWithIndent(code string, width int) []string { if width <= 0 || ansi.StringWidth(code) <= width { return []string{code} @@ -580,25 +668,9 @@ func isCodeBreakpoint(r rune) bool { func reviewAnchor(thread ReviewThread) (int, int) { if len(thread.Comments) > 0 { - comment := thread.Comments[0] - end := comment.OriginalLine - start := comment.OriginalStartLine - if end == 0 { - end = comment.Line - start = comment.StartLine - } - if end > 0 { - if start == 0 { - start = end - } - return start, end - } + return commentReviewAnchor(thread, thread.Comments[0]) } - start := thread.StartLine - if start == 0 { - start = thread.Line - } - return start, thread.Line + return commentReviewAnchor(thread, ReviewComment{}) } func shortOID(oid string) string { @@ -618,6 +690,25 @@ func selectedBackground(line string, width int) string { return background + line + reset } +func suggestionHighlight(gutter, code string, width int, change byte) string { + background := "\x1b[48;5;52m" + if change == '+' { + background = "\x1b[48;5;22m" + } + const reset = "\x1b[0m" + gutter = ansi.Truncate(gutter, width, "") + gutter = strings.ReplaceAll(gutter, reset, reset+background) + codeWidth := max(0, width-ansi.StringWidth(gutter)) + code = ansi.Truncate(code, codeWidth, "") + code = strings.ReplaceAll(code, reset, reset+background) + content := background + gutter + code + reset + padding := max(0, width-ansi.StringWidth(gutter)-ansi.StringWidth(code)) + if padding > 0 { + content += background + strings.Repeat(" ", padding) + reset + } + return content +} + var authorPalette = []lipgloss.Color{ "#61AFEF", "#C678DD", "#56B6C2", "#E5C07B", "#E06C75", "#98C379", "#D19A66", "#7FC8FF",