Skip to content

Commit 85e6a7d

Browse files
committed
fix: address code review feedback
- Fix comment formatting for consistency (Cmd+T - create new query tab) - Save trimmed query instead of original (avoid whitespace-only queries) - Remove unnecessary synchronize() calls for better performance - Add isRestoringTabs flag to prevent circular sync between tabManager and session - Improve comments and code clarity
1 parent 6f8d19c commit 85e6a7d

3 files changed

Lines changed: 11 additions & 6 deletions

File tree

OpenTable/Core/Storage/TabStateStorage.swift

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ final class TabStateStorage {
3535
let data = try encoder.encode(tabState)
3636
let key = tabStateKey(for: connectionId)
3737
defaults.set(data, forKey: key)
38-
defaults.synchronize()
3938
} catch {
4039
// Silent failure - tab state is not critical
4140
}
@@ -69,13 +68,12 @@ final class TabStateStorage {
6968
func saveLastQuery(_ query: String, for connectionId: UUID) {
7069
let key = "com.opentable.lastquery.\(connectionId.uuidString)"
7170

72-
// Only save non-empty queries
71+
// Only save non-empty queries (trimmed to avoid saving whitespace-only queries)
7372
let trimmed = query.trimmingCharacters(in: .whitespacesAndNewlines)
7473
if trimmed.isEmpty {
7574
defaults.removeObject(forKey: key)
7675
} else {
77-
defaults.set(query, forKey: key)
78-
defaults.synchronize()
76+
defaults.set(trimmed, forKey: key)
7977
}
8078
}
8179

OpenTable/Views/MainContentView.swift

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ struct MainContentView: View {
3838
@State private var currentQueryTask: Task<Void, Never>?
3939
@State private var queryGeneration: Int = 0
4040
@State private var changeManagerUpdateTask: Task<Void, Never>?
41+
@State private var isRestoringTabs = false // Prevent circular sync during restoration
4142

4243
// Error alert state
4344
@State private var showErrorAlert = false
@@ -205,6 +206,9 @@ struct MainContentView: View {
205206
}
206207
}
207208
.onChange(of: tabManager.tabs) { _, newTabs in
209+
// Skip sync if we're currently restoring tabs from session (prevents circular updates)
210+
guard !isRestoringTabs else { return }
211+
208212
// Sync tabs array to session for persistence
209213
if let sessionId = DatabaseManager.shared.currentSessionId {
210214
DatabaseManager.shared.updateSession(sessionId) { session in
@@ -304,12 +308,15 @@ struct MainContentView: View {
304308
.task {
305309
await initializeView()
306310

307-
// Restore tabs from session if available
311+
// Restore tabs from session if available (after DatabaseManager has loaded them)
308312
if let sessionId = DatabaseManager.shared.currentSessionId,
309313
let session = DatabaseManager.shared.activeSessions[sessionId],
310314
!session.tabs.isEmpty {
315+
// Set flag to prevent onChange(tabManager.tabs) from syncing back
316+
isRestoringTabs = true
311317
tabManager.tabs = session.tabs
312318
tabManager.selectedTabId = session.selectedTabId
319+
isRestoringTabs = false
313320
}
314321
}
315322
.onChange(of: selectedTables) { oldTables, newTables in
@@ -333,7 +340,7 @@ struct MainContentView: View {
333340
}
334341
}
335342
.onReceive(NotificationCenter.default.publisher(for: .newTab)) { _ in
336-
// Cmd+T to create new query tab - load last query if available
343+
// Cmd+T - create new query tab - load last query if available
337344
Task { @MainActor in
338345
let lastQuery = TabStateStorage.shared.loadLastQuery(for: connection.id)
339346
tabManager.addTab(initialQuery: lastQuery)

0 commit comments

Comments
 (0)