[PATCH] D21485: [cl] Teach cl to support the notion of sub commands (e.g. "git checkout <foo>")

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: include/llvm/Support/CommandLine.h:207
@@ +206,3 @@
+
+  Option *ConsumeAfterOpt; // The ConsumeAfter option if it exists.
+};
----------------
You can write ` = nullptr` here.

================
Comment at: lib/Support/CommandLine.cpp:113
@@ -112,1 +112,3 @@
+  void addLiteralOption(Option &Opt, SubCommand *SC, const char *Name) {
     if (!Opt.hasArgStr()) {
+      if (!SC->OptionsMap.insert(std::make_pair(Name, &Opt)).second) {
----------------
Early return.

================
Comment at: lib/Support/CommandLine.cpp:1784-1785
@@ +1783,4 @@
+
+    if (Sub == &*TopLevelSubCommand) {
+      if (Subs.size() > 2) {
+        // Compute the maximum subcommand length...
----------------
You can merge the two `if` conditions.

================
Comment at: unittests/Support/CommandLineTest.cpp:303-304
@@ +302,4 @@
+  EXPECT_TRUE(cl::ParseCommandLineOptions(2, args, nullptr, true));
+  EXPECT_EQ(true, TopLevelOpt);
+  EXPECT_EQ(false, SC1Opt);
+  EXPECT_EQ(false, SC2Opt);
----------------
Why don't you use `EXPECT_FALSE` and `EXPECT_TRUE`?

================
Comment at: unittests/Support/CommandLineTest.cpp:341
@@ +340,3 @@
+  EXPECT_FALSE(cl::ParseCommandLineOptions(3, args, nullptr, true));
+  return;
+}
----------------
Remove.


http://reviews.llvm.org/D21485