Commit 1dadf31dc2

1dadf31dc2c6bd0bbea6d413922e113aabd348aa

parent: 4ce872b9f4

Verified · cmc

cmc <hello@cleberg.net> · 2026-10-14 04:58 UTC

Review comments on diff lines, and suggested changes

Tapping a line of a merge request's diff offers a comment, and on the
new side a suggested change: a sheet anchored on the line or a range
ending on it, with the lines' text in a suggestion block to edit. A
thread with a suggestion shows the lines it replaces and proposes, and
Apply commits it on the server; where the repository requires signed
commits it gives the CLI command instead, since only the user's key
can sign. A tap rather than a context menu, because in a list row the
first line's menu covers the whole hunk.

Closes #23

Layout: unified · split

gitbay/MRs/DiffCommentDraft.swift added +63
@@ -0,0 +1,63 @@
1import Foundation
2
3/// A new review comment on lines of a merge request's diff: where it is
4/// anchored, and the ```suggestion block that proposes replacement lines.
5/// `mr diff-comment` decides whether the anchor is in the diff; this only
6/// builds what the command is asked.
7nonisolated struct DiffCommentDraft: Sendable, Hashable, Identifiable {
8 let path: String
9 /// The old side is a removed line; everything else anchors on the new file.
10 let old: Bool
11 /// The last line of the range, the one the comment was started from.
12 let line: Int
13 /// The lines on the same side of the hunk, in order, that a range may
14 /// start from: the tapped line and those above it, back to the first
15 /// line of the hunk on that side.
16 let candidates: [(number: Int, text: String)]
17
18 var id: String { "\(path):\(old ? "old" : "new"):\(line)" }
19
20 static func == (a: Self, b: Self) -> Bool { a.id == b.id }
21 func hash(into hasher: inout Hasher) { hasher.combine(id) }
22
23 /// A suggestion replaces new-file lines; a removed line has nothing
24 /// to replace.
25 var canSuggest: Bool { !old }
26
27 /// The earliest line a range may start on.
28 var firstLine: Int { candidates.first?.number ?? line }
29
30 /// The draft for a line of the diff, or nil for a line no side can
31 /// anchor (none in a well-formed diff).
32 init?(file: UnifiedDiff.File, hunk: UnifiedDiff.Hunk, line target: UnifiedDiff.Line) {
33 let old = target.kind == .deletion
34 guard let number = old ? target.oldNumber : target.newNumber else { return nil }
35 self.path = old ? file.oldPath : file.newPath
36 self.old = old
37 self.line = number
38 var lines: [(Int, String)] = []
39 for l in hunk.lines {
40 let n = old ? (l.kind == .addition ? nil : l.oldNumber) : (l.kind == .deletion ? nil : l.newNumber)
41 guard let n else { continue }
42 if n > number { break }
43 lines.append((n, l.text))
44 }
45 self.candidates = lines
46 }
47
48 /// `mr diff-comment <owner/name> <n> --path <p> --line <l> [--start-line <s>] [--old] --file -`.
49 func argv(repo: String, number: Int64, startLine: Int) -> [String] {
50 var argv = ["mr", "diff-comment", repo, String(number), "--path", path, "--line", String(line)]
51 if startLine < line { argv += ["--start-line", String(startLine)] }
52 if old { argv.append("--old") }
53 argv += ["--file", "-"]
54 return argv
55 }
56
57 /// The ```suggestion block for lines `startLine` through `line`, with
58 /// the current text in it to edit.
59 func suggestion(from startLine: Int) -> String {
60 let text = candidates.filter { $0.number >= startLine }.map(\.text)
61 return "```suggestion\n" + text.map { $0 + "\n" }.joined() + "```\n"
62 }
63}
gitbay/MRs/MRDetailViewModel.swift +13
@@ -169,6 +169,19 @@ final class MRDetailViewModel {
169169 )
170170 }
171171
172 /// Start a review thread on lines of the diff. The body goes over
173 /// stdin; a ```suggestion block in it is the server's to read. Returns
174 /// whether it was accepted, so the sheet can close.
175 func diffComment(_ draft: DiffCommentDraft, startLine: Int, body: String) async -> Bool {
176 await perform(draft.argv(repo: repoPath, number: number, startLine: startLine), stdin: body)
177 return actionError == nil
178 }
179
180 /// Commit a thread's suggestion to the source branch on the server.
181 func applySuggestion(_ thread: ReviewThread) async {
182 await perform(["mr", "apply-suggestion"] + ref + [String(thread.id)])
183 }
184
172185 func setDraft(_ draft: Bool) async {
173186 await perform(["mr", draft ? "draft" : "ready"] + ref)
174187 }
gitbay/MRs/MRModels.swift +12 −1
@@ -302,16 +302,27 @@ nonisolated struct ReviewThread: Decodable, Sendable, Hashable, Identifiable {
302302 let path: String
303303 let side: String // new | old
304304 let line: Int64
305 /// The first line of a range; absent for a one-line thread.
306 let startLine: Int64?
305307 /// Anchored to an older head; the diff has moved under it.
306308 let stale: Bool
307309 let resolvedBy: String?
310 /// What the thread's ```suggestion block proposes (v1.39.0).
311 let suggestion: Suggestion?
308312 let comments: [Comment]
309313
310314 enum CodingKeys: String, CodingKey {
311 case id, path, side, line, stale, comments
315 case id, path, side, line, stale, suggestion, comments
316 case startLine = "start_line"
312317 case resolvedBy = "resolved_by"
313318 }
314319
320 /// `path:line`, or `path:start–line` for a range.
321 var anchor: String {
322 if let startLine, startLine < line { return "\(path):\(startLine)–\(line)" }
323 return "\(path):\(line)"
324 }
325
315326 var isResolved: Bool { resolvedBy != nil }
316327
317328 nonisolated struct Comment: Decodable, Sendable, Hashable, Identifiable {
gitbay/MRs/Suggestion.swift added +36
@@ -0,0 +1,36 @@
1import Foundation
2
3/// The change a review thread's ```suggestion block proposes, as
4/// `mr threads` carries it: lines `startLine` through `endLine` of `path`,
5/// `original` replaced by `replacement`. Both texts end every line with
6/// its terminator, so "" is no lines.
7nonisolated struct Suggestion: Decodable, Sendable, Hashable {
8 let path: String
9 let startLine: Int64
10 let endLine: Int64
11 let original: String
12 let replacement: String
13 /// The lines it replaces have changed at the head, or the file moved;
14 /// it cannot be applied then, and `reason` says which.
15 let outdated: Bool
16 let reason: String?
17 /// "server" where `mr apply-suggestion` commits it; "local" where the
18 /// repository requires signed commits, which only a clone can make.
19 let apply: String
20
21 enum CodingKeys: String, CodingKey {
22 case path, original, replacement, outdated, reason, apply
23 case startLine = "start_line"
24 case endLine = "end_line"
25 }
26
27 var appliesOnServer: Bool { apply == "server" }
28
29 /// The text as lines, without the terminator the last one carries.
30 static func lines(_ text: String) -> [String] {
31 guard !text.isEmpty else { return [] }
32 var lines = text.components(separatedBy: "\n")
33 if lines.last == "" { lines.removeLast() }
34 return lines
35 }
36}
gitbay/Views/MRs/DiffCommentSheet.swift added +164
@@ -0,0 +1,164 @@
1import SwiftUI
2
3/// A new review comment on a line of the diff, or a range ending on it.
4/// On the new side the comment can carry a ```suggestion block; the
5/// server reads the block out of the body, so the sheet only writes text.
6struct DiffCommentSheet: View {
7
8 let draft: DiffCommentDraft
9 let model: MRDetailViewModel
10 let suggesting: Bool
11
12 @State private var startLine: Int
13 @State private var text: String
14 @Environment(\.dismiss) private var dismiss
15
16 init(draft: DiffCommentDraft, model: MRDetailViewModel, suggesting: Bool) {
17 self.draft = draft
18 self.model = model
19 self.suggesting = suggesting
20 _startLine = State(initialValue: draft.line)
21 _text = State(initialValue: suggesting ? draft.suggestion(from: draft.line) : "")
22 }
23
24 var body: some View {
25 NavigationStack {
26 Form {
27 Section {
28 Text(anchor)
29 .font(.gbMono(.caption))
30 .accessibilityIdentifier("diff-comment-anchor")
31 if draft.firstLine < draft.line {
32 Stepper(value: $startLine, in: draft.firstLine...draft.line) {
33 Text("From line \(startLine)")
34 .font(.gbSans(.subheadline))
35 }
36 .accessibilityIdentifier("diff-comment-start")
37 }
38 }
39 Section {
40 TextEditor(text: $text)
41 .font(.gbMono(.subheadline))
42 .frame(minHeight: 160)
43 .autocorrectionDisabled()
44 .textInputAutocapitalization(.never)
45 .accessibilityIdentifier("diff-comment-body")
46 if draft.canSuggest {
47 Button("Insert suggestion") {
48 if !text.isEmpty && !text.hasSuffix("\n") { text += "\n" }
49 text += draft.suggestion(from: startLine)
50 }
51 .font(.gbSans(.subheadline))
52 .accessibilityIdentifier("diff-comment-insert-suggestion")
53 }
54 MarkupPreview(text: text, format: "md", identifier: "diff-comment-preview")
55 } footer: {
56 if draft.canSuggest {
57 Text("A suggestion replaces the lines in its range with what the block holds. An empty block deletes them.")
58 }
59 }
60 if let error = model.actionError {
61 Section { GBNotice(error) }
62 }
63 }
64 .scrollDismissesKeyboard(.interactively)
65 .navigationTitle(suggesting ? "Suggest a Change" : "Comment")
66 .navigationBarTitleDisplayMode(.inline)
67 .toolbar {
68 ToolbarItem(placement: .cancellationAction) {
69 Button("Cancel") { dismiss() }
70 }
71 ToolbarItem(placement: .confirmationAction) {
72 if model.working {
73 ProgressView()
74 } else {
75 Button("Send") {
76 Task {
77 if await model.diffComment(draft, startLine: startLine, body: text) {
78 dismiss()
79 }
80 }
81 }
82 .disabled(text.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty)
83 .accessibilityIdentifier("diff-comment-send")
84 }
85 }
86 }
87 .interactiveDismissDisabled(model.working)
88 }
89 }
90
91 private var anchor: String {
92 let side = draft.old ? " (old)" : ""
93 return startLine < draft.line
94 ? "\(draft.path):\(startLine)–\(draft.line)\(side)"
95 : "\(draft.path):\(draft.line)\(side)"
96 }
97}
98
99/// What a thread's suggestion replaces and proposes, as removed and added
100/// lines, with the action the server allows.
101struct SuggestionView: View {
102
103 let suggestion: Suggestion
104 let thread: ReviewThread
105 let model: MRDetailViewModel
106
107 var body: some View {
108 VStack(alignment: .leading, spacing: 6) {
109 HStack(spacing: 6) {
110 Text("Suggested change")
111 .font(.gbSans(.caption).weight(.semibold))
112 if suggestion.outdated {
113 GBChip("outdated", .gbWarn)
114 }
115 }
116 VStack(alignment: .leading, spacing: 0) {
117 ForEach(Array(Suggestion.lines(suggestion.original).enumerated()), id: \.offset) { _, line in
118 row("−", line, .gbBad, .gbDiffDel)
119 }
120 ForEach(Array(Suggestion.lines(suggestion.replacement).enumerated()), id: \.offset) { _, line in
121 row("+", line, .gbOK, .gbDiffAdd)
122 }
123 }
124 .font(.gbMono(.caption2))
125 if suggestion.outdated {
126 Text(suggestion.reason ?? "The lines it replaces have changed.")
127 .font(.gbSans(.caption))
128 .foregroundStyle(.secondary)
129 } else if !thread.isResolved {
130 if suggestion.appliesOnServer {
131 Button("Apply suggestion") {
132 Task { await model.applySuggestion(thread) }
133 }
134 .font(.gbSans(.caption))
135 .buttonStyle(.bordered)
136 .disabled(model.working)
137 .accessibilityIdentifier("apply-suggestion-\(thread.id)")
138 } else {
139 // The repository requires signed commits, and only the
140 // user's own key can sign; the CLI applies it from a clone.
141 Text("This repository requires signed commits. Apply it from a clone:")
142 .font(.gbSans(.caption))
143 .foregroundStyle(.secondary)
144 Text("gitbay mr apply-suggestion \(model.number) \(thread.id)")
145 .font(.gbMono(.caption))
146 .textSelection(.enabled)
147 }
148 }
149 }
150 }
151
152 private func row(_ marker: String, _ text: String, _ markerColor: Color, _ background: Color) -> some View {
153 HStack(spacing: 0) {
154 Text(marker)
155 .frame(width: 14)
156 .foregroundStyle(markerColor)
157 Text(text.isEmpty ? " " : text)
158 Spacer(minLength: 0)
159 }
160 .background(background)
161 .accessibilityElement(children: .ignore)
162 .accessibilityLabel("\(marker == "+" ? "Proposed" : "Replaced"): \(text)")
163 }
164}
gitbay/Views/MRs/DiffView.swift +45 −1
@@ -100,6 +100,16 @@ private struct HunkView: View {
100100 let file: UnifiedDiff.File
101101 let hunk: UnifiedDiff.Hunk
102102 let model: MRDetailViewModel?
103 @State private var composing: Compose?
104 /// The line tapped, whose actions are on offer.
105 @State private var picked: DiffCommentDraft?
106
107 /// A comment being written on a line, and whether it opened as a suggestion.
108 private struct Compose: Identifiable {
109 let draft: DiffCommentDraft
110 let suggesting: Bool
111 var id: String { draft.id + (suggesting ? ":s" : "") }
112 }
103113
104114 var body: some View {
105115 ScrollView(.horizontal) {
@@ -119,7 +129,19 @@ private struct HunkView: View {
119129 Rectangle().fill(Color.gbLine).frame(height: 1)
120130 }
121131 ForEach(hunk.lines) { line in
122 LineView(line: line)
132 if model != nil {
133 // A tap rather than a context menu: in a list row SwiftUI
134 // hoists the first line's menu to the whole hunk, so every
135 // line would offer line one's anchor.
136 LineView(line: line)
137 .contentShape(Rectangle())
138 .onTapGesture { pick(line) }
139 .accessibilityAddTraits(.isButton)
140 .accessibilityHint("Comment on this line")
141 .accessibilityAction { pick(line) }
142 } else {
143 LineView(line: line)
144 }
123145 if let model {
124146 ForEach(model.threads.filter { line.anchors($0, in: file) }) { thread in
125147 ReviewThreadView(thread: thread, model: model)
@@ -132,6 +154,27 @@ private struct HunkView: View {
132154 }
133155 }
134156 .listRowInsets(EdgeInsets(top: 2, leading: 8, bottom: 2, trailing: 8))
157 .confirmationDialog(
158 picked.map { "\($0.path):\($0.line)\($0.old ? " (old)" : "")" } ?? "",
159 isPresented: Binding(get: { picked != nil }, set: { if !$0 { picked = nil } }),
160 titleVisibility: .visible,
161 presenting: picked
162 ) { draft in
163 Button("Comment") { composing = Compose(draft: draft, suggesting: false) }
164 if draft.canSuggest {
165 Button("Suggest a change") { composing = Compose(draft: draft, suggesting: true) }
166 }
167 Button("Cancel", role: .cancel) {}
168 }
169 .sheet(item: $composing) { compose in
170 if let model {
171 DiffCommentSheet(draft: compose.draft, model: model, suggesting: compose.suggesting)
172 }
173 }
174 }
175
176 private func pick(_ line: UnifiedDiff.Line) {
177 picked = DiffCommentDraft(file: file, hunk: hunk, line: line)
135178 }
136179}
137180
@@ -159,6 +202,7 @@ private struct LineView: View {
159202 .background(background)
160203 .accessibilityElement(children: .ignore)
161204 .accessibilityLabel(line.accessibilityLabel)
205 .accessibilityIdentifier("diff-line-\(line.oldNumber ?? 0)-\(line.newNumber ?? 0)")
162206 }
163207
164208 private var marker: String {
gitbay/Views/MRs/MRView.swift +4 −1
@@ -679,7 +679,7 @@ struct ReviewThreadView: View {
679679 ? "checkmark.bubble" : "bubble.left.and.exclamationmark.bubble.right")
680680 .font(.gbSans(.caption))
681681 .foregroundStyle(thread.isResolved ? Color.gbOK : Color.gbWarn)
682 Text("\(thread.path):\(thread.line)")
682 Text(thread.anchor)
683683 .font(.gbMono(.caption))
684684 .lineLimit(1)
685685 if thread.stale {
@@ -693,6 +693,9 @@ struct ReviewThreadView: View {
693693 .buttonStyle(.bordered)
694694 .disabled(model.working)
695695 }
696 if let suggestion = thread.suggestion {
697 SuggestionView(suggestion: suggestion, thread: thread, model: model)
698 }
696699 ForEach(thread.comments) { comment in
697700 VStack(alignment: .leading, spacing: 2) {
698701 HStack {
gitbayTests/SuggestionTests.swift added +146
@@ -0,0 +1,146 @@
1import Foundation
2import Testing
3@testable import gitbay
4
5private func makeClient() throws -> (GitbayClient, StubProtocol.Box) {
6 let box = StubProtocol.box()
7 let client = GitbayClient(
8 instance: try GitbayInstance(url: "https://gitbay.org"),
9 token: "test-token",
10 session: box.session()
11 )
12 return (client, box)
13}
14
15private func posted(_ stub: StubProtocol.Box) throws -> ([String], String?) {
16 let seen = try #require(stub.seen.first { $0.method == "POST" })
17 let body = try #require(try JSONSerialization.jsonObject(with: seen.body) as? [String: Any])
18 return (try #require(body["argv"] as? [String]), body["stdin"] as? String)
19}
20
21private let diffText = """
22 diff --git a/main.go b/main.go
23 --- a/main.go
24 +++ b/main.go
25 @@ -10,4 +10,4 @@ func main() {
26 \tname := os.Args[1]
27 -\tlog.Print("starting")
28 +\tlog.Printf("starting %s", name)
29 \trun(name)
30 }
31 """
32
33private let mrShowJSON = """
34 {"protocol_version":1,"data":{"number":4,"title":"t","state":"open","author":"krz",\
35 "source":"feat","target_ref":"main","head_sha":"abc","created_at":"2026-08-20T10:00:00.000Z"},\
36 "exit_code":0}
37 """
38
39private let threadsJSON = """
40 {"protocol_version":1,"data":[{"id":9,"path":"main.go","side":"new","start_line":11,"line":12,\
41 "stale":false,"suggestion":{"path":"main.go","start_line":11,"end_line":12,"commit":"abc",\
42 "original":"\\tlog.Printf(\\"starting %s\\", name)\\n\\trun(name)\\n",\
43 "replacement":"\\tlog.Printf(\\"starting %q\\", name)\\n\\trun(name)\\n",\
44 "outdated":false,"apply":"server"},\
45 "comments":[{"id":9,"author":"cmc","body":"quote it","created_at":"2026-08-22T09:00:00.000Z"}]}],\
46 "exit_code":0}
47 """
48
49private let ok = #"{"protocol_version":1,"data":{},"exit_code":0}"#
50
51private func hunk() throws -> (UnifiedDiff.File, UnifiedDiff.Hunk) {
52 let file = try #require(UnifiedDiff.parse(diffText).files.first)
53 return (file, try #require(file.hunks.first))
54}
55
56struct DiffCommentDraftTests {
57
58 @Test func aNewSideLineAnchorsOnTheNewFileAndCanSuggest() throws {
59 let (file, hunk) = try hunk()
60 let target = try #require(hunk.lines.first { $0.newNumber == 12 && $0.kind == .context })
61 let draft = try #require(DiffCommentDraft(file: file, hunk: hunk, line: target))
62
63 #expect(draft.canSuggest)
64 #expect(draft.line == 12)
65 // The removed line has no new-side number and is not a candidate.
66 #expect(draft.candidates.map(\.number) == [10, 11, 12])
67 #expect(draft.firstLine == 10)
68 #expect(draft.argv(repo: "krz/gitbay", number: 4, startLine: 12)
69 == ["mr", "diff-comment", "krz/gitbay", "4", "--path", "main.go", "--line", "12", "--file", "-"])
70 #expect(draft.argv(repo: "krz/gitbay", number: 4, startLine: 11)
71 == ["mr", "diff-comment", "krz/gitbay", "4", "--path", "main.go", "--line", "12",
72 "--start-line", "11", "--file", "-"])
73 }
74
75 @Test func theSuggestionBlockCarriesTheRangesCurrentText() throws {
76 let (file, hunk) = try hunk()
77 let target = try #require(hunk.lines.first { $0.newNumber == 12 && $0.kind == .context })
78 let draft = try #require(DiffCommentDraft(file: file, hunk: hunk, line: target))
79
80 #expect(draft.suggestion(from: 11) == "```suggestion\n\tlog.Printf(\"starting %s\", name)\n\trun(name)\n```\n")
81 }
82
83 @Test func aRemovedLineAnchorsOnTheOldSideAndCannotSuggest() throws {
84 let (file, hunk) = try hunk()
85 let target = try #require(hunk.lines.first { $0.kind == .deletion })
86 let draft = try #require(DiffCommentDraft(file: file, hunk: hunk, line: target))
87
88 #expect(!draft.canSuggest)
89 #expect(draft.candidates.map(\.number) == [10, 11])
90 #expect(draft.argv(repo: "krz/gitbay", number: 4, startLine: 11)
91 == ["mr", "diff-comment", "krz/gitbay", "4", "--path", "main.go", "--line", "11", "--old", "--file", "-"])
92 }
93}
94
95@MainActor
96struct SuggestionViewModelTests {
97
98 private func loaded() async throws -> (MRDetailViewModel, StubProtocol.Box) {
99 let (client, stub) = try makeClient()
100 stub.enqueue(.init(status: 200, json: mrShowJSON, match: "argv=show"))
101 stub.enqueue(.init(status: 200, json: #"{"protocol_version":1,"output":"","exit_code":0}"#, match: "argv=diff"))
102 stub.enqueue(.init(status: 200, json: threadsJSON, match: "argv=threads"))
103 stub.enqueue(.init(status: 200, json: #"{"protocol_version":1,"exit_code":0}"#, match: "argv=revisions"))
104 let model = MRDetailViewModel(client: client, repoPath: "krz/gitbay", number: 4)
105 await model.load()
106 return (model, stub)
107 }
108
109 @Test func threadsDecodeTheirRangeAndSuggestion() async throws {
110 let (model, _) = try await loaded()
111
112 let thread = try #require(model.threads.first)
113 #expect(thread.startLine == 11)
114 let suggestion = try #require(thread.suggestion)
115 #expect(suggestion.appliesOnServer)
116 #expect(!suggestion.outdated)
117 #expect(Suggestion.lines(suggestion.replacement) == ["\tlog.Printf(\"starting %q\", name)", "\trun(name)"])
118 #expect(Suggestion.lines("") == [])
119 }
120
121 @Test func applyingASuggestionNamesItsThread() async throws {
122 let (model, stub) = try await loaded()
123 stub.enqueue(.init(status: 200, json: ok, match: "cmd"))
124
125 await model.applySuggestion(try #require(model.threads.first))
126
127 let (argv, _) = try posted(stub)
128 #expect(argv == ["mr", "apply-suggestion", "krz/gitbay", "4", "9"])
129 }
130
131 @Test func aNewCommentSendsItsAnchorAndBodyOnStdin() async throws {
132 let (model, stub) = try await loaded()
133 stub.enqueue(.init(status: 200, json: ok, match: "cmd"))
134 let (file, hunk) = try hunk()
135 let target = try #require(hunk.lines.first { $0.newNumber == 12 && $0.kind == .context })
136 let draft = try #require(DiffCommentDraft(file: file, hunk: hunk, line: target))
137
138 let sent = await model.diffComment(draft, startLine: 11, body: "quote it")
139
140 #expect(sent)
141 let (argv, stdin) = try posted(stub)
142 #expect(argv == ["mr", "diff-comment", "krz/gitbay", "4", "--path", "main.go", "--line", "12",
143 "--start-line", "11", "--file", "-"])
144 #expect(stdin == "quote it")
145 }
146}