[PATCH] D57030: [CommandLine] Don't print empty sentinel values from EnumValN lists in help text

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: unittests/Support/CommandLineTest.cpp:860-861
+
+  SmallVector<char, 128> FilePath;
+  bool Valid = true;
+
----------------
Why not make them private with getters? Did you deem it over-engineering?


================
Comment at: unittests/Support/CommandLineTest.cpp:917-918
+
+  EXPECT_EQ(Output, ("  -" + OptName + "=<value> - " + HelpText +
+                     "\n    =v1                -   desc1\n")
+                        .str());
----------------
I understand the use of variable to avoid duplication but it makes it harder to see that we are testing for correct alignment. Could you choose variables with the right length to have the same alignment as the output text such that we can see the alignment by just looking at the text?


================
Comment at: unittests/Support/CommandLineTest.cpp:929-930
+  EXPECT_EQ(Output,
+            ("  -" + OptName + "         - " + HelpText + "\n  -" + OptName +
+             "=<value> - " + HelpText + "\n    =v1                -   desc1\n")
+                .str());
----------------
I think it's better to go to next line for every \n, I'd suggest also keeping the \n at end of line to make the reference string as close as the output. Likewise for other tests below and above.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57030/new/

https://reviews.llvm.org/D57030