From 6e31e14639766869c40d2c2613c2831efc6731ab Mon Sep 17 00:00:00 2001 From: dmiedema Date: Wed, 26 Sep 2018 23:28:06 -0700 Subject: [PATCH 1/4] Add `make test` command Lets make running tests easier by adding a Make command for it --- Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 94574ba..f77e193 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ # By default export all variables export -.PHONY: install release debug build setup clean +.PHONY: install release debug build setup clean test PROJECT ?= 'modulo.xcodeproj' SCHEME ?= 'modulo' @@ -11,6 +11,9 @@ CONFIGURATION ?= 'Debug' # Build for debugging debug: build +test: + xcodebuild -project $(PROJECT) -scheme ModuloKit test + # Install `modulo` to `/usr/local/bin` install: release cp $(SYMROOT)/Release/modulo /usr/local/bin/ From 8a3f6a6e4180967201b0275f87d752503dc9c6ce Mon Sep 17 00:00:00 2001 From: dmiedema Date: Thu, 27 Sep 2018 11:33:33 -0700 Subject: [PATCH 2/4] Support tracking dependencies by branch In addition to `version` and all the features around that, we can also explicitly track a remote `branch` that will be fetched/updated when `modulo update` is run. --- ModuloKit/Actions.swift | 21 ++++++-- ModuloKit/Commands/AddCommand.swift | 13 +++-- ModuloKit/SCM/Git.swift | 50 ++++++++++++++++++- ModuloKit/SCM/SCM.swift | 1 + ModuloKit/Specs/DependencySpec.swift | 10 ++-- ModuloKitTests/TestAdd.swift | 29 +++++++++++ .../xcshareddata/xcschemes/modulo.xcscheme | 4 +- 7 files changed, 116 insertions(+), 12 deletions(-) diff --git a/ModuloKit/Actions.swift b/ModuloKit/Actions.swift index faadac5..51c4f45 100644 --- a/ModuloKit/Actions.swift +++ b/ModuloKit/Actions.swift @@ -22,8 +22,8 @@ open class Actions { } } - open func addDependency(_ url: String, version: SemverRange?, unmanaged: Bool) -> ErrorCode { - let dep = DependencySpec(repositoryURL: url, version: version) + open func addDependency(_ url: String, version: SemverRange?, branch: String?, unmanaged: Bool) -> ErrorCode { + let dep = DependencySpec(repositoryURL: url, version: version, branch: branch) if var workingSpec = ModuleSpec.workingSpec() { // does this dep already exist in here?? if let _ = workingSpec.dependencyForURL(url) { @@ -77,6 +77,16 @@ open class Actions { exit(checkoutResult.errorMessage()) } } + if let branch = dep.branch { + let checkoutResult = scm.checkout(branch: branch, path: clonePath) + if checkoutResult != .success { + exit(checkoutResult.errorMessage()) + } + let pullResult = scm.pull(clonePath, remoteData: nil) + if pullResult != .success { + exit(pullResult.errorMessage()) + } + } // things worked, so add it to the approprate place in the overall state. if explicit { @@ -106,8 +116,13 @@ open class Actions { if checkoutResult != .success { exit(checkoutResult.errorMessage()) } + } else if let branch = dep.branch { + let checkoutResult = scm.checkout(branch: branch, path: clonePath) + if checkoutResult != .success { + exit(checkoutResult.errorMessage()) + } } else { - exit("\(dep.name()) doesn't have a version and isn't unmanaged, not sure what to do.") + exit("\(dep.name()) doesn't have a version, branch, and isn't marked as 'unmanaged', not sure what to do.") } } } diff --git a/ModuloKit/Commands/AddCommand.swift b/ModuloKit/Commands/AddCommand.swift index 66de609..da6674f 100644 --- a/ModuloKit/Commands/AddCommand.swift +++ b/ModuloKit/Commands/AddCommand.swift @@ -16,6 +16,7 @@ import Foundation open class AddCommand: NSObject, Command { // internal properties fileprivate var version: SemverRange? = nil + fileprivate var branch: String? = nil fileprivate var repositoryURL: String! = nil fileprivate var shouldUpdate: Bool = false fileprivate var unmanaged: Bool = false @@ -40,6 +41,12 @@ open class AddCommand: NSObject, Command { self.version = SemverRange(value) } } + + addOptionValue(["--branch"], usage: "specify the branch to track", valueSignature: "") { (option, value) in + if let value = value { + self.branch = value + } + } addOption(["--unmanaged"], usage: "specifies that this module will be unmanaged") { (option, value) in self.unmanaged = true @@ -57,8 +64,8 @@ open class AddCommand: NSObject, Command { open func execute(_ otherParams: Array?) -> Int { let actions = Actions() - if version == nil && unmanaged == false { - writeln(.stderr, "A version or range must be specified via --version, or --unmanaged must be used.") + if version == nil && branch == nil && unmanaged == false { + writeln(.stderr, "A version or range must be specified via --version, a branch must be specified via --branch, or --unmanaged must be used.") return ErrorCode.commandError.rawValue } @@ -69,7 +76,7 @@ open class AddCommand: NSObject, Command { } } - let result = actions.addDependency(repositoryURL, version: version, unmanaged: unmanaged) + let result = actions.addDependency(repositoryURL, version: version, branch: branch, unmanaged: unmanaged) if result == .success { if shouldUpdate { writeln(.stdout, "Added \(String(describing: repositoryURL)).") diff --git a/ModuloKit/SCM/Git.swift b/ModuloKit/SCM/Git.swift index f1fa95a..301611d 100644 --- a/ModuloKit/SCM/Git.swift +++ b/ModuloKit/SCM/Git.swift @@ -100,7 +100,7 @@ open class Git: SCM { let initialWorkingPath = FileManager.workingPath() FileManager.setWorkingPath(path) - let updateCommand = "git fetch --recurse-submodules --all --tags" + let updateCommand = "git fetch --recurse-submodules --all" let status = runCommand(updateCommand) FileManager.setWorkingPath(initialWorkingPath) @@ -175,6 +175,54 @@ open class Git: SCM { return .success } + + open func checkout(branch: String, path: String) -> SCMResult { + if !FileManager.fileExists(path) { + return .error(code: 1, message: "Module path '\(path)' does not exist.") + } + + var checkoutCommand = "" + + let initialWorkingPath = FileManager.workingPath() + FileManager.setWorkingPath(path) + + let existingBranches = branches(".") + + var neededFetch = false + var fetchResult: Int32? = nil + if !existingBranches.contains(branch) { + // try fetching it directly + fetchResult = runCommand("git fetch origin \(branch)") + neededFetch = true + } + + checkoutCommand = "git checkout origin/\(branch) --quiet" + + if neededFetch, + let fetchResult = fetchResult, + fetchResult != 0 { + if verbose { + writeln(.stderr, "Unable to find branch '\(branch)'.") + } + return .error(code: SCMDefaultError, message: "Unable to find a match for \(branch).") + } + + let status = runCommand(checkoutCommand) + + let submodulesResult = collectAnySubmodules() + + FileManager.setWorkingPath(initialWorkingPath) + + if status != 0 { + return .error(code: status, message: "Unable to checkout '\(branch)'.") + } + + if submodulesResult != .success { + return submodulesResult + } + + return .success + } open func adjustIgnoreFile(pattern: String, removing: Bool) -> SCMResult { let localModulesPath = State.instance.modulePathName diff --git a/ModuloKit/SCM/SCM.swift b/ModuloKit/SCM/SCM.swift index 5f130a4..6cb58cf 100644 --- a/ModuloKit/SCM/SCM.swift +++ b/ModuloKit/SCM/SCM.swift @@ -76,6 +76,7 @@ public protocol SCM { func fetch(_ path: String) -> SCMResult func pull(_ path: String, remoteData: String?) -> SCMResult func checkout(version: SemverRange, path: String) -> SCMResult + func checkout(branch: String, path: String) -> SCMResult func remove(_ path: String) -> SCMResult func adjustIgnoreFile(pattern: String, removing: Bool) -> SCMResult func checkStatus(_ path: String) -> SCMResult diff --git a/ModuloKit/Specs/DependencySpec.swift b/ModuloKit/Specs/DependencySpec.swift index 03f8f63..31b6ca3 100644 --- a/ModuloKit/Specs/DependencySpec.swift +++ b/ModuloKit/Specs/DependencySpec.swift @@ -17,10 +17,12 @@ public struct DependencySpec { var repositoryURL: String // version or version range var version: SemverRange? + /// Branch to track + var branch: String? var unmanaged: Bool { get { - return (version == nil) + return (version == nil) && (branch == nil) } } } @@ -29,7 +31,8 @@ extension DependencySpec: ELDecodable { public static func decode(_ json: JSON?) throws -> DependencySpec { return try DependencySpec( repositoryURL: json ==> "repositoryURL", - version: json ==> "version" + version: json ==> "version", + branch: json ==> "branch" ) } @@ -42,7 +45,8 @@ extension DependencySpec: ELEncodable { public func encode() throws -> JSON { return try encodeToJSON([ "repositoryURL" <== repositoryURL, - "version" <== version + "version" <== version, + "branch" <== branch ]) } } diff --git a/ModuloKitTests/TestAdd.swift b/ModuloKitTests/TestAdd.swift index fcc41ee..45a2b5d 100644 --- a/ModuloKitTests/TestAdd.swift +++ b/ModuloKitTests/TestAdd.swift @@ -78,4 +78,33 @@ class TestAdd: XCTestCase { FileManager.setWorkingPath("..") } + + func testAddModuleByBranch() { + let status = Git().clone("git@github.com:modulo-dm/test-add.git", path: "test-add") + XCTAssertTrue(status == .success) + + FileManager.setWorkingPath("test-add") + + let repoURL = "git@github.com:modulo-dm/test-add-update.git" + + let result = Modulo.run(["add", repoURL, "--branch", "master", "-v"]) + XCTAssertTrue(result == .success) + + + guard let spec = ModuleSpec.load(contentsOfFile: specFilename) else { + XCTFail("Failed to get spec from file \(specFilename)") + return } + XCTAssertTrue(spec.dependencies.count > 0) + guard let dep = spec.dependencyForURL(repoURL) else { + XCTFail("Failed to find dependency for url \(repoURL) in spec \(spec)") + return } + XCTAssertNil(dep.version) + XCTAssertFalse(dep.unmanaged) + XCTAssertNotNil(dep.branch) + XCTAssertTrue(dep.branch == "master") + + FileManager.setWorkingPath("..") + + Git().remove("test-add") + } } diff --git a/modulo.xcodeproj/xcshareddata/xcschemes/modulo.xcscheme b/modulo.xcodeproj/xcshareddata/xcschemes/modulo.xcscheme index b00a1fb..19a2b13 100644 --- a/modulo.xcodeproj/xcshareddata/xcschemes/modulo.xcscheme +++ b/modulo.xcodeproj/xcshareddata/xcschemes/modulo.xcscheme @@ -69,11 +69,11 @@ + isEnabled = "NO"> + isEnabled = "YES"> Date: Thu, 27 Sep 2018 15:21:04 -0700 Subject: [PATCH 3/4] Update for feedback & remove `--branch` option Make for the `--unmanaged` flag on `add` to have an optional argument of a commit or a branch. However this lead to complications around other flags being passed such as `add blah --unmanaged -u -v`. To resolve that issue, the properties on `Option` were move to `public` so the add command could try its best to still apply arguments when they were inadvertantly swallowed by the now `--unmanaged` optional value. Also the `unmanagedValue` is tracked in the `.modulo` file like `branch` was but is more generic supporting an entire commit hash or a branch name --- Modules/ELCLI/ELCLI/Options.swift | 8 +++--- ModuloKit/Actions.swift | 16 ++++++------ ModuloKit/Commands/AddCommand.swift | 31 +++++++++++++++-------- ModuloKit/SCM/Git.swift | 27 ++++++++------------ ModuloKit/SCM/SCM.swift | 4 ++- ModuloKit/Specs/DependencySpec.swift | 11 ++++---- ModuloKitTests/TestAdd.swift | 38 ++++++++++++++++++++++++---- ModuloKitTests/TestDummyApp.swift | 2 +- 8 files changed, 86 insertions(+), 51 deletions(-) diff --git a/Modules/ELCLI/ELCLI/Options.swift b/Modules/ELCLI/ELCLI/Options.swift index 1a9be70..2fed79b 100755 --- a/Modules/ELCLI/ELCLI/Options.swift +++ b/Modules/ELCLI/ELCLI/Options.swift @@ -11,10 +11,10 @@ import Foundation public typealias OptionClosure = (_ option: String?, _ value: String?) -> Void public struct Option { - let usage: String? - let flags: Array? - let valueSignatures: Array? - let closure: OptionClosure + public let usage: String? + public let flags: Array? + public let valueSignatures: Array? + public let closure: OptionClosure init(flags: Array?, usage: String?, valueSignatures: Array?, closure: @escaping OptionClosure) { self.flags = flags diff --git a/ModuloKit/Actions.swift b/ModuloKit/Actions.swift index 51c4f45..98244de 100644 --- a/ModuloKit/Actions.swift +++ b/ModuloKit/Actions.swift @@ -22,8 +22,8 @@ open class Actions { } } - open func addDependency(_ url: String, version: SemverRange?, branch: String?, unmanaged: Bool) -> ErrorCode { - let dep = DependencySpec(repositoryURL: url, version: version, branch: branch) + open func addDependency(_ url: String, version: SemverRange?, unmanagedValue: String?, unmanaged: Bool) -> ErrorCode { + let dep = DependencySpec(repositoryURL: url, version: version, unmanagedValue: unmanagedValue) if var workingSpec = ModuleSpec.workingSpec() { // does this dep already exist in here?? if let _ = workingSpec.dependencyForURL(url) { @@ -77,8 +77,8 @@ open class Actions { exit(checkoutResult.errorMessage()) } } - if let branch = dep.branch { - let checkoutResult = scm.checkout(branch: branch, path: clonePath) + if let unmanagedValue = dep.unmanagedValue { + let checkoutResult = scm.checkout(branchOrHash: unmanagedValue, path: clonePath) if checkoutResult != .success { exit(checkoutResult.errorMessage()) } @@ -102,7 +102,7 @@ open class Actions { } // if they're unmanaged and on a branch, tracking a remote, just do a pull - if dep.unmanaged == true, let currentBranch = scm.branchName(clonePath) { + if dep.unmanaged == true && dep.unmanagedValue == nil, let currentBranch = scm.branchName(clonePath) { if scm.remoteTrackingBranch(currentBranch) != nil { let pullResult = scm.pull(clonePath, remoteData: nil) if pullResult != .success { @@ -116,13 +116,13 @@ open class Actions { if checkoutResult != .success { exit(checkoutResult.errorMessage()) } - } else if let branch = dep.branch { - let checkoutResult = scm.checkout(branch: branch, path: clonePath) + } else if let unmanagedValue = dep.unmanagedValue { + let checkoutResult = scm.checkout(branchOrHash: unmanagedValue, path: clonePath) if checkoutResult != .success { exit(checkoutResult.errorMessage()) } } else { - exit("\(dep.name()) doesn't have a version, branch, and isn't marked as 'unmanaged', not sure what to do.") + exit("\(dep.name()) doesn't have a version and isn't marked as 'unmanaged', not sure what to do.") } } } diff --git a/ModuloKit/Commands/AddCommand.swift b/ModuloKit/Commands/AddCommand.swift index da6674f..f4493ce 100644 --- a/ModuloKit/Commands/AddCommand.swift +++ b/ModuloKit/Commands/AddCommand.swift @@ -16,10 +16,10 @@ import Foundation open class AddCommand: NSObject, Command { // internal properties fileprivate var version: SemverRange? = nil - fileprivate var branch: String? = nil fileprivate var repositoryURL: String! = nil fileprivate var shouldUpdate: Bool = false fileprivate var unmanaged: Bool = false + fileprivate var unmanagedValue: String? = nil // Protocol conformance open var name: String { return "add" } @@ -41,15 +41,24 @@ open class AddCommand: NSObject, Command { self.version = SemverRange(value) } } - - addOptionValue(["--branch"], usage: "specify the branch to track", valueSignature: "") { (option, value) in - if let value = value { - self.branch = value - } - } - addOption(["--unmanaged"], usage: "specifies that this module will be unmanaged") { (option, value) in + addOptionValue(["--unmanaged"], usage: "specifies that this module will be unmanaged", valueSignature: "<[hash|branch|nothing]>") { (option, value) -> Void in self.unmanaged = true + if let value = value { + if !value.hasPrefix("-") { + self.unmanagedValue = value + } else { + if self.verbose { + writeln(.stderr, "Assuming '\(value)' is a flag and not a branch/commit hash since it begins with '-' and will not track it.") + } + // TODO: Somehow reprocess this `value` as a flag + self.options.filter({ (option) -> Bool in + option.flags?.contains(value) ?? false + }).forEach({ (option) in + option.closure(value, nil) + }) + } + } } addOption(["-u", "--update"], usage: "performs the update command after adding a module") { (option, value) in @@ -64,8 +73,8 @@ open class AddCommand: NSObject, Command { open func execute(_ otherParams: Array?) -> Int { let actions = Actions() - if version == nil && branch == nil && unmanaged == false { - writeln(.stderr, "A version or range must be specified via --version, a branch must be specified via --branch, or --unmanaged must be used.") + if version == nil && unmanaged == false { + writeln(.stderr, "A version or range must be specified via --version or --unmanaged must be used.") return ErrorCode.commandError.rawValue } @@ -76,7 +85,7 @@ open class AddCommand: NSObject, Command { } } - let result = actions.addDependency(repositoryURL, version: version, branch: branch, unmanaged: unmanaged) + let result = actions.addDependency(repositoryURL, version: version, unmanagedValue: unmanagedValue, unmanaged: unmanaged) if result == .success { if shouldUpdate { writeln(.stdout, "Added \(String(describing: repositoryURL)).") diff --git a/ModuloKit/SCM/Git.swift b/ModuloKit/SCM/Git.swift index 301611d..c4bd287 100644 --- a/ModuloKit/SCM/Git.swift +++ b/ModuloKit/SCM/Git.swift @@ -176,7 +176,7 @@ open class Git: SCM { return .success } - open func checkout(branch: String, path: String) -> SCMResult { + open func checkout(branchOrHash: String, path: String) -> SCMResult { if !FileManager.fileExists(path) { return .error(code: 1, message: "Module path '\(path)' does not exist.") } @@ -186,25 +186,20 @@ open class Git: SCM { let initialWorkingPath = FileManager.workingPath() FileManager.setWorkingPath(path) - let existingBranches = branches(".") + // try fetching it directly + let fetchResult = runCommand("git fetch origin \(branchOrHash)") - var neededFetch = false - var fetchResult: Int32? = nil - if !existingBranches.contains(branch) { - // try fetching it directly - fetchResult = runCommand("git fetch origin \(branch)") - neededFetch = true + if branches(".").contains(branchOrHash) { + checkoutCommand = "git checkout origin/\(branchOrHash) --quiet" + } else { + checkoutCommand = "git checkout \(branchOrHash) --quiet" } - checkoutCommand = "git checkout origin/\(branch) --quiet" - - if neededFetch, - let fetchResult = fetchResult, - fetchResult != 0 { + if fetchResult != 0 { if verbose { - writeln(.stderr, "Unable to find branch '\(branch)'.") + writeln(.stderr, "Unable to find unmanaged value '\(branchOrHash)'.") } - return .error(code: SCMDefaultError, message: "Unable to find a match for \(branch).") + return .error(code: SCMDefaultError, message: "Unable to find a match for \(branchOrHash).") } let status = runCommand(checkoutCommand) @@ -214,7 +209,7 @@ open class Git: SCM { FileManager.setWorkingPath(initialWorkingPath) if status != 0 { - return .error(code: status, message: "Unable to checkout '\(branch)'.") + return .error(code: status, message: "Unable to checkout '\(branchOrHash)'.") } if submodulesResult != .success { diff --git a/ModuloKit/SCM/SCM.swift b/ModuloKit/SCM/SCM.swift index 6cb58cf..710a859 100644 --- a/ModuloKit/SCM/SCM.swift +++ b/ModuloKit/SCM/SCM.swift @@ -76,7 +76,9 @@ public protocol SCM { func fetch(_ path: String) -> SCMResult func pull(_ path: String, remoteData: String?) -> SCMResult func checkout(version: SemverRange, path: String) -> SCMResult - func checkout(branch: String, path: String) -> SCMResult + /// Check out an arbitrary point or the HEAD of a branch (in git) + /// or the equivalent in other SCM solutions + func checkout(branchOrHash: String, path: String) -> SCMResult func remove(_ path: String) -> SCMResult func adjustIgnoreFile(pattern: String, removing: Bool) -> SCMResult func checkStatus(_ path: String) -> SCMResult diff --git a/ModuloKit/Specs/DependencySpec.swift b/ModuloKit/Specs/DependencySpec.swift index 31b6ca3..8ff1b0c 100644 --- a/ModuloKit/Specs/DependencySpec.swift +++ b/ModuloKit/Specs/DependencySpec.swift @@ -17,12 +17,13 @@ public struct DependencySpec { var repositoryURL: String // version or version range var version: SemverRange? - /// Branch to track - var branch: String? + /// Optional unmanaged property to track + /// such as a branch name, commit hash, or nothing + var unmanagedValue: String? var unmanaged: Bool { get { - return (version == nil) && (branch == nil) + return (version == nil) } } } @@ -32,7 +33,7 @@ extension DependencySpec: ELDecodable { return try DependencySpec( repositoryURL: json ==> "repositoryURL", version: json ==> "version", - branch: json ==> "branch" + unmanagedValue: json ==> "unmanagedValue" ) } @@ -46,7 +47,7 @@ extension DependencySpec: ELEncodable { return try encodeToJSON([ "repositoryURL" <== repositoryURL, "version" <== version, - "branch" <== branch + "unmanagedValue" <== unmanagedValue ]) } } diff --git a/ModuloKitTests/TestAdd.swift b/ModuloKitTests/TestAdd.swift index 45a2b5d..113fe3a 100644 --- a/ModuloKitTests/TestAdd.swift +++ b/ModuloKitTests/TestAdd.swift @@ -79,7 +79,7 @@ class TestAdd: XCTestCase { FileManager.setWorkingPath("..") } - func testAddModuleByBranch() { + func testAddUnmanagedModuleWithBranch() { let status = Git().clone("git@github.com:modulo-dm/test-add.git", path: "test-add") XCTAssertTrue(status == .success) @@ -87,7 +87,7 @@ class TestAdd: XCTestCase { let repoURL = "git@github.com:modulo-dm/test-add-update.git" - let result = Modulo.run(["add", repoURL, "--branch", "master", "-v"]) + let result = Modulo.run(["add", repoURL, "--unmanaged", "master", "-v"]) XCTAssertTrue(result == .success) @@ -99,9 +99,37 @@ class TestAdd: XCTestCase { XCTFail("Failed to find dependency for url \(repoURL) in spec \(spec)") return } XCTAssertNil(dep.version) - XCTAssertFalse(dep.unmanaged) - XCTAssertNotNil(dep.branch) - XCTAssertTrue(dep.branch == "master") + XCTAssertTrue(dep.unmanaged) + XCTAssertNotNil(dep.unmanagedValue) + XCTAssertTrue(dep.unmanagedValue == "master") + + FileManager.setWorkingPath("..") + + Git().remove("test-add") + } + + func testAddModuleUnmanagedNoArgs() { + let status = Git().clone("git@github.com:modulo-dm/test-add.git", path: "test-add") + XCTAssertTrue(status == .success) + + FileManager.setWorkingPath("test-add") + + let repoURL = "git@github.com:modulo-dm/test-add-update.git" + + let result = Modulo.run(["add", repoURL, "--unmanaged", "-v"]) + XCTAssertTrue(result == .success) + + + guard let spec = ModuleSpec.load(contentsOfFile: specFilename) else { + XCTFail("Failed to get spec from file \(specFilename)") + return } + XCTAssertTrue(spec.dependencies.count > 0) + guard let dep = spec.dependencyForURL(repoURL) else { + XCTFail("Failed to find dependency for url \(repoURL) in spec \(spec)") + return } + XCTAssertNil(dep.version) + XCTAssertTrue(dep.unmanaged) + XCTAssertNil(dep.unmanagedValue) FileManager.setWorkingPath("..") diff --git a/ModuloKitTests/TestDummyApp.swift b/ModuloKitTests/TestDummyApp.swift index 871646c..1259c23 100644 --- a/ModuloKitTests/TestDummyApp.swift +++ b/ModuloKitTests/TestDummyApp.swift @@ -41,7 +41,7 @@ class TestDummyApp: XCTestCase { XCTAssertTrue(spec!.dependencyForURL("git@github.com:modulo-dm/test-add-update.git") != nil) let checkedOut = Git().branchName("modules/test-add-update") - XCTAssertTrue(checkedOut == "master") + XCTAssertTrue(checkedOut == "master", "checkedOut should have been 'master' but was '\(String(describing: checkedOut))'") XCTAssertTrue(FileManager.fileExists("modules/test-add-update")) XCTAssertTrue(FileManager.fileExists("modules/test-dep1")) From 1a3a5dc39e84e1aeeb4c5c301ae45f8a30406cd1 Mon Sep 17 00:00:00 2001 From: dmiedema Date: Sat, 29 Sep 2018 00:10:11 -0700 Subject: [PATCH 4/4] Cleanup hack to process flags in add command Rather than have the add command hack in support for optional values to flags make it a core feature of `ELCLI`. That seems like the proper place to handle this situation and also doesn't break existing functionality/assumptions in all the other uses/tests. --- Modules/ELCLI/ELCLI/CLI.swift | 15 ++++++++++++++- Modules/ELCLI/ELCLI/Options.swift | 8 ++++---- ModuloKit/Commands/AddCommand.swift | 16 +--------------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/Modules/ELCLI/ELCLI/CLI.swift b/Modules/ELCLI/ELCLI/CLI.swift index 84a3a70..b320799 100755 --- a/Modules/ELCLI/ELCLI/CLI.swift +++ b/Modules/ELCLI/ELCLI/CLI.swift @@ -154,7 +154,20 @@ open class CLI { // it's a "--flag value" type argument. if index < arguments.count - 1 { value = arguments[index + 1] - skipNext = true + // If we're assuming `--flag value` make sure + // our `value` isn't a stop marker or flag. If it is + // our value should instead be `nil`'d out so our + // command does not get a value where it is our next + // argument. + if let argValue = value, + isStopMarker(argValue) || isFlag(argValue) { + value = nil + } else { + // However if that's not the case we should skip + // the next command because we really did get + // `--flag value` + skipNext = true + } } } } diff --git a/Modules/ELCLI/ELCLI/Options.swift b/Modules/ELCLI/ELCLI/Options.swift index 2fed79b..1a9be70 100755 --- a/Modules/ELCLI/ELCLI/Options.swift +++ b/Modules/ELCLI/ELCLI/Options.swift @@ -11,10 +11,10 @@ import Foundation public typealias OptionClosure = (_ option: String?, _ value: String?) -> Void public struct Option { - public let usage: String? - public let flags: Array? - public let valueSignatures: Array? - public let closure: OptionClosure + let usage: String? + let flags: Array? + let valueSignatures: Array? + let closure: OptionClosure init(flags: Array?, usage: String?, valueSignatures: Array?, closure: @escaping OptionClosure) { self.flags = flags diff --git a/ModuloKit/Commands/AddCommand.swift b/ModuloKit/Commands/AddCommand.swift index f4493ce..599e004 100644 --- a/ModuloKit/Commands/AddCommand.swift +++ b/ModuloKit/Commands/AddCommand.swift @@ -44,21 +44,7 @@ open class AddCommand: NSObject, Command { addOptionValue(["--unmanaged"], usage: "specifies that this module will be unmanaged", valueSignature: "<[hash|branch|nothing]>") { (option, value) -> Void in self.unmanaged = true - if let value = value { - if !value.hasPrefix("-") { - self.unmanagedValue = value - } else { - if self.verbose { - writeln(.stderr, "Assuming '\(value)' is a flag and not a branch/commit hash since it begins with '-' and will not track it.") - } - // TODO: Somehow reprocess this `value` as a flag - self.options.filter({ (option) -> Bool in - option.flags?.contains(value) ?? false - }).forEach({ (option) in - option.closure(value, nil) - }) - } - } + self.unmanagedValue = value } addOption(["-u", "--update"], usage: "performs the update command after adding a module") { (option, value) in