diff --git a/Config/products/macpacker.json b/Config/products/macpacker.json index efd4888..694164d 100644 --- a/Config/products/macpacker.json +++ b/Config/products/macpacker.json @@ -381,6 +381,29 @@ "issues": [ "256" ] + }, + { + "type": "fix", + "title": { + "en": "Deleting a file could remove a different one", + "zh-Hans": "删除文件时可能误删另一个文件", + "fr": "Supprimer un fichier pouvait en retirer un autre", + "de": "Löschen einer Datei konnte eine andere entfernen", + "it": "Eliminare un file poteva rimuoverne un altro", + "ja": "ファイルを削除すると別のファイルが消えることがあった", + "fa": "حذف یک فایل ممکن بود فایل دیگری را پاک کند", + "pl": "Usunięcie pliku mogło usunąć inny plik", + "pt-BR": "Excluir um arquivo podia remover outro", + "ru": "Удаление файла могло стереть другой файл", + "uk": "Видалення файлу могло стерти інший файл", + "es-MX": "Eliminar un archivo podía quitar otro", + "ko": "파일을 삭제하면 다른 파일이 지워질 수 있었음", + "nl": "Een bestand verwijderen kon een ander wissen", + "tr": "Bir dosyayı silmek başka birini silebiliyordu" + }, + "issues": [ + "267" + ] } ] }, diff --git a/MacPacker/Features/ArchiveContentViewer/ArchiveContentToolbarView.swift b/MacPacker/Features/ArchiveContentViewer/ArchiveContentToolbarView.swift index 5928eca..94bde50 100644 --- a/MacPacker/Features/ArchiveContentViewer/ArchiveContentToolbarView.swift +++ b/MacPacker/Features/ArchiveContentViewer/ArchiveContentToolbarView.swift @@ -84,7 +84,7 @@ struct ArchiveContentToolbarView: ToolbarContent { } } .help("Add files or folders to the archive") - .disabled(!archiveState.canBeEdited || archiveState.isSaving) + .disabled(!archiveState.canAddHere) Button { archiveState.remove(items: archiveState.selectedItems) @@ -96,7 +96,7 @@ struct ArchiveContentToolbarView: ToolbarContent { } } .help("Delete the selected items from the archive") - .disabled(!archiveState.canBeEdited || archiveState.selectedItems.isEmpty || archiveState.isSaving) + .disabled(!archiveState.canRemove(archiveState.selectedItems)) Spacer() diff --git a/MacPacker/Features/ArchiveContentViewer/ArchiveTableViewRepresentable.swift b/MacPacker/Features/ArchiveContentViewer/ArchiveTableViewRepresentable.swift index 1ec2869..aa79111 100644 --- a/MacPacker/Features/ArchiveContentViewer/ArchiveTableViewRepresentable.swift +++ b/MacPacker/Features/ArchiveContentViewer/ArchiveTableViewRepresentable.swift @@ -410,7 +410,7 @@ struct ArchiveTableViewRepresentable: NSViewRepresentable { delete.keyEquivalentModifierMask = [] delete.target = self delete.image = NSImage(systemSymbolName: "trash", accessibilityDescription: nil) - delete.isEnabled = hasSelection && state.canBeEdited && !state.isSaving + delete.isEnabled = state.canRemove(state.selectedItems) menu.addItem(delete) } @@ -446,7 +446,7 @@ struct ArchiveTableViewRepresentable: NSViewRepresentable { @objc func contextDelete(_ sender: Any?) { let state = parent.archiveState - guard state.canBeEdited, !state.selectedItems.isEmpty else { return } + guard state.canRemove(state.selectedItems) else { return } state.remove(items: state.selectedItems) } @@ -716,12 +716,12 @@ class ArchiveTableView: NSTableView, NSMenuItemValidation { } /// Enables Edit ▸ Delete only when the front archive is editable and has a - /// selection; every other item keeps the default "enabled if the responder - /// handles it" behavior. + /// selection that is its own, not inside an archive opened within it; every + /// other item keeps the default "enabled if the responder handles it" behavior. func validateMenuItem(_ menuItem: NSMenuItem) -> Bool { if menuItem.action == #selector(delete(_:)) { guard let state else { return false } - return state.canBeEdited && !state.selectedItems.isEmpty && !state.isSaving + return state.canRemove(state.selectedItems) } return true } diff --git a/MacPacker/Features/ArchiveContentViewer/ArchiveView.swift b/MacPacker/Features/ArchiveContentViewer/ArchiveView.swift index bbb18c9..b198c51 100644 --- a/MacPacker/Features/ArchiveContentViewer/ArchiveView.swift +++ b/MacPacker/Features/ArchiveContentViewer/ArchiveView.swift @@ -43,9 +43,10 @@ struct ArchiveView: View { /// How far the archive window's drop cards sit inside the window. private static let dropInset: CGFloat = 12 - /// Adding needs an archive that the format can write, and no in-flight save. + /// Adding needs an archive that the format can write, no in-flight save, and + /// the window in that archive, not in one opened within it. private var canAdd: Bool { - state.hasArchive && state.canBeEdited && !state.isSaving + state.hasArchive && state.canAddHere } /// Dropping a non-archive doesn't open anything — it starts a new archive with diff --git a/MacPacker/Features/Settings/FormatSettingsView.swift b/MacPacker/Features/Settings/FormatSettingsView.swift index 132db5b..1b56f34 100644 --- a/MacPacker/Features/Settings/FormatSettingsView.swift +++ b/MacPacker/Features/Settings/FormatSettingsView.swift @@ -13,6 +13,9 @@ struct ArchiveFormatSettings: Identifiable { let name: String let extensions: String let engines: [ArchiveEngineType] + /// The engines that can also edit archives of this format, as the catalog + /// says: one that only reads opens them read-only. + let editors: Set var selectedEngine: ArchiveEngineType var defaultOpen: Bool = false } @@ -56,6 +59,7 @@ struct FormatSettingsView: View { appState.archiveEngineConfigStore.selectedEngine(for: formatId) else { continue } let engines = engineOptions.compactMap { ArchiveEngineType(configId: $0.id) } + let editors = Set(engineOptions.filter(\.canEdit).compactMap { ArchiveEngineType(configId: $0.id) }) let extString = type.extensions.joined(separator: ", ") // Default app detection: keep false for now (toggle is disabled anyway) @@ -69,6 +73,7 @@ struct FormatSettingsView: View { name: type.name, extensions: extString, engines: engines, + editors: editors, selectedEngine: selectedEngine, defaultOpen: isDefaultApp ) @@ -132,7 +137,8 @@ struct FormatSettingsView: View { TableColumn("File Format", value: \.name) TableColumn("Extensions", value: \.extensions) TableColumn("Engine") { - supportedPicker(identifier: $0.id, selectedEngine: $0.selectedEngine, supportedEngines: $0.engines) + supportedPicker(identifier: $0.id, selectedEngine: $0.selectedEngine, + supportedEngines: $0.engines, editors: $0.editors) } } .tableStyle(.bordered) @@ -184,6 +190,15 @@ struct FormatSettingsView: View { } } .font(.footnote) + + Divider() + + Label { + Text("Edits archives, not only opens them", comment: "Legend in the engine info popover: the pencil next to an engine in the format table means that engine can also change archives of that format, where the others only open and extract them") + } icon: { + Image(systemName: "pencil") + } + .font(.footnote) } .frame(width: 260) .padding() @@ -228,11 +243,23 @@ struct FormatSettingsView: View { .disabled(true) } + /// A menu takes the width of its widest engine. Every row's menu is as wide + /// as the longest engine name at the table's small size, with room for the + /// pencil, so they all line up. + private static let engineMenuWidth: CGFloat = { + let font = NSFont.systemFont(ofSize: NSFont.systemFontSize(for: .small)) + let widest = ArchiveEngineType.allCases + .map { ($0.rawValue as NSString).size(withAttributes: [.font: font]).width } + .max() ?? 0 + return ceil(widest) + 17 // the pencil and its spacing + }() + @ViewBuilder func supportedPicker( identifier: String, selectedEngine: ArchiveEngineType, - supportedEngines: [ArchiveEngineType] + supportedEngines: [ArchiveEngineType], + editors: Set ) -> some View { let binding = Binding( get: { selectedEngine }, @@ -246,7 +273,15 @@ struct FormatSettingsView: View { Picker(String(""), selection: binding) { ForEach(supportedEngines, id: \.self) { engine in - Text(engine.rawValue).tag(engine) + Group { + if editors.contains(engine) { + Label(engine.rawValue, systemImage: "pencil") + } else { + Text(engine.rawValue) + } + } + .frame(width: Self.engineMenuWidth, alignment: .leading) + .tag(engine) } } .labelsHidden() @@ -257,4 +292,3 @@ struct FormatSettingsView: View { .disabled(isAutomatic) } } - diff --git a/MacPacker/Localizable.xcstrings b/MacPacker/Localizable.xcstrings index 951496b..29b8afb 100644 --- a/MacPacker/Localizable.xcstrings +++ b/MacPacker/Localizable.xcstrings @@ -3879,6 +3879,9 @@ } } }, + "Edits archives, not only opens them" : { + "comment" : "Legend in the engine info popover: the pencil next to an engine in the format table means that engine can also change archives of that format, where the others only open and extract them" + }, "Enabled" : { "comment" : "A label indicating that the file provider extension is enabled.", "isCommentAutoGenerated" : true, diff --git a/MacPackerUITests/MacPackerUITests.swift b/MacPackerUITests/MacPackerUITests.swift index 0c087de..6051a4a 100644 --- a/MacPackerUITests/MacPackerUITests.swift +++ b/MacPackerUITests/MacPackerUITests.swift @@ -337,10 +337,12 @@ final class MacPackerUITests: XCTestCase { let source = finder.textFields.matching(NSPredicate(format: "value == %@", "dropped.txt")).firstMatch XCTAssertTrue(source.waitForExistence(timeout: 15), "Finder does not show the file to drag") - // upper half of the window = the "add" zone + // upper half of the window = the "add" zone. A click-drag, not a press: on + // macOS 27 XCTest plays `press` back as a touch gesture, and no mouse drag + // comes of it — the pointer never moves. let target = app.windows.firstMatch.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.35)) source.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) - .press(forDuration: 1, thenDragTo: target) + .click(forDuration: 1, thenDragTo: target) XCTAssertTrue(app.staticTexts["dropped.txt"].waitForExistence(timeout: 15), "the dropped file was not added to the archive") @@ -355,7 +357,7 @@ final class MacPackerUITests: XCTestCase { sleep(1) let openZone = app.windows.firstMatch.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.8)) source.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) - .press(forDuration: 1, thenDragTo: openZone) + .click(forDuration: 1, thenDragTo: openZone) XCTAssertTrue(app.staticTexts["New Archive"].waitForExistence(timeout: 15), "the open zone did not start a new archive for the plain file") XCTAssertTrue(app.staticTexts["Open in a new window"].waitForNonExistence(timeout: 5), @@ -375,7 +377,8 @@ final class MacPackerUITests: XCTestCase { defer { try? FileManager.default.removeItem(at: dir) } try "dropped".write(to: dir.appendingPathComponent("dropped.txt"), atomically: true, encoding: .utf8) - let app = launchApp(arguments: ["-DropWindow", "1"]) + // zip, whatever format Quick Compress was last used with on this Mac + let app = launchApp(arguments: ["-DropWindow", "1", "-dropWindowFormat", "zip"]) // by identifier, not by window title: the window is titled after the app, // exactly like the archive windows let dropArea = app.descendants(matching: .any)["quickCompress.dropArea"].firstMatch @@ -388,7 +391,7 @@ final class MacPackerUITests: XCTestCase { let target = dropArea.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) source.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) - .press(forDuration: 1, thenDragTo: target) + .click(forDuration: 1, thenDragTo: target) confirmAccessPanel(app, button: "Grant Access") @@ -442,7 +445,8 @@ final class MacPackerUITests: XCTestCase { defer { try? FileManager.default.removeItem(at: dir) } try "dropped".write(to: dir.appendingPathComponent("dropped.txt"), atomically: true, encoding: .utf8) - let app = launchApp(arguments: ["-DropWindow", "1", "-dropWindowOptionsExpanded", "YES"]) + let app = launchApp(arguments: ["-DropWindow", "1", "-dropWindowOptionsExpanded", "YES", + "-dropWindowFormat", "zip"]) let dropArea = app.descendants(matching: .any)["quickCompress.dropArea"].firstMatch XCTAssertTrue(dropArea.waitForExistence(timeout: 15), "the drop window did not open") let password = app.secureTextFields["saveOptions.password"].firstMatch @@ -459,7 +463,7 @@ final class MacPackerUITests: XCTestCase { XCTAssertTrue(source.waitForExistence(timeout: 15), "Finder does not show the file to drag") let drop = { source.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) - .press(forDuration: 1, thenDragTo: dropArea.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5))) + .click(forDuration: 1, thenDragTo: dropArea.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5))) } drop() diff --git a/Modules/Sources/Core/ArchiveState.swift b/Modules/Sources/Core/ArchiveState.swift index 84fdc86..ae8547f 100644 --- a/Modules/Sources/Core/ArchiveState.swift +++ b/Modules/Sources/Core/ArchiveState.swift @@ -108,10 +108,12 @@ public class ArchiveState: ObservableObject { private let catalog: ArchiveTypeCatalog private let archiveEngineSelector: ArchiveEngineSelectorProtocol - /// Formats whose configured engine could not open *this* archive, mapped to - /// the engine that could. Set by the loader's fallback and used for every - /// later lookup so extraction does not resolve back to the engine that - /// already failed. Cleared on reset with the rest of the archive state. + /// The engine that read this window's archives, by format — the archive and + /// any opened inside it. Every later lookup gets it, whatever Settings say + /// by then: entries are numbered by the engine that listed them, and 7-Zip + /// takes XAD's numbers for other entries. It is also where the loader's + /// fallback stays, when the configured engine could not open the archive. + /// Cleared on reset with the rest of the archive state. private var pinnedEngines: [String: ArchiveEngineType] = [:] /// The selector everything downstream should use. @@ -236,6 +238,7 @@ extension ArchiveState { /// Resets the state of the archive private func reset() { self.hasArchive = false + self.canBeEdited = false // A failed open sets its reason after this, so its alert still shows. self.openError = nil self.saveError = nil @@ -547,6 +550,12 @@ extension ArchiveState { return } guard let selectedItem else { return } + // A save writes only this archive. Added inside one opened within it, + // the file would land in this one, in a folder named like that archive. + guard !isWithinOpenedArchive(selectedItem) else { + log.notice("Ignoring add — inside an archive opened within this one", context: ["file": url.lastPathComponent]) + return + } let base = (selectedItem.virtualPath?.isEmpty == false && selectedItem.virtualPath != "/") ? selectedItem.virtualPath! + "/" : "" @@ -634,6 +643,10 @@ extension ArchiveState { log.notice("Ignoring delete — a save is in progress", context: ["items": "\(items.count)"]) return } + guard canRemove(items) else { + log.notice("Ignoring delete — nothing this archive can remove", context: ["items": "\(items.count)"]) + return + } let dropped = discard(items: items) @@ -685,25 +698,42 @@ extension ArchiveState { /// Depth-first: collects the source indices (real archive entries) and /// pending-addition paths of the item and all of its descendants, and - /// drops them from `entries`. + /// drops them from `entries`. An archive opened within this one goes as the + /// entry it is: what it holds leaves the tree with it, but belongs to that + /// archive, numbered as that one's entries. private func collectRemovals( _ item: ArchiveItem, indices: inout Set, - pendingPaths: inout Set + pendingPaths: inout Set, + inThisArchive: Bool = true ) { for childId in item.children ?? [] { if let child = entries[childId] { - collectRemovals(child, indices: &indices, pendingPaths: &pendingPaths) + collectRemovals(child, indices: &indices, pendingPaths: &pendingPaths, + inThisArchive: inThisArchive && item.archiveTypeId == nil) } } - if let index = item.index { - indices.insert(index) - } else if let path = item.virtualPath, path != "/" { - pendingPaths.insert(path) + if inThisArchive { + if let index = item.index { + indices.insert(index) + } else if let path = item.virtualPath, path != "/" { + pendingPaths.insert(path) + } } entries.removeValue(forKey: item.id) } + /// Whether `item` is an archive opened within this one, or inside one: what + /// such an archive holds is that archive's, and a save writes only this one. + private func isWithinOpenedArchive(_ item: ArchiveItem) -> Bool { + var current: ArchiveItem? = item + while let node = current, node.type != .root { + if node.archiveTypeId != nil { return true } + current = node.parent.flatMap { entries[$0] } + } + return false + } + /// Opens a dropped file in this window: a supported archive is opened directly, /// unless another window has it open already — that one comes forward instead; /// anything else becomes the first entry of a new archive. `open`/`create` reset @@ -721,6 +751,20 @@ extension ArchiveState { /// Whether there are unsaved changes (pending additions/removals). public var hasPendingChanges: Bool { !diff.isEmpty } + /// Whether `items` can be removed: none of them is inside an archive opened + /// within this one. The row of such an archive is this archive's own entry, + /// and can go. + public func canRemove(_ items: [ArchiveItem]) -> Bool { + canBeEdited && !isSaving && !items.isEmpty + && !items.contains { item in item.parent.flatMap { entries[$0] }.map(isWithinOpenedArchive) ?? false } + } + + /// Whether files can be added where the window is: into this archive, not + /// into one opened within it. + public var canAddHere: Bool { + canBeEdited && !isSaving && selectedItem.map { !isWithinOpenedArchive($0) } == true + } + /// Saves the pending changes. /// /// - For an archive loaded from disk, the changes are applied in place. @@ -786,6 +830,7 @@ extension ArchiveState { log.notice("Archive saved", context: ["target": target.lastPathComponent]) // reload from disk so entries and indices reflect the file + let engines = pinnedEngines open(url: saved.url) // Known already — reopening must not ask again. Set after open(), // which starts by forgetting every password, and before the first @@ -793,6 +838,9 @@ extension ArchiveState { if let password = saved.password { passwords[saved.url] = password } + // Likewise the engine: it is still the archive the window opened, + // whatever Settings have said since. + pinnedEngines = engines _ = try? await openTask?.value self.isSaving = false } catch { @@ -894,25 +942,31 @@ extension ArchiveState { if let firstVolume = loaderResult.firstVolumeURL { self.url = firstVolume } - if let type, - type.engines.contains(where: {$0.capabilities.contains(where: {$0 == "edit"})}) { + // Changed only by an engine that writes the format, since a + // change names entries by the numbers of the engine that listed + // them. XAD only reads, and 7-Zip's writer would take its numbers + // for other entries. + if let type, let used = loaderResult.engineType, + type.engines.contains(where: { $0.id == used.configId && $0.canEdit }) { canBeEdited = true } self.uncompressedSize = loaderResult.uncompressedSize self.isEncrypted = loaderResult.isEncrypted - // The loader may have fallen back to another engine because the - // configured one cannot read this archive. Pin it, or the first - // extraction would resolve the failing engine all over again. + // The archive stays with the engine that read it (see + // `pinnedEngines`). That may be a fallback, when the configured + // engine cannot read this archive: the first extraction would + // otherwise resolve the failing engine all over again. self.activeEngine = loaderResult.engineType - if let used = loaderResult.engineType, - used != archiveEngineSelector.engineType(for: loaderResult.type.id) { + if let used = loaderResult.engineType { pinnedEngines[loaderResult.type.id] = used - log.notice("Engine pinned for this archive", context: [ - "file": url.lastPathComponent, - "type": loaderResult.type.id, - "engine": used.configId - ]) + if used != archiveEngineSelector.engineType(for: loaderResult.type.id) { + log.notice("Engine pinned for this archive", context: [ + "file": url.lastPathComponent, + "type": loaderResult.type.id, + "engine": used.configId + ]) + } } updateStatusText(String(localized: "building tree...", bundle: .module, comment: "Archive operation status")) @@ -1177,6 +1231,13 @@ extension ArchiveState { updateStatusText(String(localized: "loading...", bundle: .module, comment: "Archive operation status")) let loaderResult = try await archiveLoader.loadEntries(url: url) + // Stays with the engine that read it, like the archive around it. + // One of that archive's format keeps its pin: the pin is per + // format, and moving it would hand the outer archive's entries to + // an engine that numbers them otherwise. + if let used = loaderResult.engineType, pinnedEngines[loaderResult.type.id] == nil { + pinnedEngines[loaderResult.type.id] = used + } if let tempDirectory = loaderResult.tempDirectory { tempDirectories.append(tempDirectory) diff --git a/Modules/Sources/Core/Engine/AutomaticEngineSelector.swift b/Modules/Sources/Core/Engine/AutomaticEngineSelector.swift index 9dd58c5..74b3339 100644 --- a/Modules/Sources/Core/Engine/AutomaticEngineSelector.swift +++ b/Modules/Sources/Core/Engine/AutomaticEngineSelector.swift @@ -15,9 +15,9 @@ import Foundation /// the header has to be decrypted during `XADArchive` init and XADArchive only /// takes a password afterwards. /// -/// In automatic mode `ArchiveLoader` may fall back to another engine to read an -/// archive; `ArchiveState` then pins that engine here for the rest of the -/// window. Without it the archive would list through the fallback and then fail +/// `ArchiveState` pins here the engine that read each archive, for the rest of +/// the window. In automatic mode that may be a fallback `ArchiveLoader` took; +/// without the pin the archive would list through the fallback and then fail /// on the first extraction, which resolves the engine from the format all over /// again. Fallback stays on so the pinned engine keeps its own alternatives. struct AutomaticEngineSelector: ArchiveEngineSelectorProtocol { diff --git a/Modules/Sources/Core/Formats/Catalog.swift b/Modules/Sources/Core/Formats/Catalog.swift index a8007ba..f36b20e 100644 --- a/Modules/Sources/Core/Formats/Catalog.swift +++ b/Modules/Sources/Core/Formats/Catalog.swift @@ -126,6 +126,11 @@ public struct EngineDto: Codable, Sendable { public let id: String // engine ID ("xad", "7zip", ...) public let capabilities: [String] // ["listContents", "extractFiles", "splitVolumes"] public let `default`: Bool? // optional, only present on one item + + /// Whether the engine writes archives of its format, so one it opened can be + /// changed. An engine that only reads numbers entries its own way, and the + /// writer would take those numbers for other entries. + public var canEdit: Bool { capabilities.contains("edit") } } // MARK: - Compounds diff --git a/Modules/Tests/CoreTests/CoverageGapTests.swift b/Modules/Tests/CoreTests/CoverageGapTests.swift index 9449878..faa8cb6 100644 --- a/Modules/Tests/CoreTests/CoverageGapTests.swift +++ b/Modules/Tests/CoreTests/CoverageGapTests.swift @@ -462,6 +462,16 @@ extension AllCoreTests { let engine = catalog.defaultEngine(for: "zip") #expect(engine != nil) } + + /// What the window and the Settings pencil go by: only 7-Zip edits, and + /// only zip. A format gaining an editor shows up here first (#265). + @Test func onlySevenZipEditsAndOnlyZip() { + let catalog = ArchiveTypeCatalog() + let editors = catalog.allFormatIds().flatMap { format in + catalog.engineOptions(for: format).filter(\.canEdit).map { "\(format)/\($0.id)" } + } + #expect(editors == ["zip/7zip"]) + } } // MARK: - ArchiveEngineConfigStore: persistence diff --git a/Modules/Tests/CoreTests/ZipWriteTests.swift b/Modules/Tests/CoreTests/ZipWriteTests.swift index 3ee8add..7bb05f3 100644 --- a/Modules/Tests/CoreTests/ZipWriteTests.swift +++ b/Modules/Tests/CoreTests/ZipWriteTests.swift @@ -615,6 +615,179 @@ extension AllCoreTests { #expect(try String(contentsOf: out.appendingPathComponent("folder/two.txt"), encoding: .utf8) == "two") } + // MARK: - The engine that read the archive + + /// A window with its Settings: the engine picked for zip, which can change + /// while the window is open. + private func stateWithSettings(zip engine: ArchiveEngineType) -> (ArchiveEngineConfigStore, ArchiveState) { + let catalog = ArchiveTypeCatalog() + let store = ArchiveEngineConfigStore(catalog: catalog, defaults: isolatedDefaults()) + store.isAutomatic = false + store.setSelectedEngine(engine, for: "zip") + let state = ArchiveState(catalog: catalog, engineSelector: ArchiveEngineSelector(catalog: catalog, configStore: store)) + state.folderAccessProvider = { _ in true } + return (store, state) + } + + /// Finder's `__MACOSX/._name` sidecars, which XAD folds into the files + /// they describe, so its entries are numbered unlike 7-Zip's: XAD's + /// number for `helper` is 7-Zip's for the sidecar of `icon.png`. + private func appleDoubleZip(in dir: URL) throws -> URL { + let zip = dir.appendingPathComponent("appledouble.zip") + try FileManager.default.copyItem( + at: Bundle.module.url(forResource: "zip", withExtension: nil)!.appendingPathComponent("appledouble.zip"), + to: zip) + return zip + } + + /// `helper` as 7-Zip finds it by its own path. + private func helperContents(in zip: URL, dir: URL) throws -> Data { + let archive = try SevenZipArchive(url: zip) + let entry = try #require(try archive.entries.first { $0.path == "payload/Contents/MacOS/helper" }) + let out = dir.appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: out, withIntermediateDirectories: true) + return try Data(contentsOf: #require(try archive.extract(index: entry.index, to: out)[entry.index])) + } + + /// Only an engine that writes a format can change an archive it opened, + /// and for zip that is 7-Zip. XAD numbers entries its own way — it folds + /// Mac metadata sidecars into the files they describe — and a delete made + /// through its listing handed those numbers to 7-Zip's writer: deleting + /// `helper` removed the sidecar of `icon.png`. A zip XAD opened is + /// read-only. + @Test func aZipXADOpenedCannotBeChanged() async throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + let (_, state) = stateWithSettings(zip: .xad) + state.open(url: try appleDoubleZip(in: dir)) + try await state.openTask?.value + #expect(state.activeEngine == .xad) + #expect(!state.canBeEdited) + + state.remove(items: [try #require(item("payload/Contents/MacOS/helper", in: state))]) + #expect(!state.hasPendingChanges) + } + + /// The archive a save reloads is still the one the window opened: it keeps + /// its engine, and a switch to XAD in Settings meanwhile does not turn it + /// read-only once it is saved. + @Test func aSaveKeepsTheEngineThatOpenedTheArchive() async throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + let (settings, state) = stateWithSettings(zip: .`7zip`) + state.open(url: try makeSystemZipFixture(in: dir)) + try await state.openTask?.value + + settings.setSelectedEngine(.xad, for: "zip") + state.remove(items: [try #require(item("folder/one.txt", in: state))]) + await state.save()?.value + #expect(state.saveError == nil, "\(state.saveError ?? "")") + #expect(item("folder/one.txt", in: state) == nil) + #expect(state.activeEngine == .`7zip`) + #expect(state.canBeEdited) + } + + /// A window holds one archive's editing at a time: opening one that cannot + /// be edited does not carry over the editing of the one before. + @Test func editingEndsWithTheArchiveItBelongsTo() async throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + let sevenZ = dir.appendingPathComponent("plain.7z") + try SevenZipArchive.writeArchive( + destination: sevenZ, items: [.addData(archivePath: "a.txt", data: Data("a".utf8))], + options: .init(format: .sevenZ)) + let state = makeState() + state.open(url: try makeSystemZipFixture(in: dir)) + try await state.openTask?.value + #expect(state.canBeEdited) + + state.open(url: sevenZ) + try await state.openTask?.value + #expect(!state.canBeEdited) + } + + /// An archive opened inside the archive keeps its entries in an archive of + /// its own, and a save writes only the outer one: nothing inside can be + /// removed or added from here. Deleting `x.txt` inside `inner.zip` deleted + /// the outer archive's `a.txt`, since both were entry 0. The row of the + /// inner archive is the outer one's, and goes alone. + @Test func anArchiveOpenedInsideIsNotChangedThroughTheOuterOne() async throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + let inner = dir.appendingPathComponent("inner.zip") + try SevenZipArchive.writeArchive( + destination: inner, + items: [.addData(archivePath: "x.txt", data: Data("x".utf8)), + .addData(archivePath: "y.txt", data: Data("y".utf8))], + options: .init(format: .zip)) + let outer = dir.appendingPathComponent("outer.zip") + try SevenZipArchive.writeArchive( + destination: outer, + items: [.addData(archivePath: "a.txt", data: Data("a".utf8)), + .addData(archivePath: "b.txt", data: Data("b".utf8)), + .addFile(archivePath: "inner.zip", diskPath: inner)], + options: .init(format: .zip)) + let state = makeState() + state.open(url: outer) + try await state.openTask?.value + let innerRow = try #require(item("inner.zip", in: state)) + try await state.openAsync(item: innerRow) + + let x = try #require(state.entries.values.first { $0.name == "x.txt" }) + #expect(!state.canRemove([x])) + state.remove(items: [x]) + #expect(!state.canAddHere) + state.add(url: inner) + #expect(!state.hasPendingChanges) + + state.openParent() + #expect(state.canAddHere) + #expect(state.canRemove([innerRow])) + state.remove(items: [innerRow]) + await state.save()?.value + #expect(state.saveError == nil, "\(state.saveError ?? "")") + #expect(try systemZipEntries(outer) == ["a.txt", "b.txt"]) + } + + /// A switch of engine in Settings applies to the archives opened after it. + /// The open one stays on the engine that read it and numbered its entries: + /// 7-Zip extracts by number, and would have taken XAD's number for `helper` + /// for the sidecar of `icon.png`. + @Test func anOpenArchiveKeepsTheEngineThatReadIt() async throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + let zip = try appleDoubleZip(in: dir) + let (settings, state) = stateWithSettings(zip: .xad) + state.open(url: zip) + try await state.openTask?.value + let helper = try #require(item("payload/Contents/MacOS/helper", in: state)) + + settings.setSelectedEngine(.`7zip`, for: "zip") + let extracted = try await state.extractToTemp(item: helper) + #expect(try Data(contentsOf: extracted) == helperContents(in: zip, dir: dir)) + } + + /// So does an archive opened inside the archive, with the engine that read + /// it — not the outer one's, which is another format's. + @Test func anArchiveOpenedInsideKeepsTheEngineThatReadIt() async throws { + let dir = try makeTempDir() + defer { try? FileManager.default.removeItem(at: dir) } + let zip = try appleDoubleZip(in: dir) + let outer = dir.appendingPathComponent("outer.7z") + try SevenZipArchive.writeArchive( + destination: outer, items: [.addFile(archivePath: "appledouble.zip", diskPath: zip)], + options: .init(format: .sevenZ)) + let (settings, state) = stateWithSettings(zip: .xad) + state.open(url: outer) + try await state.openTask?.value + try await state.openAsync(item: try #require(item("appledouble.zip", in: state))) + let helper = try #require(item("payload/Contents/MacOS/helper", in: state)) + + settings.setSelectedEngine(.`7zip`, for: "zip") + let extracted = try await state.extractToTemp(item: helper) + #expect(try Data(contentsOf: extracted) == helperContents(in: zip, dir: dir)) + } + // MARK: - macOS metadata (#191, #216) // A zip has nowhere to keep a resource fork or an extended attribute, so