Respin request for KConfigWidgets

[Alexander Lohnau]

Hello,

I would like to make a respin for kconfigwidgets because of the porting
aid introduced in
https://invent.kde.org/frameworks/kconfigwidgets/-/merge_requests/191.

The change about making KF5::CoreAddons a public was discussed at the
KF6 weekly. Because we already require KCoreAddons implicitly and will
explicitly require it in KF6's KCMUtils it should be fine.
https://invent.kde.org/frameworks/kconfigwidgets/-/merge_requests/193
should be merged before the respin, because I used 5.106 in the @since
tags initially.

Thanks in advance
Alex

KF6 weekly changes

[Alexander . Lohnau]

Hello,

regarding the low attendance at the KF6 weeklies, we decided that it is best to 
have this meeting every two weeks.
With the recent Plasma sprint and previous meetings we were able to unblock 
lots of tasks, but they still need doing .

To better structure the meetings, we will be creating issues with the topics 
for the meetings
in https://invent.kde.org/teams/frameworks-devs/kf6-workboard/-/issues/. Please 
add topics you want to discuss there too.

Regarding the daylight saving time change, we are keeping the meeting at 17:00 
CEST.

Hopefully until next week.

Regards
Alexander

Aw: Re: kcm_bluetooth changed ?

[Alexander . Lohnau]

Hello,

 

Usually the KCMs are opened by starting kcmshell or systemsettings with the module names as arguments,

meaning we resolve the location of the KCMs internally and don't require changes for consumers. kcmshell also checks if the plugin id

would match if the "kcm_" prefix is to the module name prepended, consequently "kcmshell5 bluetooth" works in both Plasma 5.23 and 5.24.

 

> If KDE/plamsa is such a moving target where  can not rely on compatibility even between minor versions, that is really not fun

 

Looking at liquidshell, there are multiple  cases where the available KCMs are checked at runtime to be compatible with old Plasma versions. While having to do this is not ideal, it is not anything new.

Luckily most of the changes to port the KCMs to the new metadata approach have already landed.
The KWin KCMs remain though, when porting those I will make sure to create a accompanying MR in liquidshell.

 

PS: You can check if the KPluginMetaData object is valid, instead of checking if the name is not empty :).

 

Regards
Alex


 
 

Gesendet: Sonntag, 13. Februar 2022 um 14:41 Uhr
Von: "Fusion Future" 
An: "Martin Koller" 
Cc: kde-frameworks-devel@kde.org, plasma-de...@kde.org, alexander.loh...@gmx.de
Betreff: Re: kcm_bluetooth changed ?

On 2022/2/13 21:31, Martin Koller wrote:
> But still I wonder if such a change is something like breaking ABI compatibility.
> If KDE/plamsa is such a moving target where I can not rely on compatibility even
> between minor versions, that is really not fun.
>

Perhaps the breakage is due to these commits [1-3]. Need to ask
Alexander Lohnau if there is a proper solution. I added him to the Cc list.

Sorry for the inconvenience.

[1]
https://invent.kde.org/plasma/bluedevil/-/commit/ac71faf781e1b3690f33bf54f27eeaaee7ec30e3
[2]
https://invent.kde.org/plasma/bluedevil/-/commit/87ad90b54592817d77d50c4284ff899659fa0550
[3]
https://invent.kde.org/plasma/bluedevil/-/commit/62856fe4d95ab4c1b795cf487ea0dbc61459412c

Aw: KF6 meeting notes 2021-10-26

[Alexander . Lohnau]

Hello,

 

regarding the kcmutils core lib discussion I have a quick addition to the topic: KPluginSelector

should get a KPluginMetaData based replacement. This will have a core model and a QML/QWidgets

version. Meaning that it makes sense to introduce a core lib even in KF5 for this new class.

 

See https://phabricator.kde.org/T12265 for the related task.

 

Regards

Alex

 
 

Gesendet: Dienstag, 26. Oktober 2021 um 18:05 Uhr
Von: "Volker Krause" 
An: kde-frameworks-devel@kde.org
Betreff: KF6 meeting notes 2021-10-26

!! Meeting will move to 17:00 CET next week (due to DST change in continental
Europe)

https://invent.kde.org/frameworks/kcoreaddons/-/merge_requests/149
* static plugin support draft, needs review
* MRs to demonstrate the usage exists for KWin and KMyMoney

https://invent.kde.org/frameworks/kcoreaddons/-/merge_requests/117
* generic ctor vs. named ctor pattern?
* pro named ctor: explicitness, safety, and ability to deprecate indivdual
features
* pro generic ctor: easier for code that supports multiple formats (do we have
that at all?)
* currently no known issues with this
* can be changed later, but having named ctors now might help with porting
* having both at the same time is also possible, as both methods do different
things/solve different use-cases
* postpone until this becomes an actual problem

https://phabricator.kde.org/T14517
* JSON KCM loading works fine
* URL to pin KCMs to taskbar requires a .desktop file
* would require to keep the .desktop files around for KCMs that behave
"application-like"
* generate .desktop file from JSON, to avoid duplication?
* would the JSON file have all the necessary content?
* alternative: manually maintained .desktop files
* not an alternative: keep .desktop-based plugin loading code and json from
desktop generation around

https://phabricator.kde.org/T14763
* should we already install unversioned symlinks for CLI tools right now?
* unversioned implies CLI interface compatibility
* should have done a long time ago, so yes, do this now
* kcmshell will move to kcmutils and become versioned

https://phabricator.kde.org/T14367
* QML bindings from KDeclarative and KCMModule from KConfigWidgets planned to
move to KCMUtils (https://phabricator.kde.org/T12150)
* should we split KCMUtils internally between classes for creating KCMs and
classes for consuming KCMs (which is where the KXmlGui dependency comes in)?
* dependency on KXmlGui is for KAboutPluginDialog, which is tied to the entire
about dialog code in xml gui
* could we move the entire about dialog stuff down to a tier2 framework? but
there's no good framework there to place this in? but does that even make
sense for something every widget-based app needs anyway?
* so the above suggestions of multiple libs in the kcmutils framework might
make more sense
* related to https://phabricator.kde.org/T14355, which might eventually also
require a core library in KCMUtils

Threaded KIO workers:
* David F is removing the POP3 kioslave to enable that

D19565: kconfig_compiler: new kcfgc args HeaderExtension & SourceExtension

[Alexander Lohnau]

alex added a comment.


  However the option seems pretty unused, even from a github search: 
https://github.com/search?q=HeaderExtension%3D+extension%3Akcfgc=Code=advsearch==
  
  > KF targets also consumers outside of KDE spheres
  
  Also I think there are currently quite a lot of kcfgc options, which makes is 
more difficult to work with IMHO. That might also be more true for third party 
users which are not very familiar with the API.
  
  > to see that developers of other projects prefer those suffixes for C++ 
headers. Using .h, so the same suffix as used for C headers
  
  Though from having a quick look it seems like (pretty much all) of those 
headers are not using any of KDE/Qt libs. Which means that they would not be a 
very likely audience anyways.
  If we want to attract our frameworks to more users, making them slimmer is 
also a possibility one needs to consider.

REPOSITORY
  R237 KConfig

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

To: kossebau, #frameworks, apol
Cc: alex, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ahmadsamir, 
ngraham, bruns, vkrause

D19565: kconfig_compiler: new kcfgc args HeaderExtension & SourceExtension

[Alexander Lohnau]

alex added a comment.


  I am wondering if there is really a need for it. SourceExtension seems 
completely unused and HeaderExtension is only used in okteta.
  
  Though in KDE code, Qt code  (and most other that I know of) it is the ".h" 
extension is the most common one for headers.
  
  Are there any reasons for having it that I am not aware of?

REPOSITORY
  R237 KConfig

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

To: kossebau, #frameworks, apol
Cc: alex, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ahmadsamir, 
ngraham, bruns, vkrause

Aw: Re: Deprecation wrappers for pre-KF6 time (was: Re: KF6 meeting notes 2021-06-05)

[Alexander . Lohnau]

In KService (KPluginInfo) the BUILD macro variant is used for non-virtual
methods, because it is used in public API of other frameworks. Though that does not seem to be used consistently.

A good argument against using the BUILD variant in the header is that consumers/devs are not at all able to

test the build when excluding deprecated API completely.

 



My primary concern is that we need to keep using the classes/method, because
of compatibility, like KServiceTypeTrader they should all have a viable alternative.

But this means that in large parts of frameworks and plasma we can not utilize the macros that much.

 

PS: Forgot about this thread and had this mail lying around as a draft for quite some time.


 

Regards

Alex


Gesendet: Sonntag, 06. Juni 2021 um 00:46 Uhr
Von: "Friedrich W. H. Kossebau" 
An: kde-frameworks-devel@kde.org
Betreff: Re: Deprecation wrappers for pre-KF6 time (was: Re: KF6 meeting notes 2021-06-05)

Am Samstag, 5. Juni 2021, 22:39:15 CEST schrieb Alexander Lohnau:
> Or could we use the BUILD_DEPRECATED_SINCE variant in this case? Like we
> already have to do with virtual methods.
>
> This way one would still be able to test it when compiling the framework
> without deprecations. Existing API users will still be informed about the
> deprecations by the compiler warnings.

Using BUILD_DEPRECATED_SINCE with what will become the KF6 Beta version (i.e.
the version where something will disappear in preparation for KF6) should be
possible as well. The current way of testing that no hidden dependency by
usages of deprecated-with-already-present-replacement API still exists, by
building all KF libraries with DISABLE_DEPRECATED_BEFORE_AND_AT=CURRENT (or
explicit version) should not be affected by this, as after all the macro
should yield true for bigger versions.


And the version-controlled deprecation warning macros could also be used with
that BETA version (though we should then adapt any current usages of
KF_DEPRECATED_WARNINGS_SINCE to not use 0x06, but the respective KF6 Beta
version, to silence warnings for the stuff only deprecated without parallel
replacement and replaced/dropped for the KF5 beta.
Think e.g. as negative exapmple for what to avoid the useless warnings for
QNetworkConfigurationManager, where we cannot do anything about while using
Qt5 and have to do that extra work to tell the compiler to not warn for that
class on our side.


E.g. by the (broken) example I linked before:

#if KPARTS_VERSION <= QT_VERSION_CHECK(5, 900, 0) # the 900 is broken
#include 
#include 
#include 
#include 
#endif

could instead be

#if KPARTS_BUILD_DEPRECATED_SINCE(5, 190) # or whatever if KF6 Beta
#include 
#include 
#include 
#include 
#endif

On the other hand the term DEPRECATED could be confusing here, as nothing is
really deprecated in the traditional sense., where something is tagged to be
removed either because being unused or in favour of another now available
approach. So this could be considered an abuse of those macros rather, and
where things are already complex now they might get even more complex by that
additional meaning.

The BUILD_DEPRECATED_SINCE are there for being able to configure a build with
or without, with a filter level by version even. Will that be needed for
things which get fully replaced? I.e. will there be a transition time where
both old & new are available? If not, I would make this rather a simple hard
condition against the foreseen version where something will be replaced.

Also, if X gets replaced by Y, that X is right now also used by other things
often in the same module, so being able to completely build without X might
not be doable. So the macro usage cannot even be tested properly. And things
will only be found out when actually replacing X by Y (and Y might only be
created in the process), so any wrapping beforehand does not really prepare
anything for someone?

Is there an example where things could be demoed/tested/experimented with?

Cheers
Friedrich
 

 

 

 

KF6 meeting notes 2021-07-26

[Alexander . Lohnau]

Hi all,

 

here are the notes from the KF6 weekly.

 

Regards
Alexander

 

https://phabricator.kde.org/T12176:

If a usecase comes up one could think if implementing a cross-platform autostart API. In its current state & the current usages it should be deprecated. The needed logic should be imported in plasma-workspace; where the actual autostarting is done and the autostart KCM is.


https://phabricator.kde.org/T14314:

Question about how logging should be handled: KCoreAddons should provide both translated and untranslated error strings for the API users. Like the KJob error handling with the errorString and errorText.

The critical error should also be logged by kcoreaddons.

See https://invent.kde.org/frameworks/kcoreaddons/-/merge_requests/106 for the Merge Request.

 

https://phabricator.kde.org/T14744:

The keyword stuff is not really needed and should be deprecated. Multiple plugins can be loaded without it being involved, if the classes are different. That is in most of the current usages the case. The KCMs in kinfocenter should be ported to KPluginMetaData and will not need the keyword stuff afterwards. See https://phabricator.kde.org/T14517.

The X-KDE-KCM-Args property might help with the KWin usages. But it needs to be read from the plugin which has the config modules, not the config module itself.  


Frederik has been working on the API documentation, https://phabricator.kde.org/T12004. See https://invent.kde.org/frameworks/kapidox/-/merge_requests/20 for a draft MR about making kapidox run in a docker container.

 

 

Re: KService as a platform abstraction framework?

[Alexander Lohnau]

On Saturday, 3 July 2021 19:39:17 CEST Nicolas Fella wrote:
> On 03.07.21 10:25, Volker Krause wrote:
> > Hi,
> >
> > while looking at implementing a pretty straightforward KApplicationTrader/
> > KIO::ApplicationLauncherJob use ([1]) for Android, I found myself
> > wondering
> > whether we should have an Android backend for KService.
> >
> > KService conceptually matches
> > android.content.pm.PackageManager/PackageInfo
> > [2, 3], ie. the platform API to list installed applications and their
> > respective features (essentially what's in the Android manifest XML
> > files). In detail it however is full of .desktop file specifics, and
> > leaks platform implementation details (e.g. KService inheriting from
> > sycoca types).
> >
> > KIO::ApplicationLauncherJob is also something that makes sense
> > conceptually on Android, implemented on top of Intents there.
> >
> > Has anyone thought about/looked into using KService as platform
> > abstraction
> > rather than as an functional/platform implementation framework already? I
> > could imagine this to also be relevant on Windows.
> >
> > Is anyone aware of a current use on Android relying on the .desktop based
> > implementation of KService? That might be theoretically possible, unlike
> > for ApplicationLauncherJob.
> >
> > And while retrofitting platform abstraction support into KService wont be
> > pretty, the alternative approaches (a new abstraction framework on top, or
> > let applications deal with that with platform-specific code paths) aren't
> > exactly convincing either.
> >
> > Thoughts?
> >
> > Thanks,
> > Volker
> >
> > [1] https://invent.kde.org/plasma-mobile/qrca/-/merge_requests/35
> > [2]
> > https://developer.android.com/reference/android/content/pm/PackageManager
> > [3]
> > https://developer.android.com/reference/android/content/pm/PackageInfo
> Hi,
>
> I think overall it makes sense.
>
> In our ongoing KF6 work we tend to move away from using KService for
> non-application cases (plugins and kparts). Assuming we fully execute that 
> quite a bit
of stuff then can be dropped from KService. That should
> make it easier to adapt/make it less awkward to retrofit Android support
> then. I think it's worth going over the KService class and investigate
> how much of it will be relevant on a post-KF6 world.
>
> Some XDG-isms are going to remain, but as long as it's just a bunch of
> properties this should not be a big issue.
>
> Cheers
>
> Nico

Hi,

> In our ongoing KF6 work we tend to move away from using KService for
> non-application cases (plugins and kparts).

This also includes getting rid of KServiceTypeTrader, which is actively being 
worked on.


Also https://phabricator.kde.org/T12183[1] is related to KService being a 
platform
abstraction framework.

Regards
Alex




[1] https://phabricator.kde.org/T12183#255228

Re: Deprecation wrappers for pre-KF6 time (was: Re: KF6 meeting notes 2021-06-05)

[Alexander Lohnau]

Or could we use the BUILD_DEPRECATED_SINCE variant in this case? Like we
already have to do with virtual methods.

This way one would still be able to test it when compiling the framework
without deprecations. Existing API users will still be informed about the
deprecations by the compiler warnings.

Regards
Alex

On Saturday, 5 June 2021 16:57:43 CEST Friedrich W. H. Kossebau wrote:
> Am Samstag, 5. Juni 2021, 16:29:10 CEST schrieb Volker Krause:
> > Deprecation wrappers macros:
> > - should those also be added for things that have no replacement yet, or
> > where the replacement will only become available for KF6?
> > - trader queries is one such example (https://phabricator.kde.org/T14543)
> > - so should we set the version to 6.0 for those instead? avoids leaking
> > into KF6 by accident and doesn't block the usage of those macros
> > elsewhere?
> I propose to not use 6.0.0. but some beta-like version number, e.g. 5.190.0.
> ((Beware, our use of the hexnumber version scheme 0xXXYYZZ limits the
> individual numbers in the version to the range 0..255)).
> Otherwise those macros will trigger only on release version bump time, which
> would be too late.
>
> Now the challenge is to find a good minor version number for KF6 Betas :)
> And have this properly documented, so people use the same working trigger
> version. At least one place uses QT_VERSION_CHECK(5, 900, 0) which does not
> do what we expect...
> See https://invent.kde.org/frameworks/kparts/-/blob/master/src/
> partloader.cpp#L21
> (should be fixed in the process)
>
> Cheers
> Friedrich (sadly no resources currently to help more with KF5->KF6)

Re: Re KIO workers

[Alexander Lohnau]

On Saturday, 5 June 2021 17:51:18 CEST David Faure wrote:
> On samedi 5 juin 2021 16:29:10 CEST Volker Krause wrote:
> > Do KIO slaves still need the klauncher/kinit loading mechanism?
>
> No. My request for developers to test KIO_FORK_SLAVES=1
> for daily use is so that apps fork kio worker processes directly, without
> going via klauncher/kinit. BTW it seems to work fine. I wonder if we should
> toggle that in 5.84, as part of the incremental move to the KF6 world.
>
> > or could
> > that be replaced by json metadata based plugin loading as well?
>
> Err, that's an orthogonal question.


That was meant regarding 
https://invent.kde.org/frameworks/kio/-/blob/master/src/
kioslave/kioslave.cpp#L73[1], similar to https://phabricator.kde.org/T13808[2].

But because SlaveBase is not a QObject we could not do this during KF5 times  
easily. It
was just an idea :)

>
> When not going via klauncher/kinit, the app first launches the kioslave5
> process, which then loads the .so with the kio worker plugin. As you can
> see from your process list:
>
> PREFIX/lib64/libexec/kf5/kioslave5 PREFIX/lib64/plugins/kf5/kio/file.so file
>  local:/run/user/1000/kded5ymjnPa.3.slave-socket
>
> That .so is determined by slave.cpp using
>  QString lib_path = KPluginLoader::findPlugin(_name);
> which I believe means it finds the plugin by filename, no .protocol file
> needed and no json metadata needed, right?
>
> > - is the performance benefit of kinit still relevant there?
>
> We decided it wasn't. For KIO workers it was never measured anyway.
>
> > - for in-process KIO that would be needed anyway
>
> That would remove the separate process (kioslave5) from the equation
> but that's unrelated to plugin loading.


Regards
Alex



[1] 
https://invent.kde.org/frameworks/kio/-/blob/master/src/kioslave/kioslave.cpp#L73
[2] https://phabricator.kde.org/T13808

T12173: KService: provide solution to migrate away from KServiceTypeTrader/KMimeTypeTrader for loading plugins and parts

[Alexander Lohnau]

alex added a comment.


  > KRunner uses KServiceTypeTrader to load plugins. This is non-trivial to 
port because of the DBus runners
  
  It is ported and the old methods are deprecated

TASK DETAIL
  https://phabricator.kde.org/T12173

To: alex
Cc: alex, #frameworks, nicolasfella, dfaure, mart, davidre, GB_2, ahmadsamir, 
ngraham, kpiwowarski, usta, asturmlechner, jucato, cfeck, cgiboudeaux, 
cullmann, vkrause, cordlandwehr, knauss

Proposing reroll of KNewStuff for Frameworks 5.76

[Alexander Lohnau]

Hello,

 

I would like to request a reroll of KNewStuff to include c15fbc05dbc275ef1bd85f1578c078f6a873909c, which fixes a severe issue for day to day use cases.

Without it entries could end being marked as uninstalled when a KCM is reopened, while they are actually installed.

Also entries could show up as updatable, even after they were updated.

 

Alexander

 

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex added a comment.


  Ufff, never thought sth. simple like deprecating a few functions could cause 
such an issue!
  
  @davidre Thanks a lot for fixing this!

REPOSITORY
  R308 KRunner

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

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: davidre, kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29050: KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex closed this revision.

REPOSITORY
  R308 KRunner

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

To: alex, meven, ngraham, broulik
Cc: davidedmundson, cfeck, 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

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex closed this revision.

REPOSITORY
  R308 KRunner

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

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex marked an inline comment as done.

REPOSITORY
  R308 KRunner

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

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 83119.
alex added a comment.


  Fix typo in version number

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=83115=83119

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> meven wrote in querymatch.cpp:338
> Did you really need to remove this code ?

The question this method should answer is: Does this match have a configuration 
interface?
And the answer is no, because the functionality does not exist anymore.
And if it is not removed a deprecated function would be called.

REPOSITORY
  R308 KRunner

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

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> kossebau wrote in abstractrunner.h:154
> Should be "5.0", not "5,0" here in the API dox text and elsewhere, no? (dot 
> instead of comma)

Yes. I accidentally did it this way, because in German you use a comma :D

> kossebau wrote in abstractrunner.h:156
> ECMGenerateExportHeader now (for 5.71) features support for using a different 
> version number to be shown in the compiler warning: 
> KRUNNER_DEPRECATED_VERSION_BELATED
> 
> So you could make this:
> 
>   KRUNNER_DEPRECATED_VERSION_BELATED(5, 71,  5, 0, "No longer use, feature 
> removed")

This is pretty cool, many thanks again.

REPOSITORY
  R308 KRunner

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

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex marked 9 inline comments as done.

REPOSITORY
  R308 KRunner

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

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 83115.
alex added a comment.


  Fix comma, KRUNNER_DEPRECATED_VERSION_BELATED macro

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=83110=83115

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex added a reviewer: meven.

REPOSITORY
  R308 KRunner

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

To: alex, #plasma, broulik, davidedmundson, vkrause, meven
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29050: KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex added a comment.


  Now I understand where it is called and how it works exactly.
  May I ship this?

REPOSITORY
  R308 KRunner

BRANCH
  krunner_signal_bugfix (branched from master)

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

To: alex, meven, ngraham, broulik
Cc: davidedmundson, cfeck, 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

D29050: KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex edited the summary of this revision.

REPOSITORY
  R308 KRunner

BRANCH
  krunner_signal_bugfix (branched from master)

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

To: alex, meven, ngraham, broulik
Cc: davidedmundson, cfeck, 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

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 83110.
alex added a comment.


  Change deprecation version from 5.70 to 5.71

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=81546=83110

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

To: alex, #plasma, broulik, davidedmundson, vkrause
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29050: KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex added a comment.


  > Repo is called milou
  
  Thanks!

REPOSITORY
  R308 KRunner

BRANCH
  krunner_signal_bugfix (branched from master)

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

To: alex, meven, ngraham, broulik
Cc: davidedmundson, cfeck, 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

D29050: KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex added a comment.


  But can you please explain, where in the QML code the actual KRunner backend 
is used?
  Maybe then I can understand it better .

REPOSITORY
  R308 KRunner

BRANCH
  krunner_signal_bugfix (branched from master)

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

To: alex, meven, ngraham, broulik
Cc: cfeck, 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

D29050: KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex added a comment.


  Thanks and I get why it is hard to review.
  
  Could @broulik please help/have a look at this.
  A issue I have with understanding some of the KRunner stuff is that I don't 
know where exactly
  the runner "backend" is used (from the QML side of things).
  
  Thanks 

REPOSITORY
  R308 KRunner

BRANCH
  krunner_signal_bugfix (branched from master)

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

To: alex, meven, ngraham, broulik
Cc: cfeck, 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

D29050: KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex edited the summary of this revision.

REPOSITORY
  R308 KRunner

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

To: alex, meven, ngraham, broulik
Cc: cfeck, 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

D29050: KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex retitled this revision from "WIP KRunner fix prepare/teardown signals" to 
"KRunner fix prepare/teardown signals".

REPOSITORY
  R308 KRunner

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

To: alex, meven, ngraham, broulik
Cc: cfeck, 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

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex abandoned this revision.
alex added a comment.


  Abandoning this in favor of 
https://invent.kde.org/frameworks/kconfigwidgets/-/merge_requests/1 and a 
follow up PR.

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> meven wrote in kpluginselector.cpp:855
> Seems like a property would make sense, after all it is about it, or a ref to 
> the KCModuleInfo

I have created a PR on Gitlab for this

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> meven wrote in kpluginselector.cpp:855
> moduleInfo is part of the ctor here, so the fileName is already available 
> indirectly.
> A good thing would be to avoid having an option for such a niche usage 
> although legitimate.

> moduleInfo is part of the ctor here, so the fileName is already available 
> indirectly.

Yes, but the KCModuleProxy is just a wrapper class for the KCModule.
The actual KCModule gets created in kcmoduleloader.cpp line 93+.

PS: The moduleInfo is also available there, but I don't see any elegant way to 
make it available to the KCModule other than a property or appending it to the 
list of arguments.

>   A good thing would be to avoid having an option for such a niche usage 
> although legitimate.

Always open to suggestions 

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> meven wrote in kpluginselector.cpp:855
> Adding a fileName field to KCModuleProxy would make more sense to me, and do 
> it by default.
> Plus KCModuleProxy has already access to the fileName since it receives 
> moduleInfo. 
> But I am not a specialist.

You could add a field to the KCModule class (thats where you ultimately want to 
access the fileName).
But then the downside is that you don't have them as a constructor argument.

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex added a comment.


  Friendly Ping 

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

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

[Alexander Lohnau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R304:a2ecdd333c49: Do not mark entry as uninstalled if 
uninstallation script failed (authored by alex).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29123?vs=82228=82606

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h

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

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

[Alexander Lohnau]

alex marked 4 inline comments as done.

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

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

[Alexander Lohnau]

alex updated this revision to Diff 82228.
alex added a comment.


  Update docs
  
  I adjusted your suggestion a bit, the signalEntryChanged gets also emitted, 
when
  the user cancels the uninstallation :-).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29123?vs=82114=82228

BRANCH
  arcpatch-D29123_1

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h

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

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

[Alexander Lohnau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R304:9575f2defb27: KNS: Deprecate isRemote method and handle 
parse error properly (authored by alex).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29455?vs=82098=82190

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

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

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

[Alexander Lohnau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R304:2838e55acd17: KNS: Do not mark entry as installed if 
install script failed (authored by alex).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29451?vs=82180=82189

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

AFFECTED FILES
  src/core/installation.cpp

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

[Alexander Lohnau]

alex updated this revision to Diff 82180.
alex added a comment.


  Use qOverload
  
  No problem :-)

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29451?vs=82177=82180

BRANCH
  arcpatch-D29451

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

AFFECTED FILES
  src/core/installation.cpp

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

[Alexander Lohnau]

alex updated this revision to Diff 82177.
alex added a comment.


  Fix connect
  
  Thanks, I wasn't aware of that until now :-)

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29451?vs=82100=82177

BRANCH
  arcpatch-D29451

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

AFFECTED FILES
  src/core/installation.cpp

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

[Alexander Lohnau]

alex updated this revision to Diff 82114.
alex added a comment.


  Ensure bic

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29123?vs=81918=82114

BRANCH
  arcpatch-D29123_1

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h

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

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

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> leinir wrote in installation.h:124
> 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.

I told myself yesterday that I am going to have a look at this patch again^^
And you are absolutely right :-)

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

[Alexander Lohnau]

alex updated this revision to Diff 82100.
alex added a comment.


  Make lambda inline, capture variables

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29451?vs=82004=82100

BRANCH
  handle_install_script_error (branched from master)

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

AFFECTED FILES
  src/core/installation.cpp

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

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

[Alexander Lohnau]

alex marked 2 inline comments as done.

REPOSITORY
  R304 KNewStuff

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

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

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

[Alexander Lohnau]

alex updated this revision to Diff 82098.
alex marked an inline comment as done.
alex added a comment.


  Typo and error message
  
  If the error message is on two lines they aren't very readable, but
  that is an issue for another day :-).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29455?vs=82024=82098

BRANCH
  fix_isremote_stuff (branched from master)

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

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

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

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

[Alexander Lohnau]

alex updated this revision to Diff 82024.
alex retitled this revision from "KNS: Remove isRemote method and handle parse 
error properly" to "KNS: Deprecate isRemote method and handle parse error 
properly".
alex added a comment.


  Make isRemote deprecated

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29455?vs=82019=82024

BRANCH
  fix_isremote_stuff (branched from master)

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

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

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

D29447: Fix showing updates when the option is selected

[Alexander Lohnau]

alex added a comment.


  No problem, always happy to help 

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

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

[Alexander Lohnau]

alex created this revision.
alex added reviewers: KNewStuff, ngraham, leinir.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
alex requested review of this revision.

REVISION SUMMARY
  The isRemote check and the FIXMEs are removed, instead the result from the 
empty target check is properly handled.
  
  The same check as in isRemote is done in the Installation::readConfig method 
(line 122).
  If this fails a warning is given and in the engine is an error emitted. This 
error gets shown in the GUI.
  Then there are no other checks needed.

TEST PLAN
  Delete the line TargetDir from the dolphin servicemenu knsrc file.
  (Dolphin repository src/settings/services/servicemenu.knsrc).
  
  Before:
  F8256199: Screenshot_20200423_202404.png 

  After, an error message is shown:
  F8256197: Screenshot_20200423_202239.png 


REPOSITORY
  R304 KNewStuff

BRANCH
  fix_isremote_stuff (branched from master)

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h

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

D29447: Fix showing updates when the option is selected

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> itemsmodel.cpp:71
> +bool duplicate{false};
> +for (const EntryInternal  : qAsConst(m_entries)) {
> +if (existingEntry == entry) {

On second thought why not just use:
`bool duplicate =  m_entries.contains(entry);`

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

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

[Alexander Lohnau]

alex edited the summary of this revision.
alex edited the test plan for this revision.

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

[Alexander Lohnau]

alex created this revision.
alex added reviewers: KNewStuff, ngraham, leinir.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
alex requested review of this revision.

REVISION SUMMARY
  If the install script failed/was aborted the entry
  gets marked as installed. Now the exit code is checked
  and a error message is displayed.

TEST PLAN
  Make sure to have D29119 . Install the 
deb package of the "Jetbrains" dolphin addon.
  When the authorization popup comes press escape. The dolphin installer
  shows an error popup and the entry is not marked as installed.
  If you are not on a debian based system it should throw an error anyway ;-).

REPOSITORY
  R304 KNewStuff

BRANCH
  handle_install_script_error (branched from master)

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

AFFECTED FILES
  src/core/installation.cpp

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

D29050: WIP KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex added a comment.


  How should I proceed with this patch?

REPOSITORY
  R308 KRunner

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

To: alex, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29447: Fix showing updates when the option is selected

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> itemsmodel.cpp:71
> +bool duplicate{false};
> +for (const EntryInternal& existingEntry : m_entries) {
> +if (existingEntry == entry) {

Use qAsConst(m_entries) and space before & not after :-)

REPOSITORY
  R304 KNewStuff

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

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

[Alexander Lohnau]

alex updated this revision to Diff 81918.
alex added a comment.


  Merge branch master

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29123?vs=81476=81918

BRANCH
  arcpatch-D29123_1

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h

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

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex updated this revision to Diff 81873.
alex added a comment.


  Increase @since to 5.71

REPOSITORY
  R295 KCMUtils

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29201?vs=81856=81873

BRANCH
  service_path_append (branched from master)

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

AFFECTED FILES
  src/kpluginselector.cpp
  src/kpluginselector.h
  src/kpluginselector_p.h

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex added a reviewer: mart.

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik, mart
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex updated this revision to Diff 81856.
alex added a comment.


  Linebreak

REPOSITORY
  R295 KCMUtils

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29201?vs=81855=81856

BRANCH
  service_path_append (branched from master)

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

AFFECTED FILES
  src/kpluginselector.cpp
  src/kpluginselector.h
  src/kpluginselector_p.h

To: alex, #plasma, ngraham, meven, broulik
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex updated this revision to Diff 81855.
alex added a comment.


  Improve documentation
  
  Is that what you had in mind :-) ?
  
  And should I maybe add a comment to make this the default in KF6?

REPOSITORY
  R295 KCMUtils

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29201?vs=81799=81855

BRANCH
  service_path_append (branched from master)

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

AFFECTED FILES
  src/kpluginselector.cpp
  src/kpluginselector.h
  src/kpluginselector_p.h

To: alex, #plasma, ngraham, meven, broulik
Cc: mart, apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex added a comment.


  > Would it make sense to always pass it? Would it regress in any way?
  
  It would make sense, but it would change the existing behavior. 
  If you use the method setConfigurationArguments then you expect your argument 
list to be the list of arguments you specified + an entry for the metadata.
  So adding an entry by default could break things.

REPOSITORY
  R295 KCMUtils

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

To: alex, #plasma, ngraham, meven, broulik
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

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

[Alexander Lohnau]

alex marked 2 inline comments as done.

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

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex added a dependent revision: D29384: KCM Runners: Use setAppendServiceFile 
method for plugin selector.

REPOSITORY
  R295 KCMUtils

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

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

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex updated this revision to Diff 81799.
alex added a comment.


  Little typo :-)

REPOSITORY
  R295 KCMUtils

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29201?vs=81798=81799

BRANCH
  service_path_append (branched from master)

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

AFFECTED FILES
  src/kpluginselector.cpp
  src/kpluginselector.h
  src/kpluginselector_p.h

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

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex updated this revision to Diff 81798.
alex added a comment.


  Rename method

REPOSITORY
  R295 KCMUtils

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29201?vs=81239=81798

BRANCH
  service_path_append (branched from master)

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

AFFECTED FILES
  src/kpluginselector.cpp
  src/kpluginselector.h
  src/kpluginselector_p.h

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

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex edited the summary of this revision.
alex added reviewers: meven, broulik.

REPOSITORY
  R295 KCMUtils

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

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

D29101: KNewStuff: Fix file path and process call

[Alexander Lohnau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R304:4c6b3afcd80a: KNewStuff: Fix file path and process call 
(authored by alex).

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29101?vs=81467=81753

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

AFFECTED FILES
  src/core/installation.cpp

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

D29101: KNewStuff: Fix file path and process call

[Alexander Lohnau]

alex added a comment.


  Ups, that makes sense ^^
  Thanks!

REPOSITORY
  R304 KNewStuff

BRANCH
  arcpatch-D29101

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

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

D29101: KNewStuff: Fix file path and process call

[Alexander Lohnau]

alex added a comment.


  Friendly ping regarding the question :-)

REPOSITORY
  R304 KNewStuff

BRANCH
  arcpatch-D29101

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

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

D29175: DBus Runner: Add service property to request actions once

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> meven wrote in dbusrunnertest.cpp:206
> You should have added a QSignalSpy to check prepare was called, and another 
> for requestAction to check it has been called only once.

The concept behind this test is, that the runner is just loaded. If the 
Request-Actions-Once property is correctly implemented the actions are 
requested when the plugin is initialized. That means that they should be 
available and `prepare` doesn't need to be called.

So it would make sense to add a signal spy to check if prepare was not called 
at all, or am I missing something?

REPOSITORY
  R308 KRunner

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

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

D29175: DBus Runner: Add service property to request actions once

[Alexander Lohnau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R308:7b6222d8a10b: DBus Runner: Add service property to 
request actions once (authored by alex).

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29175?vs=81623=81687

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

AFFECTED FILES
  autotests/dbusrunnertest.cpp
  autotests/dbusrunnertest.desktop
  src/data/servicetypes/plasma-runner.desktop
  src/dbusrunner.cpp

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

D29175: DBus Runner: Add service property to request actions once

[Alexander Lohnau]

alex added a dependent revision: D29320: Baloo Runner: Use 
X-Plasma-Request-Actions-Once property in service.

REPOSITORY
  R308 KRunner

BRANCH
  request_actions_once (branched from master)

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

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

D29050: WIP KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex added a comment.


  > Adding a test would be the best way to demonstrate the issue and the fix.
  
  I debugged this and the debugger said that the `matchSessionComplete` method 
got called somewhere from QML, but I don't know from where.
  I also know only basic QML and the way the QML frontend interacts with the 
runner is also really confusing to me ;-).
  Thats why I originally filed a bug and not directly submitted a patch.

REPOSITORY
  R308 KRunner

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

To: alex, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

[Alexander Lohnau]

alex updated this revision to Diff 81623.
alex added a comment.


  Create test

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29175?vs=81165=81623

BRANCH
  request_actions_once (branched from master)

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

AFFECTED FILES
  autotests/dbusrunnertest.cpp
  autotests/dbusrunnertest.desktop
  src/data/servicetypes/plasma-runner.desktop
  src/dbusrunner.cpp

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

D29050: WIP KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex edited the test plan for this revision.

REPOSITORY
  R308 KRunner

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

To: alex, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 81546.
alex added a comment.


  Wrap macro about single method, change deprecation text
  
  And thanks for the feedback and explanations :-).

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=81543=81546

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

To: alex, #plasma, broulik, davidedmundson, vkrause
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 81543.
alex added a comment.


  Wrap implementations

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=81539=81543

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

To: alex, #plasma, broulik, davidedmundson, vkrause
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> kossebau wrote in abstractrunner.h:154
> The text in the API dox can stay "Since 5.0".
> 
> The only crititical thing where the number of an older, already released 
> version should not be used is the KRUNNER_ENABLE_DEPRECATED_SINCE macro. 
> Because that reacts to KF_DISABLE_DEPRECATED_BEFORE_AND_AT (or 
> KRUNNER_DISABLE_DEPRECATED_BEFORE_AND_AT if set). And someone using, say 
> KF_DISABLE_DEPRECATED_BEFORE_AND_AT=0x05 in their projects because they 
> checked their software and it has not used any deprecated API up to that 
> version, and who still uses this newly deprecated API, would suddenly get no 
> longer building software, which especially is annoying with already released 
> code.
> 
> So: when retroactively tagging deprecated API, in the API dox text mention 
> the correct version where actual deprecation happened, in the macros the 
> version of the upcoming release where the deprecation macros are first 
> existing for API users. Makes sense?

That makes sense, but I want to express that this method has been defunct  
since version 5.0
so that the maintainers of existing plugins can be sure that they can savely 
remove the method calls.
Whats the best way to express this?

REPOSITORY
  R308 KRunner

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

To: alex, #plasma, broulik, davidedmundson, vkrause
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 81539.
alex added a comment.


  Use mordern macros
  
  Thanks :-)

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=81528=81539

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

To: alex, #plasma, broulik, davidedmundson, vkrause
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex updated this revision to Diff 81528.
alex added a comment.


  Add missing KRUNNER_DEPRECATED

REPOSITORY
  R308 KRunner

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29281?vs=81527=81528

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

To: alex, #plasma, broulik, davidedmundson, vkrause
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29281: Deprecate defunct functions

[Alexander Lohnau]

alex created this revision.
alex added reviewers: Plasma, broulik, davidedmundson, vkrause.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
alex requested review of this revision.

REVISION SUMMARY
  As in T12163  explained KRunner should be 
ported away from QWidgets and the
  fuctions are defuct anyway.

TEST PLAN
  Compiles

REPOSITORY
  R308 KRunner

BRANCH
  deprecations (branched from master)

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

AFFECTED FILES
  src/abstractrunner.cpp
  src/abstractrunner.h
  src/querymatch.cpp
  src/querymatch.h

To: alex, #plasma, broulik, davidedmundson, vkrause
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

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

[Alexander Lohnau]

alex updated this revision to Diff 81476.
alex added a comment.


  Display more technical information in error message

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29123?vs=81076=81476

BRANCH
  arcpatch-D29123

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h

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

D29101: KNewStuff: Fix file path and process call

[Alexander Lohnau]

alex updated this revision to Diff 81467.
alex added a comment.


  Get program in exclusive line
  
  And should this go to release/20.04?

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29101?vs=81062=81467

BRANCH
  arcpatch-D29101

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

AFFECTED FILES
  src/core/installation.cpp

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

D29201: KCMUtils: Add option to append service file to list of arguments

[Alexander Lohnau]

alex created this revision.
alex added reviewers: Plasma, ngraham.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
alex requested review of this revision.

REVISION SUMMARY
  By setting the new variable to true the service path gets appended
  to the list of arguments. Otherwise there is no way 
  (at least not that I know of, please correct me if I am wrong) of getting the 
service
  file which was used to launch the module.

TEST PLAN
  Add `m_pluginSelector->setAppendServicePath(true);` in the
  kcms/runners/kcm.cpp in the repo plasma-desktop. If you print out the 
arguments of
  a runner config module (plasma-workspace contains some) you should see the 
service file.

REPOSITORY
  R295 KCMUtils

BRANCH
  service_path_append (branched from master)

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

AFFECTED FILES
  src/kpluginselector.cpp
  src/kpluginselector.h
  src/kpluginselector_p.h

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

D29175: DBus Runner: Add service property to request actions once

[Alexander Lohnau]

alex added a comment.


  No, they are independent.
  I just mentioned this to avoid confusion when debugging.

REPOSITORY
  R308 KRunner

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

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

D29050: WIP KRunner fix prepare/teardown signals

[Alexander Lohnau]

alex retitled this revision from "WIP KRunner: Fix Bug 420311" to "WIP KRunner 
fix prepare/teardown signals".

REPOSITORY
  R308 KRunner

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

To: alex, meven, ngraham, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29175: DBus Runner: Add service property to request actions once

[Alexander Lohnau]

alex created this revision.
alex added reviewers: Plasma, meven, ngraham, broulik.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
alex requested review of this revision.

REVISION SUMMARY
  With the `X-Plasma-Request-Actions-Once` property the plugin
  can specify if the actions should only be requested when the plugin is 
initialized.
  
  This is useful if the plugin always used the same actions, because
  then the we don't need to make the dbus calls for every match session.

TEST PLAN
  Create dbus runner and don't set the extra property, the actions should be 
requested for each session.
  (More like each query because of BUG: 420311).
  After setting the property to true the actions should only be requested when 
the runner is initialized.

REPOSITORY
  R308 KRunner

BRANCH
  request_actions_once (branched from master)

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

AFFECTED FILES
  src/data/servicetypes/plasma-runner.desktop
  src/dbusrunner.cpp

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

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

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

> leinir wrote in installation.cpp:632
> 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).

Yes I get what you mean :-).

I was just not sure where this patch should land (I asked about this in  a 
previous comment) and wanted to know this before editing translations.
But I guess it will go on master then?

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

D29101: KNewStuff: Fix file path and process call

[Alexander Lohnau]

alex added a dependent revision: D29123: Do not mark entry as uninstalled if 
uninstallation script failed.

REPOSITORY
  R304 KNewStuff

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

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

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

[Alexander Lohnau]

alex edited the summary of this revision.
alex added a dependency: D29101: KNewStuff: Fix file path and process call.

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: Do not mark entry as uninstalled if uninstallation script failed

[Alexander Lohnau]

alex updated this revision to Diff 81076.
alex retitled this revision from "WIP BUG: 420312. Do not mark entry as 
uninstalled if uninstallation script failed" to "Do not mark entry as 
uninstalled if uninstallation script failed".
alex edited the summary of this revision.
alex added a comment.


  Emit signalEntryChanged for manually deletet entry

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29123?vs=81075=81076

BRANCH
  bugfix_install_uninstall_messages (branched from master)

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h

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

[Alexander Lohnau]

alex updated this revision to Diff 81075.
alex added a comment.


  Merge branch 'bugfix_uninstall'

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29123?vs=80976=81075

BRANCH
  bugfix_install_uninstall_messages (branched from master)

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h

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

D29101: KNewStuff: Fix file path and process call

[Alexander Lohnau]

alex retitled this revision from "WIP Fix file path and process call" to 
"KNewStuff: Fix file path and process call".

REPOSITORY
  R304 KNewStuff

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

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

D29101: WIP BUG: 420312 Fix file path and process call

[Alexander Lohnau]

alex edited the summary of this revision.

REPOSITORY
  R304 KNewStuff

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

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

D29101: WIP BUG: 420312 Fix file path and process call

[Alexander Lohnau]

alex updated this revision to Diff 81062.
alex added a comment.


  Use KShell to split args

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29101?vs=80907=81062

BRANCH
  bugfix_uninstall (branched from master)

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

AFFECTED FILES
  src/core/installation.cpp

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

D28582: KRunner: Show error message for Actions in dbus runner

[Alexander Lohnau]

alex added a comment.


  I am also going to try a friendly ping here :-).

REPOSITORY
  R308 KRunner

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

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

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

[Alexander Lohnau]

alex marked 3 inline comments as done.

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

[Alexander Lohnau]

alex updated this revision to Diff 80976.
alex added a comment.


  Use internal question system
  
  PS: I am not sure on which branch this should land,
  thats why I haven't edited the translations.

REPOSITORY
  R304 KNewStuff

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29123?vs=80967=80976

BRANCH
  bugfix_install_uninstall_messages (branched from master)

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

AFFECTED FILES
  src/core/engine.cpp
  src/core/installation.cpp
  src/core/installation.h

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

[Alexander Lohnau]

alex added a comment.


  No problem :-). And good to know that the concept of this patch makes sense 
^^.

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