Add option to specify defaults configs to --config flag in gm. This makes it easier to run gm defau…

gm/gmmain.cpp

Index: gm/gmmain.cpp
===================================================================
--- gm/gmmain.cpp	(revision 8795)
+++ gm/gmmain.cpp	(working copy)
@@ -1168,10 +1168,14 @@
 #endif // SK_SUPPORT_PDF
 };
 
+static const char kDefaultsConfigStr[] = "defaults";
+static const char kExcludeConfigChar = '~';
+
 static SkString configUsage() {
     SkString result;
     result.appendf("Space delimited list of which configs to run. Possible options: [");
     for (size_t i = 0; i < SK_ARRAY_COUNT(gRec); ++i) {
+        SkASSERT(gRec[i].fName != kDefaultsConfigStr);
         if (i > 0) {
             result.append("|");
         }
@@ -1179,16 +1183,38 @@
     }
     result.append("]\n");
     result.appendf("The default value is: \"");
+    SkString firstDefault;
+    SkString allButFirstDefaults;
+    SkString firstNonDefault;
     for (size_t i = 0; i < SK_ARRAY_COUNT(gRec); ++i) {
         if (gRec[i].fRunByDefault) {
             if (i > 0) {
                 result.append(" ");
             }
-            result.appendf("%s", gRec[i].fName);
+            result.append(gRec[i].fName);
+            if (firstDefault.isEmpty()) {
+                firstDefault = gRec[i].fName;
+            } else {
+                if (!allButFirstDefaults.isEmpty()) {
+                    allButFirstDefaults.append(" ");
+                }
+                allButFirstDefaults.append(gRec[i].fName);
+            }
+        } else if (firstNonDefault.isEmpty()) {

[epoger, 2013/04/23 16:29:40]

you could just remember *a* nonDefault config, which would be slightly simpler:

else {
  oneNonDefault = gRec[i].fName;
}

[bsalomon, 2013/04/24 15:13:54]

Done.

+            firstNonDefault = gRec[i].fName;
         }
     }
-    result.appendf("\"");
-
+    result.append("\"\n");
+    result.appendf("\"%s\" evaluates to the default set of configs.\n", kDefaultsConfigStr);
+    result.appendf("Prepending \"%c\" on a config name excludes it from the set of configs to run.\n",
+                  kExcludeConfigChar);
+    result.appendf("E.g. \"--config %s %c%s %s\" will run these configs:\n\t%s %s",
+                   kDefaultsConfigStr,
+                   kExcludeConfigChar,
+                   firstDefault.c_str(),
+                   firstNonDefault.c_str(),
+                   allButFirstDefaults.c_str(),
+                   firstNonDefault.c_str());
     return result;
 }
 
@@ -1620,12 +1646,32 @@
     }
 
     for (int i = 0; i < FLAGS_config.count(); i++) {
-        int index = findConfig(FLAGS_config[i]);
+        const char* config = FLAGS_config[i];
+        userConfig = true;
+        bool exclude = false;
+        if (*config == kExcludeConfigChar) {
+            exclude = true;
+            config += 1;
+        }
+        int index = findConfig(config);
         if (index >= 0) {
-            appendUnique<size_t>(&configs, index);
-            userConfig = true;
+            if (exclude) {
+                *excludeConfigs.append() = index;
+            } else {
+                appendUnique<size_t>(&configs, index);
+            }
+        } else if (0 == strcmp(kDefaultsConfigStr, config)) {
+            for (size_t c = 0; c < SK_ARRAY_COUNT(gRec); ++c) {
+                if (gRec[c].fRunByDefault) {
+                    if (exclude) {

[epoger, 2013/04/23 16:29:40]

OMG.  The caller can pass "~defaults" to exclude all of the default configs?  My
mind has just been blown.

It looks to me like, once a config has been excluded, it cannot be re-included
(excludeConfigs is applied at the end).  So, that means if the caller specifies
"~defaults angle", then angle may or may not run depending on whether it was in
the default set.  Right?

If so, that seems problematic.  Is there some way we can get all the behavior
you want without allowing "~defaults"?

[bsalomon, 2013/04/24 15:13:54]

The existing --excludeConfig works this way (exclude always overrides any
includes). I added ~defaults simply for orthogonality of  "~" and "defaults". It
isn't useful. I've barred ~defaults and added a comment to the help print about
~ overriding any non-~. 

+                        *excludeConfigs.append() = c;
+                    } else {
+                        appendUnique<size_t>(&configs, c);
+                    }
+                }
+            }
         } else {
-            gm_fprintf(stderr, "unrecognized config %s\n", FLAGS_config[i]);
+            gm_fprintf(stderr, "unrecognized config %s\n", config);
             return -1;
         }
     }
@@ -1739,6 +1785,19 @@
     GrContextFactory* grFactory = NULL;
 #endif
 
+    if (configs.isEmpty()) {
+        gm_fprintf(stderr, "No configs to run.");
+        return -1;
+    }
+
+    // now show the user the set of configs that will be run.
+    SkString configStr("These configs will be run: ");

[epoger, 2013/04/23 16:29:40]

Maybe this should be guarded by if (FLAGS_verbose)?

Either way, this will definitely require running gm/tests/rebaseline.sh.

And you may want to consider adding some new tests gm/tests/run.sh, to make sure
that this new feature stays in working order as other changes are made...

[bsalomon, 2013/04/24 15:13:54]

If we're already printing each test as it runs in the non-verbose output then it
feels OK to me to print the configs over which those tests will be run.

txt outputs updated.

I didn't add new tests. TBH from where I sit these self-tests that rely on
specific output text feel like more of a burden than a benefit. I'm willing to
concede that I'm missing some info. But it feels fragile and for this feature is
perhaps not a good test. Just because gm prints that 8888 will be run, it
doesn't mean that it actually runs. I suppose we could test that the images are
written but I'm not sure this feature warrants that level of testing.

+    // show the user the config that will run.
+    for (int i = 0; i < configs.count(); ++i) {
+        configStr.appendf("%s%s", gRec[configs[i]].fName, (i == configs.count() - 1) ? "\n" : " ");
+    }
+    gm_fprintf(stdout, "%s", configStr.c_str());
+
     if (FLAGS_resourcePath.count() == 1) {
         GM::SetResourcePath(FLAGS_resourcePath[0]);
     }