Skip to content

Commit ba47743

Browse files
paulwickingQt Cherry-pick Bot
paulwicking
authored and
Qt Cherry-pick Bot
committed
QDoc: Group property role notes in output
When a signal (or other role) applies to multiple properties, QDoc repeats the same “<Role> for property X.” line for each property. This creates noisy, hard-to-scan documentation. This change modifies `Generator::generateAddendum()` such that it groups properties by `FunctionRole`. This allows QDoc to emit a single admonition per role, listing all applicable properties. For example: Notifier signal for properties Foo, Bar, and Baz. Properties are listed alphabetically; roles follow enum order (Getter, Setter, Resetter, Notifier, Bindable). This ensures deterministic output. There's no change to semantics or links, as this is a wording/format improvement only. The change also includes a validate-output test (HTML/DocBook/WebXML) that demonstrates the previous repetition and the new grouped output. Fixes: QTBUG-86490 Pick-to: 6.9 6.8 Change-Id: I5221c25f8040cc617dd2ec595f58c95d16120ac7 Reviewed-by: Topi Reiniö <[email protected]> (cherry picked from commit 74711c4) Reviewed-by: Qt Cherry-pick Bot <[email protected]>
1 parent 30cdb81 commit ba47743

File tree

14 files changed

+874
-18
lines changed

14 files changed

+874
-18
lines changed

‎src/qdoc/qdoc/src/qdoc/docbookgenerator.cpp‎

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4062,14 +4062,37 @@ void DocBookGenerator::generateAddendum(const Node *node, Addendum type, CodeMar
40624062
if (!node->isFunction())
40634063
return;
40644064
const auto *fn = static_cast<const FunctionNode *>(node);
4065-
auto propertyNodes = fn->associatedProperties();
4066-
if (propertyNodes.isEmpty())
4065+
auto nodes = fn->associatedProperties();
4066+
if (nodes.isEmpty())
40674067
return;
4068-
std::sort(propertyNodes.begin(), propertyNodes.end(), Node::nodeNameLessThan);
4069-
for (const auto propertyNode : std::as_const(propertyNodes)) {
4068+
std::sort(nodes.begin(), nodes.end(), Node::nodeNameLessThan);
4069+
4070+
// Group properties by their role for more concise output
4071+
QMap<PropertyNode::FunctionRole, QList<const PropertyNode *>> roleGroups;
4072+
for (const auto *n : std::as_const(nodes)) {
4073+
const auto *pn = static_cast<const PropertyNode *>(n);
4074+
PropertyNode::FunctionRole role = pn->role(fn);
4075+
roleGroups[role].append(pn);
4076+
}
4077+
4078+
// Generate text for each role group in an explicit order
4079+
static constexpr PropertyNode::FunctionRole roleOrder[] = {
4080+
PropertyNode::FunctionRole::Getter,
4081+
PropertyNode::FunctionRole::Setter,
4082+
PropertyNode::FunctionRole::Resetter,
4083+
PropertyNode::FunctionRole::Notifier,
4084+
PropertyNode::FunctionRole::Bindable,
4085+
};
4086+
4087+
for (auto role : roleOrder) {
4088+
const auto it = roleGroups.constFind(role);
4089+
if (it == roleGroups.cend())
4090+
continue;
4091+
4092+
const auto &properties = it.value();
4093+
40704094
QString msg;
4071-
const auto pn = static_cast<const PropertyNode *>(propertyNode);
4072-
switch (pn->role(fn)) {
4095+
switch (role) {
40734096
case PropertyNode::FunctionRole::Getter:
40744097
msg = QStringLiteral("Getter function");
40754098
break;
@@ -4082,13 +4105,28 @@ void DocBookGenerator::generateAddendum(const Node *node, Addendum type, CodeMar
40824105
case PropertyNode::FunctionRole::Notifier:
40834106
msg = QStringLiteral("Notifier signal");
40844107
break;
4108+
case PropertyNode::FunctionRole::Bindable:
4109+
msg = QStringLiteral("Bindable function");
4110+
break;
40854111
default:
40864112
continue;
40874113
}
4114+
40884115
m_writer->writeStartElement(dbNamespace, "para");
4089-
m_writer->writeCharacters(msg + " for property ");
4090-
generateSimpleLink(linkForNode(pn, nullptr), pn->name());
4091-
m_writer->writeCharacters(". ");
4116+
if (properties.size() == 1) {
4117+
const auto *pn = properties.first();
4118+
m_writer->writeCharacters(msg + " for property ");
4119+
generateSimpleLink(linkForNode(pn, nullptr), pn->name());
4120+
m_writer->writeCharacters(". ");
4121+
} else {
4122+
m_writer->writeCharacters(msg + " for properties ");
4123+
for (qsizetype i = 0; i < properties.size(); ++i) {
4124+
const auto *pn = properties.at(i);
4125+
generateSimpleLink(linkForNode(pn, nullptr), pn->name());
4126+
m_writer->writeCharacters(Utilities::separator(i, properties.size()));
4127+
}
4128+
m_writer->writeCharacters(" ");
4129+
}
40924130
m_writer->writeEndElement(); // para
40934131
newLine();
40944132
}

‎src/qdoc/qdoc/src/qdoc/generator.cpp‎

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1317,28 +1317,67 @@ void Generator::generateAddendum(const Node *node, Addendum type, CodeMarker *ma
13171317
if (nodes.isEmpty())
13181318
return;
13191319
std::sort(nodes.begin(), nodes.end(), Node::nodeNameLessThan);
1320+
1321+
// Group properties by their role for more concise output
1322+
QMap<PropertyNode::FunctionRole, QList<const PropertyNode *>> roleGroups;
13201323
for (const auto *n : std::as_const(nodes)) {
1321-
QString msg;
13221324
const auto *pn = static_cast<const PropertyNode *>(n);
1323-
switch (pn->role(fn)) {
1325+
PropertyNode::FunctionRole role = pn->role(fn);
1326+
roleGroups[role].append(pn);
1327+
}
1328+
1329+
// Generate text for each role group in an explicit order
1330+
static constexpr PropertyNode::FunctionRole roleOrder[] = {
1331+
PropertyNode::FunctionRole::Getter,
1332+
PropertyNode::FunctionRole::Setter,
1333+
PropertyNode::FunctionRole::Resetter,
1334+
PropertyNode::FunctionRole::Notifier,
1335+
PropertyNode::FunctionRole::Bindable,
1336+
};
1337+
for (auto role : roleOrder) {
1338+
const auto it = roleGroups.constFind(role);
1339+
if (it == roleGroups.cend())
1340+
continue;
1341+
1342+
const auto &properties = it.value();
1343+
1344+
QString msg;
1345+
switch (role) {
13241346
case PropertyNode::FunctionRole::Getter:
1325-
msg = QStringLiteral("Getter function");
1347+
msg = u"Getter function"_s;
13261348
break;
13271349
case PropertyNode::FunctionRole::Setter:
1328-
msg = QStringLiteral("Setter function");
1350+
msg = u"Setter function"_s;
13291351
break;
13301352
case PropertyNode::FunctionRole::Resetter:
1331-
msg = QStringLiteral("Resetter function");
1353+
msg = u"Resetter function"_s;
13321354
break;
13331355
case PropertyNode::FunctionRole::Notifier:
1334-
msg = QStringLiteral("Notifier signal");
1356+
msg = u"Notifier signal"_s;
1357+
break;
1358+
case PropertyNode::FunctionRole::Bindable:
1359+
msg = u"Bindable function"_s;
13351360
break;
13361361
default:
13371362
continue;
13381363
}
1339-
text << msg << " for property " << Atom(Atom::Link, pn->name())
1340-
<< Atom(Atom::FormattingLeft, ATOM_FORMATTING_LINK) << pn->name()
1341-
<< Atom(Atom::FormattingRight, ATOM_FORMATTING_LINK) << ". ";
1364+
1365+
if (properties.size() == 1) {
1366+
const auto *pn = properties.first();
1367+
text << msg << u" for property "_s << Atom(Atom::Link, pn->name())
1368+
<< Atom(Atom::FormattingLeft, ATOM_FORMATTING_LINK) << pn->name()
1369+
<< Atom(Atom::FormattingRight, ATOM_FORMATTING_LINK) << u". "_s;
1370+
} else {
1371+
text << msg << u" for properties "_s;
1372+
for (qsizetype i = 0; i < properties.size(); ++i) {
1373+
const auto *pn = properties.at(i);
1374+
text << Atom(Atom::Link, pn->name())
1375+
<< Atom(Atom::FormattingLeft, ATOM_FORMATTING_LINK) << pn->name()
1376+
<< Atom(Atom::FormattingRight, ATOM_FORMATTING_LINK)
1377+
<< Utilities::separator(i, properties.size());
1378+
}
1379+
text << u" "_s;
1380+
}
13421381
}
13431382
break;
13441383
}

‎src/qdoc/qdoc/tests/validateqdocoutputfiles/testdata/multisignal/expected/docbook/testaction.xml‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<db:article xmlns:db="http://docbook.org/ns/docbook" xmlns:xlink="http://www.w3.org/1999/xlink" version="5.2" xml:lang="en">
3+
<db:info>
4+
<db:title>TestAction Class</db:title>
5+
<db:productname>MultiSignalTest</db:productname>
6+
<db:titleabbrev>Test for improved multi-property signal documentation</db:titleabbrev>
7+
<db:abstract>
8+
<db:para>A test class to demonstrate improved multi-property signal documentation.</db:para>
9+
</db:abstract>
10+
</db:info>
11+
<db:variablelist>
12+
<db:varlistentry>
13+
<db:term>Header</db:term>
14+
<db:listitem>
15+
<db:para>TestAction</db:para>
16+
</db:listitem>
17+
</db:varlistentry>
18+
<db:varlistentry>
19+
<db:term>Inherits</db:term>
20+
<db:listitem>
21+
<db:para/>
22+
</db:listitem>
23+
</db:varlistentry>
24+
</db:variablelist>
25+
<db:section xml:id="details">
26+
<db:title>Detailed Description</db:title>
27+
<db:para>This class has properties with different notification patterns.</db:para>
28+
</db:section>
29+
<db:section xml:id="property-documentation">
30+
<db:title>Property Documentation</db:title>
31+
<db:section xml:id="checkable-prop">
32+
<db:title>checkable : bool</db:title>
33+
<db:para>This property holds whether the action is checkable</db:para>
34+
<db:para>This property determines if the action can be toggled.</db:para>
35+
<db:para>
36+
<db:emphasis role="bold">Access functions:
37+
</db:emphasis>
38+
</db:para>
39+
<db:itemizedlist>
40+
<db:listitem>
41+
<db:para><db:type>bool</db:type> <db:emphasis role="bold"><db:link xlink:href="">isCheckable</db:link></db:emphasis>() const</db:para>
42+
</db:listitem>
43+
<db:listitem>
44+
<db:para><db:type>void</db:type> <db:emphasis role="bold"><db:link xlink:href="">setCheckable</db:link></db:emphasis>(<db:type>bool</db:type> <db:emphasis>checkable</db:emphasis>)</db:para>
45+
</db:listitem>
46+
</db:itemizedlist>
47+
<db:para>
48+
<db:emphasis role="bold">Notifier signal:
49+
</db:emphasis>
50+
</db:para>
51+
<db:itemizedlist>
52+
<db:listitem>
53+
<db:para><db:type>void</db:type> <db:emphasis role="bold"><db:link xlink:href="testaction.xml#singlePropertySignal">singlePropertySignal</db:link></db:emphasis>()</db:para>
54+
</db:listitem>
55+
</db:itemizedlist>
56+
</db:section>
57+
<db:section xml:id="enabled-prop">
58+
<db:title>enabled : bool</db:title>
59+
<db:para>This property holds whether the action is enabled</db:para>
60+
<db:para>This property determines if the action can be triggered.</db:para>
61+
<db:para>
62+
<db:emphasis role="bold">Access functions:
63+
</db:emphasis>
64+
</db:para>
65+
<db:itemizedlist>
66+
<db:listitem>
67+
<db:para><db:type>bool</db:type> <db:emphasis role="bold"><db:link xlink:href="">isEnabled</db:link></db:emphasis>() const</db:para>
68+
</db:listitem>
69+
<db:listitem>
70+
<db:para><db:type>void</db:type> <db:emphasis role="bold"><db:link xlink:href="">setEnabled</db:link></db:emphasis>(<db:type>bool</db:type> <db:emphasis>enabled</db:emphasis>)</db:para>
71+
</db:listitem>
72+
</db:itemizedlist>
73+
<db:para>
74+
<db:emphasis role="bold">Notifier signal:
75+
</db:emphasis>
76+
</db:para>
77+
<db:itemizedlist>
78+
<db:listitem>
79+
<db:para><db:type>void</db:type> <db:emphasis role="bold"><db:link xlink:href="testaction.xml#changed">changed</db:link></db:emphasis>()</db:para>
80+
</db:listitem>
81+
</db:itemizedlist>
82+
</db:section>
83+
<db:section xml:id="text-prop">
84+
<db:title>text : QString</db:title>
85+
<db:para>This property holds the action's display text</db:para>
86+
<db:para>This property holds the text that is displayed for the action.</db:para>
87+
<db:para>
88+
<db:emphasis role="bold">Access functions:
89+
</db:emphasis>
90+
</db:para>
91+
<db:itemizedlist>
92+
<db:listitem>
93+
<db:para><db:type>QString</db:type> <db:emphasis role="bold"><db:link xlink:href="">text</db:link></db:emphasis>() const</db:para>
94+
</db:listitem>
95+
<db:listitem>
96+
<db:para><db:type>void</db:type> <db:emphasis role="bold"><db:link xlink:href="">setText</db:link></db:emphasis>(const <db:type>QString</db:type> &amp;<db:emphasis>text</db:emphasis>)</db:para>
97+
</db:listitem>
98+
</db:itemizedlist>
99+
<db:para>
100+
<db:emphasis role="bold">Notifier signal:
101+
</db:emphasis>
102+
</db:para>
103+
<db:itemizedlist>
104+
<db:listitem>
105+
<db:para><db:type>void</db:type> <db:emphasis role="bold"><db:link xlink:href="testaction.xml#changed">changed</db:link></db:emphasis>()</db:para>
106+
</db:listitem>
107+
</db:itemizedlist>
108+
</db:section>
109+
<db:section xml:id="visible-prop">
110+
<db:title>visible : bool</db:title>
111+
<db:para>This property holds whether the action is visible</db:para>
112+
<db:para>This property determines if the action is shown in UI.</db:para>
113+
<db:para>
114+
<db:emphasis role="bold">Access functions:
115+
</db:emphasis>
116+
</db:para>
117+
<db:itemizedlist>
118+
<db:listitem>
119+
<db:para><db:type>bool</db:type> <db:emphasis role="bold"><db:link xlink:href="">isVisible</db:link></db:emphasis>() const</db:para>
120+
</db:listitem>
121+
<db:listitem>
122+
<db:para><db:type>void</db:type> <db:emphasis role="bold"><db:link xlink:href="">setVisible</db:link></db:emphasis>(<db:type>bool</db:type> <db:emphasis>visible</db:emphasis>)</db:para>
123+
</db:listitem>
124+
</db:itemizedlist>
125+
<db:para>
126+
<db:emphasis role="bold">Notifier signal:
127+
</db:emphasis>
128+
</db:para>
129+
<db:itemizedlist>
130+
<db:listitem>
131+
<db:para><db:type>void</db:type> <db:emphasis role="bold"><db:link xlink:href="testaction.xml#changed">changed</db:link></db:emphasis>()</db:para>
132+
</db:listitem>
133+
</db:itemizedlist>
134+
</db:section>
135+
</db:section>
136+
<db:section xml:id="member-function-documentation">
137+
<db:title>Member Function Documentation</db:title>
138+
<db:section xml:id="TestAction">
139+
<db:title>[explicit] TestAction::TestAction(QObject *<db:emphasis>parent</db:emphasis> = nullptr)</db:title>
140+
<db:para>Constructs a <db:link xlink:href="testaction.xml">TestAction</db:link> with the given <db:code role="parameter">parent</db:code>.</db:para>
141+
</db:section>
142+
<db:section xml:id="changed">
143+
<db:title>[signal] void TestAction::changed()</db:title>
144+
<db:para>This signal is emitted when certain properties of the action change. The properties that trigger this signal are enabled, visible, and text.</db:para>
145+
<db:note>
146+
<db:para>Notifier signal for properties <db:link xlink:href="testaction.xml#enabled-prop">enabled</db:link>, <db:link xlink:href="testaction.xml#text-prop">text</db:link>, and <db:link xlink:href="testaction.xml#visible-prop">visible</db:link>. </db:para>
147+
</db:note>
148+
</db:section>
149+
<db:section xml:id="singlePropertySignal">
150+
<db:title>[signal] void TestAction::singlePropertySignal()</db:title>
151+
<db:para>This signal is emitted when the checkable property changes.</db:para>
152+
<db:note>
153+
<db:para>Notifier signal for property <db:link xlink:href="testaction.xml#checkable-prop">checkable</db:link>. </db:para>
154+
</db:note>
155+
</db:section>
156+
</db:section>
157+
</db:article>

‎src/qdoc/qdoc/tests/validateqdocoutputfiles/testdata/multisignal/expected/docbook/testmodule-module.xml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<db:article xmlns:db="http://docbook.org/ns/docbook" xmlns:xlink="http://www.w3.org/1999/xlink" version="5.2" xml:lang="en">
3+
<db:info>
4+
<db:title></db:title>
5+
<db:productname>MultiSignalTest</db:productname>
6+
<db:titleabbrev>Test for improved multi-property signal documentation</db:titleabbrev>
7+
<db:abstract>
8+
<db:para>A test module.</db:para>
9+
</db:abstract>
10+
</db:info>
11+
<db:para>A test module.</db:para>
12+
<db:section xml:id="classes">
13+
<db:title>Classes</db:title>
14+
<db:variablelist role="classes">
15+
<db:varlistentry>
16+
<db:term><db:link xlink:href="testaction.xml" xlink:role="class">TestAction</db:link></db:term>
17+
<db:listitem>
18+
<db:para>A test class to demonstrate improved multi-property signal documentation.</db:para>
19+
</db:listitem>
20+
</db:varlistentry>
21+
</db:variablelist>
22+
</db:section>
23+
<db:section xml:id="details">
24+
<db:title>Detailed Description</db:title>
25+
</db:section>
26+
</db:article>

‎src/qdoc/qdoc/tests/validateqdocoutputfiles/testdata/multisignal/expected/html/multisignaltest.index‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!DOCTYPE QDOCINDEX>
3+
<INDEX url="" title="Test for improved multi-property signal documentation" version="" project="MultiSignalTest">
4+
<namespace name="" status="active" access="public" module="multisignaltest">
5+
<class name="TestAction" href="testaction.html" status="active" access="public" location="testaction.h" documented="true" bases="QObject" module="TestModule" brief="A test class to demonstrate improved multi-property signal documentation">
6+
<function name="TestAction" fullname="TestAction::TestAction" href="testaction.html#TestAction" status="active" access="public" location="testaction.h" documented="true" meta="constructor" explicit="true" signature="TestAction(QObject *parent)">
7+
<parameter type="QObject *" name="parent" default="nullptr"/>
8+
</function>
9+
<function name="changed" fullname="TestAction::changed" href="testaction.html#changed" status="active" access="public" location="testaction.h" documented="true" meta="signal" associated-property="enabled,text,visible" type="void" signature="void changed()"/>
10+
<function name="isCheckable" fullname="TestAction::isCheckable" href="testaction.html#checkable-prop" status="active" access="public" location="testaction.h" meta="plain" const="true" associated-property="checkable" type="bool" signature="bool isCheckable() const"/>
11+
<function name="isEnabled" fullname="TestAction::isEnabled" href="testaction.html#enabled-prop" status="active" access="public" location="testaction.h" meta="plain" const="true" associated-property="enabled" type="bool" signature="bool isEnabled() const"/>
12+
<function name="isVisible" fullname="TestAction::isVisible" href="testaction.html#visible-prop" status="active" access="public" location="testaction.h" meta="plain" const="true" associated-property="visible" type="bool" signature="bool isVisible() const"/>
13+
<function name="setCheckable" fullname="TestAction::setCheckable" href="testaction.html#checkable-prop" status="active" access="public" location="testaction.h" meta="plain" associated-property="checkable" type="void" signature="void setCheckable(bool checkable)">
14+
<parameter type="bool" name="checkable" default=""/>
15+
</function>
16+
<function name="setEnabled" fullname="TestAction::setEnabled" href="testaction.html#enabled-prop" status="active" access="public" location="testaction.h" meta="plain" associated-property="enabled" type="void" signature="void setEnabled(bool enabled)">
17+
<parameter type="bool" name="enabled" default=""/>
18+
</function>
19+
<function name="setText" fullname="TestAction::setText" href="testaction.html#text-prop" status="active" access="public" location="testaction.h" meta="plain" associated-property="text" type="void" signature="void setText(const QString &amp;text)">
20+
<parameter type="const QString &amp;" name="text" default=""/>
21+
</function>
22+
<function name="setVisible" fullname="TestAction::setVisible" href="testaction.html#visible-prop" status="active" access="public" location="testaction.h" meta="plain" associated-property="visible" type="void" signature="void setVisible(bool visible)">
23+
<parameter type="bool" name="visible" default=""/>
24+
</function>
25+
<function name="singlePropertySignal" fullname="TestAction::singlePropertySignal" href="testaction.html#singlePropertySignal" status="active" access="public" location="testaction.h" documented="true" meta="signal" associated-property="checkable" type="void" signature="void singlePropertySignal()"/>
26+
<function name="text" fullname="TestAction::text" href="testaction.html#text-prop" status="active" access="public" location="testaction.h" meta="plain" const="true" associated-property="text" type="QString" signature="QString text() const"/>
27+
<property name="checkable" fullname="TestAction::checkable" href="testaction.html#checkable-prop" status="active" access="public" location="testaction.h" documented="true" brief="This property holds whether the action is checkable">
28+
<getter name="isCheckable"/>
29+
<setter name="setCheckable"/>
30+
<notifier name="singlePropertySignal"/>
31+
</property>
32+
<property name="enabled" fullname="TestAction::enabled" href="testaction.html#enabled-prop" status="active" access="public" location="testaction.h" documented="true" brief="This property holds whether the action is enabled">
33+
<getter name="isEnabled"/>
34+
<setter name="setEnabled"/>
35+
<notifier name="changed"/>
36+
</property>
37+
<property name="text" fullname="TestAction::text" href="testaction.html#text-prop" status="active" access="public" location="testaction.h" documented="true" brief="This property holds the action's display text">
38+
<getter name="text"/>
39+
<setter name="setText"/>
40+
<notifier name="changed"/>
41+
</property>
42+
<property name="visible" fullname="TestAction::visible" href="testaction.html#visible-prop" status="active" access="public" location="testaction.h" documented="true" brief="This property holds whether the action is visible">
43+
<getter name="isVisible"/>
44+
<setter name="setVisible"/>
45+
<notifier name="changed"/>
46+
</property>
47+
</class>
48+
<module name="TestModule" href="testmodule-module.html" status="active" documented="true" seen="true" title="" brief="A test module"/>
49+
</namespace>
50+
</INDEX>

0 commit comments

Comments
 (0)