Treat default command line argument properly.

tools/flags/SkCommandLineFlags.cpp

Index: tools/flags/SkCommandLineFlags.cpp
diff --git a/tools/flags/SkCommandLineFlags.cpp b/tools/flags/SkCommandLineFlags.cpp
index d634edb31100dafa02404bd24502b9ee8f5d1cb7..ddedb8e1c75e932ea5236fcfedb767c0c4940f86 100644
--- a/tools/flags/SkCommandLineFlags.cpp
+++ b/tools/flags/SkCommandLineFlags.cpp
@@ -8,6 +8,39 @@
 #include "SkCommandLineFlags.h"
 #include "SkTDArray.h"
 
+bool SkFlagInfo::CreateStringFlag(const char* name, const char* shortName,
+                                  SkCommandLineFlags::StringArray* pStrings,
+                                  const char* defaultValue, const char* helpString) {
+    SkFlagInfo* info = SkNEW_ARGS(SkFlagInfo, (name, shortName, kString_FlagType, helpString));

[epoger, 2013/04/23 16:54:13]

Is this ever freed?  Does it matter?

[scroggo, 2013/04/24 18:42:36]

Yes. It is freed at
https://code.google.com/p/skia/source/browse/trunk/tools/flags/SkCommandLineF...
so long as the caller calls Parse.

+    info->fDefaultString.set(defaultValue);
+
+    info->fStrings = pStrings;

[epoger, 2013/04/23 16:54:13]

Why is it important for the strings to be stored in the StringArray object
pointed to by pStrings, rather than SkFlagInfo just allocating its own
StringArray object?

[scroggo, 2013/04/24 18:42:36]

Because pStrings is what the client will actually use to access the arguments
after parsing (when the SkFlagInfo will be deleted).

So if you wrote:

DEFINE_string(ignoreErrorTypes, ...)

the macro creates an array called FLAGS_ignoreErrorTypes with global scope (in
that file), which you can then access to determine what settings the user
supplied on the command line.

I've added some comments that I hope make that clearer.

+    info->fStrings->reset();
+    // If default is "", leave the array empty.
+    size_t defaultLength = strlen(defaultValue);
+    if (defaultLength > 0) {
+        const char* const defaultEnd = defaultValue + defaultLength;
+        const char* begin = defaultValue;
+        while (true) {
+            while (begin < defaultEnd && ' ' == *begin) {

[epoger, 2013/04/23 16:54:13]

I think it would be cleaner to write a helper function that converts a
space-delimited string to a StringArray, and call that here.

[scroggo, 2013/04/24 18:42:36]

I like that approach too, except that I also like making StringArray's
modification functions being private because it means the client won't
accidentally modify them. I compromised by making the helper function a member
function of SkFlagInfo.

+                begin++;
+            }
+            if (begin < defaultEnd) {
+                const char* end = begin + 1;
+                while (end < defaultEnd && ' ' != *end) {
+                    end++;
+                }
+                size_t length = end - begin;
+                info->fStrings->append(begin, length);
+                begin = end + 1;
+            } else {
+                break;
+            }
+        }
+    }
+    return true;
+}
+
 static bool string_is_in(const char* target, const char* set[], size_t len) {
     for (size_t i = 0; i < len; i++) {
         if (0 == strcmp(target, set[i])) {