diff --git a/common/command_line.cpp b/common/command_line.cpp index c8d7efa5b8d7..f17878ce716c 100644 --- a/common/command_line.cpp +++ b/common/command_line.cpp @@ -55,6 +55,21 @@ auto operator<<(llvm::raw_ostream& output, CommandKind kind) } CARBON_FATAL("Corrupt command kind!"); } + +template +static auto PrintListOfAlternatives(llvm::raw_ostream& output, + llvm::ArrayRef alternatives, + ToPrintable to_printable) -> void { + for (const auto& alternative : alternatives.drop_back()) { + output << "`" << to_printable(alternative) + << (alternatives.size() > 2 ? "`, " : "` "); + } + if (alternatives.size() > 1) { + output << "or "; + } + output << "`" << to_printable(alternatives.back()) << "`"; +} + Arg::Arg(ArgInfo info) : info(info) {} Arg::~Arg() { @@ -328,14 +343,10 @@ void MetaPrinter::PrintVersion(const Command& command) const { } void MetaPrinter::PrintSubcommands(const Command& command) const { - for (const auto& subcommand : - llvm::ArrayRef(command.subcommands).drop_back()) { - *out_ << "`" << subcommand->info.name << "`, "; - } - if (command.subcommands.size() > 1) { - *out_ << "or "; - } - *out_ << "`" << command.subcommands.back()->info.name << "`"; + PrintListOfAlternatives(*out_, llvm::ArrayRef(command.subcommands), + [](const std::unique_ptr& subcommand) { + return subcommand->info.name; + }); } void MetaPrinter::PrintRawVersion(const Command& command, @@ -903,13 +914,8 @@ auto Parser::ParseOneOfArgValue(const Arg& arg, llvm::StringRef value) error << "` has an invalid value `"; llvm::printEscapedString(value, error); error << "`; valid values are: "; - for (auto value_string : arg.value_strings.drop_back()) { - error << "`" << value_string << "`, "; - } - if (arg.value_strings.size() > 1) { - error << "or "; - } - error << "`" << arg.value_strings.back() << "`"; + PrintListOfAlternatives(error, arg.value_strings, + [](llvm::StringRef x) { return x; }); return Error(error_str); } return Success(); diff --git a/common/command_line_test.cpp b/common/command_line_test.cpp index 65259472a01f..2a45cb6817a5 100644 --- a/common/command_line_test.cpp +++ b/common/command_line_test.cpp @@ -361,7 +361,7 @@ TEST(ArgParserTest, BasicSubcommands) { EXPECT_THAT(parse({"sub2", "--option=abc", "subsub42", "--option=xyz"}, os), IsError(StrEq("invalid subcommand `subsub42`; available " - "subcommands: `subsub`, or `help`"))); + "subcommands: `subsub` or `help`"))); EXPECT_THAT( parse({"sub2", "--option=abc", "subsub", "--option=xyz"}, llvm::errs()), @@ -434,20 +434,27 @@ TEST(ArgParserTest, PositionalAppending) { EXPECT_THAT(strings3, ElementsAre(StrEq("e"), StrEq("f"))); } +static auto ParseOneOfOption(llvm::ArrayRef args, + llvm::raw_ostream& s, + llvm::function_ref build) + -> ErrorOr { + return Parse(args, s, TestCommandInfo, [&](auto& b) { + b.AddOneOfOption({.name = "option"}, build); + b.Do([] {}); + }); +} + TEST(ArgParserTest, OneOfOption) { int value = 0; auto parse = [&](llvm::ArrayRef args, llvm::raw_ostream& s) { - return Parse(args, s, TestCommandInfo, [&](auto& b) { - b.AddOneOfOption({.name = "option"}, [&](auto& arg_b) { - arg_b.SetOneOf( - { - arg_b.OneOfValue("x", 1), - arg_b.OneOfValue("y", 2), - arg_b.OneOfValue("z", 3), - }, - &value); - }); - b.Do([] {}); + return ParseOneOfOption(args, s, [&](auto& arg_b) { + arg_b.SetOneOf( + { + arg_b.OneOfValue("x", 1), + arg_b.OneOfValue("y", 2), + arg_b.OneOfValue("z", 3), + }, + &value); }); }; @@ -489,20 +496,48 @@ TEST(ArgParserTest, OneOfOption) { IsError(StrEq(llvm::formatv(ErrorStr, "\\00")))); } +TEST(ArgParserTest, OneOfOptionWithTwoOptions) { + int value = 0; + TestRawOstream os; + EXPECT_THAT(ParseOneOfOption({"--option=z"}, os, + [&](auto& arg_b) { + arg_b.SetOneOf( + { + arg_b.OneOfValue("x", 1), + arg_b.OneOfValue("y", 2), + }, + &value); + }), + IsError(StrEq("option `--option=z` has an invalid value `z`; " + "valid values are: `x` or `y`"))); +} + +TEST(ArgParserTest, OneOfOptionWithOneOption) { + int value = 0; + TestRawOstream os; + EXPECT_THAT(ParseOneOfOption({"--option=z"}, os, + [&](auto& arg_b) { + arg_b.SetOneOf( + { + arg_b.OneOfValue("x", 1), + }, + &value); + }), + IsError(StrEq("option `--option=z` has an invalid value `z`; " + "valid values are: `x`"))); +} + TEST(ArgParserTest, OneOfOptionAppending) { llvm::SmallVector values; auto parse = [&](llvm::ArrayRef args, llvm::raw_ostream& s) { - return Parse(args, s, TestCommandInfo, [&](auto& b) { - b.AddOneOfOption({.name = "option"}, [&](auto& arg_b) { - arg_b.AppendOneOf( - { - arg_b.OneOfValue("x", 1), - arg_b.OneOfValue("y", 2), - arg_b.OneOfValue("z", 3), - }, - &values); - }); - b.Do([] {}); + return ParseOneOfOption(args, s, [&](auto& arg_b) { + arg_b.AppendOneOf( + { + arg_b.OneOfValue("x", 1), + arg_b.OneOfValue("y", 2), + arg_b.OneOfValue("z", 3), + }, + &values); }); };