Merge pull request #282 from datlechin/refactor/remove-dead-notifications

datlechin · web-flow · commit 8ba85ceee5db · 2026-03-12T10:28:23.000+07:00
refactor: replace settings notifications with @observable and direct calls
diff --git a/TablePro/Core/Services/Infrastructure/SettingsNotifications.swift b/TablePro/Core/Services/Infrastructure/SettingsNotifications.swift
@@ -2,78 +2,22 @@
 //  SettingsNotifications.swift
 //  TablePro
 //
-//  Notification names and payload structures for settings changes.
-//  Follows existing NotificationCenter pattern used throughout the app.
+//  Notification names for settings changes that require AppKit bridging.
+//  SwiftUI views observe @Observable AppSettingsManager directly instead.
 //
 
 import Foundation
 
-// MARK: - Settings Notification Names
-
 extension Notification.Name {
-    // MARK: - Domain-Specific Notifications
-
     /// Posted when data grid settings change (row height, date format, etc.)
+    /// Used by AppKit components that cannot observe @Observable directly.
     static let dataGridSettingsDidChange = Notification.Name("dataGridSettingsDidChange")
 
-    /// Posted when history settings change (retention, auto-cleanup, etc.)
-    static let historySettingsDidChange = Notification.Name("historySettingsDidChange")
-
     /// Posted when editor settings change (font, line numbers, etc.)
+    /// Used by AppKit components that cannot observe @Observable directly.
     static let editorSettingsDidChange = Notification.Name("editorSettingsDidChange")
 
-    /// Posted when appearance settings change (theme, accent color)
-    static let appearanceSettingsDidChange = Notification.Name("appearanceSettingsDidChange")
-
-    /// Posted when general settings change (startup behavior, confirmations)
-    static let generalSettingsDidChange = Notification.Name("generalSettingsDidChange")
-
-    /// Posted when tab settings change (reuse behavior, etc.)
-    static let tabSettingsDidChange = Notification.Name("tabSettingsDidChange")
-
-    /// Posted when keyboard shortcut settings change
-    static let keyboardSettingsDidChange = Notification.Name("keyboardSettingsDidChange")
-
-    /// Posted when AI settings change (providers, routing, context options)
-    static let aiSettingsDidChange = Notification.Name("aiSettingsDidChange")
-
-    // MARK: - System Notifications
-
     /// Posted when the system accessibility text size preference changes.
     /// Observers should reload fonts via SQLEditorTheme.reloadFromSettings().
     static let accessibilityTextSizeDidChange = Notification.Name("accessibilityTextSizeDidChange")
-
-    // MARK: - Generic Notification
-
-    /// Posted for any settings change (in addition to domain-specific notification)
-    /// Use this to listen for all settings changes regardless of domain
-    static let settingsDidChange = Notification.Name("settingsDidChange")
-}
-
-// MARK: - Settings Change Info
-
-/// Information about a settings change included in notification userInfo
-struct SettingsChangeInfo {
-    /// The settings domain that changed (e.g., "general", "dataGrid", "history")
-    let domain: String
-
-    /// Optional set of specific keys that changed within the domain
-    /// If nil, assume all settings in the domain may have changed
-    let changedKeys: Set<String>?
-
-    /// User info dictionary key for accessing SettingsChangeInfo
-    static let userInfoKey = "changeInfo"
-}
-
-// MARK: - Convenience Extensions
-
-extension Notification {
-    /// Extract SettingsChangeInfo from notification's userInfo
-    var settingsChangeInfo: SettingsChangeInfo? {
-        guard let userInfo = userInfo,
-              let changeInfo = userInfo[SettingsChangeInfo.userInfoKey] as? SettingsChangeInfo else {
-            return nil
-        }
-        return changeInfo
-    }
 }
diff --git a/TablePro/Core/Storage/AppSettingsManager.swift b/TablePro/Core/Storage/AppSettingsManager.swift
@@ -23,15 +23,13 @@ final class AppSettingsManager {
         didSet {
             general.language.apply()
             storage.saveGeneral(general)
-            notifyChange(domain: "general", notification: .generalSettingsDidChange)
         }
     }
 
     var appearance: AppearanceSettings {
         didSet {
             storage.saveAppearance(appearance)
             appearance.theme.apply()
-            notifyChange(domain: "appearance", notification: .appearanceSettingsDidChange)
         }
     }
 
@@ -40,7 +38,7 @@ final class AppSettingsManager {
             storage.saveEditor(editor)
             // Update cached theme values for thread-safe access
             SQLEditorTheme.reloadFromSettings(editor)
-            notifyChange(domain: "editor", notification: .editorSettingsDidChange)
+            notifyChange(.editorSettingsDidChange)
         }
     }
 
@@ -62,7 +60,7 @@ final class AppSettingsManager {
             storage.saveDataGrid(validated)
             // Update date formatting service with new format
             DateFormattingService.shared.updateFormat(validated.dateFormat)
-            notifyChange(domain: "dataGrid", notification: .dataGridSettingsDidChange)
+            notifyChange(.dataGridSettingsDidChange)
         }
     }
 
@@ -84,28 +82,24 @@ final class AppSettingsManager {
             storage.saveHistory(validated)
             // Apply history settings immediately (cleanup if auto-cleanup enabled)
             Task { await applyHistorySettingsImmediately() }
-            notifyChange(domain: "history", notification: .historySettingsDidChange)
         }
     }
 
     var tabs: TabSettings {
         didSet {
             storage.saveTabs(tabs)
-            notifyChange(domain: "tabs", notification: .tabSettingsDidChange)
         }
     }
 
     var keyboard: KeyboardSettings {
         didSet {
             storage.saveKeyboard(keyboard)
-            notifyChange(domain: "keyboard", notification: .keyboardSettingsDidChange)
         }
     }
 
     var ai: AISettings {
         didSet {
             storage.saveAI(ai)
-            notifyChange(domain: "ai", notification: .aiSettingsDidChange)
         }
     }
 
@@ -147,24 +141,8 @@ final class AppSettingsManager {
 
     // MARK: - Notification Propagation
 
-    /// Notify listeners that settings have changed
-    /// Posts both domain-specific and generic notifications
-    private func notifyChange(domain: String, notification: Notification.Name) {
-        let changeInfo = SettingsChangeInfo(domain: domain, changedKeys: nil)
-
-        // Post domain-specific notification
-        NotificationCenter.default.post(
-            name: notification,
-            object: self,
-            userInfo: [SettingsChangeInfo.userInfoKey: changeInfo]
-        )
-
-        // Post generic notification for listeners that want all settings changes
-        NotificationCenter.default.post(
-            name: .settingsDidChange,
-            object: self,
-            userInfo: [SettingsChangeInfo.userInfoKey: changeInfo]
-        )
+    private func notifyChange(_ notification: Notification.Name) {
+        NotificationCenter.default.post(name: notification, object: self)
     }
 
     // MARK: - Accessibility Text Size
@@ -197,11 +175,8 @@ final class AppSettingsManager {
         }
     }
 
-    /// Apply history settings immediately (triggered on settings change)
     private func applyHistorySettingsImmediately() async {
-        // This will be called by QueryHistoryManager
-        // We post a notification and let the manager handle the actual cleanup
-        // This keeps the settings manager decoupled from history storage implementation
+        QueryHistoryManager.shared.applySettingsChange()
     }
 
     // MARK: - Actions
diff --git a/TablePro/Core/Storage/QueryHistoryManager.swift b/TablePro/Core/Storage/QueryHistoryManager.swift
@@ -3,10 +3,8 @@
 //  TablePro
 //
 //  Thread-safe coordinator for query history
-//  Communicates via NotificationCenter (NOT ObservableObject)
 //
 
-import Combine
 import Foundation
 
 /// Thread-safe manager for query history
@@ -16,32 +14,13 @@ final class QueryHistoryManager {
 
     private let storage: QueryHistoryStorage
 
-    // Settings observer for immediate cleanup when settings change
-    private var settingsObserver: AnyCancellable?
-
     /// Creates an isolated manager with its own storage. For testing only.
     init(isolatedStorage: QueryHistoryStorage) {
         self.storage = isolatedStorage
     }
 
     private init() {
         self.storage = QueryHistoryStorage.shared
-        // Subscribe to history settings changes for immediate cleanup
-        settingsObserver = NotificationCenter.default.publisher(for: .historySettingsDidChange)
-            .receive(on: DispatchQueue.main)
-            .sink { [weak self] _ in
-                guard let self else { return }
-
-                MainActor.assumeIsolated {
-                    // Update settings cache
-                    self.storage.updateSettingsCache()
-
-                    // Perform cleanup if auto-cleanup is enabled
-                    if AppSettingsManager.shared.history.autoCleanup {
-                        self.storage.cleanup()
-                    }
-                }
-            }
     }
 
     /// Perform cleanup if auto-cleanup is enabled in settings
@@ -58,6 +37,15 @@ final class QueryHistoryManager {
         storage.cleanup()
     }
 
+    /// Apply settings changes directly (called by AppSettingsManager)
+    @MainActor
+    func applySettingsChange() {
+        storage.updateSettingsCache()
+        if AppSettingsManager.shared.history.autoCleanup {
+            storage.cleanup()
+        }
+    }
+
     // MARK: - History Capture
 
     /// Record a query execution (fire-and-forget background write)
diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings
@@ -2226,6 +2226,7 @@
       }
     },
     "Are you sure you want to disconnect from this database?" : {
+      "extractionState" : "stale",
       "localizations" : {
         "vi" : {
           "stringUnit" : {
@@ -5602,6 +5603,7 @@
       }
     },
     "Disconnect" : {
+      "extractionState" : "stale",
       "localizations" : {
         "vi" : {
           "stringUnit" : {
diff --git a/TablePro/Views/Editor/QueryEditorView.swift b/TablePro/Views/Editor/QueryEditorView.swift
@@ -24,7 +24,6 @@ struct QueryEditorView: View {
     var onExecuteQuery: (() -> Void)?
 
     @State private var vimMode: VimMode = .normal
-    @State private var isVimEnabled = AppSettingsManager.shared.editor.vimModeEnabled
 
     var body: some View {
         let hasQuery = appState.hasQueryText
@@ -50,9 +49,6 @@ struct QueryEditorView: View {
             .clipped()
         }
         .background(Color(nsColor: .textBackgroundColor))
-        .onReceive(NotificationCenter.default.publisher(for: .editorSettingsDidChange)) { _ in
-            isVimEnabled = AppSettingsManager.shared.editor.vimModeEnabled
-        }
     }
 
     // MARK: - Toolbar
@@ -63,7 +59,7 @@ struct QueryEditorView: View {
                 .font(.headline)
                 .foregroundStyle(.secondary)
 
-            if isVimEnabled {
+            if AppSettingsManager.shared.editor.vimModeEnabled {
                 VimModeIndicatorView(mode: vimMode)
             }
 
diff --git a/TablePro/Views/Editor/SQLEditorView.swift b/TablePro/Views/Editor/SQLEditorView.swift
@@ -77,7 +77,7 @@ struct SQLEditorView: View {
             .onChange(of: colorScheme) {
                 editorConfiguration = Self.makeConfiguration()
             }
-            .onReceive(NotificationCenter.default.publisher(for: .editorSettingsDidChange)) { _ in
+            .onChange(of: AppSettingsManager.shared.editor) {
                 editorConfiguration = Self.makeConfiguration()
             }
             .onReceive(NotificationCenter.default.publisher(for: .accessibilityTextSizeDidChange)) { _ in
diff --git a/docs/development/plugin-settings-tracking.md b/docs/development/plugin-settings-tracking.md
@@ -18,15 +18,15 @@ Plugin enable/disable state lives in `UserDefaults["com.TablePro.disabledPlugins
 | Feature                                                  | Status | File                                                | Notes                                                       |
 | -------------------------------------------------------- | ------ | --------------------------------------------------- | ----------------------------------------------------------- |
 | Installed plugins list with toggle                       | Done   | `Views/Settings/Plugins/InstalledPluginsView.swift` | One row per `PluginEntry`, inline detail expansion          |
-| Enable/disable toggle (live)                             | Done   | `Core/Plugins/PluginManager.swift:365`              | Immediate capability register/unregister, no restart needed |
+| Enable/disable toggle (live)                             | Done   | `Core/Plugins/PluginManager.swift:382`              | Immediate capability register/unregister, no restart needed |
 | Plugin detail (version, bundle ID, source, capabilities) | Done   | `Views/Settings/Plugins/InstalledPluginsView.swift` | Shown on row expansion                                      |
 | Install from file (.tableplugin, .zip)                   | Done   | `Views/Settings/Plugins/InstalledPluginsView.swift` | NSOpenPanel + drag-and-drop                                 |
 | Uninstall user plugins                                   | Done   | `Views/Settings/Plugins/InstalledPluginsView.swift` | Destructive button with AlertHelper.confirmDestructive      |
 | Restart-required banner                                  | Done   | `Views/Settings/Plugins/InstalledPluginsView.swift` | Orange dismissible banner after uninstall                   |
 | Browse registry                                          | Done   | `Views/Settings/Plugins/BrowsePluginsView.swift`    | Remote manifest from GitHub, search + category filter       |
 | Registry install with progress                           | Done   | `Views/Settings/Plugins/RegistryPluginRow.swift`    | Download + SHA-256 verification, multi-phase progress       |
 | Contextual install prompt (connection flow)              | Done   | `Views/Connection/PluginInstallModifier.swift`      | Alert when opening DB type with missing driver              |
-| Code signature verification                              | Done   | `Core/Plugins/PluginManager.swift:558`              | Team ID `D7HJ5TFYCU` for user-installed plugins            |
+| Code signature verification                              | Done   | `Core/Plugins/PluginManager.swift:586`              | Team ID `D7HJ5TFYCU` for user-installed plugins            |
 
 ## Per-Plugin Configuration
 
@@ -50,7 +50,7 @@ All export plugins conform to `SettablePlugin` which provides automatic `loadSet
 
 ### Driver Plugins
 
-All 8 driver plugins have zero per-plugin settings. They can adopt `SettablePlugin` when settings are needed.
+All 9 driver plugins have zero per-plugin settings. They can adopt `SettablePlugin` when settings are needed.
 
 ---
 

Original file line number	Diff line number	Diff line change
`@@ -23,15 +23,13 @@ final class AppSettingsManager {`
`23`	`23`	`didSet {`
`24`	`24`	`general.language.apply()`
`25`	`25`	`storage.saveGeneral(general)`
`26`		`- notifyChange(domain: "general", notification: .generalSettingsDidChange)`
`27`	`26`	`}`
`28`	`27`	`}`
`29`	`28`
`30`	`29`	`var appearance: AppearanceSettings {`
`31`	`30`	`didSet {`
`32`	`31`	`storage.saveAppearance(appearance)`
`33`	`32`	`appearance.theme.apply()`
`34`		`- notifyChange(domain: "appearance", notification: .appearanceSettingsDidChange)`
`35`	`33`	`}`
`36`	`34`	`}`
`37`	`35`
`@@ -40,7 +38,7 @@ final class AppSettingsManager {`
`40`	`38`	`storage.saveEditor(editor)`
`41`	`39`	`// Update cached theme values for thread-safe access`
`42`	`40`	`SQLEditorTheme.reloadFromSettings(editor)`
`43`		`- notifyChange(domain: "editor", notification: .editorSettingsDidChange)`
	`41`	`+ notifyChange(.editorSettingsDidChange)`
`44`	`42`	`}`
`45`	`43`	`}`
`46`	`44`
`@@ -62,7 +60,7 @@ final class AppSettingsManager {`
`62`	`60`	`storage.saveDataGrid(validated)`
`63`	`61`	`// Update date formatting service with new format`
`64`	`62`	`DateFormattingService.shared.updateFormat(validated.dateFormat)`
`65`		`- notifyChange(domain: "dataGrid", notification: .dataGridSettingsDidChange)`
	`63`	`+ notifyChange(.dataGridSettingsDidChange)`
`66`	`64`	`}`
`67`	`65`	`}`
`68`	`66`
`@@ -84,28 +82,24 @@ final class AppSettingsManager {`
`84`	`82`	`storage.saveHistory(validated)`
`85`	`83`	`// Apply history settings immediately (cleanup if auto-cleanup enabled)`
`86`	`84`	`Task { await applyHistorySettingsImmediately() }`
`87`		`- notifyChange(domain: "history", notification: .historySettingsDidChange)`
`88`	`85`	`}`
`89`	`86`	`}`
`90`	`87`
`91`	`88`	`var tabs: TabSettings {`
`92`	`89`	`didSet {`
`93`	`90`	`storage.saveTabs(tabs)`
`94`		`- notifyChange(domain: "tabs", notification: .tabSettingsDidChange)`
`95`	`91`	`}`
`96`	`92`	`}`
`97`	`93`
`98`	`94`	`var keyboard: KeyboardSettings {`
`99`	`95`	`didSet {`
`100`	`96`	`storage.saveKeyboard(keyboard)`
`101`		`- notifyChange(domain: "keyboard", notification: .keyboardSettingsDidChange)`
`102`	`97`	`}`
`103`	`98`	`}`
`104`	`99`
`105`	`100`	`var ai: AISettings {`
`106`	`101`	`didSet {`
`107`	`102`	`storage.saveAI(ai)`
`108`		`- notifyChange(domain: "ai", notification: .aiSettingsDidChange)`
`109`	`103`	`}`
`110`	`104`	`}`
`111`	`105`
`@@ -147,24 +141,8 @@ final class AppSettingsManager {`
`147`	`141`
`148`	`142`	`// MARK: - Notification Propagation`
`149`	`143`
`150`		`- /// Notify listeners that settings have changed`
`151`		`- /// Posts both domain-specific and generic notifications`
`152`		`- private func notifyChange(domain: String, notification: Notification.Name) {`
`153`		`- let changeInfo = SettingsChangeInfo(domain: domain, changedKeys: nil)`
`154`		`-`
`155`		`- // Post domain-specific notification`
`156`		`- NotificationCenter.default.post(`
`157`		`- name: notification,`
`158`		`- object: self,`
`159`		`- userInfo: [SettingsChangeInfo.userInfoKey: changeInfo]`
`160`		`- )`
`161`		`-`
`162`		`- // Post generic notification for listeners that want all settings changes`
`163`		`- NotificationCenter.default.post(`
`164`		`- name: .settingsDidChange,`
`165`		`- object: self,`
`166`		`- userInfo: [SettingsChangeInfo.userInfoKey: changeInfo]`
`167`		`- )`
	`144`	`+ private func notifyChange(_ notification: Notification.Name) {`
	`145`	`+ NotificationCenter.default.post(name: notification, object: self)`
`168`	`146`	`}`
`169`	`147`
`170`	`148`	`// MARK: - Accessibility Text Size`
`@@ -197,11 +175,8 @@ final class AppSettingsManager {`
`197`	`175`	`}`
`198`	`176`	`}`
`199`	`177`
`200`		`- /// Apply history settings immediately (triggered on settings change)`
`201`	`178`	`private func applyHistorySettingsImmediately() async {`
`202`		`- // This will be called by QueryHistoryManager`
`203`		`- // We post a notification and let the manager handle the actual cleanup`
`204`		`- // This keeps the settings manager decoupled from history storage implementation`
	`179`	`+ QueryHistoryManager.shared.applySettingsChange()`
`205`	`180`	`}`
`206`	`181`
`207`	`182`	`// MARK: - Actions`
Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,7 @@ struct SQLEditorView: View {`
`77`	`77`	`.onChange(of: colorScheme) {`
`78`	`78`	`editorConfiguration = Self.makeConfiguration()`
`79`	`79`	`}`
`80`		`- .onReceive(NotificationCenter.default.publisher(for: .editorSettingsDidChange)) { _ in`
	`80`	`+ .onChange(of: AppSettingsManager.shared.editor) {`
`81`	`81`	`editorConfiguration = Self.makeConfiguration()`
`82`	`82`	`}`
`83`	`83`	`.onReceive(NotificationCenter.default.publisher(for: .accessibilityTextSizeDidChange)) { _ in`