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

[James Henderson via Phabricator via llvm-commits]

jhenderson marked an inline comment as done.
jhenderson added a comment.

In D57030#1365762 <https://reviews.llvm.org/D57030#1365762>, @thopre wrote:

> Do such sentinel trigger if not passing the equal sign (ie. when using only "-mhvx" in the example you quote) or only with equal sign without a value (one has to use "-mhvx=" to select the option)?
>
> If the former I think this change is fine. I would also suggest adding an early continue when only the description is empty on line 1683 to avoid the empty '-'.
>
> But if the latter (one has to do "-mhvx=" to select the option) I'm not sure hiding the option is right since it is a value (albeit empty) people can use.


Assuming ValueOptional is specified, then yes, you don't need the equal sign. I've written up a table below of different input options versus their behaviour to clarify things:

| Code                       | --option    | --option=   | --option foo | --option=foo |
| default, no sentinel       | not allowed | not allowed | allowed      | allowed      |
| default, sentinel          | not allowed | allowed     | allowed      | allowed      |
| ValueOptional, no sentinel | not allowed | not allowed | not allowed  | allowed      |
| ValueOptional, sentinel    | allowed     | allowed     | not allowed  | allowed      |



================
Comment at: lib/Support/CommandLine.cpp:1683
       outs() << "    =" << getOption(i);
       outs().indent(NumSpaces) << " -   " << getDescription(i) << '\n';
     }
----------------
thopre wrote:
> Add early continue here as well when only description is empty?
Seems reasonable to me, although the '\n' is required still. To avoid repeating it, I'll write it as:
```
if (!getDescription(i).empty())
  outs().indent(NumSpaces) << " -   " << getDescription(i);
outs() << '\n';
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57030