Skip to content

[Option] Support MultiArg options #1609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
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
4 changes: 2 additions & 2 deletions Sources/SwiftDriver/Jobs/CommandLineArguments.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ extension Array where Element == Job.ArgTemplate {
/// Append an option's spelling to the command line arguments.
mutating func appendFlag(_ option: Option) {
switch option.kind {
case .flag, .joinedOrSeparate, .remaining, .separate:
case .flag, .joinedOrSeparate, .remaining, .separate, .multiArg:
break
case .commaJoined, .input, .joined:
fatalError("Option cannot be appended as a flag: \(option)")
Expand Down Expand Up @@ -95,7 +95,7 @@ extension Array where Element == Job.ArgTemplate {
assert(!option.attributes.contains(.argumentIsPath))
appendFlag(option.spelling + argument.asMultiple.joined(separator: ","))

case .remaining:
case .remaining, .multiArg:
appendFlag(option.spelling)
for arg in argument.asMultiple {
try appendSingleArgument(option: option, argument: arg)
Expand Down
10 changes: 9 additions & 1 deletion Sources/SwiftOptions/Option.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ public struct Option {
/// An option with multiple arguments, which are collected by splitting
/// the text directly following the spelling at each comma.
case commaJoined
/// An option with multiple arguments, which the number of arguments is
/// specified by numArgs.
case multiArg
}

/// The spelling of the option, including any leading dashes.
Expand All @@ -81,18 +84,23 @@ public struct Option {
/// The group in which this option occurs.
public let group: Group?

/// The number of arguments for MultiArg options.
public let numArgs: UInt

public init(_ spelling: String, _ kind: Kind,
alias: Option? = nil,
attributes: OptionAttributes = [], metaVar: String? = nil,
helpText: String? = nil,
group: Group? = nil) {
group: Group? = nil,
numArgs: UInt = 0) {
self.spelling = spelling
self.kind = kind
self.aliasFunction = alias.map { aliasOption in { aliasOption }}
self.attributes = attributes
self.metaVar = metaVar
self.helpText = helpText
self.group = group
self.numArgs = numArgs
}
}

Expand Down
15 changes: 15 additions & 0 deletions Sources/SwiftOptions/OptionParsing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,21 @@ extension OptionTable {

parsedOptions.addOption(option, argument: .single(arguments[index]))
index += 1

case .multiArg:
if argument != option.spelling {
throw OptionParseError.unknownOption(
index: index - 1, argument: argument)
}
let endIdx = index + Int(option.numArgs)
if endIdx > arguments.endIndex {
throw OptionParseError.missingArgument(
index: index - 1, argument: argument)
}
parsedOptions.addOption(option, argument: .multiple(Array()))
arguments[index..<endIdx].map { String($0) }.forEach { parsedOptions.addInput($0) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this index off the end of the array if the user doesn't provide enough args?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Added a testcase for that too.

index = endIdx

}
}
parsedOptions.buildIndex()
Expand Down
2 changes: 1 addition & 1 deletion Sources/SwiftOptions/OptionTable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ extension OptionTable {
case .joined, .commaJoined:
displayName += option.metaVar ?? "<value>"

case .separate, .remaining, .joinedOrSeparate:
case .separate, .remaining, .joinedOrSeparate, .multiArg:
displayName += " " + (option.metaVar ?? "<value>")
}

Expand Down
36 changes: 27 additions & 9 deletions Sources/SwiftOptions/Options.swift

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions Sources/SwiftOptions/ParsedOptions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ extension ParsedOption: CustomStringConvertible {
case .joinedOrSeparate, .separate:
return option.spelling + " " + argument.asSingle.spm_shellEscaped()

case .remaining:
case .remaining, .multiArg:
let args = argument.asMultiple
if args.isEmpty {
return option.spelling
Expand Down Expand Up @@ -193,7 +193,7 @@ extension ParsedOptions {
result.append(parsed.option.spelling)
result.append(parsed.argument.asSingle)

case .remaining:
case .remaining, .multiArg:
result.append(parsed.option.spelling)
result.append(contentsOf: parsed.argument.asMultiple)
}
Expand Down
11 changes: 10 additions & 1 deletion Sources/makeOptions/makeOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ enum class OptionKind {
RemainingArgs,
CommaJoined,
JoinedOrSeparate,
MultiArg,
};

#define LLVM_MAKE_OPT_ID_WITH_ID_PREFIX(ID_PREFIX, PREFIX, NAME, ID, KIND, \
Expand Down Expand Up @@ -113,6 +114,7 @@ struct RawOption {
unsigned flags;
const char *helpText;
const char *metaVar;
unsigned numArgs;

bool isGroup() const {
return kind == OptionKind::Group;
Expand All @@ -135,7 +137,7 @@ static const RawOption rawOptions[] = {
#define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
HELPTEXT, METAVAR, VALUES) \
{ OptionID::Opt_##ID, PREFIX, NAME, swiftify(#ID), OptionKind::KIND, \
OptionID::Opt_##GROUP, OptionID::Opt_##ALIAS, FLAGS, HELPTEXT, METAVAR },
OptionID::Opt_##GROUP, OptionID::Opt_##ALIAS, FLAGS, HELPTEXT, METAVAR, PARAM },
#include "swift/Option/Options.inc"
#undef OPTION
};
Expand Down Expand Up @@ -291,6 +293,10 @@ int makeOptions_main() {
out << ".separate";
break;

case OptionKind::MultiArg:
out << ".multiArg";
break;

case OptionKind::Group:
case OptionKind::Unknown:
assert(false && "Should have been filtered out");
Expand Down Expand Up @@ -350,6 +356,9 @@ int makeOptions_main() {
if (option.group != OptionID::Opt_INVALID) {
out << ", group: ." << groups[groupIndexByID[option.group]].id;
}
if (option.kind == OptionKind::MultiArg) {
out << ", numArgs: " << option.numArgs;
}
out << ")\n";
});
});
Expand Down
20 changes: 20 additions & 0 deletions Tests/SwiftOptionsTests/OptionParsingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,20 @@ final class SwiftDriverTests: XCTestCase {
XCTAssertEqual(results.description, "-- input1 input2")
}

func testParsingMultiArg() throws {
var options = OptionTable()
let two = Option("-two", .multiArg, attributes: [], numArgs: 2)
let three = Option("-three", .multiArg, attributes: [], numArgs: 3)
options.addNewOption(two)
options.addNewOption(three)
let results = try options.parse(["-two", "1", "2", "-three", "1", "2", "3", "-two", "2", "3"], for: .batch)
XCTAssertEqual(results.description, "-two 1 2 -three 1 2 3 -two 2 3")
// Check not enough arguments are passed.
XCTAssertThrowsError(try options.parse(["-two", "1"], for: .batch)) { error in
XCTAssertEqual(error as? OptionParseError, .missingArgument(index: 0, argument: "-two"))
}
}

func testParseErrors() {
let options = OptionTable()

Expand Down Expand Up @@ -85,3 +99,9 @@ final class SwiftDriverTests: XCTestCase {
}
}
}

extension OptionTable {
mutating func addNewOption(_ opt: Option) {
options.append(opt)
}
}