update bookmaker to ToT

more image docs, still not done
add ability to comment out non-working examples easily
start work on additional self-checks
clean up use of this-> in docs

TBR=reed@google.com
Docs-Preview: https://skia.org/?cl=91720
Bug: skia:6898
Change-Id: I706ab8145290e53ab67d3f509ccf4e1225adb3c3
Reviewed-on: https://skia-review.googlesource.com/91720
Commit-Queue: Cary Clark <caryclark@skia.org>
Reviewed-by: Cary Clark <caryclark@skia.org>

7 files changed, 238 insertions, 32 deletions

diff --git a/tools/bookmaker/bookmaker.cpp b/tools/bookmaker/bookmaker.cpp
index eb1ce1e366..b63813d667 100644
--- a/tools/bookmaker/bookmaker.cpp
+++ b/tools/bookmaker/bookmaker.cpp
@@ -12,8 +12,9 @@ DEFINE_string2(bmh, b, "", "Path to a *.bmh file or a directory.");
 DEFINE_bool2(catalog, c, false, "Write example catalog.htm. (Requires -b -f -r)");
 DEFINE_string2(examples, e, "", "File of fiddlecli input, usually fiddle.json (For now, disables -r -f -s)");
 DEFINE_string2(fiddle, f, "", "File of fiddlecli output, usually fiddleout.json.");
+DEFINE_bool2(hack, h, false, "Do a find/replace hack to update all *.bmh files. (Requires -b)");
 DEFINE_string2(include, i, "", "Path to a *.h file or a directory.");
-DEFINE_bool2(hack, k, false, "Do a find/replace hack to update all *.bmh files. (Requires -b)");
+DEFINE_bool2(selfcheck, k, false, "Check bmh against itself. (Requires -b)");
 DEFINE_bool2(stdout, o, false, "Write file out to standard out.");
 DEFINE_bool2(populate, p, false, "Populate include from bmh. (Requires -b -i)");
 DEFINE_string2(ref, r, "", "Resolve refs and write *.md files to path. (Requires -b -f)");
@@ -25,11 +26,10 @@ DEFINE_bool2(skip, z, false, "Skip degenerate missed in legacy preprocessor.");
 /*  recipe for generating timestamps for existing doxygen comments
 find include/core -type f -name '*.h' -print -exec git blame {} \; > ~/all.blame.txt
 
+todos:
 space table better for Constants
 should Return be on same line as 'Return Value'?
 remove anonymous header, e.g. Enum SkPaint::::anonymous_2
-Text Encoding anchors in paragraph are echoed instead of being linked to anchor names
-    also should not point to 'undocumented' since they are resolvable links
 #Member lost all formatting
 #List needs '# content ##', formatting
 consts like enum members need fully qualfied refs to make a valid link
@@ -38,7 +38,7 @@ enum comments should be disallowed unless after #Enum and before first #Const
 trouble with aliases, plurals
     need to keep first letter of includeWriter @param / @return lowercase
     Quad -> quad, Quads -> quads
-check for summary containing all methods
+see head of selfCheck.cpp for additional todos
  */
 
 /*
@@ -334,6 +334,7 @@ bool BmhParser::addDefinition(const char* defStart, bool hasEnd, MarkType markTy
         case MarkType::kAlias:
         case MarkType::kAnchor:
         case MarkType::kDefine:
+        case MarkType::kDuration:
         case MarkType::kError:
         case MarkType::kFile:
         case MarkType::kHeight:
@@ -1254,6 +1255,7 @@ vector<string> BmhParser::typeName(MarkType markType, bool* checkEnd) {
         case MarkType::kBug:  // fixme: expect number
         case MarkType::kDefine:
         case MarkType::kDefinedBy:
+        case MarkType::kDuration:
         case MarkType::kError:
         case MarkType::kFile:
         case MarkType::kHeight:
@@ -1511,8 +1513,8 @@ int main(int argc, char** const argv) {
         SkCommandLineFlags::Parse(argc, argv);
     } else {
         SkCommandLineFlags::PrintUsage();
-        const char* const commands[] = { "", "-h", "bmh", "-h", "examples", "-h", "include", "-h", "fiddle",
-            "-h", "ref", "-h", "status", "-h", "tokens",
+        const char* const commands[] = { "", "-h", "bmh", "-h", "examples", "-h", "include",
+            "-h", "fiddle", "-h", "ref", "-h", "status", "-h", "tokens",
             "-h", "crosscheck", "-h", "populate", "-h", "spellcheck" };
         SkCommandLineFlags::Parse(SK_ARRAY_COUNT(commands), commands);
         return 0;
@@ -1587,17 +1589,19 @@ int main(int argc, char** const argv) {
         SkCommandLineFlags::PrintUsage();
         return 1;
     }
+    bmhParser.reset();
     if (!FLAGS_bmh.isEmpty()) {
-        bmhParser.reset();
         if (!bmhParser.parseFile(FLAGS_bmh[0], ".bmh")) {
             return -1;
         }
     } else if (!FLAGS_status.isEmpty()) {
-        bmhParser.reset();
         if (!bmhParser.parseStatus(FLAGS_status[0], ".bmh", StatusFilter::kInProgress)) {
             return -1;
         }
     }
+    if (FLAGS_selfcheck && !SelfCheck(bmhParser)) {
+        return -1;
+    }
     bool done = false;
     if (!FLAGS_include.isEmpty() && FLAGS_tokens) {
         IncludeParser includeParser;
diff --git a/tools/bookmaker/bookmaker.h b/tools/bookmaker/bookmaker.h
index c83927822a..6752225d52 100644
--- a/tools/bookmaker/bookmaker.h
+++ b/tools/bookmaker/bookmaker.h
@@ -95,6 +95,7 @@ enum class MarkType {
     kDeprecated,
     kDescription,
     kDoxygen,
+    kDuration,
     kEnum,
     kEnumClass,
     kError,
@@ -827,6 +828,7 @@ public:
     string fiddleName() const;
     string formatFunction() const;
     const Definition* hasChild(MarkType markType) const;
+    bool hasMatch(const string& name) const;
     const Definition* hasParam(const string& ref) const;
     bool isClone() const { return fClone; }
 
@@ -1209,20 +1211,21 @@ public:
 , { "Define",      nullptr,      MarkType::kDefine,       R_O, E_N, M_ST }
 , { "DefinedBy",   nullptr,      MarkType::kDefinedBy,    R_N, E_N, M(Method) }
 , { "Deprecated",  nullptr,      MarkType::kDeprecated,   R_Y, E_N, 0 }
-, { "Description", nullptr,      MarkType::kDescription,  R_Y, E_N, M(Example) }
+, { "Description", nullptr,      MarkType::kDescription,  R_Y, E_N, M(Example) | M(NoExample) }
 , { "Doxygen",     nullptr,      MarkType::kDoxygen,      R_Y, E_N, 0 }
+, { "Duration",    nullptr,      MarkType::kDuration,     R_N, E_N, M(Example) | M(NoExample) }
 , { "Enum",        &fEnumMap,    MarkType::kEnum,         R_Y, E_O, M_CSST | M(Root) }
 , { "EnumClass",   &fClassMap,   MarkType::kEnumClass,    R_Y, E_O, M_CSST | M(Root) }
-, { "Error",       nullptr,      MarkType::kError,        R_N, E_N, M(Example) }
+, { "Error",       nullptr,      MarkType::kError,        R_N, E_N, M(Example) | M(NoExample) }
 , { "Example",     nullptr,      MarkType::kExample,      R_O, E_N, M_CSST | M_E | M(Method) }
 , { "Experimental", nullptr,     MarkType::kExperimental, R_Y, E_N, 0 }
 , { "External",    nullptr,      MarkType::kExternal,     R_Y, E_N, M(Root) }
 , { "File",        nullptr,      MarkType::kFile,         R_N, E_N, M(Track) }
 , { "Formula",     nullptr,      MarkType::kFormula,      R_O, E_N,
                                                     M(Column) | M_ST | M(Member) | M(Method) | M_D }
-, { "Function",    nullptr,      MarkType::kFunction,     R_O, E_N, M(Example) }
-, { "Height",      nullptr,      MarkType::kHeight,       R_N, E_N, M(Example) }
-, { "Image",       nullptr,      MarkType::kImage,        R_N, E_N, M(Example) }
+, { "Function",    nullptr,      MarkType::kFunction,     R_O, E_N, M(Example) | M(NoExample) }
+, { "Height",      nullptr,      MarkType::kHeight,       R_N, E_N, M(Example) | M(NoExample) }
+, { "Image",       nullptr,      MarkType::kImage,        R_N, E_N, M(Example) | M(NoExample) }
 , { "Legend",      nullptr,      MarkType::kLegend,       R_Y, E_N, M(Table) }
 , { "",            nullptr,      MarkType::kLink,         R_N, E_N, M(Anchor) }
 , { "List",        nullptr,      MarkType::kList,         R_Y, E_N, M(Method) | M_CSST | M_E | M_D }
@@ -1230,18 +1233,18 @@ public:
 , { "",            nullptr,      MarkType::kMarkChar,     R_N, E_N, 0 }
 , { "Member",      nullptr,      MarkType::kMember,       R_Y, E_N, M(Class) | M(Struct) }
 , { "Method",      &fMethodMap,  MarkType::kMethod,       R_Y, E_Y, M_CSST }
-, { "NoExample",   nullptr,      MarkType::kNoExample,    R_Y, E_N, 0 }
+, { "NoExample",   nullptr,      MarkType::kNoExample,    R_O, E_N, M_CSST | M_E | M(Method) }
 , { "Outdent",     nullptr,      MarkType::kOutdent,      R_N, E_N, M(Code) }
 , { "Param",       nullptr,      MarkType::kParam,        R_Y, E_N, M(Method) }
-, { "Platform",    nullptr,      MarkType::kPlatform,     R_N, E_N, M(Example) }
+, { "Platform",    nullptr,      MarkType::kPlatform,     R_N, E_N, M(Example) | M(NoExample) }
 , { "Private",     nullptr,      MarkType::kPrivate,      R_N, E_N, 0 }
 , { "Return",      nullptr,      MarkType::kReturn,       R_Y, E_N, M(Method) }
 , { "",            nullptr,      MarkType::kRoot,         R_Y, E_N, 0 }
 , { "",            nullptr,      MarkType::kRow,          R_Y, E_N, M(Table) | M(List) }
 , { "SeeAlso",     nullptr,      MarkType::kSeeAlso,      R_Y, E_N,
                                                              M_CSST | M_E | M(Method) | M(Typedef) }
-, { "Set",         nullptr,      MarkType::kSet,          R_N, E_N, M(Example) }
-, { "StdOut",      nullptr,      MarkType::kStdOut,       R_N, E_N, M(Example) }
+, { "Set",         nullptr,      MarkType::kSet,          R_N, E_N, M(Example) | M(NoExample) }
+, { "StdOut",      nullptr,      MarkType::kStdOut,       R_N, E_N, M(Example) | M(NoExample) }
 , { "Struct",      &fClassMap,   MarkType::kStruct,       R_Y, E_O, M(Class) | M(Root) | M_ST }
 , { "Substitute",  nullptr,      MarkType::kSubstitute,   R_N, E_N, M_ST }
 , { "Subtopic",    nullptr,      MarkType::kSubtopic,     R_Y, E_Y, M_CSST }
@@ -1255,7 +1258,7 @@ public:
 , { "Typedef",     &fTypedefMap, MarkType::kTypedef,      R_Y, E_N, M(Class) | M_ST }
 , { "",            nullptr,      MarkType::kUnion,        R_Y, E_N, 0 }
 , { "Volatile",    nullptr,      MarkType::kVolatile,     R_N, E_N, M(StdOut) }
-, { "Width",       nullptr,      MarkType::kWidth,        R_N, E_N, M(Example) } }
+, { "Width",       nullptr,      MarkType::kWidth,        R_N, E_N, M(Example) | M(NoExample) } }
 , fSkip(skip)
         {
             this->reset();
@@ -1403,6 +1406,7 @@ public:
         , { nullptr,        MarkType::kDeprecated }
         , { nullptr,        MarkType::kDescription }
         , { nullptr,        MarkType::kDoxygen }
+        , { nullptr,        MarkType::kDuration }
         , { &fIEnumMap,     MarkType::kEnum }
         , { &fIEnumMap,     MarkType::kEnumClass }
         , { nullptr,        MarkType::kError }
@@ -2116,4 +2120,6 @@ private:
     typedef TextParser INHERITED;
 };
 
+bool SelfCheck(const BmhParser& );
+
 #endif
diff --git a/tools/bookmaker/definition.cpp b/tools/bookmaker/definition.cpp
index 82b0a33a81..74f4d5a7fb 100644
--- a/tools/bookmaker/definition.cpp
+++ b/tools/bookmaker/definition.cpp
@@ -527,8 +527,12 @@ bool Definition::exampleToScript(string* result, ExampleOptions exampleOptions)
     string code;
     string imageStr = "0";
     string srgbStr = "false";
+    string durationStr = "0";
     for (auto const& iter : fChildren) {
         switch (iter->fMarkType) {
+            case MarkType::kDuration:
+                durationStr = string(iter->fContentStart, iter->fContentEnd - iter->fContentStart);
+                break;
             case MarkType::kError:
                 result->clear();
                 return true;
@@ -579,6 +583,7 @@ bool Definition::exampleToScript(string* result, ExampleOptions exampleOptions)
                 SkASSERT(0);  // more coding to do
         }
     }
+    string animatedStr = "0" != durationStr ? "true" : "false";
     string textOutStr = textOut ? "true" : "false";
     size_t pos = 0;
     while (pos < text.length() && ' ' > text[pos]) {
@@ -622,8 +627,8 @@ bool Definition::exampleToScript(string* result, ExampleOptions exampleOptions)
             example += "        \"srgb\": " + srgbStr + ",\n";
             example += "        \"f16\": false,\n";
             example += "        \"textOnly\": " + textOutStr + ",\n";
-            example += "        \"animated\": false,\n";
-            example += "        \"duration\": 0\n";
+            example += "        \"animated\": " + animatedStr + ",\n";
+            example += "        \"duration\": " + durationStr + "\n";
             example += "    },\n";
             example += "    \"fast\": true";
         }
@@ -1035,6 +1040,18 @@ const Definition* Definition::hasParam(const string& ref) const {
     return nullptr;
 }
 
+bool Definition::hasMatch(const string& name) const {
+    for (auto child : fChildren) {
+        if (name == child->fName) {
+            return true;
+        }
+        if (child->hasMatch(name)) {
+            return true;
+        }
+    }
+    return false;
+}
+
 bool Definition::methodHasReturn(const string& name, TextParser* methodParser) const {
     if (methodParser->skipExact("static")) {
         methodParser->skipWhiteSpace();
diff --git a/tools/bookmaker/includeWriter.cpp b/tools/bookmaker/includeWriter.cpp
index aeb1200a49..230e524703 100644
--- a/tools/bookmaker/includeWriter.cpp
+++ b/tools/bookmaker/includeWriter.cpp
@@ -585,9 +585,6 @@ void IncludeWriter::enumSizeItems(const Definition& child) {
 
 // walk children and output complete method doxygen description
 void IncludeWriter::methodOut(const Definition* method, const Definition& child) {
-    if (string::npos != method->fName.find("validate")) {
-        SkDebugf("");
-    }
     if (fPendingMethod) {
         fIndent -= 4;
         fPendingMethod = false;
diff --git a/tools/bookmaker/mdOut.cpp b/tools/bookmaker/mdOut.cpp
index 0443199155..24009d02f8 100644
--- a/tools/bookmaker/mdOut.cpp
+++ b/tools/bookmaker/mdOut.cpp
@@ -182,15 +182,8 @@ string MdOut::addReferences(const char* refStart, const char* refEnd,
                         const Definition* paramType = this->findParamType();
                         if (paramType) {
                             string fullName = paramType->fName + "::" + ref;
-                            bool found = false;
-                            for (auto child : paramType->fChildren) {
-                                if (fullName == child->fName) {
-                                    result += linkRef(leadingSpaces, paramType, ref);
-                                    found = true;
-                                    break;
-                                }
-                            }
-                            if (found) {
+                            if (paramType->hasMatch(fullName)) {
+                                result += linkRef(leadingSpaces, paramType, ref);
                                 continue;
                             }
                         }
@@ -754,6 +747,8 @@ void MdOut::markTypeOut(Definition* def) {
             break;
         case MarkType::kDoxygen:
             break;
+        case MarkType::kDuration:
+            break;
         case MarkType::kEnum:
         case MarkType::kEnumClass:
             this->mdHeaderOut(2);
diff --git a/tools/bookmaker/selfCheck.cpp b/tools/bookmaker/selfCheck.cpp
new file mode 100644
index 0000000000..0b28d16c70
--- /dev/null
+++ b/tools/bookmaker/selfCheck.cpp
@@ -0,0 +1,185 @@
+/*
+ * Copyright 2018 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "bookmaker.h"
+
+// Check that summary contains all methods
+
+// Check that mutiple like-named methods are under one Subtopic
+
+// Check that all subtopics are in table of contents
+
+// Check that all constructors are in a table of contents
+//          should be 'creators' instead of constructors?
+
+// Check that SeeAlso reference each other
+
+// Would be nice to check if other classes have 'create' methods that are included
+//          SkSurface::makeImageSnapShot should be referenced under SkImage 'creators'
+
+class SelfChecker {
+public:
+    SelfChecker(const BmhParser& bmh)
+        : fBmhParser(bmh)
+        {}
+
+    bool check() {
+        for (const auto& topic : fBmhParser.fTopicMap) {
+            Definition* topicDef = topic.second;
+            if (topicDef->fParent) {
+                continue;
+            }
+            if (!topicDef->isRoot()) {
+                return fBmhParser.reportError<bool>("expected root topic");
+            }
+            fRoot = topicDef->asRoot();
+            if (!this->checkMethodSummary()) {
+                return false;
+            }
+            if (!this->checkMethodSubtopic()) {
+                return false;
+            }
+            if (!this->checkSubtopicContents()) {
+                return false;
+            }
+            if (!this->checkConstructors()) {
+                return false;
+            }
+            if (!this->checkSeeAlso()) {
+                return false;
+            }
+            if (!this->checkCreators()) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+protected:
+    bool checkConstructors() {
+        return true;
+    }
+
+    bool checkCreators() {
+        return true;
+    }
+
+    bool checkMethodSubtopic() {
+        return true;
+    }
+
+    bool checkMethodSummary() {
+        SkDebugf("");
+        // look for struct or class in fChildren
+        for (auto& rootChild : fRoot->fChildren) {
+            if (MarkType::kStruct == rootChild->fMarkType ||
+                    MarkType::kClass == rootChild->fMarkType) {
+                auto& cs = rootChild;
+                // expect Overview as Topic in every main class or struct
+                Definition* overview = nullptr;
+                for (auto& csChild : cs->fChildren) {
+                    if ("Overview" == csChild->fName) {
+                        if (!overview) {
+                            return cs->reportError<bool>("expected only one Overview");
+                        }
+                        overview = csChild;
+                    }
+                }
+                if (!overview) {
+                    return cs->reportError<bool>("missing #Topic Overview");
+                }
+                Definition* memberFunctions = nullptr;
+                for (auto& overChild : overview->fChildren) {
+                    if ("Member_Functions" == overChild->fName) {
+                        memberFunctions = overChild;
+                        break;
+                    }
+                }
+                if (!memberFunctions) {
+                    return overview->reportError<bool>("missing #Subtopic Member_Functions");
+                }
+                if (MarkType::kSubtopic != memberFunctions->fMarkType) {
+                    return memberFunctions->reportError<bool>("expected #Subtopic Member_Functions");
+                }
+                Definition* memberTable = nullptr;
+                for (auto& memberChild : memberFunctions->fChildren) {
+                    if (MarkType::kTable == memberChild->fMarkType &&
+                            memberChild->fName == memberFunctions->fName) {
+                        memberTable = memberChild;
+                        break;
+                    }
+                }
+                if (!memberTable) {
+                    return memberFunctions->reportError<bool>("missing #Table in Member_Functions");
+                }
+                vector<string> overviewEntries; // build map of overview entries
+                bool expectLegend = true;
+                string prior = " ";  // expect entries to be alphabetical
+                for (auto& memberRow : memberTable->fChildren) {
+                    if (MarkType::kLegend == memberRow->fMarkType) {
+                        if (!expectLegend) {
+                            return memberRow->reportError<bool>("expect #Legend only once");
+                        }
+                        // todo: check if legend format matches table's rows' format
+                        expectLegend = false;
+                    } else if (expectLegend) {
+                        return memberRow->reportError<bool>("expect #Legend first");
+                    }
+                    if (MarkType::kRow != memberRow->fMarkType) {
+                        continue;  // let anything through for now; can tighten up in the future
+                    }
+                    // expect column 0 to point to function name
+                    // todo: content end points past space; could tighten that up
+                    Definition* column0 = memberRow->fChildren[0];
+                    string name = string(column0->fContentStart,
+                            column0->fTerminator - column0->fContentStart);
+                    if (prior > name) {
+                        return memberRow->reportError<bool>("expect alphabetical order");
+                    }
+                    if (prior == name) {
+                        return memberRow->reportError<bool>("expect unique names");
+                    }
+                    // todo: error if name is all lower case and doesn't end in ()
+                    overviewEntries.push_back(name);
+                    prior = name;
+                }
+                // mark corresponding methods as visited (may be more than one per entry)
+                for (auto& csChild : cs->fChildren) {
+                    if (MarkType::kMethod != csChild->fMarkType) {
+                        // only check methods for now
+                        continue;
+                    }
+                    auto start = csChild->fName.find_last_of(':');
+                    start = string::npos == start ? 0 : start + 1;
+                    string name = csChild->fName.substr(start);
+                    if (overviewEntries.end() ==
+                            std::find(overviewEntries.begin(), overviewEntries.end(), name)) {
+                        return csChild->reportError<bool>("missing in Overview");
+                    }
+                }
+            }
+        }
+        return true;
+    }
+
+    bool checkSeeAlso() {
+        return true;
+    }
+
+    bool checkSubtopicContents() {
+        return true;
+    }
+
+private:
+    const BmhParser& fBmhParser;
+    RootDefinition* fRoot;
+};
+
+bool SelfCheck(const BmhParser& bmh) {
+    SelfChecker checker(bmh);
+    return checker.check();
+}
diff --git a/tools/bookmaker/spellCheck.cpp b/tools/bookmaker/spellCheck.cpp
index 33c0578853..838e44ec6e 100644
--- a/tools/bookmaker/spellCheck.cpp
+++ b/tools/bookmaker/spellCheck.cpp
@@ -181,6 +181,8 @@ bool SpellCheck::check(Definition* def) {
             break;
         case MarkType::kDoxygen:
             break;
+        case MarkType::kDuration:
+            break;
         case MarkType::kEnum:
         case MarkType::kEnumClass:
             this->wordCheck(def->fName);