SkFlags now follows proper dashing convention.

Two dashes are used for flags with multiple characters, and one
dash is used for flags with single characters.

In GM, changed '-wp' to '-p' (the command to choose a directory
for writing SKPs) to fit with the convention.

In render_pictures and bench_pictures, changed the flag for
read and write path to have full names (which are consistent)
and use the old single character names as their shortcuts.

SkCommandLineFlags: Updated the documentation, and only allow
-h or --help for help (again, to match the convention).
Also enforce the single character limit for the short name, and
require the full name to be at least two characters.

Provide full names for skhello.

BUG=https://code.google.com/p/skia/issues/detail?id=1174

Review URL: https://codereview.chromium.org/12521019

git-svn-id: http://skia.googlecode.com/svn/trunk@8582 2bbb7eff-a529-9590-31e7-b0007b416f81

7 files changed, 44 insertions, 39 deletions

diff --git a/gm/gmmain.cpp b/gm/gmmain.cpp
index 850106408c..27a7bdaf9a 100644
--- a/gm/gmmain.cpp
+++ b/gm/gmmain.cpp
@@ -1187,7 +1187,7 @@ DEFINE_string(tileGridReplayScales, "", "Space separated list of floating-point
 DEFINE_string(writeJsonSummaryPath, "", "Write a JSON-formatted result summary to this file.");
 DEFINE_bool2(verbose, v, false, "Print diagnostics (e.g. list each config to be tested).");
 DEFINE_string2(writePath, w, "",  "Write rendered images into this directory.");
-DEFINE_string2(writePicturePath, wp, "", "Write .skp files into this directory.");
+DEFINE_string2(writePicturePath, p, "", "Write .skp files into this directory.");
 
 static int findConfig(const char config[]) {
     for (size_t i = 0; i < SK_ARRAY_COUNT(gRec); i++) {
diff --git a/tools/PictureRenderingFlags.cpp b/tools/PictureRenderingFlags.cpp
index dd2aeecb5a..ca984efa06 100644
--- a/tools/PictureRenderingFlags.cpp
+++ b/tools/PictureRenderingFlags.cpp
@@ -60,7 +60,7 @@ DEFINE_string(mode, "simple", "Run in the corresponding mode:\n"
 DEFINE_int32(multi, 1, "Set the number of threads for multi threaded drawing. "
              "If > 1, requires tiled rendering.");
 DEFINE_bool(pipe, false, "Use SkGPipe rendering. Currently incompatible with \"mode\".");
-DEFINE_string(r, "", "skp files or directories of skp files to process.");
+DEFINE_string2(readPath, r, "", "skp files or directories of skp files to process.");
 DEFINE_double(scale, 1, "Set the scale factor.");
 DEFINE_string(tiles, "", "Used with --mode copyTile to specify number of tiles per larger tile "
               "in the x and y directions.");
diff --git a/tools/bench_pictures_main.cpp b/tools/bench_pictures_main.cpp
index 1013145a90..b5126a68bd 100644
--- a/tools/bench_pictures_main.cpp
+++ b/tools/bench_pictures_main.cpp
@@ -40,7 +40,7 @@ DEFINE_string(logFile, "", "Destination for writing log output, in addition to s
 DEFINE_bool(logPerIter, false, "Log each repeat timer instead of mean.");
 DEFINE_bool(min, false, "Print the minimum times (instead of average).");
 DECLARE_int32(multi);
-DECLARE_string(r);
+DECLARE_string(readPath);
 DEFINE_int32(repeat, 1, "Set the number of times to repeat each test.");
 DEFINE_bool(timeIndividualTiles, false, "Report times for drawing individual tiles, rather than "
             "times for drawing the whole page. Requires tiled rendering.");
@@ -345,7 +345,7 @@ static void setup_benchmark(sk_tools::PictureBenchmark* benchmark) {
         benchmark->setTimeIndividualTiles(true);
     }
 
-    if (FLAGS_r.count() < 1) {
+    if (FLAGS_readPath.count() < 1) {
         gLogger.logError(".skp files or directories are required.\n");
         exit(-1);
     }
@@ -423,8 +423,8 @@ int tool_main(int argc, char** argv) {
     setup_benchmark(&benchmark);
 
     int failures = 0;
-    for (int i = 0; i < FLAGS_r.count(); ++i) {
-        failures += process_input(FLAGS_r[i], benchmark);
+    for (int i = 0; i < FLAGS_readPath.count(); ++i) {
+        failures += process_input(FLAGS_readPath[i], benchmark);
     }
 
     if (failures != 0) {
diff --git a/tools/flags/SkCommandLineFlags.cpp b/tools/flags/SkCommandLineFlags.cpp
index 95811b906a..451daf7f6f 100644
--- a/tools/flags/SkCommandLineFlags.cpp
+++ b/tools/flags/SkCommandLineFlags.cpp
@@ -41,19 +41,25 @@ static bool parse_bool_arg(const char* string, bool* result) {
 bool SkFlagInfo::match(const char* string) {
     if (SkStrStartsWith(string, '-') && strlen(string) > 1) {
         string++;
-        // Allow one or two dashes
+        const SkString* compareName;
         if (SkStrStartsWith(string, '-') && strlen(string) > 1) {
             string++;
+            // There were two dashes. Compare against full name.
+            compareName = &fName;
+        } else {
+            // One dash. Compare against the short name.
+            compareName = &fShortName;
         }
         if (kBool_FlagType == fFlagType) {
             // In this case, go ahead and set the value.
-            if (fName.equals(string) || fShortName.equals(string)) {
+            if (compareName->equals(string)) {
                 *fBoolValue = true;
                 return true;
             }
             if (SkStrStartsWith(string, "no") && strlen(string) > 2) {
                 string += 2;
-                if (fName.equals(string) || fShortName.equals(string)) {
+                // Only allow "no" to be prepended to the full name.
+                if (fName.equals(string)) {
                     *fBoolValue = false;
                     return true;
                 }
@@ -63,15 +69,17 @@ bool SkFlagInfo::match(const char* string) {
             if (equalIndex > 0) {
                 // The string has an equal sign. Check to see if the string matches.
                 SkString flag(string, equalIndex);
-                if (flag.equals(fName) || flag.equals(fShortName)) {
+                if (flag.equals(*compareName)) {
                     // Check to see if the remainder beyond the equal sign is true or false:
                     string += equalIndex + 1;
                     parse_bool_arg(string, fBoolValue);
                     return true;
+                } else {
+                    return false;
                 }
             }
         }
-        return fName.equals(string) || fShortName.equals(string);
+        return compareName->equals(string);
     } else {
         // Has no dash
         return false;
@@ -143,8 +151,7 @@ void SkCommandLineFlags::Parse(int argc, char** argv) {
     // Only allow calling this function once.
     static bool gOnce;
     if (gOnce) {
-        SkDebugf("Parse should only be called once at the beginning"
-                 " of main!\n");
+        SkDebugf("Parse should only be called once at the beginning of main!\n");
         SkASSERT(false);
         return;
     }
@@ -153,8 +160,7 @@ void SkCommandLineFlags::Parse(int argc, char** argv) {
     bool helpPrinted = false;
     // Loop over argv, starting with 1, since the first is just the name of the program.
     for (int i = 1; i < argc; i++) {
-        if (0 == strcmp("-h", argv[i]) || 0 == strcmp("--h", argv[i])
-                || 0 == strcmp("-help", argv[i]) || 0 == strcmp("--help", argv[i])) {
+        if (0 == strcmp("-h", argv[i]) || 0 == strcmp("--help", argv[i])) {
             // Print help message.
             SkTDArray<const char*> helpFlags;
             for (int j = i + 1; j < argc; j++) {
diff --git a/tools/flags/SkCommandLineFlags.h b/tools/flags/SkCommandLineFlags.h
index 1d4f409fbf..c705948348 100644
--- a/tools/flags/SkCommandLineFlags.h
+++ b/tools/flags/SkCommandLineFlags.h
@@ -41,8 +41,6 @@
  *  "--boolean true" (where "true" can be replaced by "false", "TRUE", "FALSE",
  *  "1" or "0").
  *
- *  Single dashes are also permitted for this and other flags.
- *
  *  The helpString will be printed if the help flag (-h or -help) is used.
  *
  *  Similarly, the line
@@ -149,8 +147,8 @@ static bool unused_##name = SkFlagInfo::CreateStringFlag(TO_STRING(name),   \
                                                          defaultValue,      \
                                                          helpString)
 
-// string2 allows specifying a short name. No check is done to ensure that shortName
-// is actually shorter than name.
+// string2 allows specifying a short name. There is an assert that shortName
+// is only 1 character.
 #define DEFINE_string2(name, shortName, defaultValue, helpString)               \
 SkTDArray<const char*> FLAGS_##name;                                            \
 static bool unused_##name = SkFlagInfo::CreateStringFlag(TO_STRING(name),       \
@@ -192,8 +190,7 @@ public:
     // Create flags of the desired type, and append to the list.
     static bool CreateBoolFlag(const char* name, const char* shortName, bool* pBool,
                                bool defaultValue, const char* helpString) {
-        SkFlagInfo* info = SkNEW_ARGS(SkFlagInfo, (name, kBool_FlagType, helpString));
-        info->fShortName.set(shortName);
+        SkFlagInfo* info = SkNEW_ARGS(SkFlagInfo, (name, shortName, kBool_FlagType, helpString));
         info->fBoolValue = pBool;
         *info->fBoolValue = info->fDefaultBool = defaultValue;
         return true;
@@ -202,8 +199,7 @@ public:
     static bool CreateStringFlag(const char* name, const char* shortName,
                                  SkTDArray<const char*>* pStrings,
                                  const char* defaultValue, const char* helpString) {
-        SkFlagInfo* info = SkNEW_ARGS(SkFlagInfo, (name, kString_FlagType, helpString));
-        info->fShortName.set(shortName);
+        SkFlagInfo* info = SkNEW_ARGS(SkFlagInfo, (name, shortName, kString_FlagType, helpString));
         info->fDefaultString.set(defaultValue);
 
         info->fStrings = pStrings;
@@ -217,7 +213,7 @@ public:
 
     static bool CreateIntFlag(const char* name, int32_t* pInt,
                               int32_t defaultValue, const char* helpString) {
-        SkFlagInfo* info = SkNEW_ARGS(SkFlagInfo, (name, kInt_FlagType, helpString));
+        SkFlagInfo* info = SkNEW_ARGS(SkFlagInfo, (name, NULL, kInt_FlagType, helpString));
         info->fIntValue = pInt;
         *info->fIntValue = info->fDefaultInt = defaultValue;
         return true;
@@ -225,7 +221,7 @@ public:
 
     static bool CreateDoubleFlag(const char* name, double* pDouble,
                                  double defaultValue, const char* helpString) {
-        SkFlagInfo* info = SkNEW_ARGS(SkFlagInfo, (name, kDouble_FlagType, helpString));
+        SkFlagInfo* info = SkNEW_ARGS(SkFlagInfo, (name, NULL, kDouble_FlagType, helpString));
         info->fDoubleValue = pDouble;
         *info->fDoubleValue = info->fDefaultDouble = defaultValue;
         return true;
@@ -333,8 +329,9 @@ public:
     }
 
 private:
-    SkFlagInfo(const char* name, FlagTypes type, const char* helpString)
+    SkFlagInfo(const char* name, const char* shortName, FlagTypes type, const char* helpString)
         : fName(name)
+        , fShortName(shortName)
         , fFlagType(type)
         , fHelpString(helpString)
         , fBoolValue(NULL)
@@ -346,6 +343,8 @@ private:
         , fStrings(NULL) {
         fNext = SkCommandLineFlags::gHead;
         SkCommandLineFlags::gHead = this;
+        SkASSERT(NULL != name && strlen(name) > 1);
+        SkASSERT(NULL == shortName || 1 == strlen(shortName));
     }
     // Name of the flag, without initial dashes
     SkString             fName;
diff --git a/tools/render_pictures_main.cpp b/tools/render_pictures_main.cpp
index 81fb96c05a..736e8264d5 100644
--- a/tools/render_pictures_main.cpp
+++ b/tools/render_pictures_main.cpp
@@ -27,8 +27,8 @@ DECLARE_bool(deferImageDecoding);
 DEFINE_int32(maxComponentDiff, 256, "Maximum diff on a component, 0 - 256. Components that differ "
              "by more than this amount are considered errors, though all diffs are reported. "
              "Requires --validate.");
-DECLARE_string(r);
-DEFINE_string(w, "", "Directory to write the rendered images.");
+DECLARE_string(readPath);
+DEFINE_string2(writePath, w, "", "Directory to write the rendered images.");
 DEFINE_bool(writeWholeImage, false, "In tile mode, write the entire rendered image to a "
             "file, instead of an image for each tile.");
 DEFINE_bool(validate, false, "Verify that the rendered image contains the same pixels as "
@@ -277,7 +277,7 @@ int tool_main(int argc, char** argv) {
     SkCommandLineFlags::SetUsage("Render .skp files.");
     SkCommandLineFlags::Parse(argc, argv);
 
-    if (FLAGS_r.isEmpty()) {
+    if (FLAGS_readPath.isEmpty()) {
         SkDebugf(".skp files or directories are required.\n");
         exit(-1);
     }
@@ -311,13 +311,13 @@ int tool_main(int argc, char** argv) {
     SkAutoGraphics ag;
 
     SkString outputDir;
-    if (FLAGS_w.count() == 1) {
-        outputDir.set(FLAGS_w[0]);
+    if (FLAGS_writePath.count() == 1) {
+        outputDir.set(FLAGS_writePath[0]);
     }
 
     int failures = 0;
-    for (int i = 0; i < FLAGS_r.count(); i ++) {
-        failures += process_input(FLAGS_r[i], &outputDir, *renderer.get());
+    for (int i = 0; i < FLAGS_readPath.count(); i ++) {
+        failures += process_input(FLAGS_readPath[i], &outputDir, *renderer.get());
     }
     if (failures != 0) {
         SkDebugf("Failed to render %i pictures.\n", failures);
diff --git a/tools/skhello.cpp b/tools/skhello.cpp
index 271041589f..a4451ac91a 100644
--- a/tools/skhello.cpp
+++ b/tools/skhello.cpp
@@ -11,8 +11,8 @@
 #include "SkImageEncoder.h"
 #include "SkString.h"
 
-DEFINE_string(o, "skhello.png", "The filename to write the image.");
-DEFINE_string(t, "Hello", "The string to write.");
+DEFINE_string2(outFile, o, "skhello.png", "The filename to write the image.");
+DEFINE_string2(text, t, "Hello", "The string to write.");
 
 int tool_main(int argc, char** argv);
 int tool_main(int argc, char** argv) {
@@ -23,11 +23,11 @@ int tool_main(int argc, char** argv) {
     SkString path("skhello.png");
     SkString text("Hello");
 
-    if (!FLAGS_o.isEmpty()) {
-        path.set(FLAGS_o[0]);
+    if (!FLAGS_outFile.isEmpty()) {
+        path.set(FLAGS_outFile[0]);
     }
-    if (!FLAGS_t.isEmpty()) {
-        text.set(FLAGS_t[0]);
+    if (!FLAGS_text.isEmpty()) {
+        text.set(FLAGS_text[0]);
     }
 
     SkPaint paint;