[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 a comment.

In D57030#1365979 <https://reviews.llvm.org/D57030#1365979>, @jhenderson wrote:

> 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      |
> |


Thanks for the explanation. In that case I'd like to differentiate between the optional and non optional case.

1. For optional omitting the line is probably fine, or perhaps put a [default] instead of empty equal sign.
2. To handle the non optional case I think we should introduce single quotes after the equal sign to indicate that this is not an error but an empty value, ie. =''. Perhaps also put quote around other values for consistency.

Does that make sense?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57030