fix(tabs): keep unsaved edits when a .sql file already open is opened again

CHANGELOG.md

@@ -25,6 +25,8 @@ 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.
 - Table maintenance from the object browser (OPTIMIZE, ANALYZE, CHECK, REPAIR, and VACUUM on PostgreSQL) now runs against the database the table you picked lives in. It ran on whichever database the connection happened to be on, which a tab from another database moves for the length of its query, so the command could maintain the same-named table in the wrong database and still report success.
 - A table that exists in two databases can be opened in both. The object browser marked the row for a table you had open in another database, so clicking it did nothing at all: the row was already selected and the click changed nothing. The browser now marks a row only while the tab you are looking at belongs to the database on screen. (#2217)
 - Server Dashboard, Users & Roles, Query Insights, ER Diagram and a linked SQL file now select the tab they already opened instead of doing nothing when another tab is in front. (#2217)

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) {

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,

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
     }

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])
+        }
+    }
+}

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 = FileTextLoader.modificationDate(of: sourceFileURL)
         }
         tabs.append(newTab)
         selectedTabId = newTab.id
@@ -177,6 +177,27 @@ 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) {
+        /// 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 = FileTextLoader.modificationDate(of: url)
+        tabs[index].content.externalModificationDetected = false
+    }
+
     /// 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.

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)
+    }
+}

TableProTests/Models/SQLFileDeduplicationTests.swift

@@ -99,6 +99,63 @@ 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("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() {
+        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