From b62cfc2d38c34699d91550d37b96f25c1a0b6dd5 Mon Sep 17 00:00:00 2001 From: Siim Raud Date: Sat, 28 Feb 2026 16:01:44 +0200 Subject: [PATCH 1/3] feat: replace sidebar NSOutlineView with SwiftUI List (#1/3) Replace the Editor.xib sidebar (NSOutlineView + Cell + ListController) with a SwiftUI List, following the same pattern established in PR #227 for URLSheet. New files: - HostsDataStore: ObservableObject singleton bridging ObjC notifications - HostsRowView: SF Symbol-based row replacing 400-line custom Cell drawing - SidebarView: SwiftUI List with drag-and-drop, context menus, inline rename - SidebarInstaller: @objc bridge that swaps the XIB pane at runtime ObjC changes: - HostsMainController: add selectHosts: for programmatic selection - ListController: add deactivate to remove duplicate notification observers - EditorController: call SidebarInstaller from awakeFromNib --- Gas Mask.xcodeproj/project.pbxproj | 20 ++ Source/EditorController.m | 3 + Source/HostsMainController.h | 1 + Source/HostsMainController.m | 10 + Source/ListController.h | 1 + Source/ListController.m | 5 + Source/Swift/GasMask-Bridging-Header.h | 10 + Source/Swift/HostsDataStore.swift | 147 ++++++++++++++ Source/Swift/HostsRowView.swift | 117 +++++++++++ Source/Swift/SidebarInstaller.swift | 18 ++ Source/Swift/SidebarView.swift | 199 +++++++++++++++++++ Tests/GasMaskTests/HostsDataStoreTests.swift | 56 ++++++ 12 files changed, 587 insertions(+) create mode 100644 Source/Swift/HostsDataStore.swift create mode 100644 Source/Swift/HostsRowView.swift create mode 100644 Source/Swift/SidebarInstaller.swift create mode 100644 Source/Swift/SidebarView.swift create mode 100644 Tests/GasMaskTests/HostsDataStoreTests.swift diff --git a/Gas Mask.xcodeproj/project.pbxproj b/Gas Mask.xcodeproj/project.pbxproj index c7152da..5522cc2 100644 --- a/Gas Mask.xcodeproj/project.pbxproj +++ b/Gas Mask.xcodeproj/project.pbxproj @@ -150,6 +150,11 @@ AA00000C000000000000AAAA /* URLValidatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA00000B000000000000AAAA /* URLValidatorTests.swift */; }; AA00000E000000000000AAAA /* URLSheetPresenterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA00000D000000000000AAAA /* URLSheetPresenterTests.swift */; }; AA000010000000000000AAAA /* URLSheetViewTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA00000F000000000000AAAA /* URLSheetViewTests.swift */; }; + AA000012000000000000AAAA /* HostsDataStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA000011000000000000AAAA /* HostsDataStore.swift */; }; + AA000014000000000000AAAA /* HostsRowView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA000013000000000000AAAA /* HostsRowView.swift */; }; + AA000016000000000000AAAA /* SidebarView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA000015000000000000AAAA /* SidebarView.swift */; }; + AA000018000000000000AAAA /* SidebarInstaller.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA000017000000000000AAAA /* SidebarInstaller.swift */; }; + AA00001A000000000000AAAA /* HostsDataStoreTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AA000019000000000000AAAA /* HostsDataStoreTests.swift */; }; /* Begin PBXContainerItemProxy section */ 353D18A01114C067005C4E54 /* PBXContainerItemProxy */ = { isa = PBXContainerItemProxy; @@ -340,6 +345,11 @@ AA00000B000000000000AAAA /* URLValidatorTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = URLValidatorTests.swift; sourceTree = ""; }; AA00000D000000000000AAAA /* URLSheetPresenterTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = URLSheetPresenterTests.swift; sourceTree = ""; }; AA00000F000000000000AAAA /* URLSheetViewTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = URLSheetViewTests.swift; sourceTree = ""; }; + AA000011000000000000AAAA /* HostsDataStore.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = HostsDataStore.swift; path = "Source/Swift/HostsDataStore.swift"; sourceTree = ""; }; + AA000013000000000000AAAA /* HostsRowView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = HostsRowView.swift; path = "Source/Swift/HostsRowView.swift"; sourceTree = ""; }; + AA000015000000000000AAAA /* SidebarView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = SidebarView.swift; path = "Source/Swift/SidebarView.swift"; sourceTree = ""; }; + AA000017000000000000AAAA /* SidebarInstaller.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = SidebarInstaller.swift; path = "Source/Swift/SidebarInstaller.swift"; sourceTree = ""; }; + AA000019000000000000AAAA /* HostsDataStoreTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HostsDataStoreTests.swift; sourceTree = ""; }; 35A183A71A0ACF37002D6289 /* menuIcon@2x.tiff */ = {isa = PBXFileReference; lastKnownFileType = image.tiff; name = "menuIcon@2x.tiff"; path = "Resources/Images/menuIcon@2x.tiff"; sourceTree = ""; }; 35A4CD2F1534927F005176BD /* Combined Hosts Hint.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; name = "Combined Hosts Hint.png"; path = "Resources/Images/Combined Hosts Hint.png"; sourceTree = ""; }; 35B0498E1A46234100EB89CA /* Editor.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; name = Editor.png; path = Resources/Images/Preferences/Editor.png; sourceTree = ""; }; @@ -784,6 +794,10 @@ AA000005000000000000AAAA /* NetworkStatusObserver.swift */, AA000007000000000000AAAA /* URLSheetView.swift */, AA000009000000000000AAAA /* URLSheetPresenter.swift */, + AA000011000000000000AAAA /* HostsDataStore.swift */, + AA000013000000000000AAAA /* HostsRowView.swift */, + AA000015000000000000AAAA /* SidebarView.swift */, + AA000017000000000000AAAA /* SidebarInstaller.swift */, ); name = Swift; sourceTree = ""; @@ -875,6 +889,7 @@ AA00000B000000000000AAAA /* URLValidatorTests.swift */, AA00000D000000000000AAAA /* URLSheetPresenterTests.swift */, AA00000F000000000000AAAA /* URLSheetViewTests.swift */, + AA000019000000000000AAAA /* HostsDataStoreTests.swift */, CC2B3C4D5E6F000100000010 /* NodeTests.m */, CC2B3C4D5E6F000100000011 /* HostsTests.m */, CC2B3C4D5E6F000100000012 /* HostsGroupTests.m */, @@ -1124,6 +1139,10 @@ AA000006000000000000AAAA /* NetworkStatusObserver.swift in Sources */, AA000008000000000000AAAA /* URLSheetView.swift in Sources */, AA00000A000000000000AAAA /* URLSheetPresenter.swift in Sources */, + AA000012000000000000AAAA /* HostsDataStore.swift in Sources */, + AA000014000000000000AAAA /* HostsRowView.swift in Sources */, + AA000016000000000000AAAA /* SidebarView.swift in Sources */, + AA000018000000000000AAAA /* SidebarInstaller.swift in Sources */, 356DB76A1824EAFD0020CEA0 /* ExtendedNSSplitView.m in Sources */, 35E9008A1147F42900851A25 /* MAAttachedWindow.m in Sources */, 350E7D3E121093E400D2F5F5 /* AlertBadge.m in Sources */, @@ -1165,6 +1184,7 @@ AA00000C000000000000AAAA /* URLValidatorTests.swift in Sources */, AA00000E000000000000AAAA /* URLSheetPresenterTests.swift in Sources */, AA000010000000000000AAAA /* URLSheetViewTests.swift in Sources */, + AA00001A000000000000AAAA /* HostsDataStoreTests.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/Source/EditorController.m b/Source/EditorController.m index 482aada..6c6895c 100644 --- a/Source/EditorController.m +++ b/Source/EditorController.m @@ -21,6 +21,7 @@ #import "EditorController.h" #import "Preferences.h" #import "ExtendedNSSplitView.h" +#import "Gas_Mask-Swift.h" #define SplitViewMinWidth 140 #define SplitViewMaxWidth 300 @@ -41,6 +42,8 @@ - (void)awakeFromNib if (position > SplitViewMaxWidth) { [splitView setPosition:SplitViewDefaultWidth ofDividerAtIndex:dividerIndex]; } + + [SidebarInstaller installIn:splitView]; } #pragma mark - Split View Delegate diff --git a/Source/HostsMainController.h b/Source/HostsMainController.h index 4d0c0dc..e8d7ba3 100644 --- a/Source/HostsMainController.h +++ b/Source/HostsMainController.h @@ -132,6 +132,7 @@ - (Hosts*)activeHostsFile; - (Hosts*)selectedHosts; +- (void)selectHosts:(Hosts*)hosts; - (NSArray*)allHostsFilesGrouped; - (NSArray*)allHostsFiles; - (int)filesCount; diff --git a/Source/HostsMainController.m b/Source/HostsMainController.m index 05f3641..cd485fc 100644 --- a/Source/HostsMainController.m +++ b/Source/HostsMainController.m @@ -441,6 +441,16 @@ - (Hosts*)selectedHosts return [[self selectedObjects] lastObject]; } +- (void)selectHosts:(Hosts *)hosts +{ + NSIndexPath *path = [self hostsIndexPath:hosts]; + if (path) { + dispatch_async(dispatch_get_main_queue(), ^{ + [self setSelectionIndexPath:path]; + }); + } +} + - (NSArray*)allHostsFilesGrouped { int nrControllers = [controllers count]; diff --git a/Source/ListController.h b/Source/ListController.h index efdce49..6129004 100644 --- a/Source/ListController.h +++ b/Source/ListController.h @@ -31,5 +31,6 @@ + (ListController*)defaultInstance; - (Hosts*)selectedHosts; +- (void)deactivate; @end diff --git a/Source/ListController.m b/Source/ListController.m index af67526..f1b6b22 100644 --- a/Source/ListController.m +++ b/Source/ListController.m @@ -84,6 +84,11 @@ - (void)awakeFromNib [self selectActiveHostsFile]; } +- (void)deactivate +{ + [[NSNotificationCenter defaultCenter] removeObserver:self]; +} + - (void)updateItem:(NSNotification *)notification { int index = [self indexOfHosts:[notification object]]; diff --git a/Source/Swift/GasMask-Bridging-Header.h b/Source/Swift/GasMask-Bridging-Header.h index 4e8bda7..559a634 100644 --- a/Source/Swift/GasMask-Bridging-Header.h +++ b/Source/Swift/GasMask-Bridging-Header.h @@ -1,4 +1,14 @@ #import #import "Network.h" +#import "Node.h" +#import "Hosts.h" +#import "HostsGroup.h" +#import "RemoteHosts.h" +#import "CombinedHosts.h" #import "HostsMainController.h" #import "RemoteHostsController.h" +#import "Preferences.h" +#import "Error.h" +#import "LocalHostsController.h" +#import "CombinedHostsController.h" +#import "ListController.h" diff --git a/Source/Swift/HostsDataStore.swift b/Source/Swift/HostsDataStore.swift new file mode 100644 index 0000000..15db574 --- /dev/null +++ b/Source/Swift/HostsDataStore.swift @@ -0,0 +1,147 @@ +import Foundation +import Combine + +// MARK: - Notification Names +// These are defined as #define macros in Gas_Mask_Prefix.pch, which Swift cannot import. +// Mirror them here as NSNotification.Name constants. + +extension NSNotification.Name { + static let hostsFileCreated = NSNotification.Name("HostsFileCreatedNotification") + static let hostsFileRemoved = NSNotification.Name("HostsFileRemovedNotification") + static let hostsFileRenamed = NSNotification.Name("HostsFileRenamedNotification") + static let hostsFileSaved = NSNotification.Name("HostsFileSavedNotification") + static let hostsNodeNeedsUpdate = NSNotification.Name("HostsNodeNeedsUpdateNotification") + static let hostsFileShouldBeRenamed = NSNotification.Name("HostsFileShouldBeRenamedNotification") + static let hostsFileShouldBeSelected = NSNotification.Name("HostsFileShouldBeSelectedNotification") + static let synchronizingStatusChanged = NSNotification.Name("SynchronizingStatusChangedNotification") + static let allHostsFilesLoadedFromDisk = NSNotification.Name("AllHostsFilesLoadedFromDiskNotification") +} + +// MARK: - HostsDataStore + +final class HostsDataStore: ObservableObject { + + static let shared = HostsDataStore() + + // MARK: Published Properties + + @Published var hostsGroups: [HostsGroup] = [] + @Published var selectedHosts: Hosts? { + didSet { + guard selectedHosts !== oldValue, !isSyncingSelection else { return } + if let hosts = selectedHosts { + HostsMainController.defaultInstance()?.select(hosts) + } + } + } + @Published var filesCount: Int = 0 + @Published var canRemoveFiles: Bool = false + @Published var renamingHosts: Hosts? + + // MARK: Private + + private var notificationObservers: [NSObjectProtocol] = [] + private var isSyncingSelection = false + + // MARK: Init + + private init() { + refreshGroups() + refreshFilesCount() + observeNotifications() + } + + deinit { + notificationObservers.forEach { NotificationCenter.default.removeObserver($0) } + } + + // MARK: Refresh + + func refreshGroups() { + guard let controller = HostsMainController.defaultInstance(), + let content = controller.content as? [HostsGroup] else { + hostsGroups = [] + return + } + hostsGroups = content + } + + private func refreshFilesCount() { + guard let controller = HostsMainController.defaultInstance() else { return } + filesCount = Int(controller.filesCount()) + canRemoveFiles = controller.canRemoveFiles() + } + + // MARK: Selection Sync + + /// Called when the ObjC layer selects a hosts file (via notification). + private func syncSelectionFromModel(_ hosts: Hosts?) { + isSyncingSelection = true + selectedHosts = hosts + isSyncingSelection = false + } + + // MARK: Notification Observers + + private func observeNotifications() { + let nc = NotificationCenter.default + + // Data change notifications — refresh groups and counts + let refreshNames: [NSNotification.Name] = [ + .hostsFileCreated, + .hostsFileRemoved, + .hostsFileRenamed, + .allHostsFilesLoadedFromDisk + ] + + for name in refreshNames { + let observer = nc.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in + self?.refreshGroups() + self?.refreshFilesCount() + } + notificationObservers.append(observer) + } + + // Single-row refresh notifications + let rowRefreshNames: [NSNotification.Name] = [ + .hostsFileSaved, + .hostsNodeNeedsUpdate, + .synchronizingStatusChanged + ] + + for name in rowRefreshNames { + let observer = nc.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in + // Trigger a refresh by reassigning hostsGroups to force SwiftUI update + self?.objectWillChange.send() + } + notificationObservers.append(observer) + } + + // UI action notifications + let renameObserver = nc.addObserver( + forName: .hostsFileShouldBeRenamed, object: nil, queue: .main + ) { [weak self] notification in + self?.renamingHosts = notification.object as? Hosts + } + notificationObservers.append(renameObserver) + + let selectObserver = nc.addObserver( + forName: .hostsFileShouldBeSelected, object: nil, queue: .main + ) { [weak self] notification in + if let hosts = notification.object as? Hosts { + self?.syncSelectionFromModel(hosts) + } + } + notificationObservers.append(selectObserver) + + // After all files loaded, select the active one + let loadedObserver = nc.addObserver( + forName: .allHostsFilesLoadedFromDisk, object: nil, queue: .main + ) { [weak self] _ in + let active = HostsMainController.defaultInstance()?.activeHostsFile() + self?.syncSelectionFromModel(active) + } + notificationObservers.append(loadedObserver) + } + +} diff --git a/Source/Swift/HostsRowView.swift b/Source/Swift/HostsRowView.swift new file mode 100644 index 0000000..e7dfd86 --- /dev/null +++ b/Source/Swift/HostsRowView.swift @@ -0,0 +1,117 @@ +import SwiftUI + +struct HostsRowView: View { + let hosts: Hosts + let isGroup: Bool + + var body: some View { + if isGroup { + groupRow + } else { + fileRow + } + } + + // MARK: - Group Row + + private var groupRow: some View { + HStack(spacing: 4) { + Text(hosts.name() ?? "") + .font(.system(size: NSFont.smallSystemFontSize, weight: .semibold)) + Spacer() + groupBadges + } + } + + @ViewBuilder + private var groupBadges: some View { + if let group = hosts as? HostsGroup { + if !group.online() { + Image(systemName: "wifi.slash") + .font(.system(size: 10)) + .foregroundStyle(.secondary) + .help("Offline") + } + if group.synchronizing() { + ProgressView() + .controlSize(.small) + .help("Synchronizing") + } + } + if let error = hosts.error() { + Image(systemName: "exclamationmark.triangle.fill") + .font(.system(size: 10)) + .foregroundStyle(.yellow) + .help(error.description ?? "Error") + } + } + + // MARK: - File Row + + private var fileRow: some View { + HStack(spacing: 4) { + if hosts.active() { + Circle() + .fill(.green) + .frame(width: 6, height: 6) + .accessibilityLabel("Active") + } + + fileIcon + .frame(width: 16, height: 16) + + Text(hosts.name() ?? "") + .font(.system(size: NSFont.smallSystemFontSize)) + .lineLimit(1) + .opacity(hosts.enabled() ? 1.0 : 0.5) + + Spacer() + + trailingBadges + } + .accessibilityElement(children: .combine) + .accessibilityLabel(accessibilityDescription) + } + + @ViewBuilder + private var fileIcon: some View { + if hosts is CombinedHosts { + Image(systemName: "doc.on.doc") + .foregroundStyle(.secondary) + } else if hosts is RemoteHosts { + Image(systemName: "globe") + .foregroundStyle(hosts.enabled() ? .secondary : .tertiary) + } else { + Image(systemName: "doc") + .foregroundStyle(.secondary) + } + } + + @ViewBuilder + private var trailingBadges: some View { + if let error = hosts.error() { + Image(systemName: "exclamationmark.triangle.fill") + .font(.system(size: 10)) + .foregroundStyle(.yellow) + .help(error.description ?? "Error") + } + if !hosts.saved() { + Circle() + .fill(.blue) + .frame(width: 6, height: 6) + .accessibilityLabel("Unsaved") + } + } + + // MARK: - Accessibility + + private var accessibilityDescription: String { + var parts: [String] = [] + parts.append(hosts.name() ?? "") + if hosts.active() { parts.append("active") } + if !hosts.saved() { parts.append("unsaved") } + if hosts.error() != nil { parts.append("has error") } + if !hosts.enabled() { parts.append("disabled") } + return parts.joined(separator: ", ") + } +} diff --git a/Source/Swift/SidebarInstaller.swift b/Source/Swift/SidebarInstaller.swift new file mode 100644 index 0000000..07d5725 --- /dev/null +++ b/Source/Swift/SidebarInstaller.swift @@ -0,0 +1,18 @@ +import SwiftUI + +@objc final class SidebarInstaller: NSObject { + @objc static func install(in splitView: NSSplitView) { + let store = HostsDataStore.shared + let sidebar = SidebarView(store: store) + let hostingView = NSHostingView(rootView: sidebar) + + guard !splitView.subviews.isEmpty else { return } + let leftPane = splitView.subviews[0] + hostingView.frame = leftPane.frame + hostingView.autoresizingMask = [.width, .height] + splitView.replaceSubview(leftPane, with: hostingView) + + // Deactivate the old ListController to prevent duplicate notification handling + ListController.defaultInstance()?.deactivate() + } +} diff --git a/Source/Swift/SidebarView.swift b/Source/Swift/SidebarView.swift new file mode 100644 index 0000000..52969ce --- /dev/null +++ b/Source/Swift/SidebarView.swift @@ -0,0 +1,199 @@ +import SwiftUI +import UniformTypeIdentifiers + +struct SidebarView: View { + @ObservedObject var store: HostsDataStore + + @State private var editingName: String = "" + @State private var isEditing = false + @State private var renameError: String? + + var body: some View { + List(selection: $store.selectedHosts) { + ForEach(store.hostsGroups, id: \.self) { group in + let children = (group.children as? [Hosts]) ?? [] + if !children.isEmpty { + Section(header: HostsRowView(hosts: group, isGroup: true)) { + ForEach(children, id: \.self) { hosts in + rowContent(for: hosts) + .tag(hosts) + } + } + } + } + } + .listStyle(.sidebar) + .onDrop(of: [.fileURL, .url, .utf8PlainText], delegate: SidebarDropDelegate(store: store)) + .onChange(of: store.renamingHosts) { newValue in + if let hosts = newValue { + beginRename(hosts) + } + } + .alert("Rename Error", isPresented: Binding( + get: { renameError != nil }, + set: { if !$0 { renameError = nil } } + )) { + Button("OK") { renameError = nil } + } message: { + Text(renameError ?? "") + } + } + + // MARK: - Row Content + + @ViewBuilder + private func rowContent(for hosts: Hosts) -> some View { + if isEditing, store.renamingHosts === hosts { + renameField(for: hosts) + } else { + HostsRowView(hosts: hosts, isGroup: false) + .draggable(hosts.contents() ?? "") { + Text(hosts.name() ?? "") + } + .contextMenu { contextMenuItems(for: hosts) } + } + } + + // MARK: - Inline Rename + + private func renameField(for hosts: Hosts) -> some View { + TextField("Name", text: $editingName, onCommit: { + commitRename(hosts) + }) + .textFieldStyle(.plain) + .font(.system(size: NSFont.smallSystemFontSize)) + .onExitCommand { + cancelRename() + } + } + + private func beginRename(_ hosts: Hosts) { + editingName = hosts.name() ?? "" + isEditing = true + } + + private func commitRename(_ hosts: Hosts) { + defer { + isEditing = false + store.renamingHosts = nil + } + + let trimmed = editingName.trimmingCharacters(in: .whitespaces) + guard !trimmed.isEmpty else { return } + + if trimmed.contains("/") { + renameError = "File name cannot contain forward slash." + return + } + + guard let controller = HostsMainController.defaultInstance() else { return } + let renamed = controller.rename(hosts, to: trimmed) + if renamed { + NotificationCenter.default.post(name: .hostsFileRenamed, object: hosts) + } else { + renameError = "A file with that name already exists." + } + } + + private func cancelRename() { + isEditing = false + store.renamingHosts = nil + } + + // MARK: - Context Menu + + @ViewBuilder + private func contextMenuItems(for hosts: Hosts) -> some View { + if !hosts.saved() { + Button("Save") { + HostsMainController.defaultInstance()?.save(hosts) + } + } + + if !hosts.active() { + Button("Activate") { + HostsMainController.defaultInstance()?.activateHostsFile(hosts) + } + .disabled(!hosts.exists) + } + + Button("Show In Finder") { + if let path = hosts.path { + NSWorkspace.shared.selectFile(path, inFileViewerRootedAtPath: "") + } + } + .disabled(!hosts.exists) + + if let remote = hosts as? RemoteHosts { + Button("Open in Browser") { + if let url = remote.url { + NSWorkspace.shared.open(url) + } + } + } + + Divider() + + if hosts is RemoteHosts { + Button("Move to Local") { + HostsMainController.defaultInstance()?.move(hosts, toControllerClass: LocalHostsController.self) + } + .disabled(!hosts.exists) + } + + Button("Rename") { + store.renamingHosts = hosts + } + + Divider() + + if store.canRemoveFiles { + Button("Remove") { + HostsMainController.defaultInstance()?.removeHostsFile(hosts, moveToTrash: false) + } + } + } +} + +// MARK: - Drop Support + +extension SidebarView { + + struct SidebarDropDelegate: DropDelegate { + let store: HostsDataStore + + func validateDrop(info: DropInfo) -> Bool { + info.hasItemsConforming(to: [.fileURL, .url, .utf8PlainText]) + } + + func performDrop(info: DropInfo) -> Bool { + guard let controller = HostsMainController.defaultInstance() else { return false } + + let providers = info.itemProviders(for: [.fileURL, .url, .utf8PlainText]) + for provider in providers { + if provider.canLoadObject(ofClass: URL.self) { + provider.loadObject(ofClass: URL.self) { url, error in + if let error { + NSLog("Drop URL load failed: %@", error.localizedDescription) + return + } + guard let url else { return } + DispatchQueue.main.async { + if url.isFileURL { + _ = controller.createHosts(fromLocalURL: url, to: defaultGroup()) + } else { + _ = controller.createHosts(from: url, to: defaultGroup()) + } + } + } + return true + } + } + return false + } + + private func defaultGroup() -> HostsGroup? { + store.hostsGroups.first + } + } +} diff --git a/Tests/GasMaskTests/HostsDataStoreTests.swift b/Tests/GasMaskTests/HostsDataStoreTests.swift new file mode 100644 index 0000000..9020831 --- /dev/null +++ b/Tests/GasMaskTests/HostsDataStoreTests.swift @@ -0,0 +1,56 @@ +import XCTest +@testable import Gas_Mask + +final class HostsDataStoreTests: XCTestCase { + + // MARK: - Notification Name Constants + + /// Verify Swift notification names match the ObjC #define values from Gas_Mask_Prefix.pch + func testNotificationNames_matchObjCDefines() { + XCTAssertEqual(NSNotification.Name.hostsFileCreated.rawValue, "HostsFileCreatedNotification") + XCTAssertEqual(NSNotification.Name.hostsFileRemoved.rawValue, "HostsFileRemovedNotification") + XCTAssertEqual(NSNotification.Name.hostsFileRenamed.rawValue, "HostsFileRenamedNotification") + XCTAssertEqual(NSNotification.Name.hostsFileSaved.rawValue, "HostsFileSavedNotification") + XCTAssertEqual(NSNotification.Name.hostsNodeNeedsUpdate.rawValue, "HostsNodeNeedsUpdateNotification") + XCTAssertEqual(NSNotification.Name.hostsFileShouldBeRenamed.rawValue, "HostsFileShouldBeRenamedNotification") + XCTAssertEqual(NSNotification.Name.hostsFileShouldBeSelected.rawValue, "HostsFileShouldBeSelectedNotification") + XCTAssertEqual(NSNotification.Name.synchronizingStatusChanged.rawValue, "SynchronizingStatusChangedNotification") + XCTAssertEqual(NSNotification.Name.allHostsFilesLoadedFromDisk.rawValue, "AllHostsFilesLoadedFromDiskNotification") + } + + // MARK: - Singleton + + func testShared_returnsSameInstance() { + let a = HostsDataStore.shared + let b = HostsDataStore.shared + XCTAssertTrue(a === b) + } + + // MARK: - Notification Response + + func testRenameNotification_setsRenamingHosts() { + let store = HostsDataStore.shared + addTeardownBlock { store.renamingHosts = nil } + + let hosts = Hosts(path: "/tmp/test.hst")! + store.renamingHosts = nil + + NotificationCenter.default.post(name: .hostsFileShouldBeRenamed, object: hosts) + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.1)) + + XCTAssertTrue(store.renamingHosts === hosts) + } + + func testSelectNotification_updatesSelectedHosts() { + let store = HostsDataStore.shared + addTeardownBlock { store.selectedHosts = nil } + + let hosts = Hosts(path: "/tmp/test.hst")! + store.selectedHosts = nil + + NotificationCenter.default.post(name: .hostsFileShouldBeSelected, object: hosts) + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.1)) + + XCTAssertTrue(store.selectedHosts === hosts) + } +} From 912434a4a9707af97474128ede97eeebc55c6218 Mon Sep 17 00:00:00 2001 From: Siim Raud Date: Sat, 28 Feb 2026 16:48:53 +0200 Subject: [PATCH 2/3] fix: address PR review feedback for SwiftUI sidebar - Add confirmation dialog before removing hosts files - Move .onDrop to per-Section for group-aware drops (files land in the correct group instead of always the first one) - Handle multi-file drops (process all URL providers, not just first) - Remove .utf8PlainText from drop types (was accepted but never handled) - Show empty group sections so users can drop into them - Replace deprecated onCommit with .onSubmit - Suppress unused result warning on loadObject --- Source/Swift/SidebarView.swift | 57 ++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/Source/Swift/SidebarView.swift b/Source/Swift/SidebarView.swift index 52969ce..7e98f37 100644 --- a/Source/Swift/SidebarView.swift +++ b/Source/Swift/SidebarView.swift @@ -7,23 +7,22 @@ struct SidebarView: View { @State private var editingName: String = "" @State private var isEditing = false @State private var renameError: String? + @State private var hostsToRemove: Hosts? var body: some View { List(selection: $store.selectedHosts) { ForEach(store.hostsGroups, id: \.self) { group in let children = (group.children as? [Hosts]) ?? [] - if !children.isEmpty { - Section(header: HostsRowView(hosts: group, isGroup: true)) { - ForEach(children, id: \.self) { hosts in - rowContent(for: hosts) - .tag(hosts) - } + Section(header: HostsRowView(hosts: group, isGroup: true)) { + ForEach(children, id: \.self) { hosts in + rowContent(for: hosts) + .tag(hosts) } } + .onDrop(of: [.fileURL, .url], delegate: SidebarDropDelegate(group: group)) } } .listStyle(.sidebar) - .onDrop(of: [.fileURL, .url, .utf8PlainText], delegate: SidebarDropDelegate(store: store)) .onChange(of: store.renamingHosts) { newValue in if let hosts = newValue { beginRename(hosts) @@ -37,6 +36,20 @@ struct SidebarView: View { } message: { Text(renameError ?? "") } + .alert("Remove Hosts File", isPresented: Binding( + get: { hostsToRemove != nil }, + set: { if !$0 { hostsToRemove = nil } } + )) { + Button("Cancel", role: .cancel) { hostsToRemove = nil } + Button("Remove", role: .destructive) { + if let hosts = hostsToRemove { + HostsMainController.defaultInstance()?.removeHostsFile(hosts, moveToTrash: false) + } + hostsToRemove = nil + } + } message: { + Text("Are you sure you want to remove \"\(hostsToRemove?.name() ?? "")\"?") + } } // MARK: - Row Content @@ -57,11 +70,12 @@ struct SidebarView: View { // MARK: - Inline Rename private func renameField(for hosts: Hosts) -> some View { - TextField("Name", text: $editingName, onCommit: { - commitRename(hosts) - }) + TextField("Name", text: $editingName) .textFieldStyle(.plain) .font(.system(size: NSFont.smallSystemFontSize)) + .onSubmit { + commitRename(hosts) + } .onExitCommand { cancelRename() } @@ -149,7 +163,7 @@ struct SidebarView: View { if store.canRemoveFiles { Button("Remove") { - HostsMainController.defaultInstance()?.removeHostsFile(hosts, moveToTrash: false) + hostsToRemove = hosts } } } @@ -160,19 +174,21 @@ struct SidebarView: View { extension SidebarView { struct SidebarDropDelegate: DropDelegate { - let store: HostsDataStore + let group: HostsGroup func validateDrop(info: DropInfo) -> Bool { - info.hasItemsConforming(to: [.fileURL, .url, .utf8PlainText]) + info.hasItemsConforming(to: [.fileURL, .url]) } func performDrop(info: DropInfo) -> Bool { guard let controller = HostsMainController.defaultInstance() else { return false } - let providers = info.itemProviders(for: [.fileURL, .url, .utf8PlainText]) + let providers = info.itemProviders(for: [.fileURL, .url]) + var handled = false for provider in providers { if provider.canLoadObject(ofClass: URL.self) { - provider.loadObject(ofClass: URL.self) { url, error in + handled = true + _ = provider.loadObject(ofClass: URL.self) { url, error in if let error { NSLog("Drop URL load failed: %@", error.localizedDescription) return @@ -180,20 +196,15 @@ extension SidebarView { guard let url else { return } DispatchQueue.main.async { if url.isFileURL { - _ = controller.createHosts(fromLocalURL: url, to: defaultGroup()) + _ = controller.createHosts(fromLocalURL: url, to: group) } else { - _ = controller.createHosts(from: url, to: defaultGroup()) + _ = controller.createHosts(from: url, to: group) } } } - return true } } - return false - } - - private func defaultGroup() -> HostsGroup? { - store.hostsGroups.first + return handled } } } From d89316701f759456114f8b0c9407e5a0135d27d7 Mon Sep 17 00:00:00 2001 From: Siim Raud Date: Sat, 28 Feb 2026 17:34:42 +0200 Subject: [PATCH 3/3] fix: address code review feedback for SwiftUI sidebar - Combine duplicate .allHostsFilesLoadedFromDisk observer into one - Remove unreachable deinit from singleton HostsDataStore - Fix row refresh: reassign hostsGroups instead of objectWillChange.send() - Change removeHostsFile to moveToTrash:true for user recovery - Add comment explaining dispatch_async in selectHosts: --- Source/HostsMainController.m | 2 ++ Source/Swift/HostsDataStore.swift | 33 ++++++++++++++----------------- Source/Swift/SidebarView.swift | 4 ++-- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/Source/HostsMainController.m b/Source/HostsMainController.m index cd485fc..03286cc 100644 --- a/Source/HostsMainController.m +++ b/Source/HostsMainController.m @@ -445,6 +445,8 @@ - (void)selectHosts:(Hosts *)hosts { NSIndexPath *path = [self hostsIndexPath:hosts]; if (path) { + // Defer to next run loop iteration so the tree controller + // finishes any in-flight insert/remove before we change selection. dispatch_async(dispatch_get_main_queue(), ^{ [self setSelectionIndexPath:path]; }); diff --git a/Source/Swift/HostsDataStore.swift b/Source/Swift/HostsDataStore.swift index 15db574..639a070 100644 --- a/Source/Swift/HostsDataStore.swift +++ b/Source/Swift/HostsDataStore.swift @@ -51,10 +51,6 @@ final class HostsDataStore: ObservableObject { observeNotifications() } - deinit { - notificationObservers.forEach { NotificationCenter.default.removeObserver($0) } - } - // MARK: Refresh func refreshGroups() { @@ -90,8 +86,7 @@ final class HostsDataStore: ObservableObject { let refreshNames: [NSNotification.Name] = [ .hostsFileCreated, .hostsFileRemoved, - .hostsFileRenamed, - .allHostsFilesLoadedFromDisk + .hostsFileRenamed ] for name in refreshNames { @@ -102,7 +97,18 @@ final class HostsDataStore: ObservableObject { notificationObservers.append(observer) } - // Single-row refresh notifications + // All files loaded — refresh data then select the active file + let loadedObserver = nc.addObserver( + forName: .allHostsFilesLoadedFromDisk, object: nil, queue: .main + ) { [weak self] _ in + self?.refreshGroups() + self?.refreshFilesCount() + let active = HostsMainController.defaultInstance()?.activeHostsFile() + self?.syncSelectionFromModel(active) + } + notificationObservers.append(loadedObserver) + + // Single-row refresh notifications — reassign hostsGroups to force SwiftUI diffing let rowRefreshNames: [NSNotification.Name] = [ .hostsFileSaved, .hostsNodeNeedsUpdate, @@ -111,8 +117,8 @@ final class HostsDataStore: ObservableObject { for name in rowRefreshNames { let observer = nc.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in - // Trigger a refresh by reassigning hostsGroups to force SwiftUI update - self?.objectWillChange.send() + guard let self else { return } + self.hostsGroups = self.hostsGroups } notificationObservers.append(observer) } @@ -133,15 +139,6 @@ final class HostsDataStore: ObservableObject { } } notificationObservers.append(selectObserver) - - // After all files loaded, select the active one - let loadedObserver = nc.addObserver( - forName: .allHostsFilesLoadedFromDisk, object: nil, queue: .main - ) { [weak self] _ in - let active = HostsMainController.defaultInstance()?.activeHostsFile() - self?.syncSelectionFromModel(active) - } - notificationObservers.append(loadedObserver) } } diff --git a/Source/Swift/SidebarView.swift b/Source/Swift/SidebarView.swift index 7e98f37..4002d9a 100644 --- a/Source/Swift/SidebarView.swift +++ b/Source/Swift/SidebarView.swift @@ -43,12 +43,12 @@ struct SidebarView: View { Button("Cancel", role: .cancel) { hostsToRemove = nil } Button("Remove", role: .destructive) { if let hosts = hostsToRemove { - HostsMainController.defaultInstance()?.removeHostsFile(hosts, moveToTrash: false) + HostsMainController.defaultInstance()?.removeHostsFile(hosts, moveToTrash: true) } hostsToRemove = nil } } message: { - Text("Are you sure you want to remove \"\(hostsToRemove?.name() ?? "")\"?") + Text("Are you sure you want to remove \"\(hostsToRemove?.name() ?? "")\"? The file will be moved to Trash.") } }