D6067: Make it possible to use QXmlStreamReader to read a KNS registry file
This revision was automatically updated to reflect the committed changes. Closed by commit R304:00aa6b29644b: Make it possible to use QXmlStreamReader to read a KNS registry file (authored by apol). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6067?vs=15176&id=15218 REVISION DETAIL https://phabricator.kde.org/D6067 AFFECTED FILES autotests/knewstuffentrytest.cpp src/core/cache.cpp src/core/entryinternal.cpp src/core/entryinternal.h To: apol, #frameworks, leinir Cc: leinir, dfaure
D6067: Make it possible to use QXmlStreamReader to read a KNS registry file
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. In https://phabricator.kde.org/D6067#114528, @apol wrote: > In https://phabricator.kde.org/D6067#114498, @leinir wrote: > > > On a similar note to handling comments, how does it now handle unknown/garbage tags? While it won't affect the cache code, it would potentially affect other things (ocs is not guaranteed to be perfectly formed, and it's one of the ways the framework's retained backwards compatibility). From what i can tell, this would assert when an unknown tag is encountered, right? > > > This is for .knsregistry files, not OCS. These files are generated by the very same class here. You are quite right, i forgot the attica content's not parsed using knscore's functions, just the staticxml provider... because i'm a silly person :) Obviously just fine, then, go for it! REPOSITORY R304 KNewStuff BRANCH master REVISION DETAIL https://phabricator.kde.org/D6067 To: apol, #frameworks, leinir Cc: leinir, dfaure
D6067: Make it possible to use QXmlStreamReader to read a KNS registry file
apol added a comment. In https://phabricator.kde.org/D6067#114498, @leinir wrote: > On a similar note to handling comments, how does it now handle unknown/garbage tags? While it won't affect the cache code, it would potentially affect other things (ocs is not guaranteed to be perfectly formed, and it's one of the ways the framework's retained backwards compatibility). From what i can tell, this would assert when an unknown tag is encountered, right? This is for .knsregistry files, not OCS. These files are generated by the very same class here. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D6067 To: apol, #frameworks Cc: leinir, dfaure
D6067: Make it possible to use QXmlStreamReader to read a KNS registry file
leinir added a comment. On a similar note to handling comments, how does it now handle unknown/garbage tags? While it won't affect the cache code, it would potentially affect other things (ocs is not guaranteed to be perfectly formed, and it's one of the ways the framework's retained backwards compatibility). From what i can tell, this would assert when an unknown tag is encountered, right? REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D6067 To: apol, #frameworks Cc: leinir, dfaure
D6067: Make it possible to use QXmlStreamReader to read a KNS registry file
apol updated this revision to Diff 15176. apol added a comment. More testing, fix issues REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6067?vs=15172&id=15176 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6067 AFFECTED FILES autotests/knewstuffentrytest.cpp src/core/cache.cpp src/core/entryinternal.cpp src/core/entryinternal.h To: apol, #frameworks Cc: dfaure
D6067: Make it possible to use QXmlStreamReader to read a KNS registry file
apol updated this revision to Diff 15172. apol added a comment. Adds test, better parsing when there's comments, addresses comments by dfaure REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6067?vs=15096&id=15172 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6067 AFFECTED FILES autotests/knewstuffentrytest.cpp src/core/cache.cpp src/core/entryinternal.cpp src/core/entryinternal.h To: apol, #frameworks Cc: dfaure
D6067: Make it possible to use QXmlStreamReader to read a KNS registry file
dfaure added inline comments.
INLINE COMMENTS
> entryinternal.cpp:443
> +{
> +Q_ASSERT(xml->tokenType()==QXmlStreamReader::StartElement);
> +QStringRef ret;
missing spaces around ==
> entryinternal.cpp:446
> +const auto token = xml->readNext();
> +if (token==QXmlStreamReader::Characters) {
> +ret = xml->text();
same
> entryinternal.cpp:450
> +}
> +Q_ASSERT(xml->tokenType()==QXmlStreamReader::EndElement);
> +return ret;
same
> entryinternal.cpp:527
> +
> +Q_ASSERT(reader.tokenType() == QXmlStreamReader::EndElement);
> +
Did you test parsing a file with `` comments here and there? Just
to make sure the code handles this correctly and doesn't assert.
> entryinternal.h:391
> + */
> +KNEWSTUFFCORE_DEPRECATED bool setEntryXML(const QDomElement &xmldata);
>
"@deprecated since 5.36, use setEntryXML(QXmlStreamReader&) instead"
REPOSITORY
R304 KNewStuff
REVISION DETAIL
https://phabricator.kde.org/D6067
To: apol, #frameworks
Cc: dfaure
D6067: Make it possible to use QXmlStreamReader to read a KNS registry file
apol created this revision. Restricted Application added a project: Frameworks. REVISION SUMMARY It's more efficient, especially in memory TEST PLAN tests pass, discover kns test passes, discover works REPOSITORY R304 KNewStuff BRANCH master REVISION DETAIL https://phabricator.kde.org/D6067 AFFECTED FILES src/core/cache.cpp src/core/entryinternal.cpp src/core/entryinternal.h To: apol, #frameworks
