D29678: Fix incorrect colours in the KNS Quick messagebox

2020-05-13 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes.
Closed by commit R304:5ce4d1a29b96: Fix incorrect colours in the KNS Quick 
messagebox (authored by leinir).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29678?vs=82744=82791

REVISION DETAIL
  https://phabricator.kde.org/D29678

AFFECTED FILES
  src/qtquick/qml/private/MessageBoxSheet.qml

To: leinir, #frameworks, #knewstuff, #plasma, bugseforuns, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D29678: Fix incorrect colours in the KNS Quick messagebox

2020-05-13 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 82744.
leinir added a comment.


  Address comment by @ngraham
  
  - Add a comment for the purpose of the control being a TextEdit

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29678?vs=82659=82744

BRANCH
  fix-colour-issue-for-messagebox (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29678

AFFECTED FILES
  src/qtquick/qml/private/MessageBoxSheet.qml

To: leinir, #frameworks, #knewstuff, #plasma, bugseforuns
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D29678: Fix incorrect colours in the KNS Quick messagebox

2020-05-13 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D29678#670125 , @ngraham wrote:
  
  > Does this even need to be a TextEdit? Or is that to get mouse-selectability 
for the error text? If so, +1 but please add comments indicating this reason.
  
  
  That is precisely what it's for - i'll add a comment to that effect :)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29678

To: leinir, #frameworks, #knewstuff, #plasma, bugseforuns
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D29678: Fix incorrect colours in the KNS Quick messagebox

2020-05-12 Thread Dan Leinir Turthra Jensen
leinir edited the test plan for this revision.
leinir added reviewers: Frameworks, KNewStuff, Plasma, bugseforuns.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29678

To: leinir, #frameworks, #knewstuff, #plasma, bugseforuns
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29678: Fix incorrect colours in the KNS Quick messagebox

2020-05-12 Thread Dan Leinir Turthra Jensen
leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  Explicitly set the colours used in the messagebox' content item,
  as the theme does not propagate to the simpler items, only to
  those based on top of QtQuick Controls.
  
  BUG:421270

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-colour-issue-for-messagebox (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29678

AFFECTED FILES
  src/qtquick/qml/private/MessageBoxSheet.qml

To: leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29455: KNS: Deprecate isRemote method and handle parse error properly

2020-05-07 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  Sorted, nicely done :) Makes the code just a touch simpler as well, which is 
always good :)

REPOSITORY
  R304 KNewStuff

BRANCH
  fix_isremote_stuff (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29455

To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29123: Do not mark entry as uninstalled if uninstallation script failed

2020-05-07 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
leinir added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> installation.h:113
>   *
>   * The entry instance will be updated with any new information:
>   * 

Not blocking, but the documentation probably wants fixing before you push - 
just swap this line for "The entry emitted by signalEntryChanged (only happens 
when the uninstallation is completed with non-critical errors) will be updated 
with any new information, in particular the following:"

REPOSITORY
  R304 KNewStuff

BRANCH
  arcpatch-D29123_1

REVISION DETAIL
  https://phabricator.kde.org/D29123

To: alex, #knewstuff, meven, ngraham, leinir
Cc: leinir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29451: KNS: Do not mark entry as installed if install script failed

2020-05-07 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  Thank you :D Land away :)

REPOSITORY
  R304 KNewStuff

BRANCH
  arcpatch-D29451

REVISION DETAIL
  https://phabricator.kde.org/D29451

To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29451: KNS: Do not mark entry as installed if install script failed

2020-05-07 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  (oops, mistakenly marked as accepted, sorry...)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29451

To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29451: KNS: Do not mark entry as installed if install script failed

2020-05-07 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  In D29451#665420 , @alex wrote:
  
  > Fix connect
  >
  > Thanks, I wasn't aware of that until now :-)
  
  
  Same, until a couple of weeks ago :D i'd sort of... taken to doing it anyway, 
because it just seemed nice, but yup, turns out that you really definitely want 
to do that unless you know very precisely what you're doing :)

INLINE COMMENTS

> installation.cpp:355
>  QProcess* p = runPostInstallationCommand(installedFiles.size() 
> == 1 ? installedFiles.first() : targetPath);
> -connect(p, static_cast QProcess::ExitStatus)>(::finished), this, installationFinished);
> +connect(p, static_cast QProcess::ExitStatus)>(::finished), this,
> +[entry, installationFinished, this] (int exitCode, 
> QProcess::ExitStatus) {

Terribly sorry to keep doing this, but... yeah, noticed the old-style overload 
thing, but since we require higher than Qt 5.6 and already require a 
sufficiently high version of c++, we can use qOverload instead of the 
static_cast :) https://doc.qt.io/qt-5/qtglobal.html#qOverload

REPOSITORY
  R304 KNewStuff

BRANCH
  arcpatch-D29451

REVISION DETAIL
  https://phabricator.kde.org/D29451

To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29451: KNS: Do not mark entry as installed if install script failed

2020-05-07 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision.
leinir added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> installation.cpp:355
>  QProcess* p = runPostInstallationCommand(installedFiles.size() 
> == 1 ? installedFiles.first() : targetPath);
> -connect(p, static_cast QProcess::ExitStatus)>(::finished), this, installationFinished);
> +connect(p, static_cast QProcess::ExitStatus)>(::finished),
> +[entry, installationFinished, this] (int exitCode, 
> QProcess::ExitStatus) {

Remember to give connect an object context for the slot (i did not realise 
until recently what leaving that out means, but it turns out to be potentially 
quite bad and crashy in difficult to track ways - in short, if our this 
instance is destroyed (say, the user quits the application) while an 
installation is ongoing, this would crash when attempting to emit or call 
installationFinished - it /should be fine, as i said, in most cases, as 
installation's a long lifetime object, but also just good style to add that 
context - see 
https://wiki.qt.io/New_Signal_Slot_Syntax#New:_connecting_to_simple_function 
for a detailed explanation, but in short, just add `this,` before the lambda 
and you're good to go :) )

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29451

To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29455: KNS: Deprecate isRemote method and handle parse error properly

2020-05-06 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  Apart from these couple of details, it looks pretty good :) (i'd say just fix 
and commit, but one of them's a tiny bit larger than just a typo fix ;) )

INLINE COMMENTS

> engine.cpp:174
>  if (!m_installation->readConfig(group)) {
> +Q_EMIT signalError(i18n("Failed to read config file %1", 
> configfile));
>  return false;

"Failed to read config file" is more a specific issue with setting up the 
installation handler, so... "Could not initialise the installation handler for 
%1. This is a critical error and should be reported to the application author" 
would probably be a little clearer... It is /supposed/ to not happen, and is 
really quite critical, so the best we can reasonably do is direct the blame to 
the appropriate location.

> installation.h:78
> +#if KNEWSTUFFCORE_ENABLE_DEPRECATED_SINCE(5, 71)
> +KNEWSTUFFCORE_DEPRECATED_VERSION(5, 71, "No longer use, feature 
> obsulete")
>  bool isRemote() const;

Obsolete, not obsulete :) (just a nitpick, really :) )

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29455

To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29123: Do not mark entry as uninstalled if uninstallation script failed

2020-05-06 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  Sorry about missing that bic issue before...

INLINE COMMENTS

> leinir wrote in installation.cpp:653
> Unless you report the entry as changed, the cache will not be updated and the 
> entire reporting side will fall down. Please put that line back :)

In reference to the comment about bic issues above (and specifically how the 
function is /supposed/ to be interpreted), this is exactly the point where that 
confusion becomes perhaps a little more obvious - i hadn't noticed what you 
were doing before, expecting that entry instance to change, but as you can see, 
the signal there is important :)

> installation.h:124
>   */
> -void uninstall(KNSCore::EntryInternal entry);
>  

Hmm... i find myself wondering what effect this has on BIC... something tells 
me it might not be so great... Specifically, 
https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B#The_Do.27s_and_Don.27ts
 says that you cannot change the signature of existing functions of any type... 
However, it feels more like a problem with the documentation - the idea is that 
the entries passed into functions aren't changed (and this really should 
probably be a const reference, but for some reason it isn't, and again we can't 
change that for bic-iness), and so really what /should/ happen is that rather 
than saying "The entry instance will be updated" and so on, what it should be 
doing is say "If the entry is successfully uninstalled, listening to 
signalEntryChanged(const KNSCore::EntryInternal &) for an entry equal to the 
one you have passed in will allow you to detect the result of calling the 
function".

That's what it's already doing, and how it is used in places which call the 
function, so probably makes sense to fix that.

In the longer term (think KF6), i would also quite like all the functionality 
here to end up entirely asynchronous.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29123

To: alex, #knewstuff, meven, ngraham, leinir
Cc: leinir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29451: KNS: Do not mark entry as installed if install script failed

2020-05-06 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  A bit nitpicky, that first one, the second's more serious (i'd like to avoid 
that in new code), but looks good otherwise :)

INLINE COMMENTS

> installation.cpp:355
>  QProcess* p = runPostInstallationCommand(installedFiles.size() 
> == 1 ? installedFiles.first() : targetPath);
> -connect(p, static_cast QProcess::ExitStatus)>(::finished), this, installationFinished);
> +auto finishedLambda = [=](int exitCode, QProcess::ExitStatus) {
> +if (exitCode) {

Not really any reason to store it in a variable so much... In the rest of the 
codebase, unless the same lambda is used in more than one location, it's 
defined inline in the connect statement. Also, i realise some of the code has 
the capture everything thing going on, but that's me being silly when i first 
learned how to use them - if possible, only capture the things you actually 
need to capture :)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29451

To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29447: Fix showing updates when the option is selected

2020-05-06 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes.
Closed by commit R304:25d391c6f36b: Fix showing updates when the option is 
selected (authored by leinir).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29447?vs=82021=82064

REVISION DETAIL
  https://phabricator.kde.org/D29447

AFFECTED FILES
  src/core/itemsmodel.cpp
  src/qtquick/qml/Page.qml
  src/qtquick/quickitemsmodel.cpp

To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks
Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart


D29455: KNS: Remove isRemote method and handle parse error properly

2020-05-05 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision.
leinir added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> installation.h:76
>  bool readConfig(const KConfigGroup );
> -bool isRemote() const;
>  

i'm afraid this is an exported class, at most we can mark this as deprecated 
(with a bit of explanation), we can't remove it...

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29455

To: alex, #knewstuff, ngraham, leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29447: Fix showing updates when the option is selected

2020-05-05 Thread Dan Leinir Turthra Jensen
leinir marked 2 inline comments as done.
leinir added a comment.


  Thanks for making me realise that it doesn't have to be quite so elaborate, 
@alex ;)

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-show-only-updates (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29447

To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks
Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart


D29447: Fix showing updates when the option is selected

2020-05-05 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 82021.
leinir added a comment.


  As @alex suggests, just use qlist::contains, it is supposed to be
  reasonably cheap, so... yup, trust the framework! ;)
  
  - Just use qlist::contains

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29447?vs=82003=82021

BRANCH
  fix-show-only-updates (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29447

AFFECTED FILES
  src/core/itemsmodel.cpp
  src/qtquick/qml/Page.qml
  src/qtquick/quickitemsmodel.cpp

To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks
Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart


D29447: Fix showing updates when the option is selected

2020-05-05 Thread Dan Leinir Turthra Jensen
leinir added inline comments.

INLINE COMMENTS

> alex wrote in itemsmodel.cpp:71
> On second thought why not just use:
> `bool duplicate =  m_entries.contains(entry);`

Hmm... i do wonder slight of the cost of that, but also... much simpler code, 
so... basically can just go

  if (!m_entries.contains(entry)) {

below, rather than this more elaborate version.

> alex wrote in itemsmodel.cpp:71
> Use qAsConst(m_entries) and space before & not after :-)

Quite, thank you :)

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-show-only-updates (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29447

To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks
Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart


D29447: Fix showing updates when the option is selected

2020-05-05 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 82003.
leinir added a comment.


  Address comment by @alex
  
  - Fix style (and consty things)

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29447?vs=81993=82003

BRANCH
  fix-show-only-updates (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29447

AFFECTED FILES
  src/core/itemsmodel.cpp
  src/qtquick/qml/Page.qml
  src/qtquick/quickitemsmodel.cpp

To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks
Cc: alex, kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart


D29447: Fix showing updates when the option is selected

2020-05-05 Thread Dan Leinir Turthra Jensen
leinir added reviewers: KNewStuff, Plasma, bugseforuns, ngraham, Frameworks.
leinir added projects: Plasma, KNewStuff.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29447

To: leinir, #knewstuff, #plasma, bugseforuns, ngraham, #frameworks
Cc: kde-frameworks-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, ragreen, michaelh, ZrenBot, ngraham, bruns, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D29447: Fix showing updates when the option is selected

2020-05-05 Thread Dan Leinir Turthra Jensen
leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  This patch primarily fixes the issue that when picking the "Updateable Only" 
option on NewStuff.Page, we would be shown all the entries anyway. It also 
fixes duplicated entries on the "Installed Only" option, and further adds a 
little placeholder for when the view is expectedly empty.
  
  - Use the right connections for update entry display
  - While potentially expensive, only add entries to the model once
  - Use Kirigami.PlaceholderMessage to show we have no entries when we don't
  
  BUG:416762

TEST PLAN
  Without this patch, behaviour is as shown in 
https://bugs.kde.org/show_bug.cgi?id=416762
  
  With this patch, the behaviour is as expected (shows only updateable entries 
with that option selected, only shows installed entries once with that option, 
and shows a useful message when the list is empty and expected to be empty)

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-show-only-updates (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29447

AFFECTED FILES
  src/core/itemsmodel.cpp
  src/qtquick/qml/Page.qml
  src/qtquick/quickitemsmodel.cpp

To: leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29222: Fix update auto selection

2020-05-04 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes.
Closed by commit R304:12642a1c0174: Fix update auto selection (authored by 
leinir).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29222?vs=81412=81869

REVISION DETAIL
  https://phabricator.kde.org/D29222

AFFECTED FILES
  src/core/cache.cpp
  src/core/engine.cpp

To: leinir, #frameworks, #plasma, bugseforuns, ngraham, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D28701: Add KPackage support to KNewStuffCore

2020-05-04 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes.
Closed by commit R304:3f38da8a70d8: Add KPackage support to KNewStuffCore 
(authored by leinir).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28701?vs=81859=81868

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/cache.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/installation.cpp
  src/core/installation.h
  src/core/jobs/kpackagejob.cpp
  src/core/jobs/kpackagejob.h
  src/downloaddialog.h

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: alex, ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-05-04 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 81859.
leinir added a comment.


  Since we have had a new Frameworks release while this is waiting
  for the thumbs up, increase the @since to the next version.
  
  - Merge branch 'master' into add-kpackage-support
  - Merge branch 'master' into add-kpackage-support
  - I was trying to avoid this. Update @since to 0.71

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28701?vs=80862=81859

BRANCH
  add-kpackage-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/cache.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/installation.cpp
  src/core/installation.h
  src/core/jobs/kpackagejob.cpp
  src/core/jobs/kpackagejob.h
  src/downloaddialog.h

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: alex, ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-30 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D28701#660186 , @mart wrote:
  
  > +1 from me.
  
  
  Thanks! :)
  
  > does it need to go into next release or can also get in next+1?
  
  It really wants to go in soon as possible (since it fixes the whole global 
themes nastiness that people have been reporting - transparently, of course, 
what with the fallback sniffing going on in the ctor), same as the update fix 
in D29222 

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: alex, ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


Re: Information regarding upcoming Gitlab Migration

2020-04-28 Thread Dan Leinir Turthra Jensen
On Monday, 27 April 2020 21:25:09 BST Albert Astals Cid wrote:
> El dilluns, 27 d’abril de 2020, a les 13:58:02 CEST, Bhushan Shah va 
escriure:
> > In part I am mostly re-iterating what Ben already mentioned in different
> > messages.
> > 
> > On Mon, Apr 27, 2020 at 12:38:42PM +0200, Aleix Pol wrote:
> > > Does this mean that to clone it we'll have to go "git clone
> > > kde:games/knetwalk" or something along the lines?
> > 
> > Yes
> > 
> > [Rest of message is with sysadmin hat off and as a developer]
> > 
> > > If that's the case I'd much prefer if we didn't do this, at the moment
> > > it's already uncomfortable for me to remember the URL for some of the
> > > repos (e.g. is it sysadmin/ or not?), this will only increase the
> > > problem and I personally don't see the advantage.
> > 
> > I do agree that it maybe small inconvience, but let's be honest, most of
> > us have been using kdesrc-build or some kind of automated tooling for
> > building everything, apart from very rare case we never have to manually
> > clone any of KDE repository, at least it is true for me personally. I am
> > not sure about others.
> 
> Please let's refrain from saying things like "most of us have been using
> kdesrc-build" when you don't have any data to back that up.
> 
> Cheers,
>   Albert

Just adding my "i don't use kdesrc-build, and git clone kde:x everything 
myself" voice, here. Now, if a simple(ish) script can be created to make 
something akin to the kde: rewriting work, even if what it really does is to 
search gitlab and create a clone with the appropriate command, i could deal 
with that, but having the ability to simply ask for the project name is more 
than a little useful.

-- 
..dan / leinir..
http://leinir.dk/




D28701: Add KPackage support to KNewStuffCore

2020-04-28 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  Ping team framework and such? (i realise we're all a tiny bit more stressy 
than usual...)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: alex, ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D29101: KNewStuff: Fix file path and process call

2020-04-28 Thread Dan Leinir Turthra Jensen
leinir accepted this revision as: leinir.
leinir added a comment.
This revision is now accepted and ready to land.


  That's well spotted. Thanks for taking that one on :)

REPOSITORY
  R304 KNewStuff

BRANCH
  bugfix_uninstall (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29101

To: alex, #knewstuff, ngraham, nicolasfella, elvisangelaccio, meven, mlaurent, 
leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29222: Fix update auto selection

2020-04-28 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 81412.
leinir added a comment.


  Thanks @ngraham, learn me to add debug information and then forget to commit 
it :P
  
  - Compile...

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29222?vs=81318=81412

BRANCH
  fix-update-autoselection (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29222

AFFECTED FILES
  src/core/cache.cpp
  src/core/engine.cpp

To: leinir, #frameworks, #plasma, bugseforuns, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29222: Fix update auto selection

2020-04-27 Thread Dan Leinir Turthra Jensen
leinir edited the summary of this revision.
leinir added reviewers: Frameworks, Plasma, bugseforuns, ngraham.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29222

To: leinir, #frameworks, #plasma, bugseforuns, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29222: Fix update auto selection

2020-04-27 Thread Dan Leinir Turthra Jensen
leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  The new autoselection for updating installed entries would firstly
  not be looking at the right amount of download links, and further it
  would not be able to identify the links it needed to, as it did not
  look at the descriptive names (where the filename is often stored),
  which then would not match. These changes ensure that we look at
  the right bits of information when doing this check. End result is
  that updating works.
  
  - Ensure that the cache doesn't forget installing and updating items
  - Analyse more of the download link information, and use correct counts

TEST PLAN
  Find an item in some KNS dialog in need of updating, and click the Update 
button
  
  On the console, when launched using the correct logging categories, will
  tell you about the selection process (and the UI will show that an update
  is in progress, and eventually that it is completed). To do this check with
  the icons kcm, run it like so:
  
QT_LOGGING_RULES="org.kde.knewstuff*=true" kcmshell5 kcm_icons

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-update-autoselection (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29222

AFFECTED FILES
  src/core/cache.cpp
  src/core/engine.cpp

To: leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-24 Thread Dan Leinir Turthra Jensen
leinir added inline comments.

INLINE COMMENTS

> mart wrote in kpackagejob.cpp:55
> is it worth encapsulating it in a runnable? the installation of a package is 
> already in a separathe thread.

Annoyingly, the caller waits for that thread to complete, so the KPackage 
internals work synchronously, and block the UI thread... (which is how we ended 
up doing this)

> mart wrote in kpackagejob.cpp:97
> this would freeze a thread waiting another thread is done, while just using 
> the job in an async wayshouldn't be much more heavy on the gui thread?

The Runner doesn't inherently have an event loop, so without this, the job just 
sort of... sits there and does nothing (doesn't block, but also doesn't do 
anything)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: alex, ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D29123: Do not mark entry as uninstalled if uninstallation script failed

2020-04-24 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  On a related note, i'm waiting on reviews on D28701 
 at the moment...  which i'm afraid might 
wreck some havoc with your patch, as they touch some of the same bits of the 
codebase.

INLINE COMMENTS

> installation.cpp:632
> +Question question(Question::ContinueCancelQuestion);
> +question.setQuestion(err);
> +Question::Response response = question.ask();

This will want to be more... question like... "The thing failed" isn't really a 
question, not sure how the user's supposed to make an informed choice based on 
that... Perhaps something like "The uninstallation process failed to run the 
command %1. The output was:\n%2\nIf you think this is incorrect, you can 
continue, or you can cancel the process." (given how much this is an error 
situation, it feels like we can give the user a bit of technical information... 
cancelling in a panic would be the appropriate reaction to "I don't know" 
anyway for this sort of thing, so thinking we'd be ok with doing that).

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29123

To: alex, #knewstuff, meven, ngraham, leinir
Cc: leinir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29126: KNewStuff: port from KRun::runApplication to KIO::ApplicationLauncherJob

2020-04-23 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D29126#655523 , @dfaure wrote:
  
  > And KRun was widgets based too. At least now you can get rid of widgets 
completely (for this bit about starting apps) by connecting to the result() 
signal yourself instead of using a dialog ui delegate. There's also a 
knotifications based ui delegate, but not sure it's appropriate here (that's 
more for plasma itself I would say).
  
  
  Indeed! 'tis all very handy - and thanks :)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29126

To: dfaure, ngraham, broulik, leinir
Cc: leinir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29126: KNewStuff: port from KRun::runApplication to KIO::ApplicationLauncherJob

2020-04-23 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  Was /just/ about to be all "nooo, widgets in core, crying forever" but this 
isn't core, so go for it ;)

REPOSITORY
  R304 KNewStuff

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D29126

To: dfaure, ngraham, broulik, leinir
Cc: leinir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29123: WIP BUG: 420312. Do not mark entry as uninstalled if uninstallation script failed

2020-04-23 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  (and now i've done it myself, terribly sorry about that, missed the WIP at 
the start of the title! Hope some of my comments were useful, though :) )

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29123

To: alex, #knewstuff, meven, ngraham, leinir
Cc: leinir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29123: WIP BUG: 420312. Do not mark entry as uninstalled if uninstallation script failed

2020-04-23 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  The reporting side of this seems based on a misunderstanding of what the 
UI-less Core is supposed to be doing... The conceptual intention in general 
isn't bad, but it needs a bit of work. Thanks for spotting it, too :)

INLINE COMMENTS

> CMakeLists.txt:71
>  KF5::CoreAddons
> +KF5::WidgetsAddons # KMessageBox error messages
>  Qt5::Xml

No, that's what the Question system is for. No widget stuff in Core, thanks :)

> installation.cpp:631
> +// can delete the files manually
> +entry.setStatus(KNS3::Entry::Installed);
> +KMessageBox::error(nullptr, err);

If you are changing the status, you need to also emit entryChanged, otherwise 
the cache will be inconsistent

> installation.cpp:632
> +entry.setStatus(KNS3::Entry::Installed);
> +KMessageBox::error(nullptr, err);
> +return;

As you are already issuing the signal with the error, intercept that instead. 
Don't spawn widgets from Core, that adds a widget dependency to the Qtquick 
module.

> installation.cpp:653
> -
> -emit signalEntryChanged(entry);
>  }

Unless you report the entry as changed, the cache will not be updated and the 
entire reporting side will fall down. Please put that line back :)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D29123

To: alex, #knewstuff, meven, ngraham, leinir
Cc: leinir, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-22 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 80862.
leinir added a comment.


  Some documentation and whitespace fixes for Frameworksiness
  
  - Fix up a few whitespace errors
  - Add some documentation to KPackageJob, and a few @since

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28701?vs=80617=80862

BRANCH
  add-kpackage-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/cache.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/installation.cpp
  src/core/installation.h
  src/core/jobs/kpackagejob.cpp
  src/core/jobs/kpackagejob.h
  src/downloaddialog.h

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-21 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D28701#653954 , @ngraham wrote:
  
  > I mean, GHNS basically *is* a userspace package manager, and things like 
dependency management and removal of stale files are why package managers 
exist. :)
  >
  > If it needs to be fixed/implemented in other patches, that's okay.
  
  
  Definitely, yes, it wants implementing, though i think we need to consider 
precisely where we want it to go... Thinking it probably wants to go in 
KPackage itself for this case (since that's what installs those dependencies - 
it's at the end of the packagejobthread's installPackage function, but i'm 
thinking the optional removal thing likely needs to go into the packagejob, 
possibly as an overload... just brainfarting a bit here), but there's also a 
dependencies and referencing type thing in OCS (and consequently KNewStuff) 
which we'll need to implement more... properly than it is right now. But yup, 
that's sort of orthogonal to this patch anyway, more a "this wants to be done" 
type thing :)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-21 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D28701#652898 , @ngraham wrote:
  
  > Great. There are still a few more bugs though:
  >
  > When you install certain global themes, they ask for authentication so 
install an SDDM theme. However when you uninstall that theme, it doesn't 
request authentication to remove them SDDM theme. So `/usr/share/sddm/themes` 
accumulates a growing collection of unused themes:
  >
  >   ls /usr/share/sddm/themes/
  >   breeze  breeze-openSUSE  elarun  Layan  maldives  maya  McMojave  plasmaX 
 Sweet
  >
  
  
  Not a huge amount knewstuff can do about that, that'll need to be done by the 
sddm kpackage plugin (mind you, having not looked i don't imagine this would be 
a huge issue, more a bit of forgotten implementation work in said plugin, since 
it already has the logic to ask on installation i don't imagine it would be a 
huge amount of effort to get it to do that on uninstallation as well).
  
  > When I install and uninstall the Sweet global theme, its Plasma theme still 
shows up in the Plasma style KCM. And looking in 
`~/.local/share/plasma/desktoptheme/`, there are several orphaned plasma themes 
left over from global themes that I deleted from the GHNS dialog on the global 
themes KCM:
  > 
  >   ls ~/.local/share/plasma/desktoptheme/
  >   Arc-Dark  kpluginindex.json  Layan  Sweet
  > 
  > 
  > Their color schemes and icon themes are still there too.
  
  i can confirm this is happening, yup - however, this isn't knewstuff 
specifically, this is kpackage in general (you will notice this with any 
kpackage which has dependencies). I would suggest it is out of the scope for 
this specific patch to fix that problem, but it's definitely something we'll 
want to sort out (but also, if you have ever run apt-get purge or autoremove, 
that's the territory we're veering into here... that is to say, doable, but not 
trivial or automatically doable, as there's no guarantee that nothing else 
depends on that package being there).
  
  Now, what we /could/ do is allow the user to just go "yup, please remove all 
the dependencies as well" for packages which have dependencies listed, but i am 
thinking that that is starting to sound quite dangerous... As in, what happens 
when a user goes "yes, i definitely know what i'm doing" and clicks that 
option, only to then discover that no, those dependencies were also needed by 
some other thing (i imagine a fair few global themes would have similar 
dependencies) and now that other thing just outright breaks when attempting to 
apply it. Again, this should hopefully be solvable, but i'm not sure how much 
work that would be. Arguably in scope here if we want to ask, but it's not a 
regression (by which i mean kpackage -r Sweet -t Plasma/LookAndFeel would also 
leave those other bits behind).

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-20 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 80617.
leinir added a comment.


  Thank you to @ngraham for noticing this one! It only really pokes its head
  out if you have multiple things installed and then try and uninstall one of
  them - if you only have the one thing installed, it looks very much like
  as though it's just cleaning up after itself and removing the containing
  folder... Easy enough fix, though :)
  
  - Make sure we use package name, not install location, when uninstalling

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28701?vs=80289=80617

BRANCH
  add-kpackage-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/cache.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/installation.cpp
  src/core/installation.h
  src/core/jobs/kpackagejob.cpp
  src/core/jobs/kpackagejob.h
  src/downloaddialog.h

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-20 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  As i thought, i was indeed holding the KPackage APi incorrectly ;) The 
culprit is hinted at in the line
  
  >   org.kde.knewstuff.core: Attempting to perform an installation operation 
of type 3 on the package "/home/leinir/.local/share/plasma/desktoptheme/Sweet" 
of type "Plasma/Theme" in the package root 
"/home/leinir/.local/share/plasma/desktoptheme/"
  >   
  
  which says that it is installing a package, which has the full path, and not 
only the package name. The kpackage api expects a package name, and so handing 
it a directory name is not as helpful. So, delightfully simple fix, really, but 
yeah, annoying on a Friday night while cooking dinner ;) Updated patch incoming 
momentarily...

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-17 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D28701#650616 , @ngraham wrote:
  
  > Thanks, the hang is gone now. However I have a new problem:
  >
  > 1. Open Global Themes KCM and click new new global themes
  > 2. Install Sweet KDE and McMojave LAF global themes
  > 3. Close the GHNS dialog
  > 4. Open the dialog again
  > 5. Uninstall the McMojave LAF theme
  > 6. Close the dialog
  >
  >   Both McMojave LAF and also Sweet KDE have disappeared from the KCM. Sweet 
KDE still shows up as installed in the GHNS dialog and Discover, but does not 
appear in the KCM.
  >
  >   The same thing happens for plasma themes too. It seems that deleting one 
item from the GHNS dialog marks all of the installed GHNS items of that type as 
deleted.
  
  
  i can confirm this, and looking at it a bit with some debug output i have a 
suspicion that i might be holding some part of kpackage's api incorrectly... 
Perhaps someone with a longer experience with KPackage than me can tell me 
whether that is the case perhaps?
  
org.kde.knewstuff.core: Install:  "Sweet KDE"  from  
"/tmp/AfOprO-Sweet.tar.xz"
org.kde.knewstuff.core: installdir:  "/tmp"
org.kde.knewstuff.core: Using KPackage for installation
org.kde.knewstuff.core: Package metadata is valid
org.kde.knewstuff.core: Service type discovered as "Plasma/Theme"
org.kde.knewstuff.core: About to attempt to install "Sweet" into 
"/home/leinir/.local/share/plasma/desktoptheme/"
org.kde.knewstuff.core: Attempting to perform an installation operation of 
type 2 on the package "/tmp/AfOprO-Sweet.tar.xz" of type "Plasma/Theme" in the 
package root "/home/leinir/.local/share/plasma/desktoptheme/"
org.kde.knewstuff.core: Service type understood
org.kde.knewstuff.core: Installer successfully created and has a valid 
structure
kf5.kpackage: Generated  
"/home/leinir/.local/share/plasma/desktoptheme//kpluginindex.json"  ( 2  
plugins)
org.kde.knewstuff.core: Created job, now let's wait for it to do its 
thing...
org.kde.knewstuff.core: Install job finished with no error and we now have 
files "/home/leinir/.local/share/plasma/desktoptheme/Sweet"
org.kde.knewstuff.core: Write registry
org.kde.knewstuff.core: about to uninstall entry  "1294174"
org.kde.knewstuff.core: Attempting to perform an installation operation of 
type 3 on the package "/home/leinir/.local/share/plasma/desktoptheme/Sweet" of 
type "Plasma/Theme" in the package root 
"/home/leinir/.local/share/plasma/desktoptheme/"
org.kde.knewstuff.core: Service type understood
org.kde.knewstuff.core: Installer successfully created and has a valid 
structure
org.kde.knewstuff.core: Created job, now let's wait for it to do its 
thing...
org.kde.knewstuff.core: Write registry
org.kde.knewstuff.core: about to uninstall entry  "1305006"
org.kde.knewstuff.core: Write registry

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-17 Thread Dan Leinir Turthra Jensen
leinir added reviewers: broulik, bshah.
leinir added a comment.


  Tagging in a couple of people who were in the original chat about doing this 
integration... :)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, 
broulik, bshah
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-16 Thread Dan Leinir Turthra Jensen
leinir edited the summary of this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-16 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 80289.
leinir added a comment.


  Address @ngraham's (and my own) worry about the synchronous behaviour 
exhibited by KPackage... Something a bit like this probably wants to go into 
KPackage itself, perhaps we can consider this after we've done a bit of testing 
of its solidity here...
  
  - Add a simple async job wrapper for KPackage operations
  - Add the KPackage job to the build
  - Switch KNSCore::Installation to using the new async KPackageJob

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28701?vs=80195=80289

BRANCH
  add-kpackage-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/cache.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/installation.cpp
  src/core/installation.h
  src/core/jobs/kpackagejob.cpp
  src/core/jobs/kpackagejob.h
  src/downloaddialog.h

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-16 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D28701#649012 , @ngraham wrote:
  
  > This has fixed the issues I was seeing with installation and 
uninstallation, nice! However I'm not seeing a very long hang when installing 
certain global themes--for example Sweet KDE and Layan Look and Feel Theme. It 
does ultimately work, but the dialog freezes for multiple minutes before 
finally becoming interactive again.
  
  
  That freeze really is amazingly annoying, and i'm afraid the only reasonable 
way to fix that is to move the installer into its own thread... which /should/ 
work ok, but i'm super scared of threads at the best of times ;) Needs must, 
though.

INLINE COMMENTS

> ngraham wrote in installation.cpp:105
> Oof.
> 
> Is there really no way around this? I mean, it did more or less work before 
> the new QtQuick dialog was rolled out.

Going by the code (in kpackagetool and here), i don't see how it could've 
really ever worked the way it looked before - i feel like what happened 
previously was that it /looked/ like it sort of worked, because installation 
did, but uninstallation never worked...

The extreme case described in this comment is just that, though - it turns out 
that unless you clean your tmp dir regularly, you're likely to actually have 
those downloaded files available, which will cause the items to remain marked 
as installed, which in turn allows the fallback path in the uninstall step 
below here to kick in, which will both remove the downloaded file, and also (if 
the file is identified as a kpackage and we can do a reverse lookup match on an 
installed kpackage with that type and name) remove that package with kpackage. 
So... there's a lot of bits in here to try and ensure people won't notice that 
everything really was very badly broken with kpackages previously, and that it 
now just sort of works :)

Further, a detail i noticed earlier, is that the uninstall call would just 
outright mark the entry you asked to uninstall as deleted, whether or not that 
was actually successful (which incidentally may happen and is there to allow 
some manual cleaning up if they've just deleted stuff underneath kns). I've 
moved that around just a touch, so that it doesn't happen if a kpackage failed 
to be uninstalled.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-15 Thread Dan Leinir Turthra Jensen
leinir added reviewers: ngraham, mart, davidedmundson.
leinir added a comment.


  Tagging in those active in the referenced bug, except for the reporter who 
doesn't have a phabricator account

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: Add KPackage support to KNewStuffCore

2020-04-15 Thread Dan Leinir Turthra Jensen
leinir retitled this revision from "[WIP] Add KPackage support to 
KNewStuffCore" to "Add KPackage support to KNewStuffCore".
leinir edited the summary of this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-15 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 80195.
leinir added a comment.


  Think we're at the point where testing would be good, now. This update
  means we now attempt to adopt already installed kpackages if you try and
  install the package from knewstuff, and removal of entries installed
  using the previous implementation should now also happen during a fallback
  step, intended to make life a bit simpler for those who have used this
  before...
  
  - Add a KPackageType property to Installation, for fallback purposes
  - Add documentation for the new knsrc bits
  - Handle adopting an already installed kpackage item
  - Also uninstall not-adopted-but-there possibly installed kpackage bits

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28701?vs=80184=80195

BRANCH
  add-kpackage-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/cache.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/installation.cpp
  src/core/installation.h
  src/downloaddialog.h

To: leinir, #plasma, #knewstuff, #frameworks
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-15 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 80184.
leinir edited the test plan for this revision.
leinir added a comment.


  (the fallback handling needs some more work, but also progress)
  
  - Clean up some of the error reporting, and reset the entry's state
  - Check if installedFile is a file, if so bypass KPackage and delete

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28701?vs=80099=80184

BRANCH
  add-kpackage-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/cache.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/installation.cpp
  src/core/installation.h

To: leinir, #plasma, #knewstuff, #frameworks
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-14 Thread Dan Leinir Turthra Jensen
leinir edited the test plan for this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-14 Thread Dan Leinir Turthra Jensen
leinir edited the test plan for this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-14 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 80099.
leinir added a comment.


  - Make a touch of noise when encountering the fallback

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28701?vs=80098=80099

BRANCH
  add-kpackage-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/cache.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/installation.cpp
  src/core/installation.h

To: leinir, #plasma, #knewstuff, #frameworks
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-14 Thread Dan Leinir Turthra Jensen
leinir edited the summary of this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-14 Thread Dan Leinir Turthra Jensen
leinir edited the summary of this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-14 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 80098.
leinir added a comment.


  Bunch of new functionality, with a fallback which ought to allow it to work
  without requiring any kind of changes to the existing knsrc files based on
  kpackage, as long as they follow the pattern they seem to mostly follow.
  
  - Introduce a getter (and enum) for the uncompress Installation setting
  - Add a redirection to the knsrc documentation location
  - Add a function to clean the cache of functionally stale entries
  - Clean the cache when the uncompression method is set to kpackage
  - Add a fallback for uncinverted kpackage based knsrc files

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28701?vs=79704=80098

BRANCH
  add-kpackage-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/cache.h
  src/core/engine.cpp
  src/core/engine.h
  src/core/installation.cpp
  src/core/installation.h

To: leinir, #plasma, #knewstuff, #frameworks
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-14 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D28701#644973 , @ngraham wrote:
  
  > Okay thanks, can you update the test plan then? Also I think the diff 
currently does not actually fix 418466.
  
  
  Your comments (and adding on reviewers) seems to suggest to me that you 
either haven't noticed the [WIP] at the start of the diff's title... or that 
i'm marking work in progress patches incorrectly? (and to answer the question, 
i will of course be adding testing instructions)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir, #plasma, #knewstuff, #frameworks
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-09 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D28701#644943 , @ngraham wrote:
  
  > Am I not testing properly?
  
  
  Correct, that's the bit where i've not added instructions as required for 
switching to using the kpackage support. Short version, comment out the 
uninstall and install commands, and add an Uncompress=kpackage line, in the 
lookandfeel.knsrc file :) (also considering perhaps doing some smart lookup 
type stuff, and switching things which call kpackagetool automatically to using 
this, but also am not entirely certain if there might not be some fallout from 
that... not sure, will need to look at that)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28701

To: leinir
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-09 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 79704.
leinir added a comment.


  - Remove a stray newline

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28701?vs=79703=79704

BRANCH
  add-kpackage-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/installation.cpp

To: leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28701: [WIP] Add KPackage support to KNewStuffCore

2020-04-09 Thread Dan Leinir Turthra Jensen
leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  Adding support for KPackage directly to KNewStuff means that we are
  able to deal more gracefully with things like Plasma's Global Themes
  (and indeed any other kpackage based thing).
  
  This is done by adding another archive specialisation to the installer
  class, and by also adding a check to the cache to ensure that even
  when a kpackage is removed from the system outside of KNewStuff,
  it does not remain seemingly installed in the KNS lists.
  
  - Make sure the cache gets written
  - Add KPackage support to KNSCore::Installation
  
  BUG:418466

REPOSITORY
  R304 KNewStuff

BRANCH
  add-kpackage-support (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28701

AFFECTED FILES
  CMakeLists.txt
  src/core/CMakeLists.txt
  src/core/cache.cpp
  src/core/installation.cpp

To: leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28532: Introduce more user-visible error reporting for installations

2020-04-06 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes.
Closed by commit R304:eb23f549ea9b: Introduce more user-visible error reporting 
for installations (authored by leinir).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28532?vs=79474=79489

REVISION DETAIL
  https://phabricator.kde.org/D28532

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h
  src/qtquick/qml/EntryDetails.qml
  src/qtquick/qml/Page.qml
  src/qtquick/qml/private/EntryCommentsPage.qml
  src/qtquick/qml/private/ErrorDisplayer.qml
  src/qtquick/qml/private/MessageBoxSheet.qml

To: leinir, #knewstuff, #plasma, ngraham, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28532: Introduce more user-visible error reporting for installations

2020-04-06 Thread Dan Leinir Turthra Jensen
leinir edited the test plan for this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28532

To: leinir, #knewstuff, #plasma, ngraham, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28532: Introduce more user-visible error reporting for installations

2020-04-06 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 79474.
leinir added a comment.


  - Merge branch 'master' into more-installation-error-handling
  - Actually read the error (not just all)

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28532?vs=79195=79474

BRANCH
  more-installation-error-handling (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28532

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h
  src/qtquick/qml/EntryDetails.qml
  src/qtquick/qml/Page.qml
  src/qtquick/qml/private/EntryCommentsPage.qml
  src/qtquick/qml/private/ErrorDisplayer.qml
  src/qtquick/qml/private/MessageBoxSheet.qml

To: leinir, #knewstuff, #plasma, ngraham, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28532: Introduce more user-visible error reporting for installations

2020-04-06 Thread Dan Leinir Turthra Jensen
leinir edited the test plan for this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28532

To: leinir, #knewstuff, #plasma, ngraham, #frameworks
Cc: pino, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D28540: Fix layout in DownloadItemsSheet

2020-04-04 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes.
Closed by commit R304:449d056a2d77: Fix layout in DownloadItemsSheet (authored 
by leinir).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28540?vs=79226=79290

REVISION DETAIL
  https://phabricator.kde.org/D28540

AFFECTED FILES
  src/qtquick/qml/DownloadItemsSheet.qml

To: leinir, #frameworks, bugseforuns, mart, #plasma, #knewstuff, ngraham
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28540: Fix layout in DownloadItemsSheet

2020-04-03 Thread Dan Leinir Turthra Jensen
leinir edited the summary of this revision.
leinir added reviewers: Frameworks, bugseforuns, mart, Plasma, KNewStuff.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28540

To: leinir, #frameworks, bugseforuns, mart, #plasma, #knewstuff
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28540: Fix layout in DownloadItemsSheet

2020-04-03 Thread Dan Leinir Turthra Jensen
leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  The previous code was primarily there to work around misfeatures of 
OverlaySheet which no longer exist. Consequently, we don't have to leave in the 
hacks. Nice.
  
  This further builds on the new OverlaySheet work done by Marco. The whole 
thing works much better, and with less client code. Nice.
  
  BUG:419535

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-downloaditemssheet-layout (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28540

AFFECTED FILES
  src/qtquick/qml/DownloadItemsSheet.qml

To: leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28532: Introduce more user-visible error reporting for installations

2020-04-03 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 79195.
leinir added a comment.


  - Address @pino's comment re i18n overload usage

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28532?vs=79191=79195

BRANCH
  more-installation-error-handling (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28532

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h
  src/qtquick/qml/EntryDetails.qml
  src/qtquick/qml/Page.qml
  src/qtquick/qml/private/EntryCommentsPage.qml
  src/qtquick/qml/private/ErrorDisplayer.qml
  src/qtquick/qml/private/MessageBoxSheet.qml

To: leinir, #knewstuff, #plasma, ngraham, #frameworks
Cc: pino, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D28532: Introduce more user-visible error reporting for installations

2020-04-03 Thread Dan Leinir Turthra Jensen
leinir edited the test plan for this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28532

To: leinir, #knewstuff, #plasma, ngraham, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28532: Introduce more user-visible error reporting for installations

2020-04-03 Thread Dan Leinir Turthra Jensen
leinir edited the summary of this revision.
leinir added reviewers: KNewStuff, Plasma, ngraham, Frameworks.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28532

To: leinir, #knewstuff, #plasma, ngraham, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28532: Introduce more user-visible error reporting for installations

2020-04-03 Thread Dan Leinir Turthra Jensen
leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  Prior to this, we did have error reporting, but only in the way of
  writing errors out on the command line (through qCCritical). While
  this is fine for debugging purposes, it is unfortunate when errors
  can be expected to occur in day to day use (as we are dealing with
  a wide range of things that can (and often do) go wrong), and giving
  proper feedback to the user as to why, for example, their new icons
  aren't showing up when they seem to have installed just fine would
  seem a reasonable thing to do.
  
  - Add a signal to Installation fired when there's a critical error
  - Forward the installer errors through KNSCore::Engine
  - Add the MessageBoxSheet component from kaccounts-integration
  - Add a component for displaying errors from the engine
  - Use the ErrorDisplayer component (only show on the current page)

REPOSITORY
  R304 KNewStuff

BRANCH
  more-installation-error-handling (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28532

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h
  src/qtquick/qml/EntryDetails.qml
  src/qtquick/qml/Page.qml
  src/qtquick/qml/private/EntryCommentsPage.qml
  src/qtquick/qml/private/ErrorDisplayer.qml
  src/qtquick/qml/private/MessageBoxSheet.qml

To: leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-04-02 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes.
Closed by commit R304:aa29f344928e: Switch to using Kirigamis 
ShadowedRectangle (authored by leinir).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28220?vs=78430=79109

REVISION DETAIL
  https://phabricator.kde.org/D28220

AFFECTED FILES
  CMakeLists.txt
  src/qtquick/qml/private/EntryScreenshots.qml
  src/qtquick/qml/private/GridTileDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart, #vdg
Cc: davidedmundson, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-31 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  Ping? Not super critical, i guess, but it just feels sad to have things 
pending for weeks...

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28220

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart, #vdg
Cc: davidedmundson, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns


D28298: Reduce superfluous changedEntriesChanged emissions

2020-03-30 Thread Dan Leinir Turthra Jensen
leinir added inline comments.

INLINE COMMENTS

> Button.qml:90
> -value: ghnsDialog.engine.changedEntries
> -}
>  

Given the various annoying side effects of the cleaner solutions, perhaps 
reintroduce this, but add a "when" clause to ensure it only happens when we 
want it to? Maybe something as simple as just checking it is only forwarded 
when engine is, in fact, not null might work... or, perhaps, if engine is not 
null, and also initialised? since it really shouldn't be giving you any changed 
entries until after initialisation has been completed... Meaning something like

  when: ghnsDialog.engine !== null && ghndDialog.engine.isInitialized

maybe perhaps could be...

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28298

To: broulik, #frameworks, leinir, ahiemstra
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-25 Thread Dan Leinir Turthra Jensen
leinir marked 8 inline comments as done.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28220

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart, #vdg
Cc: davidedmundson, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-25 Thread Dan Leinir Turthra Jensen
leinir added inline comments.

INLINE COMMENTS

> ahiemstra wrote in EntryScreenshots.qml:134
> The x and y offsets should be 0 by default, so there's little reason to 
> specify that I'd say.

Ha, yes, quite, setting defaults does seem a little silly ;)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28220

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart, #vdg
Cc: davidedmundson, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-25 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 78430.
leinir added a comment.


  Address comment by @ahiemstra
  
  - Remove some unneeded values

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28220?vs=78342=78430

BRANCH
  switch-to-kirigami-shadowedrectangle (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28220

AFFECTED FILES
  CMakeLists.txt
  src/qtquick/qml/private/EntryScreenshots.qml
  src/qtquick/qml/private/GridTileDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart, #vdg
Cc: davidedmundson, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-24 Thread Dan Leinir Turthra Jensen
leinir added a subscriber: davidedmundson.
leinir added a comment.


  In D28220#633557 , @ngraham wrote:
  
  > Just an observation, we should maybe consider making a shared component for 
this screenshots list so that Discover can use it too.
  
  
  i would like that :) Perhaps something for kirigami-addons? (was that 
@davidedmundson or someone else?)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28220

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart, #vdg
Cc: davidedmundson, ngraham, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-24 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 78342.
leinir added a comment.


  - Less magic numbers
  - Remove some unneeded bits

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28220?vs=78341=78342

BRANCH
  switch-to-kirigami-shadowedrectangle (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28220

AFFECTED FILES
  CMakeLists.txt
  src/qtquick/qml/private/EntryScreenshots.qml
  src/qtquick/qml/private/GridTileDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart, #vdg
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-24 Thread Dan Leinir Turthra Jensen
leinir edited the test plan for this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28220

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart, #vdg
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-24 Thread Dan Leinir Turthra Jensen
leinir added a reviewer: VDG.
leinir marked 3 inline comments as done.
leinir added a comment.


  (pending some screenshots of the visible change)

INLINE COMMENTS

> broulik wrote in EntryScreenshots.qml:138
> You used `largeSpacing` as `verticalOffset` before

Hm, on second thought, it looks /terrible/ doing it that way... i'm going to 
call consistency on this one and tag in the VDG, thanks for making me notice :)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28220

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart, #vdg
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-24 Thread Dan Leinir Turthra Jensen
leinir marked 5 inline comments as done.
leinir added inline comments.

INLINE COMMENTS

> broulik wrote in EntryScreenshots.qml:133
> Not needed given you `anchors.fill`

Ah yes, quite :)

> broulik wrote in EntryScreenshots.qml:135
> What's this for?

That's a good question, really - i found that in the KCM GridDelegate and just 
sort of left it in... but yup, probably not needed :)

> broulik wrote in TileDelegate.qml:123
> `shadow.size` corresponds to the old `radius`, so you'd want to set 
> `Kirigami.Units.largeSpacing` here

Less magic numbers are a nice thing ;)

> broulik wrote in TileDelegate.qml:124
> `#80` used in the old code is 128, i.e. `0.5` alpha, unless you want to make 
> it look consistently with the shadows used elsewhere?

Yeah, that was for consistency, but well spotted :)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28220

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-24 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 78341.
leinir added a comment.


  - Increase Kirigami runtime dep version to 2.12

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28220?vs=78290=78341

BRANCH
  switch-to-kirigami-shadowedrectangle (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28220

AFFECTED FILES
  CMakeLists.txt
  src/qtquick/qml/private/EntryScreenshots.qml
  src/qtquick/qml/private/GridTileDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-23 Thread Dan Leinir Turthra Jensen
leinir added reviewers: ahiemstra, broulik, mart.
leinir added a comment.


  tagging in a couple of people who definitely know things about the new thing 
:)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28220

To: leinir, #knewstuff, #frameworks, #plasma, ahiemstra, broulik, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-23 Thread Dan Leinir Turthra Jensen
leinir added reviewers: KNewStuff, Frameworks, Plasma.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D28220

To: leinir, #knewstuff, #frameworks, #plasma
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28220: Switch to using Kirigami's ShadowedRectangle

2020-03-23 Thread Dan Leinir Turthra Jensen
leinir created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
leinir requested review of this revision.

REVISION SUMMARY
  This switches from using the QML DropShadow to using the
  new, high efficiency ShadowedRectangle in Kirigami.
  
  - Modify (and equalise logic) in GridTileDelegate
  - Clean up EntryScreenshots, and use ShadowedRectangle
  - Also use ShadowedRectangle in the bigpreview and tile delegates

TEST PLAN
  No visible changes (as expected) from the switch
  Minor visible change from the EntryScreenshots modifications (now correctly 
shows the shadows all the way around)

REPOSITORY
  R304 KNewStuff

BRANCH
  switch-to-kirigami-shadowedrectangle (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28220

AFFECTED FILES
  src/qtquick/qml/private/EntryScreenshots.qml
  src/qtquick/qml/private/GridTileDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml

To: leinir
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D20986: Don't call click on the delegate, when we just want it to become current item

2020-03-23 Thread Dan Leinir Turthra Jensen
leinir abandoned this revision.
leinir added a comment.


  No longer needed

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D20986

To: leinir, ngraham
Cc: broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D26794: [WIP] Implement comment posting (and reenable voting)

2020-03-19 Thread Dan Leinir Turthra Jensen
leinir edited the summary of this revision.

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D26794

To: leinir
Cc: mart, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D27544: Fix update scenarios with no explicit downloadlink selected

2020-03-19 Thread Dan Leinir Turthra Jensen
This revision was automatically updated to reflect the committed changes.
Closed by commit R304:f1dcbddf12ea: Fix update scenarios with no explicit 
downloadlink selected (authored by leinir).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D27544?vs=77818=77989#toc

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27544?vs=77818=77989

REVISION DETAIL
  https://phabricator.kde.org/D27544

AFFECTED FILES
  CMakeLists.txt
  src/core/engine.cpp
  src/qtquick/qml/EntryDetails.qml
  src/qtquick/qml/NewStuffItem.qml
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/ThumbDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml
  src/qtquick/quickitemsmodel.cpp
  src/qtquick/quickitemsmodel.h

To: leinir, #knewstuff, #frameworks, #plasma, ngraham, apol, 
#discover_software_store, ahiemstra
Cc: ahiemstra, alexde, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns


D27635: Restore kio-webdav plugin

2020-03-18 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R155 KAccounts Integration

BRANCH
  arcpatch-D27635

REVISION DETAIL
  https://phabricator.kde.org/D27635

To: nicolasfella, bshah, leinir, #frameworks


D27544: Fix update scenarios with no explicit downloadlink selected

2020-03-17 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 77818.
leinir marked 4 inline comments as done.
leinir added a comment.


  Address @ahiemstra's comments - thanks!
  
  - Turn on C++14 support
  - Fix some whitespace issues, a leak, and add a warning
  - Less magic numbers, with the power of enums

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27544?vs=77494=77818

BRANCH
  fix-update (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27544

AFFECTED FILES
  CMakeLists.txt
  src/core/engine.cpp
  src/core/jobs/httpworker.cpp
  src/core/security.cpp
  src/qtquick/qml/EntryDetails.qml
  src/qtquick/qml/NewStuffItem.qml
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/ThumbDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml
  src/qtquick/quickitemsmodel.cpp
  src/qtquick/quickitemsmodel.h

To: leinir, #knewstuff, #frameworks, #plasma, ngraham, apol, 
#discover_software_store
Cc: ahiemstra, alexde, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns


D27544: Fix update scenarios with no explicit downloadlink selected

2020-03-17 Thread Dan Leinir Turthra Jensen
leinir marked 5 inline comments as done.
leinir added a comment.


  Thanks for those, @ahiemstra, good stuff there :)

INLINE COMMENTS

> ahiemstra wrote in engine.cpp:614
> Code style: & attaches to the name, not the type. (Yes I hate it too).
> 
> There's a few instances of this around.

so nasty... right, thanks for pointing those out, it feels so wrong to type :P

> ahiemstra wrote in engine.cpp:631
> This object will linger until the engine is destroyed, which seems like a 
> suboptimal thing. Maybe better to do:
> 
>   auto question = 
> std::make_unique(Question::SelectFromListQuestion);
> 
> then it will be automatically cleaned up once you exit the scope.

Hmm... That's something that'll want doing in a fair few places i think 
(including the documentation for Question) - but yup, might as well start 
somewhere :) It does mean turning on C++14 for KNewStuff, which... should be 
fine?

> ahiemstra wrote in engine.cpp:644
> You may want to log something in the else here, at least then it will be 
> possible to figure out why an update is not happening.

That's probably a good idea, yes... It'll commonly be due to the user 
cancelling the dialog, but if there's no good questionasker registered, it may 
happen anyway - plus i guess it's just good style anyway :)

> ahiemstra wrote in EntryDetails.qml:101
> That "1" here is a bit mysterious, what does it actually mean?

It's that whole one-indexed list thing that OCS has... But yup, your idea below 
sounds pretty good.

> ahiemstra wrote in quickitemsmodel.h:154
> Rather than using -1, you could add an enum that defines these values a bit 
> more, like:
> 
>   enum LinkId {
>   AutoDetectLink = -1,
>   FirstItem = 1
>   }
> 
> You can then also expose that to QML to avoid the magical 1 up there.

A good call, less magic numbers are always good. Did feel a little... 
unpleasant to add those.

REPOSITORY
  R304 KNewStuff

BRANCH
  fix-update (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27544

To: leinir, #knewstuff, #frameworks, #plasma, ngraham, apol, 
#discover_software_store
Cc: ahiemstra, alexde, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns


D27544: Fix update scenarios with no explicit downloadlink selected

2020-03-12 Thread Dan Leinir Turthra Jensen
leinir updated this revision to Diff 77494.
leinir added a comment.


  - Merge branch 'master' into fix-update
  - Update @since

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27544?vs=76096=77494

BRANCH
  fix-update (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27544

AFFECTED FILES
  src/core/engine.cpp
  src/core/jobs/httpworker.cpp
  src/core/security.cpp
  src/qtquick/qml/EntryDetails.qml
  src/qtquick/qml/NewStuffItem.qml
  src/qtquick/qml/private/entrygriddelegates/BigPreviewDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/ThumbDelegate.qml
  src/qtquick/qml/private/entrygriddelegates/TileDelegate.qml
  src/qtquick/quickitemsmodel.cpp
  src/qtquick/quickitemsmodel.h

To: leinir, #knewstuff, #frameworks, #plasma, ngraham, apol, 
#discover_software_store
Cc: alexde, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D27544: Fix update scenarios with no explicit downloadlink selected

2020-03-12 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D27544#622575 , @alexde wrote:
  
  > In D27544#622272 , @ngraham 
wrote:
  >
  > > (...) there has to be a way to (...) label everything properly etc.
  >
  >
  >
  >
  > In D27544#622501 , @leinir wrote:
  >
  > > We are at least two people not too happy with that situation... The label 
we're using for that now is technically a free-text field. (...) However, each 
downloaditem does have a tags section, and while that would need some thought 
put into it, i think we could do something clever with that. A bit outside the 
scope of this patch, of course, but something to spent some braining time on :)
  >
  >
  > Sounds similar to bug #415483.
  
  
  Vaguely, but they're more orthogonal than anything - the problem here is a 
lack of well named downloads, which is a generic issue, where the issue 
described in that bug is content-type-specific.
  
  It is also not exactly what this diff is about and i'd like to avoid this 
going off on a tangent, especially given the issue this patch addresses is 
causing updates to just not work...
  
  With that in mind - ping? Been a few weeks now, and i'll have to update the 
patch because we've had a frameworks release in the meantime...

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D27544

To: leinir, #knewstuff, #frameworks, #plasma, ngraham, apol, 
#discover_software_store
Cc: alexde, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D27635: Restore kio-webdav plugin

2020-03-09 Thread Dan Leinir Turthra Jensen
leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


  Minus that little API funniness, looks pretty good from here :)

INLINE COMMENTS

> CMakeLists.txt:9
>  
> -add_library(kaccounts_kio_webdav_plugin MODULE ${kio-webdav_SRCS})
> +
> +kcoreaddons_add_plugin(kaccounts_kio_webdav_plugin

wouldn't mention it, but as there's other stuff as well, looks like a stray 
newline :)

> kaccountsdplugin.h:57
>  public:
> -KAccountsDPlugin(QObject *parent = nullptr);
> +KAccountsDPlugin(QObject *parent, const QVariantList& args);
>  virtual ~KAccountsDPlugin();

This still introduces a API break, though... Which seems especially silly given 
the arguments aren't actually used. Perhaps add a new ctor, and have the one 
without the arguments call the new one, but also make a qwarning (or something 
similar) note that it's using an old ctor and really out to upgrade?

REPOSITORY
  R155 KAccounts Integration

REVISION DETAIL
  https://phabricator.kde.org/D27635

To: nicolasfella, bshah, leinir, #frameworks


D27704: Drop FindAccountsFileDir.cmake

2020-03-09 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  Removing unused stuff is always nice, well spotted :)

REPOSITORY
  R155 KAccounts Integration

BRANCH
  cma

REVISION DETAIL
  https://phabricator.kde.org/D27704

To: nicolasfella, #frameworks, bshah, leinir


D27708: Cleanup accounts kded module

2020-03-09 Thread Dan Leinir Turthra Jensen
leinir accepted this revision.
leinir added a comment.
This revision is now accepted and ready to land.


  With the caveat that i'm not a big pile of brains about the plugin system, 
this looks like pretty sensible to me :)

REPOSITORY
  R155 KAccounts Integration

BRANCH
  arcpatch-D27708

REVISION DETAIL
  https://phabricator.kde.org/D27708

To: nicolasfella, #frameworks, bshah, leinir


D27544: Fix update scenarios with no explicit downloadlink selected

2020-03-05 Thread Dan Leinir Turthra Jensen
leinir added a comment.


  In D27544#622272 , @ngraham wrote:
  
  > As an aside, I'm somewhat dissatisfied with the current UX when there are 
multiple files. :( I converted a friend of mine to Plasma the other day and he 
was very confused by the multiple items available when he was downloading new 
stuff using the new GHNS window. I know that some of this is the fault of 
content uploaders, but if so, there has to be a way to guide them to upload 
only one file, or to mark a specific file as the real/primary one, and to label 
everything properly etc.
  
  
  We are at least two people not too happy with that situation... The label 
we're using for that now is technically a free-text field, so it certainly 
should be possible to fix in a backwards-compatible sort of way (as it's a 
question of what the server provides us with)... but as for "the real one", 
that's not a thing that makes sense except in very specific cases (for example, 
it makes no sense conceptually when you have e.g. icon sets or mouse cursor 
sets with multiple variants...). However, each downloaditem does have a tags 
section, and while that would need some thought put into it, i think we could 
do something clever with that. A bit outside the scope of this patch, of 
course, but something to spent some braining time on :)
  
  In the meantime, the situation needs some work anyway, as what currently 
happens when there's crazy long downloaditem names is that it gets elided on 
the right... which would be sort of fine, if you could get to that information 
some other way, which you can't, so... yup, wants fixing ;) (also outside the 
scope of this patch, mind, but still)

REPOSITORY
  R304 KNewStuff

REVISION DETAIL
  https://phabricator.kde.org/D27544

To: leinir, #knewstuff, #frameworks, #plasma, ngraham, apol, 
#discover_software_store
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


  1   2   3   4   5   6   >