<table><tr><td style="">ahiemstra requested changes to this revision.<br />ahiemstra added a comment.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D21721">View Revision</a></tr></table><br /><div><div><p>I have probably missed stuff, but here are my initial comments.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132793">View Inline</a><span style="color: #4b4d51; font-weight: bold;">atticaprovider.cpp:355</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="n">QList</span><span style="color: #aa2211"><</span><span class="n">std</span><span style="color: #aa2211">::</span><span class="n">shared_ptr</span><span style="color: #aa2211"><</span><span class="n">KNSCore</span><span style="color: #aa2211">::</span><span class="n">Comment</span><span style="color: #aa2211">>></span> <span class="n">getCommentsList</span><span class="p">(</span><span style="color: #aa4000">const</span> <span class="n">Attica</span><span style="color: #aa2211">::</span><span class="n">Comment</span><span style="color: #aa2211">::</span><span class="n">List</span> <span style="color: #aa2211">&</span><span class="n">comments</span><span class="p">,</span> <span class="n">std</span><span style="color: #aa2211">::</span><span class="n">shared_ptr</span><span style="color: #aa2211"><</span><span class="n">KNSCore</span><span style="color: #aa2211">::</span><span class="n">Comment</span><span style="color: #aa2211">></span> <span class="n">parent</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QList</span><span style="color: #aa2211"><</span><span class="n">std</span><span style="color: #aa2211">::</span><span class="n">shared_ptr</span><span style="color: #aa2211"><</span><span class="n">KNSCore</span><span style="color: #aa2211">::</span><span class="n">Comment</span><span style="color: #aa2211">>></span> <span class="n">knsComments</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Since upstream Qt is starting to discourage usage of QList, maybe use QVector instead.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132791">View Inline</a><span style="color: #4b4d51; font-weight: bold;">atticaprovider.cpp:359</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">qDebug</span><span class="p">()</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"Appending comment with id"</span> <span style="color: #aa2211"><<</span> <span class="n">comment</span><span class="p">.</span><span class="n">id</span><span class="p">()</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">", which has"</span> <span style="color: #aa2211"><<</span> <span class="n">comment</span><span class="p">.</span><span class="n">childCount</span><span class="p">()</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"children"</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">std</span><span style="color: #aa2211">::</span><span class="n">shared_ptr</span><span style="color: #aa2211"><</span><span class="n">KNSCore</span><span style="color: #aa2211">::</span><span class="n">Comment</span><span style="color: #aa2211">></span> <span class="n">knsComment</span><span class="p">(</span><span style="color: #aa4000">new</span> <span class="n">KNSCore</span><span style="color: #aa2211">::</span><span class="n">Comment</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">knsComment</span><span style="color: #aa2211">-></span><span class="n">id</span> <span style="color: #aa2211">=</span> <span class="n">comment</span><span class="p">.</span><span class="n">id</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Generally, it's recommended to use <tt style="background: #ebebeb; font-size: 13px;">std::shared_ptr<KNSCore::Comment> knsComment = std::make_shared<KNSCore::Comment>()</tt>. As a bonus, that allows you to use <tt style="background: #ebebeb; font-size: 13px;">auto knsComment</tt>.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132792">View Inline</a><span style="color: #4b4d51; font-weight: bold;">atticaprovider.cpp:370</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">comment</span><span class="p">.</span><span class="n">childCount</span><span class="p">()</span> <span style="color: #aa2211">></span> <span style="color: #601200">0</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">qDebug</span><span class="p">()</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"Getting more comments, as this one has children, and we currently have this number of comments:"</span> <span style="color: #aa2211"><<</span> <span class="n">knsComments</span><span class="p">.</span><span class="n">count</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">knsComments</span> <span style="color: #aa2211"><<</span> <span class="n">getCommentsList</span><span class="p">(</span><span class="n">comment</span><span class="p">.</span><span class="n">children</span><span class="p">(),</span> <span class="n">knsComment</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Probably better to use categorised logging for this?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132794">View Inline</a><span style="color: #4b4d51; font-weight: bold;">atticaprovider.cpp:410</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">std</span><span style="color: #aa2211">::</span><span class="n">shared_ptr</span><span style="color: #aa2211"><</span><span class="n">KNSCore</span><span style="color: #aa2211">::</span><span class="n">Author</span><span style="color: #aa2211">></span> <span class="n">author</span><span class="p">(</span><span style="color: #aa4000">new</span> <span class="n">KNSCore</span><span style="color: #aa2211">::</span><span class="n">Author</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">author</span><span style="color: #aa2211">-></span><span class="n">setId</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">property</span><span class="p">(</span><span style="color: #766510">"username"</span><span class="p">).</span><span class="n">toString</span><span class="p">());</span> <span style="color: #74777d">// This is a touch hack-like, but it ensures we actually have the data in case it is not returned by the server</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Like above, std::make_shared<KNSCore::Author></p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132795">View Inline</a><span style="color: #4b4d51; font-weight: bold;">commentsmodel.cpp:135</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">QVariant</span> <span class="n">value</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">index</span><span class="p">.</span><span class="n">isValid</span><span class="p">()</span> <span style="color: #aa2211">&&</span> <span class="n">index</span><span class="p">.</span><span class="n">row</span><span class="p">()</span> <span style="color: #aa2211"><</span> <span class="n">d</span><span style="color: #aa2211">-></span><span class="n">comments</span><span class="p">.</span><span class="n">count</span><span class="p">())</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Might want to add a call to checkIndex here (<a href="https://doc.qt.io/qt-5/qabstractitemmodel.html#checkIndex" class="remarkup-link" target="_blank" rel="noreferrer">https://doc.qt.io/qt-5/qabstractitemmodel.html#checkIndex</a>), it would obsolete some additional checking you do later.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132799">View Inline</a><span style="color: #4b4d51; font-weight: bold;">commentsmodel.cpp:177</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa2211">++</span><span class="n">depth</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">child</span><span style="color: #aa2211">-></span><span class="n">parent</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">child</span> <span style="color: #aa2211">=</span> <span class="n">child</span><span style="color: #aa2211">-></span><span class="n">parent</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Since you're already checking for <tt style="background: #ebebeb; font-size: 13px;">child</tt> being true (aka not-null), this if becomes rather superfluous.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132820">View Inline</a><span style="color: #4b4d51; font-weight: bold;">commentsmodel.h:50</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> *</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * This model should preferably be constructed by asking the Engine to give a model</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> * instance to you for a specific entry using the commentsForEntry function. If you</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Considering this is intended to be used from QML, I would drop this, especially since the current code in engine does nothing special. And changing this would allow for doing <tt style="background: #ebebeb; font-size: 13px;">CommentsModel { entry: someEntry }</tt> from QML.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132804">View Inline</a><span style="color: #4b4d51; font-weight: bold;">author.cpp:103</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">d</span><span style="color: #aa2211">-></span><span class="n">engine</span> <span style="color: #aa2211">=</span> <span class="n">qobject_cast</span><span style="color: #aa2211"><</span><span class="n">Engine</span><span style="color: #aa2211">*></span><span class="p">(</span><span class="n">newEngine</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">d</span><span style="color: #aa2211">-></span><span class="n">resetConnections</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">You should check if the actual property value has changed before changing it. I generally do something along the lines of <tt style="background: #ebebeb; font-size: 13px;">if (newEngine == d->engine) return;</tt>. This helps with preventing binding loops in properties exposed to QML.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132808">View Inline</a><span style="color: #4b4d51; font-weight: bold;">Button.qml:39</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #004012">property</span> <span style="color: #004012">string</span> <span style="color: #aa4000">configFile:</span> <span style="color: #004012">ghnsDialog</span><span class="p">.</span><span style="color: #004012">configFile</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Any particular reason not to alias these directly to the dialog?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132805">View Inline</a><span style="color: #4b4d51; font-weight: bold;">Dialog.qml:34</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">import</span> <span style="color: #004012">QtQuick</span><span class="p">.</span><span style="color: #004012">Dialogs</span> <span style="color: #601200">1.3</span> <span style="color: #004012">as</span> <span style="color: #004012">QtDialogs</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">import</span> <span style="color: #004012">QtQuick</span><span class="p">.</span><span style="color: #004012">Layouts</span> <span style="color: #601200">1.12</span> <span style="color: #004012">as</span> <span style="color: #004012">QtLayouts</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #304a96">import</span> <span style="color: #004012">org</span><span class="p">.</span><span style="color: #004012">kde</span><span class="p">.</span><span style="color: #004012">newstuff</span> <span style="color: #601200">1.1</span> <span style="color: #004012">as</span> <span style="color: #004012">NewStuff</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">If you're using a Qt 5.12 import anyway, might want to unify the other imports to also use .12 versions.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132807">View Inline</a><span style="color: #4b4d51; font-weight: bold;">Dialog.qml:76</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span style="color: #74777d"> */</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #004012">signal</span> <span style="color: #004012">dialogFinished</span><span class="p">(</span><span style="color: #aa4000">var</span> <span style="color: #004012">changedEntries</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Any particular reason for using a signal parameter here instead of just setting a property on the dialog? Using a property generally allows for more declarative use of the item.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132810">View Inline</a><span style="color: #4b4d51; font-weight: bold;">DownloadItemsSheet.qml:49</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #004012">Kirigami</span><span class="p">.</span><span style="color: #004012">AbstractCard</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">QtLayouts.Layout.leftMargin:</span> <span style="color: #004012">Kirigami</span><span class="p">.</span><span style="color: #004012">Units</span><span class="p">.</span><span style="color: #004012">iconSizes</span><span class="p">.</span><span style="color: #004012">smallMedium</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Design wise, I don't think having a card inside an overlaysheet looks too nice. I'm also not sure if the extra attention is needed, so maybe just use a label?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132818">View Inline</a><span style="color: #4b4d51; font-weight: bold;">Page.qml:210</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #004012">Component</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">id: bigPreviewDelegate</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">For readability, it would be better to move these to their own files.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21721#inline-132819">View Inline</a><span style="color: #4b4d51; font-weight: bold;">qmlplugin.cpp:54</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Q_UNUSED</span><span class="p">(</span><span class="n">scriptEngine</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="n">KNewStuffQuick</span><span style="color: #aa2211">::</span><span class="n">QuickQuestionListener</span><span style="color: #aa2211">::</span><span class="n">instance</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="p">});</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Note that this makes the qml engine take ownvership of your singleton instance and it will try to delete it when the engine gets deleted. You probably want to do <tt style="background: #ebebeb; font-size: 13px;">engine->setOwnership(instance, QQmlEngine::CppOwnership)</tt> before returning the instance.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R304 KNewStuff</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D21721">https://phabricator.kde.org/D21721</a></div></div><br /><div><strong>To: </strong>leinir, KNewStuff, VDG, Frameworks, ahiemstra<br /><strong>Cc: </strong>ahiemstra, anthonyfieroni, pino, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns<br /></div>