From 78de7519692ea93a2d2c70f8c0e773668df49fce Mon Sep 17 00:00:00 2001 From: Cary Clark Date: Wed, 7 Feb 2018 07:27:09 -0500 Subject: add subtopics to all methods add self-check looking for #In markup on every method, pointing to an existing #Subtopic to reference the method. Docs-Preview: https://skia.org/?cl=104325 Bug: skia:6898 Change-Id: I749a25b9a43033ae68d193249b2c0b810dcf8fc8 Reviewed-on: https://skia-review.googlesource.com/104325 Commit-Queue: Cary Clark Reviewed-by: Cary Clark --- tools/bookmaker/bookmaker.h | 15 ++++-- tools/bookmaker/definition.cpp | 82 +++++++++++++++++++++++++++-- tools/bookmaker/mdOut.cpp | 32 +++++++---- tools/bookmaker/selfCheck.cpp | 117 ++++++++++++++++++++++++++--------------- tools/bookmaker/spellCheck.cpp | 1 - 5 files changed, 183 insertions(+), 64 deletions(-) (limited to 'tools/bookmaker') diff --git a/tools/bookmaker/bookmaker.h b/tools/bookmaker/bookmaker.h index 96372756a1..d1b6382320 100644 --- a/tools/bookmaker/bookmaker.h +++ b/tools/bookmaker/bookmaker.h @@ -767,6 +767,11 @@ public: kSubtractFrom, }; + enum class Format { + kIncludeReturn, + kOmitReturn, + }; + Definition() {} Definition(const char* start, const char* end, int line, Definition* parent) @@ -843,7 +848,7 @@ public: string extractText(TrimExtract trimExtract) const; string fiddleName() const; const Definition* findClone(string match) const; - string formatFunction() const; + string formatFunction(Format format) const; const Definition* hasChild(MarkType markType) const; bool hasMatch(const string& name) const; const Definition* hasParam(const string& ref) const; @@ -1181,11 +1186,11 @@ public: }; enum class Resolvable { - kNo, // neither resolved nor output - kYes, // resolved, output - kOut, // not resolved, but output + kNo, // neither resolved nor output + kYes, // resolved, output + kOut, // not resolved, but output kLiteral, // output untouched (FIXME: is this really different from kOut?) - kClone, // resolved, output, with references to clones as well + kClone, // resolved, output, with references to clones as well }; enum class Exemplary { diff --git a/tools/bookmaker/definition.cpp b/tools/bookmaker/definition.cpp index 757169b1a9..25d154386d 100644 --- a/tools/bookmaker/definition.cpp +++ b/tools/bookmaker/definition.cpp @@ -897,7 +897,7 @@ bool Definition::crossCheckInside(const char* start, const char* end, return false; } -string Definition::formatFunction() const { +string Definition::formatFunction(Format format) const { const char* end = fContentStart; while (end > fStart && ' ' >= end[-1]) { --end; @@ -913,6 +913,9 @@ string Definition::formatFunction() const { const char* nameInParser = methodParser.strnstr(name.c_str(), methodParser.fEnd); methodParser.skipTo(nameInParser); const char* lastEnd = methodParser.fChar; + if (Format::kOmitReturn == format) { + lastStart = lastEnd; + } const char* paren = methodParser.strnchr('(', methodParser.fEnd); size_t indent; if (paren) { @@ -983,8 +986,10 @@ string Definition::formatFunction() const { if (delimiter) { if (nextEnd - nextStart >= (ptrdiff_t) (limit - written)) { written = indent; - methodStr += '\n'; - methodStr += string(indent, ' '); + if (Format::kIncludeReturn == format) { + methodStr += '\n'; + methodStr += string(indent, ' '); + } } methodParser.skipTo(delimiter); } @@ -1214,7 +1219,76 @@ string Definition::NormalizedName(string name) { return normalizedName; } -bool Definition::paramsMatch(const string& match, const string& name) const { +static string unpreformat(const string& orig) { + string result; + int amp = 0; + for (auto c : orig) { + switch (amp) { + case 0: + if ('&' == c) { + amp = 1; + } else { + amp = 0; + result += c; + } + break; + case 1: + if ('l' == c) { + amp = 2; + } else if ('g' == c) { + amp = 3; + } else { + amp = 0; + result += "&"; + result += c; + } + break; + case 2: + if ('t' == c) { + amp = 4; + } else { + amp = 0; + result += "&l"; + result += c; + } + break; + case 3: + if ('t' == c) { + amp = 5; + } else { + amp = 0; + result += "&g"; + result += c; + } + break; + case 4: + if (';' == c) { + result += '<'; + } else { + result += "<"; + result += c; + } + amp = 0; + break; + case 5: + if (';' == c) { + result += '>'; + } else { + result += ">"; + result += c; + } + amp = 0; + break; + } + } + return result; +} + +bool Definition::paramsMatch(const string& matchFormatted, const string& name) const { + if (string::npos != matchFormatted.find("readPixels")) { + SkDebugf(""); + } + string match = unpreformat(matchFormatted); TextParser def(fFileName, fStart, fContentStart, fLineCount); const char* dName = def.strnstr(name.c_str(), fContentStart); if (!dName) { diff --git a/tools/bookmaker/mdOut.cpp b/tools/bookmaker/mdOut.cpp index 7bc70af669..31f8ef4f6f 100644 --- a/tools/bookmaker/mdOut.cpp +++ b/tools/bookmaker/mdOut.cpp @@ -357,6 +357,7 @@ bool MdOut::buildRefFromFile(const char* name, const char* outDir) { this->lfAlways(1); FPRINTF("==="); } + fPopulators.clear(); fPopulators[kClassesAndStructs].fDescription = "embedded struct and class members"; fPopulators[kConstants].fDescription = "enum and enum class, const values"; fPopulators[kConstructors].fDescription = "functions that construct"; @@ -907,7 +908,7 @@ void MdOut::markTypeOut(Definition* def) { } break; case MarkType::kMethod: { string method_name = def->methodName(); - string formattedStr = def->formatFunction(); + string formattedStr = def->formatFunction(Definition::Format::kIncludeReturn); this->lfAlways(2); FPRINTF("", def->fFiddle.c_str()); @@ -972,6 +973,9 @@ void MdOut::markTypeOut(Definition* def) { case MarkType::kPopulate: { SkASSERT(MarkType::kSubtopic == def->fParent->fMarkType); string name = def->fParent->fName; + if ("Bitmap_Related_Function" == def->fParent->fFiddle) { + SkDebugf(""); + } if (kSubtopics == name) { this->subtopicsOut(); } else { @@ -1183,13 +1187,20 @@ void MdOut::mdHeaderOutLF(int depth, int lf) { void MdOut::populateTables(const Definition* def) { const Definition* csParent = this->csParent(); + if (!csParent) { + return; + } for (auto child : def->fChildren) { - if (string::npos != child->fName.find("Rect_Set")) { + if (string::npos != child->fFiddle.find("Bitmap_Set")) { SkDebugf(""); } if (MarkType::kTopic == child->fMarkType || MarkType::kSubtopic == child->fMarkType) { - bool legacyTopic = fPopulators.end() != fPopulators.find(child->fName); - if (!legacyTopic && child->fName != kOverview) { + string name = child->fName; + bool builtInTopic = name == kClassesAndStructs || name == kConstants + || name == kConstructors || name == kMemberFunctions || name == kMembers + || name == kOperators || name == kOverview || name == kRelatedFunctions + || name == kSubtopics; + if (!builtInTopic && child->fName != kOverview) { this->populator(kRelatedFunctions).fMembers.push_back(child); } this->populateTables(child); @@ -1370,6 +1381,9 @@ void MdOut::subtopicOut(const TableContents& tableContents) { items[name] = entry; } for (auto entry : items) { + if (string::npos != entry.second->fName.find("SkRect::set")) { + SkDebugf(""); + } if (entry.second->fDeprecated) { continue; } @@ -1380,15 +1394,9 @@ void MdOut::subtopicOut(const TableContents& tableContents) { break; } } - if (!oneLiner) { - SkDebugf(""); - } SkASSERT(oneLiner); this->rowOut(entry.first.c_str(), string(oneLiner->fContentStart, oneLiner->fContentEnd - oneLiner->fContentStart)); - if (string::npos != entry.second->fName.find("SkRect::set")) { - SkDebugf(""); - } if (tableContents.fShowClones && entry.second->fCloned) { int cloneNo = 2; string builder = entry.second->fName; @@ -1396,13 +1404,15 @@ void MdOut::subtopicOut(const TableContents& tableContents) { builder = builder.substr(0, builder.length() - 2); } builder += '_'; + this->rowOut("", + preformat(entry.second->formatFunction(Definition::Format::kOmitReturn))); do { string match = builder + to_string(cloneNo); auto child = csParent->findClone(match); if (!child) { break; } - this->rowOut("", child->methodName()); + this->rowOut("", preformat(child->formatFunction(Definition::Format::kOmitReturn))); } while (++cloneNo); } } diff --git a/tools/bookmaker/selfCheck.cpp b/tools/bookmaker/selfCheck.cpp index 3392945a9f..5f1eb38ece 100644 --- a/tools/bookmaker/selfCheck.cpp +++ b/tools/bookmaker/selfCheck.cpp @@ -11,6 +11,27 @@ #include #endif + + /* SkDebugf works in both visual studio and git shell, but + in git shell output is not piped to grep. + printf does not generate output in visual studio, but + does in git shell and can be piped. + */ +#ifdef SK_BUILD_FOR_WIN +#define PRINTF(...) \ +do { \ + if (IsDebuggerPresent()) { \ + SkDebugf(__VA_ARGS__); \ + } else { \ + printf(__VA_ARGS__); \ + } \ +} while (false) +#else +#define PRINTF(...) \ + printf(__VA_ARGS__) +#endif + + // Check that mutiple like-named methods are under one Subtopic // Check that SeeAlso reference each other @@ -47,52 +68,62 @@ public: protected: - bool checkRelatedFunctions() { - const Definition* cs = this->classOrStruct(); - vector methodNames; - if (cs) { - string prefix = cs->fName + "::"; - for (auto& csChild : cs->fChildren) { - if (MarkType::kMethod != csChild->fMarkType) { - // only check methods for now - continue; - } - if (Definition::MethodType::kConstructor == csChild->fMethodType) { - continue; - } - if (Definition::MethodType::kDestructor == csChild->fMethodType) { - continue; - } - if (Definition::MethodType::kOperator == csChild->fMethodType) { - continue; - } - if (csChild->fClone) { - // FIXME: check to see if all cloned methods are in table - // since format of clones is in flux, defer this check for now - continue; - } - bool containsMarkTypeIn = csChild->fDeprecated; // no markup for deprecated - for (auto child : csChild->fChildren) { - if (MarkType::kIn == child->fMarkType) { - containsMarkTypeIn = true; - break; + void checkMethod(string topic, const Definition* csChild, vector* reported) { + if (MarkType::kSubtopic == csChild->fMarkType) { + for (auto child : csChild->fChildren) { + checkMethod(topic, child, reported); + } + return; + } else if (MarkType::kMethod != csChild->fMarkType) { + // only check methods for now + return; + } + bool containsMarkTypeIn = csChild->fDeprecated // no markup for deprecated + || Definition::MethodType::kConstructor == csChild->fMethodType + || Definition::MethodType::kDestructor == csChild->fMethodType + || Definition::MethodType::kOperator == csChild->fMethodType + || csChild->fClone; + for (auto child : csChild->fChildren) { + if (MarkType::kIn == child->fMarkType) { + containsMarkTypeIn = true; + string subtopic(child->fContentStart, + child->fContentEnd - child->fContentStart); + string fullname = topic + '_' + subtopic; + auto topEnd = fBmhParser.fTopicMap.end(); + auto topFind = fBmhParser.fTopicMap.find(fullname); + auto reportEnd = reported->end(); + auto reportFind = std::find(reported->begin(), reported->end(), subtopic); + if (topEnd == topFind) { + if (reportEnd == reportFind) { + reported->push_back(subtopic); } } - if (!containsMarkTypeIn) { -#ifdef SK_BUILD_FOR_WIN - /* SkDebugf works in both visual studio and git shell, but - in git shell output is not piped to grep. - printf does not generate output in visual studio, but - does in git shell and can be piped. - */ - if (IsDebuggerPresent()) { - SkDebugf("No #In: %s\n", csChild->fName.c_str()); - } else -#endif - printf("No #In: %s\n", csChild->fName.c_str()); - } - } + } + } + if (!containsMarkTypeIn) { + PRINTF("No #In: %s\n", csChild->fName.c_str()); + } + } + + bool checkRelatedFunctions() { + const Definition* cs = this->classOrStruct(); + if (!cs) { + return true; + } + const Definition* topic = cs->fParent; + SkASSERT(topic); + SkASSERT(MarkType::kTopic == topic->fMarkType); + string topicName = topic->fName; + vector methodNames; + vector reported; + string prefix = cs->fName + "::"; + for (auto& csChild : cs->fChildren) { + checkMethod(topicName, csChild, &reported); } + for (auto missing : reported) { + string fullname = topicName + '_' + missing; + PRINTF("No #Subtopic: %s\n", fullname.c_str()); + } return true; } diff --git a/tools/bookmaker/spellCheck.cpp b/tools/bookmaker/spellCheck.cpp index efc79ca528..bafc969ef4 100644 --- a/tools/bookmaker/spellCheck.cpp +++ b/tools/bookmaker/spellCheck.cpp @@ -214,7 +214,6 @@ bool SpellCheck::check(Definition* def) { if (all_lower(method_name)) { method_name += "()"; } - string formattedStr = def->formatFunction(); if (!def->isClone() && Definition::MethodType::kOperator != def->fMethodType) { this->wordCheck(method_name); } -- cgit v1.2.3