[Support] Indent multi-line descr of enum cli options.

[fodinabor]

Summary

As noted in https://reviews.llvm.org/D93459, the formatting of
multi-line descriptions of clEnumValN and the likes is unfavorable.
Thus this patch adds support for correctly indenting these.

[fodinabor]

Fix naming to make clang-tidy happy.

[fodinabor]

Happy new year's ping :)

[serge-sans-paille]

Would be good to have a test entry for that new pretty printer . That would also make the review easier ;-)

[fodinabor]

Add print option info test for multiline value descriptions.

[fodinabor]

Given the awful formatting in clang-format is merged in release/12.x, would be great to get this in, before 12.0.0 final.. :)

[fodinabor]

Ping :)

[serge-sans-paille]

LGTM, thanks for providing a test case!

[chapuni]

It might causes assertion failure when a name in enum is long.
Reproducible with clang-scan-deps -help

llvm/lib/Support/CommandLine.cpp

1731

This is not reflected to MaxWidth.

1948

Could we adjust it with ValHelpPrefix?

[fodinabor]

mhh... I'd just say the assert was wrong.
The BaseIndent is the maximum width of an option. e.g. --compilation-database=<string>.
printEnumValHelpStr does not deal with that part at all, only with the help string that comes after that (e.g. This is the first enum value\nwhich has a really long description\nthus it is multi-line.).
For nice formatting in the first line - and are prepended for the enum values in the first line and later BaseIndent + are used for the indent.
What the assert should have verified is only that BaseIndent >= FirstLineIndentedBy so that we can later have the indent: BaseIndent - FirstLineIndentedBy and be sure it's >=0.
(and that really should always be true..)

So I will just adapt the assert..

llvm/include/llvm/Support/CommandLine.h

public:
   // to-be-maintained width is specified.
   //
   virtual void printOptionInfo(size_t GlobalWidth) const = 0;
 
   virtual void printOptionValue(size_t GlobalWidth, bool Force) const = 0;
 
   virtual void setDefault() = 0;
 
+  // Prints the help string for an option.
+  //
+  // This maintains the Indent for multi-line descriptions.
+  // FirstLineIndentedBy is the count of chars of the first line
+  //      i.e. the one containing the --<option name>.
   static void printHelpStr(StringRef HelpStr, size_t Indent,
                            size_t FirstLineIndentedBy);
 
+  // Prints the help string for an enum value.
+  //
+  // This maintains the Indent for multi-line descriptions.
+  // FirstLineIndentedBy is the count of chars of the first line
+  //      i.e. the one containing the =<value>.
+  static void printEnumValHelpStr(StringRef HelpStr, size_t Indent,
+                                  size_t FirstLineIndentedBy);
+
   virtual void getExtraOptionNames(SmallVectorImpl<StringRef> &) {}
 
   // addOccurrence - Wrapper around handleOccurrence that enforces Flags.
   //
   virtual bool addOccurrence(unsigned pos, StringRef ArgName, StringRef Value,
                              bool MultiArg = false);
 
   // Prints option name followed by message.  Always returns true.

llvm/lib/Support/CommandLine.cpp

void Option::printHelpStr(StringRef HelpStr, size_t Indent,
   outs().indent(Indent - FirstLineIndentedBy)
       << ArgHelpPrefix << Split.first << "\n";
   while (!Split.second.empty()) {
     Split = Split.second.split('\n');
     outs().indent(Indent) << Split.first << "\n";
   }
 }
 
+void Option::printEnumValHelpStr(StringRef HelpStr, size_t BaseIndent,
+                                 size_t FirstLineIndentedBy) {
+  const StringRef ValHelpPrefix = "  ";
+  assert(BaseIndent >= FirstLineIndentedBy + ValHelpPrefix.size());
+  std::pair<StringRef, StringRef> Split = HelpStr.split('\n');
+  outs().indent(BaseIndent - FirstLineIndentedBy)
+      << ArgHelpPrefix << ValHelpPrefix << Split.first << "\n";
+  while (!Split.second.empty()) {
+    Split = Split.second.split('\n');
+    outs().indent(BaseIndent + ValHelpPrefix.size()) << Split.first << "\n";
+  }
+}
+
 // Print out the option for the alias.
 void alias::printOptionInfo(size_t GlobalWidth) const {
   outs() << PrintArg(ArgStr);
   printHelpStr(HelpStr, GlobalWidth, argPlusPrefixesSize(ArgStr));
 }
 
 //===----------------------------------------------------------------------===//
 // Parser Implementation code...
size_t generic_parser_base::getOptionWidth(const Option &O) const {
   if (O.hasArgStr()) {
     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))
         continue;
       size_t NameSize = Name.empty() ? EmptyOption.size() : Name.size();
       Size = std::max(Size, NameSize + OptionPrefixesSize);
     }
     return Size;
   } else {
     size_t BaseSize = 0;
     for (unsigned i = 0, e = getNumOptions(); i != e; ++i)
       BaseSize = std::max(BaseSize, getOption(i).size() + 8);
     return BaseSize;
   }
if (O.hasArgStr()) {
     Option::printHelpStr(O.HelpStr, GlobalWidth,
                          EqValue.size() +
                              argPlusPrefixesSize(O.ArgStr));
     for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
       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;
+      size_t FirstLineIndent = OptionName.size() + OptionPrefixesSize;
       outs() << OptionPrefix << OptionName;
       if (OptionName.empty()) {
         outs() << EmptyOption;
-        assert(NumSpaces >= EmptyOption.size());
-        NumSpaces -= EmptyOption.size();
+        assert(FirstLineIndent >= EmptyOption.size());
+        FirstLineIndent += EmptyOption.size();
       }
       if (!Description.empty())
-        outs().indent(NumSpaces) << ArgHelpPrefix << "  " << Description;
+        Option::printEnumValHelpStr(Description, GlobalWidth, FirstLineIndent);
+      else
         outs() << '\n';
     }
   } else {
     if (!O.HelpStr.empty())
       outs() << "  " << O.HelpStr << '\n';
     for (unsigned i = 0, e = getNumOptions(); i != e; ++i) {
       StringRef Option = getOption(i);
       outs() << "    " << PrintArg(Option);
       Option::printHelpStr(getDescription(i), GlobalWidth, Option.size() + 8);

llvm/unittests/Support/CommandLineTest.cpp

TEST_F(PrintOptionInfoTest, PrintOptionInfoEmptyValueDescription) {
 
   // clang-format off
   EXPECT_EQ(Output,
             ("  --" + Opt + "=<value> - " + HelpText + "\n"
              "    =v1\n").str());
   // clang-format on
 }
 
+TEST_F(PrintOptionInfoTest, PrintOptionInfoMultilineValueDescription) {
+  std::string Output =
+      runTest(cl::ValueRequired,
+              cl::values(clEnumValN(OptionValue::Val, "v1",
+                                    "This is the first enum value\n"
+                                    "which has a really long description\n"
+                                    "thus it is multi-line."),
+                         clEnumValN(OptionValue::Val, "",
+                                    "This is an unnamed enum value option\n"
+                                    "Should be indented as well")));
+
+  // clang-format off
+  EXPECT_EQ(Output,
+            ("  --" + Opt + "=<value> - " + HelpText + "\n"
+             "    =v1                 -   This is the first enum value\n"
+             "                            which has a really long description\n"
+             "                            thus it is multi-line.\n"
+             "    =<empty>            -   This is an unnamed enum value option\n"
+             "                            Should be indented as well\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...);