D6513: Add support for Attica tags support

2018-09-21 Thread Dan Leinir Turthra Jensen
leinir added a comment. For ease of re-reviewing, the relevant bits of the patch (the bits that didn't already get the tickmark) can be seen on their own like so: https://phabricator.kde.org/D6513?vs=41133&id=41886&whitespace=ignore-most#toc (thank you very much differential for being a

D15591: Add Open Document thumbnailer

2018-09-18 Thread Dan Leinir Turthra Jensen
leinir added a comment. In D15591#328150 , @broulik wrote: > [...]actually uses KOffice classes ...that doesn't seem likely, that'd be like Plasma using Kicker classes from KDE3 times, which is the last time Calligra was called th

D6513: Add support for Attica tags support

2018-09-18 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 41886. leinir added a comment. Forgot `@since 5.51` in the new api in Provider.h REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6513?vs=41815&id=41886 REVISION DETAIL https://phabricator.kde.org/D6513 AFFECTED FILE

D6513: Add support for Attica tags support

2018-09-17 Thread Dan Leinir Turthra Jensen
leinir requested review of this revision. leinir added a comment. Previous patch had BIC issues - new patch attempts to address this, but as a result (of course) requires another round of review. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D6513 To: leinir, #kn

D6513: Add support for Attica tags support

2018-09-17 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 41815. leinir added a comment. After discovering that the previous version of this patch had introduced a binary incompatibility, and panicking momentarily that this had been done in a release, Jonathan reverted it (as i was away for a bit and unable to do

D6513: Add support for Attica tags support

2018-09-15 Thread Dan Leinir Turthra Jensen
leinir reopened this revision. leinir added a comment. This revision is now accepted and ready to land. In D6513#326519 , @rikmills wrote: > In D6513#326071 , @ngraham wrote: > > > FYI, this included an

D6513: Add support for Attica tags support

2018-09-13 Thread Dan Leinir Turthra Jensen
leinir added a comment. In D6513#324915 , @ngraham wrote: > In D6513#324458 , @leinir wrote: > > > In D6513#324419 , @ngraham wrote: > > > > > So I see `gh

D15407: Fix OCS provider URL in about dialog

2018-09-12 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. This revision is now accepted and ready to land. REPOSITORY R263 KXmlGui REVISION DETAIL https://phabricator.kde.org/D15407 To: broulik, #frameworks, leinir, alexanderschmidt Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D6513: Add support for Attica tags support

2018-09-11 Thread Dan Leinir Turthra Jensen
leinir added a comment. In D6513#324419 , @ngraham wrote: > In D6513#322167 , @leinir wrote: > > > In D6513#322078 , @ngraham wrote: > > > > > So is this e

D6513: Add support for Attica tags support

2018-09-08 Thread Dan Leinir Turthra Jensen
leinir added a comment. In D6513#322078 , @ngraham wrote: > So is this enough for people to start tagging KDE4 content as such? Or is anything else still required before that capability lands? In short, this is basically enough :) When peo

D6513: Add support for Attica tags support

2018-09-07 Thread Dan Leinir Turthra Jensen
leinir added inline comments. INLINE COMMENTS > dfaure wrote in tagsfilterchecker.cpp:103 > Yes. Great, thank you :) Yeah, going to have to nab a copy of that for a readthrough :) REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D6513 To: leinir, #knewstuff, apol, #k

D6513: Add support for Attica tags support

2018-09-07 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R304:2ad3e66d81b6: Add support for Attica tags support (authored by leinir). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6513?vs=41131&id=41133 REVISION DETAIL h

D6513: Add support for Attica tags support

2018-09-07 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 41131. leinir marked 2 inline comments as done. leinir added a comment. Address @dfaure's comments REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6513?vs=41034&id=41131 REVISION DETAIL https://phabricator.kde.org/D6

D6513: Add support for Attica tags support

2018-09-07 Thread Dan Leinir Turthra Jensen
leinir marked 6 inline comments as done. leinir added inline comments. INLINE COMMENTS > dfaure wrote in tagsfilterchecker.cpp:103 > .insert(tag, val) is raster, see Effective STL Scott Meyers' book? (might need to grab a copy of that, then :) ) > dfaure wrote in CMakeLists.txt:4 > Qick ? :) O

D6513: Add support for Attica tags support

2018-09-05 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 41034. leinir marked an inline comment as done. leinir added a comment. More codestyle fixes. I also notice some which were there before this work, but fixing that seems distinctly out of scope for this patch... REPOSITORY R304 KNewStuff CHANGES SINCE L

D6513: Add support for Attica tags support

2018-09-05 Thread Dan Leinir Turthra Jensen
leinir marked 16 inline comments as done. leinir added a comment. i guess uncrustify isn't a magic bullet either, eh? ;) Thanks for the findings! INLINE COMMENTS > cfeck wrote in khotnewstuff_test.cpp:66 > Any rationale for using `fromLocal8Bit()` for fixed strings? If, for whatever > reaso

D6513: Add support for Attica tags support

2018-09-04 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 40981. leinir added a comment. Uncrustify-kf5 things as requested, hope this is better! REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6513?vs=39174&id=40981 REVISION DETAIL https://phabricator.kde.org/D6513 AFFECT

D6512: Add support for proposed tags addition in OCS 1.7

2018-08-29 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R235:76f66bab8841: Add support for proposed tags addition in OCS 1.7 (authored by leinir). REPOSITORY R235 Attica CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6512?vs=39175&id=40648 REVISION

D6513: Add support for Attica tags support

2018-08-29 Thread Dan Leinir Turthra Jensen
leinir added a comment. Welcome back from Akademy - sorry for being a pest, but it'd be really nifty if we could get this and it's dependent in D6512 through before 5.50... :) REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D6513

D6512: Add support for proposed tags addition in OCS 1.7

2018-08-06 Thread Dan Leinir Turthra Jensen
leinir requested review of this revision. REPOSITORY R235 Attica REVISION DETAIL https://phabricator.kde.org/D6512 To: leinir, #knewstuff, apol, whiting, #kde_store, ahiemstra Cc: cfeck, ahiemstra, ngraham, kde-frameworks-devel, #kde_store, michaelh, ZrenBot, bruns, akiraohgaki, alexandersc

D6512: Add support for proposed tags addition in OCS 1.7

2018-08-06 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 39175. leinir added a comment. Address comments by cfeck Also mark new public API as @since 5.50 REPOSITORY R235 Attica CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6512?vs=16200&id=39175 REVISION DETAIL https://phabricator.kde.org/D6512

D6512: Add support for proposed tags addition in OCS 1.7

2018-08-06 Thread Dan Leinir Turthra Jensen
leinir marked 2 inline comments as done. REPOSITORY R235 Attica REVISION DETAIL https://phabricator.kde.org/D6512 To: leinir, #knewstuff, apol, whiting, #kde_store, ahiemstra Cc: cfeck, ahiemstra, ngraham, kde-frameworks-devel, #kde_store, michaelh, ZrenBot, bruns, akiraohgaki, alexandersch

D6513: Add support for Attica tags support

2018-08-06 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 39174. leinir marked 3 inline comments as done. leinir added a comment. Address the various comments by dfaure, cfeck and ngraham - with thanks! REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6513?vs=38506&id=39174 RE

D6513: Add support for Attica tags support

2018-08-06 Thread Dan Leinir Turthra Jensen
leinir marked 17 inline comments as done. leinir added inline comments. INLINE COMMENTS > dfaure wrote in engine.cpp:133 > That's out of bounds, if tagFilter is empty!! It will assert. > > You meant append or push_back, I think. i did indeed! > dfaure wrote in engine.cpp:138 > !? What's the po

D6513: Add support for Attica tags support

2018-08-01 Thread Dan Leinir Turthra Jensen
leinir added a comment. @mlaurent quick ping? :) Also in general, sorry, would really like to get this merged in asap - i realise it's the summer holidays for a lot of people ;) REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D6513 To: leinir, #knewstuff, apol

D6513: Add support for Attica tags support

2018-07-26 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 38506. leinir marked an inline comment as done. leinir added a comment. Address mlaurent's comments REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6513?vs=38489&id=38506 REVISION DETAIL https://phabricator.kde.org/D

D6513: Add support for Attica tags support

2018-07-26 Thread Dan Leinir Turthra Jensen
leinir marked 3 inline comments as done. leinir added inline comments. INLINE COMMENTS > mlaurent wrote in entryinternal.cpp:495 > qCWarning(KNEWSTUFF_CORE) ? I was just adding in a bit of verbosity to the output... but yes, makes sense to fix that while i'm there anyway :) REPOSITORY R304 K

D6513: Add support for Attica tags support

2018-07-26 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 38489. leinir marked 4 inline comments as done. leinir added a comment. Address ahiemstra's comments REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6513?vs=37047&id=38489 REVISION DETAIL https://phabricator.kde.org/

D6513: Add support for Attica tags support

2018-07-26 Thread Dan Leinir Turthra Jensen
leinir marked 7 inline comments as done. leinir added inline comments. INLINE COMMENTS > ahiemstra wrote in atticaprovider.cpp:283 > Wouldn't it be simpler to first check if the content should be considered at > all, and then check if one of the downloads is valid? So something like: > > if(!

D6512: Add support for proposed tags addition in OCS 1.7

2018-07-24 Thread Dan Leinir Turthra Jensen
leinir added a comment. In D6512#296329 , @ahiemstra wrote: > T6133 suggests that tags are formatted as "group##key=value" or something similar. Wouldn't it make sense to handle parsing that format here as well

D14097: Change default sort order in the download dialog to Rating

2018-07-16 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. As an aside, I'd probably like this to end up being remembered between sessions, but this no different to the current state, and so outside the scope of this patch. As a default setting, yu

D14095: Fix DownloadDialog window margins to meet general theme margins

2018-07-16 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. In principle, this would really want to go via the designers, but at the same time it does seem that this was what was originally intended anyway. Shippit :) REPOSITORY R304 KNewStuff BR

D6513: Add support for Attica tags support

2018-07-12 Thread Dan Leinir Turthra Jensen
leinir added a comment. Pingaling, it would be super handy if we could get this merged soon :) It's needed to support the moderation happening on the KDE Store, and also needed for better handling of specific types of content like e.g. comic books in Peruse. Thanks! :D REPOSITORY R304 KNe

D6512: Add support for proposed tags addition in OCS 1.7

2018-07-12 Thread Dan Leinir Turthra Jensen
leinir added a comment. Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. Ping, if we could get this merged soon, it would be terribly handy :) (it's the basis for moderating the ghns content, and needed for the knewstuff patch in D6513

D6513: Add support for Attica tags support

2018-07-02 Thread Dan Leinir Turthra Jensen
leinir added a comment. Test tool using the default settings, and with QT__LOGGING_RULES="org.kde.knewstuff.*=true" F5998853: image.png REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D6513 To: leinir, #knewstuff, apol, #kde

D6513: Add support for Attica tags support

2018-07-02 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 37047. leinir edited the summary of this revision. leinir edited the test plan for this revision. leinir added a comment. Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. - add test tool - add support for filter

D13732: Add debug output for all network requests

2018-06-28 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. Sorted! Good digging, nice work :) REPOSITORY R235 Attica BRANCH master REVISION DETAIL https://phabricator.kde.org/D13732 To: habacker, leinir Cc: ltoscano, bruns, kde-frameworks-d

D13743: Migrate build system to use find_package in autotests/ki18n_install

2018-06-27 Thread Dan Leinir Turthra Jensen
leinir added a comment. This seems quite sensible to me, a cleanup and simplification of the test build system is not bad... That said, i'm unsure why you think i'm qualified to speak about ki18n and the general cmake build system to a level high enough that you highlight me specificall

D13732: Add debug output for all network requests

2018-06-27 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision. leinir added a comment. This revision now requires changes to proceed. Straight-up qDebug would cause quite a lot of spam... I'd be much happier if this was done using qCDebug (which is already used in other parts of the code, so just need to include

D13733: Add provider auto test

2018-06-27 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. This revision is now accepted and ready to land. REPOSITORY R235 Attica BRANCH master REVISION DETAIL https://phabricator.kde.org/D13733 To: habacker, leinir Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13733: Add provider auto test

2018-06-26 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision. leinir added a comment. This revision now requires changes to proceed. Autotests are good, but so is documentation - in principle this is good, but new public functions without documentation isn't really acceptable :) Apart from that, though, looks

D13714: Fix broken url to API specification

2018-06-25 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. This revision is now accepted and ready to land. REPOSITORY R235 Attica BRANCH master REVISION DETAIL https://phabricator.kde.org/D13714 To: habacker, mlaurent, leinir Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13515: Remove KNS::Engine d-pointer hack

2018-06-13 Thread Dan Leinir Turthra Jensen
leinir added a comment. H... this is tricky, but... yes, it seems reasonable to me that replacing one pointer with another would cause no BIC issues... and i certainly am happy to get rid of the d-pointer hack, so... if we're absolutely super-sure that this is BC, i'm all for it... would

D12082: Don't offer qml plugin as a link target

2018-04-11 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R304:469db5c6934b: Don't offer qml plugin as a link target (authored by leinir). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12082?vs=31790&id=31859 REVISION DETAI

D12082: Don't offer qml plugin as a link target

2018-04-10 Thread Dan Leinir Turthra Jensen
leinir created this revision. leinir added reviewers: Frameworks, KNewStuff. leinir added a project: KNewStuff. Restricted Application added a project: Frameworks. leinir requested review of this revision. REVISION SUMMARY This removes the KNSQuick plugin as a link target for cmake. It was never

Re: KNewStuff - CMake targets hardcoding paths

2018-04-08 Thread Dan Leinir Turthra Jensen
On Sunday, 8 April 2018 03:30:15 BST Ben Cooksley wrote: > Hi all, > > On Windows the Binary Factory has recently struck an issue with > KNewStuff where the paths supplied in the newstuffqmlplugin target it > installs are hardcoded. > > This can be seen at the bottom of > https://binary-factory.k

D11173: Actually vote when clicking stars in the list view

2018-03-09 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R304:77e5c43a881c: Actually vote when clicking stars in the list view (authored by leinir). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11173?vs=29066&id=29076 REV

D11172: Explicitly set content type to form data

2018-03-09 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R235:dfdbf55a4cda: Explicitly set content type to form data (authored by leinir). REPOSITORY R235 Attica CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11172?vs=29065&id=29075 REVISION DETAIL

D11172: Explicitly set content type to form data

2018-03-09 Thread Dan Leinir Turthra Jensen
leinir added a comment. In D11172#221854 , @apol wrote: > Which warning? How does changing to "application/x-www-form-urlencoded" help? Sorry, i forgot to add it in the description. Specifically, this warning: content-type missing in HT

D11173: Actually vote when clicking stars in the list view

2018-03-09 Thread Dan Leinir Turthra Jensen
leinir retitled this revision from "Actually vote when clicking stars in the details view" to "Actually vote when clicking stars in the list view". leinir edited the summary of this revision. REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D11173 To: leinir, #knewstuff

D11173: Actually vote when clicking stars in the details view

2018-03-09 Thread Dan Leinir Turthra Jensen
leinir created this revision. leinir added reviewers: KNewStuff, Frameworks, sitter. leinir added a project: KNewStuff. Restricted Application added a project: Frameworks. leinir requested review of this revision. REVISION SUMMARY This connects up the ratings widget with the engine, using the sa

D11172: Explicitly set content type to form data

2018-03-09 Thread Dan Leinir Turthra Jensen
leinir created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. leinir requested review of this revision. REVISION SUMMARY Without this patch, Attica will make a lot of noise when interacting with an OCS server, in the for

D11172: Explicitly set content type to form data

2018-03-09 Thread Dan Leinir Turthra Jensen
leinir added a reviewer: Frameworks. REPOSITORY R235 Attica REVISION DETAIL https://phabricator.kde.org/D11172 To: leinir, #frameworks Cc: #frameworks, michaelh

D10017: Find Aspell dictionaries on Windows

2018-01-22 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R246:9038a8effc41: Find Aspell dictionaries on Windows (authored by leinir). REPOSITORY R246 Sonnet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10017?vs=25749&id=25784 REVISION DETAIL htt

D10017: Find Aspell dictionaries on Windows

2018-01-22 Thread Dan Leinir Turthra Jensen
leinir created this revision. leinir added a reviewer: vonreth. leinir added a project: Frameworks. Restricted Application added a subscriber: Frameworks. leinir requested review of this revision. REVISION SUMMARY This feeds the sonnet aspell plugin some information to assist it in locating dic

D9421: Remove anchient and broken workaround

2017-12-20 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. Ha, yes, i concur with Aleix there, workaround-be-gone! ;) (i have a suspicion the reason i thought this worked before is because i was testing on my local machine... which also explains nicely why the packages from BF don't have functio

D9018: Don't cause circular linking on Windows

2017-12-20 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R246:bdc6562faf73: Don't cause circular linking on Windows (authored by leinir). REPOSITORY R246 Sonnet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D9018?vs=23028&id=24150 REVISION DETAIL

D9018: Don't cause circular linking on Windows

2017-12-19 Thread Dan Leinir Turthra Jensen
leinir added a comment. Would it be possible to get an accept here, or is there something people fundamentally disagree with about it? REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D9018 To: leinir, #frameworks Cc: vonreth, cgiboudeaux, alexeymin, apol, #frameworks

D8311: Require the same internal version as you're building

2017-12-18 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R304:e33d06f9857a: Require the same internal version as you're building (authored by leinir). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8311?vs=20791&id=24076 RE

D9398: Fix TagLib detection and build on Windows

2017-12-18 Thread Dan Leinir Turthra Jensen
leinir added a reviewer: Frameworks. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D9398 To: leinir, #frameworks

D9018: Don't cause circular linking on Windows

2017-11-28 Thread Dan Leinir Turthra Jensen
leinir added a comment. In https://phabricator.kde.org/D9018#172789, @alexeymin wrote: > So it resulted in something like aspell.dll requiring aspell.dll? Exactly that, yes... took me running it through Dependency Walker to work out what was going on :P > What is the

D9012: Revert "Detach before setting the d pointer"

2017-11-27 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. This revision is now accepted and ready to land. REPOSITORY R304 KNewStuff BRANCH unbreakEntryInternalDataSyncing REVISION DETAIL https://phabricator.kde.org/D9012 To: kossebau, whiting, leinir, apol Cc: #frameworks

D9018: Don't cause circular linking on Windows

2017-11-27 Thread Dan Leinir Turthra Jensen
leinir added a reviewer: Frameworks. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D9018 To: leinir, #frameworks Cc: #frameworks

D9018: Don't cause circular linking on Windows

2017-11-27 Thread Dan Leinir Turthra Jensen
leinir created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY When building plugins, don't arbitrarily rename the output files (as this will occasionally result in circular dependencies). In this re

D9012: Revert "Detach before setting the d pointer"

2017-11-27 Thread Dan Leinir Turthra Jensen
leinir added a comment. Quicker is better here, i think... Perhaps it is worth adding the documentation we discussed as well in this review? Thinking about making it easier to track the history and whatnot of what happened and why... REPOSITORY R304 KNewStuff REVISION DETAIL https://pha

D8811: [knewstuff] Do not leak ImageLoader on error

2017-11-16 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. Looks good to me :) REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D8811 To: anthonyfieroni, leinir, dfaure Cc: broulik, #frameworks, ZrenBot

D8811: [knewstuff] Do not leak ImageLoader on error

2017-11-15 Thread Dan Leinir Turthra Jensen
leinir added inline comments. INLINE COMMENTS > anthonyfieroni wrote in engine.cpp:536 > Job can be nullptr, so signal can be a text, it can capture entry by > reference? i guess you could extend the signal further with the error text added in instead of capturing l in the lambda, yes REPOSIT

D8811: [knewstuff] Do not leak ImageLoader on error

2017-11-15 Thread Dan Leinir Turthra Jensen
leinir added inline comments. INLINE COMMENTS > engine.cpp:536 > +connect(l, &ImageLoader::signalError, this, [this, l](const > KNSCore::EntryInternal &entry, EntryInternal::PreviewType type) { > +qCDebug(KNEWSTUFFCORE) << "ERROR preview: " << entry.name() << type; > +--m_num

D8811: [knewstuff] Do not leak ImageLoader on error

2017-11-15 Thread Dan Leinir Turthra Jensen
leinir added inline comments. INLINE COMMENTS > engine.cpp:536 > +connect(l, &ImageLoader::signalError, this, [=]() { > +qCDebug(KNEWSTUFFCORE) << "ERROR preview: " << entry.name() << type; > +--m_numPictureJobs; A more descriptive error message would be good, but this is alr

D8188: Remove PreferCache from network requests

2017-10-27 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. Right, yes, i think that was a bit of left over from working on it all. Go for it. REPOSITORY R304 KNewStuff BRANCH master REVISION DETAIL https://phabricator.kde.org/D8188 To: aac

D7194: Detach before setting the d pointer

2017-10-27 Thread Dan Leinir Turthra Jensen
leinir added a comment. This already went into one release, and it would be quite useful to get it sorted before the next one rolls around, and the consensus seems to be reverting, as leaving kns with this patch in has some fairly unfortunate side effects. Could we get it reverted before the

D7194: Detach before setting the d pointer

2017-10-25 Thread Dan Leinir Turthra Jensen
leinir added a subscriber: sitter. leinir added a comment. Damn... Well spotted, @kossebau. Right, so immediate (at least temporary solution) to make things not broken would annoyingly enough be to revert the patch, yes... I am now thinking that another oddity noticed by @sitter last week wa

D8311: Require the same internal version as you're building

2017-10-15 Thread Dan Leinir Turthra Jensen
leinir created this revision. leinir added a reviewer: KNewStuff. leinir added a project: KNewStuff. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY Previously the KNewStuff sub-frameworks would require the dependency f

D8111: Require Kirigami 2.1 instead of 1.0 for KNewStuffQuick

2017-10-02 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R304:aad1ca02f183: Require Kirigami 2.1 instead of 1.0 for KNewStuffQuick (authored by leinir). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8111?vs=20248&id=20251

D8111: Require Kirigami 2.1 instead of 1.0 for KNewStuffQuick

2017-10-02 Thread Dan Leinir Turthra Jensen
leinir created this revision. leinir added a reviewer: KNewStuff. leinir added a project: KNewStuff. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY This reduces the amount of things which require the by now quite out of

D7190: Don't complain the knsregistry file is not present before it's useful

2017-08-23 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. Good call, yes - gets rid of a lump of irrelevant and technically correct but subjectively incorrect information. REPOSITORY R304 KNewStuff BRANCH master REVISION DETAIL https://pha

D7194: Detach before setting the d pointer

2017-08-23 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. Ah, yes, good catch :) REPOSITORY R304 KNewStuff BRANCH detach REVISION DETAIL https://phabricator.kde.org/D7194 To: apol, leinir Cc: broulik, #frameworks

D6532: When requesting from the cache, report all entries at bulk

2017-07-07 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. Hmm... i was thinking this //could// cause issues, i don't see anywhere assumptions are made about page size on the consumption side (damn we're lucky this whole split thing's so new ;) ).

D6512: Add support for proposed tags addition in OCS 1.7

2017-07-05 Thread Dan Leinir Turthra Jensen
leinir added a comment. In https://phabricator.kde.org/D6512#121834, @apol wrote: > +1 looks sensible to me. Sweet :) > What's the OCS state in this regard? When will 1.7 be a thing? OCS 1.7 will be a thing hopefully in the not too distant future... Incidentally, i need

D6513: Add support for Attica tags support

2017-07-05 Thread Dan Leinir Turthra Jensen
leinir created this revision. leinir added a project: KNewStuff. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY This is to add support for the new tags support in Attica found in https://phabricator.kde.org/D6512, whic

D6512: Add support for proposed tags addition in OCS 1.7

2017-07-05 Thread Dan Leinir Turthra Jensen
leinir added a dependent revision: D6513: Add support for Attica tags support. REPOSITORY R235 Attica REVISION DETAIL https://phabricator.kde.org/D6512 To: leinir, #knewstuff, apol, whiting, #kde_store Cc: #kde_store, #frameworks, ZrenBot, akiraohgaki, alexanderschmidt, siyuandong, ronaldv,

D6512: Add support for proposed tags addition in OCS 1.7

2017-07-05 Thread Dan Leinir Turthra Jensen
leinir created this revision. leinir added a project: KDE Store. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY This patch is to add support for the proposed addition of tags to the OCS standard version 1.7, as seen in

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. Different approach, but yeah, telling people what'll actually happen works :) REPOSITORY R304 KNewStuff BRANCH master REVISION DETAIL https://phabricator.kde.org/D6492 To: apol, #fr

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision. leinir added a comment. This revision now requires changes to proceed. derp, not accepted, my bad... REPOSITORY R304 KNewStuff REVISION DETAIL https://phabricator.kde.org/D6492 To: apol, #frameworks, leinir

D6492: Also use m_currentRequest when checking for installed and updates

2017-07-04 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. That is indeed a very good point. I think we might possibly have an issue here, though, in that since these two searches are no longer stand-alone, if the filter is not explicitly reset bef

D5902: Expand KNewStuff documentation

2017-06-27 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R304:4b37ece102dd: Expand KNewStuff documentation (authored by leinir). REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5902?vs=15908&id=15924 REVISION DETAIL https:

D5902: Expand KNewStuff documentation

2017-06-27 Thread Dan Leinir Turthra Jensen
leinir added inline comments. INLINE COMMENTS > apol wrote in README.md:35 > Still it shouldn't be looked-for, should it? It still need to be found before you can set it as runtime (unless i am fundamentally misunderstanding something of course) - like how it's done in Discover... or am i misu

D5902: Expand KNewStuff documentation

2017-06-27 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 15908. leinir added a comment. Add/fix a whole bunch of more documentation, identified as missing (or incorrect) by Aniketh REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5902?vs=14660&id=15908 REVISION DETAIL http

D6340: Fix incorrect error detection for missing knsrc files

2017-06-23 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. leinir marked an inline comment as done. Closed by commit R304:eb2e65a882f0: Fix incorrect error detection for missing knsrc files (authored by leinir). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D6340?vs=15742&i

D6340: Fix incorrect error detection for missing knsrc files

2017-06-23 Thread Dan Leinir Turthra Jensen
leinir marked an inline comment as done. leinir added inline comments. INLINE COMMENTS > apol wrote in engine.cpp:130 > are you sure the `endl` is necessary? i am, in fact, reasonably certain it is not required - leftovers are fun, i'll get rid of that :) REPOSITORY R304 KNewStuff REVISION

D6340: Fix incorrect error detection for missing knsrc files

2017-06-22 Thread Dan Leinir Turthra Jensen
leinir created this revision. leinir added a project: KNewStuff. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY This fixes the error detection for knsrc files, which previously would report missing files only if the di

D6190: Expose and use Engine's page size variable

2017-06-14 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes. Closed by commit R304:2f8580b9e604: Expose and use Engine's page size variable (authored by leinir). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D6190?vs=15377&id=15443#toc REPOSITORY R304 KNewStuff CHANGES SI

D6190: Expose and use Engine's page size variable

2017-06-12 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 15377. leinir added a comment. Don't const & an int, that's just silly. REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6190?vs=15373&id=15377 REVISION DETAIL https://phabricator.kde.org/D6190 AFFECTED FILES src/c

D6190: Expose and use Engine's page size variable

2017-06-12 Thread Dan Leinir Turthra Jensen
leinir added a comment. As far as i can gather, it was simply never added because, well, it was never used in a lot of places... This really is more a case of equalising some features between requestData and other parts of the engine (so they can all be paginated by the size they really want

D6190: Expose and use Engine's page size variable

2017-06-12 Thread Dan Leinir Turthra Jensen
leinir created this revision. leinir added a project: KNewStuff. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY Engine has a pageSize variable which has been mostly unused, but which comes in very handy when getting la

D6067: Make it possible to use QXmlStreamReader to read a KNS registry file

2017-06-06 Thread Dan Leinir Turthra Jensen
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

D6067: Make it possible to use QXmlStreamReader to read a KNS registry file

2017-06-06 Thread Dan Leinir Turthra Jensen
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 backwar

D6104: Use the right scope for the installpath variable

2017-06-06 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. Personally less fond of auto than you are... But, that is just me, and this is a framework, and it's fine :) A massive reduction in allocations is a very good thing, go for it :) REPOSITOR

D6049: Extend unittests to test stable sort.

2017-06-05 Thread Dan Leinir Turthra Jensen
leinir accepted this revision. leinir added a comment. This revision is now accepted and ready to land. Sorting correctness (and more thorough testing) is good, yes. LGTM! :) As to the missing arcconfig... will need someone to produce one of those who actually uses arc to fix that ;) Remo

D5902: Expand KNewStuff documentation

2017-05-18 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 14660. leinir marked 2 inline comments as done. REPOSITORY R304 KNewStuff CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D5902?vs=14634&id=14660 REVISION DETAIL https://phabricator.kde.org/D5902 AFFECTED FILES README.md src/core/engine.h s

<    1   2   3   4   5   6   >