Commit 3e71822629
Verified · cmc
Layout: unified · split
gitbay/MRs/DiffCommentDraft.swift +11 −1
| @@ -32,7 +32,9 @@ nonisolated struct DiffCommentDraft: Sendable, Hashable, Identifiable { | |||
| 32 | init?(file: UnifiedDiff.File, hunk: UnifiedDiff.Hunk, line target: UnifiedDiff.Line) { | 32 | init?(file: UnifiedDiff.File, hunk: UnifiedDiff.Hunk, line target: UnifiedDiff.Line) { |
| 33 | let old = target.kind == .deletion | 33 | let old = target.kind == .deletion |
| 34 | guard let number = old ? target.oldNumber : target.newNumber else { return nil } | 34 | guard let number = old ? target.oldNumber : target.newNumber else { return nil } |
| 35 | self.path = old ? file.oldPath : file.newPath | 35 | // The server checks --path against the diff's file names, which |
| 36 | // list a rename under its new name only, on either side. | ||
| 37 | self.path = file.displayPath | ||
| 36 | self.old = old | 38 | self.old = old |
| 37 | self.line = number | 39 | self.line = number |
| 38 | var lines: [(Int, String)] = [] | 40 | var lines: [(Int, String)] = [] |
| @@ -60,4 +62,12 @@ nonisolated struct DiffCommentDraft: Sendable, Hashable, Identifiable { | |||
| 60 | let text = candidates.filter { $0.number >= startLine }.map(\.text) | 62 | let text = candidates.filter { $0.number >= startLine }.map(\.text) |
| 61 | return "```suggestion\n" + text.map { $0 + "\n" }.joined() + "```\n" | 63 | return "```suggestion\n" + text.map { $0 + "\n" }.joined() + "```\n" |
| 62 | } | 64 | } |
| 65 | |||
| 66 | /// The body after the range's start moves from `old` to `new`: an | ||
| 67 | /// untouched suggestion is rebuilt to cover the new range, since | ||
| 68 | /// sending the old one would replace the whole range with fewer | ||
| 69 | /// lines. Text the user changed is theirs and stays. | ||
| 70 | func rebuilt(_ text: String, from old: Int, to new: Int) -> String { | ||
| 71 | text == suggestion(from: old) ? suggestion(from: new) : text | ||
| 72 | } | ||
| 63 | } | 73 | } |
gitbay/MRs/Suggestion.swift +10
| @@ -33,4 +33,14 @@ nonisolated struct Suggestion: Decodable, Sendable, Hashable { | |||
| 33 | if lines.last == "" { lines.removeLast() } | 33 | if lines.last == "" { lines.removeLast() } |
| 34 | return lines | 34 | return lines |
| 35 | } | 35 | } |
| 36 | |||
| 37 | /// A comment's text without its ```suggestion block, which the thread | ||
| 38 | /// draws as replaced and proposed lines instead. | ||
| 39 | static func withoutBlock(_ body: String) -> String { | ||
| 40 | guard let start = body.range(of: "```suggestion") else { return body } | ||
| 41 | let afterOpen = body[start.upperBound...] | ||
| 42 | guard let close = afterOpen.range(of: "```") else { return body } | ||
| 43 | let rest = body[..<start.lowerBound] + body[close.upperBound...] | ||
| 44 | return rest.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| 45 | } | ||
| 36 | } | 46 | } |
gitbay/Views/MRs/DiffCommentSheet.swift +10 −2
| @@ -11,6 +11,7 @@ struct DiffCommentSheet: View { | |||
| 11 | 11 | ||
| 12 | @State private var startLine: Int | 12 | @State private var startLine: Int |
| 13 | @State private var text: String | 13 | @State private var text: String |
| 14 | @State private var sent = false | ||
| 14 | @Environment(\.dismiss) private var dismiss | 15 | @Environment(\.dismiss) private var dismiss |
| 15 | 16 | ||
| 16 | init(draft: DiffCommentDraft, model: MRDetailViewModel, suggesting: Bool) { | 17 | init(draft: DiffCommentDraft, model: MRDetailViewModel, suggesting: Bool) { |
| @@ -33,6 +34,9 @@ struct DiffCommentSheet: View { | |||
| 33 | Text("From line \(startLine)") | 34 | Text("From line \(startLine)") |
| 34 | .font(.gbSans(.subheadline)) | 35 | .font(.gbSans(.subheadline)) |
| 35 | } | 36 | } |
| 37 | .onChange(of: startLine) { old, new in | ||
| 38 | text = draft.rebuilt(text, from: old, to: new) | ||
| 39 | } | ||
| 36 | .accessibilityIdentifier("diff-comment-start") | 40 | .accessibilityIdentifier("diff-comment-start") |
| 37 | } | 41 | } |
| 38 | } | 42 | } |
| @@ -57,7 +61,9 @@ struct DiffCommentSheet: View { | |||
| 57 | Text("A suggestion replaces the lines in its range with what the block holds. An empty block deletes them.") | 61 | Text("A suggestion replaces the lines in its range with what the block holds. An empty block deletes them.") |
| 58 | } | 62 | } |
| 59 | } | 63 | } |
| 60 | if let error = model.actionError { | 64 | // Only this sheet's own send: an error left from an earlier |
| 65 | // action on the screen is not this comment's. | ||
| 66 | if sent, let error = model.actionError { | ||
| 61 | Section { GBNotice(error) } | 67 | Section { GBNotice(error) } |
| 62 | } | 68 | } |
| 63 | } | 69 | } |
| @@ -73,6 +79,7 @@ struct DiffCommentSheet: View { | |||
| 73 | ProgressView() | 79 | ProgressView() |
| 74 | } else { | 80 | } else { |
| 75 | Button("Send") { | 81 | Button("Send") { |
| 82 | sent = true | ||
| 76 | Task { | 83 | Task { |
| 77 | if await model.diffComment(draft, startLine: startLine, body: text) { | 84 | if await model.diffComment(draft, startLine: startLine, body: text) { |
| 78 | dismiss() | 85 | dismiss() |
| @@ -103,6 +110,7 @@ struct SuggestionView: View { | |||
| 103 | let suggestion: Suggestion | 110 | let suggestion: Suggestion |
| 104 | let thread: ReviewThread | 111 | let thread: ReviewThread |
| 105 | let model: MRDetailViewModel | 112 | let model: MRDetailViewModel |
| 113 | @ScaledMetric(relativeTo: .caption2) private var markerWidth: CGFloat = 14 | ||
| 106 | 114 | ||
| 107 | var body: some View { | 115 | var body: some View { |
| 108 | VStack(alignment: .leading, spacing: 6) { | 116 | VStack(alignment: .leading, spacing: 6) { |
| @@ -152,7 +160,7 @@ struct SuggestionView: View { | |||
| 152 | private func row(_ marker: String, _ text: String, _ markerColor: Color, _ background: Color) -> some View { | 160 | private func row(_ marker: String, _ text: String, _ markerColor: Color, _ background: Color) -> some View { |
| 153 | HStack(spacing: 0) { | 161 | HStack(spacing: 0) { |
| 154 | Text(marker) | 162 | Text(marker) |
| 155 | .frame(width: 14) | 163 | .frame(width: markerWidth) |
| 156 | .foregroundStyle(markerColor) | 164 | .foregroundStyle(markerColor) |
| 157 | Text(text.isEmpty ? " " : text) | 165 | Text(text.isEmpty ? " " : text) |
| 158 | Spacer(minLength: 0) | 166 | Spacer(minLength: 0) |
gitbay/Views/MRs/MRView.swift +4 −1
| @@ -706,7 +706,10 @@ struct ReviewThreadView: View { | |||
| 706 | .foregroundStyle(.tertiary) | 706 | .foregroundStyle(.tertiary) |
| 707 | } | 707 | } |
| 708 | // Review comments carry no format; the web renders them as Markdown. | 708 | // Review comments carry no format; the web renders them as Markdown. |
| 709 | FormattedBodyView(text: comment.body, repo: model.repoPath) | 709 | // The thread's first comment holds its suggestion, drawn above. |
| 710 | FormattedBodyView(text: thread.suggestion != nil && comment.id == thread.comments.first?.id | ||
| 711 | ? Suggestion.withoutBlock(comment.body) : comment.body, | ||
| 712 | repo: model.repoPath) | ||
| 710 | .font(.gbSans(.subheadline)) | 713 | .font(.gbSans(.subheadline)) |
| 711 | } | 714 | } |
| 712 | } | 715 | } |
gitbayTests/SuggestionTests.swift +41
| @@ -92,6 +92,47 @@ struct DiffCommentDraftTests { | |||
| 92 | } | 92 | } |
| 93 | } | 93 | } |
| 94 | 94 | ||
| 95 | struct SuggestionReviewFixTests { | ||
| 96 | |||
| 97 | /// The server checks --path against the diff's names, which list a | ||
| 98 | /// renamed file under its new name only, so both sides send it. | ||
| 99 | @Test func aRemovedLineInARenamedFileSendsTheNewPath() throws { | ||
| 100 | let diff = UnifiedDiff.parse(""" | ||
| 101 | diff --git a/old.go b/new.go | ||
| 102 | --- a/old.go | ||
| 103 | +++ b/new.go | ||
| 104 | @@ -1,2 +1,2 @@ | ||
| 105 | -a | ||
| 106 | +b | ||
| 107 | c | ||
| 108 | """) | ||
| 109 | let file = try #require(diff.files.first) | ||
| 110 | let hunk = try #require(file.hunks.first) | ||
| 111 | let removed = try #require(hunk.lines.first { $0.kind == .deletion }) | ||
| 112 | #expect(DiffCommentDraft(file: file, hunk: hunk, line: removed)?.path == "new.go") | ||
| 113 | } | ||
| 114 | |||
| 115 | /// Moving the start of the range rebuilds an untouched suggestion so it | ||
| 116 | /// covers the range, and leaves an edited one alone. | ||
| 117 | @Test func movingTheRangeStartRebuildsAnUntouchedSuggestion() throws { | ||
| 118 | let (file, hunk) = try hunk() | ||
| 119 | let target = try #require(hunk.lines.first { $0.newNumber == 12 && $0.kind == .context }) | ||
| 120 | let draft = try #require(DiffCommentDraft(file: file, hunk: hunk, line: target)) | ||
| 121 | let untouched = draft.suggestion(from: 12) | ||
| 122 | |||
| 123 | #expect(draft.rebuilt(untouched, from: 12, to: 11) == draft.suggestion(from: 11)) | ||
| 124 | #expect(draft.rebuilt("my own words", from: 12, to: 11) == "my own words") | ||
| 125 | } | ||
| 126 | |||
| 127 | /// The thread draws the suggestion as replaced and proposed lines, so | ||
| 128 | /// the comment shows the rest of its text without the block. | ||
| 129 | @Test func theBlockIsLeftOutOfTheCommentShownBesideTheSuggestion() { | ||
| 130 | #expect(Suggestion.withoutBlock("Say who.\n```suggestion\n return x\n```\n") == "Say who.") | ||
| 131 | #expect(Suggestion.withoutBlock("```suggestion\n```\n") == "") | ||
| 132 | #expect(Suggestion.withoutBlock("no block here") == "no block here") | ||
| 133 | } | ||
| 134 | } | ||
| 135 | |||
| 95 | @MainActor | 136 | @MainActor |
| 96 | struct SuggestionViewModelTests { | 137 | struct SuggestionViewModelTests { |
| 97 | 138 | ||