From ccec322f8eac4d14638f5abdae7dae7abc95eb7e Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 21:42:38 -0500 Subject: refactor: share the email body diff splitter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit segmentMessageBody and its helpers were private to ThreadViewModel, reachable from tests only through a segmentMessageBodyForTesting shim. Patchset review needs the same splitting, because sr.ht's Patch type carries no diff — the diff only exists inside the email body — so this has to be shared rather than duplicated. Moved to InboxThreadUtilities. The shim is gone; the existing test calls the real function directly now. Also adds the Patchset model layer that the coming views build on. --- Hutch/Models/Patchset.swift | 132 +++++++++++++++++++++++++++ Hutch/Views/Inbox/InboxThreadUtilities.swift | 88 ++++++++++++++++++ Hutch/Views/Inbox/ThreadViewModel.swift | 91 +----------------- HutchTests/InboxViewModelTests.swift | 2 +- 4 files changed, 223 insertions(+), 90 deletions(-) create mode 100644 Hutch/Models/Patchset.swift diff --git a/Hutch/Models/Patchset.swift b/Hutch/Models/Patchset.swift new file mode 100644 index 0000000..330bbbc --- /dev/null +++ b/Hutch/Models/Patchset.swift @@ -0,0 +1,132 @@ +import Foundation + +/// Review state of a patchset on lists.sr.ht. +enum PatchsetStatus: String, Codable, Sendable, CaseIterable { + case unknown = "UNKNOWN" + case proposed = "PROPOSED" + case needsRevision = "NEEDS_REVISION" + case superseded = "SUPERSEDED" + case approved = "APPROVED" + case rejected = "REJECTED" + case applied = "APPLIED" + + var displayName: String { + switch self { + case .unknown: "Unknown" + case .proposed: "Proposed" + case .needsRevision: "Needs Revision" + case .superseded: "Superseded" + case .approved: "Approved" + case .rejected: "Rejected" + case .applied: "Applied" + } + } + + var systemImage: String { + switch self { + case .unknown: "questionmark.circle" + case .proposed: "paperplane" + case .needsRevision: "exclamationmark.arrow.circlepath" + case .superseded: "arrow.triangle.branch" + case .approved: "checkmark.seal" + case .rejected: "xmark.circle" + case .applied: "checkmark.circle.fill" + } + } + + /// Whether the patchset is still awaiting a decision. + var isOpen: Bool { + switch self { + case .unknown, .proposed, .needsRevision: true + case .superseded, .approved, .rejected, .applied: false + } + } + + /// Statuses a reviewer can set directly. + /// + /// `unknown` is a sentinel for patchsets sr.ht could not classify, and + /// `superseded` is set by the server when a later version arrives, so neither + /// is offered as a choice. + static var assignable: [PatchsetStatus] { + [.proposed, .needsRevision, .approved, .rejected, .applied] + } +} + +/// A patchset as it appears in a mailing list listing, derived from the thread's +/// root email rather than a dedicated patchsets query — `MailingList` exposes no +/// such field. +struct PatchsetSummary: Identifiable, Hashable, Sendable { + let id: Int + let subject: String + let version: Int + let prefix: String? + let status: PatchsetStatus + + /// The `[PATCH v2]`-style prefix sr.ht parsed from the subject, if any. + var versionLabel: String? { + guard version > 1 else { return nil } + return "v\(version)" + } +} + +/// One email within a patchset: either the cover letter or a single patch. +struct PatchsetEmail: Identifiable, Hashable, Sendable { + let id: Int + let subject: String + let date: Date? + let sender: Entity + /// Split into commit message and diff blocks for rendering. + let contentBlocks: [InboxMessageContentBlock] + /// Position within the series, from the `[PATCH 2/5]` prefix. + let index: Int? + let count: Int? + + var seriesLabel: String? { + guard let index, let count, count > 1 else { return nil } + return "\(index)/\(count)" + } +} + +/// A build or check reported against a patchset. +struct PatchsetToolResult: Identifiable, Hashable, Sendable { + let id: Int + let icon: PatchsetToolIcon + let details: String +} + +enum PatchsetToolIcon: String, Codable, Sendable { + case pending = "PENDING" + case waiting = "WAITING" + case success = "SUCCESS" + case failed = "FAILED" + case cancelled = "CANCELLED" + + var systemImage: String { + switch self { + case .pending, .waiting: "clock" + case .success: "checkmark.circle.fill" + case .failed: "xmark.circle.fill" + case .cancelled: "minus.circle" + } + } +} + +/// A patchset with its cover letter, patches, and review context. +struct PatchsetDetail: Sendable { + let id: Int + let created: Date + let updated: Date + let subject: String + let version: Int + let prefix: String? + let status: PatchsetStatus + let submitter: Entity + let coverLetter: PatchsetEmail? + let patches: [PatchsetEmail] + /// Set when a newer version of this series exists. + let supersededBy: Int? + /// Set when this series revises an earlier one. + let supersedes: Int? + let tools: [PatchsetToolResult] + let mbox: URL? +} diff --git a/Hutch/Views/Inbox/InboxThreadUtilities.swift b/Hutch/Views/Inbox/InboxThreadUtilities.swift index 1dd88a3..6958b50 100644 --- a/Hutch/Views/Inbox/InboxThreadUtilities.swift +++ b/Hutch/Views/Inbox/InboxThreadUtilities.swift @@ -8,4 +8,92 @@ enum InboxThreadUtilities { } return nil } + + /// Splits an email body into its commit message and diff, so patch mail can be + /// rendered as prose plus a diff rather than one undifferentiated blob. + /// + /// Shared by the inbox thread view and patchset review: sr.ht's `Patch` type + /// carries no diff, so the diff has to be recovered from the email body. + nonisolated static func segmentMessageBody(_ body: String, isPatch: Bool) -> [InboxMessageContentBlock] { + guard isPatch else { + let trimmedBody = body.trimmingCharacters(in: .whitespacesAndNewlines) + return trimmedBody.isEmpty ? [] : [.plainText(trimmedBody)] + } + + let normalizedBody = normalizeLineEndings(in: body) + let lines = normalizedBody.components(separatedBy: "\n") + guard let diffStartIndex = actualDiffStartIndex(in: lines) else { + let trimmedBody = normalizedBody.trimmingCharacters(in: .whitespacesAndNewlines) + return trimmedBody.isEmpty ? [] : [.plainText(trimmedBody)] + } + + var blocks: [InboxMessageContentBlock] = [] + let leadingPlainText = lines[.. + let trailingPlainText: String + if let signatureIndex { + diffLines = remainingLines[.. Int? { + if let explicitDiffIndex = lines.firstIndex(where: { $0.hasPrefix("diff --git ") }) { + return explicitDiffIndex + } + + for index in lines.indices { + let line = lines[index] + guard line.hasPrefix("--- ") else { continue } + let nextIndex = lines.index(after: index) + guard nextIndex < lines.endIndex else { continue } + let nextLine = lines[nextIndex] + guard nextLine.hasPrefix("+++ ") else { continue } + + let oldPath = String(line.dropFirst(4)) + let newPath = String(nextLine.dropFirst(4)) + let looksLikeUnifiedDiff = (oldPath.hasPrefix("a/") || oldPath == "/dev/null") && + (newPath.hasPrefix("b/") || newPath == "/dev/null") + + if looksLikeUnifiedDiff { + return index + } + } + + return nil + } + + nonisolated static func isEmailSignatureSeparator(_ line: String) -> Bool { + line == "-- " || line == "--" + } + + nonisolated static func normalizeLineEndings(in text: String) -> String { + text + .replacingOccurrences(of: "\r\n", with: "\n") + .replacingOccurrences(of: "\r", with: "\n") + } } diff --git a/Hutch/Views/Inbox/ThreadViewModel.swift b/Hutch/Views/Inbox/ThreadViewModel.swift index 850b83c..f422fe3 100644 --- a/Hutch/Views/Inbox/ThreadViewModel.swift +++ b/Hutch/Views/Inbox/ThreadViewModel.swift @@ -446,7 +446,7 @@ final class ThreadViewModel { let normalizedIdentity = normalizedSenderIdentity(from: body, fallbackAuthor: author) let displayBody = sanitizedDisplayBody(from: body) - let contentBlocks = segmentMessageBody(displayBody, isPatch: payload.patch != nil) + let contentBlocks = InboxThreadUtilities.segmentMessageBody(displayBody, isPatch: payload.patch != nil) return InboxMessage( id: id, @@ -540,7 +540,7 @@ final class ThreadViewModel { } private static func sanitizedDisplayBody(from body: String) -> String { - let normalizedBody = normalizeLineEndings(in: body) + let normalizedBody = InboxThreadUtilities.normalizeLineEndings(in: body) let lines = normalizedBody.components(separatedBy: "\n") let headerPrefixes = ["From:", "Date:", "To:", "Cc:", "Subject:"] var headerCount = 0 @@ -565,93 +565,6 @@ final class ThreadViewModel { return lines.dropFirst(blankLineIndex + 1).joined(separator: "\n") } - nonisolated static func segmentMessageBodyForTesting(_ body: String, isPatch: Bool) -> [InboxMessageContentBlock] { - segmentMessageBody(body, isPatch: isPatch) - } - - private nonisolated static func segmentMessageBody(_ body: String, isPatch: Bool) -> [InboxMessageContentBlock] { - guard isPatch else { - let trimmedBody = body.trimmingCharacters(in: .whitespacesAndNewlines) - return trimmedBody.isEmpty ? [] : [.plainText(trimmedBody)] - } - - let normalizedBody = normalizeLineEndings(in: body) - let lines = normalizedBody.components(separatedBy: "\n") - guard let diffStartIndex = actualDiffStartIndex(in: lines) else { - let trimmedBody = normalizedBody.trimmingCharacters(in: .whitespacesAndNewlines) - return trimmedBody.isEmpty ? [] : [.plainText(trimmedBody)] - } - - var blocks: [InboxMessageContentBlock] = [] - let leadingPlainText = lines[.. - let trailingPlainText: String - if let signatureIndex { - diffLines = remainingLines[.. Int? { - if let explicitDiffIndex = lines.firstIndex(where: { $0.hasPrefix("diff --git ") }) { - return explicitDiffIndex - } - - for index in lines.indices { - let line = lines[index] - guard line.hasPrefix("--- ") else { continue } - let nextIndex = lines.index(after: index) - guard nextIndex < lines.endIndex else { continue } - let nextLine = lines[nextIndex] - guard nextLine.hasPrefix("+++ ") else { continue } - - let oldPath = String(line.dropFirst(4)) - let newPath = String(nextLine.dropFirst(4)) - let looksLikeUnifiedDiff = (oldPath.hasPrefix("a/") || oldPath == "/dev/null") && - (newPath.hasPrefix("b/") || newPath == "/dev/null") - - if looksLikeUnifiedDiff { - return index - } - } - - return nil - } - - private nonisolated static func isEmailSignatureSeparator(_ line: String) -> Bool { - line == "-- " || line == "--" - } - - private nonisolated static func normalizeLineEndings(in text: String) -> String { - text - .replacingOccurrences(of: "\r\n", with: "\n") - .replacingOccurrences(of: "\r", with: "\n") - } - private static func stripLeadingFromLineIfPresent(in body: String) -> String { let lines = body.components(separatedBy: "\n") guard let firstLine = lines.first, firstLine.hasPrefix("From:") else { diff --git a/HutchTests/InboxViewModelTests.swift b/HutchTests/InboxViewModelTests.swift index 7bfc199..e67d317 100644 --- a/HutchTests/InboxViewModelTests.swift +++ b/HutchTests/InboxViewModelTests.swift @@ -186,7 +186,7 @@ struct InboxViewModelTests { 2.50.1 (Apple Git-155) """ - let segments = ThreadViewModel.segmentMessageBodyForTesting(body, isPatch: true) + let segments = InboxThreadUtilities.segmentMessageBody(body, isPatch: true) #expect(segments.count == 3) -- cgit v1.2.3 From 0eceec357b7ef5251ed0ae9d42b513c17380e0af Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 21:48:58 -0500 Subject: feat: review patchsets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patchsets are how contributions reach sourcehut, and Hutch had no reference to them anywhere. This adds review and triage: read a series, see its checks and version chain, and set its status. Two schema facts shaped the design. MailingList exposes no patchsets field, so a list's patchsets cannot be queried directly. They are reachable only through thread roots, so the existing threads query now also selects root.patchset — no extra request — and the Patches tab is derived from that. It appears only on lists that actually carry patches. Patch carries no diff. index, count, version, prefix, subject, and trailers are all it has; the diff exists only inside the email body. Patch bodies are split with the same InboxThreadUtilities.segmentMessageBody the inbox uses and rendered through the existing DiffView. Patches are ordered by their [PATCH n/m] index rather than receipt order, since mail arrives out of sequence. Patches with no index are kept at the end rather than dropped, because a one-off patch has no prefix. updatePatchset is nullable, so a null response is treated as a declined change and the local status is left alone rather than advanced optimistically. UNKNOWN and SUPERSEDED are not offered: the first is a sentinel, the second is set by the server when a newer version lands. Patch submission stays out of scope. It is a git send-email flow, not a GraphQL mutation. --- Hutch/App/RootView.swift | 3 + Hutch/Views/Lookup/LookupView.swift | 2 + Hutch/Views/Patchsets/PatchsetDetailView.swift | 248 +++++++++++++++++ .../Views/Patchsets/PatchsetDetailViewModel.swift | 293 +++++++++++++++++++++ Hutch/Views/Projects/ProjectMailingListView.swift | 124 ++++++++- HutchTests/PatchsetTests.swift | 141 ++++++++++ 6 files changed, 810 insertions(+), 1 deletion(-) create mode 100644 Hutch/Views/Patchsets/PatchsetDetailView.swift create mode 100644 Hutch/Views/Patchsets/PatchsetDetailViewModel.swift create mode 100644 HutchTests/PatchsetTests.swift diff --git a/Hutch/App/RootView.swift b/Hutch/App/RootView.swift index 1ae4651..59d7caa 100644 --- a/Hutch/App/RootView.swift +++ b/Hutch/App/RootView.swift @@ -439,6 +439,7 @@ enum MoreRoute: Hashable { case projectDashboard(id: String, title: String?) case mailingList(InboxMailingListReference) case thread(InboxThreadSummary) + case patchset(id: Int, listName: String?) case manPageBrowser case manPage(URL) } @@ -472,6 +473,8 @@ private struct MoreNavigationRoot: View { ProjectDashboardDeepLinkView(projectID: id, title: title) case .mailingList(let mailingList): MailingListDetailView(mailingList: mailingList) + case .patchset(let id, let listName): + PatchsetDetailView(patchsetID: id, listName: listName) case .thread(let thread): ThreadDetailView( thread: thread, diff --git a/Hutch/Views/Lookup/LookupView.swift b/Hutch/Views/Lookup/LookupView.swift index 2a26282..b52bb3f 100644 --- a/Hutch/Views/Lookup/LookupView.swift +++ b/Hutch/Views/Lookup/LookupView.swift @@ -465,6 +465,8 @@ struct LookupView: View { ProjectDashboardDeepLinkView(projectID: id, title: title) case .mailingList(let mailingList): MailingListDetailView(mailingList: mailingList) + case .patchset(let id, let listName): + PatchsetDetailView(patchsetID: id, listName: listName) case .thread(let thread): ThreadDetailView( thread: thread, diff --git a/Hutch/Views/Patchsets/PatchsetDetailView.swift b/Hutch/Views/Patchsets/PatchsetDetailView.swift new file mode 100644 index 0000000..d76aab4 --- /dev/null +++ b/Hutch/Views/Patchsets/PatchsetDetailView.swift @@ -0,0 +1,248 @@ +import SwiftUI + +struct PatchsetDetailView: View { + let patchsetID: Int + let listName: String? + + @Environment(AppState.self) private var appState + @State private var viewModel: PatchsetDetailViewModel? + @State private var showStatusPicker = false + + var body: some View { + Group { + if let viewModel { + content(viewModel) + } else { + SRHTLoadingStateView(message: "Loading Patchset…") + } + } + .navigationTitle("Patchset") + .navigationBarTitleDisplayMode(.inline) + .toolbar { + if let viewModel, viewModel.patchset != nil { + ToolbarItem(placement: .topBarTrailing) { + actionsMenu(viewModel) + } + } + } + .task { + let model = viewModel ?? PatchsetDetailViewModel(patchsetID: patchsetID, client: appState.client) + viewModel = model + await model.loadPatchset() + } + } + + @ViewBuilder + private func content(_ viewModel: PatchsetDetailViewModel) -> some View { + if viewModel.isLoading && viewModel.patchset == nil { + SRHTLoadingStateView(message: "Loading Patchset…") + } else if let patchset = viewModel.patchset { + List { + headerSection(patchset) + if let coverLetter = patchset.coverLetter { + emailSection(coverLetter, title: "Cover Letter") + } + if !patchset.tools.isEmpty { + toolsSection(patchset) + } + patchesSection(patchset) + } + .themedList() + .refreshable { await viewModel.loadPatchset() } + .overlay { + if viewModel.isUpdatingStatus { + ProgressView() + } + } + .confirmationDialog( + "Set Status", + isPresented: $showStatusPicker, + titleVisibility: .visible + ) { + ForEach(PatchsetStatus.assignable, id: \.self) { status in + Button(status.displayName) { + Task { await viewModel.updateStatus(to: status) } + } + } + Button("Cancel", role: .cancel) {} + } + .alert( + "Couldn't Update Patchset", + isPresented: .init( + get: { viewModel.error != nil }, + set: { if !$0 { viewModel.error = nil } } + ) + ) { + Button("OK", role: .cancel) { viewModel.error = nil } + } message: { + Text(viewModel.error ?? "") + } + } else if let error = viewModel.error { + SRHTErrorStateView( + title: "Couldn't Load Patchset", + message: error, + retryAction: { await viewModel.loadPatchset() } + ) + } + } + + // MARK: - Sections + + @ViewBuilder + private func headerSection(_ patchset: PatchsetDetail) -> some View { + Section { + VStack(alignment: .leading, spacing: 8) { + Text(patchset.subject) + .font(.headline) + + HStack(spacing: 8) { + PatchsetStatusBadge(status: patchset.status) + if patchset.version > 1 { + Text("v\(patchset.version)") + .font(.caption.weight(.medium)) + .foregroundStyle(.secondary) + } + Text("\(patchset.patches.count) patch\(patchset.patches.count == 1 ? "" : "es")") + .font(.caption) + .foregroundStyle(.secondary) + } + + Text("\(patchset.submitter.canonicalName) • \(patchset.updated.relativeDescription)") + .font(.caption) + .foregroundStyle(.secondary) + + if let listName { + Text(listName) + .font(.caption) + .foregroundStyle(.secondary) + } + } + .padding(.vertical, 2) + .themedRow() + + // The version chain matters during review: a superseded series should + // usually be read at its newest version instead. + if let supersededBy = patchset.supersededBy { + NavigationLink(value: MoreRoute.patchset(id: supersededBy, listName: listName)) { + SwiftUI.Label("Superseded by a newer version", systemImage: "arrow.right.circle") + .font(.subheadline) + } + .themedRow() + } + + if let supersedes = patchset.supersedes { + NavigationLink(value: MoreRoute.patchset(id: supersedes, listName: listName)) { + SwiftUI.Label("Revises an earlier version", systemImage: "arrow.left.circle") + .font(.subheadline) + } + .themedRow() + } + } + } + + @ViewBuilder + private func toolsSection(_ patchset: PatchsetDetail) -> some View { + Section("Checks") { + ForEach(patchset.tools) { tool in + HStack(spacing: 8) { + Image(systemName: tool.icon.systemImage) + .foregroundStyle(tool.icon == .failed ? .red : .secondary) + Text(tool.details) + .font(.subheadline) + } + .themedRow() + } + } + } + + @ViewBuilder + private func patchesSection(_ patchset: PatchsetDetail) -> some View { + ForEach(patchset.patches) { patch in + emailSection(patch, title: patch.seriesLabel.map { "Patch \($0)" } ?? "Patch") + } + } + + @ViewBuilder + private func emailSection(_ email: PatchsetEmail, title: String) -> some View { + Section(title) { + VStack(alignment: .leading, spacing: 10) { + Text(email.subject) + .font(.subheadline.weight(.semibold)) + .textSelection(.enabled) + + ForEach(Array(email.contentBlocks.enumerated()), id: \.offset) { _, block in + switch block { + case .plainText(let text): + Text(text) + .font(.body) + .textSelection(.enabled) + .frame(maxWidth: .infinity, alignment: .leading) + .fixedSize(horizontal: false, vertical: true) + case .diff(let diff): + DiffView(diff: diff) + .textSelection(.enabled) + } + } + } + .padding(.vertical, 4) + .themedRow() + } + } + + // MARK: - Actions + + @ViewBuilder + private func actionsMenu(_ viewModel: PatchsetDetailViewModel) -> some View { + Menu { + Button { + showStatusPicker = true + } label: { + SwiftUI.Label("Set Status", systemImage: "flag") + } + .disabled(viewModel.isUpdatingStatus) + + if let mbox = viewModel.patchset?.mbox { + Divider() + ShareLink(item: mbox) { + SwiftUI.Label("Share mbox", systemImage: "square.and.arrow.up") + } + Button { + appState.copyToPasteboard(mbox.absoluteString, label: "mbox URL") + } label: { + SwiftUI.Label("Copy mbox URL", systemImage: "doc.on.doc") + } + } + } label: { + Image(systemName: "ellipsis.circle") + } + .accessibilityLabel("Patchset actions") + } +} + +// MARK: - Status Badge + +struct PatchsetStatusBadge: View { + let status: PatchsetStatus + + var body: some View { + SwiftUI.Label(status.displayName, systemImage: status.systemImage) + .font(.caption.weight(.medium)) + .padding(.horizontal, 8) + .padding(.vertical, 3) + .background(background, in: Capsule()) + .foregroundStyle(foreground) + } + + private var foreground: Color { + switch status { + case .applied, .approved: .green + case .rejected: .red + case .needsRevision: .orange + case .superseded, .unknown, .proposed: .secondary + } + } + + private var background: Color { + foreground.opacity(0.12) + } +} diff --git a/Hutch/Views/Patchsets/PatchsetDetailViewModel.swift b/Hutch/Views/Patchsets/PatchsetDetailViewModel.swift new file mode 100644 index 0000000..88af328 --- /dev/null +++ b/Hutch/Views/Patchsets/PatchsetDetailViewModel.swift @@ -0,0 +1,293 @@ +import Foundation + +// MARK: - Response types (file-private to avoid @MainActor Decodable issues) + +private struct PatchsetDetailResponse: Decodable, Sendable { + let patchset: PatchsetDetailPayload? +} + +private struct PatchsetDetailPayload: Decodable, Sendable { + let id: Int + let created: Date + let updated: Date + let subject: String + let version: Int + let prefix: String? + let status: PatchsetStatus + let submitter: Entity + let coverLetter: PatchsetEmailPayload? + let supersededBy: PatchsetReferencePayload? + let supersedes: PatchsetReferencePayload? + let patches: PatchsetPatchPage + let tools: [PatchsetToolPayload] + let mbox: URL? +} + +private struct PatchsetReferencePayload: Decodable, Sendable { + let id: Int +} + +private struct PatchsetPatchPage: Decodable, Sendable { + let results: [PatchsetEmailPayload] + let cursor: String? +} + +private struct PatchsetEmailPayload: Decodable, Sendable { + let id: Int + let subject: String + let date: Date? + let sender: Entity + let body: String + let patch: PatchIndexPayload? +} + +private struct PatchIndexPayload: Decodable, Sendable { + let index: Int? + let count: Int? +} + +private struct PatchsetToolPayload: Decodable, Sendable { + let id: Int + let icon: PatchsetToolIcon + let details: String +} + +private struct UpdatePatchsetResponse: Decodable, Sendable { + let patchset: UpdatedPatchsetPayload? +} + +private struct UpdatedPatchsetPayload: Decodable, Sendable { + let status: PatchsetStatus +} + +// MARK: - View Model + +@Observable +@MainActor +final class PatchsetDetailViewModel { + + let patchsetID: Int + + private(set) var patchset: PatchsetDetail? + private(set) var isLoading = false + private(set) var isUpdatingStatus = false + var error: String? + + private let client: SRHTClient + + init(patchsetID: Int, client: SRHTClient) { + self.patchsetID = patchsetID + self.client = client + } + + // MARK: - Queries + + /// `patches` is paginated, but a series is small and reviewing half of one is + /// worse than useless, so every page is walked before rendering. + private static let detailQuery = """ + query patchset($id: Int!, $cursor: Cursor) { + patchset(id: $id) { + id + created + updated + subject + version + prefix + status + submitter { canonicalName } + supersededBy { id } + supersedes { id } + coverLetter { + id + subject + date + sender { canonicalName } + body + patch { index count } + } + patches(cursor: $cursor) { + results { + id + subject + date + sender { canonicalName } + body + patch { index count } + } + cursor + } + tools { id icon details } + mbox + } + } + """ + + private static let updateStatusMutation = """ + mutation updatePatchset($id: Int!, $status: PatchsetStatus!) { + patchset: updatePatchset(id: $id, status: $status) { + status + } + } + """ + + // MARK: - Loading + + func loadPatchset() async { + guard !isLoading else { return } + isLoading = true + error = nil + defer { isLoading = false } + + do { + patchset = try await fetchPatchset() + } catch { + self.error = error.userFacingMessage + } + } + + private func fetchPatchset() async throws -> PatchsetDetail { + var cursor: String? + var payload: PatchsetDetailPayload? + var patches: [PatchsetEmailPayload] = [] + + // Walk the patches pages, keeping the first page's patchset fields. + while true { + var variables: [String: any Sendable] = ["id": patchsetID] + if let cursor { + variables["cursor"] = cursor + } + + let response = try await client.execute( + service: .lists, + query: Self.detailQuery, + variables: variables, + responseType: PatchsetDetailResponse.self + ) + + guard let page = response.patchset else { + throw SRHTError.graphQLErrors([ + GraphQLError(message: "That patchset is no longer available.", locations: nil) + ]) + } + + if payload == nil { + payload = page + } + patches.append(contentsOf: page.patches.results) + + guard let next = page.patches.cursor, !next.isEmpty else { break } + cursor = next + } + + guard let payload else { + throw SRHTError.graphQLErrors([ + GraphQLError(message: "That patchset is no longer available.", locations: nil) + ]) + } + + return PatchsetDetail( + id: payload.id, + created: payload.created, + updated: payload.updated, + subject: payload.subject, + version: payload.version, + prefix: payload.prefix, + status: payload.status, + submitter: payload.submitter, + coverLetter: payload.coverLetter.map { Self.makeEmail(from: $0, isPatch: false) }, + patches: Self.orderPatches(patches.map { Self.makeEmail(from: $0, isPatch: true) }), + supersededBy: payload.supersededBy?.id, + supersedes: payload.supersedes?.id, + tools: payload.tools.map { + PatchsetToolResult(id: $0.id, icon: $0.icon, details: $0.details) + }, + mbox: payload.mbox + ) + } + + // MARK: - Status + + /// Sets the review status. Returns true on success. + @discardableResult + func updateStatus(to newStatus: PatchsetStatus) async -> Bool { + guard !isUpdatingStatus, let current = patchset else { return false } + guard newStatus != current.status else { return true } + + isUpdatingStatus = true + error = nil + defer { isUpdatingStatus = false } + + do { + let response = try await client.execute( + service: .lists, + query: Self.updateStatusMutation, + variables: [ + "id": patchsetID, + "status": newStatus.rawValue + ], + responseType: UpdatePatchsetResponse.self + ) + + // updatePatchset is nullable: null means the server declined without + // erroring, so the local status must not be advanced. + guard let updated = response.patchset else { + self.error = "SourceHut did not apply that status change." + return false + } + + apply(status: updated.status) + return true + } catch { + self.error = error.userFacingMessage + return false + } + } + + private func apply(status: PatchsetStatus) { + guard let current = patchset else { return } + patchset = PatchsetDetail( + id: current.id, + created: current.created, + updated: current.updated, + subject: current.subject, + version: current.version, + prefix: current.prefix, + status: status, + submitter: current.submitter, + coverLetter: current.coverLetter, + patches: current.patches, + supersededBy: current.supersededBy, + supersedes: current.supersedes, + tools: current.tools, + mbox: current.mbox + ) + } + + // MARK: - Mapping + + private nonisolated static func makeEmail( + from payload: PatchsetEmailPayload, + isPatch: Bool + ) -> PatchsetEmail { + PatchsetEmail( + id: payload.id, + subject: payload.subject, + date: payload.date, + sender: payload.sender, + contentBlocks: InboxThreadUtilities.segmentMessageBody(payload.body, isPatch: isPatch), + index: payload.patch?.index, + count: payload.patch?.count + ) + } + + /// Orders a series by its `[PATCH n/m]` index. + /// + /// sr.ht returns patches in receipt order, which is not series order when a + /// contributor's mail arrives out of sequence. Patches without an index keep + /// their relative position at the end rather than being dropped. + nonisolated static func orderPatches(_ patches: [PatchsetEmail]) -> [PatchsetEmail] { + let indexed = patches.filter { $0.index != nil } + let unindexed = patches.filter { $0.index == nil } + return indexed.sorted { ($0.index ?? 0) < ($1.index ?? 0) } + unindexed + } +} diff --git a/Hutch/Views/Projects/ProjectMailingListView.swift b/Hutch/Views/Projects/ProjectMailingListView.swift index d466121..3933bf3 100644 --- a/Hutch/Views/Projects/ProjectMailingListView.swift +++ b/Hutch/Views/Projects/ProjectMailingListView.swift @@ -24,12 +24,25 @@ private struct ProjectMailingListRootPayload: Decodable, Sendable { let id: Int let messageID: String let patch: InboxPatchPreview? + /// Null unless the thread's root email opens a patchset. `MailingList` has no + /// patchsets field, so this is the only way to enumerate a list's patchsets. + let patchset: PatchsetSummaryPayload? +} + +private struct PatchsetSummaryPayload: Decodable, Sendable { + let id: Int + let subject: String + let version: Int + let prefix: String? + let status: PatchsetStatus } @Observable @MainActor final class MailingListDetailViewModel { private(set) var threads: [InboxThreadSummary] = [] + /// Patchsets on this list, derived from thread roots — see the query below. + private(set) var patchsets: [PatchsetSummary] = [] private(set) var isLoading = false var error: String? var searchText = "" @@ -52,6 +65,13 @@ final class MailingListDetailViewModel { id messageID patch { subject } + patchset { + id + subject + version + prefix + status + } } } } @@ -87,11 +107,46 @@ final class MailingListDetailViewModel { threads = deduplicateThreads( response.list.threads.results.map(makeSummary(from:)) ) + patchsets = Self.patchsets(from: response.list.threads.results) } catch { self.error = "Failed to load mailing list" } } + var filteredPatchsets: [PatchsetSummary] { + let query = searchText.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() + guard !query.isEmpty else { return patchsets } + return patchsets.filter { $0.subject.lowercased().contains(query) } + } + + /// Collects the patchsets opened by these threads, newest first. + /// + /// A revised series arrives as its own thread, so the same subject can appear + /// at several versions; they are kept as distinct patchsets and the version + /// chain is shown in the detail view. + private nonisolated static func patchsets( + from threads: [ProjectMailingListThreadPayload] + ) -> [PatchsetSummary] { + var seenIDs = Set() + var results: [PatchsetSummary] = [] + + for thread in threads { + guard let payload = thread.root.patchset, !seenIDs.contains(payload.id) else { continue } + seenIDs.insert(payload.id) + results.append( + PatchsetSummary( + id: payload.id, + subject: payload.subject, + version: payload.version, + prefix: payload.prefix, + status: payload.status + ) + ) + } + + return results + } + func markThreadRead(_ thread: InboxThreadSummary) { let viewedAt = max(Date(), thread.lastActivityAt) InboxReadStateStore.markViewed(viewedAt, for: thread.threadGroupingKey, defaults: defaults) @@ -257,12 +312,25 @@ final class MailingListDetailViewModel { } } +enum MailingListScope: String, CaseIterable, Hashable { + case threads + case patches + + var displayName: String { + switch self { + case .threads: "Threads" + case .patches: "Patches" + } + } +} + struct MailingListDetailView: View { let mailingList: InboxMailingListReference @Environment(AppState.self) private var appState @State private var viewModel: MailingListDetailViewModel? @State private var pinChangeCount = 0 + @State private var scope: MailingListScope = .threads private var currentUserKey: String? { appState.currentUser?.canonicalName @@ -337,6 +405,27 @@ struct MailingListDetailView: View { @Bindable var vm = viewModel List { + // Only offered when the list actually carries patches, so discussion + // lists do not grow an empty tab. + if !viewModel.patchsets.isEmpty { + Picker("Scope", selection: $scope) { + ForEach(MailingListScope.allCases, id: \.self) { scope in + Text(scope.displayName).tag(scope) + } + } + .pickerStyle(.segmented) + .listRowInsets(EdgeInsets(top: 4, leading: 12, bottom: 4, trailing: 12)) + .themedRow() + } + + if showingPatches(viewModel) { + ForEach(viewModel.filteredPatchsets) { patchset in + NavigationLink(value: MoreRoute.patchset(id: patchset.id, listName: mailingList.name)) { + PatchsetRow(patchset: patchset) + } + .themedRow() + } + } else { ForEach(viewModel.filteredThreads) { thread in NavigationLink { ThreadDetailView( @@ -373,13 +462,14 @@ struct MailingListDetailView: View { } } .themedRow() + } } .themedList() .listStyle(.plain) .searchable( text: $vm.searchText, placement: .navigationBarDrawer(displayMode: .always), - prompt: "Search messages" + prompt: showingPatches(viewModel) ? "Search patches" : "Search messages" ) .overlay { if viewModel.isLoading, viewModel.threads.isEmpty { @@ -390,6 +480,10 @@ struct MailingListDetailView: View { message: error, retryAction: { await viewModel.loadThreads() } ) + } else if showingPatches(viewModel) { + if !viewModel.patchsets.isEmpty, viewModel.filteredPatchsets.isEmpty { + ContentUnavailableView.search(text: viewModel.searchText) + } } else if !viewModel.threads.isEmpty, viewModel.filteredThreads.isEmpty { ContentUnavailableView.search(text: viewModel.searchText) } else if viewModel.threads.isEmpty { @@ -405,6 +499,34 @@ struct MailingListDetailView: View { } .srhtErrorBanner(error: $vm.error) } + + private func showingPatches(_ viewModel: MailingListDetailViewModel) -> Bool { + scope == .patches && !viewModel.patchsets.isEmpty + } +} + +struct PatchsetRow: View { + let patchset: PatchsetSummary + + var body: some View { + VStack(alignment: .leading, spacing: 6) { + Text(patchset.subject) + .font(.subheadline.weight(.medium)) + .lineLimit(2) + + HStack(spacing: 8) { + PatchsetStatusBadge(status: patchset.status) + if let versionLabel = patchset.versionLabel { + Text(versionLabel) + .font(.caption.weight(.medium)) + .foregroundStyle(.secondary) + } + } + } + .padding(.vertical, 2) + .accessibilityElement(children: .combine) + .accessibilityLabel("\(patchset.subject), \(patchset.status.displayName)") + } } struct ProjectMailingListView: View { diff --git a/HutchTests/PatchsetTests.swift b/HutchTests/PatchsetTests.swift new file mode 100644 index 0000000..945131f --- /dev/null +++ b/HutchTests/PatchsetTests.swift @@ -0,0 +1,141 @@ +import Foundation +import Testing +@testable import Hutch + +struct PatchsetStatusTests { + + @Test + func statusRawValuesMatchTheGraphQLEnum() { + // lists.sr.ht's PatchsetStatus enum values, which are sent verbatim to + // updatePatchset. + #expect(PatchsetStatus.unknown.rawValue == "UNKNOWN") + #expect(PatchsetStatus.proposed.rawValue == "PROPOSED") + #expect(PatchsetStatus.needsRevision.rawValue == "NEEDS_REVISION") + #expect(PatchsetStatus.superseded.rawValue == "SUPERSEDED") + #expect(PatchsetStatus.approved.rawValue == "APPROVED") + #expect(PatchsetStatus.rejected.rawValue == "REJECTED") + #expect(PatchsetStatus.applied.rawValue == "APPLIED") + } + + @Test + func assignableStatusesExcludeServerManagedOnes() { + // UNKNOWN is a sentinel and SUPERSEDED is set by the server when a newer + // version lands, so neither should be offered as a reviewer choice. + #expect(!PatchsetStatus.assignable.contains(.unknown)) + #expect(!PatchsetStatus.assignable.contains(.superseded)) + #expect(PatchsetStatus.assignable.contains(.approved)) + #expect(PatchsetStatus.assignable.contains(.rejected)) + #expect(PatchsetStatus.assignable.contains(.applied)) + #expect(PatchsetStatus.assignable.contains(.needsRevision)) + #expect(PatchsetStatus.assignable.contains(.proposed)) + } + + @Test + func openStatusesAreThoseAwaitingADecision() { + #expect(PatchsetStatus.proposed.isOpen) + #expect(PatchsetStatus.needsRevision.isOpen) + #expect(!PatchsetStatus.applied.isOpen) + #expect(!PatchsetStatus.rejected.isOpen) + #expect(!PatchsetStatus.superseded.isOpen) + } + + @Test + func statusDecodesFromTheWireFormat() throws { + let decoded = try JSONDecoder().decode(PatchsetStatus.self, from: Data("\"NEEDS_REVISION\"".utf8)) + #expect(decoded == .needsRevision) + } +} + +struct PatchsetSummaryTests { + + @Test + func versionLabelIsHiddenForFirstVersion() { + let summary = PatchsetSummary( + id: 1, + subject: "[PATCH] fix the thing", + version: 1, + prefix: nil, + status: .proposed + ) + + #expect(summary.versionLabel == nil) + } + + @Test + func versionLabelIsShownForRevisions() { + let summary = PatchsetSummary( + id: 1, + subject: "[PATCH v3] fix the thing", + version: 3, + prefix: nil, + status: .proposed + ) + + #expect(summary.versionLabel == "v3") + } +} + +@MainActor +struct PatchsetOrderingTests { + + private func makePatch(id: Int, index: Int?, count: Int?) -> PatchsetEmail { + PatchsetEmail( + id: id, + subject: "patch \(id)", + date: nil, + sender: Entity(canonicalName: "~someone"), + contentBlocks: [], + index: index, + count: count + ) + } + + @Test + func patchesAreOrderedBySeriesIndexNotReceiptOrder() { + let patches = [ + makePatch(id: 30, index: 3, count: 3), + makePatch(id: 10, index: 1, count: 3), + makePatch(id: 20, index: 2, count: 3) + ] + + let ordered = PatchsetDetailViewModel.orderPatches(patches) + + #expect(ordered.map(\.index) == [1, 2, 3]) + } + + @Test + func unindexedPatchesAreKeptAtTheEndRatherThanDropped() { + let patches = [ + makePatch(id: 99, index: nil, count: nil), + makePatch(id: 20, index: 2, count: 2), + makePatch(id: 10, index: 1, count: 2) + ] + + let ordered = PatchsetDetailViewModel.orderPatches(patches) + + #expect(ordered.count == 3) + #expect(ordered.map(\.index) == [1, 2, nil]) + } + + @Test + func orderingIsStableForASingleUnindexedPatch() { + // A lone patch with no [PATCH n/m] prefix is the common one-off case. + let patches = [makePatch(id: 1, index: nil, count: nil)] + + let ordered = PatchsetDetailViewModel.orderPatches(patches) + + #expect(ordered.map(\.id) == [1]) + } + + @Test + func seriesLabelIsHiddenForSinglePatchSeries() { + let patch = makePatch(id: 1, index: 1, count: 1) + #expect(patch.seriesLabel == nil) + } + + @Test + func seriesLabelShowsPositionForMultiPatchSeries() { + let patch = makePatch(id: 1, index: 2, count: 5) + #expect(patch.seriesLabel == "2/5") + } +} -- cgit v1.2.3 From 22c7a0e45a5cc344e2fa0635431d95c23024ad63 Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 21:50:30 -0500 Subject: chore: bump to 3.7.0 and record Phase 2 MARKETING_VERSION 3.6.0 -> 3.7.0, build 88 -> 89. The README feature list also picks up Phase 1's ticket editing, subscriptions, and email preferences, which it never gained. --- Hutch.xcodeproj/project.pbxproj | 24 ++++++++++++------------ README.md | 3 +++ ROADMAP.md | 37 +++++++++++++++++++++++-------------- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/Hutch.xcodeproj/project.pbxproj b/Hutch.xcodeproj/project.pbxproj index 4d1898a..8f71248 100644 --- a/Hutch.xcodeproj/project.pbxproj +++ b/Hutch.xcodeproj/project.pbxproj @@ -597,7 +597,7 @@ ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor; CODE_SIGN_ENTITLEMENTS = Hutch/Hutch.entitlements; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 88; + CURRENT_PROJECT_VERSION = 89; DEVELOPMENT_TEAM = ZCNAX3VL9D; ENABLE_PREVIEWS = YES; GENERATE_INFOPLIST_FILE = YES; @@ -614,7 +614,7 @@ "$(inherited)", "@executable_path/Frameworks", ); - MARKETING_VERSION = 3.6.0; + MARKETING_VERSION = 3.7.0; PRODUCT_BUNDLE_IDENTIFIER = net.cleberg.Hutch; PRODUCT_NAME = "$(TARGET_NAME)"; STRING_CATALOG_GENERATE_SYMBOLS = YES; @@ -634,7 +634,7 @@ ASSETCATALOG_COMPILER_GLOBAL_ACCENT_COLOR_NAME = AccentColor; CODE_SIGN_ENTITLEMENTS = Hutch/Hutch.entitlements; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 88; + CURRENT_PROJECT_VERSION = 89; DEVELOPMENT_TEAM = ZCNAX3VL9D; ENABLE_PREVIEWS = YES; GENERATE_INFOPLIST_FILE = YES; @@ -651,7 +651,7 @@ "$(inherited)", "@executable_path/Frameworks", ); - MARKETING_VERSION = 3.6.0; + MARKETING_VERSION = 3.7.0; PRODUCT_BUNDLE_IDENTIFIER = net.cleberg.Hutch; PRODUCT_NAME = "$(TARGET_NAME)"; STRING_CATALOG_GENERATE_SYMBOLS = YES; @@ -714,7 +714,7 @@ APPLICATION_EXTENSION_API_ONLY = YES; CODE_SIGN_ENTITLEMENTS = HutchWidgetExtension/HutchWidgetExtension.entitlements; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 88; + CURRENT_PROJECT_VERSION = 89; DEVELOPMENT_TEAM = ZCNAX3VL9D; GENERATE_INFOPLIST_FILE = NO; INFOPLIST_FILE = HutchWidgetExtension/Info.plist; @@ -724,7 +724,7 @@ "@executable_path/Frameworks", "@executable_path/../../Frameworks", ); - MARKETING_VERSION = 3.6.0; + MARKETING_VERSION = 3.7.0; PRODUCT_BUNDLE_IDENTIFIER = net.cleberg.Hutch.HutchWidgetExtension; PRODUCT_NAME = "$(TARGET_NAME)"; SKIP_INSTALL = YES; @@ -743,7 +743,7 @@ APPLICATION_EXTENSION_API_ONLY = YES; CODE_SIGN_ENTITLEMENTS = HutchWidgetExtension/HutchWidgetExtension.entitlements; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 88; + CURRENT_PROJECT_VERSION = 89; DEVELOPMENT_TEAM = ZCNAX3VL9D; GENERATE_INFOPLIST_FILE = NO; INFOPLIST_FILE = HutchWidgetExtension/Info.plist; @@ -753,7 +753,7 @@ "@executable_path/Frameworks", "@executable_path/../../Frameworks", ); - MARKETING_VERSION = 3.6.0; + MARKETING_VERSION = 3.7.0; PRODUCT_BUNDLE_IDENTIFIER = net.cleberg.Hutch.HutchWidgetExtension; PRODUCT_NAME = "$(TARGET_NAME)"; SKIP_INSTALL = YES; @@ -772,7 +772,7 @@ APPLICATION_EXTENSION_API_ONLY = YES; ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 88; + CURRENT_PROJECT_VERSION = 89; DEVELOPMENT_TEAM = ZCNAX3VL9D; GENERATE_INFOPLIST_FILE = NO; INFOPLIST_FILE = HutchSafariExtension/Info.plist; @@ -782,7 +782,7 @@ "@executable_path/Frameworks", "@executable_path/../../Frameworks", ); - MARKETING_VERSION = 3.6.0; + MARKETING_VERSION = 3.7.0; PRODUCT_BUNDLE_IDENTIFIER = net.cleberg.Hutch.HutchSafariExtension; PRODUCT_NAME = "$(TARGET_NAME)"; SKIP_INSTALL = YES; @@ -801,7 +801,7 @@ APPLICATION_EXTENSION_API_ONLY = YES; ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon; CODE_SIGN_STYLE = Automatic; - CURRENT_PROJECT_VERSION = 88; + CURRENT_PROJECT_VERSION = 89; DEVELOPMENT_TEAM = ZCNAX3VL9D; GENERATE_INFOPLIST_FILE = NO; INFOPLIST_FILE = HutchSafariExtension/Info.plist; @@ -811,7 +811,7 @@ "@executable_path/Frameworks", "@executable_path/../../Frameworks", ); - MARKETING_VERSION = 3.6.0; + MARKETING_VERSION = 3.7.0; PRODUCT_BUNDLE_IDENTIFIER = net.cleberg.Hutch.HutchSafariExtension; PRODUCT_NAME = "$(TARGET_NAME)"; SKIP_INSTALL = YES; diff --git a/README.md b/README.md index c81a7c4..fd553a8 100644 --- a/README.md +++ b/README.md @@ -15,10 +15,13 @@ The app currently includes: - Repository browsing for Git and Mercurial repositories - Repository details including README, references, commits, diffs, files, artifacts, and settings - Tracker and ticket browsing, ticket detail views, and tracker creation +- Ticket editing and deletion, with subscriptions for tickets and trackers - Build job browsing, build detail views, and build submission - Inbox and mailing list reading flows +- Patchset review: cover letters, per-patch diffs, checks, version chains, and status changes - Paste browsing, creation, and detail views - Profile and account settings, including SSH keys, PGP keys, and personal access token management +- Email preferences for todo.sr.ht and lists.sr.ht - Deep links for repositories, tickets, and build jobs Some SourceHut services are still browser-only from within Hutch. Unsupported areas currently open in Safari instead of rendering in-app. diff --git a/ROADMAP.md b/ROADMAP.md index e86ddb0..66e56cf 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -74,24 +74,33 @@ Known follow-up: `BuildListViewModel`, `RepositoryListViewModel`, and two different cache keys. That predates `APICacheKeys` and should be folded into `cachedPayload`. -## Phase 2: Patchsets +## Phase 2: Patchsets — done (v3.7.0) -The flagship gap. There is currently no reference to `patchset` anywhere in the -Swift source, yet lists.sr.ht exposes a full `Patchset` type (subject, version, -prefix, status, coverLetter, patches, tools, mbox), a `patchset` query, and an -`updatePatchset` mutation. Sending and reviewing patches over email is the -SourceHut contribution model, and Hutch cannot currently participate in it. +The flagship gap. Sending and reviewing patches over email is the SourceHut +contribution model, and Hutch had no reference to `patchset` anywhere. -Scope this as review-and-triage, not submission: +Scoped as review-and-triage, not submission: -- Patchset list per mailing list. -- Patchset detail: cover letter, per-patch diffs (reuse the existing - `DiffView`), version and superseded-by chain. -- Status transitions via `updatePatchset`. +- ~~Patchset list per mailing list~~ — see the caveat below. +- ~~Patchset detail~~: cover letter, per-patch diffs (via the existing + `DiffView`), checks, and the version / superseded-by chain. +- ~~Status transitions via `updatePatchset`~~. -Patch *submission* is an email / `git send-email` flow and is likely out of -reach from the app. Treat that boundary as explicit rather than half-building -it. +Two schema facts shaped the result, and are worth knowing before extending this: + +- **`MailingList` has no `patchsets` field.** A list's patchsets cannot be + queried directly; they are reachable only through thread roots. The existing + threads query now also selects `root.patchset`, so the Patches tab costs no + extra request — but it also means patchsets cannot be filtered by status + server-side, and only patchsets whose thread appears in the current page are + listed. +- **`Patch` carries no diff.** It has only `index`, `count`, `version`, + `prefix`, `subject`, and `trailers`. The diff exists solely inside the email + body, so it is recovered with `InboxThreadUtilities.segmentMessageBody` — the + same splitter the inbox thread view uses. + +Patch *submission* remains out of reach: it is a `git send-email` flow, not a +GraphQL mutation. Treat that boundary as explicit rather than half-building it. ## Phase 3: Polish and reach -- cgit v1.2.3 From 575e62f6dab44b0c9836623fe8b7d17a219f8e0f Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 21:56:56 -0500 Subject: fix: push patchset views directly instead of by route Tapping a patch failed with "no matching navigationDestination declaration visible from the location of the link". MailingListDetailView is presented from four places, but only the More tab and Lookup declare a MoreRoute destination. Reached from a project, via ProjectMailingListView, there is no such destination in the surrounding stack, so a NavigationLink carrying MoreRoute.patchset had nowhere to resolve. The thread rows beside it already use the closure form for exactly this reason. Push PatchsetDetailView directly, from the rows and from the version-chain links inside the detail view, which inherits whatever stack presented it. That leaves MoreRoute.patchset with no users, so it and its two destinations are removed rather than left as a route nothing links to. Neither the compiler nor the tests catch this: it is a runtime SwiftUI resolution failure. --- Hutch/App/RootView.swift | 3 --- Hutch/Views/Lookup/LookupView.swift | 2 -- Hutch/Views/Patchsets/PatchsetDetailView.swift | 12 ++++++++++-- Hutch/Views/Projects/ProjectMailingListView.swift | 6 +++++- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/Hutch/App/RootView.swift b/Hutch/App/RootView.swift index 59d7caa..1ae4651 100644 --- a/Hutch/App/RootView.swift +++ b/Hutch/App/RootView.swift @@ -439,7 +439,6 @@ enum MoreRoute: Hashable { case projectDashboard(id: String, title: String?) case mailingList(InboxMailingListReference) case thread(InboxThreadSummary) - case patchset(id: Int, listName: String?) case manPageBrowser case manPage(URL) } @@ -473,8 +472,6 @@ private struct MoreNavigationRoot: View { ProjectDashboardDeepLinkView(projectID: id, title: title) case .mailingList(let mailingList): MailingListDetailView(mailingList: mailingList) - case .patchset(let id, let listName): - PatchsetDetailView(patchsetID: id, listName: listName) case .thread(let thread): ThreadDetailView( thread: thread, diff --git a/Hutch/Views/Lookup/LookupView.swift b/Hutch/Views/Lookup/LookupView.swift index b52bb3f..2a26282 100644 --- a/Hutch/Views/Lookup/LookupView.swift +++ b/Hutch/Views/Lookup/LookupView.swift @@ -465,8 +465,6 @@ struct LookupView: View { ProjectDashboardDeepLinkView(projectID: id, title: title) case .mailingList(let mailingList): MailingListDetailView(mailingList: mailingList) - case .patchset(let id, let listName): - PatchsetDetailView(patchsetID: id, listName: listName) case .thread(let thread): ThreadDetailView( thread: thread, diff --git a/Hutch/Views/Patchsets/PatchsetDetailView.swift b/Hutch/Views/Patchsets/PatchsetDetailView.swift index d76aab4..7bc5630 100644 --- a/Hutch/Views/Patchsets/PatchsetDetailView.swift +++ b/Hutch/Views/Patchsets/PatchsetDetailView.swift @@ -122,8 +122,14 @@ struct PatchsetDetailView: View { // The version chain matters during review: a superseded series should // usually be read at its newest version instead. + // + // Pushed directly rather than by value, for the same reason as the rows + // that lead here — this view inherits whatever stack presented it, and + // not all of them declare a MoreRoute destination. if let supersededBy = patchset.supersededBy { - NavigationLink(value: MoreRoute.patchset(id: supersededBy, listName: listName)) { + NavigationLink { + PatchsetDetailView(patchsetID: supersededBy, listName: listName) + } label: { SwiftUI.Label("Superseded by a newer version", systemImage: "arrow.right.circle") .font(.subheadline) } @@ -131,7 +137,9 @@ struct PatchsetDetailView: View { } if let supersedes = patchset.supersedes { - NavigationLink(value: MoreRoute.patchset(id: supersedes, listName: listName)) { + NavigationLink { + PatchsetDetailView(patchsetID: supersedes, listName: listName) + } label: { SwiftUI.Label("Revises an earlier version", systemImage: "arrow.left.circle") .font(.subheadline) } diff --git a/Hutch/Views/Projects/ProjectMailingListView.swift b/Hutch/Views/Projects/ProjectMailingListView.swift index 3933bf3..696cf6b 100644 --- a/Hutch/Views/Projects/ProjectMailingListView.swift +++ b/Hutch/Views/Projects/ProjectMailingListView.swift @@ -420,7 +420,11 @@ struct MailingListDetailView: View { if showingPatches(viewModel) { ForEach(viewModel.filteredPatchsets) { patchset in - NavigationLink(value: MoreRoute.patchset(id: patchset.id, listName: mailingList.name)) { + // Pushed directly rather than by value: this view is also shown + // from a project, whose stack declares no MoreRoute destination. + NavigationLink { + PatchsetDetailView(patchsetID: patchset.id, listName: mailingList.name) + } label: { PatchsetRow(patchset: patchset) } .themedRow() -- cgit v1.2.3 From 2913d950a5c82d80226050e05bd64be7e30ff87d Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 22:02:55 -0500 Subject: fix: collapse patches to stop a recursive layout loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening a patchset wedged the app. UICollectionView reported a row oscillating between 3674pt and 1647pt and trapped in a recursive layout loop, leaving the UI unresponsive. The detail view rendered every patch in the series expanded, so a List held one enormous self-sizing row per patch, each with a full diff. Self-sizing cells that large do not settle. Patches now start collapsed and expand on tap, so at most the ones a reviewer opens are measured. This is what ThreadDetailView already does — it collapses every message but the last, and renders the same diffs through the same DiffView without trouble. Reviewing a series one patch at a time is also closer to how the reading actually goes. The rendering of a block list is shared between the cover letter and patches rather than duplicated. --- Hutch/Views/Patchsets/PatchsetDetailView.swift | 104 +++++++++++++++++++++---- 1 file changed, 89 insertions(+), 15 deletions(-) diff --git a/Hutch/Views/Patchsets/PatchsetDetailView.swift b/Hutch/Views/Patchsets/PatchsetDetailView.swift index 7bc5630..f904424 100644 --- a/Hutch/Views/Patchsets/PatchsetDetailView.swift +++ b/Hutch/Views/Patchsets/PatchsetDetailView.swift @@ -7,6 +7,7 @@ struct PatchsetDetailView: View { @Environment(AppState.self) private var appState @State private var viewModel: PatchsetDetailViewModel? @State private var showStatusPicker = false + @State private var expandedPatchIDs: Set = [] var body: some View { Group { @@ -163,10 +164,31 @@ struct PatchsetDetailView: View { } } + /// Patches start collapsed. + /// + /// A diff is tall, and a series is many of them. Rendering every patch expanded + /// puts a dozen self-sizing diffs in one List, which drives UICollectionView + /// into a recursive layout loop and wedges the app. The inbox thread view + /// collapses all but the last message for the same reason. @ViewBuilder private func patchesSection(_ patchset: PatchsetDetail) -> some View { - ForEach(patchset.patches) { patch in - emailSection(patch, title: patch.seriesLabel.map { "Patch \($0)" } ?? "Patch") + Section("Patches") { + ForEach(patchset.patches) { patch in + PatchRow( + patch: patch, + isExpanded: expandedPatchIDs.contains(patch.id), + onToggle: { + withAnimation(.easeInOut(duration: 0.2)) { + if expandedPatchIDs.contains(patch.id) { + expandedPatchIDs.remove(patch.id) + } else { + expandedPatchIDs.insert(patch.id) + } + } + } + ) + .themedRow() + } } } @@ -178,19 +200,7 @@ struct PatchsetDetailView: View { .font(.subheadline.weight(.semibold)) .textSelection(.enabled) - ForEach(Array(email.contentBlocks.enumerated()), id: \.offset) { _, block in - switch block { - case .plainText(let text): - Text(text) - .font(.body) - .textSelection(.enabled) - .frame(maxWidth: .infinity, alignment: .leading) - .fixedSize(horizontal: false, vertical: true) - case .diff(let diff): - DiffView(diff: diff) - .textSelection(.enabled) - } - } + PatchsetContentBlocks(blocks: email.contentBlocks) } .padding(.vertical, 4) .themedRow() @@ -227,6 +237,70 @@ struct PatchsetDetailView: View { } } +// MARK: - Patch Row + +private struct PatchRow: View { + let patch: PatchsetEmail + let isExpanded: Bool + let onToggle: () -> Void + + var body: some View { + VStack(alignment: .leading, spacing: isExpanded ? 10 : 0) { + Button(action: onToggle) { + HStack(alignment: .top, spacing: 12) { + Image(systemName: isExpanded ? "chevron.down" : "chevron.right") + .font(.caption) + .foregroundStyle(.tertiary) + .padding(.top, 3) + + VStack(alignment: .leading, spacing: 2) { + Text(patch.subject) + .font(.subheadline.weight(.medium)) + .lineLimit(isExpanded ? nil : 2) + .multilineTextAlignment(.leading) + .frame(maxWidth: .infinity, alignment: .leading) + + if let seriesLabel = patch.seriesLabel { + Text(seriesLabel) + .font(.caption) + .foregroundStyle(.secondary) + } + } + } + } + .buttonStyle(.plain) + .accessibilityHint(isExpanded ? "Collapses this patch" : "Expands this patch") + + if isExpanded { + PatchsetContentBlocks(blocks: patch.contentBlocks) + } + } + .padding(.vertical, 4) + } +} + +// MARK: - Content Blocks + +private struct PatchsetContentBlocks: View { + let blocks: [InboxMessageContentBlock] + + var body: some View { + ForEach(Array(blocks.enumerated()), id: \.offset) { _, block in + switch block { + case .plainText(let text): + Text(text) + .font(.body) + .textSelection(.enabled) + .frame(maxWidth: .infinity, alignment: .leading) + .fixedSize(horizontal: false, vertical: true) + case .diff(let diff): + DiffView(diff: diff) + .textSelection(.enabled) + } + } + } +} + // MARK: - Status Badge struct PatchsetStatusBadge: View { -- cgit v1.2.3 From 1d2769fc7a347939275e9130ee174d61d96ea401 Mon Sep 17 00:00:00 2001 From: Christian Cleberg Date: Wed, 15 Jul 2026 22:08:53 -0500 Subject: fix: render patchsets on a plain list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Collapsing patches shrank the layout loop from 3674pt/1647pt to 718pt/600pt but did not end it. The oscillating item is section 1 item 0 — the cover letter, not a patch — so size alone was not the cause. The log shows the cell laid out at width 390.0 while the content reports its preferred size at 390.333. That is inset grouped's 20pt insets landing on a fractional width: the Text reflows to a different height than the cell was sized for, each size triggers the other, and it never settles. ThreadDetailView renders the same bodies through the same DiffView with the same modifiers and does not loop. The difference is .listStyle(.plain), which this view never set and so inherited inset grouped. --- Hutch/Views/Patchsets/PatchsetDetailView.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Hutch/Views/Patchsets/PatchsetDetailView.swift b/Hutch/Views/Patchsets/PatchsetDetailView.swift index f904424..f560326 100644 --- a/Hutch/Views/Patchsets/PatchsetDetailView.swift +++ b/Hutch/Views/Patchsets/PatchsetDetailView.swift @@ -49,6 +49,12 @@ struct PatchsetDetailView: View { patchesSection(patchset) } .themedList() + // Inset grouped lays cells out at a rounded width while the content + // measures itself at the unrounded one, so a long Text reflows to a + // different height than the cell was sized for and the two chase each + // other into a layout loop. ThreadDetailView renders the same bodies + // through the same DiffView on a plain list without that fight. + .listStyle(.plain) .refreshable { await viewModel.loadPatchset() } .overlay { if viewModel.isUpdatingStatus { -- cgit v1.2.3