From 6717901daca1775ce108d040f96e925e423efe5e Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 12 Oct 2026 08:22:11 +0700 Subject: [PATCH 1/4] feat: Add configurable diff font, colors, and syntax highlighting Persist diff appearance settings locally and in Firestore, expose them through a new diff settings view, and apply them in the diff renderer. --- macgit/App/AppState.swift | 50 +++- macgit/Models/AppSettingsSnapshot.swift | 40 +++- .../Models/DiffAppearanceSettingsDraft.swift | 17 ++ .../DiffAppearanceSettingsStore.swift | 222 ++++++++++++++++++ macgit/Services/FirestoreSettingsStore.swift | 78 +++++- .../Views/Common/AdvancedSettingsView.swift | 6 - .../Views/Common/AppSettingsDetailView.swift | 7 + macgit/Views/Common/AppSettingsSection.swift | 6 + macgit/Views/Common/AppSettingsView.swift | 16 +- macgit/Views/Common/Diff/DiffHunkCanvas.swift | 14 +- .../Common/Diff/DiffNativeTextStore.swift | 33 ++- macgit/Views/Common/DiffHunkGeometry.swift | 4 +- macgit/Views/Common/DiffNativeTable.swift | 23 +- macgit/Views/Common/DiffSettingsView.swift | 167 +++++++++++++ macgit/Views/Common/DiffView.swift | 24 +- macgitTests/AppSettingsSnapshotTests.swift | 9 +- macgitTests/CloudSettingsDocumentTests.swift | 58 ++++- macgitTests/DiffNativeTableTests.swift | 1 + 18 files changed, 730 insertions(+), 45 deletions(-) create mode 100644 macgit/Models/DiffAppearanceSettingsDraft.swift create mode 100644 macgit/Services/DiffAppearanceSettingsStore.swift create mode 100644 macgit/Views/Common/DiffSettingsView.swift diff --git a/macgit/App/AppState.swift b/macgit/App/AppState.swift index 49438fb3..c528ad25 100644 --- a/macgit/App/AppState.swift +++ b/macgit/App/AppState.swift @@ -51,6 +51,8 @@ final class AppState: ObservableObject { private static let preferredSearchFileApplicationKey = "preferredSearchFileApplication" private let userDefaults: UserDefaults + private let diffAppearanceSettingsStore: DiffAppearanceSettingsStore + private let advancedSettingsStore: AdvancedSettingsStore private var isApplyingSnapshot = false @Published private var currentSettingsSnapshot: AppSettingsSnapshot = AppSettingsSnapshot(showToolbarButtonText: true, showSubmodules: false, showSubtrees: false) @@ -286,8 +288,20 @@ final class AppState: ObservableObject { .eraseToAnyPublisher() } - init(userDefaults: UserDefaults = .standard) { + init( + userDefaults: UserDefaults = .standard, + diffAppearanceSettingsStore: DiffAppearanceSettingsStore? = nil, + advancedSettingsStore: AdvancedSettingsStore? = nil + ) { self.userDefaults = userDefaults + self.diffAppearanceSettingsStore = diffAppearanceSettingsStore + ?? (userDefaults === UserDefaults.standard + ? .shared + : DiffAppearanceSettingsStore(userDefaults: userDefaults)) + self.advancedSettingsStore = advancedSettingsStore + ?? (userDefaults === UserDefaults.standard + ? .shared + : AdvancedSettingsStore(userDefaults: userDefaults)) let appearance = userDefaults.string(forKey: Self.appearanceKey) .flatMap(AppAppearance.init(rawValue:)) ?? .system let textSize = userDefaults.string(forKey: Self.textSizeKey) @@ -325,6 +339,7 @@ final class AppState: ObservableObject { let preferredSearchFileApplicationBundleIdentifier = userDefaults.string( forKey: Self.preferredSearchFileApplicationKey ) + let diffAppearance = self.diffAppearanceSettingsStore.snapshotValues let loadedSnapshot = AppSettingsSnapshot( appearance: appearance, showToolbarButtonText: showToolbarButtonText, @@ -349,7 +364,14 @@ final class AppState: ObservableObject { historyBranchFilter: historyBranchFilter, historyIncludeRemotes: historyIncludeRemotes, autoFetchEnabled: autoFetchEnabled, - refreshOnAppActive: refreshOnAppActive + refreshOnAppActive: refreshOnAppActive, + diffFontName: diffAppearance.fontName, + diffFontSize: diffAppearance.fontSize, + diffAddedBackgroundColor: diffAppearance.addedBackgroundColor, + diffAddedTextColor: diffAppearance.addedTextColor, + diffRemovedBackgroundColor: diffAppearance.removedBackgroundColor, + diffRemovedTextColor: diffAppearance.removedTextColor, + diffSyntaxHighlighting: self.advancedSettingsStore.diffSyntaxHighlighting ).normalizedRepositoryToolbarShortcuts() self.appearance = appearance @@ -385,7 +407,8 @@ final class AppState: ObservableObject { } var snapshot: AppSettingsSnapshot { - AppSettingsSnapshot( + let diffAppearance = diffAppearanceSettingsStore.snapshotValues + return AppSettingsSnapshot( appearance: appearance, showToolbarButtonText: showToolbarButtonText, showGitFlow: showGitFlow, @@ -409,10 +432,27 @@ final class AppState: ObservableObject { historyBranchFilter: historyBranchFilter, historyIncludeRemotes: historyIncludeRemotes, autoFetchEnabled: autoFetchEnabled, - refreshOnAppActive: refreshOnAppActive + refreshOnAppActive: refreshOnAppActive, + diffFontName: diffAppearance.fontName, + diffFontSize: diffAppearance.fontSize, + diffAddedBackgroundColor: diffAppearance.addedBackgroundColor, + diffAddedTextColor: diffAppearance.addedTextColor, + diffRemovedBackgroundColor: diffAppearance.removedBackgroundColor, + diffRemovedTextColor: diffAppearance.removedTextColor, + diffSyntaxHighlighting: advancedSettingsStore.diffSyntaxHighlighting ) } + func applyLocalDiffSettings( + appearance: DiffAppearanceSettingsDraft, + syntaxHighlighting: Bool + ) { + diffAppearanceSettingsStore.apply(appearance) + advancedSettingsStore.diffSyntaxHighlighting = syntaxHighlighting + guard !isApplyingSnapshot else { return } + currentSettingsSnapshot = snapshot + } + func apply(_ snapshot: AppSettingsSnapshot) { let snapshot = snapshot.normalizedRepositoryToolbarShortcuts() isApplyingSnapshot = true @@ -441,6 +481,8 @@ final class AppState: ObservableObject { historyIncludeRemotes = snapshot.historyIncludeRemotes autoFetchEnabled = snapshot.autoFetchEnabled refreshOnAppActive = snapshot.refreshOnAppActive + diffAppearanceSettingsStore.apply(snapshot) + advancedSettingsStore.diffSyntaxHighlighting = snapshot.diffSyntaxHighlighting currentSettingsSnapshot = snapshot } diff --git a/macgit/Models/AppSettingsSnapshot.swift b/macgit/Models/AppSettingsSnapshot.swift index b4f7c68b..af2504c6 100644 --- a/macgit/Models/AppSettingsSnapshot.swift +++ b/macgit/Models/AppSettingsSnapshot.swift @@ -44,6 +44,13 @@ struct AppSettingsSnapshot: Codable, Equatable, Sendable { var historyIncludeRemotes: Bool var autoFetchEnabled: Bool var refreshOnAppActive: Bool + var diffFontName: String + var diffFontSize: Double + var diffAddedBackgroundColor: String + var diffAddedTextColor: String + var diffRemovedBackgroundColor: String + var diffRemovedTextColor: String + var diffSyntaxHighlighting: Bool init( appearance: AppAppearance = .system, @@ -69,7 +76,14 @@ struct AppSettingsSnapshot: Codable, Equatable, Sendable { historyBranchFilter: HistoryBranchFilter = .all, historyIncludeRemotes: Bool = false, autoFetchEnabled: Bool = false, - refreshOnAppActive: Bool = true + refreshOnAppActive: Bool = true, + diffFontName: String = "Menlo-Regular", + diffFontSize: Double = 12, + diffAddedBackgroundColor: String = "#E6FAF0FF", + diffAddedTextColor: String = "#006E4FFF", + diffRemovedBackgroundColor: String = "#FFEDE8FF", + diffRemovedTextColor: String = "#C72E0DFF", + diffSyntaxHighlighting: Bool = false ) { schemaVersion = 1 self.appearance = appearance @@ -96,6 +110,13 @@ struct AppSettingsSnapshot: Codable, Equatable, Sendable { self.historyIncludeRemotes = historyIncludeRemotes self.autoFetchEnabled = autoFetchEnabled self.refreshOnAppActive = refreshOnAppActive + self.diffFontName = diffFontName + self.diffFontSize = diffFontSize + self.diffAddedBackgroundColor = diffAddedBackgroundColor + self.diffAddedTextColor = diffAddedTextColor + self.diffRemovedBackgroundColor = diffRemovedBackgroundColor + self.diffRemovedTextColor = diffRemovedTextColor + self.diffSyntaxHighlighting = diffSyntaxHighlighting } init(from decoder: Decoder) throws { @@ -125,5 +146,22 @@ struct AppSettingsSnapshot: Codable, Equatable, Sendable { historyIncludeRemotes = try container.decode(Bool.self, forKey: .historyIncludeRemotes) autoFetchEnabled = try container.decode(Bool.self, forKey: .autoFetchEnabled) refreshOnAppActive = try container.decode(Bool.self, forKey: .refreshOnAppActive) + diffFontName = try container.decodeIfPresent(String.self, forKey: .diffFontName) ?? "Menlo-Regular" + diffFontSize = try container.decodeIfPresent(Double.self, forKey: .diffFontSize) ?? 12 + diffAddedBackgroundColor = try container.decodeIfPresent( + String.self, forKey: .diffAddedBackgroundColor + ) ?? "#E6FAF0FF" + diffAddedTextColor = try container.decodeIfPresent( + String.self, forKey: .diffAddedTextColor + ) ?? "#006E4FFF" + diffRemovedBackgroundColor = try container.decodeIfPresent( + String.self, forKey: .diffRemovedBackgroundColor + ) ?? "#FFEDE8FF" + diffRemovedTextColor = try container.decodeIfPresent( + String.self, forKey: .diffRemovedTextColor + ) ?? "#C72E0DFF" + diffSyntaxHighlighting = try container.decodeIfPresent( + Bool.self, forKey: .diffSyntaxHighlighting + ) ?? false } } diff --git a/macgit/Models/DiffAppearanceSettingsDraft.swift b/macgit/Models/DiffAppearanceSettingsDraft.swift new file mode 100644 index 00000000..8f0b88a4 --- /dev/null +++ b/macgit/Models/DiffAppearanceSettingsDraft.swift @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import AppKit + +struct DiffAppearanceSettingsDraft { + var fontName: String + var fontSize: Double + var addedBackgroundColor: NSColor + var addedTextColor: NSColor + var removedBackgroundColor: NSColor + var removedTextColor: NSColor + + var font: NSFont { + NSFont(name: fontName, size: fontSize) + ?? NSFont.monospacedSystemFont(ofSize: fontSize, weight: .regular) + } +} diff --git a/macgit/Services/DiffAppearanceSettingsStore.swift b/macgit/Services/DiffAppearanceSettingsStore.swift new file mode 100644 index 00000000..b5f70f5e --- /dev/null +++ b/macgit/Services/DiffAppearanceSettingsStore.swift @@ -0,0 +1,222 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import AppKit +import Combine +import Foundation + +@MainActor +final class DiffAppearanceSettingsStore: ObservableObject { + static let shared = DiffAppearanceSettingsStore() + + static let defaultFontName = "Menlo-Regular" + static let defaultFontSize = 12.0 + + @Published var fontName: String { + didSet { + userDefaults.set(fontName, forKey: Keys.fontName) + revision += 1 + } + } + + @Published var fontSize: Double { + didSet { + userDefaults.set(fontSize, forKey: Keys.fontSize) + revision += 1 + } + } + + @Published var addedBackgroundColor: NSColor { + didSet { + save(addedBackgroundColor, forKey: Keys.addedBackgroundColor) + revision += 1 + } + } + + @Published var addedTextColor: NSColor { + didSet { + save(addedTextColor, forKey: Keys.addedTextColor) + revision += 1 + } + } + + @Published var removedBackgroundColor: NSColor { + didSet { + save(removedBackgroundColor, forKey: Keys.removedBackgroundColor) + revision += 1 + } + } + + @Published var removedTextColor: NSColor { + didSet { + save(removedTextColor, forKey: Keys.removedTextColor) + revision += 1 + } + } + + @Published private(set) var revision = 0 + + var font: NSFont { + NSFont(name: fontName, size: fontSize) + ?? NSFont.monospacedSystemFont(ofSize: fontSize, weight: .regular) + } + + var draft: DiffAppearanceSettingsDraft { + DiffAppearanceSettingsDraft( + fontName: fontName, + fontSize: fontSize, + addedBackgroundColor: addedBackgroundColor, + addedTextColor: addedTextColor, + removedBackgroundColor: removedBackgroundColor, + removedTextColor: removedTextColor + ) + } + + var snapshotValues: ( + fontName: String, + fontSize: Double, + addedBackgroundColor: String, + addedTextColor: String, + removedBackgroundColor: String, + removedTextColor: String + ) { + ( + fontName, + fontSize, + Self.hexString(from: addedBackgroundColor), + Self.hexString(from: addedTextColor), + Self.hexString(from: removedBackgroundColor), + Self.hexString(from: removedTextColor) + ) + } + + private enum Keys { + static let fontName = "diff.appearance.fontName" + static let fontSize = "diff.appearance.fontSize" + static let addedBackgroundColor = "diff.appearance.addedBackgroundColor" + static let addedTextColor = "diff.appearance.addedTextColor" + static let removedBackgroundColor = "diff.appearance.removedBackgroundColor" + static let removedTextColor = "diff.appearance.removedTextColor" + } + + private enum Defaults { + static let addedBackgroundColor = NSColor( + srgbRed: 0.90, green: 0.98, blue: 0.94, alpha: 1 + ) + static let addedTextColor = NSColor( + srgbRed: 0.00, green: 0.43, blue: 0.31, alpha: 1 + ) + static let removedBackgroundColor = NSColor( + srgbRed: 1.00, green: 0.93, blue: 0.91, alpha: 1 + ) + static let removedTextColor = NSColor( + srgbRed: 0.78, green: 0.18, blue: 0.05, alpha: 1 + ) + } + + private let userDefaults: UserDefaults + + init(userDefaults: UserDefaults = .standard) { + self.userDefaults = userDefaults + fontName = userDefaults.string(forKey: Keys.fontName) ?? Self.defaultFontName + let storedFontSize = userDefaults.object(forKey: Keys.fontSize) as? Double + fontSize = storedFontSize.map { min(max($0, 8), 32) } ?? Self.defaultFontSize + addedBackgroundColor = Self.color( + storedIn: userDefaults, key: Keys.addedBackgroundColor, + fallback: Defaults.addedBackgroundColor + ) + addedTextColor = Self.color( + storedIn: userDefaults, key: Keys.addedTextColor, + fallback: Defaults.addedTextColor + ) + removedBackgroundColor = Self.color( + storedIn: userDefaults, key: Keys.removedBackgroundColor, + fallback: Defaults.removedBackgroundColor + ) + removedTextColor = Self.color( + storedIn: userDefaults, key: Keys.removedTextColor, + fallback: Defaults.removedTextColor + ) + } + + func restoreDefaults() { + apply(Self.defaultDraft) + } + + func apply(_ draft: DiffAppearanceSettingsDraft) { + fontName = draft.fontName + fontSize = min(max(draft.fontSize, 8), 32) + addedBackgroundColor = draft.addedBackgroundColor + addedTextColor = draft.addedTextColor + removedBackgroundColor = draft.removedBackgroundColor + removedTextColor = draft.removedTextColor + } + + func apply(_ snapshot: AppSettingsSnapshot) { + apply( + DiffAppearanceSettingsDraft( + fontName: snapshot.diffFontName, + fontSize: snapshot.diffFontSize, + addedBackgroundColor: Self.color( + hexString: snapshot.diffAddedBackgroundColor, + fallback: Defaults.addedBackgroundColor + ), + addedTextColor: Self.color( + hexString: snapshot.diffAddedTextColor, + fallback: Defaults.addedTextColor + ), + removedBackgroundColor: Self.color( + hexString: snapshot.diffRemovedBackgroundColor, + fallback: Defaults.removedBackgroundColor + ), + removedTextColor: Self.color( + hexString: snapshot.diffRemovedTextColor, + fallback: Defaults.removedTextColor + ) + ) + ) + } + + static var defaultDraft: DiffAppearanceSettingsDraft { + DiffAppearanceSettingsDraft( + fontName: defaultFontName, + fontSize: defaultFontSize, + addedBackgroundColor: Defaults.addedBackgroundColor, + addedTextColor: Defaults.addedTextColor, + removedBackgroundColor: Defaults.removedBackgroundColor, + removedTextColor: Defaults.removedTextColor + ) + } + + private func save(_ color: NSColor, forKey key: String) { + guard let data = try? NSKeyedArchiver.archivedData( + withRootObject: color, requiringSecureCoding: true + ) else { return } + userDefaults.set(data, forKey: key) + } + + private static func color(storedIn defaults: UserDefaults, key: String, fallback: NSColor) -> NSColor { + guard let data = defaults.data(forKey: key), + let color = try? NSKeyedUnarchiver.unarchivedObject( + ofClass: NSColor.self, from: data + ) else { return fallback } + return color + } + + private static func hexString(from color: NSColor) -> String { + guard let rgb = color.usingColorSpace(.sRGB) else { return "#000000FF" } + let components = [rgb.redComponent, rgb.greenComponent, rgb.blueComponent, rgb.alphaComponent] + .map { Int(round(min(max($0, 0), 1) * 255)) } + return String(format: "#%02X%02X%02X%02X", arguments: components) + } + + private static func color(hexString: String, fallback: NSColor) -> NSColor { + let value = hexString.hasPrefix("#") ? String(hexString.dropFirst()) : hexString + guard value.count == 8, let rgba = UInt64(value, radix: 16) else { return fallback } + return NSColor( + srgbRed: CGFloat((rgba >> 24) & 0xFF) / 255, + green: CGFloat((rgba >> 16) & 0xFF) / 255, + blue: CGFloat((rgba >> 8) & 0xFF) / 255, + alpha: CGFloat(rgba & 0xFF) / 255 + ) + } +} diff --git a/macgit/Services/FirestoreSettingsStore.swift b/macgit/Services/FirestoreSettingsStore.swift index 7b50cb1a..f3d238bf 100644 --- a/macgit/Services/FirestoreSettingsStore.swift +++ b/macgit/Services/FirestoreSettingsStore.swift @@ -50,6 +50,13 @@ enum CloudSettingsDocument { "historyIncludeRemotes": snapshot.historyIncludeRemotes, "autoFetchEnabled": snapshot.autoFetchEnabled, "refreshOnAppActive": snapshot.refreshOnAppActive, + "diffFontName": snapshot.diffFontName, + "diffFontSize": snapshot.diffFontSize, + "diffAddedBackgroundColor": snapshot.diffAddedBackgroundColor, + "diffAddedTextColor": snapshot.diffAddedTextColor, + "diffRemovedBackgroundColor": snapshot.diffRemovedBackgroundColor, + "diffRemovedTextColor": snapshot.diffRemovedTextColor, + "diffSyntaxHighlighting": snapshot.diffSyntaxHighlighting, "updatedAt": updatedAt ] } @@ -77,6 +84,29 @@ enum CloudSettingsDocument { appearance = .system } + let diffFontName = try optionalString( + data, key: "diffFontName", defaultValue: "Menlo-Regular" + ) { !$0.isEmpty && $0.count <= 128 } + let diffFontSize = try optionalNumber(data, key: "diffFontSize", defaultValue: 12) + guard (8...32).contains(diffFontSize) else { + throw CloudSettingsError.invalidDocument + } + let diffAddedBackgroundColor = try optionalColor( + data, key: "diffAddedBackgroundColor", defaultValue: "#E6FAF0FF" + ) + let diffAddedTextColor = try optionalColor( + data, key: "diffAddedTextColor", defaultValue: "#006E4FFF" + ) + let diffRemovedBackgroundColor = try optionalColor( + data, key: "diffRemovedBackgroundColor", defaultValue: "#FFEDE8FF" + ) + let diffRemovedTextColor = try optionalColor( + data, key: "diffRemovedTextColor", defaultValue: "#C72E0DFF" + ) + let diffSyntaxHighlighting = try optionalBool( + data, key: "diffSyntaxHighlighting", defaultValue: false + ) + return AppSettingsSnapshot( appearance: appearance, showToolbarButtonText: showToolbarButtonText, @@ -102,9 +132,55 @@ enum CloudSettingsDocument { .flatMap(HistoryBranchFilter.init(storageValue:)) ?? .all, historyIncludeRemotes: data["historyIncludeRemotes"] as? Bool ?? false, autoFetchEnabled: data["autoFetchEnabled"] as? Bool ?? false, - refreshOnAppActive: data["refreshOnAppActive"] as? Bool ?? true + refreshOnAppActive: data["refreshOnAppActive"] as? Bool ?? true, + diffFontName: diffFontName, + diffFontSize: diffFontSize, + diffAddedBackgroundColor: diffAddedBackgroundColor, + diffAddedTextColor: diffAddedTextColor, + diffRemovedBackgroundColor: diffRemovedBackgroundColor, + diffRemovedTextColor: diffRemovedTextColor, + diffSyntaxHighlighting: diffSyntaxHighlighting ) } + + private static func optionalString( + _ data: [String: Any], + key: String, + defaultValue: String, + isValid: (String) -> Bool + ) throws -> String { + guard let value = data[key] else { return defaultValue } + guard let value = value as? String, isValid(value) else { + throw CloudSettingsError.invalidDocument + } + return value + } + + private static func optionalNumber( + _ data: [String: Any], key: String, defaultValue: Double + ) throws -> Double { + guard let value = data[key] else { return defaultValue } + guard !(value is Bool), let number = value as? NSNumber else { + throw CloudSettingsError.invalidDocument + } + return number.doubleValue + } + + private static func optionalBool( + _ data: [String: Any], key: String, defaultValue: Bool + ) throws -> Bool { + guard let value = data[key] else { return defaultValue } + guard let value = value as? Bool else { throw CloudSettingsError.invalidDocument } + return value + } + + private static func optionalColor( + _ data: [String: Any], key: String, defaultValue: String + ) throws -> String { + try optionalString(data, key: key, defaultValue: defaultValue) { + $0.range(of: "^#[0-9A-F]{8}$", options: .regularExpression) != nil + } + } } @MainActor diff --git a/macgit/Views/Common/AdvancedSettingsView.swift b/macgit/Views/Common/AdvancedSettingsView.swift index 75c29fe8..6f191cdf 100644 --- a/macgit/Views/Common/AdvancedSettingsView.swift +++ b/macgit/Views/Common/AdvancedSettingsView.swift @@ -74,12 +74,6 @@ struct AdvancedSettingsView: View { } Section { - SettingsToggleRow( - title: "Syntax highlighting in diffs", - detail: "Color code syntax in diff views. This uses more CPU and memory, especially for large changes, so it is off by default.", - isOn: $settings.diffSyntaxHighlighting - ) - LabeledContent("History load size") { Picker("History load size", selection: $settings.historyLoadSize) { ForEach(HistoryLoadSize.allCases) { size in diff --git a/macgit/Views/Common/AppSettingsDetailView.swift b/macgit/Views/Common/AppSettingsDetailView.swift index dca24d58..6990b1d9 100644 --- a/macgit/Views/Common/AppSettingsDetailView.swift +++ b/macgit/Views/Common/AppSettingsDetailView.swift @@ -27,6 +27,8 @@ struct AppSettingsDetailView: View { @ObservedObject var customActionStore: CustomActionStore let restrictedAIProviderAccess: FeatureAccessDecision @Binding var aiProviderDrafts: [AIProviderConfigurationDraft] + @Binding var diffAppearanceDraft: DiffAppearanceSettingsDraft + @Binding var diffSyntaxHighlightingDraft: Bool var body: some View { switch section { @@ -34,6 +36,11 @@ struct AppSettingsDetailView: View { GeneralSettingsView(appState: appState) case .appearance: AppearanceSettingsView(appState: appState) + case .diff: + DiffSettingsView( + draft: $diffAppearanceDraft, + syntaxHighlighting: $diffSyntaxHighlightingDraft + ) case .git: GitSettingsView() case .accounts: diff --git a/macgit/Views/Common/AppSettingsSection.swift b/macgit/Views/Common/AppSettingsSection.swift index 4325165f..37c57fca 100644 --- a/macgit/Views/Common/AppSettingsSection.swift +++ b/macgit/Views/Common/AppSettingsSection.swift @@ -20,6 +20,7 @@ import SwiftUI enum AppSettingsSection: String, CaseIterable, Identifiable { case general case appearance + case diff case git case accounts case connections @@ -35,6 +36,7 @@ enum AppSettingsSection: String, CaseIterable, Identifiable { switch self { case .general: "General" case .appearance: "Appearance" + case .diff: "Diff" case .git: "Git" case .accounts: "Account" case .connections: "Connections" @@ -50,6 +52,7 @@ enum AppSettingsSection: String, CaseIterable, Identifiable { switch self { case .general: "gearshape" case .appearance: "paintbrush" + case .diff: "plusminus" case .git: "point.3.connected.trianglepath.dotted" case .accounts: "person.crop.circle" case .connections: "network" @@ -65,6 +68,7 @@ enum AppSettingsSection: String, CaseIterable, Identifiable { switch self { case .general: .blue case .appearance: .orange + case .diff: .pink case .git: .green case .accounts: .indigo case .connections: .blue @@ -82,6 +86,8 @@ enum AppSettingsSection: String, CaseIterable, Identifiable { "Configure the default behavior of Commit+." case .appearance: "Customize how Commit+ looks and presents repository information." + case .diff: + "Customize the font and colors used to display code changes." case .git: "Manage global Git behavior and command preferences." case .accounts: diff --git a/macgit/Views/Common/AppSettingsView.swift b/macgit/Views/Common/AppSettingsView.swift index 0ae92f4f..bfd2c588 100644 --- a/macgit/Views/Common/AppSettingsView.swift +++ b/macgit/Views/Common/AppSettingsView.swift @@ -29,6 +29,8 @@ struct AppSettingsView: View { @ObservedObject var customActionStore: CustomActionStore @Binding private var selectedSection: AppSettingsSection @State private var aiProviderDrafts: [AIProviderConfigurationDraft] + @State private var diffAppearanceDraft: DiffAppearanceSettingsDraft + @State private var diffSyntaxHighlightingDraft: Bool @State private var saveErrorMessage: String? @State private var isShowingSaveError = false @@ -51,6 +53,10 @@ struct AppSettingsView: View { self.customActionStore = customActionStore _selectedSection = selectedSection _aiProviderDrafts = State(initialValue: aiProviderController.configurationDrafts()) + _diffAppearanceDraft = State(initialValue: DiffAppearanceSettingsStore.shared.draft) + _diffSyntaxHighlightingDraft = State( + initialValue: AdvancedSettingsStore.shared.diffSyntaxHighlighting + ) } var body: some View { @@ -77,7 +83,9 @@ struct AppSettingsView: View { appUpdateController: appUpdateController, customActionStore: customActionStore, restrictedAIProviderAccess: restrictedAIProviderAccess, - aiProviderDrafts: $aiProviderDrafts + aiProviderDrafts: $aiProviderDrafts, + diffAppearanceDraft: $diffAppearanceDraft, + diffSyntaxHighlightingDraft: $diffSyntaxHighlightingDraft ) Divider() @@ -117,6 +125,10 @@ struct AppSettingsView: View { aiProviderDrafts, restrictedProviderAccess: restrictedAIProviderAccess ) + appState.applyLocalDiffSettings( + appearance: diffAppearanceDraft, + syntaxHighlighting: diffSyntaxHighlightingDraft + ) close() Task { await aiProviderController.refreshAvailability() } } catch { @@ -126,6 +138,8 @@ struct AppSettingsView: View { } private func close() { + diffAppearanceDraft = DiffAppearanceSettingsStore.shared.draft + diffSyntaxHighlightingDraft = AdvancedSettingsStore.shared.diffSyntaxHighlighting dismissWindow(id: "settings") } diff --git a/macgit/Views/Common/Diff/DiffHunkCanvas.swift b/macgit/Views/Common/Diff/DiffHunkCanvas.swift index 5404921b..acd02f82 100644 --- a/macgit/Views/Common/Diff/DiffHunkCanvas.swift +++ b/macgit/Views/Common/Diff/DiffHunkCanvas.swift @@ -9,6 +9,8 @@ final class DiffHunkCanvas: DiffFlippedView { private var textStore: DiffNativeTextStore? private var storeGeneration = -1 private var selectedLineIDs: Set = [] + private var addedBackgroundColor = NSColor.clear + private var removedBackgroundColor = NSColor.clear private(set) var lineHeight: CGFloat = 22 private(set) var contentOrigin = CGPoint.zero var onLineTap: ((Int, NSEvent.ModifierFlags) -> Void)? @@ -16,17 +18,23 @@ final class DiffHunkCanvas: DiffFlippedView { func configure( hunk: DiffHunk, textStore: DiffNativeTextStore, - selectedLineIDs: Set, lineHeight: CGFloat + selectedLineIDs: Set, lineHeight: CGFloat, + addedBackgroundColor: NSColor = .systemGreen.withAlphaComponent(0.08), + removedBackgroundColor: NSColor = .systemRed.withAlphaComponent(0.08) ) { let changed = self.hunk?.id != hunk.id || self.hunk?.contentRevision != hunk.contentRevision || storeGeneration != textStore.generation || self.lineHeight != lineHeight + || self.addedBackgroundColor != addedBackgroundColor + || self.removedBackgroundColor != removedBackgroundColor let selectionChanges = self.selectedLineIDs.symmetricDifference(selectedLineIDs) self.hunk = hunk self.textStore = textStore self.storeGeneration = textStore.generation self.selectedLineIDs = selectedLineIDs self.lineHeight = lineHeight + self.addedBackgroundColor = addedBackgroundColor + self.removedBackgroundColor = removedBackgroundColor if changed { needsDisplay = true } else { invalidateLines(selectionChanges) } prepareVisibleLines() } @@ -178,8 +186,8 @@ final class DiffHunkCanvas: DiffFlippedView { private func background(for line: DiffLine) -> NSColor { if selectedLineIDs.contains(line.id) { return .controlAccentColor.withAlphaComponent(0.12) } switch line.type { - case .added: return .systemGreen.withAlphaComponent(0.08) - case .removed: return .systemRed.withAlphaComponent(0.08) + case .added: return addedBackgroundColor + case .removed: return removedBackgroundColor case .conflictMarker: return .systemPurple.withAlphaComponent(0.10) case .context, .header: return .clear } diff --git a/macgit/Views/Common/Diff/DiffNativeTextStore.swift b/macgit/Views/Common/Diff/DiffNativeTextStore.swift index 86321e80..b41a94ca 100644 --- a/macgit/Views/Common/Diff/DiffNativeTextStore.swift +++ b/macgit/Views/Common/Diff/DiffNativeTextStore.swift @@ -25,14 +25,24 @@ final class DiffNativeTextStore { private var task: Task? private(set) var generation = 0 private(set) var fontSize: CGFloat = 12 + private var fontName = DiffAppearanceSettingsStore.defaultFontName + private var addedTextColor = NSColor.textColor + private var removedTextColor = NSColor.textColor private var fileExtension = "" private var syntaxHighlighting = false var onReady: ((Set) -> Void)? - func reset(fontSize: CGFloat, fileExtension: String, syntaxHighlighting: Bool) { + func reset( + fontName: String, fontSize: CGFloat, + addedTextColor: NSColor, removedTextColor: NSColor, + fileExtension: String, syntaxHighlighting: Bool + ) { cancel() generation += 1 self.fontSize = fontSize + self.fontName = fontName + self.addedTextColor = addedTextColor + self.removedTextColor = removedTextColor self.fileExtension = fileExtension self.syntaxHighlighting = syntaxHighlighting prepared.removeAll() @@ -67,7 +77,7 @@ final class DiffNativeTextStore { inFlight = Set(batch.map(\.id)) let generation = generation let fontSize = fontSize - let fontName = NSFont.monospacedSystemFont(ofSize: fontSize, weight: .regular).fontName + let fontName = fontName let fileExtension = fileExtension task = Task { [weak self] in let worker = Task.detached(priority: .userInitiated) { @@ -127,12 +137,11 @@ final class DiffNativeTextStore { let value = NSMutableAttributedString( string: string, attributes: [ - .font: NSFont.monospacedSystemFont(ofSize: fontSize, weight: .regular), - .foregroundColor: line.type == .header - ? NSColor.secondaryLabelColor - : line.type == .conflictMarker ? NSColor.systemPurple : NSColor.textColor, + .font: NSFont(name: fontName, size: fontSize) + ?? NSFont.monospacedSystemFont(ofSize: fontSize, weight: .regular), + .foregroundColor: foregroundColor(for: line.type), ]) - if chunk == nil, syntaxHighlighting, line.type != .header, line.type != .conflictMarker, + if chunk == nil, syntaxHighlighting, line.type == .context, let tokens { for (range, type) in tokens { @@ -150,4 +159,14 @@ final class DiffNativeTextStore { #endif return result } + + private func foregroundColor(for type: DiffLineType) -> NSColor { + switch type { + case .added: addedTextColor + case .removed: removedTextColor + case .header: .secondaryLabelColor + case .conflictMarker: .systemPurple + case .context: .textColor + } + } } diff --git a/macgit/Views/Common/DiffHunkGeometry.swift b/macgit/Views/Common/DiffHunkGeometry.swift index 9ef8a9c7..d8a45a53 100644 --- a/macgit/Views/Common/DiffHunkGeometry.swift +++ b/macgit/Views/Common/DiffHunkGeometry.swift @@ -10,9 +10,9 @@ nonisolated struct DiffHunkGeometry { let lineHeight: CGFloat let height: CGFloat - init(lineCounts: [Int], scale: CGFloat) { + init(lineCounts: [Int], scale: CGFloat, fontSize: CGFloat = 12) { headerHeight = 32 * scale - lineHeight = 22 * scale + lineHeight = max(22, fontSize + 8) * scale var starts: [CGFloat] = [] var heights: [CGFloat] = [] var y: CGFloat = 4 diff --git a/macgit/Views/Common/DiffNativeTable.swift b/macgit/Views/Common/DiffNativeTable.swift index 9e4743a2..cd259de9 100644 --- a/macgit/Views/Common/DiffNativeTable.swift +++ b/macgit/Views/Common/DiffNativeTable.swift @@ -8,6 +8,7 @@ struct DiffNativeTable: NSViewRepresentable { let hunks: [DiffHunk] let textScale: CGFloat let syntaxHighlighting: Bool + let appearance: DiffAppearanceSettingsStore let fileExtension: String let selectedLineIDs: Set let onLineTap: (DiffHunk, Int, NSEvent.ModifierFlags) -> Void @@ -51,6 +52,7 @@ struct DiffNativeTable: NSViewRepresentable { private var revisions: [DiffContentRevision] = [] private let textStore = DiffNativeTextStore() private var scale: CGFloat = 0 + private var appearanceRevision = -1 private var widths: [CGFloat] = [] private var offsets: [Int: CGFloat] = [:] private var panels: [Int: DiffNativeHunkView] = [:] @@ -69,18 +71,24 @@ struct DiffNativeTable: NSViewRepresentable { let ids = model.hunks.map(\.id) let revisions = model.hunks.map(\.contentRevision) let changed = ids != hunkIDs || revisions != self.revisions || scale != model.textScale + let appearanceChanged = appearanceRevision != model.appearance.revision if changed || self.model?.syntaxHighlighting != model.syntaxHighlighting || self.model?.fileExtension != model.fileExtension || self.model?.colorScheme != model.colorScheme + || appearanceChanged { textStore.reset( - fontSize: 12 * model.textScale, + fontName: model.appearance.font.fontName, + fontSize: model.appearance.fontSize * model.textScale, + addedTextColor: model.appearance.addedTextColor, + removedTextColor: model.appearance.removedTextColor, fileExtension: model.fileExtension, syntaxHighlighting: model.syntaxHighlighting) } self.revisions = revisions + appearanceRevision = model.appearance.revision self.model = model - if changed { + if changed || appearanceChanged { if ids != hunkIDs { offsets.removeAll() } else { @@ -89,7 +97,8 @@ struct DiffNativeTable: NSViewRepresentable { hunkIDs = ids scale = model.textScale geometry = DiffHunkGeometry( - lineCounts: model.hunks.map { $0.lines.count }, scale: scale) + lineCounts: model.hunks.map { $0.lines.count }, scale: scale, + fontSize: model.appearance.fontSize) widths = Array(repeating: 0, count: ids.count) for panel in panels.values { panel.removeFromSuperview() } panels.removeAll() @@ -102,8 +111,8 @@ struct DiffNativeTable: NSViewRepresentable { private func measureWidths(_ model: DiffNativeTable) { widthTask?.cancel() let hunks = model.hunks - let fontSize = 12 * model.textScale - let fontName = NSFont.monospacedSystemFont(ofSize: fontSize, weight: .regular).fontName + let fontSize = model.appearance.fontSize * model.textScale + let fontName = model.appearance.font.fontName widthTask = Task { [weak self] in let worker = Task.detached(priority: .userInitiated) { var widths: [CGFloat] = [] @@ -208,7 +217,9 @@ struct DiffNativeTable: NSViewRepresentable { panel.canvas.configure( hunk: hunk, textStore: textStore, selectedLineIDs: model.selectedLineIDs, - lineHeight: geometry.lineHeight) + lineHeight: geometry.lineHeight, + addedBackgroundColor: model.appearance.addedBackgroundColor, + removedBackgroundColor: model.appearance.removedBackgroundColor) // Read the coordinator's latest model when the event occurs. // A hosted header and native rows share exactly the same actions. panel.canvas.onLineTap = { [weak self] line, flags in diff --git a/macgit/Views/Common/DiffSettingsView.swift b/macgit/Views/Common/DiffSettingsView.swift new file mode 100644 index 00000000..14279402 --- /dev/null +++ b/macgit/Views/Common/DiffSettingsView.swift @@ -0,0 +1,167 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import AppKit +import SwiftUI + +struct DiffSettingsView: View { + @Binding var draft: DiffAppearanceSettingsDraft + @Binding var syntaxHighlighting: Bool + @State private var showingResetConfirmation = false + + private let fontNames = NSFontManager.shared.availableFonts + .filter { NSFont(name: $0, size: 12)?.isFixedPitch == true } + .sorted { $0.localizedStandardCompare($1) == .orderedAscending } + + var body: some View { + Form { + Section { + SettingsToggleRow( + title: "Syntax highlighting", + detail: "Color code syntax in diff views. This uses more CPU and memory, especially for large changes.", + isOn: $syntaxHighlighting + ) + } header: { + Label("Code", systemImage: "chevron.left.forwardslash.chevron.right") + } + + Section { + LabeledContent("Font") { + Picker("Font", selection: $draft.fontName) { + ForEach(fontNames, id: \.self) { fontName in + Text(displayName(for: fontName)) + .font(.custom(fontName, size: 13)) + .tag(fontName) + } + } + .labelsHidden() + .frame(width: 280) + } + + LabeledContent("Size") { + HStack { + Slider(value: $draft.fontSize, in: 8...32, step: 1) + .frame(width: 210) + Text("\(Int(draft.fontSize)) pt") + .monospacedDigit() + .frame(width: 42, alignment: .trailing) + } + } + } header: { + Label("Diff Font", systemImage: "textformat") + } footer: { + Text("Only fixed-width fonts are listed. Diff text size is independent of the app-wide text-size preference.") + } + + Section { + LabeledContent("Added line") { + HStack(spacing: 14) { + colorControl("Background", color: $draft.addedBackgroundColor) + colorControl("Text", color: $draft.addedTextColor) + } + } + + LabeledContent("Removed line") { + HStack(spacing: 14) { + colorControl("Background", color: $draft.removedBackgroundColor) + colorControl("Text", color: $draft.removedTextColor) + } + } + + DiffSettingsPreview(draft: draft) + .padding(.vertical, 6) + } header: { + Label("Diff Colors", systemImage: "paintpalette") + } footer: { + Text("Changed-line text colors take priority over syntax highlighting so additions and removals remain easy to distinguish.") + } + + Section { + } footer: { + SettingsActionRow { + Button("Restore Diff Defaults…", action: showResetConfirmation) + } + .buttonStyle(.bordered) + .controlSize(.regular) + } + } + .formStyle(.grouped) + .navigationTitle("Diff") + .confirmationDialog("Restore Diff Defaults?", isPresented: $showingResetConfirmation) { + Button("Restore Defaults", role: .destructive, action: restoreDefaults) + Button("Cancel", role: .cancel) {} + } message: { + Text("The diff font and all four diff colors will return to their defaults.") + } + } + + private func colorControl(_ title: String, color: Binding) -> some View { + VStack(alignment: .leading, spacing: 4) { + Text(title) + .font(.caption) + .foregroundStyle(.secondary) + ColorPicker( + title, + selection: Binding( + get: { Color(nsColor: color.wrappedValue) }, + set: { color.wrappedValue = NSColor($0) } + ), + supportsOpacity: true + ) + .labelsHidden() + } + } + + private func displayName(for fontName: String) -> String { + NSFont(name: fontName, size: 12)?.displayName ?? fontName + } + + private func showResetConfirmation() { + showingResetConfirmation = true + } + + private func restoreDefaults() { + draft = DiffAppearanceSettingsStore.defaultDraft + syntaxHighlighting = false + } +} + +private struct DiffSettingsPreview: View { + let draft: DiffAppearanceSettingsDraft + + var body: some View { + VStack(spacing: 0) { + previewLine( + prefix: "−", text: "let oldValue = calculate()", + background: draft.removedBackgroundColor, + foreground: draft.removedTextColor + ) + previewLine( + prefix: "+", text: "let newValue = calculate()", + background: draft.addedBackgroundColor, + foreground: draft.addedTextColor + ) + } + .clipShape(.rect(cornerRadius: 6)) + .overlay { + RoundedRectangle(cornerRadius: 6) + .stroke(.separator) + } + .accessibilityElement(children: .combine) + .accessibilityLabel("Diff color preview") + } + + private func previewLine( + prefix: String, text: String, background: NSColor, foreground: NSColor + ) -> some View { + HStack(spacing: 10) { + Text(prefix) + Text(text) + Spacer() + } + .font(.custom(draft.font.fontName, size: draft.fontSize)) + .foregroundStyle(Color(nsColor: foreground)) + .padding(.horizontal, 10) + .padding(.vertical, 5) + .background(Color(nsColor: background)) + } +} diff --git a/macgit/Views/Common/DiffView.swift b/macgit/Views/Common/DiffView.swift index 345a1bd9..587f50c0 100644 --- a/macgit/Views/Common/DiffView.swift +++ b/macgit/Views/Common/DiffView.swift @@ -27,8 +27,8 @@ private func isChangedDiffLine(_ line: DiffLine) -> Bool { } struct DiffView: View { - @Environment(\.appTextScale) private var textScale @ObservedObject private var advancedSettings = AdvancedSettingsStore.shared + @ObservedObject private var diffAppearance = DiffAppearanceSettingsStore.shared let hunks: [DiffHunk] let file: StatusFile? let repositoryURL: URL? @@ -121,8 +121,9 @@ struct DiffView: View { } } else { DiffNativeTable( - hunks: hunks, textScale: textScale, + hunks: hunks, textScale: 1, syntaxHighlighting: advancedSettings.diffSyntaxHighlighting, + appearance: diffAppearance, fileExtension: syntaxFileExtension, selectedLineIDs: selectedLineIDs, onLineTap: { hunk, index, flags in @@ -144,7 +145,7 @@ struct DiffView: View { private func hunkRenderer(_ hunk: DiffHunk) -> HunkView { HunkView( - textScale: textScale, hunk: hunk, file: file, + textScale: 1, hunk: hunk, file: file, repositoryURL: repositoryURL, undoManager: undoManager, selectedLineIDs: $selectedLineIDs, lastSelectedLineID: $lastSelectedLineID, onRefresh: onRefresh, onError: onError, @@ -585,7 +586,6 @@ struct HunkView: View { } struct DiffLineView: View { - @Environment(\.appTextScale) private var textScale let line: DiffLine let fileExtension: String let isSelected: Bool @@ -644,7 +644,7 @@ struct DiffLineView: View { HStack(spacing: 0) { if showsDiffGutter { Text(line.oldLineNumber.map(String.init) ?? "") - .font(.system(size: 10, design: .monospaced).scaled(by: textScale)) + .font(.system(size: 10, design: .monospaced)) .foregroundStyle(.tertiary) .frame(width: 36, alignment: .trailing) .padding(.trailing, 6) @@ -652,7 +652,7 @@ struct DiffLineView: View { // New line number Text(line.newLineNumber.map(String.init) ?? "") - .font(.system(size: 10, design: .monospaced).scaled(by: textScale)) + .font(.system(size: 10, design: .monospaced)) .foregroundStyle(.tertiary) .frame(width: 36, alignment: .trailing) .padding(.trailing, 6) @@ -660,7 +660,7 @@ struct DiffLineView: View { // Prefix if showsDiffGutter && !prefix.isEmpty { Text(prefix) - .font(.system(size: 11, weight: .semibold, design: .monospaced).scaled(by: textScale)) + .font(.system(size: 11, weight: .semibold, design: .monospaced)) .foregroundStyle(textColor.opacity(0.7)) .frame(width: 14, alignment: .center) } @@ -671,13 +671,13 @@ struct DiffLineView: View { lineID: line.id, text: line.text, viewport: horizontalViewport.offsetBy(dx: -contentLeadingInset, dy: 0), - fontSize: 12 * textScale + fontSize: 12 ) } else { Text(highlightedText) .lineLimit(1) .fixedSize(horizontal: true, vertical: false) - .task(id: "\(line.id)-\(textScale)-\(highlightCache != nil)") { + .task(id: "\(line.id)-\(highlightCache != nil)") { await loadHighlightedTextIfNeeded() } } @@ -703,11 +703,11 @@ struct DiffLineView: View { attributed = cached } else if highlightCache != nil { attributed = AttributedString(line.text) - attributed.font = Font(NSFont.monospacedSystemFont(ofSize: 12 * textScale, weight: .regular)) + attributed.font = Font(NSFont.monospacedSystemFont(ofSize: 12, weight: .regular)) attributed.foregroundColor = .primary } else { attributed = AttributedString(line.text) - attributed.font = Font(NSFont.monospacedSystemFont(ofSize: 12 * textScale, weight: .regular)) + attributed.font = Font(NSFont.monospacedSystemFont(ofSize: 12, weight: .regular)) attributed.foregroundColor = .primary } @@ -732,7 +732,7 @@ struct DiffLineView: View { // Let the plain monospaced row reach the first frame before regex work. await Task.yield() guard !Task.isCancelled else { return } - let highlighted = highlightCache.text(for: line, fileExtension: fileExtension, fontSize: 12 * textScale) + let highlighted = highlightCache.text(for: line, fileExtension: fileExtension, fontSize: 12) guard !Task.isCancelled else { return } deferredHighlightedText = highlighted } diff --git a/macgitTests/AppSettingsSnapshotTests.swift b/macgitTests/AppSettingsSnapshotTests.swift index afa2143d..041c69e4 100644 --- a/macgitTests/AppSettingsSnapshotTests.swift +++ b/macgitTests/AppSettingsSnapshotTests.swift @@ -79,7 +79,14 @@ final class AppSettingsSnapshotTests: XCTestCase { "historyBranchFilter", "historyIncludeRemotes", "autoFetchEnabled", - "refreshOnAppActive" + "refreshOnAppActive", + "diffFontName", + "diffFontSize", + "diffAddedBackgroundColor", + "diffAddedTextColor", + "diffRemovedBackgroundColor", + "diffRemovedTextColor", + "diffSyntaxHighlighting" ] ) } diff --git a/macgitTests/CloudSettingsDocumentTests.swift b/macgitTests/CloudSettingsDocumentTests.swift index bd0dab20..41dccba1 100644 --- a/macgitTests/CloudSettingsDocumentTests.swift +++ b/macgitTests/CloudSettingsDocumentTests.swift @@ -45,7 +45,14 @@ final class CloudSettingsDocumentTests: XCTestCase { historyBranchFilter: .branch("origin/feature/login"), historyIncludeRemotes: true, autoFetchEnabled: true, - refreshOnAppActive: false + refreshOnAppActive: false, + diffFontName: "SFMono-Regular", + diffFontSize: 14, + diffAddedBackgroundColor: "#11223344", + diffAddedTextColor: "#55667788", + diffRemovedBackgroundColor: "#99AABBCC", + diffRemovedTextColor: "#DDEEFF00", + diffSyntaxHighlighting: true ) func testEncodingUsesExactDocumentSchema() throws { @@ -81,6 +88,13 @@ final class CloudSettingsDocumentTests: XCTestCase { "historyIncludeRemotes", "autoFetchEnabled", "refreshOnAppActive", + "diffFontName", + "diffFontSize", + "diffAddedBackgroundColor", + "diffAddedTextColor", + "diffRemovedBackgroundColor", + "diffRemovedTextColor", + "diffSyntaxHighlighting", "updatedAt" ] ) @@ -109,6 +123,13 @@ final class CloudSettingsDocumentTests: XCTestCase { XCTAssertEqual(document["historyIncludeRemotes"] as? Bool, true) XCTAssertEqual(document["autoFetchEnabled"] as? Bool, true) XCTAssertEqual(document["refreshOnAppActive"] as? Bool, false) + XCTAssertEqual(document["diffFontName"] as? String, "SFMono-Regular") + XCTAssertEqual(document["diffFontSize"] as? Double, 14) + XCTAssertEqual(document["diffAddedBackgroundColor"] as? String, "#11223344") + XCTAssertEqual(document["diffAddedTextColor"] as? String, "#55667788") + XCTAssertEqual(document["diffRemovedBackgroundColor"] as? String, "#99AABBCC") + XCTAssertEqual(document["diffRemovedTextColor"] as? String, "#DDEEFF00") + XCTAssertEqual(document["diffSyntaxHighlighting"] as? Bool, true) XCTAssertEqual(document["updatedAt"] as? Timestamp, timestamp) } @@ -195,6 +216,27 @@ final class CloudSettingsDocumentTests: XCTestCase { XCTAssertEqual(decoded.appearance, .system) } + func testDecodingDefaultsMissingDiffSettings() throws { + var document = validDocument() + document.removeValue(forKey: "diffFontName") + document.removeValue(forKey: "diffFontSize") + document.removeValue(forKey: "diffAddedBackgroundColor") + document.removeValue(forKey: "diffAddedTextColor") + document.removeValue(forKey: "diffRemovedBackgroundColor") + document.removeValue(forKey: "diffRemovedTextColor") + document.removeValue(forKey: "diffSyntaxHighlighting") + + let decoded = try CloudSettingsDocument.decode(document) + + XCTAssertEqual(decoded.diffFontName, "Menlo-Regular") + XCTAssertEqual(decoded.diffFontSize, 12) + XCTAssertEqual(decoded.diffAddedBackgroundColor, "#E6FAF0FF") + XCTAssertEqual(decoded.diffAddedTextColor, "#006E4FFF") + XCTAssertEqual(decoded.diffRemovedBackgroundColor, "#FFEDE8FF") + XCTAssertEqual(decoded.diffRemovedTextColor, "#C72E0DFF") + XCTAssertFalse(decoded.diffSyntaxHighlighting) + } + func testDecodingRejectsInvalidAppearance() { var unsupported = validDocument() unsupported["appearance"] = "sepia" @@ -205,6 +247,20 @@ final class CloudSettingsDocumentTests: XCTestCase { XCTAssertThrowsError(try CloudSettingsDocument.decode(wrongType)) } + func testDecodingRejectsInvalidDiffSettings() { + for (key, value) in [ + ("diffFontName", "" as Any), + ("diffFontSize", 40 as Any), + ("diffFontSize", true as Any), + ("diffAddedBackgroundColor", "red" as Any), + ("diffSyntaxHighlighting", "true" as Any) + ] { + var document = validDocument() + document[key] = value + XCTAssertThrowsError(try CloudSettingsDocument.decode(document), key) + } + } + func testDecodingValidDocumentReturnsCompleteSnapshot() throws { let decoded = try CloudSettingsDocument.decode(validDocument()) diff --git a/macgitTests/DiffNativeTableTests.swift b/macgitTests/DiffNativeTableTests.swift index 62509072..10b77780 100644 --- a/macgitTests/DiffNativeTableTests.swift +++ b/macgitTests/DiffNativeTableTests.swift @@ -72,6 +72,7 @@ final class DiffNativeTableTests: XCTestCase { private func makeModel(_ hunks: [DiffHunk]) -> DiffNativeTable { DiffNativeTable( hunks: hunks, textScale: 1, syntaxHighlighting: false, + appearance: .shared, fileExtension: "swift", selectedLineIDs: [], onLineTap: { _, _, _ in }, lineMenu: { _, _ in NSMenu() } ) { hunk in From 36dd139eb47764430c6f5f463e6869f712e5aa48 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 12 Oct 2026 08:34:21 +0700 Subject: [PATCH 2/4] feat: Constrain sidebar native menu item title width Truncate long menu titles with an ellipsis via binary search on measured width and expose the full text as a tooltip; propagate the limit to submenus. --- .../Sidebar/SidebarNativeList.swift | 53 ++++++++++++++++--- .../MainWindow/SidebarView+NativeMenus.swift | 2 +- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/macgit/Views/MainWindow/Sidebar/SidebarNativeList.swift b/macgit/Views/MainWindow/Sidebar/SidebarNativeList.swift index d2218e11..7dc55356 100644 --- a/macgit/Views/MainWindow/Sidebar/SidebarNativeList.swift +++ b/macgit/Views/MainWindow/Sidebar/SidebarNativeList.swift @@ -592,13 +592,19 @@ final class SidebarNativeMenu: NSMenu, NSMenuDelegate { var canPerform: () -> Bool = { true } } private let validation = Validation() + private let maximumItemTitleWidth: CGFloat? var onClose: (() -> Void)? final class Action: NSObject { let perform: () -> Void init(_ perform: @escaping () -> Void) { self.perform = perform } @objc func invoke() { perform() } } - init() { super.init(title: ""); autoenablesItems = false; delegate = self } + init(maximumItemTitleWidth: CGFloat? = nil) { + self.maximumItemTitleWidth = maximumItemTitleWidth + super.init(title: "") + autoenablesItems = false + delegate = self + } required init(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } func item(_ title: String, enabled: Bool = true, checked: Bool = false, _ perform: @escaping () -> Void) { let validation = validation @@ -606,7 +612,13 @@ final class SidebarNativeMenu: NSMenu, NSMenuDelegate { guard validation.canPerform() else { return } perform() } - let item = NSMenuItem(title: title, action: #selector(Action.invoke), keyEquivalent: "") + let displayedTitle = constrainedTitle(title) + let item = NSMenuItem( + title: displayedTitle, + action: #selector(Action.invoke), + keyEquivalent: "" + ) + if displayedTitle != title { item.toolTip = title } item.target = target item.representedObject = target item.isEnabled = enabled @@ -620,11 +632,13 @@ final class SidebarNativeMenu: NSMenu, NSMenuDelegate { } func menuDidClose(_ menu: NSMenu) { onClose?() } func lazySubmenu(_ title: String, enabled: Bool, _ build: @escaping (SidebarNativeMenu) -> Void) { - let menu = SidebarNativeMenu() + let menu = SidebarNativeMenu(maximumItemTitleWidth: maximumItemTitleWidth) menu.populate = build menu.delegate = menu menu.item("Loading…", enabled: false) {} - let item = NSMenuItem(title: title, action: nil, keyEquivalent: "") + let displayedTitle = constrainedTitle(title) + let item = NSMenuItem(title: displayedTitle, action: nil, keyEquivalent: "") + if displayedTitle != title { item.toolTip = title } item.submenu = menu item.isEnabled = enabled addItem(item) @@ -637,9 +651,11 @@ final class SidebarNativeMenu: NSMenu, NSMenuDelegate { installGuard(validation.canPerform) } func submenu(_ title: String, _ build: (SidebarNativeMenu) -> Void) { - let menu = SidebarNativeMenu() + let menu = SidebarNativeMenu(maximumItemTitleWidth: maximumItemTitleWidth) build(menu) - let item = NSMenuItem(title: title, action: nil, keyEquivalent: "") + let displayedTitle = constrainedTitle(title) + let item = NSMenuItem(title: displayedTitle, action: nil, keyEquivalent: "") + if displayedTitle != title { item.toolTip = title } item.submenu = menu item.isEnabled = !menu.items.isEmpty addItem(item) @@ -647,4 +663,29 @@ final class SidebarNativeMenu: NSMenu, NSMenuDelegate { func copy(_ title: String, value: String) { item(title) { NSPasteboard.general.clearContents(); NSPasteboard.general.setString(value, forType: .string) } } + + private func constrainedTitle(_ title: String) -> String { + guard let maximumItemTitleWidth else { return title } + let font = NSFont.menuFont(ofSize: 0) + let attributes: [NSAttributedString.Key: Any] = [.font: font] + guard (title as NSString).size(withAttributes: attributes).width > maximumItemTitleWidth else { + return title + } + + let ellipsis = "…" + var lowerBound = 0 + var upperBound = title.count + while lowerBound < upperBound { + let candidateLength = (lowerBound + upperBound + 1) / 2 + let endIndex = title.index(title.startIndex, offsetBy: candidateLength) + let candidate = String(title[.. NSMenu { - let menu = SidebarNativeMenu() + let menu = SidebarNativeMenu(maximumItemTitleWidth: 520) let actions = branchSectionActions let upstream = upstreamByBranch[branch] let remotes = BranchUpstreamActionPolicy.pushRemotes(branch: branch, upstream: upstream, remotes: remoteNames) From d400373673f869fe568ee28ed58eaeb14025da74 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 12 Oct 2026 08:42:53 +0700 Subject: [PATCH 3/4] feat: Add option to delete matching remote branches with local branches Resolve remote delete targets from upstream or unique matching branches, preserving undo by capturing remote hash before deletion. --- .../Models/RemoteBranchDeleteResolver.swift | 26 +++ macgit/Views/Common/BranchSheetView.swift | 172 +++++++++++++++--- .../Sidebar/RemoteBranchDeleteTarget.swift | 14 +- .../SidebarBranchPresentationModifier.swift | 4 + .../Sidebar/SidebarDeleteBranchSheet.swift | 17 ++ .../SidebarView+BranchActions.swift | 119 ++++++++++-- macgit/Views/MainWindow/SidebarView.swift | 3 + .../RemoteBranchDeleteResolverTests.swift | 56 ++++++ 8 files changed, 372 insertions(+), 39 deletions(-) create mode 100644 macgit/Models/RemoteBranchDeleteResolver.swift create mode 100644 macgitTests/RemoteBranchDeleteResolverTests.swift diff --git a/macgit/Models/RemoteBranchDeleteResolver.swift b/macgit/Models/RemoteBranchDeleteResolver.swift new file mode 100644 index 00000000..0ec69129 --- /dev/null +++ b/macgit/Models/RemoteBranchDeleteResolver.swift @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import Foundation + +enum RemoteBranchDeleteResolver { + static func target( + localBranch: String, + upstream: String?, + branchesByRemote: [String: [String]] + ) -> RemoteBranchDeleteTarget? { + if let upstream, + let tracked = RemoteBranchDeleteTarget(fullPath: upstream) { + return tracked + } + + let matches = branchesByRemote + .filter { $0.value.contains(localBranch) } + .map { RemoteBranchDeleteTarget(remote: $0.key, branch: localBranch) } + .sorted { $0.fullPath < $1.fullPath } + + if let origin = matches.first(where: { $0.remote == "origin" }) { + return origin + } + return matches.count == 1 ? matches[0] : nil + } +} diff --git a/macgit/Views/Common/BranchSheetView.swift b/macgit/Views/Common/BranchSheetView.swift index bdd463d6..893d65e0 100644 --- a/macgit/Views/Common/BranchSheetView.swift +++ b/macgit/Views/Common/BranchSheetView.swift @@ -41,7 +41,20 @@ struct BranchDeleteItem: Identifiable { let id = UUID() let name: String let type: BranchType + let remoteTarget: RemoteBranchDeleteTarget? var isSelected: Bool = false + + init( + name: String, + type: BranchType, + remoteTarget: RemoteBranchDeleteTarget? = nil, + isSelected: Bool = false + ) { + self.name = name + self.type = type + self.remoteTarget = remoteTarget + self.isSelected = isSelected + } } enum BranchType: String { @@ -86,6 +99,7 @@ struct BranchSheetView: View { // Delete tab state @State private var branches: [BranchDeleteItem] = [] @State private var forceDelete = false + @State private var deleteMatchingRemoteBranches = false @State private var hasAppliedInitialDeleteSelection = false // Confirmation overlay @@ -143,6 +157,12 @@ struct BranchSheetView: View { !selectedBranches.isEmpty } + private var matchingRemoteTargets: [RemoteBranchDeleteTarget] { + Array(Set(selectedBranches.compactMap { item in + item.type == .local ? item.remoteTarget : nil + })).sorted { $0.fullPath < $1.fullPath } + } + private var commitPickerOptions: [BranchCommitInfo] { Self.commitPickerOptions( selectedStartPoint: selectedStartPoint, @@ -187,10 +207,22 @@ struct BranchSheetView: View { // Buttons HStack(spacing: 12) { if selectedTab == .delete { - Toggle("Force delete", isOn: $forceDelete) - .toggleStyle(.checkbox) - .font(.system(size: 12)) - .help("Delete branches regardless of merge status") + VStack(alignment: .leading, spacing: 6) { + Toggle("Force delete", isOn: $forceDelete) + .help("Delete branches regardless of merge status") + + if !matchingRemoteTargets.isEmpty { + Toggle( + matchingRemoteTargets.count == 1 + ? "Also delete \(matchingRemoteTargets[0].fullPath)" + : "Also delete matching remote branches", + isOn: $deleteMatchingRemoteBranches + ) + .help("Delete tracked or matching branches from their remotes") + } + } + .toggleStyle(.checkbox) + .font(.system(size: 12)) } Spacer() @@ -266,6 +298,14 @@ struct BranchSheetView: View { .font(.system(size: 12)) .frame(maxWidth: .infinity, alignment: .leading) } + if deleteMatchingRemoteBranches { + ForEach(matchingRemoteTargets, id: \.self) { target in + Text("• \(target.fullPath) (remote)") + .font(.system(size: 12)) + .foregroundStyle(.secondary) + .frame(maxWidth: .infinity, alignment: .leading) + } + } } } .frame(maxWidth: .infinity, maxHeight: 240, alignment: .leading) @@ -566,6 +606,10 @@ struct BranchSheetView: View { return } + let remoteTargetsFromLocal: Set = deleteMatchingRemoteBranches + ? Set(matchingRemoteTargets) + : [] + do { for branch in selectedBranches { switch branch.type { @@ -573,25 +617,49 @@ struct BranchSheetView: View { let support = GitBranchUndoSupport() let tip = try await support.tip(of: branch.name, in: repositoryURL) let upstream = await support.upstream(of: branch.name, in: repositoryURL) + let remoteTarget = branch.remoteTarget.flatMap { + remoteTargetsFromLocal.contains($0) ? $0 : nil + } + let remoteHash: String? + if let remoteTarget { + remoteHash = try await GitRemoteUndoSupport().remoteHash( + remote: remoteTarget.remote, + branch: remoteTarget.branch, + in: repositoryURL + ) + } else { + remoteHash = nil + } _ = try await GitStatusService.shared.deleteBranch( name: branch.name, force: forceDelete, in: repositoryURL ) - await MainActor.run { - var undoOperations: [GitUndoOperation] = [ - .createLocalBranch(name: branch.name, startPoint: tip, checkout: false) - ] - if let upstream { - undoOperations.append(.setUpstream(branch: branch.name, upstream: upstream)) - } - undoManager?.register( - GitUndoEntry( - repositoryURL: repositoryURL, - label: "Delete branch \(branch.name)", - undoOperation: .sequence(undoOperations), - redoOperation: .deleteLocalBranch(name: branch.name, force: forceDelete, expectedTip: tip) + do { + if let remoteTarget { + _ = try await GitStatusService.shared.deleteRemoteBranch( + remote: remoteTarget.remote, + name: remoteTarget.branch, + in: repositoryURL ) + } + } catch { + registerDeletedBranchUndo( + branch: branch.name, + tip: tip, + upstream: upstream, + remoteTarget: nil, + remoteHash: nil + ) + throw error + } + await MainActor.run { + registerDeletedBranchUndo( + branch: branch.name, + tip: tip, + upstream: upstream, + remoteTarget: remoteTarget, + remoteHash: remoteHash ) } case .remote: @@ -599,6 +667,11 @@ struct BranchSheetView: View { guard parts.count == 2 else { continue } let remote = String(parts[0]) let name = String(parts[1]) + if remoteTargetsFromLocal.contains( + RemoteBranchDeleteTarget(remote: remote, branch: name) + ) { + continue + } _ = try await GitStatusService.shared.deleteRemoteBranch( remote: remote, name: name, @@ -658,23 +731,34 @@ struct BranchSheetView: View { let remotesList = await GitStatusService.shared.remotes(in: repositoryURL) let initialSelection = hasAppliedInitialDeleteSelection ? [] : initiallySelectedLocalBranches + var remoteBranchesByRemote: [String: [String]] = [:] + for remote in remotesList { + remoteBranchesByRemote[remote] = await GitStatusService.shared.cachedRemoteBranches( + remote: remote, + in: repositoryURL + ).filter { $0 != "HEAD" } + } + var items: [BranchDeleteItem] = [] for name in locals { + let upstream = await GitBranchUndoSupport().upstream(of: name, in: repositoryURL) items.append( BranchDeleteItem( name: name, type: .local, + remoteTarget: RemoteBranchDeleteResolver.target( + localBranch: name, + upstream: upstream, + branchesByRemote: remoteBranchesByRemote + ), isSelected: initialSelection.contains(name) ) ) } for remote in remotesList { - let remoteBranches = await GitStatusService.shared.cachedRemoteBranches(remote: remote, in: repositoryURL) - for branchName in remoteBranches { - // Skip HEAD symbolic refs - if branchName == "HEAD" { continue } + for branchName in remoteBranchesByRemote[remote] ?? [] { items.append(BranchDeleteItem(name: "\(remote)/\(branchName)", type: .remote)) } } @@ -687,6 +771,52 @@ struct BranchSheetView: View { // MARK: - Helpers + @MainActor + private func registerDeletedBranchUndo( + branch: String, + tip: String, + upstream: String?, + remoteTarget: RemoteBranchDeleteTarget?, + remoteHash: String? + ) { + var undoOperations: [GitUndoOperation] = [ + .createLocalBranch(name: branch, startPoint: tip, checkout: false) + ] + if let upstream { + undoOperations.append(.setUpstream(branch: branch, upstream: upstream)) + } + if let remoteTarget, remoteHash != nil { + undoOperations.append( + .pushBranch( + remote: remoteTarget.remote, + localBranch: branch, + remoteBranch: remoteTarget.branch + ) + ) + } + + var redoOperations: [GitUndoOperation] = [ + .deleteLocalBranch(name: branch, force: forceDelete, expectedTip: tip) + ] + if let remoteTarget, let remoteHash { + redoOperations.append( + .deleteRemoteBranch( + remote: remoteTarget.remote, + branch: remoteTarget.branch, + expectedHash: remoteHash + ) + ) + } + undoManager?.register( + GitUndoEntry( + repositoryURL: repositoryURL, + label: "Delete branch \(branch)", + undoOperation: .sequence(undoOperations), + redoOperation: .sequence(redoOperations) + ) + ) + } + static func initialCreateState( initialStartPoint: GitBranchStartPoint?, recentCommits: [BranchCommitInfo] diff --git a/macgit/Views/MainWindow/Sidebar/RemoteBranchDeleteTarget.swift b/macgit/Views/MainWindow/Sidebar/RemoteBranchDeleteTarget.swift index 6f1bc5ec..7d1d4945 100644 --- a/macgit/Views/MainWindow/Sidebar/RemoteBranchDeleteTarget.swift +++ b/macgit/Views/MainWindow/Sidebar/RemoteBranchDeleteTarget.swift @@ -17,10 +17,22 @@ // import Foundation -struct RemoteBranchDeleteTarget: Identifiable, Equatable { +struct RemoteBranchDeleteTarget: Identifiable, Equatable, Hashable { let remote: String let branch: String var id: String { "\(remote)/\(branch)" } var fullPath: String { id } + + init(remote: String, branch: String) { + self.remote = remote + self.branch = branch + } + + init?(fullPath: String) { + let parts = fullPath.split(separator: "/", maxSplits: 1, omittingEmptySubsequences: false) + guard parts.count == 2, !parts[0].isEmpty, !parts[1].isEmpty else { return nil } + remote = String(parts[0]) + branch = String(parts[1]) + } } diff --git a/macgit/Views/MainWindow/Sidebar/SidebarBranchPresentationModifier.swift b/macgit/Views/MainWindow/Sidebar/SidebarBranchPresentationModifier.swift index 3374b464..f3e42f8c 100644 --- a/macgit/Views/MainWindow/Sidebar/SidebarBranchPresentationModifier.swift +++ b/macgit/Views/MainWindow/Sidebar/SidebarBranchPresentationModifier.swift @@ -20,12 +20,14 @@ import SwiftUI struct SidebarBranchPresentationModifier: ViewModifier { @Binding var deleteConfirmationTarget: DeleteConfirmationTarget? @Binding var forceDeleteBranch: Bool + @Binding var deleteRemoteBranchAlongsideLocal: Bool @Binding var remoteBranchDeleteTarget: RemoteBranchDeleteTarget? let repositoryURL: URL let undoManager: GitUndoManager? let currentBranch: String let branchesUnderPrefix: (String) -> [String] + let remoteDeleteTarget: (String) -> RemoteBranchDeleteTarget? let cancelDeleteConfirmation: () -> Void let confirmDeleteBranch: (String) -> Void let onPrefixDeleteCompleted: () -> Void @@ -50,6 +52,8 @@ struct SidebarBranchPresentationModifier: ViewModifier { SidebarDeleteBranchSheet( branchName: branch, forceDelete: $forceDeleteBranch, + remoteBranch: remoteDeleteTarget(branch), + deleteRemoteBranch: $deleteRemoteBranchAlongsideLocal, onCancel: cancelDeleteConfirmation, onDelete: { confirmDeleteBranch(branch) } ) diff --git a/macgit/Views/MainWindow/Sidebar/SidebarDeleteBranchSheet.swift b/macgit/Views/MainWindow/Sidebar/SidebarDeleteBranchSheet.swift index 068d37fe..bcafe8dd 100644 --- a/macgit/Views/MainWindow/Sidebar/SidebarDeleteBranchSheet.swift +++ b/macgit/Views/MainWindow/Sidebar/SidebarDeleteBranchSheet.swift @@ -20,6 +20,8 @@ import SwiftUI struct SidebarDeleteBranchSheet: View { let branchName: String @Binding var forceDelete: Bool + let remoteBranch: RemoteBranchDeleteTarget? + @Binding var deleteRemoteBranch: Bool let onCancel: () -> Void let onDelete: () -> Void @@ -44,6 +46,21 @@ struct SidebarDeleteBranchSheet: View { .fixedSize(horizontal: false, vertical: true) } + if let remoteBranch { + VStack(alignment: .leading, spacing: 6) { + Toggle( + "Also delete remote branch \(remoteBranch.fullPath)", + isOn: $deleteRemoteBranch + ) + .toggleStyle(.checkbox) + .font(.system(size: 12)) + + Text("This removes the branch from the remote repository for every collaborator.") + .font(.system(size: 11)) + .foregroundStyle(.secondary) + } + } + HStack(spacing: 12) { Spacer() Button("Cancel", role: .cancel, action: onCancel) diff --git a/macgit/Views/MainWindow/SidebarView+BranchActions.swift b/macgit/Views/MainWindow/SidebarView+BranchActions.swift index e82ef1dd..1b43dbb2 100644 --- a/macgit/Views/MainWindow/SidebarView+BranchActions.swift +++ b/macgit/Views/MainWindow/SidebarView+BranchActions.swift @@ -125,43 +125,80 @@ extension SidebarView { func cancelDeleteConfirmation() { deleteConfirmationTarget = nil forceDeleteBranch = false + deleteRemoteBranchAlongsideLocal = false } func confirmDeleteBranch(_ branch: String) { let force = forceDeleteBranch + let remoteTarget = deleteRemoteBranchAlongsideLocal + ? matchingRemoteDeleteTarget(for: branch) + : nil deleteConfirmationTarget = nil forceDeleteBranch = false + deleteRemoteBranchAlongsideLocal = false onRunRepositoryOperation("Deleting \(branch)...") { - await deleteBranch(branch, force: force) + await deleteBranch(branch, force: force, remoteTarget: remoteTarget) } } - func deleteBranch(_ branch: String, force: Bool = false) async { + func matchingRemoteDeleteTarget(for branch: String) -> RemoteBranchDeleteTarget? { + RemoteBranchDeleteResolver.target( + localBranch: branch, + upstream: upstreamByBranch[branch], + branchesByRemote: branchesByRemote + ) + } + + func deleteBranch( + _ branch: String, + force: Bool = false, + remoteTarget: RemoteBranchDeleteTarget? = nil + ) async { do { let support = GitBranchUndoSupport() let tip = try await support.tip(of: branch, in: repositoryURL) let upstream = await support.upstream(of: branch, in: repositoryURL) + let remoteHash: String? + if let remoteTarget { + remoteHash = try await GitRemoteUndoSupport().remoteHash( + remote: remoteTarget.remote, + branch: remoteTarget.branch, + in: repositoryURL + ) + } else { + remoteHash = nil + } _ = try await GitStatusService.shared.deleteBranch(name: branch, force: force, in: repositoryURL) - await MainActor.run { - var undoOperations: [GitUndoOperation] = [ - .createLocalBranch(name: branch, startPoint: tip, checkout: false) - ] - - if let upstream { - undoOperations.append(.setUpstream(branch: branch, upstream: upstream)) - } - - undoManager?.register( - GitUndoEntry( - repositoryURL: repositoryURL, - label: "Delete branch \(branch)", - undoOperation: .sequence(undoOperations), - redoOperation: .deleteLocalBranch(name: branch, force: force, expectedTip: tip) + do { + if let remoteTarget { + _ = try await GitStatusService.shared.deleteRemoteBranch( + remote: remoteTarget.remote, + name: remoteTarget.branch, + in: repositoryURL ) + } + } catch { + registerDeletedBranchUndo( + branch: branch, + force: force, + tip: tip, + upstream: upstream, + remoteTarget: nil, + remoteHash: nil ) + throw error } + registerDeletedBranchUndo( + branch: branch, + force: force, + tip: tip, + upstream: upstream, + remoteTarget: remoteTarget, + remoteHash: remoteHash + ) + NotificationCenter.default.post(name: .repositoryDidChange, object: nil, userInfo: ["repositoryURL": repositoryURL]) } catch { await MainActor.run { @@ -171,6 +208,54 @@ extension SidebarView { } } + @MainActor + private func registerDeletedBranchUndo( + branch: String, + force: Bool, + tip: String, + upstream: String?, + remoteTarget: RemoteBranchDeleteTarget?, + remoteHash: String? + ) { + var undoOperations: [GitUndoOperation] = [ + .createLocalBranch(name: branch, startPoint: tip, checkout: false) + ] + if let upstream { + undoOperations.append(.setUpstream(branch: branch, upstream: upstream)) + } + if let remoteTarget, remoteHash != nil { + undoOperations.append( + .pushBranch( + remote: remoteTarget.remote, + localBranch: branch, + remoteBranch: remoteTarget.branch + ) + ) + } + + var redoOperations: [GitUndoOperation] = [ + .deleteLocalBranch(name: branch, force: force, expectedTip: tip) + ] + if let remoteTarget, let remoteHash { + redoOperations.append( + .deleteRemoteBranch( + remote: remoteTarget.remote, + branch: remoteTarget.branch, + expectedHash: remoteHash + ) + ) + } + + undoManager?.register( + GitUndoEntry( + repositoryURL: repositoryURL, + label: "Delete branch \(branch)", + undoOperation: .sequence(undoOperations), + redoOperation: .sequence(redoOperations) + ) + ) + } + func refreshAfterBranchDeletion() { Task { await loadBranches(force: true) diff --git a/macgit/Views/MainWindow/SidebarView.swift b/macgit/Views/MainWindow/SidebarView.swift index cc908da5..c6e15aac 100644 --- a/macgit/Views/MainWindow/SidebarView.swift +++ b/macgit/Views/MainWindow/SidebarView.swift @@ -196,6 +196,7 @@ struct SidebarView: View { @State var showingError = false @State var deleteConfirmationTarget: DeleteConfirmationTarget? @State var remoteBranchDeleteTarget: RemoteBranchDeleteTarget? + @State var deleteRemoteBranchAlongsideLocal = false @State var forceDeleteBranch = false @State var activeDropTarget: GitDragTarget? @State var activeDropLabel: String? @@ -597,11 +598,13 @@ struct SidebarView: View { SidebarBranchPresentationModifier( deleteConfirmationTarget: $deleteConfirmationTarget, forceDeleteBranch: $forceDeleteBranch, + deleteRemoteBranchAlongsideLocal: $deleteRemoteBranchAlongsideLocal, remoteBranchDeleteTarget: $remoteBranchDeleteTarget, repositoryURL: repositoryURL, undoManager: undoManager, currentBranch: currentBranch, branchesUnderPrefix: branchesUnderPrefix, + remoteDeleteTarget: matchingRemoteDeleteTarget, cancelDeleteConfirmation: cancelDeleteConfirmation, confirmDeleteBranch: confirmDeleteBranch, onPrefixDeleteCompleted: refreshAfterBranchDeletion, diff --git a/macgitTests/RemoteBranchDeleteResolverTests.swift b/macgitTests/RemoteBranchDeleteResolverTests.swift new file mode 100644 index 00000000..e95c9c80 --- /dev/null +++ b/macgitTests/RemoteBranchDeleteResolverTests.swift @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +import XCTest + +@testable import macgit + +final class RemoteBranchDeleteResolverTests: XCTestCase { + func testTrackedUpstreamTakesPriorityOverMatchingBranchName() { + let target = RemoteBranchDeleteResolver.target( + localBranch: "feature/demo", + upstream: "fork/review/demo", + branchesByRemote: [ + "origin": ["feature/demo"], + "fork": ["review/demo"] + ] + ) + + XCTAssertEqual(target, RemoteBranchDeleteTarget(remote: "fork", branch: "review/demo")) + } + + func testFallsBackToOriginBranchWithSameName() { + let target = RemoteBranchDeleteResolver.target( + localBranch: "feature/demo", + upstream: nil, + branchesByRemote: [ + "origin": ["feature/demo"], + "fork": ["feature/demo"] + ] + ) + + XCTAssertEqual(target, RemoteBranchDeleteTarget(remote: "origin", branch: "feature/demo")) + } + + func testFallsBackToOnlyMatchingRemote() { + let target = RemoteBranchDeleteResolver.target( + localBranch: "feature/demo", + upstream: nil, + branchesByRemote: ["fork": ["feature/demo"]] + ) + + XCTAssertEqual(target, RemoteBranchDeleteTarget(remote: "fork", branch: "feature/demo")) + } + + func testReturnsNilForAmbiguousNonOriginMatches() { + let target = RemoteBranchDeleteResolver.target( + localBranch: "feature/demo", + upstream: nil, + branchesByRemote: [ + "fork-a": ["feature/demo"], + "fork-b": ["feature/demo"] + ] + ) + + XCTAssertNil(target) + } +} From da2cc804b950a5969728c3a0906059c13a182bc2 Mon Sep 17 00:00:00 2001 From: Thanh Tran Date: Mon, 12 Oct 2026 08:50:36 +0700 Subject: [PATCH 4/4] Fix PR review findings for diff colors and branch deletion --- .../Models/RemoteBranchDeleteResolver.swift | 3 ++- .../DiffAppearanceSettingsStore.swift | 12 ++++++---- macgit/Views/Common/BranchSheetView.swift | 6 ++--- .../SidebarView+BranchActions.swift | 6 ++--- macgitTests/GitUndoExecutorTests.swift | 23 +++++++++++++++++++ .../RemoteBranchDeleteResolverTests.swift | 10 ++++++++ 6 files changed, 49 insertions(+), 11 deletions(-) diff --git a/macgit/Models/RemoteBranchDeleteResolver.swift b/macgit/Models/RemoteBranchDeleteResolver.swift index 0ec69129..6912d9f6 100644 --- a/macgit/Models/RemoteBranchDeleteResolver.swift +++ b/macgit/Models/RemoteBranchDeleteResolver.swift @@ -9,7 +9,8 @@ enum RemoteBranchDeleteResolver { branchesByRemote: [String: [String]] ) -> RemoteBranchDeleteTarget? { if let upstream, - let tracked = RemoteBranchDeleteTarget(fullPath: upstream) { + let tracked = RemoteBranchDeleteTarget(fullPath: upstream), + branchesByRemote[tracked.remote]?.contains(tracked.branch) == true { return tracked } diff --git a/macgit/Services/DiffAppearanceSettingsStore.swift b/macgit/Services/DiffAppearanceSettingsStore.swift index b5f70f5e..6eed5243 100644 --- a/macgit/Services/DiffAppearanceSettingsStore.swift +++ b/macgit/Services/DiffAppearanceSettingsStore.swift @@ -145,10 +145,14 @@ final class DiffAppearanceSettingsStore: ObservableObject { func apply(_ draft: DiffAppearanceSettingsDraft) { fontName = draft.fontName fontSize = min(max(draft.fontSize, 8), 32) - addedBackgroundColor = draft.addedBackgroundColor - addedTextColor = draft.addedTextColor - removedBackgroundColor = draft.removedBackgroundColor - removedTextColor = draft.removedTextColor + addedBackgroundColor = draft.addedBackgroundColor.usingColorSpace(.sRGB) + ?? Defaults.addedBackgroundColor + addedTextColor = draft.addedTextColor.usingColorSpace(.sRGB) + ?? Defaults.addedTextColor + removedBackgroundColor = draft.removedBackgroundColor.usingColorSpace(.sRGB) + ?? Defaults.removedBackgroundColor + removedTextColor = draft.removedTextColor.usingColorSpace(.sRGB) + ?? Defaults.removedTextColor } func apply(_ snapshot: AppSettingsSnapshot) { diff --git a/macgit/Views/Common/BranchSheetView.swift b/macgit/Views/Common/BranchSheetView.swift index 893d65e0..103741b8 100644 --- a/macgit/Views/Common/BranchSheetView.swift +++ b/macgit/Views/Common/BranchSheetView.swift @@ -785,12 +785,12 @@ struct BranchSheetView: View { if let upstream { undoOperations.append(.setUpstream(branch: branch, upstream: upstream)) } - if let remoteTarget, remoteHash != nil { + if let remoteTarget, let remoteHash { undoOperations.append( .pushBranch( remote: remoteTarget.remote, - localBranch: branch, - remoteBranch: remoteTarget.branch + localBranch: remoteHash, + remoteBranch: "refs/heads/\(remoteTarget.branch)" ) ) } diff --git a/macgit/Views/MainWindow/SidebarView+BranchActions.swift b/macgit/Views/MainWindow/SidebarView+BranchActions.swift index 1b43dbb2..a5994d01 100644 --- a/macgit/Views/MainWindow/SidebarView+BranchActions.swift +++ b/macgit/Views/MainWindow/SidebarView+BranchActions.swift @@ -223,12 +223,12 @@ extension SidebarView { if let upstream { undoOperations.append(.setUpstream(branch: branch, upstream: upstream)) } - if let remoteTarget, remoteHash != nil { + if let remoteTarget, let remoteHash { undoOperations.append( .pushBranch( remote: remoteTarget.remote, - localBranch: branch, - remoteBranch: remoteTarget.branch + localBranch: remoteHash, + remoteBranch: "refs/heads/\(remoteTarget.branch)" ) ) } diff --git a/macgitTests/GitUndoExecutorTests.swift b/macgitTests/GitUndoExecutorTests.swift index f642b56e..24ec7263 100644 --- a/macgitTests/GitUndoExecutorTests.swift +++ b/macgitTests/GitUndoExecutorTests.swift @@ -183,6 +183,29 @@ final class GitUndoExecutorTests: XCTestCase { ]) } + func testPushBranchCanRestoreRemoteBranchFromExactHash() async throws { + let runner = RecordingGitRunner() + let executor = GitUndoExecutor(runner: runner) + let repoURL = URL(fileURLWithPath: "/tmp/repo") + + try await executor.execute( + .pushBranch( + remote: "origin", + localBranch: "abc123", + remoteBranch: "refs/heads/feature" + ), + in: repoURL + ) + + let calls = await runner.recordedCalls() + XCTAssertEqual(calls, [ + GitCommandCall( + arguments: ["push", "origin", "abc123:refs/heads/feature"], + directory: repoURL + ) + ]) + } + func testSetUpstreamRunsGitBranchSetUpstreamTo() async throws { let runner = RecordingGitRunner() let executor = GitUndoExecutor(runner: runner) diff --git a/macgitTests/RemoteBranchDeleteResolverTests.swift b/macgitTests/RemoteBranchDeleteResolverTests.swift index e95c9c80..0f104c53 100644 --- a/macgitTests/RemoteBranchDeleteResolverTests.swift +++ b/macgitTests/RemoteBranchDeleteResolverTests.swift @@ -31,6 +31,16 @@ final class RemoteBranchDeleteResolverTests: XCTestCase { XCTAssertEqual(target, RemoteBranchDeleteTarget(remote: "origin", branch: "feature/demo")) } + func testFallsBackWhenTrackedUpstreamNoLongerExists() { + let target = RemoteBranchDeleteResolver.target( + localBranch: "feature/demo", + upstream: "fork/review/demo", + branchesByRemote: ["origin": ["feature/demo"]] + ) + + XCTAssertEqual(target, RemoteBranchDeleteTarget(remote: "origin", branch: "feature/demo")) + } + func testFallsBackToOnlyMatchingRemote() { let target = RemoteBranchDeleteResolver.target( localBranch: "feature/demo",