[PATCH] D58711: [CommandLine] Allow grouping options which can have values.

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin marked 8 inline comments as done.
ikudrin added a comment.

By the way, we have some inconsistency in how `cl::Prefix` and `cl::AlwaysPrefix` options work in case of `-opt=val` form. The former takes only `val`, while the latter keeps `=`. Moreover, prefix grouping options in GNU's binutils, like `-j` in `objdump`, work partially as ours `cl::AlwaysPrefix`, because they preserve `=`, and partially as `cl::Prefix`, because they allow passing the value in a separate argument. Thus, we still cannot fully reproduce that behavior.

Do you think it is worth adding a new modifier, say, `cl::PreserveEqSign`, to improve compatibility?



================
Comment at: unittests/Support/CommandLineTest.cpp:1225
+
+  // The all three previous cases should work the same way
+  // if a cl::Prefix + cl::Grouping option is used at the end of a group.
----------------
jhenderson wrote:
> The all -> All
> 
> The wrapping here looks a bit weird. I think "if a" should be on this line, not the next.
I wanted not to split `cl::Prefix + cl::Grouping`, as well as not to separate an article from them.
So, I opted to rephrase the comment. How does it look now?


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

https://reviews.llvm.org/D58711