diff --git a/macgit/App/PullRequestController.swift b/macgit/App/PullRequestController.swift index c5b77e9..c83538e 100644 --- a/macgit/App/PullRequestController.swift +++ b/macgit/App/PullRequestController.swift @@ -67,6 +67,8 @@ private struct GitRepositoryIdentityKey: Hashable { final class PullRequestController: ObservableObject { @Published private(set) var items: [PullRequestSummary] = [] @Published private(set) var isLoading = false + @Published private(set) var hasLoadedList = false + private var displayedListKey: PullRequestListCacheKey? @Published var errorMessage: String? @Published var detailErrorMessage: String? @Published var stateFilter: PullRequestListFilter = .open @@ -389,6 +391,8 @@ final class PullRequestController: ObservableObject { knownGitLabHosts: connectedGitLabHosts, knownHosts: providerAccountController.accounts.map { GitProviderHost(kind: $0.provider, baseURL: $0.hostURL) } ) else { + hasLoadedList = false + displayedListKey = nil items = [] resetPagination() selectedProviderAccountID = nil @@ -409,6 +413,8 @@ final class PullRequestController: ObservableObject { let matchingAccounts = matchingAccounts(for: repository) guard !matchingAccounts.isEmpty else { + hasLoadedList = false + displayedListKey = nil items = [] resetPagination() selectedProviderAccountID = nil @@ -420,6 +426,8 @@ final class PullRequestController: ObservableObject { } guard let apiCredential = apiCredential(for: matchingAccounts) else { + hasLoadedList = false + displayedListKey = nil items = [] resetPagination() selectedProviderAccountID = nil @@ -437,6 +445,8 @@ final class PullRequestController: ObservableObject { let token = apiCredential.token guard let service = services[repository.provider] else { + hasLoadedList = false + displayedListKey = nil items = [] resetPagination() activeRepository = nil @@ -455,13 +465,18 @@ final class PullRequestController: ObservableObject { page: page, perPage: pullRequestPageSize ) - if !forceRefresh, - let cached = await diskCache.value(PullRequestListPage.self, key: cacheKey.diskKey, generation: cacheGeneration) { + hasLoadedList = displayedListKey == cacheKey + var displayedPage: PullRequestListPage? + if displayedListKey == cacheKey { + displayedPage = PullRequestListPage(items: items, page: currentPage, perPage: pullRequestPageSize, hasPreviousPage: hasPreviousPage, hasNextPage: hasNextPage) + hasLoadedList = true + } + if let cached = await diskCache.entry(PullRequestListPage.self, key: cacheKey.diskKey, generation: cacheGeneration) { guard listLoadID == loadID, !Task.isCancelled else { return } - apply(cached) - accountConnectionHost = nil - errorMessage = nil - return + if displayedPage != cached.value { apply(cached.value) } + displayedPage = cached.value + displayedListKey = cacheKey + hasLoadedList = true } guard listLoadID == loadID, !Task.isCancelled else { return } @@ -477,19 +492,25 @@ final class PullRequestController: ObservableObject { await diskCache.save(pageResult, key: cacheKey.diskKey, accountID: cacheKey.accountID, kind: "list", ttl: listCacheTTL, generation: cacheGeneration) guard listLoadID == loadID, !Task.isCancelled else { return } - apply(pageResult) + if displayedPage != pageResult { apply(pageResult) } + displayedListKey = cacheKey + hasLoadedList = true accountConnectionHost = nil errorMessage = nil } catch let error as PullRequestProviderError { guard listLoadID == loadID, !Task.isCancelled else { return } - items = [] - resetPagination() + if !hasLoadedList { + items = [] + resetPagination() + } accountConnectionHost = nil errorMessage = error.localizedDescription } catch { guard listLoadID == loadID, !Task.isCancelled else { return } - items = [] - resetPagination() + if !hasLoadedList { + items = [] + resetPagination() + } accountConnectionHost = nil errorMessage = error.localizedDescription } @@ -531,11 +552,10 @@ final class PullRequestController: ObservableObject { accountID: selectedProviderAccountID ?? "", number: summary.number ) - if !forceRefresh, - let cached = await diskCache.value(PullRequestDetail.self, key: cacheKey.diskKey, generation: cacheGeneration) { + if let cached = await diskCache.entry(PullRequestDetail.self, key: cacheKey.diskKey, generation: cacheGeneration) { guard detailLoadID == loadID, !Task.isCancelled else { return } - selectedDetail = cached - return + selectedDetail = cached.value + if !forceRefresh, cached.expiresAt > .now { return } } guard detailLoadID == loadID, !Task.isCancelled else { return } @@ -1034,6 +1054,8 @@ final class PullRequestController: ObservableObject { } func releaseVisibleData() { + hasLoadedList = false + displayedListKey = nil listLoadID = UUID() isLoading = false items = [] diff --git a/macgit/App/PullRequestDetailWindowController.swift b/macgit/App/PullRequestDetailWindowController.swift new file mode 100644 index 0000000..760bb1c --- /dev/null +++ b/macgit/App/PullRequestDetailWindowController.swift @@ -0,0 +1,61 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import AppKit +import SwiftUI + +@MainActor +final class PullRequestDetailWindowController: NSWindowController, NSWindowDelegate { + private var onClose: (() -> Void)? + + init() { + super.init(window: nil) + } + + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + func activateIfPresented() -> Bool { + guard let window else { return false } + window.makeKeyAndOrderFront(nil) + return true + } + + func show(title: String, content: Content, onClose: @escaping () -> Void) { + close() + self.onClose = onClose + let screen = NSApp.keyWindow?.screen ?? NSScreen.main + let visibleFrame = screen?.visibleFrame ?? NSRect(x: 0, y: 0, width: 1200, height: 800) + let size = NSSize(width: min(1200, visibleFrame.width - 40), + height: min(800, visibleFrame.height - 80)) + let window = NSWindow(contentRect: NSRect(origin: .zero, size: size), + styleMask: [.titled, .closable, .resizable], backing: .buffered, defer: false) + window.title = title + window.isReleasedWhenClosed = false + window.tabbingMode = .disallowed + window.collectionBehavior = [.fullScreenNone] + window.standardWindowButton(.miniaturizeButton)?.isHidden = true + window.standardWindowButton(.zoomButton)?.isHidden = true + window.delegate = self + let hostingView = NSHostingView(rootView: GeometryReader { geometry in + content.frame(width: geometry.size.width, height: geometry.size.height) + }) + // Window geometry owns the viewport; diff content must not expand the window. + hostingView.sizingOptions = [] + window.contentView = hostingView + window.contentMinSize = NSSize(width: min(960, size.width), height: min(560, size.height)) + window.setContentSize(size) + window.setFrameOrigin(NSPoint(x: visibleFrame.midX - window.frame.width / 2, + y: visibleFrame.midY - window.frame.height / 2)) + self.window = window + showWindow(nil) + window.makeKeyAndOrderFront(nil) + } + + func windowWillClose(_ notification: Notification) { + let callback = onClose + onClose = nil + window = nil + callback?() + } +} diff --git a/macgit/Models/GitBranchNameSanitizer.swift b/macgit/Models/GitBranchNameSanitizer.swift index 2ad4bc0..e9d07e8 100644 --- a/macgit/Models/GitBranchNameSanitizer.swift +++ b/macgit/Models/GitBranchNameSanitizer.swift @@ -20,6 +20,34 @@ import Foundation enum GitBranchNameSanitizer { nonisolated static func sanitize(_ input: String) -> String { - input.split(whereSeparator: { $0.isWhitespace }).joined(separator: "-") + let forbidden = CharacterSet(charactersIn: "~^:?*[\\") + .union(.controlCharacters) + var name = input.split(whereSeparator: { $0.isWhitespace }).joined(separator: "-") + name = String(name.unicodeScalars.map { forbidden.contains($0) ? "-" : String($0) }.joined()) + name = name.replacingOccurrences(of: "@{", with: "-") + while name.contains("..") { + name = name.replacingOccurrences(of: "..", with: "-") + } + // Branch names are stricter than refs: an initial dash is forbidden. + while name.hasPrefix("-") || name.hasPrefix("/") { name.removeFirst() } + + // A slash separates ref components; each component has its own rules. + let components = name.split(separator: "/").compactMap { component -> String? in + var value = String(component) + while value.hasPrefix(".") { value.removeFirst() } + // Removing one suffix can expose another forbidden suffix. + while value.hasSuffix(".") || value.hasSuffix(".lock") { + if value.hasSuffix(".") { + value.removeLast() + } else { + value.removeLast(5) + } + } + return value.isEmpty ? nil : value + } + name = components.joined(separator: "/") + if name.hasPrefix("-") { return sanitize(name) } + if name == "@" || name == "HEAD" { name += "-branch" } + return name } } diff --git a/macgit/Services/PullRequestDiskCache.swift b/macgit/Services/PullRequestDiskCache.swift index 6684f77..7060c96 100644 --- a/macgit/Services/PullRequestDiskCache.swift +++ b/macgit/Services/PullRequestDiskCache.swift @@ -26,15 +26,32 @@ actor PullRequestDiskCache { func generation() -> UUID { epoch } + struct Entry: Sendable { + let value: Value + let fetchedAt: Date + let expiresAt: Date + } + func value(_ type: T.Type, key: String, generation: UUID, now: Date = .now) -> T? { + guard let cached = entry(type, key: key, generation: generation, now: now), + cached.expiresAt > now else { return nil } + return cached.value + } + + /// Expiration controls revalidation, not whether local data may be displayed. + func entry(_ type: T.Type, key: String, generation: UUID, + now: Date = .now) -> Entry? { guard generation == epoch else { return nil } return try? database { db in - var result: T? - try query(db, "SELECT payload FROM entries WHERE key = ? AND expires > ?", [key, String(now.timeIntervalSince1970)]) { statement in + var result: Entry? + try query(db, "SELECT payload, fetched, expires FROM entries WHERE key = ?", [key]) { statement in let count = Int(sqlite3_column_bytes(statement, 0)) - guard count <= maximumEntryBytes, let bytes = sqlite3_column_blob(statement, 0) else { return } - result = try? JSONDecoder().decode(type, from: Data(bytes: bytes, count: count)) + guard count <= maximumEntryBytes, let bytes = sqlite3_column_blob(statement, 0), + let value = try? JSONDecoder().decode(type, from: Data(bytes: bytes, count: count)) else { return } + result = Entry(value: value, + fetchedAt: Date(timeIntervalSince1970: sqlite3_column_double(statement, 1)), + expiresAt: Date(timeIntervalSince1970: sqlite3_column_double(statement, 2))) } if result != nil { try query(db, "UPDATE entries SET accessed = ? WHERE key = ?", [String(now.timeIntervalSince1970), key]) @@ -57,10 +74,9 @@ actor PullRequestDiskCache { try? database { db in try query(db, "BEGIN IMMEDIATE") do { - try query(db, "DELETE FROM entries WHERE expires <= ?", [String(now.timeIntervalSince1970)]) - try query(db, "INSERT OR REPLACE INTO entries (key, account, kind, number, expires, accessed, payload) VALUES (?, ?, ?, ?, ?, ?, ?)", + try query(db, "INSERT OR REPLACE INTO entries (key, account, kind, number, expires, accessed, fetched, payload) VALUES (?, ?, ?, ?, ?, ?, ?, ?)", [key, accountID, kind, String(number), String(now.addingTimeInterval(ttl).timeIntervalSince1970), - String(now.timeIntervalSince1970), String(decoding: data, as: UTF8.self)]) + String(now.timeIntervalSince1970), String(now.timeIntervalSince1970), String(decoding: data, as: UTF8.self)]) var size = 0 var count = 0 try query(db, "SELECT COALESCE(SUM(length(CAST(payload AS BLOB))), 0), COUNT(*) FROM entries") { @@ -118,9 +134,12 @@ actor PullRequestDiskCache { try query(db, "PRAGMA max_page_count = 10240") var version = 0 try query(db, "PRAGMA user_version") { version = Int(sqlite3_column_int($0, 0)) } - guard version <= 1 else { throw CocoaError(.fileReadCorruptFile) } + guard version <= 2 else { throw CocoaError(.fileReadCorruptFile) } try query(db, "CREATE TABLE IF NOT EXISTS entries (key TEXT PRIMARY KEY, account TEXT NOT NULL, kind TEXT NOT NULL, number INTEGER NOT NULL, expires REAL NOT NULL, accessed REAL NOT NULL, payload TEXT NOT NULL)") - if version == 0 { try query(db, "PRAGMA user_version = 1") } + if version < 2 { + try query(db, "ALTER TABLE entries ADD COLUMN fetched REAL NOT NULL DEFAULT 0") + try query(db, "PRAGMA user_version = 2") + } return try operation(db) } diff --git a/macgit/Services/SyncState.swift b/macgit/Services/SyncState.swift index eaa3f86..7c18a3f 100644 --- a/macgit/Services/SyncState.swift +++ b/macgit/Services/SyncState.swift @@ -123,10 +123,11 @@ class SyncState: ObservableObject { private let refreshCoordinator = SyncRefreshCoordinator() private var refreshGeneration = 0 private var refreshedCurrentBranches: [URL: String] = [:] - private var refreshedLocalStatuses: [URL: GitStatus] = [:] + @Published private(set) var refreshedLocalStatuses: [URL: GitStatus] = [:] private static let backgroundRefreshInterval: Duration = .seconds(10) + private static let backgroundPollingInterval: Duration = .seconds(5) private static let localRefreshCoalescingInterval: TimeInterval = 2 - private static let autoFetchInterval: TimeInterval = 60 + private static let autoFetchInterval: TimeInterval = 15 init() { networkMonitor.start(queue: networkMonitorQueue) @@ -193,8 +194,10 @@ class SyncState: ObservableObject { let previousBranch = refreshedCurrentBranches.updateValue(currentBranch, forKey: repositoryURL) let localStatusDidChange: Bool if let localStatus { - let previousStatus = refreshedLocalStatuses.updateValue(localStatus, forKey: repositoryURL) - localStatusDidChange = previousStatus != localStatus + localStatusDidChange = refreshedLocalStatuses[repositoryURL] != localStatus + if localStatusDidChange { + refreshedLocalStatuses[repositoryURL] = localStatus + } } else { localStatusDidChange = false } @@ -240,8 +243,13 @@ class SyncState: ObservableObject { stopBackgroundSync() let autoFetchEnabled = settings.resolvedAutoFetchEnabled(globalValue: globalAutoFetchEnabled) backgroundTask = Task { + let clock = ContinuousClock() + var nextLocalRefresh = clock.now while !Task.isCancelled { - await refresh(repositoryURL: repositoryURL, force: false) + if clock.now >= nextLocalRefresh { + await refresh(repositoryURL: repositoryURL, force: false) + nextLocalRefresh = clock.now.advanced(by: Self.backgroundRefreshInterval) + } if autoFetchEnabled, networkMonitor.currentPath.status == .satisfied, await performAutomaticFetch( @@ -258,7 +266,7 @@ class SyncState: ObservableObject { ) } } - try? await Task.sleep(for: Self.backgroundRefreshInterval) + try? await Task.sleep(for: Self.backgroundPollingInterval) } } } diff --git a/macgit/Views/Common/FetchSheetView.swift b/macgit/Views/Common/FetchSheetView.swift index 62d5a62..1944e74 100644 --- a/macgit/Views/Common/FetchSheetView.swift +++ b/macgit/Views/Common/FetchSheetView.swift @@ -40,47 +40,58 @@ struct FetchSheetView: View { } var body: some View { - VStack(spacing: 0) { - VStack(alignment: .leading, spacing: 16) { - VStack(alignment: .leading, spacing: 10) { + VStack(alignment: .leading, spacing: 20) { + VStack(alignment: .leading, spacing: 4) { + Text("Fetch Changes") + .font(.title2) + .bold() + Text("Download remote updates without changing your local branches.") + .foregroundStyle(.secondary) + } + + GroupBox { + VStack(alignment: .leading, spacing: 12) { Toggle("Fetch from all remotes", isOn: $fetchAllRemotes) - .font(.system(size: 13)) - .toggleStyle(.checkbox) - Toggle("Prune tracking branches no longer present on remote(s)", isOn: $pruneBranches) - .font(.system(size: 13)) - .toggleStyle(.checkbox) + VStack(alignment: .leading, spacing: 4) { + Toggle("Prune deleted remote branches", isOn: $pruneBranches) + Text("Remove remote-tracking references that no longer exist on the remote.") + .font(.callout) + .foregroundStyle(.secondary) + .padding(.leading, 20) + } Toggle("Fetch and store all tags locally", isOn: $fetchTags) - .font(.system(size: 13)) - .toggleStyle(.checkbox) } - .padding(12) - .background(.quaternary.opacity(0.3)) - .clipShape(RoundedRectangle(cornerRadius: 8, style: .continuous)) + .toggleStyle(.checkbox) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(8) + } label: { + Text("Options") + .bold() } - .padding(24) - - Spacer() - HStack(spacing: 12) { + HStack(spacing: 8) { Spacer() Button("Cancel", role: .cancel) { dismiss() } .keyboardShortcut(.cancelAction) + .buttonStyle(.bordered) - Button("OK") { + Button("Fetch") { dismiss() onFetch(fetchOptions) } .keyboardShortcut(.defaultAction) - .buttonStyle(GlassProminentButtonStyle(tint: .accentColor, fontSize: 13)) + .buttonStyle(.borderedProminent) } - .padding([.horizontal, .bottom], 24) } - .frame(minWidth: 420, idealWidth: 420, maxWidth: 420) - .frame(minHeight: 200, idealHeight: 220) + .font(.body) + .controlSize(.regular) + .padding(24) + .frame(width: 480) + .fixedSize(horizontal: false, vertical: true) } } diff --git a/macgit/Views/Common/PullSheetView.swift b/macgit/Views/Common/PullSheetView.swift index 61f75ae..505590c 100644 --- a/macgit/Views/Common/PullSheetView.swift +++ b/macgit/Views/Common/PullSheetView.swift @@ -58,19 +58,28 @@ struct PullSheetView: View { } var body: some View { - VStack(spacing: 0) { - ScrollView { - VStack(alignment: .leading, spacing: 16) { - // Remote repository - VStack(alignment: .leading, spacing: 4) { - Text("Pull from repository:") - .font(.system(size: 13)) - Picker("", selection: $selectedRemote) { + VStack(alignment: .leading, spacing: 20) { + VStack(alignment: .leading, spacing: 4) { + Text("Pull Changes") + .font(.title2) + .bold() + Text("Integrate changes from a remote branch into your current branch.") + .foregroundStyle(.secondary) + } + + Grid(alignment: .leading, horizontalSpacing: 16, verticalSpacing: 14) { + GridRow(alignment: .firstTextBaseline) { + Text("Remote:") + .gridColumnAlignment(.trailing) + VStack(alignment: .leading, spacing: 6) { + Picker("Remote", selection: $selectedRemote) { ForEach(remotes, id: \.self) { remote in Text(remote).tag(remote) } } .pickerStyle(.menu) + .labelsHidden() + .frame(maxWidth: .infinity, alignment: .leading) .onChange(of: selectedRemote) { _, newValue in guard !isLoading else { return } Task { @@ -81,86 +90,97 @@ struct PullSheetView: View { if !remoteURL.isEmpty { Text(remoteURL) - .font(.system(size: 11)) + .font(.callout) .foregroundStyle(.secondary) .lineLimit(1) + .truncationMode(.middle) + .help(remoteURL) } } + .gridColumnAlignment(.leading) + } - // Remote branch - HStack(spacing: 12) { - VStack(alignment: .leading, spacing: 4) { - Text("Remote branch to pull:") - .font(.system(size: 13)) - HStack(spacing: 8) { - SearchableReferencePicker( - title: "Remote branch to pull", selection: selectedBranch, - options: remoteBranches, searchPrompt: "Search remote branches", - onSelect: { selectedBranch = $0 }) - .pickerStyle(.menu) - - Button("Refresh") { - Task { await loadRemoteBranches(remote: selectedRemote) } - } - .buttonStyle(GlassButtonStyle(tint: .accentColor, fontSize: 11)) - } + GridRow(alignment: .firstTextBaseline) { + Text("Remote branch:") + HStack(spacing: 8) { + SearchableReferencePicker( + title: "Remote branch to pull", selection: selectedBranch, + options: remoteBranches, searchPrompt: "Search remote branches", + onSelect: { selectedBranch = $0 }) + .buttonStyle(.bordered) + .help(selectedBranch) + + Button("Refresh", systemImage: "arrow.clockwise") { + Task { await loadRemoteBranches(remote: selectedRemote) } } + .labelStyle(.iconOnly) + .help("Refresh remote branches") } + } - // Local branch - HStack(spacing: 12) { - Text("Pull into local branch:") - .font(.system(size: 13)) - Text(localBranch) - .font(.system(size: 13, weight: .medium)) - Spacer() - } - - // Options - VStack(alignment: .leading, spacing: 10) { - Text("Options") - .font(.system(size: 13, weight: .semibold)) - .padding(.bottom, 2) - - Toggle("Commit merged changes immediately", isOn: $commitMerged) - .font(.system(size: 12)) - - Toggle("Include messages from commits being merged in merge commit", isOn: $includeMessages) - .font(.system(size: 12)) - - Toggle("Create new commit even if fast-forward merge", isOn: $noFastForward) - .font(.system(size: 12)) + GridRow(alignment: .firstTextBaseline) { + Text("Local branch:") + Text(localBranch.isEmpty ? "—" : localBranch) + .fontWeight(.medium) + .lineLimit(2) + .truncationMode(.middle) + .textSelection(.enabled) + .help(localBranch) + .frame(maxWidth: .infinity, alignment: .leading) + } + } - Toggle("Rebase instead of merge (WARNING: make sure you haven't pushed your changes)", isOn: $rebaseInstead) - .font(.system(size: 12)) + GroupBox { + VStack(alignment: .leading, spacing: 12) { + Toggle("Commit merged changes immediately", isOn: $commitMerged) + Toggle("Include merged commit messages", isOn: $includeMessages) + .help("Include messages from commits being merged in the merge commit") + Toggle("Create a merge commit even when fast-forward is possible", isOn: $noFastForward) + VStack(alignment: .leading, spacing: 4) { + Toggle("Rebase instead of merge", isOn: $rebaseInstead) + Text("Only rebase changes you haven’t pushed.") + .font(.callout) + .foregroundStyle(.secondary) + .padding(.leading, 20) } - .padding(12) - .background(.quaternary.opacity(0.3)) - .clipShape(RoundedRectangle(cornerRadius: 8, style: .continuous)) } - .padding(24) + .toggleStyle(.checkbox) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(8) + } label: { + Text("Options") + .bold() } - // Buttons - HStack(spacing: 12) { + HStack(spacing: 8) { + if isLoading { + ProgressView() + .controlSize(.small) + Text("Loading branches…") + .font(.callout) + .foregroundStyle(.secondary) + } Spacer() Button("Cancel", role: .cancel) { dismiss() } .keyboardShortcut(.cancelAction) + .buttonStyle(.bordered) - Button("OK") { + Button("Pull") { dismiss() onPull(selectedRemote, selectedBranch, pullOptions) } .keyboardShortcut(.defaultAction) - .buttonStyle(GlassProminentButtonStyle(tint: .accentColor, fontSize: 13)) + .buttonStyle(.borderedProminent) .disabled(isLoading || selectedRemote.isEmpty || selectedBranch.isEmpty) } - .padding([.horizontal, .bottom], 24) } - .frame(minWidth: 520, idealWidth: 520, maxWidth: 520) - .frame(minHeight: 420, idealHeight: 460) + .font(.body) + .controlSize(.regular) + .padding(24) + .frame(width: 560) + .fixedSize(horizontal: false, vertical: true) .task { await loadData() } diff --git a/macgit/Views/FileStatus/FileStatusNativeList.swift b/macgit/Views/FileStatus/FileStatusNativeList.swift index 9529829..3068a21 100644 --- a/macgit/Views/FileStatus/FileStatusNativeList.swift +++ b/macgit/Views/FileStatus/FileStatusNativeList.swift @@ -101,16 +101,38 @@ struct FileStatusNativeList: NSViewRepresentable { let oldRows = rows let structureChanged = oldRows.map(\.file.id) != model.rows.map(\.file.id) rows = model.rows - if structureChanged || scaleChanged { + if scaleChanged { let anchor = viewportAnchor(in: oldRows) tableView.reloadData() restoreViewport(anchor) return } + if structureChanged { + let anchor = viewportAnchor(in: oldRows) + let diff = rows.map(\.file.id).difference(from: oldRows.map(\.file.id)) + var removed = IndexSet(), inserted = IndexSet() + for change in diff { + switch change { + case .remove(let index, _, _): removed.insert(index) + case .insert(let index, _, _): inserted.insert(index) + } + } + NSAnimationContext.runAnimationGroup { context in + context.duration = 0 + tableView.beginUpdates() + tableView.removeRows(at: removed, withAnimation: []) + tableView.insertRows(at: inserted, withAnimation: []) + tableView.endUpdates() + } + restoreViewport(anchor) + } + let previousRows = Dictionary(uniqueKeysWithValues: oldRows.map { ($0.file.id, $0) }) var changed = IndexSet() - for index in rows.indices where rows[index] != oldRows[index] { - changed.insert(index) + for index in rows.indices { + if let previous = previousRows[rows[index].file.id], previous != rows[index] { + changed.insert(index) + } } if !changed.isEmpty { tableView.reloadData(forRowIndexes: changed, columnIndexes: IndexSet(integer: 0)) diff --git a/macgit/Views/FileStatus/FileStatusView.swift b/macgit/Views/FileStatus/FileStatusView.swift index 2da1482..c519fe6 100644 --- a/macgit/Views/FileStatus/FileStatusView.swift +++ b/macgit/Views/FileStatus/FileStatusView.swift @@ -53,6 +53,7 @@ struct FileStatusView: View { @ObservedObject private var integrationSettings = IntegrationSettingsStore.shared @State private var gitStatus: GitStatus = GitStatus(staged: [], unstaged: [], untracked: []) + @State private var statusLoadGeneration = 0 @State private var stagedLineCounts: [String: FileLineChangeCount] = [:] @State private var changedLineCounts: [String: FileLineChangeCount] = [:] @State private var stagedLineCountRefreshID = UUID() @@ -368,11 +369,8 @@ struct FileStatusView: View { requestRefresh() } } - .onReceive(NotificationCenter.default.publisher(for: .repositoryLocalStateDidRefresh)) { notification in - guard let url = notification.userInfo?["repositoryURL"] as? URL, - url == repositoryURL, - let status = notification.userInfo?["gitStatus"] as? GitStatus - else { return } + .onReceive(syncState.$refreshedLocalStatuses) { snapshots in + guard let status = snapshots[repositoryURL] else { return } applyBackgroundChangedFiles(from: status) } .onReceive(NotificationCenter.default.publisher(for: .repositoryRemoteRefsDidRefresh)) { notification in @@ -1504,6 +1502,8 @@ struct FileStatusView: View { } private func loadStatus() async { + statusLoadGeneration += 1 + let generation = statusLoadGeneration let shouldShowLoading = !hasChanges && !hasInProgressOperation if shouldShowLoading { isLoading = true @@ -1529,12 +1529,17 @@ struct FileStatusView: View { } let paths = Set((loadedStatus.staged + loadedStatus.unstaged + loadedStatus.untracked).map(\.path)) - lfsPaths = (try? await GitStatusService.shared.lfsPaths(Array(paths), in: repositoryURL)) ?? [] - stagedLFSPaths = (try? await GitStatusService.shared.lfsPaths(loadedStatus.staged.map(\.path), cached: true, in: repositoryURL)) ?? [] + let loadedLFSPaths = (try? await GitStatusService.shared.lfsPaths(Array(paths), in: repositoryURL)) ?? [] + let loadedStagedLFSPaths = (try? await GitStatusService.shared.lfsPaths(loadedStatus.staged.map(\.path), cached: true, in: repositoryURL)) ?? [] + let loadedRecentCommits = await GitStatusService.shared.recentCommits(in: repositoryURL) + guard generation == statusLoadGeneration else { return } + if lfsPaths != loadedLFSPaths { lfsPaths = loadedLFSPaths } + if stagedLFSPaths != loadedStagedLFSPaths { stagedLFSPaths = loadedStagedLFSPaths } if gitStatus != loadedStatus { gitStatus = loadedStatus - changedFiles = loadedStatus.unstaged + loadedStatus.untracked } + let loadedChangedFiles = loadedStatus.unstaged + loadedStatus.untracked + if changedFiles != loadedChangedFiles { changedFiles = loadedChangedFiles } // The same paths/statuses can have different content after an edit. // Refresh counts even when the file-list snapshot is unchanged, and // keep the previous badges visible until their replacement arrives. @@ -1542,7 +1547,7 @@ struct FileStatusView: View { changedLineCountRefreshID = UUID() currentBranch = loadedCurrentBranch currentBranchIntegrationStatus = loadedIntegrationStatus - recentCommits = await GitStatusService.shared.recentCommits(in: repositoryURL) + recentCommits = loadedRecentCommits diffRefreshID = UUID() restoreSelectedFileAfterStatusRefresh() @@ -1556,6 +1561,7 @@ struct FileStatusView: View { } onCustomActionSelectionChanged(actionSelection.selectedFiles.map(\.path)) } catch { + guard generation == statusLoadGeneration else { return } errorMessage = error.localizedDescription showingError = true } @@ -1605,10 +1611,16 @@ struct FileStatusView: View { private func applyBackgroundChangedFiles(from status: GitStatus) { let loadedChangedFiles = status.unstaged + status.untracked - guard changedFiles != loadedChangedFiles else { return } - - changedFiles = loadedChangedFiles - changedLineCountRefreshID = UUID() + guard gitStatus != status || changedFiles != loadedChangedFiles else { return } + statusLoadGeneration += 1 + let stagedChanged = gitStatus.staged != status.staged + if gitStatus != status { gitStatus = status } + if stagedChanged { stagedLineCountRefreshID = UUID() } + if changedFiles != loadedChangedFiles { + changedFiles = loadedChangedFiles + changedLineCountRefreshID = UUID() + } + restoreSelectedFileAfterStatusRefresh() if !selectedActionFileKeys.isEmpty { selectedActionFileKeys = actionSelection.prunedSelection @@ -1618,22 +1630,8 @@ struct FileStatusView: View { self.actionSelectionAnchorKey = selectedActionFileKeys.first } - guard let selectedFileKey, !selectedFileKey.isStaged else { return } - if let matchedFile = loadedChangedFiles.first(where: { - FileStatusSelectionKey(file: $0, isStaged: false) == selectedFileKey - }) { - selectedFile = matchedFile - } else if let firstChangedFile = loadedChangedFiles.first { - selectFileForPreview( - firstChangedFile, - selectionKey: FileStatusSelectionKey(file: firstChangedFile, isStaged: false) - ) - } else { - selectedFile = nil - self.selectedFileKey = nil - diffHunks = [] - isLoadingDiff = false - } + // Replace the invalidated full load so ancillary metadata also catches up. + requestRefresh() } private func reloadRepositoryState() async { diff --git a/macgit/Views/MainWindow/SidebarView+Loading.swift b/macgit/Views/MainWindow/SidebarView+Loading.swift index 3f4231c..99d3709 100644 --- a/macgit/Views/MainWindow/SidebarView+Loading.swift +++ b/macgit/Views/MainWindow/SidebarView+Loading.swift @@ -197,7 +197,6 @@ extension SidebarView { let expandedFoldersForLoad = hadLoadedBranches ? expandedFolders.intersection(allFolders) : currentBranchFolders - let branchStructureChanged = branchNodes != tree || currentBranch != current await MainActor.run { guard activeBranchSyncLoadID == loadID else { return } @@ -205,14 +204,15 @@ extension SidebarView { if currentBranch != current { currentBranch = current } if !headHash.isEmpty { headHash = "" } let existingBranches = Set(filteredLocals) - branchSyncStatus = branchSyncStatus.filter { existingBranches.contains($0.key) } + let retainedStatuses = branchSyncStatus.filter { existingBranches.contains($0.key) } + if branchSyncStatus != retainedStatuses { branchSyncStatus = retainedStatuses } if currentBranchIntegrationStatus?.branch != current { currentBranchIntegrationStatus = nil } - if branchStructureChanged { - loadedBranchSyncBranches = [] - syncingBranchSyncBranches = [] - } + // Remote tips can change without changing the local branch tree. + // Every reload starts a new generation of ahead/behind reads. + loadedBranchSyncBranches = [] + syncingBranchSyncBranches = [] // Reveal the current branch only on the initial load. Refreshes // preserve the user's tree state while dropping removed folders. if expandedFolders != expandedFoldersForLoad { expandedFolders = expandedFoldersForLoad } @@ -285,7 +285,7 @@ extension SidebarView { updatedStatuses.removeValue(forKey: branch) } } - branchSyncStatus = updatedStatuses + if branchSyncStatus != updatedStatuses { branchSyncStatus = updatedStatuses } loadedBranchSyncBranches.formUnion(branches) syncingBranchSyncBranches.subtract(branches) } diff --git a/macgit/Views/MainWindow/SidebarView.swift b/macgit/Views/MainWindow/SidebarView.swift index 0399a6e..4f8a541 100644 --- a/macgit/Views/MainWindow/SidebarView.swift +++ b/macgit/Views/MainWindow/SidebarView.swift @@ -730,12 +730,23 @@ struct SidebarView: View { } } } + .onReceive(NotificationCenter.default.publisher(for: .repositoryLocalStateDidRefresh)) { notification in + guard let url = notification.userInfo?["repositoryURL"] as? URL, + url == repositoryURL else { return } + Task { + // Local commits and tracking changes must refresh branch badges + // even when the subsequent automatic fetch cannot reach a remote. + await GitStatusService.shared.invalidateBranchListCache(in: repositoryURL) + await loadBranches(force: true) + } + } .onReceive(NotificationCenter.default.publisher(for: .repositoryRemoteRefsDidRefresh)) { notification in if let url = notification.userInfo?["repositoryURL"] as? URL, - url == repositoryURL, - notification.userInfo?["userInitiated"] as? Bool == true { + url == repositoryURL { Task { - await loadRemotes() + if notification.userInfo?["userInitiated"] as? Bool == true { + await loadRemotes() + } await loadBranches(force: true) } } diff --git a/macgit/Views/PullRequests/PullRequestChangesView.swift b/macgit/Views/PullRequests/PullRequestChangesView.swift index 4adbfe3..4d7a4d9 100644 --- a/macgit/Views/PullRequests/PullRequestChangesView.swift +++ b/macgit/Views/PullRequests/PullRequestChangesView.swift @@ -118,6 +118,7 @@ struct PullRequestChangesView: View { HStack(spacing: 6) { Image(systemName: isExpanded ? "chevron.down" : "chevron.right") Image(systemName: "folder") + .foregroundStyle(.blue) Text(name) .lineLimit(1) } @@ -162,9 +163,14 @@ struct PullRequestChangesView: View { if let additions = file.additions, let deletions = file.deletions { - Text("+\(additions) −\(deletions)") - .font(.system(size: 10, design: .monospaced).scaled(by: textScale)) - .foregroundStyle(.secondary) + HStack(spacing: 4) { + Text("+\(additions)") + .foregroundStyle(.green) + Text("−\(deletions)") + .foregroundStyle(.red) + } + .font(.system(size: 10, design: .monospaced).scaled(by: textScale)) + .fixedSize() } } .padding(.vertical, 2) diff --git a/macgit/Views/PullRequests/PullRequestDetailHeader.swift b/macgit/Views/PullRequests/PullRequestDetailHeader.swift index 132d7e0..89c87e2 100644 --- a/macgit/Views/PullRequests/PullRequestDetailHeader.swift +++ b/macgit/Views/PullRequests/PullRequestDetailHeader.swift @@ -21,7 +21,10 @@ import SwiftUI struct PullRequestDetailHeader: View { @Environment(\.appTextScale) private var textScale let summary: PullRequestSummary - let onClose: () -> Void + let onOpenPullRequest: () -> Void + let onMerge: () -> Void + let isMergeDisabled: Bool + let mergeButtonHelp: String var body: some View { VStack(alignment: .leading, spacing: 12) { @@ -34,10 +37,7 @@ struct PullRequestDetailHeader: View { .font(.title2.scaled(by: textScale)) .foregroundStyle(.secondary) Spacer(minLength: 12) - Button("Close detail", systemImage: "xmark", action: onClose) - .buttonStyle(.borderless) - .labelStyle(.iconOnly) - .help("Close pull request detail") + } HStack(spacing: 8) { @@ -60,17 +60,32 @@ struct PullRequestDetailHeader: View { } HStack(spacing: 7) { - Text(summary.source.ref) - .branchBadge(scale: textScale) - Image(systemName: "arrow.right") - .font(.caption) - .foregroundStyle(.secondary) - .accessibilityLabel("into") - Text(summary.target.ref) - .branchBadge(scale: textScale) + HStack(spacing: 7) { + Text(summary.source.ref) + .branchBadge(scale: textScale) + Image(systemName: "arrow.right") + .font(.caption) + .foregroundStyle(.secondary) + .accessibilityLabel("into") + Text(summary.target.ref) + .branchBadge(scale: textScale) + } + .accessibilityElement(children: .combine) + .accessibilityLabel("From \(summary.source.ref) into \(summary.target.ref)") + + Spacer(minLength: 12) + + HStack(spacing: 8) { + Button("Open PR", systemImage: "safari", action: onOpenPullRequest) + Button("Merge Pull Request", systemImage: "arrow.triangle.merge", action: onMerge) + .buttonStyle(.borderedProminent) + .tint(.green) + .disabled(isMergeDisabled) + .help(mergeButtonHelp) + } + .font(.body.scaled(by: textScale)) + .fixedSize() } - .accessibilityElement(children: .combine) - .accessibilityLabel("From \(summary.source.ref) into \(summary.target.ref)") } .frame(maxWidth: .infinity, alignment: .leading) .padding(.horizontal, 20) diff --git a/macgit/Views/PullRequests/PullRequestListView.swift b/macgit/Views/PullRequests/PullRequestListView.swift index 75d35da..097bf26 100644 --- a/macgit/Views/PullRequests/PullRequestListView.swift +++ b/macgit/Views/PullRequests/PullRequestListView.swift @@ -20,6 +20,7 @@ import MarkdownUI import SwiftUI struct PullRequestListView: View { + @EnvironmentObject private var appState: AppState @Environment(\.appTextScale) private var textScale @ObservedObject var controller: PullRequestController let repositoryURL: URL @@ -29,16 +30,23 @@ struct PullRequestListView: View { var authorizeAction: () async -> Bool = { true } @State private var pendingCommentPullRequest: PullRequestSummary? @State private var selectedPullRequestID: Int? + @State private var detailWindowController = PullRequestDetailWindowController() var body: some View { VStack(spacing: 0) { header Divider() + if controller.hasLoadedList, let error = controller.errorMessage { + Text(error) + .font(.callout.scaled(by: textScale)) + .foregroundStyle(.secondary) + .padding(8) + } - if controller.isLoading { + if controller.isLoading && !controller.hasLoadedList { ProgressView() .frame(maxWidth: .infinity, maxHeight: .infinity) - } else if let errorMessage = controller.errorMessage { + } else if let errorMessage = controller.errorMessage, !controller.hasLoadedList { VStack(spacing: 12) { Text(errorMessage) .font(.headline.scaled(by: textScale)) @@ -74,23 +82,6 @@ struct PullRequestListView: View { .font(.body.scaled(by: textScale)) .foregroundStyle(.secondary) .frame(maxWidth: .infinity, maxHeight: .infinity) - } else if selectedPullRequestID != nil { - PersistentHSplit( - autosaveName: "PullRequestMainSplit", - left: { - pullRequestListPanel - }, - right: { - detailPanel - .frame( - minWidth: 680, - idealWidth: 820, - maxWidth: .infinity, - maxHeight: .infinity, - alignment: .top - ) - } - ) } else { pullRequestListPanel } @@ -118,7 +109,10 @@ struct PullRequestListView: View { } message: { Text(controller.detailErrorMessage ?? "Could not load pull request details.") } - .onDisappear { controller.releaseVisibleData() } + .onDisappear { + closeDetail() + controller.releaseVisibleData() + } .onReceive(NotificationCenter.default.publisher(for: .advancedClearSessionCaches)) { _ in closeDetail() Task { @@ -167,10 +161,26 @@ struct PullRequestListView: View { ) .contentShape(Rectangle()) .onTapGesture { - selectedPullRequestID = item.id - controller.clearSelectedDetail() Task { guard await authorizeAction() else { return } + if selectedPullRequestID == item.id, + detailWindowController.activateIfPresented() { return } + detailWindowController.close() + controller.clearSelectedDetail() + selectedPullRequestID = item.id + detailWindowController.show( + title: "Pull Request #\(item.number) — \(repositoryURL.lastPathComponent)", + content: PullRequestDetailWindowContent( + controller: controller, + selectedPullRequestID: item.id, + authorizeAction: authorizeAction, + onClose: closeDetail + ) + .environmentObject(appState) + .environment(\.appTextScale, textScale) + .sheetPresentationScope(), + onClose: closeDetail + ) await controller.loadPullRequestDetail(item) } } @@ -183,76 +193,15 @@ struct PullRequestListView: View { .frame(minWidth: 280, idealWidth: 360, maxWidth: .infinity) } - @ViewBuilder - private var detailPanel: some View { - if controller.isLoadingDetail, - controller.selectedDetail?.id != selectedPullRequestID { - ProgressView("Loading pull request…") - .font(.body.scaled(by: textScale)) - .frame(maxWidth: .infinity, maxHeight: .infinity) - } else if let detail = controller.selectedDetail, - detail.id == selectedPullRequestID { - PullRequestDetailPane( - detail: detail, - onClose: closeDetail, - onOpenPullRequest: { - Task { - guard await authorizeAction() else { return } - controller.openInBrowser(detail.summary) - } - }, - isRefreshingDetail: controller.isLoadingDetail, - onRefreshDetail: { - Task { - guard await authorizeAction() else { return } - await controller.loadPullRequestDetail(detail.summary, forceRefresh: true) - } - }, - isSubmittingComment: controller.isPerformingAction, - onSubmitComment: { body in - Task { - guard await authorizeAction() else { return } - await controller.comment(on: detail.summary, body: body) - } - }, - changes: controller.selectedChanges, - isLoadingChanges: controller.isLoadingChanges, - changesErrorMessage: controller.changesErrorMessage, - onLoadChanges: { - Task { - guard await authorizeAction() else { return } - await controller.loadPullRequestChanges(detail.summary) - } - }, - onRefreshChanges: { - Task { - guard await authorizeAction() else { return } - await controller.loadPullRequestChanges(detail.summary, forceRefresh: true) - } - }, - isMerging: controller.isPerformingAction, - onMerge: { - Task { - guard await authorizeAction() else { return } - await controller.merge(detail.summary) - } - } - ) - } else { - ProgressView("Loading pull request…") - .font(.body.scaled(by: textScale)) - .frame(maxWidth: .infinity, maxHeight: .infinity) - } - } - private func closeDetail() { + detailWindowController.close() selectedPullRequestID = nil controller.clearSelectedDetail() } private var detailErrorPresented: Binding { Binding( - get: { controller.detailErrorMessage != nil }, + get: { selectedPullRequestID == nil && controller.detailErrorMessage != nil }, set: { isPresented in if !isPresented { controller.detailErrorMessage = nil @@ -348,6 +297,101 @@ struct PullRequestListView: View { } } +private struct PullRequestDetailWindowContent: View { + @Environment(\.appTextScale) private var textScale + @ObservedObject var controller: PullRequestController + let selectedPullRequestID: Int + let authorizeAction: () async -> Bool + let onClose: () -> Void + + var body: some View { + detailPanel + .alert("Pull Request", isPresented: detailErrorPresented) { + Button("OK", role: .cancel) { + controller.detailErrorMessage = nil + if controller.selectedDetail == nil { onClose() } + } + } message: { + Text(controller.detailErrorMessage ?? "Could not load pull request details.") + } + } + + @ViewBuilder + private var detailPanel: some View { + if controller.isLoadingDetail, + controller.selectedDetail?.id != selectedPullRequestID { + ProgressView("Loading pull request…") + .font(.body.scaled(by: textScale)) + .frame(maxWidth: .infinity, maxHeight: .infinity) + } else if let detail = controller.selectedDetail, + detail.id == selectedPullRequestID { + PullRequestDetailPane( + detail: detail, + onOpenPullRequest: { + Task { + guard await authorizeAction() else { return } + controller.openInBrowser(detail.summary) + } + }, + isRefreshingDetail: controller.isLoadingDetail, + onRefreshDetail: { + Task { + guard await authorizeAction() else { return } + await controller.loadPullRequestDetail(detail.summary, forceRefresh: true) + } + }, + isSubmittingComment: controller.isPerformingAction, + onSubmitComment: { body in + Task { + guard await authorizeAction() else { return } + await controller.comment(on: detail.summary, body: body) + } + }, + changes: controller.selectedChanges, + isLoadingChanges: controller.isLoadingChanges, + changesErrorMessage: controller.changesErrorMessage, + onLoadChanges: { + Task { + guard await authorizeAction() else { return } + await controller.loadPullRequestChanges(detail.summary) + } + }, + onRefreshChanges: { + Task { + guard await authorizeAction() else { return } + await controller.loadPullRequestChanges(detail.summary, forceRefresh: true) + } + }, + isMerging: controller.isPerformingAction, + onMerge: { + Task { + guard await authorizeAction() else { return } + await controller.merge(detail.summary) + } + } + ) + } else if controller.detailErrorMessage != nil { + ContentUnavailableView( + "Could not load pull request", + systemImage: "exclamationmark.triangle", + description: Text("Close this window and select the pull request to try again.") + ) + .frame(maxWidth: .infinity, maxHeight: .infinity) + } else { + ProgressView("Loading pull request…") + .font(.body.scaled(by: textScale)) + .frame(maxWidth: .infinity, maxHeight: .infinity) + } + } + + private var detailErrorPresented: Binding { + Binding( + get: { controller.detailErrorMessage != nil }, + set: { if !$0 { controller.detailErrorMessage = nil } } + ) + } +} + private struct PullRequestRow: View { @Environment(\.appTextScale) private var textScale let summary: PullRequestSummary @@ -502,7 +546,6 @@ private struct PullRequestRow: View { private struct PullRequestDetailPane: View { @Environment(\.appTextScale) private var textScale let detail: PullRequestDetail - let onClose: () -> Void let onOpenPullRequest: () -> Void let isRefreshingDetail: Bool let onRefreshDetail: () -> Void @@ -523,7 +566,13 @@ private struct PullRequestDetailPane: View { var body: some View { VStack(spacing: 0) { - PullRequestDetailHeader(summary: detail.summary, onClose: onClose) + PullRequestDetailHeader( + summary: detail.summary, + onOpenPullRequest: onOpenPullRequest, + onMerge: { isConfirmingMerge = true }, + isMergeDisabled: !canMerge || isMerging, + mergeButtonHelp: mergeButtonHelp + ) PullRequestDetailTabBar(selection: $selectedTab) if selectedTab == .overview { @@ -537,7 +586,6 @@ private struct PullRequestDetailPane: View { ) } - footer } .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top) .onChange(of: isCommentBarExpanded) { _, isExpanded in @@ -569,104 +617,84 @@ private struct PullRequestDetailPane: View { } private var overviewContent: some View { - HStack(spacing: 0) { - VStack(spacing: 0) { - ScrollView { - VStack(alignment: .leading, spacing: 14) { - PullRequestConversationBlock( - author: detail.summary.author, - date: detail.summary.createdAt, - action: "opened this pull request" - ) { - if detail.body.isEmpty { - Text("No description provided.") - .font(.body.scaled(by: textScale)) - .italic() - .foregroundStyle(.secondary) - } else { - Markdown(detail.body) - .markdownTheme(.gitHub) - .markdownTextStyle { FontSize(.em(textScale)) } - .textSelection(.enabled) - } - } - - ForEach(detail.comments) { comment in + PersistentHSplit( + autosaveName: "PullRequestOverviewSplit", + minimumLeftWidth: 440, + minimumRightWidth: 180, + left: { + VStack(spacing: 0) { + ScrollView { + VStack(alignment: .leading, spacing: 14) { PullRequestConversationBlock( - author: comment.author, - date: comment.createdAt, - action: "commented" + author: detail.summary.author, + date: detail.summary.createdAt, + action: "opened this pull request" ) { - Markdown(comment.body) - .markdownTheme(.gitHub) - .markdownTextStyle { FontSize(.em(textScale)) } - .textSelection(.enabled) + if detail.body.isEmpty { + Text("No description provided.") + .font(.body.scaled(by: textScale)) + .italic() + .foregroundStyle(.secondary) + } else { + Markdown(detail.body) + .markdownTheme(.gitHub) + .markdownTextStyle { FontSize(.em(textScale)) } + .textSelection(.enabled) + } } - } - if detail.comments.isEmpty { - Text("No comments yet") - .font(.subheadline.scaled(by: textScale)) - .foregroundStyle(.secondary) - .frame(maxWidth: .infinity, alignment: .center) - .padding(.vertical, 8) - } + ForEach(detail.comments) { comment in + PullRequestConversationBlock( + author: comment.author, + date: comment.createdAt, + action: "commented" + ) { + Markdown(comment.body) + .markdownTheme(.gitHub) + .markdownTextStyle { FontSize(.em(textScale)) } + .textSelection(.enabled) + } + } + + if detail.comments.isEmpty { + Text("No comments yet") + .font(.subheadline.scaled(by: textScale)) + .foregroundStyle(.secondary) + .frame(maxWidth: .infinity, alignment: .center) + .padding(.vertical, 8) + } - Button(action: onRefreshDetail) { - if isRefreshingDetail { - ProgressView() - .controlSize(.small) - } else { - Label("Refresh comments", systemImage: "arrow.clockwise") + Button(action: onRefreshDetail) { + if isRefreshingDetail { + ProgressView() + .controlSize(.small) + } else { + Label("Refresh comments", systemImage: "arrow.clockwise") + } } + .buttonStyle(.borderless) + .font(.body.scaled(by: textScale)) + .foregroundStyle(.secondary) + .disabled(isRefreshingDetail) + .frame(maxWidth: .infinity, alignment: .center) } - .buttonStyle(.borderless) - .font(.body.scaled(by: textScale)) - .foregroundStyle(.secondary) - .disabled(isRefreshingDetail) - .frame(maxWidth: .infinity, alignment: .center) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(20) } - .frame(maxWidth: .infinity, alignment: .leading) - .padding(20) - } - - commentComposer - } - .frame(minWidth: 440, maxWidth: .infinity, maxHeight: .infinity) - Divider() - - PullRequestMetadataSidebar( - reviewers: detail.reviewers, - assignees: detail.assignees - ) - .frame(width: 220) - .frame(maxHeight: .infinity) - } - } + commentComposer + } + .frame(minWidth: 440, maxWidth: .infinity, maxHeight: .infinity) - private var footer: some View { - HStack { - Button("Open PR", systemImage: "safari", action: onOpenPullRequest) - Button("Merge Pull Request", systemImage: "arrow.triangle.merge") { - isConfirmingMerge = true + }, + right: { + PullRequestMetadataSidebar( + reviewers: detail.reviewers, + assignees: detail.assignees + ) + .frame(minWidth: 180, idealWidth: 220, maxWidth: .infinity, maxHeight: .infinity) } - .buttonStyle(.borderedProminent) - .tint(.green) - .disabled(!canMerge || isMerging) - .help(mergeButtonHelp) - Spacer() - Button("Close", action: onClose) - .keyboardShortcut(.cancelAction) - } - .font(.body.scaled(by: textScale)) - .padding(16) - .background(Color(nsColor: .controlBackgroundColor)) - .overlay(alignment: .top) { - Rectangle() - .fill(.separator) - .frame(height: 0.5) - } + ) } private var canMerge: Bool { diff --git a/macgitTests/GitFlowLitePhase1Tests.swift b/macgitTests/GitFlowLitePhase1Tests.swift index c5f0433..f3b179c 100644 --- a/macgitTests/GitFlowLitePhase1Tests.swift +++ b/macgitTests/GitFlowLitePhase1Tests.swift @@ -38,6 +38,13 @@ final class GitFlowLitePhase1Tests: XCTestCase { XCTAssertEqual(GitBranchNameSanitizer.sanitize("Feature--API"), "Feature--API") } + func testBranchNameSanitizerRemovesExposedForbiddenSuffixes() { + XCTAssertEqual(GitBranchNameSanitizer.sanitize("feature.lock."), "feature") + XCTAssertEqual(GitBranchNameSanitizer.sanitize("feature.lock.lock."), "feature") + XCTAssertEqual(GitBranchNameSanitizer.sanitize("feature.lock.lock"), "feature") + XCTAssertEqual(GitBranchNameSanitizer.sanitize("feature.lock./child.lock."), "feature/child") + } + func testPlannerMapsTopicKindsToExpectedBaseAndPrefix() throws { let configuration = GitFlowConfiguration(isEnabled: true) let planner = GitFlowPlanner()