From c242be40a140df8959ce08bc6d593d9ee14ca319 Mon Sep 17 00:00:00 2001 From: Don Hinton Date: Fri, 3 May 2019 17:47:29 +0000 Subject: [PATCH] [CommandLine] Change help output to prefix long options with `--` instead of `-`. NFC . Part 3 of 5 Summary: By default, `parseCommandLineOptions()` will accept either a `-` or `--` prefix for long options -- options with names longer than a single character. While this change does not affect behavior, it will be helpful with a subsequent change that requires long options use the `--` prefix. Reviewers: rnk, thopre Reviewed By: thopre Subscribers: thopre, cfe-commits, hiraditya, llvm-commits Tags: #llvm, #clang Differential Revision: https://reviews.llvm.org/D61269 llvm-svn: 359909 --- clang/test/Driver/clang-offload-bundler.c | 8 +-- llvm/lib/Support/CommandLine.cpp | 97 +++++++++++++++++++---------- llvm/test/FileCheck/dump-input-enable.txt | 2 +- llvm/test/Support/check-default-options.txt | 6 +- llvm/unittests/Support/CommandLineTest.cpp | 28 ++++----- 5 files changed, 85 insertions(+), 56 deletions(-) diff --git a/clang/test/Driver/clang-offload-bundler.c b/clang/test/Driver/clang-offload-bundler.c index 15092dd..5411a30 100644 --- a/clang/test/Driver/clang-offload-bundler.c +++ b/clang/test/Driver/clang-offload-bundler.c @@ -74,10 +74,10 @@ // CK-ERR6: error: invalid file type specified. // RUN: not clang-offload-bundler 2>&1 | FileCheck %s --check-prefix CK-ERR7 -// CK-ERR7-DAG: clang-offload-bundler: for the -type option: must be specified at least once! -// CK-ERR7-DAG: clang-offload-bundler: for the -inputs option: must be specified at least once! -// CK-ERR7-DAG: clang-offload-bundler: for the -outputs option: must be specified at least once! -// CK-ERR7-DAG: clang-offload-bundler: for the -targets option: must be specified at least once! +// CK-ERR7-DAG: clang-offload-bundler: for the --type option: must be specified at least once! +// CK-ERR7-DAG: clang-offload-bundler: for the --inputs option: must be specified at least once! +// CK-ERR7-DAG: clang-offload-bundler: for the --outputs option: must be specified at least once! +// CK-ERR7-DAG: clang-offload-bundler: for the --targets option: must be specified at least once! // RUN: not clang-offload-bundler -type=i -targets=hxst-powerpcxxle-ibm-linux-gnu,openxp-pxxerpc64le-ibm-linux-gnu,xpenmp-x86_xx-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR8 // CK-ERR8: error: invalid target 'hxst-powerpcxxle-ibm-linux-gnu', unknown offloading kind 'hxst', unknown target triple 'powerpcxxle-ibm-linux-gnu'. diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index 18492e2..1ca31b6 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -88,8 +88,37 @@ void parser::anchor() {} //===----------------------------------------------------------------------===// +static StringRef ArgPrefix = " -"; +static StringRef ArgPrefixLong = " --"; +static StringRef ArgHelpPrefix = " - "; + +static size_t argPlusPrefixesSize(StringRef ArgName) { + size_t Len = ArgName.size(); + if (Len == 1) + return Len + ArgPrefix.size() + ArgHelpPrefix.size(); + return Len + ArgPrefixLong.size() + ArgHelpPrefix.size(); +} + +static StringRef argPrefix(StringRef ArgName) { + if (ArgName.size() == 1) + return ArgPrefix; + return ArgPrefixLong; +} + namespace { +class PrintArg { + StringRef ArgName; +public: + PrintArg(StringRef ArgName) : ArgName(ArgName) {} + friend raw_ostream &operator<<(raw_ostream &OS, const PrintArg&); +}; + +raw_ostream &operator<<(raw_ostream &OS, const PrintArg& Arg) { + OS << argPrefix(Arg.ArgName) << Arg.ArgName; + return OS; +} + class CommandLineParser { public: // Globals for name and overview of program. Program name is not a string to @@ -1339,12 +1368,12 @@ bool CommandLineParser::ParseCommandLineOptions(int argc, if (!Handler) { if (SinkOpts.empty()) { *Errs << ProgramName << ": Unknown command line argument '" << argv[i] - << "'. Try: '" << argv[0] << " -help'\n"; + << "'. Try: '" << argv[0] << " --help'\n"; if (NearestHandler) { // If we know a near match, report it as well. - *Errs << ProgramName << ": Did you mean '-" << NearestHandlerString - << "'?\n"; + *Errs << ProgramName << ": Did you mean '" + << PrintArg(NearestHandlerString) << "'?\n"; } ErrorParsing = true; @@ -1378,14 +1407,14 @@ bool CommandLineParser::ParseCommandLineOptions(int argc, << ": Not enough positional command line arguments specified!\n" << "Must specify at least " << NumPositionalRequired << " positional argument" << (NumPositionalRequired > 1 ? "s" : "") - << ": See: " << argv[0] << " -help\n"; + << ": See: " << argv[0] << " --help\n"; ErrorParsing = true; } else if (!HasUnlimitedPositionals && PositionalVals.size() > PositionalOpts.size()) { *Errs << ProgramName << ": Too many positional arguments specified!\n" << "Can specify at most " << PositionalOpts.size() - << " positional arguments: See: " << argv[0] << " -help\n"; + << " positional arguments: See: " << argv[0] << " --help\n"; ErrorParsing = true; } else if (!ConsumeAfterOpt) { @@ -1498,7 +1527,7 @@ bool Option::error(const Twine &Message, StringRef ArgName, raw_ostream &Errs) { if (ArgName.empty()) Errs << HelpStr; // Be nice for positional arguments else - Errs << GlobalParser->ProgramName << ": for the -" << ArgName; + Errs << GlobalParser->ProgramName << ": for the " << PrintArg(ArgName); Errs << " option: " << Message << "\n"; return true; @@ -1536,16 +1565,14 @@ static StringRef getValueStr(const Option &O, StringRef DefaultMsg) { return O.ValueStr; } -static StringRef ArgPrefix = " -"; -static StringRef ArgHelpPrefix = " - "; -static size_t ArgPrefixesSize = ArgPrefix.size() + ArgHelpPrefix.size(); - //===----------------------------------------------------------------------===// // cl::alias class implementation // // Return the width of the option tag for printing... -size_t alias::getOptionWidth() const { return ArgStr.size() + ArgPrefixesSize; } +size_t alias::getOptionWidth() const { + return argPlusPrefixesSize(ArgStr); +} void Option::printHelpStr(StringRef HelpStr, size_t Indent, size_t FirstLineIndentedBy) { @@ -1561,8 +1588,8 @@ void Option::printHelpStr(StringRef HelpStr, size_t Indent, // Print out the option for the alias. void alias::printOptionInfo(size_t GlobalWidth) const { - outs() << ArgPrefix << ArgStr; - printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + ArgPrefixesSize); + outs() << PrintArg(ArgStr); + printHelpStr(HelpStr, GlobalWidth, argPlusPrefixesSize(ArgStr)); } //===----------------------------------------------------------------------===// @@ -1574,7 +1601,7 @@ void alias::printOptionInfo(size_t GlobalWidth) const { // Return the width of the option tag for printing... size_t basic_parser_impl::getOptionWidth(const Option &O) const { - size_t Len = O.ArgStr.size(); + size_t Len = argPlusPrefixesSize(O.ArgStr); auto ValName = getValueName(); if (!ValName.empty()) { size_t FormattingLen = 3; @@ -1583,7 +1610,7 @@ size_t basic_parser_impl::getOptionWidth(const Option &O) const { Len += getValueStr(O, ValName).size() + FormattingLen; } - return Len + ArgPrefixesSize; + return Len; } // printOptionInfo - Print out information about this option. The @@ -1591,7 +1618,7 @@ size_t basic_parser_impl::getOptionWidth(const Option &O) const { // void basic_parser_impl::printOptionInfo(const Option &O, size_t GlobalWidth) const { - outs() << ArgPrefix << O.ArgStr; + outs() << PrintArg(O.ArgStr); auto ValName = getValueName(); if (!ValName.empty()) { @@ -1607,7 +1634,7 @@ void basic_parser_impl::printOptionInfo(const Option &O, void basic_parser_impl::printOptionName(const Option &O, size_t GlobalWidth) const { - outs() << ArgPrefix << O.ArgStr; + outs() << PrintArg(O.ArgStr); outs().indent(GlobalWidth - O.ArgStr.size()); } @@ -1739,7 +1766,8 @@ static bool shouldPrintOption(StringRef Name, StringRef Description, // Return the width of the option tag for printing... size_t generic_parser_base::getOptionWidth(const Option &O) const { if (O.hasArgStr()) { - size_t Size = O.ArgStr.size() + ArgPrefixesSize + EqValue.size(); + size_t Size = + argPlusPrefixesSize(O.ArgStr) + EqValue.size(); for (unsigned i = 0, e = getNumOptions(); i != e; ++i) { StringRef Name = getOption(i); if (!shouldPrintOption(Name, getDescription(i), O)) @@ -1767,17 +1795,18 @@ void generic_parser_base::printOptionInfo(const Option &O, if (O.getValueExpectedFlag() == ValueOptional) { for (unsigned i = 0, e = getNumOptions(); i != e; ++i) { if (getOption(i).empty()) { - outs() << ArgPrefix << O.ArgStr; + outs() << PrintArg(O.ArgStr); Option::printHelpStr(O.HelpStr, GlobalWidth, - O.ArgStr.size() + ArgPrefixesSize); + argPlusPrefixesSize(O.ArgStr)); break; } } } - outs() << ArgPrefix << O.ArgStr << EqValue; + outs() << PrintArg(O.ArgStr) << EqValue; Option::printHelpStr(O.HelpStr, GlobalWidth, - O.ArgStr.size() + EqValue.size() + ArgPrefixesSize); + EqValue.size() + + argPlusPrefixesSize(O.ArgStr)); for (unsigned i = 0, e = getNumOptions(); i != e; ++i) { StringRef OptionName = getOption(i); StringRef Description = getDescription(i); @@ -1799,8 +1828,8 @@ void generic_parser_base::printOptionInfo(const Option &O, if (!O.HelpStr.empty()) outs() << " " << O.HelpStr << '\n'; for (unsigned i = 0, e = getNumOptions(); i != e; ++i) { - auto Option = getOption(i); - outs() << " -" << Option; + StringRef Option = getOption(i); + outs() << " " << PrintArg(Option); Option::printHelpStr(getDescription(i), GlobalWidth, Option.size() + 8); } } @@ -1814,7 +1843,7 @@ static const size_t MaxOptWidth = 8; // arbitrary spacing for printOptionDiff void generic_parser_base::printGenericOptionDiff( const Option &O, const GenericOptionValue &Value, const GenericOptionValue &Default, size_t GlobalWidth) const { - outs() << " -" << O.ArgStr; + outs() << " " << PrintArg(O.ArgStr); outs().indent(GlobalWidth - O.ArgStr.size()); unsigned NumOpts = getNumOptions(); @@ -2034,7 +2063,7 @@ public: printSubCommands(Subs, MaxSubLen); outs() << "\n"; outs() << " Type \"" << GlobalParser->ProgramName - << " -help\" to get more help on a specific " + << " --help\" to get more help on a specific " "subcommand"; } @@ -2111,7 +2140,7 @@ protected: Category = SortedCategories.begin(), E = SortedCategories.end(); Category != E; ++Category) { - // Hide empty categories for -help, but show for -help-hidden. + // Hide empty categories for --help, but show for --help-hidden. const auto &CategoryOptions = CategorizedOptions[*Category]; bool IsEmptyCategory = CategoryOptions.empty(); if (!ShowHidden && IsEmptyCategory) @@ -2127,7 +2156,7 @@ protected: else outs() << "\n"; - // When using -help-hidden explicitly state if the category has no + // When using --help-hidden explicitly state if the category has no // options associated with it. if (IsEmptyCategory) { outs() << " This option category has no options.\n"; @@ -2177,11 +2206,11 @@ static HelpPrinterWrapper WrappedHiddenPrinter(UncategorizedHiddenPrinter, static cl::OptionCategory GenericCategory("Generic Options"); // Define uncategorized help printers. -// -help-list is hidden by default because if Option categories are being used -// then -help behaves the same as -help-list. +// --help-list is hidden by default because if Option categories are being used +// then --help behaves the same as --help-list. static cl::opt> HLOp( "help-list", - cl::desc("Display list of available options (-help-list-hidden for more)"), + cl::desc("Display list of available options (--help-list-hidden for more)"), cl::location(UncategorizedNormalPrinter), cl::Hidden, cl::ValueDisallowed, cl::cat(GenericCategory), cl::sub(*AllSubCommands)); @@ -2195,11 +2224,11 @@ static cl::opt> // behaviour at runtime depending on whether one or more Option categories have // been declared. static cl::opt> - HOp("help", cl::desc("Display available options (-help-hidden for more)"), + HOp("help", cl::desc("Display available options (--help-hidden for more)"), cl::location(WrappedNormalPrinter), cl::ValueDisallowed, cl::cat(GenericCategory), cl::sub(*AllSubCommands)); -static cl::alias HOpA("h", cl::desc("Alias for -help"), cl::aliasopt(HOp), +static cl::alias HOpA("h", cl::desc("Alias for --help"), cl::aliasopt(HOp), cl::DefaultOption); static cl::opt> @@ -2226,7 +2255,7 @@ void HelpPrinterWrapper::operator=(bool Value) { // registered then it is useful to show the categorized help instead of // uncategorized help. if (GlobalParser->RegisteredOptionCategories.size() > 1) { - // unhide -help-list option so user can have uncategorized output if they + // unhide --help-list option so user can have uncategorized output if they // want it. HLOp.setHiddenFlag(NotHidden); diff --git a/llvm/test/FileCheck/dump-input-enable.txt b/llvm/test/FileCheck/dump-input-enable.txt index 45592dd..8c8a807 100644 --- a/llvm/test/FileCheck/dump-input-enable.txt +++ b/llvm/test/FileCheck/dump-input-enable.txt @@ -26,7 +26,7 @@ ; RUN: not FileCheck -dump-input=foobar 2>&1 \ ; RUN: | FileCheck %s -match-full-lines -check-prefix=BADVAL -BADVAL: {{F|f}}ile{{C|c}}heck{{.*}}: for the -dump-input option: Cannot find option named 'foobar'! +BADVAL: {{F|f}}ile{{C|c}}heck{{.*}}: for the --dump-input option: Cannot find option named 'foobar'! ;-------------------------------------------------- ; Check -dump-input=help. diff --git a/llvm/test/Support/check-default-options.txt b/llvm/test/Support/check-default-options.txt index a8870ba..6f2bff9 100644 --- a/llvm/test/Support/check-default-options.txt +++ b/llvm/test/Support/check-default-options.txt @@ -8,11 +8,11 @@ # CHECK-OBJDUMP: -h - Alias for --section-headers # CHECK-READOBJ: -h - Alias for --file-headers -# CHECK-TBLGEN: -h - Alias for -help -# CHECK-OPT-RPT: -h - Alias for -help +# CHECK-TBLGEN: -h - Alias for --help +# CHECK-OPT-RPT: -h - Alias for --help # CHECK-DWARF: -h - Alias for -help # llvm-dwarfdump declares `-h` option and prints special help in that case, # which is weird, but makes for a good test, i.e., shows the default `-h` # wasn't used. -# CHECK-DWARF-H-NOT: -help-list - Display list of available options (-help-list-hidden for more) +# CHECK-DWARF-H-NOT: --help-list - Display list of available options (--help-list-hidden for more) diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp index 5ce6541..e528f51 100644 --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/llvm/unittests/Support/CommandLineTest.cpp @@ -1042,7 +1042,7 @@ public: StackOption TestOption(Opt, cl::desc(HelpText), OptionAttributes...); - printOptionInfo(TestOption, 25); + printOptionInfo(TestOption, 26); outs().flush(); } auto Buffer = MemoryBuffer::getFile(File.FilePath); @@ -1069,8 +1069,8 @@ TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithoutSentinel) { cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"))); // clang-format off - EXPECT_EQ(Output, (" -" + Opt + "= - " + HelpText + "\n" - " =v1 - desc1\n") + EXPECT_EQ(Output, (" --" + Opt + "= - " + HelpText + "\n" + " =v1 - desc1\n") .str()); // clang-format on } @@ -1082,9 +1082,9 @@ TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithSentinel) { // clang-format off EXPECT_EQ(Output, - (" -" + Opt + " - " + HelpText + "\n" - " -" + Opt + "= - " + HelpText + "\n" - " =v1 - desc1\n") + (" --" + Opt + " - " + HelpText + "\n" + " --" + Opt + "= - " + HelpText + "\n" + " =v1 - desc1\n") .str()); // clang-format on } @@ -1095,10 +1095,10 @@ TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithSentinelWithHelp) { clEnumValN(OptionValue::Val, "", "desc2"))); // clang-format off - EXPECT_EQ(Output, (" -" + Opt + " - " + HelpText + "\n" - " -" + Opt + "= - " + HelpText + "\n" - " =v1 - desc1\n" - " = - desc2\n") + EXPECT_EQ(Output, (" --" + Opt + " - " + HelpText + "\n" + " --" + Opt + "= - " + HelpText + "\n" + " =v1 - desc1\n" + " = - desc2\n") .str()); // clang-format on } @@ -1109,8 +1109,8 @@ TEST_F(PrintOptionInfoTest, PrintOptionInfoValueRequiredWithEmptyValueName) { clEnumValN(OptionValue::Val, "", ""))); // clang-format off - EXPECT_EQ(Output, (" -" + Opt + "= - " + HelpText + "\n" - " =v1 - desc1\n" + EXPECT_EQ(Output, (" --" + Opt + "= - " + HelpText + "\n" + " =v1 - desc1\n" " =\n") .str()); // clang-format on @@ -1122,7 +1122,7 @@ TEST_F(PrintOptionInfoTest, PrintOptionInfoEmptyValueDescription) { // clang-format off EXPECT_EQ(Output, - (" -" + Opt + "= - " + HelpText + "\n" + (" --" + Opt + "= - " + HelpText + "\n" " =v1\n").str()); // clang-format on } @@ -1147,7 +1147,7 @@ private: TEST_F(GetOptionWidthTest, GetOptionWidthArgNameLonger) { StringRef ArgName("a-long-argument-name"); - size_t ExpectedStrSize = (" -" + ArgName + "= - ").str().size(); + size_t ExpectedStrSize = (" --" + ArgName + "= - ").str().size(); EXPECT_EQ( runTest(ArgName, cl::values(clEnumValN(OptionValue::Val, "v", "help"))), ExpectedStrSize); -- 2.7.4