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

[James Henderson via Phabricator via llvm-commits]

jhenderson marked 4 inline comments as done.
jhenderson added inline comments.


================
Comment at: lib/Support/CommandLine.cpp:1662-1663
+      StringRef ValueName = getOption(i);
+      if (O.getValueExpectedFlag() == ValueOptional && ValueName.empty() &&
+          getDescription(i).empty())
+        continue;
----------------
thopre wrote:
> I wonder if a small inline function for this would make sense. BTW, do you know why the width is not computed from the return value of getOptionInfo?
I'm not sure I see a `getOptionInfo` function? There is `printOptionInfo` but that does the printing, and needs to know about spacing. I guess you could pull the common logic out into a function, but that would result in it being computed twice for every piece of help text, and would be a different design to the current design.


================
Comment at: unittests/Support/CommandLineTest.cpp:1023
+  // The length of a=<value> (including indentation) is actually the same as the
+  // =<empty> string, so it is impossible to distinguish via the two via
+  // testing.
----------------
thopre wrote:
> Remove the first occurrence of "via". Any reason why the text here is different from the other two below?
I can't remember there being a good reason, but I've tweaked all the comments anyway to clarify which case it is that we actually want to test and what we can't distinguish it from.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57030