Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions Sources/CodeEditorApp/Sources/MainWindowView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,27 @@ struct MainWindowView: View {
} message: {
Text("Unsaved changes will be lost if you discard them.")
}
.confirmationDialog(
"“\(appState.pendingExternalChange?.name ?? "")” changed on disk",
isPresented: externalChangeBinding,
titleVisibility: .visible
) {
Button("Reload from Disk") {
if let document = appState.pendingExternalChange {
Task { await appState.revertDocument(document) }
}
}
Button("Save Anyway", role: .destructive) {
if let document = appState.pendingExternalChange {
Task { await appState.saveAnyway(document) }
}
}
Button("Cancel", role: .cancel) {
appState.pendingExternalChange = nil
}
} message: {
Text("Reload to keep the on-disk changes, or save to overwrite them.")
}
.alert("Code Editor", isPresented: errorBinding) {
Button("OK", role: .cancel) { appState.errorMessage = nil }
} message: {
Expand Down Expand Up @@ -286,6 +307,13 @@ struct MainWindowView: View {
set: { if !$0 { appState.errorMessage = nil } }
)
}

private var externalChangeBinding: Binding<Bool> {
Binding(
get: { appState.pendingExternalChange != nil },
set: { if !$0 { appState.pendingExternalChange = nil } }
)
}
}

#Preview {
Expand Down
73 changes: 71 additions & 2 deletions Sources/CodeEditorCore/Sources/Document/Document.swift
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@ public final class Document: Identifiable {
/// Read-only flag.
public private(set) var isReadOnly: Bool = false

/// The on-disk modification date captured when the document was last read. Used to
/// detect that the file changed on disk since it was opened, so `save()` can refuse to
/// silently overwrite an external change. `nil` for untitled documents.
public private(set) var fileModificationDate: Date?

/// The primary selection.
public private(set) var selection: TextSelection = TextSelection()

Expand Down Expand Up @@ -230,12 +235,19 @@ public final class Document: Identifiable {
throw DocumentError.failedToConvertToUTF8(url, "UTF-8")
}

return Document(
let modificationDate =
try? url
.resourceValues(forKeys: [.contentModificationDateKey])
.contentModificationDate

let document = Document(
content: content,
language: language(for: url),
lineEnding: .detect(from: content),
url: url
)
document.fileModificationDate = modificationDate
return document
}

/// Resolve a language from a file's extension.
Expand Down Expand Up @@ -441,14 +453,27 @@ public final class Document: Identifiable {

// MARK: - Persistence

public func save() async throws {
/// Write the document to disk.
///
/// Throws `DocumentError.changedOnDisk` when the file on disk has been modified since
/// the document was loaded and `force` is `false`, so a save can't silently clobber an
/// external edit. Pass `force: true` to overwrite anyway (the "Save Anyway" escape hatch
/// for when the on-disk change was intentional).
public func save(force: Bool = false) async throws {
guard let url else { throw DocumentError.noURL }
if !force {
try throwIfChangedOnDisk()
}
let data = Data(content.utf8)
do {
try await Task.detached { try data.write(to: url) }.value
} catch {
throw DocumentError.failedToWrite(url, error.localizedDescription)
}
fileModificationDate =
try? url
.resourceValues(forKeys: [.contentModificationDateKey])
.contentModificationDate
isModified = false
delegate?.documentDidSave(self)
}
Expand All @@ -461,6 +486,47 @@ public final class Document: Identifiable {
public func close() {
delegate?.documentDidClose(self)
}

// MARK: - External change

/// Throws when the file on disk has a modification date newer than the one captured at
/// load, i.e. it was changed outside the editor. Untitled documents are never guarded.
private func throwIfChangedOnDisk() throws {
guard let url, let loaded = fileModificationDate else { return }
let diskDate =
try url
.resourceValues(forKeys: [.contentModificationDateKey])
.contentModificationDate
if let diskDate, diskDate > loaded {
throw DocumentError.changedOnDisk(url, modificationDate: diskDate)
}
}

/// Read the current on-disk content for this document's URL.
public func reloadFromDisk() async throws -> String {
guard let url else { throw DocumentError.noURL }
let data = try await Task.detached { try Data(contentsOf: url) }.value
guard let string = String(data: data, encoding: .utf8) else {
throw DocumentError.failedToConvertToUTF8(url, "UTF-8")
}
return string
}

/// Reload the document's content and modification date from disk, discarding in-memory
/// edits and clearing the modified flag. Used to resolve an external change when the
/// on-disk version is the one to keep.
public func revert() async throws {
guard let url else { throw DocumentError.noURL }
let refreshed = try await reloadFromDisk()
content = refreshed
lines = Self.split(refreshed, by: lineEnding)
fileModificationDate =
try? url
.resourceValues(forKeys: [.contentModificationDateKey])
.contentModificationDate
isModified = false
delegate?.documentDidModify(self)
}
}

// MARK: - Errors
Expand All @@ -470,6 +536,7 @@ public enum DocumentError: LocalizedError, Equatable {
case failedToRead(URL, String)
case failedToWrite(URL, String)
case failedToConvertToUTF8(URL, String)
case changedOnDisk(URL, modificationDate: Date)

public var errorDescription: String? {
switch self {
Expand All @@ -481,6 +548,8 @@ public enum DocumentError: LocalizedError, Equatable {
"Could not save \(url.lastPathComponent): \(reason)"
case .failedToConvertToUTF8(let url, let encoding):
"\(url.lastPathComponent) is not valid \(encoding) text."
case .changedOnDisk(let url, _):
"“\(url.lastPathComponent)” changed on disk since you opened it."
}
}
}
Expand Down
35 changes: 34 additions & 1 deletion Sources/CodeEditorUI/AppState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ public final class AppState {
/// regardless of whether the tab strip or the menu asked.
public var pendingClose: CodeEditorCore.Document?

/// A document whose file changed on disk since it was opened, surfaced as a
/// confirmation dialog offering Revert / Save Anyway instead of a silent overwrite.
public var pendingExternalChange: CodeEditorCore.Document?

// Quick open
public var isQuickOpenVisible = false
public var quickOpenQuery = ""
Expand Down Expand Up @@ -207,8 +211,14 @@ public final class AppState {
}
}

/// Save every modified document through `performSave`, so a file that changed on disk
/// surfaces the same Revert / Save-Anyway dialog as a single-document save.
public func saveAllDocuments() {
Task { await documentManager.saveAll() }
Task {
for document in documentManager.documents where document.isModified {
await performSave(document)
}
}
}

/// Save every modified document and report whether the job actually finished.
Expand Down Expand Up @@ -255,6 +265,29 @@ public final class AppState {
} else {
try await document.save()
}
} catch DocumentError.changedOnDisk {
pendingExternalChange = document
} catch {
errorMessage = error.localizedDescription
}
}

/// Reload `document` from disk, resolving a pending external-change prompt. The
/// on-disk version wins.
public func revertDocument(_ document: CodeEditorCore.Document) async {
pendingExternalChange = nil
do {
try await document.revert()
} catch {
errorMessage = error.localizedDescription
}
}

/// Overwrite `document`, ignoring an external change. Resolves the pending prompt.
public func saveAnyway(_ document: CodeEditorCore.Document) async {
pendingExternalChange = nil
do {
try await document.save(force: true)
} catch {
errorMessage = error.localizedDescription
}
Expand Down
5 changes: 5 additions & 0 deletions Sources/CodeEditorUI/Sources/Chrome/StatusBarView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ public struct StatusBarView: View {
item(document.lineEnding == .lf ? "LF" : document.lineEnding == .crlf ? "CRLF" : "CR")
separator
item(document.language.displayName)

if appState.pendingExternalChange != nil {
separator
item("⚠️ Changed on disk")
}
} else {
Spacer(minLength: 0)
}
Expand Down
123 changes: 123 additions & 0 deletions Tests/CodeEditorTests/DocumentExternalChangeTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import Foundation
import Testing

@testable import CodeEditorCore

/// Guards for #6 — "A file changed on disk while open is silently overwritten."
@MainActor
@Suite("External change detection")
struct DocumentExternalChangeTests {
private func tempURL() -> URL {
URL(fileURLWithPath: NSTemporaryDirectory())
.appendingPathComponent("codeeditor-ext-\(UUID().uuidString).swift")
}

/// Pin the on-disk mtime ahead of the loaded baseline so a conflict is deterministic
/// (no reliance on sub-second write ordering).
private func bumpModificationDate(of url: URL, aheadBy seconds: TimeInterval = 60) throws {
var target = url
var values = URLResourceValues()
values.contentModificationDate = Date().addingTimeInterval(seconds)
try target.setResourceValues(values)
}

@Test("save is blocked when the file changed on disk since load")
func saveBlockedByExternalChange() async throws {
let url = tempURL()
defer { try? FileManager.default.removeItem(at: url) }
try "original".write(to: url, atomically: true, encoding: .utf8)

let document = try await Document.load(from: url)
#expect(document.content == "original")
#expect(document.fileModificationDate != nil)
document.setContent("modified in memory")
#expect(document.isModified)

// An external actor rewrites the file and advances its mtime past the loaded baseline.
try "external".write(to: url, atomically: true, encoding: .utf8)
try bumpModificationDate(of: url)

do {
try await document.save()
Issue.record("expected DocumentError.changedOnDisk, save succeeded")
} catch DocumentError.changedOnDisk(let savedURL, _) {
#expect(savedURL == url)
} catch {
Issue.record("unexpected error: \(error)")
}

// The on-disk file must be left untouched by the rejected save.
#expect(try String(contentsOf: url, encoding: .utf8) == "external")
#expect(document.isModified)
}

@Test("no false positive when nothing changed on disk")
func saveSucceedsWithNoExternalChange() async throws {
let url = tempURL()
defer { try? FileManager.default.removeItem(at: url) }
try "hello".write(to: url, atomically: true, encoding: .utf8)

let document = try await Document.load(from: url)
document.setContent("hello, world")
try await document.save()
#expect(!document.isModified)
#expect(document.fileModificationDate != nil)
#expect(try String(contentsOf: url, encoding: .utf8) == "hello, world")
}

@Test("revert reloads the on-disk version and clears modified")
func revertReloadsFromDisk() async throws {
let url = tempURL()
defer { try? FileManager.default.removeItem(at: url) }
try "on disk v1".write(to: url, atomically: true, encoding: .utf8)

let document = try await Document.load(from: url)
document.setContent("unsaved edit")
#expect(document.isModified)

try "on disk v2".write(to: url, atomically: true, encoding: .utf8)
try bumpModificationDate(of: url)

try await document.revert()
#expect(document.content == "on disk v2")
#expect(!document.isModified)
#expect(document.fileModificationDate != nil)
}

@Test("saving resets the external-change baseline")
func saveResetsBaseline() async throws {
let url = tempURL()
defer { try? FileManager.default.removeItem(at: url) }
try "v1".write(to: url, atomically: true, encoding: .utf8)

let document = try await Document.load(from: url)
document.setContent("v2")
try await document.save()
#expect(!document.isModified)

// No external change: re-saving succeeds (no spurious conflict).
document.setContent("v3")
try await document.save()
#expect(!document.isModified)

// An external change after the save is still detected.
try "external".write(to: url, atomically: true, encoding: .utf8)
try bumpModificationDate(of: url)

do {
try await document.save()
Issue.record("expected DocumentError.changedOnDisk, save succeeded")
} catch DocumentError.changedOnDisk(let savedURL, _) {
#expect(savedURL == url)
} catch {
Issue.record("unexpected error: \(error)")
}
}

@Test("untitled documents are never guarded")
func untitledIsNotGuarded() async {
let document = Document.untitled()
#expect(document.fileModificationDate == nil)
await #expect(throws: DocumentError.noURL) { try await document.save() }
}
}
Loading