Merge lp:~zsombi/ubuntu-ui-toolkit/theming-selector-parsing into lp:ubuntu-ui-toolkit

Proposed by Zsombor Egri
Status: Merged
Approved by: Tim Peeters
Approved revision: 380
Merged at revision: 378
Proposed branch: lp:~zsombi/ubuntu-ui-toolkit/theming-selector-parsing
Merge into: lp:ubuntu-ui-toolkit
Prerequisite: lp:~zsombi/ubuntu-ui-toolkit/normalize-style-properties
Diff against target: 469 lines (+126/-136)
9 files modified
modules/Ubuntu/Components/plugin/itemstyleattached.cpp (+1/-2)
modules/Ubuntu/Components/plugin/qmlthemeloader.cpp (+1/-1)
modules/Ubuntu/Components/plugin/suffixtree.cpp (+71/-9)
modules/Ubuntu/Components/plugin/suffixtree_p.h (+11/-2)
modules/Ubuntu/Components/plugin/themeengine.cpp (+4/-50)
modules/Ubuntu/Components/plugin/themeengine_p.h (+0/-1)
modules/Ubuntu/Components/ubuntu-components-theming.qdoc (+1/-1)
tests/unit/tst_theme_engine/tst_theme_enginetest.cpp (+0/-2)
tests/unit/tst_theme_engine_private/tst_theme_engine_privatetest.cpp (+37/-68)
To merge this branch: bzr merge lp:~zsombi/ubuntu-ui-toolkit/theming-selector-parsing
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Tim Peeters Approve
Review via email: mp+150295@code.launchpad.net

Commit message

Selector parsing moved into SelectorNode. Extra white space required between selector nodes and compositors removed.

Description of the change

Selector parsing moved into SelectorNode. Extra white space required between selector nodes and compositors removed.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Tim Peeters (tpeeters) wrote :

Looks good, but I didn't check the tests.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'modules/Ubuntu/Components/plugin/itemstyleattached.cpp'
2--- modules/Ubuntu/Components/plugin/itemstyleattached.cpp 2013-01-14 14:16:54 +0000
3+++ modules/Ubuntu/Components/plugin/itemstyleattached.cpp 2013-02-27 06:21:21 +0000
4@@ -297,7 +297,6 @@
5 bool ItemStyleAttachedPrivate::registerName(const QString &id)
6 {
7 bool result = true;
8- Q_Q(ItemStyleAttached);
9 if (ThemeEnginePrivate::registerName(attachee, id)) {
10 styleData.styleId = id;
11 attachee->setProperty("name", styleData.styleId);
12@@ -461,7 +460,7 @@
13 {
14 Q_D(const ItemStyleAttached);
15 return d->styleRule ?
16- ThemeEnginePrivate::selectorToString(d->styleRule->path()) :
17+ d->styleRule->path().toString() :
18 QString("(null)");
19 }
20
21
22=== modified file 'modules/Ubuntu/Components/plugin/qmlthemeloader.cpp'
23--- modules/Ubuntu/Components/plugin/qmlthemeloader.cpp 2013-02-25 13:07:03 +0000
24+++ modules/Ubuntu/Components/plugin/qmlthemeloader.cpp 2013-02-27 06:21:21 +0000
25@@ -503,7 +503,7 @@
26 QString qmap;
27 for (int count = selector.count(); count > 0; count--) {
28 subset = selectorSubset(selector, count, SelectorNode::Normal);
29- qmap = ThemeEnginePrivate::selectorToString(subset);
30+ qmap = subset.toString();
31 if (qmlMap.contains(qmap)) {
32 return qmlMap.value(qmap);
33 }
34
35=== modified file 'modules/Ubuntu/Components/plugin/suffixtree.cpp'
36--- modules/Ubuntu/Components/plugin/suffixtree.cpp 2013-01-15 13:27:54 +0000
37+++ modules/Ubuntu/Components/plugin/suffixtree.cpp 2013-02-27 06:21:21 +0000
38@@ -23,6 +23,7 @@
39 #include <QtQml/QQmlContext>
40 #include <QtQml/QQmlComponent>
41 #include <QtQuick/QQuickItem>
42+#include <QtCore/QRegularExpression>
43
44 /*
45 This file contains the Rule-element suffix-tree handling classes. The suffix-tree
46@@ -36,16 +37,31 @@
47 relationship(Descendant),
48 sensitivity(Normal)
49 {}
50+
51 /*!
52 \internal
53- Creates an instance of a SelectorNode with a given class, name
54- and relationship. The sensitivity parameter configures the node so that during
55- string conversion and comparison ignores the relationship, the name
56- both or none. This feature is used when building up QmlTheme selectorTable.
57+ Creates an instance of a SelectorNode by parsing the selectorString. The
58+ sensitivity parameter configures the node so that during string conversion
59+ and comparison ignores the relationship, the name both or none. This feature
60+ is used when building up QmlTheme selectorTable.
61 */
62-SelectorNode::SelectorNode(const QString &styleClass, const QString &styleId, Relationship relationship, NodeSensitivity sensitivity) :
63- styleClass(styleClass.toLower()), styleId(styleId.toLower()), relationship(relationship), sensitivity(sensitivity)
64+SelectorNode::SelectorNode(const QString &selectorString, NodeSensitivity sensitivity) :
65+ relationship(Descendant), sensitivity(sensitivity)
66 {
67+ styleClass = selectorString;
68+ if (styleClass.startsWith('>')) {
69+ relationship = Child;
70+ styleClass.remove('>');
71+ }
72+ int idIndex = styleClass.indexOf('#');
73+ if (idIndex != -1) {
74+ styleId = styleClass.mid(idIndex + 1).toLower();
75+ styleClass = styleClass.left(idIndex);
76+ if (idIndex > 1 && styleClass[0] == '.')
77+ styleClass = styleClass.mid(1, idIndex - 1);
78+ } else if (styleClass[0] == '.')
79+ styleClass = styleClass.mid(1);
80+ styleClass = styleClass.toLower();
81 }
82
83 /*!
84@@ -58,7 +74,7 @@
85 QString result;
86 if (((sensitivity & IgnoreRelationship) != IgnoreRelationship) &&
87 (relationship == SelectorNode::Child))
88- result += "> ";
89+ result += ">";
90 if (!styleClass.isEmpty())
91 result += "." + styleClass;
92 else if (!className.isEmpty()) {
93@@ -81,17 +97,63 @@
94 }
95
96 /*!
97+ * \internal
98+ * Converts a selector string into Selector object.
99+ * Current support (ref: www.w3.org/TR/selector.html):
100+ * - Type selectors, e.g: "Button"
101+ * - Descendant selectors, e.g: "Dialog Button"
102+ * - Child selectors, e.g: "Dialog>Button"
103+ * - ID selectors, e.g: "Button#mySpecialButton"
104+ */
105+Selector::Selector(const QString &string, SelectorNode::NodeSensitivity sensitivity)
106+{
107+ QString tmp(string);
108+ // prepare for split
109+ if (tmp.contains('>')) {
110+ tmp.replace(QRegularExpression(" (>) "), ">").replace('>', "|>");
111+ }
112+ tmp.replace(' ', '|');
113+
114+ QStringList nodes = tmp.simplified().split('|');
115+ QStringListIterator inodes(nodes);
116+ inodes.toBack();
117+ while (inodes.hasPrevious()) {
118+ const QString &node = inodes.previous();
119+ if (node.isEmpty())
120+ continue;
121+ prepend(SelectorNode(node, sensitivity));
122+ }
123+}
124+
125+/*!
126+ \internal
127+ Converts a style path back to selector string.
128+*/
129+QString Selector::toString() const
130+{
131+ QString result;
132+
133+ QListIterator<SelectorNode> i(*this);
134+ while (i.hasNext()) {
135+ SelectorNode node = i.next();
136+ result += ' ' + node.toString();
137+ }
138+ result.replace(" >", ">");
139+ return result.simplified();
140+}
141+
142+/*!
143 \internal
144 Hash key for Selector. Uses QString's hash function.
145 */
146 uint qHash(const Selector &key)
147 {
148- return qHash(ThemeEnginePrivate::selectorToString(key));
149+ return qHash(key.toString());
150 }
151
152
153 StyleTreeNode::StyleTreeNode(StyleTreeNode *parent) :
154- parent(parent), styleNode("", "", SelectorNode::Descendant), style(0), delegate(0)
155+ parent(parent), style(0), delegate(0)
156 {
157 }
158
159
160=== modified file 'modules/Ubuntu/Components/plugin/suffixtree_p.h'
161--- modules/Ubuntu/Components/plugin/suffixtree_p.h 2012-12-11 10:46:19 +0000
162+++ modules/Ubuntu/Components/plugin/suffixtree_p.h 2013-02-27 06:21:21 +0000
163@@ -21,6 +21,7 @@
164
165 #include <QtCore/QHash>
166 #include <QtCore/QString>
167+#include <QtCore/QList>
168
169 // node of a selector
170 class SelectorNode {
171@@ -32,7 +33,7 @@
172 IgnoreStyleId = 0x02,
173 IgnoreAll = IgnoreRelationship | IgnoreStyleId};
174 SelectorNode();
175- SelectorNode(const QString &styleClass, const QString &styleId, Relationship relationship = Descendant, NodeSensitivity sensitivity = Normal);
176+ SelectorNode(const QString &selectorString, NodeSensitivity sensitivity = Normal);
177 QString toString() const;
178 bool operator==(const SelectorNode &other);
179 QString className;
180@@ -43,7 +44,15 @@
181 };
182
183 // selector type
184-typedef QList<SelectorNode> Selector;
185+class Selector : public QList<SelectorNode> {
186+public:
187+ inline Selector() {}
188+ inline Selector(const Selector& s) : QList<SelectorNode>(s){}
189+ Selector(const QString &string, SelectorNode::NodeSensitivity sensitivity = SelectorNode::Normal);
190+ virtual ~Selector() {}
191+ QString toString() const;
192+};
193+Q_DECLARE_TYPEINFO(Selector, Q_MOVABLE_TYPE);
194 uint qHash(const Selector &key);
195
196 // style rule tree
197
198=== modified file 'modules/Ubuntu/Components/plugin/themeengine.cpp'
199--- modules/Ubuntu/Components/plugin/themeengine.cpp 2012-12-11 11:47:33 +0000
200+++ modules/Ubuntu/Components/plugin/themeengine.cpp 2013-02-27 06:21:21 +0000
201@@ -231,64 +231,18 @@
202 return qobject_cast<ItemStyleAttached*>(attached);
203 }
204
205-
206-/*!
207- \internal
208- Converts a style path back to selector string.
209-*/
210-QString ThemeEnginePrivate::selectorToString(const Selector &path)
211-{
212- QString result;
213- Q_FOREACH (SelectorNode node, path) {
214- result += " " + node.toString();
215- }
216- return result.simplified();
217-}
218-
219 /*!
220 \internal
221 Parses and returns the path described by \a selector as a list of
222- class and name pairs.
223- Current support (ref: www.w3.org/TR/selector.html):
224- - Type selectors, e.g: "Button"
225- - Descendant selectors, e.g: "Dialog Button"
226- - Child selectors, e.g: "Dialog > Button"
227- - ID selectors, e.g: "Button#mySpecialButton"
228- - Grouping, e.g: "Button#foo, Checkbox, #bar"
229+ class and name pairs. Supports selector grouping (separated with commas).
230 */
231 QList<Selector> ThemeEnginePrivate::parseSelector(const QString &selectorString, SelectorNode::NodeSensitivity sensitivity)
232 {
233 QList<Selector> pathList;
234 QStringList groupList = selectorString.split(",");
235- SelectorNode::Relationship nextRelationShip = SelectorNode::Descendant;
236-
237- Q_FOREACH (QString group, groupList) {
238- Selector selector;
239- QStringList tokens = group.simplified().split(' ');
240-
241- Q_FOREACH (QString token, tokens) {
242- if (token.isEmpty() || token == " ")
243- continue;
244- if (token == ">") {
245- nextRelationShip = SelectorNode::Child;
246- } else {
247- QString styleClass;
248- QString styleId;
249- int idIndex = token.indexOf('#');
250- if (idIndex != -1) {
251- styleId = token.mid(idIndex + 1);
252- if (idIndex > 1 && token[0] == '.')
253- styleClass = token.mid(1, idIndex - 1);
254- } else if (token[0] == '.') {
255- styleClass = token.mid(1);
256- } else
257- styleClass = token;
258- if (!styleClass.isEmpty() || !styleId.isEmpty())
259- selector.append(SelectorNode(styleClass.toLower(), styleId.toLower(), nextRelationShip, sensitivity));
260- nextRelationShip = SelectorNode::Descendant;
261- }
262- }
263- pathList.append(selector);
264+
265+ Q_FOREACH (const QString &group, groupList) {
266+ pathList.append(Selector(group, sensitivity));
267 }
268 return pathList;
269 }
270
271=== modified file 'modules/Ubuntu/Components/plugin/themeengine_p.h'
272--- modules/Ubuntu/Components/plugin/themeengine_p.h 2012-12-11 10:46:19 +0000
273+++ modules/Ubuntu/Components/plugin/themeengine_p.h 2013-02-27 06:21:21 +0000
274@@ -72,7 +72,6 @@
275 static bool registerName(QQuickItem *item, const QString &newName);
276 static void setError(const QString &error);
277 static ItemStyleAttached *attachedStyle(QObject *obj);
278- static QString selectorToString(const Selector &path);
279 static QList<Selector> parseSelector(const QString &selectorString, SelectorNode::NodeSensitivity sensitivity = SelectorNode::Normal);
280
281 // private slots
282
283=== modified file 'modules/Ubuntu/Components/ubuntu-components-theming.qdoc'
284--- modules/Ubuntu/Components/ubuntu-components-theming.qdoc 2012-11-20 10:07:44 +0000
285+++ modules/Ubuntu/Components/ubuntu-components-theming.qdoc 2013-02-27 06:21:21 +0000
286@@ -206,7 +206,7 @@
287 named as "red" and which are direct children of QML elements styled as class "frame"
288 \code
289 //QmlTheme rule
290- .frame > .Button#red {
291+ .frame>.Button#red {
292 ...
293 }
294 \endcode
295
296=== modified file 'tests/unit/tst_theme_engine/tst_theme_enginetest.cpp'
297--- tests/unit/tst_theme_engine/tst_theme_enginetest.cpp 2012-12-11 11:09:44 +0000
298+++ tests/unit/tst_theme_engine/tst_theme_enginetest.cpp 2013-02-27 06:21:21 +0000
299@@ -163,8 +163,6 @@
300
301 QObject *style = qvariant_cast<QObject*>(attached->property("style"));
302 QVERIFY(style);
303-
304- QObject *anim = qvariant_cast<QObject*>(style->property(""));
305 }
306
307 void tst_ThemeEngine::testCase_selectorDelegates()
308
309=== modified file 'tests/unit/tst_theme_engine_private/tst_theme_engine_privatetest.cpp'
310--- tests/unit/tst_theme_engine_private/tst_theme_engine_privatetest.cpp 2013-02-25 07:19:19 +0000
311+++ tests/unit/tst_theme_engine_private/tst_theme_engine_privatetest.cpp 2013-02-27 06:21:21 +0000
312@@ -161,41 +161,32 @@
313 StyleTreeNode *rule;
314 Selector path, expected;
315
316- path << SelectorNode("baseA", "", SelectorNode::Descendant);
317- rule = engine->styleRuleForPath(path);
318- // should pass
319- result = (rule != 0) && (rule->path() == path);
320- QCOMPARE(result, true);
321-
322- path.clear();
323- path << SelectorNode("testA", "", SelectorNode::Descendant);
324- path << SelectorNode("baseA", "", SelectorNode::Descendant);
325- rule = engine->styleRuleForPath(path);
326- // should pass
327- result = (rule != 0) && (rule->path() == path);
328- QCOMPARE(result, true);
329-
330- path.clear();
331- path << SelectorNode("testA", "", SelectorNode::Descendant);
332- path << SelectorNode("baseA", "", SelectorNode::Child);
333- expected << SelectorNode("testA", "", SelectorNode::Descendant);
334- expected << SelectorNode("baseA", "", SelectorNode::Descendant);
335+ path = Selector(".baseA");
336+ rule = engine->styleRuleForPath(path);
337+ // should pass
338+ result = (rule != 0) && (rule->path() == path);
339+ QCOMPARE(result, true);
340+
341+ path = Selector("testA baseA");
342+ rule = engine->styleRuleForPath(path);
343+ // should pass
344+ result = (rule != 0) && (rule->path() == path);
345+ QCOMPARE(result, true);
346+
347+ path = Selector("testA>baseA");
348+ expected = Selector("testA baseA");
349 rule = engine->styleRuleForPath(path);
350 // should pass, but should be ".testA .baseA"
351 result = (rule != 0) && (rule->path() == expected);
352 QCOMPARE(result, true);
353
354- path.clear();
355- path << SelectorNode("testB", "", SelectorNode::Descendant);
356- path << SelectorNode("baseA", "", SelectorNode::Child);
357+ path = Selector("testB>baseA");
358 rule = engine->styleRuleForPath(path);
359 // should pass
360 result = (rule != 0) && (rule->path() == path);
361 QCOMPARE(result, true);
362
363- path.clear();
364- path << SelectorNode("testB", "", SelectorNode::Descendant);
365- path << SelectorNode("baseA", "", SelectorNode::Descendant);
366+ path = Selector("testB baseA");
367 rule = engine->styleRuleForPath(path);
368 QVERIFY2(rule != 0, "Rule not found.");
369 // should fail
370@@ -209,9 +200,7 @@
371
372 // build selector path
373 QList<Selector> selectors = engine->parseSelector(".testB .baseA");
374- Selector expected, expected2;
375- expected << SelectorNode("testB", "", SelectorNode::Descendant);
376- expected << SelectorNode("baseA", "", SelectorNode::Descendant);
377+ Selector expected("testB baseA");
378 // should match
379 bool result = (selectors.count() == 1) && (selectors[0] == expected);
380 QCOMPARE(result, true);
381@@ -221,23 +210,19 @@
382 result = (selectors.count() == 1) && !(selectors[0] == expected);
383 QCOMPARE(result, true);
384
385- expected.clear();
386- expected << SelectorNode("root", "id", SelectorNode::Descendant);
387- expected << SelectorNode("testB", "", SelectorNode::Child);
388- expected << SelectorNode("baseB", "", SelectorNode::Descendant);
389+ expected = Selector("root#id>testB baseB");
390 selectors = engine->parseSelector(".root#id .testB > .baseA");
391 // should not match!
392 result = (selectors.count() == 1) && !(selectors[0] == expected);
393 QCOMPARE(result, true);
394
395- selectors = engine->parseSelector(".root#id > .testB .baseB");
396+ selectors = engine->parseSelector(".root#id>.testB .baseB");
397 // should match
398 result = (selectors.count() == 1) && (selectors[0] == expected);
399 QCOMPARE(result, true);
400
401 selectors = engine->parseSelector(".root#id > .testB .baseB, .oneNode.bing .baseC");
402- expected2 << SelectorNode("oneNode.bing", "", SelectorNode::Descendant);
403- expected2 << SelectorNode("baseC", "", SelectorNode::Descendant);
404+ Selector expected2("oneNode.bing baseC");
405 result = (selectors.count() == 2) &&
406 (selectors[0] == expected) &&
407 (selectors[1] == expected2);
408@@ -247,44 +232,28 @@
409 void tst_ThemeEnginePrivate::testCase_selectorToString()
410 {
411 engine->errorString = QString();
412- bool result = true;
413 Selector selector;
414 QString expected;
415
416- selector.clear();
417- selector << SelectorNode("classA", "", SelectorNode::Descendant);
418- selector << SelectorNode("classB", "", SelectorNode::Descendant);
419+ selector = Selector(".classA .classB");
420 expected = ".classa .classb";
421- result = engine->selectorToString(selector) == expected;
422- QCOMPARE(result, true);
423-
424- selector.clear();
425- selector << SelectorNode("classA", "", SelectorNode::Descendant);
426- selector << SelectorNode("classB", "", SelectorNode::Child);
427- expected = ".classa > .classb";
428- result = engine->selectorToString(selector) == expected;
429- QCOMPARE(result, true);
430-
431- selector.clear();
432- selector << SelectorNode("classA", "id", SelectorNode::Descendant);
433- selector << SelectorNode("classB", "", SelectorNode::Descendant);
434+ QCOMPARE(selector.toString(), expected);
435+
436+ selector = Selector(".classA > .classB");
437+ expected = ".classa>.classb";
438+ QCOMPARE(selector.toString(), expected);
439+
440+ selector = Selector(".classA#id .classB");
441 expected = ".classa#id .classb";
442- result = engine->selectorToString(selector) == expected;
443- QCOMPARE(result, true);
444-
445- selector.clear();
446- selector << SelectorNode("classA", "", SelectorNode::Descendant);
447- selector << SelectorNode("classB", "id", SelectorNode::Child);
448- expected = ".classa > .classb#id";
449- result = engine->selectorToString(selector) == expected;
450- QCOMPARE(result, true);
451-
452- selector.clear();
453- selector << SelectorNode("classA.attribute", "", SelectorNode::Descendant);
454- selector << SelectorNode("classB", "id", SelectorNode::Child);
455- expected = ".classa.attribute > .classb#id";
456- result = engine->selectorToString(selector) == expected;
457- QCOMPARE(result, true);
458+ QCOMPARE(selector.toString(), expected);
459+
460+ selector = Selector(".classA > .classB#id");
461+ expected = ".classa>.classb#id";
462+ QCOMPARE(selector.toString(), expected);
463+
464+ selector = Selector(".classA.attribute>.classB#id");
465+ expected = ".classa.attribute>.classb#id";
466+ QCOMPARE(selector.toString(), expected);
467 }
468
469 QTEST_MAIN(tst_ThemeEnginePrivate)

Subscribers

People subscribed via source and target branches

to status/vote changes: