diff options
author | Topi Reinio <topi.reinio@qt.io> | 2021-11-02 22:22:25 +0100 |
---|---|---|
committer | Qt Cherry-pick Bot <cherrypick_bot@qt-project.org> | 2021-11-03 12:17:28 +0000 |
commit | 57438d581f0e36c8a9e0196a490f5762b7eaf005 (patch) | |
tree | 4f91b1fc25d7479091b8b02b312ffec1567683b1 | |
parent | 23f3724b06fc155e5062612d300b8339c7725578 (diff) | |
download | qttools-57438d581f0e36c8a9e0196a490f5762b7eaf005.tar.gz |
qdoc: Improve link text formatting
QDoc attempts to style links to C++ functions in a specific way,
by moving function arguments and the enclosing parentheses
out of the link. This was done in a way that had unintended
consequences for all link texts that included an opening
parenthesis, whether the link target was a function or not.
As a fix;
* Increase the specificity of the regular expression
used for identifying a function-like link text.
* Store the Node we're linking to in generator, so
we can match the regular expression only to function
targets, not all links. This is also a performance
improvement.
* DocParser: Stop generating AutoLink atoms inside an
open (explicit) link. This was useless as it split
the input into multiple nested link->autolink atoms,
and the nested ones are discarded/treated as strings
in the generator anyway.
* Refactor related generator functions and remove
obsolete debugging code.
Task-number: QTBUG-97949
Change-Id: I89429a73f13aa9995a79dfd24a8e473c620d6bc6
Done-with: Luca Di Sera <luca.disera@qt.io>
Reviewed-by: Luca Di Sera <luca.disera@qt.io>
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
(cherry picked from commit b3e367def6ff0f79441507e839c6cc874d4a25fd)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
15 files changed, 181 insertions, 56 deletions
diff --git a/src/qdoc/docbookgenerator.cpp b/src/qdoc/docbookgenerator.cpp index 2318cb963..242338c01 100644 --- a/src/qdoc/docbookgenerator.cpp +++ b/src/qdoc/docbookgenerator.cpp @@ -941,19 +941,21 @@ void DocBookGenerator::generateClassHierarchy(const Node *relative, NodeMultiMap void DocBookGenerator::generateLink(const Atom *atom) { + Q_ASSERT(m_inLink); + // From HtmlGenerator::generateLink. - QRegularExpression funcLeftParen("\\S(\\()"); - auto match = funcLeftParen.match(atom->string()); - if (match.hasMatch()) { - // hack for C++: move () outside of link - qsizetype k = match.capturedStart(1); - m_writer->writeCharacters(atom->string().left(k)); - m_writer->writeEndElement(); // link - m_inLink = false; - m_writer->writeCharacters(atom->string().mid(k)); - } else { - m_writer->writeCharacters(atom->string()); + if (m_linkNode && m_linkNode->isFunction()) { + auto match = XmlGenerator::m_funcLeftParen.match(atom->string()); + if (match.hasMatch()) { + // C++: move () outside of link + qsizetype leftParenLoc = match.capturedStart(1); + m_writer->writeCharacters(atom->string().left(leftParenLoc)); + endLink(); + m_writer->writeCharacters(atom->string().mid(leftParenLoc)); + return; + } } + m_writer->writeCharacters(atom->string()); } /*! @@ -969,6 +971,7 @@ void DocBookGenerator::beginLink(const QString &link, const Node *node, const No && node->isDeprecated()) m_writer->writeAttribute("role", "deprecated"); m_inLink = true; + m_linkNode = node; } void DocBookGenerator::endLink() @@ -977,6 +980,7 @@ void DocBookGenerator::endLink() if (m_inLink) m_writer->writeEndElement(); // link m_inLink = false; + m_linkNode = nullptr; } void DocBookGenerator::generateList(const Node *relative, const QString &selector) diff --git a/src/qdoc/docparser.cpp b/src/qdoc/docparser.cpp index 1294f5d7a..43e3a1d30 100644 --- a/src/qdoc/docparser.cpp +++ b/src/qdoc/docparser.cpp @@ -1255,7 +1255,10 @@ void DocParser::parse(const QString &source, DocPrivate *docPrivate, if (newWord) { qsizetype startPos = m_position; - bool autolink = isAutoLinkString(m_input, m_position); + // No auto-linking inside links + bool autolink = (!m_pendingFormats.isEmpty() && + m_pendingFormats.last() == ATOM_FORMATTING_LINK) ? + false : isAutoLinkString(m_input, m_position); if (m_position == startPos) { if (!ch.isSpace()) { appendChar(ch); diff --git a/src/qdoc/htmlgenerator.cpp b/src/qdoc/htmlgenerator.cpp index ccde100f8..d4c4dabcb 100644 --- a/src/qdoc/htmlgenerator.cpp +++ b/src/qdoc/htmlgenerator.cpp @@ -58,7 +58,6 @@ QT_BEGIN_NAMESPACE -static bool showBrokenLinks = false; bool HtmlGenerator::s_inUnorderedList { false }; static void addLink(const QString &linkTarget, QStringView nestedStuff, QString *res) @@ -338,7 +337,7 @@ qsizetype HtmlGenerator::generateAtom(const Atom *atom, const Node *relative, Co out() << protectEnc(atom->string()); } else { beginLink(link, node, relative); - generateLink(atom, marker); + generateLink(atom); endLink(); } } else { @@ -679,10 +678,9 @@ qsizetype HtmlGenerator::generateAtom(const Atom *atom, const Node *relative, Co if (link.isEmpty() && (node != relative) && !noLinkErrors()) { relative->doc().location().warning( QStringLiteral("Can't link to '%1'").arg(atom->string())); - } else { - node = nullptr; } - beginLink(link, node, relative); + beginLink(link, nullptr, relative); + m_linkNode = node; skipAhead = 1; } break; case Atom::ExampleFileLink: { @@ -863,7 +861,7 @@ qsizetype HtmlGenerator::generateAtom(const Atom *atom, const Node *relative, Co break; case Atom::String: if (m_inLink && !m_inContents && !m_inSectionHeading) { - generateLink(atom, marker); + generateLink(atom); } else { out() << protectEnc(atom->string()); } @@ -3238,24 +3236,22 @@ QString HtmlGenerator::highlightedCode(const QString &markedCode, const Node *re return html; } -void HtmlGenerator::generateLink(const Atom *atom, CodeMarker *marker) +void HtmlGenerator::generateLink(const Atom *atom) { - auto match = m_funcLeftParen.match(atom->string()); - if (match.hasMatch() && marker->recognizeLanguage("Cpp")) { - // hack for C++: move () outside of link - qsizetype k = match.capturedStart(1); - out() << protectEnc(atom->string().left(k)); - if (m_link.isEmpty()) { - if (showBrokenLinks) - out() << "</i>"; - } else { - out() << "</a>"; + Q_ASSERT(m_inLink); + + if (m_linkNode && m_linkNode->isFunction()) { + auto match = XmlGenerator::m_funcLeftParen.match(atom->string()); + if (match.hasMatch()) { + // C++: move () outside of link + qsizetype leftParenLoc = match.capturedStart(1); + out() << protectEnc(atom->string().left(leftParenLoc)); + endLink(); + out() << protectEnc(atom->string().mid(leftParenLoc)); + return; } - m_inLink = false; - out() << protectEnc(atom->string().mid(k)); - } else { - out() << protectEnc(atom->string()); } + out() << protectEnc(atom->string()); } QString HtmlGenerator::protectEnc(const QString &string) @@ -3422,39 +3418,39 @@ void HtmlGenerator::generateDetailedMember(const Node *node, const PageNode *rel void HtmlGenerator::beginLink(const QString &link) { m_link = link; - if (m_link.isEmpty()) { - if (showBrokenLinks) - out() << "<i>"; - } - out() << "<a href=\"" << m_link << "\">"; m_inLink = true; + m_linkNode = nullptr; + + if (!m_link.isEmpty()) + out() << "<a href=\"" << m_link << "\">"; } void HtmlGenerator::beginLink(const QString &link, const Node *node, const Node *relative) { m_link = link; - if (m_link.isEmpty()) { - if (showBrokenLinks) - out() << "<i>"; - } else if (node == nullptr || (relative != nullptr && node->status() == relative->status())) + m_inLink = true; + m_linkNode = node; + if (m_link.isEmpty()) + return; + + if (node == nullptr || (relative != nullptr && node->status() == relative->status())) out() << "<a href=\"" << m_link << "\">"; else if (node->isDeprecated()) out() << "<a href=\"" << m_link << "\" class=\"obsolete\">"; else out() << "<a href=\"" << m_link << "\">"; - m_inLink = true; } void HtmlGenerator::endLink() { - if (m_inLink) { - if (m_link.isEmpty() && showBrokenLinks) { - out() << "</i>"; - } else { - out() << "</a>"; - } - } + if (!m_inLink) + return; + m_inLink = false; + m_linkNode = nullptr; + + if (!m_link.isEmpty()) + out() << "</a>"; } /*! diff --git a/src/qdoc/htmlgenerator.h b/src/qdoc/htmlgenerator.h index 2b86de692..19989747a 100644 --- a/src/qdoc/htmlgenerator.h +++ b/src/qdoc/htmlgenerator.h @@ -119,7 +119,7 @@ private: void generateFullName(const Node *apparentNode, const Node *relative, const Node *actualNode = nullptr); void generateDetailedMember(const Node *node, const PageNode *relative, CodeMarker *marker); - void generateLink(const Atom *atom, CodeMarker *marker); + void generateLink(const Atom *atom); QString fileBase(const Node *node) const override; QString fileName(const Node *node); @@ -156,7 +156,6 @@ private: QString m_codeSuffix {}; HelpProjectWriter *m_helpProjectWriter { nullptr }; ManifestWriter *m_manifestWriter { nullptr }; - QRegularExpression m_funcLeftParen { "\\S(\\()" }; QString m_headerScripts {}; QString m_headerStyles {}; QString m_endHeader {}; diff --git a/src/qdoc/webxmlgenerator.h b/src/qdoc/webxmlgenerator.h index b5119891b..1efb93922 100644 --- a/src/qdoc/webxmlgenerator.h +++ b/src/qdoc/webxmlgenerator.h @@ -74,8 +74,6 @@ private: void endLink(QXmlStreamWriter &writer); QString fileBase(const Node *node) const override; - bool m_inLink { false }; - bool m_inSectionHeading { false }; bool m_hasQuotingInformation { false }; QString quoteCommand {}; QScopedPointer<QXmlStreamWriter> currentWriter {}; diff --git a/src/qdoc/xmlgenerator.cpp b/src/qdoc/xmlgenerator.cpp index a83a25143..f24264140 100644 --- a/src/qdoc/xmlgenerator.cpp +++ b/src/qdoc/xmlgenerator.cpp @@ -37,6 +37,8 @@ QT_BEGIN_NAMESPACE +const QRegularExpression XmlGenerator::m_funcLeftParen(QStringLiteral("^\\S+(\\(.*\\))$")); + /*! Do not display \brief for QML/JS types, document and collection nodes */ diff --git a/src/qdoc/xmlgenerator.h b/src/qdoc/xmlgenerator.h index c5dfdc33f..e3235cf51 100644 --- a/src/qdoc/xmlgenerator.h +++ b/src/qdoc/xmlgenerator.h @@ -66,6 +66,10 @@ protected: QPair<QString, QString> anchorForNode(const Node *node); static QString targetType(const Node *node); + +protected: + static const QRegularExpression m_funcLeftParen; + const Node *m_linkNode { nullptr }; }; QT_END_NAMESPACE diff --git a/tests/auto/qdoc/generatedoutput/expected_output/docbook/testcpp-module.xml b/tests/auto/qdoc/generatedoutput/expected_output/docbook/testcpp-module.xml index dc261f34e..01e0af163 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/docbook/testcpp-module.xml +++ b/tests/auto/qdoc/generatedoutput/expected_output/docbook/testcpp-module.xml @@ -48,5 +48,34 @@ <db:note> <db:para>This is just a test.</db:para> </db:note> +<db:section xml:id="linking-to-function-like-things"> +<db:title>Linking to function-like things</db:title> +<db:itemizedlist> +<db:listitem> +<db:para><db:link xlink:href="testqdoc-test.xml#someFunctionDefaultArg">someFunctionDefaultArg</db:link>()</db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="testqdoc-test.xml#QDOCTEST_MACRO2">QDOCTEST_MACRO2</db:link>()</db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="testqdoc-test.xml#QDOCTEST_MACRO2">QDOCTEST_MACRO2</db:link>(int &x)</db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="testcpp-module.xml#section">section()</db:link></db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="testcpp-module.xml#section">section() is a section title</db:link></db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="testqdoc-test.xml#Test">open( parenthesis</db:link></db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="https://en.cppreference.com/w/cpp/utility/move">C++11 added std::move(T&& t)</db:link></db:para> +</db:listitem> +</db:itemizedlist> +<db:section xml:id="section"> +<db:title>section()</db:title> +</db:section> +</db:section> </db:section> </db:article> diff --git a/tests/auto/qdoc/generatedoutput/expected_output/nestedmacro/testcpp-module.html b/tests/auto/qdoc/generatedoutput/expected_output/nestedmacro/testcpp-module.html index 4bc14d345..6b5049e06 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/nestedmacro/testcpp-module.html +++ b/tests/auto/qdoc/generatedoutput/expected_output/nestedmacro/testcpp-module.html @@ -13,6 +13,8 @@ <li class="level1"><a href="#namespaces">Namespaces</a></li> <li class="level1"><a href="#classes">Classes</a></li> <li class="level1"><a href="#details">Detailed Description</a></li> +<li class="level2"><a href="#linking-to-function-like-things">Linking to function-like things</a></li> +<li class="level3"><a href="#section">section()</a></li> </ul> </div> <div class="sidebar-content" id="sidebar-content"></div></div> @@ -37,6 +39,17 @@ <p><b>Note: </b>This is just a test.</p> </div> <p><b>This module was introduced in version 5.15.</b></p> +<h3 id="linking-to-function-like-things">Linking to function-like things</h3> +<ul> +<li><a href="testqdoc-test.html#someFunctionDefaultArg">someFunctionDefaultArg</a>()</li> +<li><a href="testqdoc-test.html#QDOCTEST_MACRO2">QDOCTEST_MACRO2</a>()</li> +<li><a href="testqdoc-test.html#QDOCTEST_MACRO2">QDOCTEST_MACRO2</a>(int &x)</li> +<li><a href="testcpp-module.html#section">section()</a></li> +<li><a href="testcpp-module.html#section">section() is a section title</a></li> +<li><a href="testqdoc-test.html#Test">open( parenthesis</a></li> +<li><a href="https://en.cppreference.com/w/cpp/utility/move">C++11 added std::move(T&& t)</a></li> +</ul> +<h4 id="section">section()</h4> </div> <!-- @@@TestCPP --> </body> diff --git a/tests/auto/qdoc/generatedoutput/expected_output/noautolist-docbook/testcpp-module.xml b/tests/auto/qdoc/generatedoutput/expected_output/noautolist-docbook/testcpp-module.xml index 33ceec53c..b4f50a8c7 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/noautolist-docbook/testcpp-module.xml +++ b/tests/auto/qdoc/generatedoutput/expected_output/noautolist-docbook/testcpp-module.xml @@ -14,5 +14,34 @@ <db:note> <db:para>This is just a test.</db:para> </db:note> +<db:section xml:id="linking-to-function-like-things"> +<db:title>Linking to function-like things</db:title> +<db:itemizedlist> +<db:listitem> +<db:para><db:link xlink:href="testqdoc-test.xml#someFunctionDefaultArg">someFunctionDefaultArg</db:link>()</db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="testqdoc-test.xml#QDOCTEST_MACRO2">QDOCTEST_MACRO2</db:link>()</db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="testqdoc-test.xml#QDOCTEST_MACRO2">QDOCTEST_MACRO2</db:link>(int &x)</db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="testcpp-module.xml#section">section()</db:link></db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="testcpp-module.xml#section">section() is a section title</db:link></db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="testqdoc-test.xml#Test">open( parenthesis</db:link></db:para> +</db:listitem> +<db:listitem> +<db:para><db:link xlink:href="https://en.cppreference.com/w/cpp/utility/move">C++11 added std::move(T&& t)</db:link></db:para> +</db:listitem> +</db:itemizedlist> +<db:section xml:id="section"> +<db:title>section()</db:title> +</db:section> +</db:section> </db:section> </db:article> diff --git a/tests/auto/qdoc/generatedoutput/expected_output/noautolist/testcpp-module.html b/tests/auto/qdoc/generatedoutput/expected_output/noautolist/testcpp-module.html index 6db15ca22..e31353a94 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/noautolist/testcpp-module.html +++ b/tests/auto/qdoc/generatedoutput/expected_output/noautolist/testcpp-module.html @@ -11,6 +11,8 @@ <h3 id="toc">Contents</h3> <ul> <li class="level1"><a href="#details">Detailed Description</a></li> +<li class="level2"><a href="#linking-to-function-like-things">Linking to function-like things</a></li> +<li class="level3"><a href="#section">section()</a></li> </ul> </div> <div class="sidebar-content" id="sidebar-content"></div></div> @@ -24,6 +26,17 @@ <div class="admonition note"> <p><b>Note: </b>This is just a test.</p> </div> +<h3 id="linking-to-function-like-things">Linking to function-like things</h3> +<ul> +<li><a href="testqdoc-test.html#someFunctionDefaultArg">someFunctionDefaultArg</a>()</li> +<li><a href="testqdoc-test.html#QDOCTEST_MACRO2">QDOCTEST_MACRO2</a>()</li> +<li><a href="testqdoc-test.html#QDOCTEST_MACRO2">QDOCTEST_MACRO2</a>(int &x)</li> +<li><a href="testcpp-module.html#section">section()</a></li> +<li><a href="testcpp-module.html#section">section() is a section title</a></li> +<li><a href="testqdoc-test.html#Test">open( parenthesis</a></li> +<li><a href="https://en.cppreference.com/w/cpp/utility/move">C++11 added std::move(T&& t)</a></li> +</ul> +<h4 id="section">section()</h4> </div> <!-- @@@TestCPP --> </body> diff --git a/tests/auto/qdoc/generatedoutput/expected_output/properties/testcpp.index b/tests/auto/qdoc/generatedoutput/expected_output/properties/testcpp.index index 62da1db3a..4ac50b93e 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/properties/testcpp.index +++ b/tests/auto/qdoc/generatedoutput/expected_output/properties/testcpp.index @@ -102,6 +102,9 @@ </class> </namespace> <group name="testgroup" href="testgroup.html" status="internal" seen="false" title=""/> - <module name="TestCPP" href="testcpp-module.html" status="active" documented="true" seen="true" title="QDoc Test C++ Classes" brief="A test module page"/> + <module name="TestCPP" href="testcpp-module.html" status="active" documented="true" seen="true" title="QDoc Test C++ Classes" brief="A test module page"> + <contents name="linking-to-function-like-things" title="Linking to function-like things" level="1"/> + <contents name="section" title="section()" level="2"/> + </module> </namespace> </INDEX> diff --git a/tests/auto/qdoc/generatedoutput/expected_output/testcpp-module.html b/tests/auto/qdoc/generatedoutput/expected_output/testcpp-module.html index 04bd8e487..1ad20920d 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/testcpp-module.html +++ b/tests/auto/qdoc/generatedoutput/expected_output/testcpp-module.html @@ -13,6 +13,8 @@ <li class="level1"><a href="#namespaces">Namespaces</a></li> <li class="level1"><a href="#classes">Classes</a></li> <li class="level1"><a href="#details">Detailed Description</a></li> +<li class="level2"><a href="#linking-to-function-like-things">Linking to function-like things</a></li> +<li class="level3"><a href="#section">section()</a></li> </ul> </div> <div class="sidebar-content" id="sidebar-content"></div></div> @@ -36,6 +38,17 @@ <div class="admonition note"> <p><b>Note: </b>This is just a test.</p> </div> +<h3 id="linking-to-function-like-things">Linking to function-like things</h3> +<ul> +<li><a href="testqdoc-test.html#someFunctionDefaultArg">someFunctionDefaultArg</a>()</li> +<li><a href="testqdoc-test.html#QDOCTEST_MACRO2">QDOCTEST_MACRO2</a>()</li> +<li><a href="testqdoc-test.html#QDOCTEST_MACRO2">QDOCTEST_MACRO2</a>(int &x)</li> +<li><a href="testcpp-module.html#section">section()</a></li> +<li><a href="testcpp-module.html#section">section() is a section title</a></li> +<li><a href="testqdoc-test.html#Test">open( parenthesis</a></li> +<li><a href="https://en.cppreference.com/w/cpp/utility/move">C++11 added std::move(T&& t)</a></li> +</ul> +<h4 id="section">section()</h4> </div> <!-- @@@TestCPP --> </body> diff --git a/tests/auto/qdoc/generatedoutput/expected_output/testcpp.index b/tests/auto/qdoc/generatedoutput/expected_output/testcpp.index index 34d36dd89..40445599f 100644 --- a/tests/auto/qdoc/generatedoutput/expected_output/testcpp.index +++ b/tests/auto/qdoc/generatedoutput/expected_output/testcpp.index @@ -65,6 +65,9 @@ </class> </namespace> <group name="testgroup" href="testgroup.html" status="internal" seen="false" title=""/> - <module name="TestCPP" href="testcpp-module.html" status="active" documented="true" seen="true" title="QDoc Test C++ Classes" brief="A test module page"/> + <module name="TestCPP" href="testcpp-module.html" status="active" documented="true" seen="true" title="QDoc Test C++ Classes" brief="A test module page"> + <contents name="linking-to-function-like-things" title="Linking to function-like things" level="1"/> + <contents name="section" title="section()" level="2"/> + </module> </namespace> </INDEX> diff --git a/tests/auto/qdoc/generatedoutput/testdata/testcpp/testcpp.cpp b/tests/auto/qdoc/generatedoutput/testdata/testcpp/testcpp.cpp index ef1666c90..dfef701d9 100644 --- a/tests/auto/qdoc/generatedoutput/testdata/testcpp/testcpp.cpp +++ b/tests/auto/qdoc/generatedoutput/testdata/testcpp/testcpp.cpp @@ -49,6 +49,22 @@ namespace TestQDoc { \if defined(test_nestedmacro) \versionnote {module} {\ver} \endif + + \section1 Linking to function-like things + + \list + \li \l {TestQDoc::Test::someFunctionDefaultArg} + {someFunctionDefaultArg()} + \li QDOCTEST_MACRO2() + \li \l {TestQDoc::Test::}{QDOCTEST_MACRO2(int &x)} + \li \l {section()} + \li \l {section()} {section() is a section title} + \li \l {TestQDoc::Test::Test()} {open( parenthesis} + \li \l {https://en.cppreference.com/w/cpp/utility/move} + {C++11 added std::move(T&& t)} + \endlist + + \section2 section() */ /*! |