improve bookmaker defines and references

generating replacement includes exposed
errors mostly dealing with globals like
SkAlphaType and members. Rewrite finding
and resolving links to hopefully make this
area more robust.

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

4 files changed, 65 insertions, 55 deletions

diff --git a/tools/bookmaker/bookmaker.cpp b/tools/bookmaker/bookmaker.cpp
index 6f7c698ca5..f3641a92d5 100644
--- a/tools/bookmaker/bookmaker.cpp
+++ b/tools/bookmaker/bookmaker.cpp
@@ -188,13 +188,31 @@ bool BmhParser::addDefinition(const char* defStart, bool hasEnd, MarkType markTy
                 definition = rootDefinition;
                 definition->fFileName = fFileName;
                 definition->fContentStart = fChar;
-                definition->fName = typeNameBuilder[0];
-                Definition* parent = fParent;
-                while (parent && MarkType::kTopic != parent->fMarkType
-                        && MarkType::kSubtopic != parent->fMarkType) {
-                    parent = parent->fParent;
+                if (MarkType::kTopic == markType) {
+                    if (fParent) {
+                        return this->reportError<bool>("#Topic must be root");
+                    }
+                    // topic name is unappended
+                    definition->fName = typeNameBuilder[0];
+                } else {
+                    if (!fParent) {
+                        return this->reportError<bool>("#Subtopic may not be root");
+                    }
+                    Definition* parent = fParent;
+                    while (MarkType::kTopic != parent->fMarkType && MarkType::kSubtopic != parent->fMarkType) {
+                        parent = parent->fParent;
+                        if (!parent) {
+                            // subtopic must have subtopic or topic in parent chain
+                            return this->reportError<bool>("#Subtopic missing parent");
+                        }
+                    }
+                    if (MarkType::kSubtopic == parent->fMarkType) {
+                        // subtopic prepends parent subtopic name, but not parent topic name
+                        definition->fName = parent->fName + '_';
+                    }
+                    definition->fName += typeNameBuilder[0];
+                    definition->fFiddle = parent->fFiddle + '_';
                 }
-                definition->fFiddle = parent ? parent->fFiddle + '_' : "";
                 definition->fFiddle += Definition::NormalizedName(typeNameBuilder[0]);
                 this->setAsParent(definition);
             }
@@ -399,6 +417,7 @@ bool BmhParser::addDefinition(const char* defStart, bool hasEnd, MarkType markTy
                     return this->reportError<bool>("duplicate alias");
                 }
                 fAliasMap[alias] = definition;
+                definition->fFiddle = definition->fParent->fFiddle;
 			}
 			else if (MarkType::kLine == markType) {
 				const char* nextLF = this->strnchr('\n', this->fEnd);
diff --git a/tools/bookmaker/bookmaker.h b/tools/bookmaker/bookmaker.h
index 6bd4e1e5fa..b846a47039 100644
--- a/tools/bookmaker/bookmaker.h
+++ b/tools/bookmaker/bookmaker.h
@@ -2069,7 +2069,7 @@ public:
     static constexpr const char* kOperators = "Operator";
     static constexpr const char* kOverview = "Overview";
     static constexpr const char* kRelatedFunctions = "Related_Function";
-    static constexpr const char* kSubtopics = "Subtopic";
+    static constexpr const char* kSubtopics = "Overview_Subtopic";
 
 private:
     enum class TableState {
diff --git a/tools/bookmaker/includeWriter.cpp b/tools/bookmaker/includeWriter.cpp
index 716194066c..61011e4a85 100644
--- a/tools/bookmaker/includeWriter.cpp
+++ b/tools/bookmaker/includeWriter.cpp
@@ -640,9 +640,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("scalePixels")) {
-        SkDebugf("");
-    }
     if (fPendingMethod) {
         fIndent -= 4;
         fPendingMethod = false;
@@ -1781,15 +1778,8 @@ string IncludeWriter::resolveRef(const char* start, const char* end, bool first,
                         MarkType::kEnumClass == parent->fMarkType) {
                     if (parent->fParent != fRootTopic) {
                         substitute = parent->fName;
-                        size_t under = undername.find('_');
-                        if (string::npos != under) {
-                            string secondHalf(&undername[under],
-                                    (size_t) (undername.length() - under));
-                            substitute += ConvertRef(secondHalf, false);
-                        } else {
-                            substitute += ' ';
-                            substitute += ConvertRef(undername, false);
-                        }
+                        substitute += ' ';
+                        substitute += ConvertRef(undername, false);
                     } else {
                         substitute += ConvertRef(undername, first);
                     }
@@ -1801,9 +1791,6 @@ string IncludeWriter::resolveRef(const char* start, const char* end, bool first,
     if (first && isupper(start[0]) && substitute.length() > 0 && islower(substitute[0])) {
         substitute[0] = start[0];
     }
-    if (undername == "Color_Type" && substitute == "") {
-        SkDebugf("");
-    }
     return substitute;
 }
 
@@ -1859,9 +1846,6 @@ int IncludeWriter::lookupReference(const PunctuationState punctuation, const Wor
             temp = ConvertRef(resolved, false);
         }
     }
-    if (resolved == "Color_Type" && temp == "color type") {
-        SkDebugf("");
-    }
     if (temp.length()) {
         if (start > lastWrite) {
             SkASSERT(data[start - 1] >= ' ');
diff --git a/tools/bookmaker/mdOut.cpp b/tools/bookmaker/mdOut.cpp
index 77a370e156..c14ac23a43 100644
--- a/tools/bookmaker/mdOut.cpp
+++ b/tools/bookmaker/mdOut.cpp
@@ -569,8 +569,9 @@ const Definition* MdOut::isDefined(const TextParser& parser, const string& ref,
         // try with a prefix
         if ('k' == ref[0]) {
             for (auto const& iter : fBmhParser.fEnumMap) {
-                if (iter.second.find(ref, RootDefinition::AllowParens::kYes)) {
-                    return &iter.second;
+                auto def = iter.second.find(ref, RootDefinition::AllowParens::kYes);
+                if (def) {
+                    return def;
                 }
             }
             if (fEnumClass) {
@@ -650,37 +651,41 @@ string MdOut::linkName(const Definition* ref) const {
 // def should not include SkXXX_
 string MdOut::linkRef(const string& leadingSpaces, const Definition* def,
         const string& ref, BmhParser::Resolvable resolvable) const {
-	string buildup;
+    string buildup;
+    string refName;
     const string* str = &def->fFiddle;
     SkASSERT(str->length() > 0);
-    size_t under = str->find('_');
-    const Definition* curRoot = fRoot;
-    string classPart = string::npos != under ? str->substr(0, under) : *str;
-    bool classMatch = curRoot->fName == classPart;
-    while (curRoot->fParent) {
-        curRoot = curRoot->fParent;
-        classMatch |= curRoot->fName == classPart;
+    string classPart = *str;
+    bool globalEnumMember = false;
+    if (MarkType::kAlias == def->fMarkType) {
+        def = def->fParent;
+        SkASSERT(def);
+        SkASSERT(MarkType::kSubtopic == def->fMarkType ||MarkType::kTopic == def->fMarkType);
     }
-    const Definition* defRoot;
-    const Definition* temp = def;
-    do {
-        defRoot = temp;
-        if (!(temp = temp->fParent)) {
-            break;
+    if (MarkType::kSubtopic == def->fMarkType) {
+        const Definition* topic = def->topicParent();
+        SkASSERT(topic);
+        classPart = topic->fName;
+        refName = def->fName;
+    } else if (MarkType::kTopic == def->fMarkType) {
+        refName = def->fName;
+    } else {
+        if ('k' == (*str)[0] && string::npos != str->find("_Sk")) {
+            globalEnumMember = true;
+        } else {
+            SkASSERT("Sk" == str->substr(0, 2) || "SK" == str->substr(0, 2)
+                    // FIXME: kitchen sink catch below, need to do better
+                    || string::npos != def->fFileName.find("undocumented"));
+            size_t under = str->find('_');
+            classPart = string::npos != under ? str->substr(0, under) : *str;
         }
-        classMatch |= temp != defRoot && temp->fName == classPart;
-    } while (true);
-    string namePart = string::npos != under ? str->substr(under + 1, str->length()) : *str;
+        refName = def->fFiddle;
+    }
+    bool classMatch = fRoot->fFileName == def->fFileName;
     SkASSERT(fRoot);
     SkASSERT(fRoot->fFileName.length());
-    if (classMatch) {
-        buildup = "#";
-        if (*str != classPart && "Sk" == classPart.substr(0, 2)) {
-            buildup += classPart + "_";
-        }
-        buildup += namePart;
-    } else {
-        string filename = defRoot->asRoot()->fFileName;
+    if (!classMatch) {
+        string filename = def->fFileName;
         if (filename.substr(filename.length() - 4) == ".bmh") {
             filename = filename.substr(0, filename.length() - 4);
         }
@@ -688,15 +693,18 @@ string MdOut::linkRef(const string& leadingSpaces, const Definition* def,
         while (start > 0 && (isalnum(filename[start - 1]) || '_' == filename[start - 1])) {
             --start;
         }
-        buildup = filename.substr(start) + "#" + (classMatch ? namePart : *str);
+        buildup = filename.substr(start);
     }
+    buildup += "#" + refName;
     if (MarkType::kParam == def->fMarkType) {
         const Definition* parent = def->fParent;
         SkASSERT(MarkType::kMethod == parent->fMarkType);
         buildup = '#' + parent->fFiddle + '_' + ref;
     }
     string refOut(ref);
-    std::replace(refOut.begin(), refOut.end(), '_', ' ');
+    if (!globalEnumMember) {
+        std::replace(refOut.begin(), refOut.end(), '_', ' ');
+    }
     if (ref.length() > 2 && islower(ref[0]) && "()" == ref.substr(ref.length() - 2)) {
         refOut = refOut.substr(0, refOut.length() - 2);
     }
@@ -719,7 +727,6 @@ string MdOut::linkRef(const string& leadingSpaces, const Definition* def,
 					found = true;
 				}
 			}
-
 		}
 		if (!found) {
 			SkDebugf("");  // convenient place to set a breakpoint