From cf65949d971a96d2199687648abea76230e2e633 Mon Sep 17 00:00:00 2001 From: "Jiwei,Yuan" Date: Wed, 12 Aug 2026 23:36:15 +0100 Subject: [PATCH] feat(git): expand a hunk boundary from the file, GitHub Desktop's rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A three-line-context patch never carries the lines around its hunks, so the band standing in for that gap had no control at all. The fold now takes the file's own text and offers what GitHub Desktop offers: the first hunk reads upward only, a gap within one 20-line step opens at once, and a longer one gets both ends — stacked in the gutter cell, downward reveal on top. PR files read their text through `contents_url` at the head, one request per opened file, so nothing needs fetching or checking out; without it the band stays inert, the way GitHub Desktop draws a gap it cannot expand. --- Shared/Sources/TermioShared/DiffModel.swift | 85 +++++++++++++++++-- Sources/termio/Git/DiffDocument.swift | 12 +-- Sources/termio/Git/DiffGutterRulerView.swift | 49 +++++++++-- Sources/termio/Git/PRFilesDiffView.swift | 42 ++++++++- .../termio/Issues/GitHubIssueProvider.swift | 21 ++++- Sources/termio/Issues/IssueCache.swift | 4 + Sources/termio/Issues/IssueModels.swift | 4 + Sources/termio/Issues/IssuesPanelModel.swift | 31 +++++-- Sources/termio/Issues/IssuesView.swift | 1 + Tests/termioTests/DiffModelTests.swift | 80 +++++++++++++++++ 10 files changed, 300 insertions(+), 29 deletions(-) diff --git a/Shared/Sources/TermioShared/DiffModel.swift b/Shared/Sources/TermioShared/DiffModel.swift index aa0bd65c..0058a34b 100644 --- a/Shared/Sources/TermioShared/DiffModel.swift +++ b/Shared/Sources/TermioShared/DiffModel.swift @@ -62,6 +62,41 @@ public struct DiffBandControls: OptionSet, Sendable, Equatable { public init(rawValue: Int) { self.rawValue = rawValue } public static let up = DiffBandControls(rawValue: 1 << 0) public static let down = DiffBandControls(rawValue: 1 << 1) + /// One control that opens the whole run at once, for a gap no longer than a single + /// step — GitHub Desktop's "Expand All", where two ends would each swallow the gap. + public static let all = DiffBandControls(rawValue: 1 << 2) +} + +/// The file's own text for lines a fixed-context patch never carried, keyed by new-side +/// line number. A `git diff` termio runs itself asks for the whole file as context, so its +/// gaps are already in the rows; GitHub's PR `patch` is fixed at three lines, and a hunk +/// boundary there can only be opened by reading the file. Empty means the surface has no +/// file to read, and the band draws inert — GitHub Desktop's placeholder state. +public struct DiffGapText: Sendable, Equatable { + private let lines: [Int: String] + + public static let unavailable = DiffGapText(lines: [:]) + + public init(lines: [Int: String]) { self.lines = lines } + + /// The new-side text of a whole file, `first` being the line number of `text`'s first + /// line (1 for a complete file). + public init(fileLines text: [String], startingAt first: Int = 1) { + var lines: [Int: String] = [:] + lines.reserveCapacity(text.count) + for (offset, line) in text.enumerated() { lines[first + offset] = line } + self.lines = lines + } + + public var isEmpty: Bool { lines.isEmpty } + + public func line(_ number: Int) -> String? { lines[number] } + + /// Whether every line of `range` is on hand — a partial read must not offer a control + /// that would splice a hole into the file. + public func covers(_ range: ClosedRange) -> Bool { + !lines.isEmpty && range.allSatisfy { lines[$0] != nil } + } } /// How much of each folded run the reader has revealed, keyed by the run's anchor line id. @@ -142,12 +177,18 @@ public enum DiffParser { return rows } + /// The id of a line spliced in from the file instead of parsed out of the patch. + /// Negative, so it can never collide with a parsed row's id, and stable per line + /// number, so re-folding keeps the syntax pass and the selection anchored. + public static func gapLineID(forNewLine number: Int) -> Int { -(number + 1) } + /// Folds parsed lines into the display list: hunk plumbing disappears (its gap /// becomes a band), unchanged runs longer than a handful of lines collapse to a /// band keeping 3 lines of context on the side(s) facing a change, and ids in /// `expanded` splice their hidden lines back in. public static func displayItems(lines rows: [DiffLine], - expansion: DiffExpansion) -> [DiffItem] { + expansion: DiffExpansion, + gapText: DiffGapText = .unavailable) -> [DiffItem] { var items: [DiffItem] = [] var run: [DiffLine] = [] var sawChange = false @@ -190,10 +231,44 @@ public enum DiffParser { case .hunk: flush(isLast: false) if let start = row.newLine, start > lastNewLine + 1 { - items.append(.band( - id: row.id, lines: (lastNewLine + 1)...(start - 1), - controls: [], heading: hunkHeading(row.text) - )) + let gap = (lastNewLine + 1)...(start - 1) + let heading = hunkHeading(row.text) + // The gap's own lines are never in the patch, so the reveal splices them + // from the file. Without the file the band stays inert, the way GitHub + // Desktop draws a placeholder where it cannot expand. + guard gapText.covers(gap) else { + items.append(.band(id: row.id, lines: gap, controls: [], heading: heading)) + break + } + // Unchanged through the gap, so both sides advance together: this hunk's + // own numbers give the offset back to the old side. + let oldOffset = (row.oldLine ?? start) - start + let revealed = expansion.revealed(row.id, of: gap.count) + func spliced(_ numbers: some Sequence) -> [DiffItem] { + numbers.compactMap { number in + gapText.line(number).map { text in + .line(DiffLine(id: gapLineID(forNewLine: number), kind: .context, + text: text, oldLine: number + oldOffset, + newLine: number)) + } + } + } + items += spliced(gap.prefix(revealed.head)) + let hidden = gap.dropFirst(revealed.head).dropLast(revealed.tail) + if let first = hidden.first, let last = hidden.last { + // GitHub Desktop's `getHunkExpansionType`: the file's first hunk can + // only be read upward, a gap within one step opens in a single jump, + // and anything longer offers both ends. + var controls: DiffBandControls = [.up, .down] + if items.isEmpty { + controls = .up + } else if hidden.count <= DiffExpansion.step { + controls = .all + } + items.append(.band(id: row.id, lines: first...last, + controls: controls, heading: heading)) + } + items += spliced(gap.suffix(revealed.tail)) } case .context: run.append(row) diff --git a/Sources/termio/Git/DiffDocument.swift b/Sources/termio/Git/DiffDocument.swift index c992fd12..8a30db0b 100644 --- a/Sources/termio/Git/DiffDocument.swift +++ b/Sources/termio/Git/DiffDocument.swift @@ -79,9 +79,10 @@ final class DiffDocument { /// than a handful of lines collapse to a band keeping 3 lines of context on the /// side(s) that face a change, and whatever `expansion` has revealed is spliced back in. static func build(rows: [DiffRow], expansion: DiffExpansion, palette: DiffPalette, - codeFont: NSFont, lineSpacing: CGFloat) -> DiffDocument { - build(items: displayItems(rows: rows, expansion: expansion), allRows: rows, - palette: palette, codeFont: codeFont, lineSpacing: lineSpacing) + codeFont: NSFont, lineSpacing: CGFloat, + gapText: DiffGapText = .unavailable) -> DiffDocument { + build(items: displayItems(rows: rows, expansion: expansion, gapText: gapText), + allRows: rows, palette: palette, codeFont: codeFont, lineSpacing: lineSpacing) } /// The shared assembly: lays the display items down as one attributed string with @@ -233,8 +234,9 @@ final class DiffDocument { /// skipped lines are, not how many there are: a line range tells the reader what they /// are jumping over, while "137 unchanged lines" only describes the widget. git's /// section heading rides along when the gap came from a hunk boundary. - private static func displayItems(rows: [DiffRow], expansion: DiffExpansion) -> [DisplayItem] { - DiffParser.displayItems(lines: rows, expansion: expansion).map { item in + private static func displayItems(rows: [DiffRow], expansion: DiffExpansion, + gapText: DiffGapText) -> [DisplayItem] { + DiffParser.displayItems(lines: rows, expansion: expansion, gapText: gapText).map { item in switch item { case .line(let row): return .line(row) diff --git a/Sources/termio/Git/DiffGutterRulerView.swift b/Sources/termio/Git/DiffGutterRulerView.swift index 0415d589..bcb65453 100644 --- a/Sources/termio/Git/DiffGutterRulerView.swift +++ b/Sources/termio/Git/DiffGutterRulerView.swift @@ -205,20 +205,24 @@ final class DiffGutterRulerView: NSRulerView { guard !controls.isEmpty else { return } var directions: [DiffBandDirection] = [] + if controls.contains(.all) { directions.append(.all) } if controls.contains(.down) { directions.append(.down) } if controls.contains(.up) { directions.append(.up) } - // Side by side rather than github.com's stacked pair: its expander is double - // height to fit two 16pt icons, and a diff row here is barely taller than its text. + // Stacked, GitHub Desktop's shape: one gutter cell split in half, the downward + // reveal on top and the upward one below, each reading toward the code it pulls + // from. A lone control (a first hunk, or a gap short enough to open at once) takes + // the whole cell. let padding = DiffDocument.bandPadding - let cell = (ruleThickness - Self.leadingPad - Self.trailingPad) - / CGFloat(directions.count) - var x = Self.leadingPad - for direction in directions { - let hit = NSRect(x: x, y: y - padding, width: cell, height: height + padding * 2) + let width = ruleThickness - Self.leadingPad - Self.trailingPad + let full = NSRect(x: Self.leadingPad, y: y - padding, + width: width, height: height + padding * 2) + let slice = full.height / CGFloat(directions.count) + for (index, direction) in directions.enumerated() { + let hit = NSRect(x: full.minX, y: full.minY + slice * CGFloat(index), + width: full.width, height: slice) drawRevealIcon(direction, in: hit, ink: numberColor) buttonHits.append(ButtonHit(rect: hit, anchor: line.rowId, direction: direction)) - x += cell } } @@ -228,6 +232,35 @@ final class DiffGutterRulerView: NSRulerView { let gap: CGFloat = 2.5 let dotWidth: CGFloat = 1.2 let originX = (rect.midX - width / 2).rounded() + // "Open the whole gap" reads as both arrows meeting on the dots — GitHub Desktop's + // fold glyph — rather than either single direction, neither of which is what a + // click does here. + if direction == .all { + let dotsY = (rect.midY - dotWidth / 2).rounded() + ink.setFill() + var dotX = originX + while dotX < originX + width { + NSRect(x: dotX, y: dotsY, width: dotWidth, height: dotWidth).fill() + dotX += dotWidth * 2 + } + ink.setStroke() + for pointsDown in [true, false] { + let tipY = pointsDown ? dotsY - gap : dotsY + dotWidth + gap + let tailY = pointsDown ? tipY - arrowHeight : tipY + arrowHeight + let barbY = pointsDown ? tipY - 2.4 : tipY + 2.4 + let arrow = NSBezierPath() + arrow.lineWidth = 1.2 + arrow.lineCapStyle = .round + arrow.lineJoinStyle = .round + arrow.move(to: NSPoint(x: rect.midX, y: tailY)) + arrow.line(to: NSPoint(x: rect.midX, y: tipY)) + arrow.move(to: NSPoint(x: originX + 1, y: barbY)) + arrow.line(to: NSPoint(x: rect.midX, y: tipY)) + arrow.line(to: NSPoint(x: originX + width - 1, y: barbY)) + arrow.stroke() + } + return + } // The arrow points the way the reveal walks — the same reading github.com and // GitHub Desktop use — and the dots trail behind it, on the side still hidden. // A vertical NSRulerView is flipped, so `minY` is the block's *top* edge: an diff --git a/Sources/termio/Git/PRFilesDiffView.swift b/Sources/termio/Git/PRFilesDiffView.swift index 3c1d8633..2839c7a5 100644 --- a/Sources/termio/Git/PRFilesDiffView.swift +++ b/Sources/termio/Git/PRFilesDiffView.swift @@ -12,6 +12,8 @@ struct PRFilesSplitView: View { let files: [GitChange] /// Each file's inline unified-diff `patch` from the GitHub API, keyed by path. let patches: [String: String] + /// Reads a file at the PR head, for expanding a hunk boundary the patch left out. + let fileText: (String) async -> String? /// Only used to resolve each file's language for syntax coloring. let repoRoot: String @ObservedObject var settings: AppSettings @@ -73,7 +75,7 @@ struct PRFilesSplitView: View { /// you see *all* the files, not one at a time, and selection / ⌘F run across the whole set. private var narrow: some View { StackedDiffBody( - files: files, patches: patches, repoRoot: repoRoot, + files: files, patches: patches, fileText: fileText, repoRoot: repoRoot, settings: settings, onClose: onClose ) } @@ -112,6 +114,7 @@ struct PRFilesSplitView: View { PRFileDiffBody( change: change, patch: patches[change.path], + fileText: fileText, repoRoot: repoRoot, settings: settings, onClose: onClose, @@ -149,6 +152,10 @@ struct PRFilesSplitView: View { private struct PRFileDiffBody: View { let change: GitChange let patch: String? + /// Reads the file at the PR head. GitHub's patch carries three lines of context, so a + /// hunk boundary can only be opened from the file itself; until this lands the bands + /// draw inert, the way GitHub Desktop draws a gap it cannot expand. + let fileText: (String) async -> String? let repoRoot: String @ObservedObject var settings: AppSettings let onClose: () -> Void @@ -157,6 +164,7 @@ private struct PRFileDiffBody: View { @Environment(\.colorScheme) private var colorScheme @State private var rows: [DiffRow] = [] + @State private var gapText: DiffGapText = .unavailable @State private var document: DiffDocument? @State private var styledLines: [Int: NSAttributedString] = [:] @State private var expansion = DiffExpansion() @@ -258,6 +266,7 @@ private struct PRFileDiffBody: View { rows = parsed rebuildDocument() isLoading = false + await loadGapText() await buildStyledLines(parsed) } @@ -269,7 +278,17 @@ private struct PRFileDiffBody: View { : DiffDocument.build(rows: rows, expansion: expansion, palette: settings.diffPalette(for: colorScheme), codeFont: settings.resolvedTerminalFont(), - lineSpacing: settings.codeLineSpacing(for: settings.resolvedTerminalFont())) + lineSpacing: settings.codeLineSpacing(for: settings.resolvedTerminalFont()), + gapText: gapText) + } + + + /// One read per opened file, behind the diff so the patch renders immediately. A file + /// that comes back empty (binary, too large, no access) leaves `gapText` unavailable. + private func loadGapText() async { + guard gapText.isEmpty, let text = await fileText(change.path) else { return } + gapText = DiffGapText(fileLines: text.components(separatedBy: "\n")) + rebuildDocument() } private func buildStyledLines(_ rows: [DiffRow]) async { @@ -299,6 +318,8 @@ private struct PRFileDiffBody: View { private struct StackedDiffBody: View { let files: [GitChange] let patches: [String: String] + /// Reads a file at the PR head, for expanding a hunk boundary the patch left out. + let fileText: (String) async -> String? let repoRoot: String @ObservedObject var settings: AppSettings let onClose: () -> Void @@ -313,6 +334,7 @@ private struct StackedDiffBody: View { FileDiffCard( change: change, patch: patches[change.path], + fileText: fileText, repoRoot: repoRoot, settings: settings, collapsed: collapsed.contains(change.path), @@ -337,6 +359,7 @@ private struct StackedDiffBody: View { private struct FileDiffCard: View { let change: GitChange let patch: String? + let fileText: (String) async -> String? let repoRoot: String @ObservedObject var settings: AppSettings let collapsed: Bool @@ -345,6 +368,7 @@ private struct FileDiffCard: View { @Environment(\.colorScheme) private var colorScheme @State private var rows: [DiffRow] = [] + @State private var gapText: DiffGapText = .unavailable @State private var document: DiffDocument? @State private var styledLines: [Int: NSAttributedString] = [:] @State private var expansion = DiffExpansion() @@ -453,14 +477,24 @@ private struct FileDiffCard: View { : DiffDocument.build(rows: parsed, expansion: expansion, palette: settings.diffPalette(for: colorScheme), codeFont: settings.resolvedTerminalFont(), - lineSpacing: settings.codeLineSpacing(for: settings.resolvedTerminalFont())) + lineSpacing: settings.codeLineSpacing(for: settings.resolvedTerminalFont()), + gapText: gapText) + await loadGapText() await buildStyled(parsed) } private func rebuildDocument() { document = DiffDocument.build(rows: rows, expansion: expansion, palette: settings.diffPalette(for: colorScheme), codeFont: settings.resolvedTerminalFont(), - lineSpacing: settings.codeLineSpacing(for: settings.resolvedTerminalFont())) + lineSpacing: settings.codeLineSpacing(for: settings.resolvedTerminalFont()), + gapText: gapText) + } + + /// One read per opened card, behind the diff so the patch renders immediately. + private func loadGapText() async { + guard gapText.isEmpty, let text = await fileText(change.path) else { return } + gapText = DiffGapText(fileLines: text.components(separatedBy: "\n")) + rebuildDocument() } private func buildStyled(_ parsed: [DiffRow]) async { diff --git a/Sources/termio/Issues/GitHubIssueProvider.swift b/Sources/termio/Issues/GitHubIssueProvider.swift index 382b0ac2..a4e11477 100644 --- a/Sources/termio/Issues/GitHubIssueProvider.swift +++ b/Sources/termio/Issues/GitHubIssueProvider.swift @@ -106,6 +106,7 @@ struct GitHubIssueProvider: IssueProvider { let deletions: Int let previousFilename: String? let patch: String? + let contentsUrl: String? } let raw: [RawFile] = try await get( URL(string: "https://api.github.com/repos/\(container.id)/pulls/\(number)/files?per_page=100")!) @@ -124,10 +125,28 @@ struct GitHubIssueProvider: IssueProvider { change.originalPath = file.previousFilename // No `patch` on a binary file; GitHub also drops it past its inline-size cap. change.isBinary = file.patch == nil && file.additions == 0 && file.deletions == 0 - return PullRequestFile(change: change, patch: file.patch) + return PullRequestFile(change: change, patch: file.patch, + contentsURL: file.contentsUrl.flatMap(URL.init(string:))) } } + /// One PR file's text at the PR head, for expanding the context its patch left out. + /// `contents_url` is already pinned to the head sha, and the raw media type returns the + /// file itself rather than base64 in JSON — so this stays one request with no decode. + /// Binary or over-large files come back as something other than UTF-8 text and are + /// reported as missing, leaving the diff's bands inert. + func fileText(at url: URL) async throws -> String? { + var request = URLRequest(url: url, timeoutInterval: 15) + request.setValue("Bearer \(token)", forHTTPHeaderField: "Authorization") + request.setValue("application/vnd.github.raw", forHTTPHeaderField: "Accept") + request.setValue("2022-11-28", forHTTPHeaderField: "X-GitHub-Api-Version") + let (data, response) = try await URLSession.shared.data(for: request) + if let http = response as? HTTPURLResponse, !(200...299).contains(http.statusCode) { + throw APIError.status(http.statusCode) + } + return String(data: data, encoding: .utf8) + } + /// The connected account's login, fetched once per app run — the `assignee` /// filter needs a username, not a token. private func login() async throws -> String? { diff --git a/Sources/termio/Issues/IssueCache.swift b/Sources/termio/Issues/IssueCache.swift index 1e076901..45ef6070 100644 --- a/Sources/termio/Issues/IssueCache.swift +++ b/Sources/termio/Issues/IssueCache.swift @@ -30,6 +30,9 @@ final class IssueCache { var detail: IssueDetail var prFiles: [GitChange] var prFilePatches: [String: String] + /// Each file's `contents_url` at the PR head, so a reopened-from-cache Files tab can + /// still read the file to expand a hunk boundary. + var prFileContentURLs: [String: URL] /// The conversation is cached the instant it lands — *before* a PR's heavy `/files` /// response — so even opening a PR and immediately switching away warms the cache. This /// marks whether that file list has since folded in: `false` means "files still pending", @@ -45,6 +48,7 @@ final class IssueCache { detail == other.detail && prFiles == other.prFiles && prFilePatches == other.prFilePatches + && prFileContentURLs == other.prFileContentURLs } } diff --git a/Sources/termio/Issues/IssueModels.swift b/Sources/termio/Issues/IssueModels.swift index 5f7b5ace..8b922be6 100644 --- a/Sources/termio/Issues/IssueModels.swift +++ b/Sources/termio/Issues/IssueModels.swift @@ -144,6 +144,10 @@ struct IssueDetail: Equatable, Sendable { struct PullRequestFile: Sendable { let change: GitChange let patch: String? + /// GitHub's `contents_url` for this file, already pinned to the PR head. The patch + /// carries three lines of context, so expanding a hunk boundary means reading the file + /// itself — this is where from, without fetching the PR's refs. + let contentsURL: URL? } // MARK: - Provider protocol diff --git a/Sources/termio/Issues/IssuesPanelModel.swift b/Sources/termio/Issues/IssuesPanelModel.swift index d16e8346..6cc6d82a 100644 --- a/Sources/termio/Issues/IssuesPanelModel.swift +++ b/Sources/termio/Issues/IssuesPanelModel.swift @@ -312,6 +312,7 @@ final class IssuesPanelModel: ObservableObject { detailError = nil prFiles = [] prFilePatches = [:] + prFileContentURLs = [:] prFilesLoading = false do { let conversation = try await provider.detail(item.number, in: container) @@ -320,7 +321,7 @@ final class IssuesPanelModel: ObservableObject { // Cache the conversation the moment it lands — before a PR's heavy file list — so a // switch-away still warms the cache. `filesLoaded: false` marks the pending files. cache?.store( - .init(detail: conversation, prFiles: [], prFilePatches: [:], + .init(detail: conversation, prFiles: [], prFilePatches: [:], prFileContentURLs: [:], filesLoaded: item.kind != .pullRequest, fetchedAt: Date()), container, item.number) if item.kind == .pullRequest { @@ -345,8 +346,9 @@ final class IssuesPanelModel: ObservableObject { let mapped = prFileMapping(loadedFiles) prFiles = mapped.changes prFilePatches = mapped.patches + prFileContentURLs = mapped.contentURLs cache?.store( - .init(detail: conversation, prFiles: mapped.changes, prFilePatches: mapped.patches, + .init(detail: conversation, prFiles: mapped.changes, prFilePatches: mapped.patches, prFileContentURLs: mapped.contentURLs, filesLoaded: true, fetchedAt: Date()), container, item.number) } catch { @@ -355,17 +357,30 @@ final class IssuesPanelModel: ObservableObject { prFilesLoading = false } + /// Reads one PR file at the head, for a diff pane that wants to expand past the three + /// lines of context GitHub's patch carries. Nil whenever that cannot be done — no + /// provider, no `contents_url`, a binary blob, or the request failing — and the pane + /// then leaves its bands inert rather than offering a control that would do nothing. + func prFileText(_ path: String) async -> String? { + guard let provider, let url = prFileContentURLs[path] else { return nil } + return try? await provider.fileText(at: url) + } + /// Maps GitHub's `/files` payload into the git pane's `GitChange` rows plus each file's inline /// unified-diff patch keyed by path — what the Files tab renders with no further network or git /// work. Absent patch keys are files GitHub gave none for (binary / diff too large to inline). private func prFileMapping( _ files: [PullRequestFile] - ) -> (changes: [GitChange], patches: [String: String]) { + ) -> (changes: [GitChange], patches: [String: String], contentURLs: [String: URL]) { let patches = Dictionary( uniqueKeysWithValues: files.compactMap { file in file.patch.map { (file.change.path, $0) } }) - return (files.map(\.change), patches) + let contentURLs = Dictionary( + uniqueKeysWithValues: files.compactMap { file in + file.contentsURL.map { (file.change.path, $0) } + }) + return (files.map(\.change), patches, contentURLs) } /// Publishes a fetched entry into the pane's detail state. @@ -373,6 +388,7 @@ final class IssuesPanelModel: ObservableObject { detail = entry.detail prFiles = entry.prFiles prFilePatches = entry.prFilePatches + prFileContentURLs = entry.prFileContentURLs prFilesLoading = false detailError = nil } @@ -391,12 +407,12 @@ final class IssuesPanelModel: ObservableObject { let (detail, loadedFiles) = try await (loadedDetail, files) let mapped = prFileMapping(loadedFiles) return .init( - detail: detail, prFiles: mapped.changes, prFilePatches: mapped.patches, + detail: detail, prFiles: mapped.changes, prFilePatches: mapped.patches, prFileContentURLs: mapped.contentURLs, filesLoaded: true, fetchedAt: Date()) } else { let detail = try await provider.detail(item.number, in: container) return .init( - detail: detail, prFiles: [], prFilePatches: [:], + detail: detail, prFiles: [], prFilePatches: [:], prFileContentURLs: [:], filesLoaded: true, fetchedAt: Date()) } } @@ -408,6 +424,9 @@ final class IssuesPanelModel: ObservableObject { /// Each file's inline unified-diff patch from the API, keyed by path — what the Files /// tab renders. Absent keys are files GitHub gave no patch for (binary / too large). @Published private(set) var prFilePatches: [String: String] = [:] + /// Each PR file's `contents_url`, pinned to the head — the Files tab reads the file + /// through it to expand the context GitHub's three-line patch left out. + @Published private(set) var prFileContentURLs: [String: URL] = [:] /// The PR's file list is still in flight *after* the conversation has already rendered — /// the two are fetched separately so the (small, fast) conversation isn't gated behind the diff --git a/Sources/termio/Issues/IssuesView.swift b/Sources/termio/Issues/IssuesView.swift index 28dee0b3..c3f068b8 100644 --- a/Sources/termio/Issues/IssuesView.swift +++ b/Sources/termio/Issues/IssuesView.swift @@ -754,6 +754,7 @@ struct IssueDetailView: View { // diff on the right, rendered from the API's inline patches (no fetch, no git). PRFilesSplitView( files: model.prFiles, patches: model.prFilePatches, + fileText: { await model.prFileText($0) }, repoRoot: model.repoRoot, settings: settings, onClose: onBack ) } else if model.prFilesLoading || (model.detail == nil && model.detailError == nil) { diff --git a/Tests/termioTests/DiffModelTests.swift b/Tests/termioTests/DiffModelTests.swift index 6e23744f..fb0fcfbb 100644 --- a/Tests/termioTests/DiffModelTests.swift +++ b/Tests/termioTests/DiffModelTests.swift @@ -137,6 +137,86 @@ final class DiffFoldTests: XCTestCase { XCTAssertEqual(bands.first?.0, 2...39) // the gap between the two hunks XCTAssertEqual(bands.first?.1, [], "the hidden lines were never sent, so nothing reveals them") } + + /// GitHub Desktop's `getHunkExpansionType`, which termio's gutter copies: the file's + /// first hunk can only be read upward, a gap within one step opens in a single jump, + /// and anything longer offers both ends. All three need the file on hand. + func testGapControlsFollowGitHubDesktopsRules() { + let firstHunkDiff = """ + @@ -92,2 +92,2 @@ + -ninetytwo + +NINETYTWO + """ + let file = DiffGapText(fileLines: (1...200).map { "line \($0)" }) + XCTAssertEqual(controls(of: firstHunkDiff, gapText: file), [.up], + "nothing is rendered above a first hunk to read downward from") + + let shortGap = """ + @@ -1,2 +1,2 @@ + -one + +ONE + @@ -12,2 +12,2 @@ + -twelve + +TWELVE + """ + XCTAssertEqual(controls(of: shortGap, gapText: file), [.all], + "a gap no longer than one step opens at once") + + let longGap = """ + @@ -1,2 +1,2 @@ + -one + +ONE + @@ -80,2 +80,2 @@ + -eighty + +EIGHTY + """ + XCTAssertEqual(controls(of: longGap, gapText: file), [.up, .down]) + XCTAssertEqual(controls(of: longGap, gapText: .unavailable), [], + "without the file there is nothing to splice, so the band stays inert") + } + + func testRevealingAGapSplicesTheFilesOwnLines() { + let diff = """ + @@ -1,2 +1,2 @@ + -one + +ONE + @@ -80,2 +80,2 @@ + -eighty + +EIGHTY + """ + let rows = DiffParser.lines(from: diff) + let file = DiffGapText(fileLines: (1...200).map { "line \($0)" }) + // The gap is 2…79; the band anchors on the second hunk row. + guard let anchor = rows.last(where: { $0.kind == .hunk })?.id else { + return XCTFail("the fixture has two hunks") + } + var expansion = DiffExpansion() + expansion.reveal(anchor, .down) + let items = DiffParser.displayItems(lines: rows, expansion: expansion, gapText: file) + let spliced = items.compactMap { item -> DiffLine? in + if case .line(let line) = item, line.id < 0 { return line } + return nil + } + XCTAssertEqual(spliced.count, DiffExpansion.step) + XCTAssertEqual(spliced.first?.newLine, 2, "revealing downward starts at the gap's top") + XCTAssertEqual(spliced.first?.text, "line 2") + XCTAssertEqual(spliced.first?.oldLine, 2, "unchanged lines carry the same number on both sides") + XCTAssertEqual(spliced.last?.newLine, 21) + let bands = items.compactMap { item -> ClosedRange? in + if case .band(_, let range, _, _) = item { return range } + return nil + } + XCTAssertEqual(bands, [22...79], "the band keeps what is still hidden") + } + + private func controls(of diff: String, gapText: DiffGapText) -> DiffBandControls? { + let items = DiffParser.displayItems(lines: DiffParser.lines(from: diff), + expansion: DiffExpansion(), gapText: gapText) + for item in items { + if case .band(_, _, let controls, _) = item { return controls } + } + return nil + } } final class DiffIntralineSpanTests: XCTestCase {