feat: open SQLite files from Finder by double-clicking#263
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds Finder double‑click support for SQLite files via Info.plist document/UTType declarations and refactors AppDelegate into extensions that queue and process opened URLs (database URLs, SQLite files, SQL files, deeplinks, plugin installs), managing window readiness, connection creation/reuse, post-connection actions, and failure handling. Changes
Sequence DiagramsequenceDiagram
actor Finder as Finder / User
participant AppDel as AppDelegate
participant Queue as URL Queue
participant Conn as Connection Manager
participant UI as Editor / Windows
Finder->>AppDel: open file URL (.sqlite, .db3, .sql, etc.)
AppDel->>AppDel: classify URL (sqlite, db URL, sql, deeplink, plugin)
AppDel->>Queue: enqueue if SwiftUI/windows not ready
Note over Queue,AppDel: scheduleQueuedURLProcessing (poll readiness ≈5s)
Queue->>AppDel: drain queued entries
AppDel->>Conn: find matching session or build transient connection
alt existing session
AppDel->>UI: bring connection window to front / open tab
else new connection
AppDel->>Conn: initiate async connect
Conn-->>AppDel: success / failure
AppDel->>UI: open editor window, apply schema/tabs/filters on success
AppDel->>UI: show error sheet or welcome on failure
end
AppDel->>AppDel: suppress/restore welcome window during processing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@TablePro/AppDelegate.swift`:
- Around line 205-217: The welcome-window suppression is scheduled immediately
even when handleSQLiteFile(_:) only queues opens; change the flow so suppression
isn't ended until queued SQLite opens are drained: update handleSQLiteFile(_:)
to return a Bool (or other signal) indicating "queued" versus "opened", or
expose a queued count accessor (e.g., queuedSQLiteFileCount or
scheduleQueuedSQLiteFileProcessing completion), then in the Task { `@MainActor`
... } loop record whether any call returned queued and only call
scheduleWelcomeWindowSuppression() after confirming no files remain queued (or
by awaiting scheduleQueuedSQLiteFileProcessing() to complete); keep using the
existing flags isHandlingFileOpen and fileOpenSuppressionCount but postpone
decrement/scheduling until the queued-open drain has completed.
- Around line 493-509: The current dedupe only rejects fully-connected sessions
(session.driver != nil); update the guard to also deduplicate when the same file
is already queued or is in an active session that is still connecting.
Specifically, before creating a new open request, check
queuedSQLiteFileURLs.contains(url) and check
DatabaseManager.shared.activeSessions for any session with
session.connection.type == .sqlite && session.connection.database == filePath
(regardless of session.driver), and short-circuit by returning (and call
bringConnectionWindowToFront(sessionId) only if that session/window exists)
instead of proceeding to create a duplicate; keep references to
WindowOpener.shared.openWindow, queuedSQLiteFileURLs,
DatabaseManager.shared.activeSessions, session.connection.database,
session.driver, bringConnectionWindowToFront, and
scheduleQueuedSQLiteFileProcessing to locate the relevant code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 81fdee9f-1f22-46d7-b78a-31a951393f8e
📒 Files selected for processing (3)
CHANGELOG.mdTablePro/AppDelegate.swiftTablePro/Info.plist
There was a problem hiding this comment.
♻️ Duplicate comments (2)
TablePro/AppDelegate.swift (2)
494-512:⚠️ Potential issue | 🟠 MajorDeduplication gaps remain: queued files and connecting sessions are not checked.
Three issues persist from prior review:
Queue-level dedup missing (Line 497): The same file can be appended to
queuedSQLiteFileURLsmultiple times if opened repeatedly during cold start.Connecting sessions not deduplicated (Lines 507-508): The
session.driver != nilcondition only catches fully connected sessions. If the user double-clicks the same file while it's still connecting, a duplicate window/session is created.Path normalization absent (Line 502): Comparing
url.pathdirectly fails to deduplicate when the same file is opened via a symlink or different path casing.🔁 Suggested fix
`@MainActor` private func handleSQLiteFile(_ url: URL) { + let normalizedURL = url.standardizedFileURL.resolvingSymlinksInPath() + let filePath = normalizedURL.path + guard WindowOpener.shared.openWindow != nil else { - queuedSQLiteFileURLs.append(url) + if !queuedSQLiteFileURLs.contains(where: { + $0.standardizedFileURL.resolvingSymlinksInPath() == normalizedURL + }) { + queuedSQLiteFileURLs.append(normalizedURL) + } scheduleQueuedSQLiteFileProcessing() return } - let filePath = url.path let connectionName = url.deletingPathExtension().lastPathComponent // Deduplicate: if this file is already open in an active session, bring it to front for (sessionId, session) in DatabaseManager.shared.activeSessions { - if session.connection.type == .sqlite && session.connection.database == filePath - && session.driver != nil { + if session.connection.type == .sqlite, + URL(fileURLWithPath: session.connection.database) + .standardizedFileURL + .resolvingSymlinksInPath() + .path == filePath { bringConnectionWindowToFront(sessionId) return } },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate.swift` around lines 494 - 512, The handleSQLiteFile function currently misses dedup checks for queued URLs and for sessions that are still connecting, and it compares raw paths; fix it by canonicalizing the URL before any comparisons (e.g., resolveSymlinks and standardized path/case into a single filePath variable) then check queuedSQLiteFileURLs for an existing canonicalized URL and skip appending if present, and update the active session loop over DatabaseManager.shared.activeSessions to consider both fully connected (session.driver != nil) and connecting sessions (session.connection.type == .sqlite && session.connection.database == canonicalFilePath) so duplicates are not created while connecting; if a matching session exists, call bringConnectionWindowToFront(sessionId) and return.
206-221:⚠️ Potential issue | 🟠 MajorWelcome window suppression timing still misaligned with queued file processing.
When
handleSQLiteFile(_:)queues files (becauseWindowOpener.shared.openWindow == nil), Line 219 immediately schedules suppression cleanup (~700ms). However,scheduleQueuedSQLiteFileProcessing()can poll for up to 5 seconds waiting for the SwiftUI window system. The suppression will end before queued files are processed, allowing the welcome window to reappear.Consider deferring
scheduleWelcomeWindowSuppression()until after confirming files were actually opened (not just queued), or havescheduleQueuedSQLiteFileProcessing()manage the suppression lifecycle when it drains the queue.
,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate.swift` around lines 206 - 221, The welcome-window suppression is being scheduled immediately after queuing files, which can expire before queued files are actually opened; update the flow so suppression covers the entire queued-processing lifetime by either (A) deferring the call to scheduleWelcomeWindowSuppression() until files are actually opened (move the call into the code path that runs when WindowOpener.shared.openWindow returns a non-nil window or when handleSQLiteFile(_:) confirms an opened window), or (B) have scheduleQueuedSQLiteFileProcessing() (the routine that polls for SwiftUI windows) take ownership of the suppression lifecycle: increment isHandlingFileOpen/fileOpenSuppressionCount when queuing starts and decrement/schedule cleanup only when the queue is drained and all queued files processed. Ensure you reference handleSQLiteFile(_:), scheduleWelcomeWindowSuppression(), scheduleQueuedSQLiteFileProcessing(), WindowOpener.shared.openWindow, isHandlingFileOpen and fileOpenSuppressionCount when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@TablePro/AppDelegate.swift`:
- Around line 494-512: The handleSQLiteFile function currently misses dedup
checks for queued URLs and for sessions that are still connecting, and it
compares raw paths; fix it by canonicalizing the URL before any comparisons
(e.g., resolveSymlinks and standardized path/case into a single filePath
variable) then check queuedSQLiteFileURLs for an existing canonicalized URL and
skip appending if present, and update the active session loop over
DatabaseManager.shared.activeSessions to consider both fully connected
(session.driver != nil) and connecting sessions (session.connection.type ==
.sqlite && session.connection.database == canonicalFilePath) so duplicates are
not created while connecting; if a matching session exists, call
bringConnectionWindowToFront(sessionId) and return.
- Around line 206-221: The welcome-window suppression is being scheduled
immediately after queuing files, which can expire before queued files are
actually opened; update the flow so suppression covers the entire
queued-processing lifetime by either (A) deferring the call to
scheduleWelcomeWindowSuppression() until files are actually opened (move the
call into the code path that runs when WindowOpener.shared.openWindow returns a
non-nil window or when handleSQLiteFile(_:) confirms an opened window), or (B)
have scheduleQueuedSQLiteFileProcessing() (the routine that polls for SwiftUI
windows) take ownership of the suppression lifecycle: increment
isHandlingFileOpen/fileOpenSuppressionCount when queuing starts and
decrement/schedule cleanup only when the queue is drained and all queued files
processed. Ensure you reference handleSQLiteFile(_:),
scheduleWelcomeWindowSuppression(), scheduleQueuedSQLiteFileProcessing(),
WindowOpener.shared.openWindow, isHandlingFileOpen and fileOpenSuppressionCount
when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ba96429-ea79-4163-8a6a-4c5b5cf07155
📒 Files selected for processing (1)
TablePro/AppDelegate.swift
- AppDelegate.swift: class body, stored properties, lifecycle (103 lines) - AppDelegate+FileOpen.swift: URL dispatch, deeplinks, plugins (214 lines) - AppDelegate+ConnectionHandler.swift: DB URL, SQLite, unified queue (356 lines) - AppDelegate+WindowConfig.swift: window lifecycle, dock, styling (320 lines) Unified queuedDatabaseURLs + queuedSQLiteFileURLs into single QueuedURLEntry enum with one polling loop. Extracted shared suppressWelcomeWindow() and openNewConnectionWindow() helpers. Replaced guard-return with if-block in SQL file handler.
There was a problem hiding this comment.
🧹 Nitpick comments (9)
TablePro/AppDelegate+WindowConfig.swift (4)
14-14: Add explicit access control to the extension.Per coding guidelines, always specify access control explicitly on extensions. This extension should declare its visibility.
♻️ Suggested fix
-extension AppDelegate { +internal extension AppDelegate {As per coding guidelines: "Always specify access control explicitly (private, internal, public) on both extensions and individual members."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate`+WindowConfig.swift at line 14, The extension declaration "extension AppDelegate" lacks explicit access control; update the extension to include the appropriate visibility keyword (for example "internal extension AppDelegate" or "public extension AppDelegate" depending on the module API) so the extension and its members have explicit access control as required by the coding guidelines.
62-85: Consider extracting shared connection logic to reduce duplication.
connectFromDock(_:)andattemptAutoReconnect(connectionId:)share nearly identical connect-then-handle-windows patterns: post.openMainWindow, callconnectToSession, close welcome windows on success, close main windows and open welcome on failure. Consider extracting a shared helper.♻️ Example extraction
private func performConnection( _ connection: DatabaseConnection, context: String ) { NotificationCenter.default.post(name: .openMainWindow, object: connection.id) Task { `@MainActor` in do { try await DatabaseManager.shared.connectToSession(connection) for window in NSApp.windows where self.isWelcomeWindow(window) { window.close() } } catch { windowLogger.error("\(context) failed for '\(connection.name)': \(error.localizedDescription)") for window in NSApp.windows where self.isMainWindow(window) { window.close() } self.openWelcomeWindow() } } }Also applies to: 279-311
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate`+WindowConfig.swift around lines 62 - 85, Both connectFromDock(_:) and attemptAutoReconnect(connectionId:) duplicate the same connect-and-window-handling flow; extract that logic into a single helper (e.g., performConnection(_ connection: DatabaseConnection, context: String)) that posts .openMainWindow, runs Task { `@MainActor` ... } to call DatabaseManager.shared.connectToSession(connection), closes welcome windows on success (use isWelcomeWindow), and on failure logs with windowLogger including the context and connection.name, closes main windows (isMainWindow), and calls openWelcomeWindow; then replace the bodies of connectFromDock and attemptAutoReconnect to call this helper with an appropriate context string.
288-310: RedundantDispatchQueue.main.asyncwrappingTask {@mainactor}.Since
attemptAutoReconnectis called fromapplicationDidFinishLaunching(already on main actor), and the innerTask {@mainactor}ensures main actor execution, the outerDispatchQueue.main.asyncis redundant.♻️ Simplified version
- DispatchQueue.main.async { [weak self] in - guard let self else { return } - - NotificationCenter.default.post(name: .openMainWindow, object: connection.id) + NotificationCenter.default.post(name: .openMainWindow, object: connection.id) - Task { `@MainActor` in - do { - try await DatabaseManager.shared.connectToSession(connection) + Task { `@MainActor` [weak self] in + guard let self else { return } + do { + try await DatabaseManager.shared.connectToSession(connection) - for window in NSApp.windows where self.isWelcomeWindow(window) { - window.close() - } - } catch { - windowLogger.error("Auto-reconnect failed for '\(connection.name)': \(error.localizedDescription)") + for window in NSApp.windows where self.isWelcomeWindow(window) { + window.close() + } + } catch { + windowLogger.error("Auto-reconnect failed for '\(connection.name)': \(error.localizedDescription)") - for window in NSApp.windows where self.isMainWindow(window) { - window.close() - } + for window in NSApp.windows where self.isMainWindow(window) { + window.close() + } - self.openWelcomeWindow() - } + self.openWelcomeWindow() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate`+WindowConfig.swift around lines 288 - 310, Remove the outer DispatchQueue.main.async wrapper and move the weak-self capture into the Task to avoid redundant main-queue dispatching; replace the DispatchQueue.main.async { [weak self] in guard let self else { return } Task { `@MainActor` in ... } } block with a single Task { `@MainActor` [weak self] in guard let self else { return } NotificationCenter.default.post(name: .openMainWindow, object: connection.id) try await DatabaseManager.shared.connectToSession(connection) ... } keeping the existing error handling that calls windowLogger.error, closes windows via isWelcomeWindow/isMainWindow, and calls self.openWelcomeWindow().
100-112: Add explicit access control to window identification methods.These methods lack explicit access control. Per guidelines, specify
internalorpublicexplicitly.♻️ Suggested fix
- func isMainWindow(_ window: NSWindow) -> Bool { + internal func isMainWindow(_ window: NSWindow) -> Bool { guard let identifier = window.identifier?.rawValue else { return false } return identifier.contains("main") } - func isWelcomeWindow(_ window: NSWindow) -> Bool { + internal func isWelcomeWindow(_ window: NSWindow) -> Bool { window.identifier?.rawValue == "welcome" || window.title.lowercased().contains("welcome") }As per coding guidelines: "Always specify access control explicitly (private, internal, public) on both extensions and individual members."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate`+WindowConfig.swift around lines 100 - 112, The three window-identification methods lack explicit access control; update the extension and each member (isMainWindow(_:), isWelcomeWindow(_:) and currently-private isConnectionFormWindow(_:)) to include explicit access levels per guidelines—declare the extension with the appropriate access (likely internal) and add explicit access modifiers on each function (e.g., internal func isMainWindow(_:), internal func isWelcomeWindow(_:), private/internal func isConnectionFormWindow(_:)) to match intended visibility.TablePro/AppDelegate+FileOpen.swift (1)
14-14: Add explicit access control to the extension.♻️ Suggested fix
-extension AppDelegate { +internal extension AppDelegate {As per coding guidelines: "Always specify access control explicitly (private, internal, public) on both extensions and individual members."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate`+FileOpen.swift at line 14, The AppDelegate extension declaration lacks explicit access control; update the extension declaration for AppDelegate in TablePro/AppDelegate+FileOpen.swift to include the appropriate access modifier (e.g., public, internal, or private) consistent with the surrounding API surface and coding guidelines, and ensure any members inside the extension (methods/properties) retain or specify explicit access levels as needed (e.g., make the extension "internal extension AppDelegate" or "public extension AppDelegate" and adjust member modifiers accordingly).TablePro/AppDelegate+ConnectionHandler.swift (2)
20-20: Add explicit access control to the extension.♻️ Suggested fix
-extension AppDelegate { +internal extension AppDelegate {As per coding guidelines: "Always specify access control explicitly (private, internal, public) on both extensions and individual members."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate`+ConnectionHandler.swift at line 20, The AppDelegate extension declaration lacks explicit access control; update the extension declaration "extension AppDelegate" to include the appropriate access level (e.g., public, internal, or private) consistent with the module's API rules and the visibility of its members, and ensure any member declarations inside the extension also have explicit access control modifiers if they differ from the extension's level.
15-18: Add explicit access control to the enum.♻️ Suggested fix
/// Typed queue entry for URLs waiting on the SwiftUI window system. /// Replaces the separate `queuedDatabaseURLs` and `queuedSQLiteFileURLs` arrays. -enum QueuedURLEntry { +internal enum QueuedURLEntry { case databaseURL(URL) case sqliteFile(URL) }As per coding guidelines: "Always specify access control explicitly (private, internal, public) on both extensions and individual members."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate`+ConnectionHandler.swift around lines 15 - 18, The enum declaration for QueuedURLEntry lacks an explicit access modifier; add the appropriate access control keyword (e.g., private, fileprivate, internal, or public) before the enum name — e.g., "private enum QueuedURLEntry" — to make its visibility explicit, and ensure any related members or extensions referencing QueuedURLEntry likewise have explicit access control; the enum cases databaseURL and sqliteFile will inherit that access level.TablePro/AppDelegate.swift (2)
100-102: Minor inconsistency between comment and deinit.Lines 74-76 state observers aren't explicitly removed because AppDelegate lives for the app lifetime, yet
deinitremoves observers. While defensive cleanup is fine, consider updating the comment to reflect this behavior.♻️ Suggested comment update
// NOTE: These observers are not explicitly removed because AppDelegate - // lives for the entire app lifetime. NotificationCenter uses weak - // references for selector-based observers on macOS 10.11+. + // lives for the entire app lifetime (deinit cleanup is defensive). + // NotificationCenter uses weak references for selector-based observers + // on macOS 10.11+.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate.swift` around lines 100 - 102, The comment claiming observers aren't explicitly removed because AppDelegate lives for the app lifetime is inconsistent with the presence of nonisolated deinit that calls NotificationCenter.default.removeObserver(self); update the comment near AppDelegate (and/or deinit) to state that although AppDelegate normally lives for the app lifetime, there is a defensive cleanup implemented in deinit which removes observers using NotificationCenter.default.removeObserver(self), so the comment accurately reflects the code's behavior.
29-42: Add explicit access control to properties.These properties lack explicit access control. While implicitly internal, the guidelines require explicit specification. Since they're accessed by extension files,
internalis appropriate.♻️ Suggested fix
/// Track windows that have been configured to avoid re-applying styles - var configuredWindows = Set<ObjectIdentifier>() + internal var configuredWindows = Set<ObjectIdentifier>() /// SQL files queued until a database connection is active (drained on .databaseDidConnect) - var queuedFileURLs: [URL] = [] + internal var queuedFileURLs: [URL] = [] /// Database URL and SQLite file entries queued until the SwiftUI window system is ready - var queuedURLEntries: [QueuedURLEntry] = [] + internal var queuedURLEntries: [QueuedURLEntry] = [] /// True while handling a file-open event — suppresses welcome window - var isHandlingFileOpen = false + internal var isHandlingFileOpen = false /// Counter for outstanding suppressions; welcome window is suppressed while > 0 - var fileOpenSuppressionCount = 0 + internal var fileOpenSuppressionCount = 0As per coding guidelines: "Always specify access control explicitly (private, internal, public) on both extensions and individual members."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/AppDelegate.swift` around lines 29 - 42, Add explicit access control to the listed AppDelegate properties by marking them as internal (the guideline requires explicit access control for members accessed by extensions). Update the declarations for configuredWindows, queuedFileURLs, queuedURLEntries, isHandlingFileOpen, and fileOpenSuppressionCount to include the internal modifier so their signatures read like "internal var configuredWindows = ...", etc., leaving types and initial values unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@TablePro/AppDelegate.swift`:
- Around line 100-102: The comment claiming observers aren't explicitly removed
because AppDelegate lives for the app lifetime is inconsistent with the presence
of nonisolated deinit that calls
NotificationCenter.default.removeObserver(self); update the comment near
AppDelegate (and/or deinit) to state that although AppDelegate normally lives
for the app lifetime, there is a defensive cleanup implemented in deinit which
removes observers using NotificationCenter.default.removeObserver(self), so the
comment accurately reflects the code's behavior.
- Around line 29-42: Add explicit access control to the listed AppDelegate
properties by marking them as internal (the guideline requires explicit access
control for members accessed by extensions). Update the declarations for
configuredWindows, queuedFileURLs, queuedURLEntries, isHandlingFileOpen, and
fileOpenSuppressionCount to include the internal modifier so their signatures
read like "internal var configuredWindows = ...", etc., leaving types and
initial values unchanged.
In `@TablePro/AppDelegate`+ConnectionHandler.swift:
- Line 20: The AppDelegate extension declaration lacks explicit access control;
update the extension declaration "extension AppDelegate" to include the
appropriate access level (e.g., public, internal, or private) consistent with
the module's API rules and the visibility of its members, and ensure any member
declarations inside the extension also have explicit access control modifiers if
they differ from the extension's level.
- Around line 15-18: The enum declaration for QueuedURLEntry lacks an explicit
access modifier; add the appropriate access control keyword (e.g., private,
fileprivate, internal, or public) before the enum name — e.g., "private enum
QueuedURLEntry" — to make its visibility explicit, and ensure any related
members or extensions referencing QueuedURLEntry likewise have explicit access
control; the enum cases databaseURL and sqliteFile will inherit that access
level.
In `@TablePro/AppDelegate`+FileOpen.swift:
- Line 14: The AppDelegate extension declaration lacks explicit access control;
update the extension declaration for AppDelegate in
TablePro/AppDelegate+FileOpen.swift to include the appropriate access modifier
(e.g., public, internal, or private) consistent with the surrounding API surface
and coding guidelines, and ensure any members inside the extension
(methods/properties) retain or specify explicit access levels as needed (e.g.,
make the extension "internal extension AppDelegate" or "public extension
AppDelegate" and adjust member modifiers accordingly).
In `@TablePro/AppDelegate`+WindowConfig.swift:
- Line 14: The extension declaration "extension AppDelegate" lacks explicit
access control; update the extension to include the appropriate visibility
keyword (for example "internal extension AppDelegate" or "public extension
AppDelegate" depending on the module API) so the extension and its members have
explicit access control as required by the coding guidelines.
- Around line 62-85: Both connectFromDock(_:) and
attemptAutoReconnect(connectionId:) duplicate the same
connect-and-window-handling flow; extract that logic into a single helper (e.g.,
performConnection(_ connection: DatabaseConnection, context: String)) that posts
.openMainWindow, runs Task { `@MainActor` ... } to call
DatabaseManager.shared.connectToSession(connection), closes welcome windows on
success (use isWelcomeWindow), and on failure logs with windowLogger including
the context and connection.name, closes main windows (isMainWindow), and calls
openWelcomeWindow; then replace the bodies of connectFromDock and
attemptAutoReconnect to call this helper with an appropriate context string.
- Around line 288-310: Remove the outer DispatchQueue.main.async wrapper and
move the weak-self capture into the Task to avoid redundant main-queue
dispatching; replace the DispatchQueue.main.async { [weak self] in guard let
self else { return } Task { `@MainActor` in ... } } block with a single Task {
`@MainActor` [weak self] in guard let self else { return }
NotificationCenter.default.post(name: .openMainWindow, object: connection.id)
try await DatabaseManager.shared.connectToSession(connection) ... } keeping the
existing error handling that calls windowLogger.error, closes windows via
isWelcomeWindow/isMainWindow, and calls self.openWelcomeWindow().
- Around line 100-112: The three window-identification methods lack explicit
access control; update the extension and each member (isMainWindow(_:),
isWelcomeWindow(_:) and currently-private isConnectionFormWindow(_:)) to include
explicit access levels per guidelines—declare the extension with the appropriate
access (likely internal) and add explicit access modifiers on each function
(e.g., internal func isMainWindow(_:), internal func isWelcomeWindow(_:),
private/internal func isConnectionFormWindow(_:)) to match intended visibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11c4787c-8ca1-488a-aa75-09d2a413469a
📒 Files selected for processing (4)
TablePro/AppDelegate+ConnectionHandler.swiftTablePro/AppDelegate+FileOpen.swiftTablePro/AppDelegate+WindowConfig.swiftTablePro/AppDelegate.swift
This was referenced Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #262
Summary
.sqlite,.sqlite3,.db3,.s3db,.sl3,.sqlitedb) in Info.plist withLSHandlerRank = Alternateso TablePro appears in Finder's "Open With" menuUTImportedTypeDeclarationsforcom.tablepro.sqlite-dbcovering extensions not in Apple's system UTIAppDelegate— creates a transient SQLite connection using the file path, with deduplication against active sessions and cold-start queuingTest plan
sqlite3 /tmp/test.sqlite "create table t(id integer);".sqlite3,.db3,.s3db,.sl3,.sqlitedbextensions.sqlitefile — TablePro should appear in "Open With"Summary by CodeRabbit
New Features
Quality of Life