From 2ff692e7e8bc2eb2a312f30c755d8b48fc34cd64 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 19 Aug 2026 13:31:13 +0700 Subject: [PATCH 1/2] fix(tabs): keep unsaved edits when a .sql file already open is opened again --- CHANGELOG.md | 1 + TablePro/Models/Query/QueryTabManager.swift | 26 +++++++++++- .../Models/SQLFileDeduplicationTests.swift | 42 +++++++++++++++++++ 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99992142f..17d48db80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Opening a `.sql` file that is already open shows its tab instead of replacing what you have typed in it. The buffer was overwritten with the copy on disk, with no prompt and nothing to undo it. A tab with no unsaved edits still picks up the current file, and its unsaved marker and changed-on-disk banner now follow the text it just loaded. - The inspector button stays at the right end of the toolbar when the inspector is open. It used to slide left and sit against the inspector's inner edge, so the button moved as soon as you used it. - The Tables and Favorites switch in the toolbar keeps following the sidebar after you open Customize Toolbar. It stopped responding until the window was closed and reopened. - Refresh, New Tab, Open Quickly, Export, Database, Results and Dashboard are dimmed in the toolbar's overflow menu when they cannot run. On a narrow window they looked available and did nothing. diff --git a/TablePro/Models/Query/QueryTabManager.swift b/TablePro/Models/Query/QueryTabManager.swift index 860eb5aa6..5279dbf33 100644 --- a/TablePro/Models/Query/QueryTabManager.swift +++ b/TablePro/Models/Query/QueryTabManager.swift @@ -143,7 +143,7 @@ final class QueryTabManager { if let sourceFileURL, let existingIndex = tabs.firstIndex(where: { $0.content.sourceFileURL == sourceFileURL }) { if let query = initialQuery { - tabs[existingIndex].content.query = query + adoptReopenedFile(at: existingIndex, content: query, url: sourceFileURL) } selectedTabId = tabs[existingIndex].id return @@ -168,7 +168,7 @@ final class QueryTabManager { newTab.content.sourceFileURL = sourceFileURL if let sourceFileURL { newTab.content.savedFileContent = newTab.content.query - newTab.content.loadMtime = (try? FileManager.default.attributesOfItem(atPath: sourceFileURL.path)[.modificationDate]) as? Date + newTab.content.loadMtime = Self.modificationDate(of: sourceFileURL) } tabs.append(newTab) selectedTabId = newTab.id @@ -177,6 +177,28 @@ final class QueryTabManager { } } + /// A file that is already open is shown, not reloaded over. + /// + /// Opening it again is a request to look at it, so the buffer is replaced only when there is + /// nothing of the user's in it. A tab with unsaved edits keeps them: `setText` on the editor + /// resets its storage, so the replacement was not undoable and nothing asked first. The one + /// path that does replace a dirty buffer is the file-changed-on-disk banner, which asks. + /// + /// The baseline moves with the buffer. Writing the text without it left the tab reading as + /// dirty against content it had just loaded, and armed the same banner for a change it had + /// already taken. + private func adoptReopenedFile(at index: Int, content: String, url: URL) { + guard !tabs[index].content.isFileDirty else { return } + tabs[index].content.query = content + tabs[index].content.savedFileContent = content + tabs[index].content.loadMtime = Self.modificationDate(of: url) + tabs[index].content.externalModificationDetected = false + } + + private static func modificationDate(of url: URL) -> Date? { + (try? FileManager.default.attributesOfItem(atPath: url.path)[.modificationDate]) as? Date + } + /// Take an already-built tab, such as one rebuilt from the recently closed history, rather than /// minting a fresh one. Selecting it drives the window title, toolbar, and persistence through /// the usual `selectedTabId` observation. diff --git a/TableProTests/Models/SQLFileDeduplicationTests.swift b/TableProTests/Models/SQLFileDeduplicationTests.swift index 990031687..1f6db0305 100644 --- a/TableProTests/Models/SQLFileDeduplicationTests.swift +++ b/TableProTests/Models/SQLFileDeduplicationTests.swift @@ -99,6 +99,48 @@ struct QueryTabManagerDeduplicationTests { #expect(tabManager.tabs.count == 1) #expect(tabManager.tabs.first?.content.query == "SELECT 2") } + + @Test("Reopening a file whose tab holds unsaved edits keeps them") + @MainActor + func keepsUnsavedEditsOnDuplicate() { + let tabManager = QueryTabManager() + let url = URL(fileURLWithPath: "/tmp/test.sql") + + tabManager.addTab(initialQuery: "SELECT 1", sourceFileURL: url) + tabManager.tabs[0].content.query = "SELECT 1 -- work in progress" + + tabManager.addTab(initialQuery: "SELECT 1", sourceFileURL: url) + + #expect(tabManager.tabs.count == 1) + #expect(tabManager.tabs.first?.content.query == "SELECT 1 -- work in progress") + } + + @Test("Reopening a clean file tab moves its saved baseline with the text") + @MainActor + func movesTheBaselineWithTheText() { + let tabManager = QueryTabManager() + let url = URL(fileURLWithPath: "/tmp/test.sql") + + tabManager.addTab(initialQuery: "SELECT 1", sourceFileURL: url) + tabManager.addTab(initialQuery: "SELECT 2", sourceFileURL: url) + + #expect(tabManager.tabs.first?.content.savedFileContent == "SELECT 2") + #expect(tabManager.tabs.first?.content.isFileDirty == false) + } + + @Test("Reopening a clean file tab clears a pending changed-on-disk banner") + @MainActor + func clearsTheExternalModificationBanner() { + let tabManager = QueryTabManager() + let url = URL(fileURLWithPath: "/tmp/test.sql") + + tabManager.addTab(initialQuery: "SELECT 1", sourceFileURL: url) + tabManager.tabs[0].content.externalModificationDetected = true + + tabManager.addTab(initialQuery: "SELECT 2", sourceFileURL: url) + + #expect(tabManager.tabs.first?.content.externalModificationDetected == false) + } } // MARK: - EditorTabPayload sourceFileURL Tests From 9b5c2bf3c213f0dd655477917065574fe20fdc53 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Wed, 19 Aug 2026 13:52:57 +0700 Subject: [PATCH 2/2] fix(tabs): give a reopened .sql tab the baseline it compares against --- CHANGELOG.md | 1 + .../RecentlyClosedTabReopener.swift | 4 +- .../TabPersistenceCoordinator.swift | 9 +- .../Core/Utilities/File/FileTextLoader.swift | 24 +++- TablePro/Models/Query/FileTabBaseline.swift | 30 +++++ TablePro/Models/Query/QueryTabManager.swift | 13 +-- .../Models/FileTabBaselineTests.swift | 105 ++++++++++++++++++ .../Models/SQLFileDeduplicationTests.swift | 15 +++ 8 files changed, 180 insertions(+), 21 deletions(-) create mode 100644 TablePro/Models/Query/FileTabBaseline.swift create mode 100644 TableProTests/Models/FileTabBaselineTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 17d48db80..e79918278 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Opening a `.sql` file that is already open shows its tab instead of replacing what you have typed in it. The buffer was overwritten with the copy on disk, with no prompt and nothing to undo it. A tab with no unsaved edits still picks up the current file, and its unsaved marker and changed-on-disk banner now follow the text it just loaded. +- A `.sql` tab reopened from Recently Closed knows it has unsaved edits. It compared against nothing, so it showed no unsaved marker, Save did nothing, and reopening the file replaced what you had typed. - The inspector button stays at the right end of the toolbar when the inspector is open. It used to slide left and sit against the inspector's inner edge, so the button moved as soon as you used it. - The Tables and Favorites switch in the toolbar keeps following the sidebar after you open Customize Toolbar. It stopped responding until the window was closed and reopened. - Refresh, New Tab, Open Quickly, Export, Database, Results and Dashboard are dimmed in the toolbar's overflow menu when they cannot run. On a narrow window they looked available and did nothing. diff --git a/TablePro/Core/Services/Infrastructure/RecentlyClosedTabReopener.swift b/TablePro/Core/Services/Infrastructure/RecentlyClosedTabReopener.swift index 927b3111a..92e2abc64 100644 --- a/TablePro/Core/Services/Infrastructure/RecentlyClosedTabReopener.swift +++ b/TablePro/Core/Services/Infrastructure/RecentlyClosedTabReopener.swift @@ -53,10 +53,12 @@ internal enum RecentlyClosedTabReopener { } private static func makeTab(for entry: RecentlyClosedTabEntry) -> QueryTab { - QueryTab( + var tab = QueryTab( from: entry.tab, defaultPageSize: AppSettingsManager.shared.dataGrid.defaultPageSize ) + FileTabBaseline.hydrate(&tab) + return tab } internal static func openWindowTab(for entry: RecentlyClosedTabEntry) { diff --git a/TablePro/Core/Services/Infrastructure/TabPersistenceCoordinator.swift b/TablePro/Core/Services/Infrastructure/TabPersistenceCoordinator.swift index e38bb3bc5..9d1a0bfd0 100644 --- a/TablePro/Core/Services/Infrastructure/TabPersistenceCoordinator.swift +++ b/TablePro/Core/Services/Infrastructure/TabPersistenceCoordinator.swift @@ -172,14 +172,7 @@ internal final class TabPersistenceCoordinator { let defaultPageSize = AppSettingsManager.shared.dataGrid.defaultPageSize var restoredTabs = state.tabs.map { QueryTab(from: $0, defaultPageSize: defaultPageSize) } - for index in restoredTabs.indices { - guard let url = restoredTabs[index].content.sourceFileURL else { continue } - if let loaded = FileTextLoader.load(url) { - restoredTabs[index].content.savedFileContent = loaded.content - restoredTabs[index].content.loadMtime = (try? FileManager.default - .attributesOfItem(atPath: url.path)[.modificationDate]) as? Date - } - } + FileTabBaseline.hydrate(&restoredTabs) return RestoreResult( tabs: restoredTabs, selectedTabId: state.selectedTabId, diff --git a/TablePro/Core/Utilities/File/FileTextLoader.swift b/TablePro/Core/Utilities/File/FileTextLoader.swift index 87b068b6b..f811f76a5 100644 --- a/TablePro/Core/Utilities/File/FileTextLoader.swift +++ b/TablePro/Core/Utilities/File/FileTextLoader.swift @@ -9,33 +9,47 @@ internal enum FileTextLoader { struct LoadedText { let content: String let encoding: String.Encoding + /// When the file was last written, as of just before this text was read. + /// + /// Read here rather than by the caller, because a caller that stats afterwards records a + /// date newer than the text it is holding, and a write that lands in between is then + /// invisible: the tab looks up to date against a file it never read. Taking the date first + /// fails the other way, leaving the baseline older than the text, so the changed-on-disk + /// notice can fire once too often but never go missing. + let modifiedAt: Date? var isUTF8: Bool { encoding == .utf8 } } static func load(_ url: URL) -> LoadedText? { + let modifiedAt = modificationDate(of: url) var detected: String.Encoding = .utf8 if let content = try? String(contentsOf: url, usedEncoding: &detected) { - return LoadedText(content: content, encoding: detected) + return LoadedText(content: content, encoding: detected, modifiedAt: modifiedAt) } if let content = try? String(contentsOf: url, encoding: .utf8) { - return LoadedText(content: content, encoding: .utf8) + return LoadedText(content: content, encoding: .utf8, modifiedAt: modifiedAt) } if let content = try? String(contentsOf: url, encoding: .isoLatin1) { - return LoadedText(content: content, encoding: .isoLatin1) + return LoadedText(content: content, encoding: .isoLatin1, modifiedAt: modifiedAt) } return nil } + static func modificationDate(of url: URL) -> Date? { + (try? FileManager.default.attributesOfItem(atPath: url.path)[.modificationDate]) as? Date + } + static func loadHeader(_ url: URL, maxBytes: Int = 4_096) -> LoadedText? { guard let handle = try? FileHandle(forReadingFrom: url) else { return nil } defer { try? handle.close() } guard let data = try? handle.read(upToCount: maxBytes), !data.isEmpty else { return nil } + let modifiedAt = modificationDate(of: url) if let content = String(data: data, encoding: .utf8) { - return LoadedText(content: content, encoding: .utf8) + return LoadedText(content: content, encoding: .utf8, modifiedAt: modifiedAt) } if let content = String(data: data, encoding: .isoLatin1) { - return LoadedText(content: content, encoding: .isoLatin1) + return LoadedText(content: content, encoding: .isoLatin1, modifiedAt: modifiedAt) } return nil } diff --git a/TablePro/Models/Query/FileTabBaseline.swift b/TablePro/Models/Query/FileTabBaseline.swift new file mode 100644 index 000000000..51e79b3ba --- /dev/null +++ b/TablePro/Models/Query/FileTabBaseline.swift @@ -0,0 +1,30 @@ +// +// FileTabBaseline.swift +// TablePro +// + +import Foundation + +/// What a tab backed by a `.sql` file on disk compares itself against. +/// +/// A tab rebuilt from a persisted record carries the text it was saved with and nothing to compare +/// it to, and `TabQueryContent.isFileDirty` reads a missing baseline as clean. A tab in that state +/// lies about itself three ways: it shows no unsaved marker, Save skips it because it believes +/// there is nothing to write, and reopening the file replaces what it holds. Restoring at launch +/// already read the baseline back; reopening a closed tab did not, so the same tab was honest +/// through one door and not the other. +/// +/// Every path that rebuilds a file-backed tab reads it back here, so there is one door. +internal enum FileTabBaseline { + internal static func hydrate(_ tab: inout QueryTab) { + guard let url = tab.content.sourceFileURL, let loaded = FileTextLoader.load(url) else { return } + tab.content.savedFileContent = loaded.content + tab.content.loadMtime = loaded.modifiedAt + } + + internal static func hydrate(_ tabs: inout [QueryTab]) { + for index in tabs.indices { + hydrate(&tabs[index]) + } + } +} diff --git a/TablePro/Models/Query/QueryTabManager.swift b/TablePro/Models/Query/QueryTabManager.swift index 5279dbf33..857923e19 100644 --- a/TablePro/Models/Query/QueryTabManager.swift +++ b/TablePro/Models/Query/QueryTabManager.swift @@ -168,7 +168,7 @@ final class QueryTabManager { newTab.content.sourceFileURL = sourceFileURL if let sourceFileURL { newTab.content.savedFileContent = newTab.content.query - newTab.content.loadMtime = Self.modificationDate(of: sourceFileURL) + newTab.content.loadMtime = FileTextLoader.modificationDate(of: sourceFileURL) } tabs.append(newTab) selectedTabId = newTab.id @@ -188,17 +188,16 @@ final class QueryTabManager { /// dirty against content it had just loaded, and armed the same banner for a change it had /// already taken. private func adoptReopenedFile(at index: Int, content: String, url: URL) { - guard !tabs[index].content.isFileDirty else { return } + /// An unknown baseline is not a licence to replace what the tab holds. `isFileDirty` reads a + /// missing one as clean, so without this a tab that never learned what its file said would + /// be overwritten by the very check meant to protect it. + guard tabs[index].content.savedFileContent != nil, !tabs[index].content.isFileDirty else { return } tabs[index].content.query = content tabs[index].content.savedFileContent = content - tabs[index].content.loadMtime = Self.modificationDate(of: url) + tabs[index].content.loadMtime = FileTextLoader.modificationDate(of: url) tabs[index].content.externalModificationDetected = false } - private static func modificationDate(of url: URL) -> Date? { - (try? FileManager.default.attributesOfItem(atPath: url.path)[.modificationDate]) as? Date - } - /// Take an already-built tab, such as one rebuilt from the recently closed history, rather than /// minting a fresh one. Selecting it drives the window title, toolbar, and persistence through /// the usual `selectedTabId` observation. diff --git a/TableProTests/Models/FileTabBaselineTests.swift b/TableProTests/Models/FileTabBaselineTests.swift new file mode 100644 index 000000000..3f192c02b --- /dev/null +++ b/TableProTests/Models/FileTabBaselineTests.swift @@ -0,0 +1,105 @@ +// +// FileTabBaselineTests.swift +// TableProTests +// +// A tab rebuilt from a persisted record has to learn what its file says, or it lies about being +// clean: no unsaved marker, Save skips it, and reopening the file replaces what it holds. +// + +import Foundation +@testable import TablePro +import Testing + +@Suite("File tab baseline") +@MainActor +struct FileTabBaselineTests { + private func makeFile(contents: String) throws -> URL { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("baseline-\(UUID().uuidString).sql") + try contents.write(to: url, atomically: true, encoding: .utf8) + return url + } + + private func fileTab(query: String, url: URL) -> QueryTab { + var tab = QueryTab(title: url.lastPathComponent, query: query) + tab.content.sourceFileURL = url + return tab + } + + @Test("A rebuilt tab learns what its file says") + func hydratesTheBaseline() throws { + let url = try makeFile(contents: "SELECT 1") + defer { try? FileManager.default.removeItem(at: url) } + var tab = fileTab(query: "SELECT 1", url: url) + + #expect(tab.content.savedFileContent == nil) + FileTabBaseline.hydrate(&tab) + + #expect(tab.content.savedFileContent == "SELECT 1") + #expect(tab.content.loadMtime != nil) + #expect(tab.content.isFileDirty == false) + } + + @Test("A rebuilt tab holding unsaved work reads as dirty once it has a baseline") + func reportsUnsavedWorkAfterHydrating() throws { + let url = try makeFile(contents: "SELECT 1") + defer { try? FileManager.default.removeItem(at: url) } + var tab = fileTab(query: "SELECT 1 -- work in progress", url: url) + + #expect(tab.content.isFileDirty == false, "Without a baseline it cannot tell, and says clean") + FileTabBaseline.hydrate(&tab) + + #expect(tab.content.isFileDirty) + } + + @Test("A tab with no file is left alone") + func ignoresATabWithNoFile() { + var tab = QueryTab(title: "Query 1", query: "SELECT 1") + FileTabBaseline.hydrate(&tab) + + #expect(tab.content.savedFileContent == nil) + } + + @Test("A file that cannot be read leaves the baseline unknown rather than inventing one") + func leavesTheBaselineUnknownForAMissingFile() { + let url = FileManager.default.temporaryDirectory + .appendingPathComponent("missing-\(UUID().uuidString).sql") + var tab = fileTab(query: "SELECT 1", url: url) + + FileTabBaseline.hydrate(&tab) + + #expect(tab.content.savedFileContent == nil) + } + + @Test("Hydrating a list covers every file-backed tab in it") + func hydratesEveryTabInAList() throws { + let first = try makeFile(contents: "SELECT 1") + let second = try makeFile(contents: "SELECT 2") + defer { + try? FileManager.default.removeItem(at: first) + try? FileManager.default.removeItem(at: second) + } + var tabs = [ + fileTab(query: "SELECT 1", url: first), + QueryTab(title: "Query 1", query: "SELECT 3"), + fileTab(query: "SELECT 2", url: second), + ] + + FileTabBaseline.hydrate(&tabs) + + #expect(tabs[0].content.savedFileContent == "SELECT 1") + #expect(tabs[1].content.savedFileContent == nil) + #expect(tabs[2].content.savedFileContent == "SELECT 2") + } + + @Test("The loader reports when the file it read was last written") + func loaderCarriesTheModificationDate() throws { + let url = try makeFile(contents: "SELECT 1") + defer { try? FileManager.default.removeItem(at: url) } + + let loaded = try #require(FileTextLoader.load(url)) + + #expect(loaded.content == "SELECT 1") + #expect(loaded.modifiedAt != nil) + } +} diff --git a/TableProTests/Models/SQLFileDeduplicationTests.swift b/TableProTests/Models/SQLFileDeduplicationTests.swift index 1f6db0305..6ec665206 100644 --- a/TableProTests/Models/SQLFileDeduplicationTests.swift +++ b/TableProTests/Models/SQLFileDeduplicationTests.swift @@ -128,6 +128,21 @@ struct QueryTabManagerDeduplicationTests { #expect(tabManager.tabs.first?.content.isFileDirty == false) } + @Test("A tab that never learned what its file said is not overwritten either") + @MainActor + func keepsTextWhenTheBaselineIsUnknown() { + let tabManager = QueryTabManager() + let url = URL(fileURLWithPath: "/tmp/test.sql") + var reopened = QueryTab(title: "test.sql", query: "SELECT 1 -- work in progress") + reopened.content.sourceFileURL = url + tabManager.adoptTab(reopened) + + tabManager.addTab(initialQuery: "SELECT 1", sourceFileURL: url) + + #expect(tabManager.tabs.count == 1) + #expect(tabManager.tabs.first?.content.query == "SELECT 1 -- work in progress") + } + @Test("Reopening a clean file tab clears a pending changed-on-disk banner") @MainActor func clearsTheExternalModificationBanner() {