mirror of
https://github.com/RPCS3/llvm-mirror.git
synced 2024-11-29 06:10:48 +00:00
[CommandLine] Don't print empty sentinel values from EnumValN lists in help text
In order to make an option value truly optional, both the ValueOptional attribute and an empty-named value are required. Prior to this change, this empty-named value appears in the command-line help text: -some-option - some help text =v1 - description 1 =v2 - description 2 = - This change improves the help text for these sort of options in a number of ways: 1) ValueOptional options with an empty-named value now print their help text twice: both without and then with '=<value>' after the name. The latter version then lists the allowed values after it. 2) Empty-named values with no help text in ValueOptional options are not listed in the permitted values. -some-option - some help text -some-option=<value> - some help text =v1 - description 1 =v2 - description 2 3) Otherwise empty-named options are printed as =<empty> rather than simply '='. 4) Option values without help text do not have the '-' separator printed. -some-option=<value> - some help text =v1 - description 1 =v2 =<empty> - description It also tweaks the llvm-symbolizer -functions help text to not print a trailing ':' as that looks bad combined with 1) above. This is mostly a reland of r353048 which in turn was a reland of r352750. Reviewed by: ruiu, thopre, mstorsjo Differential Revision: https://reviews.llvm.org/D57030 llvm-svn: 353053
This commit is contained in:
James Henderson
parent
08d6482cba
commit
5632da10d6
@ -1469,17 +1469,23 @@ 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() + 6; }
|
||||
size_t alias::getOptionWidth() const { return ArgStr.size() + ArgPrefixesSize; }
|
||||
|
||||
void Option::printHelpStr(StringRef HelpStr, size_t Indent,
|
||||
size_t FirstLineIndentedBy) {
|
||||
size_t FirstLineIndentedBy) {
|
||||
assert(Indent >= FirstLineIndentedBy);
|
||||
std::pair<StringRef, StringRef> Split = HelpStr.split('\n');
|
||||
outs().indent(Indent - FirstLineIndentedBy) << " - " << Split.first << "\n";
|
||||
outs().indent(Indent - FirstLineIndentedBy)
|
||||
<< ArgHelpPrefix << Split.first << "\n";
|
||||
while (!Split.second.empty()) {
|
||||
Split = Split.second.split('\n');
|
||||
outs().indent(Indent) << Split.first << "\n";
|
||||
@ -1488,8 +1494,8 @@ void Option::printHelpStr(StringRef HelpStr, size_t Indent,
|
||||
|
||||
// Print out the option for the alias.
|
||||
void alias::printOptionInfo(size_t GlobalWidth) const {
|
||||
outs() << " -" << ArgStr;
|
||||
printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + 6);
|
||||
outs() << ArgPrefix << ArgStr;
|
||||
printHelpStr(HelpStr, GlobalWidth, ArgStr.size() + ArgPrefixesSize);
|
||||
}
|
||||
|
||||
//===----------------------------------------------------------------------===//
|
||||
@ -1510,7 +1516,7 @@ size_t basic_parser_impl::getOptionWidth(const Option &O) const {
|
||||
Len += getValueStr(O, ValName).size() + FormattingLen;
|
||||
}
|
||||
|
||||
return Len + 6;
|
||||
return Len + ArgPrefixesSize;
|
||||
}
|
||||
|
||||
// printOptionInfo - Print out information about this option. The
|
||||
@ -1518,7 +1524,7 @@ size_t basic_parser_impl::getOptionWidth(const Option &O) const {
|
||||
//
|
||||
void basic_parser_impl::printOptionInfo(const Option &O,
|
||||
size_t GlobalWidth) const {
|
||||
outs() << " -" << O.ArgStr;
|
||||
outs() << ArgPrefix << O.ArgStr;
|
||||
|
||||
auto ValName = getValueName();
|
||||
if (!ValName.empty()) {
|
||||
@ -1534,7 +1540,7 @@ void basic_parser_impl::printOptionInfo(const Option &O,
|
||||
|
||||
void basic_parser_impl::printOptionName(const Option &O,
|
||||
size_t GlobalWidth) const {
|
||||
outs() << " -" << O.ArgStr;
|
||||
outs() << ArgPrefix << O.ArgStr;
|
||||
outs().indent(GlobalWidth - O.ArgStr.size());
|
||||
}
|
||||
|
||||
@ -1642,12 +1648,28 @@ unsigned generic_parser_base::findOption(StringRef Name) {
|
||||
return e;
|
||||
}
|
||||
|
||||
static StringRef EqValue = "=<value>";
|
||||
static StringRef EmptyOption = "<empty>";
|
||||
static StringRef OptionPrefix = " =";
|
||||
static size_t OptionPrefixesSize = OptionPrefix.size() + ArgHelpPrefix.size();
|
||||
|
||||
static bool shouldPrintOption(StringRef Name, StringRef Description,
|
||||
const Option &O) {
|
||||
return O.getValueExpectedFlag() != ValueOptional || !Name.empty() ||
|
||||
!Description.empty();
|
||||
}
|
||||
|
||||
// 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() + 6;
|
||||
for (unsigned i = 0, e = getNumOptions(); i != e; ++i)
|
||||
Size = std::max(Size, getOption(i).size() + 8);
|
||||
size_t Size = O.ArgStr.size() + ArgPrefixesSize + EqValue.size();
|
||||
for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
|
||||
StringRef Name = getOption(i);
|
||||
if (!shouldPrintOption(Name, getDescription(i), O))
|
||||
continue;
|
||||
size_t NameSize = Name.empty() ? EmptyOption.size() : Name.size();
|
||||
Size = std::max(Size, NameSize + OptionPrefixesSize);
|
||||
}
|
||||
return Size;
|
||||
} else {
|
||||
size_t BaseSize = 0;
|
||||
@ -1663,13 +1685,38 @@ size_t generic_parser_base::getOptionWidth(const Option &O) const {
|
||||
void generic_parser_base::printOptionInfo(const Option &O,
|
||||
size_t GlobalWidth) const {
|
||||
if (O.hasArgStr()) {
|
||||
outs() << " -" << O.ArgStr;
|
||||
Option::printHelpStr(O.HelpStr, GlobalWidth, O.ArgStr.size() + 6);
|
||||
// When the value is optional, first print a line just describing the
|
||||
// option without values.
|
||||
if (O.getValueExpectedFlag() == ValueOptional) {
|
||||
for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
|
||||
if (getOption(i).empty()) {
|
||||
outs() << ArgPrefix << O.ArgStr;
|
||||
Option::printHelpStr(O.HelpStr, GlobalWidth,
|
||||
O.ArgStr.size() + ArgPrefixesSize);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
outs() << ArgPrefix << O.ArgStr << EqValue;
|
||||
Option::printHelpStr(O.HelpStr, GlobalWidth,
|
||||
O.ArgStr.size() + EqValue.size() + ArgPrefixesSize);
|
||||
for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
|
||||
size_t NumSpaces = GlobalWidth - getOption(i).size() - 8;
|
||||
outs() << " =" << getOption(i);
|
||||
outs().indent(NumSpaces) << " - " << getDescription(i) << '\n';
|
||||
StringRef OptionName = getOption(i);
|
||||
StringRef Description = getDescription(i);
|
||||
if (!shouldPrintOption(OptionName, Description, O))
|
||||
continue;
|
||||
assert(GlobalWidth >= OptionName.size() + OptionPrefixesSize);
|
||||
size_t NumSpaces = GlobalWidth - OptionName.size() - OptionPrefixesSize;
|
||||
outs() << OptionPrefix << OptionName;
|
||||
if (OptionName.empty()) {
|
||||
outs() << EmptyOption;
|
||||
assert(NumSpaces >= EmptyOption.size());
|
||||
NumSpaces -= EmptyOption.size();
|
||||
}
|
||||
if (!Description.empty())
|
||||
outs().indent(NumSpaces) << ArgHelpPrefix << " " << Description;
|
||||
outs() << '\n';
|
||||
}
|
||||
} else {
|
||||
if (!O.HelpStr.empty())
|
||||
|
||||
@ -38,7 +38,7 @@ ClUseSymbolTable("use-symbol-table", cl::init(true),
|
||||
|
||||
static cl::opt<FunctionNameKind> ClPrintFunctions(
|
||||
"functions", cl::init(FunctionNameKind::LinkageName),
|
||||
cl::desc("Print function name for a given address:"), cl::ValueOptional,
|
||||
cl::desc("Print function name for a given address"), cl::ValueOptional,
|
||||
cl::values(clEnumValN(FunctionNameKind::None, "none", "omit function name"),
|
||||
clEnumValN(FunctionNameKind::ShortName, "short",
|
||||
"print short function name"),
|
||||
|
||||
@ -13,6 +13,7 @@
|
||||
#include "llvm/Config/config.h"
|
||||
#include "llvm/Support/FileSystem.h"
|
||||
#include "llvm/Support/InitLLVM.h"
|
||||
#include "llvm/Support/MemoryBuffer.h"
|
||||
#include "llvm/Support/Path.h"
|
||||
#include "llvm/Support/Program.h"
|
||||
#include "llvm/Support/StringSaver.h"
|
||||
@ -839,4 +840,218 @@ TEST(CommandLineTest, GetCommandLineArguments) {
|
||||
}
|
||||
#endif
|
||||
|
||||
class OutputRedirector {
|
||||
public:
|
||||
OutputRedirector(int RedirectFD)
|
||||
: RedirectFD(RedirectFD), OldFD(dup(RedirectFD)) {
|
||||
if (OldFD == -1 ||
|
||||
sys::fs::createTemporaryFile("unittest-redirect", "", NewFD,
|
||||
FilePath) ||
|
||||
dup2(NewFD, RedirectFD) == -1)
|
||||
Valid = false;
|
||||
}
|
||||
|
||||
~OutputRedirector() {
|
||||
dup2(OldFD, RedirectFD);
|
||||
close(OldFD);
|
||||
close(NewFD);
|
||||
}
|
||||
|
||||
SmallVector<char, 128> FilePath;
|
||||
bool Valid = true;
|
||||
|
||||
private:
|
||||
int RedirectFD;
|
||||
int OldFD;
|
||||
int NewFD;
|
||||
};
|
||||
|
||||
struct AutoDeleteFile {
|
||||
SmallVector<char, 128> FilePath;
|
||||
~AutoDeleteFile() {
|
||||
if (!FilePath.empty())
|
||||
sys::fs::remove(std::string(FilePath.data(), FilePath.size()));
|
||||
}
|
||||
};
|
||||
|
||||
class PrintOptionInfoTest : public ::testing::Test {
|
||||
public:
|
||||
// Return std::string because the output of a failing EXPECT check is
|
||||
// unreadable for StringRef. It also avoids any lifetime issues.
|
||||
template <typename... Ts> std::string runTest(Ts... OptionAttributes) {
|
||||
AutoDeleteFile File;
|
||||
{
|
||||
OutputRedirector Stdout(fileno(stdout));
|
||||
if (!Stdout.Valid)
|
||||
return "";
|
||||
File.FilePath = Stdout.FilePath;
|
||||
|
||||
StackOption<OptionValue> TestOption(Opt, cl::desc(HelpText),
|
||||
OptionAttributes...);
|
||||
printOptionInfo(TestOption, 25);
|
||||
outs().flush();
|
||||
}
|
||||
auto Buffer = MemoryBuffer::getFile(File.FilePath);
|
||||
if (!Buffer)
|
||||
return "";
|
||||
return Buffer->get()->getBuffer().str();
|
||||
}
|
||||
|
||||
enum class OptionValue { Val };
|
||||
const StringRef Opt = "some-option";
|
||||
const StringRef HelpText = "some help";
|
||||
|
||||
private:
|
||||
// This is a workaround for cl::Option sub-classes having their
|
||||
// printOptionInfo functions private.
|
||||
void printOptionInfo(const cl::Option &O, size_t Width) {
|
||||
O.printOptionInfo(Width);
|
||||
}
|
||||
};
|
||||
|
||||
TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithoutSentinel) {
|
||||
std::string Output =
|
||||
runTest(cl::ValueOptional,
|
||||
cl::values(clEnumValN(OptionValue::Val, "v1", "desc1")));
|
||||
|
||||
// clang-format off
|
||||
EXPECT_EQ(Output, (" -" + Opt + "=<value> - " + HelpText + "\n"
|
||||
" =v1 - desc1\n")
|
||||
.str());
|
||||
// clang-format on
|
||||
}
|
||||
|
||||
TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithSentinel) {
|
||||
std::string Output = runTest(
|
||||
cl::ValueOptional, cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"),
|
||||
clEnumValN(OptionValue::Val, "", "")));
|
||||
|
||||
// clang-format off
|
||||
EXPECT_EQ(Output,
|
||||
(" -" + Opt + " - " + HelpText + "\n"
|
||||
" -" + Opt + "=<value> - " + HelpText + "\n"
|
||||
" =v1 - desc1\n")
|
||||
.str());
|
||||
// clang-format on
|
||||
}
|
||||
|
||||
TEST_F(PrintOptionInfoTest, PrintOptionInfoValueOptionalWithSentinelWithHelp) {
|
||||
std::string Output = runTest(
|
||||
cl::ValueOptional, cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"),
|
||||
clEnumValN(OptionValue::Val, "", "desc2")));
|
||||
|
||||
// clang-format off
|
||||
EXPECT_EQ(Output, (" -" + Opt + " - " + HelpText + "\n"
|
||||
" -" + Opt + "=<value> - " + HelpText + "\n"
|
||||
" =v1 - desc1\n"
|
||||
" =<empty> - desc2\n")
|
||||
.str());
|
||||
// clang-format on
|
||||
}
|
||||
|
||||
TEST_F(PrintOptionInfoTest, PrintOptionInfoValueRequiredWithEmptyValueName) {
|
||||
std::string Output = runTest(
|
||||
cl::ValueRequired, cl::values(clEnumValN(OptionValue::Val, "v1", "desc1"),
|
||||
clEnumValN(OptionValue::Val, "", "")));
|
||||
|
||||
// clang-format off
|
||||
EXPECT_EQ(Output, (" -" + Opt + "=<value> - " + HelpText + "\n"
|
||||
" =v1 - desc1\n"
|
||||
" =<empty>\n")
|
||||
.str());
|
||||
// clang-format on
|
||||
}
|
||||
|
||||
TEST_F(PrintOptionInfoTest, PrintOptionInfoEmptyValueDescription) {
|
||||
std::string Output = runTest(
|
||||
cl::ValueRequired, cl::values(clEnumValN(OptionValue::Val, "v1", "")));
|
||||
|
||||
// clang-format off
|
||||
EXPECT_EQ(Output,
|
||||
(" -" + Opt + "=<value> - " + HelpText + "\n"
|
||||
" =v1\n").str());
|
||||
// clang-format on
|
||||
}
|
||||
|
||||
class GetOptionWidthTest : public ::testing::Test {
|
||||
public:
|
||||
enum class OptionValue { Val };
|
||||
|
||||
template <typename... Ts>
|
||||
size_t runTest(StringRef ArgName, Ts... OptionAttributes) {
|
||||
StackOption<OptionValue> TestOption(ArgName, cl::desc("some help"),
|
||||
OptionAttributes...);
|
||||
return getOptionWidth(TestOption);
|
||||
}
|
||||
|
||||
private:
|
||||
// This is a workaround for cl::Option sub-classes having their
|
||||
// printOptionInfo
|
||||
// functions private.
|
||||
size_t getOptionWidth(const cl::Option &O) { return O.getOptionWidth(); }
|
||||
};
|
||||
|
||||
TEST_F(GetOptionWidthTest, GetOptionWidthArgNameLonger) {
|
||||
StringRef ArgName("a-long-argument-name");
|
||||
size_t ExpectedStrSize = (" -" + ArgName + "=<value> - ").str().size();
|
||||
EXPECT_EQ(
|
||||
runTest(ArgName, cl::values(clEnumValN(OptionValue::Val, "v", "help"))),
|
||||
ExpectedStrSize);
|
||||
}
|
||||
|
||||
TEST_F(GetOptionWidthTest, GetOptionWidthFirstOptionNameLonger) {
|
||||
StringRef OptName("a-long-option-name");
|
||||
size_t ExpectedStrSize = (" =" + OptName + " - ").str().size();
|
||||
EXPECT_EQ(
|
||||
runTest("a", cl::values(clEnumValN(OptionValue::Val, OptName, "help"),
|
||||
clEnumValN(OptionValue::Val, "b", "help"))),
|
||||
ExpectedStrSize);
|
||||
}
|
||||
|
||||
TEST_F(GetOptionWidthTest, GetOptionWidthSecondOptionNameLonger) {
|
||||
StringRef OptName("a-long-option-name");
|
||||
size_t ExpectedStrSize = (" =" + OptName + " - ").str().size();
|
||||
EXPECT_EQ(
|
||||
runTest("a", cl::values(clEnumValN(OptionValue::Val, "b", "help"),
|
||||
clEnumValN(OptionValue::Val, OptName, "help"))),
|
||||
ExpectedStrSize);
|
||||
}
|
||||
|
||||
TEST_F(GetOptionWidthTest, GetOptionWidthEmptyOptionNameLonger) {
|
||||
size_t ExpectedStrSize = StringRef(" =<empty> - ").size();
|
||||
// The length of a=<value> (including indentation) is actually the same as the
|
||||
// =<empty> string, so it is impossible to distinguish via testing the case
|
||||
// where the empty string is picked from where the option name is picked.
|
||||
EXPECT_EQ(runTest("a", cl::values(clEnumValN(OptionValue::Val, "b", "help"),
|
||||
clEnumValN(OptionValue::Val, "", "help"))),
|
||||
ExpectedStrSize);
|
||||
}
|
||||
|
||||
TEST_F(GetOptionWidthTest,
|
||||
GetOptionWidthValueOptionalEmptyOptionWithNoDescription) {
|
||||
StringRef ArgName("a");
|
||||
// The length of a=<value> (including indentation) is actually the same as the
|
||||
// =<empty> string, so it is impossible to distinguish via testing the case
|
||||
// where the empty string is ignored from where it is not ignored.
|
||||
// The dash will not actually be printed, but the space it would take up is
|
||||
// included to ensure a consistent column width.
|
||||
size_t ExpectedStrSize = (" -" + ArgName + "=<value> - ").str().size();
|
||||
EXPECT_EQ(runTest(ArgName, cl::ValueOptional,
|
||||
cl::values(clEnumValN(OptionValue::Val, "value", "help"),
|
||||
clEnumValN(OptionValue::Val, "", ""))),
|
||||
ExpectedStrSize);
|
||||
}
|
||||
|
||||
TEST_F(GetOptionWidthTest,
|
||||
GetOptionWidthValueRequiredEmptyOptionWithNoDescription) {
|
||||
// The length of a=<value> (including indentation) is actually the same as the
|
||||
// =<empty> string, so it is impossible to distinguish via testing the case
|
||||
// where the empty string is picked from where the option name is picked
|
||||
size_t ExpectedStrSize = StringRef(" =<empty> - ").size();
|
||||
EXPECT_EQ(runTest("a", cl::ValueRequired,
|
||||
cl::values(clEnumValN(OptionValue::Val, "value", "help"),
|
||||
clEnumValN(OptionValue::Val, "", ""))),
|
||||
ExpectedStrSize);
|
||||
}
|
||||
|
||||
} // anonymous namespace
|
||||
|
||||
Loading…
Reference in New Issue
Block a user