Renamed --delete-xip and --delete-app flags to --empty-trash

juanjonol · juanjonol · commit c49767b1f08f · 2022-09-10T17:51:52.000+02:00
This two flags do essentially the same thing (skip the Trash and irrevocably delete Xcode or its .xip) so it makes sense to use the same name for both.

- Although the Trash isn't really "emptied" (the files never get to the Trash in the first place), this is the most intuitively way I've found to refer to this at a high level.
- Alternatives considered:
   - `--skip-trash`. This explains better the current implementation, but I think it's more ambiguous: if the user doesn't know that by default files are moved to the Trash, "skip Trash" could be understood as "skip deleting files from the Trash"...
   - `--delete-immediately` (the way Finder refers to deleting a single file from the Trash). This is longer to type, and "immediately" could imply that, without this flag, files will be automatically deleted in the future somehow.
diff --git a/Sources/XcodesKit/XcodeInstaller.swift b/Sources/XcodesKit/XcodeInstaller.swift
@@ -166,22 +166,22 @@ public final class XcodeInstaller {
         case aria2(Path)
     }
 
-    public func install(_ installationType: InstallationType, dataSource: DataSource, downloader: Downloader, destination: Path, experimentalUnxip: Bool = false, deleteXip: Bool, noSuperuser: Bool) -> Promise<Void> {
+    public func install(_ installationType: InstallationType, dataSource: DataSource, downloader: Downloader, destination: Path, experimentalUnxip: Bool = false, emptyTrash: Bool, noSuperuser: Bool) -> Promise<Void> {
         return firstly { () -> Promise<InstalledXcode> in
-            return self.install(installationType, dataSource: dataSource, downloader: downloader, destination: destination, attemptNumber: 0, experimentalUnxip: experimentalUnxip, deleteXip: deleteXip, noSuperuser: noSuperuser)
+            return self.install(installationType, dataSource: dataSource, downloader: downloader, destination: destination, attemptNumber: 0, experimentalUnxip: experimentalUnxip, emptyTrash: emptyTrash, noSuperuser: noSuperuser)
         }
         .done { xcode in
             Current.logging.log("\nXcode \(xcode.version.descriptionWithoutBuildMetadata) has been installed to \(xcode.path.string)".green)
             Current.shell.exit(0)
         }
     }
     
-    private func install(_ installationType: InstallationType, dataSource: DataSource, downloader: Downloader, destination: Path, attemptNumber: Int, experimentalUnxip: Bool, deleteXip: Bool, noSuperuser: Bool) -> Promise<InstalledXcode> {
+    private func install(_ installationType: InstallationType, dataSource: DataSource, downloader: Downloader, destination: Path, attemptNumber: Int, experimentalUnxip: Bool, emptyTrash: Bool, noSuperuser: Bool) -> Promise<InstalledXcode> {
         return firstly { () -> Promise<(Xcode, URL)> in
             return self.getXcodeArchive(installationType, dataSource: dataSource, downloader: downloader, destination: destination, willInstall: true)
         }
         .then { xcode, url -> Promise<InstalledXcode> in
-            return self.installArchivedXcode(xcode, at: url, to: destination, experimentalUnxip: experimentalUnxip, deleteXip: deleteXip, noSuperuser: noSuperuser)
+            return self.installArchivedXcode(xcode, at: url, to: destination, experimentalUnxip: experimentalUnxip, emptyTrash: emptyTrash, noSuperuser: noSuperuser)
         }
         .recover { error -> Promise<InstalledXcode> in
             switch error {
@@ -198,7 +198,7 @@ public final class XcodeInstaller {
                         Current.logging.log(error.legibleLocalizedDescription.red)
                         Current.logging.log("Removing damaged XIP and re-attempting installation.\n")
                         try Current.files.removeItem(at: damagedXIPURL)
-                        return self.install(installationType, dataSource: dataSource, downloader: downloader, destination: destination, attemptNumber: attemptNumber + 1, experimentalUnxip: experimentalUnxip, deleteXip: deleteXip, noSuperuser: noSuperuser)
+                        return self.install(installationType, dataSource: dataSource, downloader: downloader, destination: destination, attemptNumber: attemptNumber + 1, experimentalUnxip: experimentalUnxip, emptyTrash: emptyTrash, noSuperuser: noSuperuser)
                     }
                 }
             default:
@@ -531,7 +531,7 @@ public final class XcodeInstaller {
         }
     }
 
-    public func installArchivedXcode(_ xcode: Xcode, at archiveURL: URL, to destination: Path, experimentalUnxip: Bool = false, deleteXip: Bool, noSuperuser: Bool) -> Promise<InstalledXcode> {
+    public func installArchivedXcode(_ xcode: Xcode, at archiveURL: URL, to destination: Path, experimentalUnxip: Bool = false, emptyTrash: Bool, noSuperuser: Bool) -> Promise<InstalledXcode> {
         return firstly { () -> Promise<InstalledXcode> in
             let destinationURL = destination.join("Xcode-\(xcode.version.descriptionWithoutBuildMetadata).app").url
             switch archiveURL.pathExtension {
@@ -551,8 +551,8 @@ public final class XcodeInstaller {
             }
         }
         .then { xcode -> Promise<InstalledXcode> in
-            Current.logging.log(InstallationStep.cleaningArchive(archiveName: archiveURL.lastPathComponent, shouldDelete: deleteXip).description)
-            if deleteXip {
+            Current.logging.log(InstallationStep.cleaningArchive(archiveName: archiveURL.lastPathComponent, shouldDelete: emptyTrash).description)
+            if emptyTrash {
                 try Current.files.removeItem(at: archiveURL)
             }
             else {
@@ -595,7 +595,7 @@ public final class XcodeInstaller {
         }
     }
 
-    public func uninstallXcode(_ versionString: String, directory: Path, deleteApp: Bool) -> Promise<Void> {
+    public func uninstallXcode(_ versionString: String, directory: Path, emptyTrash: Bool) -> Promise<Void> {
         return firstly { () -> Promise<InstalledXcode> in
             guard let version = Version(xcodeVersion: versionString) else {
                 Current.logging.log(Error.invalidVersion(versionString).legibleLocalizedDescription)
@@ -610,7 +610,7 @@ public final class XcodeInstaller {
             return Promise.value(installedXcode)
         }
         .map { installedXcode -> (InstalledXcode, URL?) in
-            if deleteApp {
+            if emptyTrash {
                 try Current.files.removeItem(at: installedXcode.path.url)
                 return (installedXcode, nil)
             }
diff --git a/Sources/xcodes/main.swift b/Sources/xcodes/main.swift
@@ -189,8 +189,8 @@ struct Xcodes: ParsableCommand {
         @Flag(help: "Don't ask for superuser (root) permission. Some optional steps of the installation will be skipped.")
         var noSuperuser: Bool = false
         
-        @Flag(help: "Completely delete Xcode .xip after installation, instead of moving it to the user's Trash.")
-        var deleteXip: Bool = false
+        @Flag(help: "Completely delete Xcode .xip after installation, instead of keeping it on the user's Trash.")
+        var emptyTrash: Bool = false
         
         @Option(help: "The directory to install Xcode into. Defaults to /Applications.",
                 completion: .directory)
@@ -227,7 +227,7 @@ struct Xcodes: ParsableCommand {
             
             let destination = getDirectory(possibleDirectory: directory)
             
-            installer.install(installation, dataSource: globalDataSource.dataSource, downloader: downloader, destination: destination, experimentalUnxip: experimentalUnxip, deleteXip: deleteXip, noSuperuser: noSuperuser)
+            installer.install(installation, dataSource: globalDataSource.dataSource, downloader: downloader, destination: destination, experimentalUnxip: experimentalUnxip, emptyTrash: emptyTrash, noSuperuser: noSuperuser)
                 .done { Install.exit() }
                 .catch { error in
                     Install.processDownloadOrInstall(error: error)
@@ -351,8 +351,8 @@ struct Xcodes: ParsableCommand {
                   completion: .custom { _ in Current.files.installedXcodes(getDirectory(possibleDirectory: nil)).sorted { $0.version < $1.version }.map { $0.version.appleDescription } })
         var version: [String] = []
         
-        @Flag(help: "Completely delete Xcode, instead of moving it to the user's Trash.")
-        var deleteApp: Bool = false
+        @Flag(help: "Completely delete Xcode, instead of keeping it on the user's Trash.")
+        var emptyTrash: Bool = false
         
         @OptionGroup
         var globalDirectory: GlobalDirectoryOption
@@ -365,7 +365,7 @@ struct Xcodes: ParsableCommand {
 
             let directory = getDirectory(possibleDirectory: globalDirectory.directory)
 
-            installer.uninstallXcode(version.joined(separator: " "), directory: directory, deleteApp: deleteApp)
+            installer.uninstallXcode(version.joined(separator: " "), directory: directory, emptyTrash: emptyTrash)
                 .done { Uninstall.exit() }
                 .catch { error in Uninstall.exit(withLegibleError: error) }
             
diff --git a/Tests/XcodesKitTests/XcodesKitTests.swift b/Tests/XcodesKitTests/XcodesKitTests.swift
@@ -86,23 +86,23 @@ final class XcodesKitTests: XCTestCase {
 
         let xcode = Xcode(version: Version("0.0.0")!, url: URL(fileURLWithPath: "/"), filename: "mock", releaseDate: nil)
         let installedXcode = InstalledXcode(path: Path("/Applications/Xcode-0.0.0.app")!)!
-        installer.installArchivedXcode(xcode, at: URL(fileURLWithPath: "/Xcode-0.0.0.xip"), to: Path.root.join("Applications"), deleteXip: false, noSuperuser: false)
+        installer.installArchivedXcode(xcode, at: URL(fileURLWithPath: "/Xcode-0.0.0.xip"), to: Path.root.join("Applications"), emptyTrash: false, noSuperuser: false)
             .catch { error in XCTAssertEqual(error as! XcodeInstaller.Error, XcodeInstaller.Error.failedSecurityAssessment(xcode: installedXcode, output: "")) }
     }
 
     func test_InstallArchivedXcode_VerifySigningCertificateFails_Throws() {
         Current.shell.codesignVerify = { _ in return Promise(error: Process.PMKError.execution(process: Process(), standardOutput: nil, standardError: nil)) }
 
         let xcode = Xcode(version: Version("0.0.0")!, url: URL(fileURLWithPath: "/"), filename: "mock", releaseDate: nil)
-        installer.installArchivedXcode(xcode, at: URL(fileURLWithPath: "/Xcode-0.0.0.xip"), to: Path.root.join("Applications"), deleteXip: false, noSuperuser: false)
+        installer.installArchivedXcode(xcode, at: URL(fileURLWithPath: "/Xcode-0.0.0.xip"), to: Path.root.join("Applications"), emptyTrash: false, noSuperuser: false)
             .catch { error in XCTAssertEqual(error as! XcodeInstaller.Error, XcodeInstaller.Error.codesignVerifyFailed(output: "")) }
     }
 
     func test_InstallArchivedXcode_VerifySigningCertificateDoesntMatch_Throws() {
         Current.shell.codesignVerify = { _ in return Promise.value((0, "", "")) }
 
         let xcode = Xcode(version: Version("0.0.0")!, url: URL(fileURLWithPath: "/"), filename: "mock", releaseDate: nil)
-        installer.installArchivedXcode(xcode, at: URL(fileURLWithPath: "/Xcode-0.0.0.xip"), to: Path.root.join("Applications"), deleteXip: false, noSuperuser: false)
+        installer.installArchivedXcode(xcode, at: URL(fileURLWithPath: "/Xcode-0.0.0.xip"), to: Path.root.join("Applications"), emptyTrash: false, noSuperuser: false)
             .catch { error in XCTAssertEqual(error as! XcodeInstaller.Error, XcodeInstaller.Error.unexpectedCodeSigningIdentity(identifier: "", certificateAuthority: [])) }
     }
 
@@ -115,7 +115,7 @@ final class XcodesKitTests: XCTestCase {
 
         let xcode = Xcode(version: Version("0.0.0")!, url: URL(fileURLWithPath: "/"), filename: "mock", releaseDate: nil)
         let xipURL = URL(fileURLWithPath: "/Xcode-0.0.0.xip")
-        installer.installArchivedXcode(xcode, at: xipURL, to: Path.root.join("Applications"), deleteXip: false, noSuperuser: false)
+        installer.installArchivedXcode(xcode, at: xipURL, to: Path.root.join("Applications"), emptyTrash: false, noSuperuser: false)
             .ensure { XCTAssertEqual(trashedItemAtURL, xipURL) }
             .cauterize()
     }
@@ -203,7 +203,7 @@ final class XcodesKitTests: XCTestCase {
 
         let expectation = self.expectation(description: "Finished")
 
-        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.root.join("Applications"), deleteXip: false, noSuperuser: false)
+        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.root.join("Applications"), emptyTrash: false, noSuperuser: false)
             .ensure {
                 let url = Bundle.module.url(forResource: "LogOutput-FullHappyPath", withExtension: "txt", subdirectory: "Fixtures")!
                 XCTAssertEqual(log, try! String(contentsOf: url))
@@ -296,7 +296,7 @@ final class XcodesKitTests: XCTestCase {
 
         let expectation = self.expectation(description: "Finished")
 
-        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.root.join("Applications"), deleteXip: false, noSuperuser: false)
+        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.root.join("Applications"), emptyTrash: false, noSuperuser: false)
             .ensure {
                 let url = Bundle.module.url(forResource: "LogOutput-FullHappyPath-NoColor", withExtension: "txt", subdirectory: "Fixtures")!
                 XCTAssertEqual(log, try! String(contentsOf: url))
@@ -393,7 +393,7 @@ final class XcodesKitTests: XCTestCase {
 
         let expectation = self.expectation(description: "Finished")
 
-        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.root.join("Applications"), deleteXip: false, noSuperuser: false)
+        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.root.join("Applications"), emptyTrash: false, noSuperuser: false)
             .ensure {
                 let url = Bundle.module.url(forResource: "LogOutput-FullHappyPath-NonInteractiveTerminal", withExtension: "txt", subdirectory: "Fixtures")!
                 XCTAssertEqual(log, try! String(contentsOf: url))
@@ -486,7 +486,7 @@ final class XcodesKitTests: XCTestCase {
 
         let expectation = self.expectation(description: "Finished")
 
-        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.home.join("Xcode"), deleteXip: false, noSuperuser: false)
+        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.home.join("Xcode"), emptyTrash: false, noSuperuser: false)
             .ensure {
                 let url = Bundle.module.url(forResource: "LogOutput-AlternativeDirectory", withExtension: "txt", subdirectory: "Fixtures")!
                 let expectedText = try! String(contentsOf: url).replacingOccurrences(of: "/Users/brandon", with: Path.home.string)
@@ -600,7 +600,7 @@ final class XcodesKitTests: XCTestCase {
 
         let expectation = self.expectation(description: "Finished")
 
-        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.root.join("Applications"), deleteXip: false, noSuperuser: false)
+        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.root.join("Applications"), emptyTrash: false, noSuperuser: false)
             .ensure {
                 let url = Bundle.module.url(forResource: "LogOutput-IncorrectSavedPassword", withExtension: "txt", subdirectory: "Fixtures")!
                 XCTAssertEqual(log, try! String(contentsOf: url))
@@ -718,7 +718,7 @@ final class XcodesKitTests: XCTestCase {
 
         let expectation = self.expectation(description: "Finished")
 
-        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.root.join("Applications"), deleteXip: false, noSuperuser: false)
+        installer.install(.version("0.0.0"), dataSource: .apple, downloader: .urlSession, destination: Path.root.join("Applications"), emptyTrash: false, noSuperuser: false)
             .ensure {
                 let url = Bundle.module.url(forResource: "LogOutput-DamagedXIP", withExtension: "txt", subdirectory: "Fixtures")!
                 let expectedText = try! String(contentsOf: url).replacingOccurrences(of: "/Users/brandon", with: Path.home.string)
@@ -778,7 +778,7 @@ final class XcodesKitTests: XCTestCase {
             return Promise.value((status: 0, out: "", err: ""))
         }
 
-        installer.uninstallXcode("0.0.0", directory: Path.root.join("Applications"), deleteApp: false)
+        installer.uninstallXcode("0.0.0", directory: Path.root.join("Applications"), emptyTrash: false)
             .ensure {
                 XCTAssertEqual(selectedPaths, ["/Applications/Xcode-2.0.1.app"])
                 XCTAssertEqual(trashedItemAtURL, installedXcodes[0].path.url)
@@ -823,7 +823,7 @@ final class XcodesKitTests: XCTestCase {
             return URL(fileURLWithPath: "\(NSHomeDirectory())/.Trash/\(itemURL.lastPathComponent)")
         }
 
-        installer.uninstallXcode("999", directory: Path.root.join("Applications"), deleteApp: false)
+        installer.uninstallXcode("999", directory: Path.root.join("Applications"), emptyTrash: false)
             .ensure {
                 XCTAssertEqual(trashedItemAtURL, installedXcodes[0].path.url)
             }

Original file line number	Diff line number	Diff line change
`@@ -166,22 +166,22 @@ public final class XcodeInstaller {`
`166`	`166`	`case aria2(Path)`
`167`	`167`	`}`
`168`	`168`
`169`		`- public func install(_ installationType: InstallationType, dataSource: DataSource, downloader: Downloader, destination: Path, experimentalUnxip: Bool = false, deleteXip: Bool, noSuperuser: Bool) -> Promise<Void> {`
	`169`	`+ public func install(_ installationType: InstallationType, dataSource: DataSource, downloader: Downloader, destination: Path, experimentalUnxip: Bool = false, emptyTrash: Bool, noSuperuser: Bool) -> Promise<Void> {`
`170`	`170`	`return firstly { () -> Promise<InstalledXcode> in`
`171`		`- return self.install(installationType, dataSource: dataSource, downloader: downloader, destination: destination, attemptNumber: 0, experimentalUnxip: experimentalUnxip, deleteXip: deleteXip, noSuperuser: noSuperuser)`
	`171`	`+ return self.install(installationType, dataSource: dataSource, downloader: downloader, destination: destination, attemptNumber: 0, experimentalUnxip: experimentalUnxip, emptyTrash: emptyTrash, noSuperuser: noSuperuser)`
`172`	`172`	`}`
`173`	`173`	`.done { xcode in`
`174`	`174`	`Current.logging.log("\nXcode \(xcode.version.descriptionWithoutBuildMetadata) has been installed to \(xcode.path.string)".green)`
`175`	`175`	`Current.shell.exit(0)`
`176`	`176`	`}`
`177`	`177`	`}`
`178`	`178`
`179`		`- private func install(_ installationType: InstallationType, dataSource: DataSource, downloader: Downloader, destination: Path, attemptNumber: Int, experimentalUnxip: Bool, deleteXip: Bool, noSuperuser: Bool) -> Promise<InstalledXcode> {`
	`179`	`+ private func install(_ installationType: InstallationType, dataSource: DataSource, downloader: Downloader, destination: Path, attemptNumber: Int, experimentalUnxip: Bool, emptyTrash: Bool, noSuperuser: Bool) -> Promise<InstalledXcode> {`
`180`	`180`	`return firstly { () -> Promise<(Xcode, URL)> in`
`181`	`181`	`return self.getXcodeArchive(installationType, dataSource: dataSource, downloader: downloader, destination: destination, willInstall: true)`
`182`	`182`	`}`
`183`	`183`	`.then { xcode, url -> Promise<InstalledXcode> in`
`184`		`- return self.installArchivedXcode(xcode, at: url, to: destination, experimentalUnxip: experimentalUnxip, deleteXip: deleteXip, noSuperuser: noSuperuser)`
	`184`	`+ return self.installArchivedXcode(xcode, at: url, to: destination, experimentalUnxip: experimentalUnxip, emptyTrash: emptyTrash, noSuperuser: noSuperuser)`
`185`	`185`	`}`
`186`	`186`	`.recover { error -> Promise<InstalledXcode> in`
`187`	`187`	`switch error {`
`@@ -198,7 +198,7 @@ public final class XcodeInstaller {`
`198`	`198`	`Current.logging.log(error.legibleLocalizedDescription.red)`
`199`	`199`	`Current.logging.log("Removing damaged XIP and re-attempting installation.\n")`
`200`	`200`	`try Current.files.removeItem(at: damagedXIPURL)`
`201`		`- return self.install(installationType, dataSource: dataSource, downloader: downloader, destination: destination, attemptNumber: attemptNumber + 1, experimentalUnxip: experimentalUnxip, deleteXip: deleteXip, noSuperuser: noSuperuser)`
	`201`	`+ return self.install(installationType, dataSource: dataSource, downloader: downloader, destination: destination, attemptNumber: attemptNumber + 1, experimentalUnxip: experimentalUnxip, emptyTrash: emptyTrash, noSuperuser: noSuperuser)`
`202`	`202`	`}`
`203`	`203`	`}`
`204`	`204`	`default:`
`@@ -531,7 +531,7 @@ public final class XcodeInstaller {`
`531`	`531`	`}`
`532`	`532`	`}`
`533`	`533`
`534`		`- public func installArchivedXcode(_ xcode: Xcode, at archiveURL: URL, to destination: Path, experimentalUnxip: Bool = false, deleteXip: Bool, noSuperuser: Bool) -> Promise<InstalledXcode> {`
	`534`	`+ public func installArchivedXcode(_ xcode: Xcode, at archiveURL: URL, to destination: Path, experimentalUnxip: Bool = false, emptyTrash: Bool, noSuperuser: Bool) -> Promise<InstalledXcode> {`
`535`	`535`	`return firstly { () -> Promise<InstalledXcode> in`
`536`	`536`	`let destinationURL = destination.join("Xcode-\(xcode.version.descriptionWithoutBuildMetadata).app").url`
`537`	`537`	`switch archiveURL.pathExtension {`
`@@ -551,8 +551,8 @@ public final class XcodeInstaller {`
`551`	`551`	`}`
`552`	`552`	`}`
`553`	`553`	`.then { xcode -> Promise<InstalledXcode> in`
`554`		`- Current.logging.log(InstallationStep.cleaningArchive(archiveName: archiveURL.lastPathComponent, shouldDelete: deleteXip).description)`
`555`		`- if deleteXip {`
	`554`	`+ Current.logging.log(InstallationStep.cleaningArchive(archiveName: archiveURL.lastPathComponent, shouldDelete: emptyTrash).description)`
	`555`	`+ if emptyTrash {`
`556`	`556`	`try Current.files.removeItem(at: archiveURL)`
`557`	`557`	`}`
`558`	`558`	`else {`
`@@ -595,7 +595,7 @@ public final class XcodeInstaller {`
`595`	`595`	`}`
`596`	`596`	`}`
`597`	`597`
`598`		`- public func uninstallXcode(_ versionString: String, directory: Path, deleteApp: Bool) -> Promise<Void> {`
	`598`	`+ public func uninstallXcode(_ versionString: String, directory: Path, emptyTrash: Bool) -> Promise<Void> {`
`599`	`599`	`return firstly { () -> Promise<InstalledXcode> in`
`600`	`600`	`guard let version = Version(xcodeVersion: versionString) else {`
`601`	`601`	`Current.logging.log(Error.invalidVersion(versionString).legibleLocalizedDescription)`
`@@ -610,7 +610,7 @@ public final class XcodeInstaller {`
`610`	`610`	`return Promise.value(installedXcode)`
`611`	`611`	`}`
`612`	`612`	`.map { installedXcode -> (InstalledXcode, URL?) in`
`613`		`- if deleteApp {`
	`613`	`+ if emptyTrash {`
`614`	`614`	`try Current.files.removeItem(at: installedXcode.path.url)`
`615`	`615`	`return (installedXcode, nil)`
`616`	`616`	`}`