Stepping down as KRunner maintainer

2020-10-08 Thread Kai Uwe Broulik
Hi everyone,

you might not even have known this but officially I have been KRunner's 
maintainer for several years at this point :-)

However, I have decided to step down as its maintainer as I believe won't be 
able to really move KRunner forward for KF6 despite a grand vision [1] for a 
lack of time and, frankly, motivation.

Therefore, I'd like to pass the torch to Alexander Lohnau. He's bringing many 
fresh ideas to the table and is very enthusiastic about making KRunner shine 
again. He's also surely done more in the past months than I did in the past 
years and so it's only fair to not have his ambitions hindered by my 
unresponsiveness on code reviews.

Alexander, thank you again for hosting that KRunner BoF at your first ever 
Akademy - the stage is yours ;-)

Cheers
Kai Uwe 

[1] https://phabricator.kde.org/T12031

Re: Add loop device interface to Solid framework

2020-06-13 Thread Kai Uwe Broulik

Hi,

isn't that something that should just be in StorageVolume or something? 
We can already handle mounted ISOs in Solid just find - we only need 
some way to create/delete them?


Cheers
Kai Uwe


D29050: KRunner fix prepare/teardown signals

2020-06-07 Thread Kai Uwe Broulik
broulik added a comment.


  I now have runner queries never "finish". I type "plasmashell", get the 
results I expect, and the busy indicator keeps spinning forever.

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


D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

2020-05-29 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> ahmadsamir wrote in kopenwithdialog.cpp:1008
> https://doc.qt.io/qt-5.15/qfileinfo.html#isAbsolute ?

But I don't want to create a `QFIleInfo` just because

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, #vdg
Cc: ahmadsamir, dfaure, apol, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns


D21555: [KFilePlacesModel] Explicitly query for PTP cameras

2020-05-29 Thread Kai Uwe Broulik
broulik abandoned this revision.

REPOSITORY
  R241 KIO

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

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


D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

2020-05-29 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> broulik wrote in kopenwithdialog.cpp:1008
> Forgot that also existed, will update

Actually, that doesn't exist

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, #vdg
Cc: ahmadsamir, dfaure, apol, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns


D21126: [KUiServerJobTracker] Register jobs asynchronously

2020-05-29 Thread Kai Uwe Broulik
broulik abandoned this revision.
broulik added a comment.


  I've got a jobviewv2 thing in the works which is qvariantmap-based so it can 
easily queue any changes and send them out once registered. I think jobviewv1 
should just stay as is and be phased out eventually, cf. T12574 


REPOSITORY
  R288 KJobWidgets

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

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


D21127: [KDynamicJobTracker] Ask kuiserver asynchronously

2020-05-29 Thread Kai Uwe Broulik
broulik abandoned this revision.
broulik added a comment.


  I've got a jobviewv2 thing in the works which is qvariantmap-based so it can 
easily queue any changes and send them out once registered. I think jobviewv1 
should just stay as is and be phased out eventually, cf. T12574 


REPOSITORY
  R241 KIO

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

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


D29035: Install service files for kwin

2020-05-22 Thread Kai Uwe Broulik
broulik marked an inline comment as done.

REPOSITORY
  R108 KWin

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

To: broulik, #plasma, #frameworks
Cc: davidedmundson, kwin, Orage, cacarry, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, mkulinski, ragreen, jackyalcine, iodelay, 
crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
hardening, romangg, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D29050: KRunner fix prepare/teardown signals

2020-05-22 Thread Kai Uwe Broulik
broulik added a comment.


  I'm sorry, I don't really know how this teardown stuff all works :/

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


D29035: Install service files for kwin

2020-05-22 Thread Kai Uwe Broulik
broulik updated this revision to Diff 83108.
broulik added a comment.


  - Use `After` instead of `Wants`

REPOSITORY
  R108 KWin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29035?vs=80723=83108

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

AFFECTED FILES
  CMakeLists.txt
  plasma-kwin_wayland.service.in
  plasma-kwin_x11.service.in

To: broulik, #plasma, #frameworks
Cc: davidedmundson, kwin, Orage, cacarry, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, mkulinski, ragreen, jackyalcine, iodelay, 
crozbo, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
hardening, romangg, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D29825: Add SystemdService to DBus service file

2020-05-22 Thread Kai Uwe Broulik
broulik created this revision.
broulik added reviewers: Frameworks, davidedmundson, kossebau.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  Using the new `ECMGenerateDBusServiceFile` of D29051 


TEST PLAN
  - Had a proper file generated and installed

REPOSITORY
  R297 KDED

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

AFFECTED FILES
  CMakeLists.txt
  src/CMakeLists.txt

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


D29051: Add ecm_generate_dbus_service_file

2020-05-22 Thread Kai Uwe Broulik
broulik updated this revision to Diff 83105.
broulik added a comment.


  - Clarify docs

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29051?vs=80776=83105

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

AFFECTED FILES
  docs/module/ECMGenerateDBusServiceFile.rst
  modules/ECMGenerateDBusServiceFile.cmake

To: broulik, #frameworks, davidedmundson, kossebau, kfunk, habacker
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, 
michaelh, ngraham, bruns


D29051: Add ecm_generate_dbus_service_file

2020-05-22 Thread Kai Uwe Broulik
broulik marked 3 inline comments as done.

REPOSITORY
  R240 Extra CMake Modules

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

To: broulik, #frameworks, davidedmundson, kossebau, kfunk, habacker
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, 
michaelh, ngraham, bruns


D25461: [KCM GridDelegate] Use non-attached ToolTip

2020-05-21 Thread Kai Uwe Broulik
broulik abandoned this revision.

REPOSITORY
  R296 KDeclarative

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

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


D29420: Generate DBus interface

2020-05-20 Thread Kai Uwe Broulik
broulik accepted this revision.
broulik added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> notifybypopup.cpp:363
> +watcher->deleteLater();
> +QDBusPendingReply reply = *watcher;
> +notifications.insert(reply.argumentAt<0>(), notification);

I think we should do an error check and whether we got the correct argument 
count back but we previously also didn't do it, so probably fine

REPOSITORY
  R289 KNotifications

BRANCH
  geninterface

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

To: nicolasfella, #frameworks, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29228: [KProcessRunner] Use only executable name for scope

2020-05-19 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:19d23d386cf7: [KProcessRunner] Use only executable name 
for scope (authored by broulik).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29228?vs=81338=83058

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

AFFECTED FILES
  src/gui/kprocessrunner.cpp

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


D28673: [PackageUrlInterceptor] Make QRegularExpression static

2020-05-19 Thread Kai Uwe Broulik
broulik added a comment.


  so, can the regexp be replaced or does it need to stay?

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D29391: Introduce setWindow and CloseWhenWindowActivated

2020-05-19 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> knotification.h:253
>   */
>  CloseWhenWidgetActivated = 0x04,
>  

Should we deprecate this? Or just use this one instead. I don't see why we 
would have separate enum values for those

REPOSITORY
  R289 KNotifications

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

To: nicolasfella, #frameworks, broulik
Cc: anthonyfieroni, kossebau, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns


D25984: Load translations

2020-05-18 Thread Kai Uwe Broulik
broulik abandoned this revision.
broulik added a comment.


  Looks like D27595  fixes the issue

REPOSITORY
  R169 Kirigami

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

To: broulik, #kirigami, #frameworks, kossebau, aacid
Cc: vkrause, mart, davidedmundson, plasma-devel, fbampaloukas, GB_2, domson, 
dkardarakos, ngraham, apol, ahiemstra


D29815: Fix blurry icons in titlebar appmenu by adding UseHighDpiPixmaps flag

2020-05-18 Thread Kai Uwe Broulik
broulik added a comment.


  I am not sure we can just do this given the amount of modules kded loads. Can 
we be certain that all the modules don't do custom pixmap painting?

REPOSITORY
  R297 KDED

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

To: mthw, #frameworks
Cc: broulik, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D28312: Implement urls using hints

2020-05-18 Thread Kai Uwe Broulik
broulik accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R289 KNotifications

BRANCH
  urlshint

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

To: nicolasfella, #frameworks, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29034: Add systemd user service file for kded

2020-05-15 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R297:4ae4de9ff469: Add systemd user service file for kded 
(authored by broulik).

REPOSITORY
  R297 KDED

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29034?vs=82868=82949

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

AFFECTED FILES
  src/CMakeLists.txt
  src/plasma-kded.service.in

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


D29034: Add systemd user service file for kded

2020-05-14 Thread Kai Uwe Broulik
broulik updated this revision to Diff 82868.
broulik added a comment.


  - fix exec
  - add slice

REPOSITORY
  R297 KDED

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29034?vs=80722=82868

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

AFFECTED FILES
  src/CMakeLists.txt
  src/plasma-kded.service.in

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


D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-08 Thread Kai Uwe Broulik
broulik accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  2020_05_openurljob

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

To: dfaure, ahmadsamir, broulik, meven, kossebau, davidedmundson, nicolasfella, 
svuorela
Cc: feverfew, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-08 Thread Kai Uwe Broulik
broulik added a comment.


  +1

REPOSITORY
  R241 KIO

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

To: dfaure, ahmadsamir, broulik, meven, kossebau, davidedmundson, nicolasfella, 
svuorela
Cc: feverfew, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29502: kwidgetsaddons: Add a named colors support in KColorCombo.

2020-05-07 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> kcolorcombo.h:60
> +/** Struct used in NamedColors list */
> +struct KNamedColor
> +{

Do we really want to leak this `struct` into public API?

REPOSITORY
  R236 KWidgetsAddons

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

To: araujoluis, tcanabrava, patrickelectric, hindenburg, ngraham
Cc: broulik, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns


D29503: Pixel align children of GridViewInternal

2020-05-07 Thread Kai Uwe Broulik
broulik accepted this revision.

REPOSITORY
  R296 KDeclarative

BRANCH
  master

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

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


D29479: Fix rounded borders

2020-05-06 Thread Kai Uwe Broulik
broulik accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

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


D29479: Fix rounded borders

2020-05-06 Thread Kai Uwe Broulik
broulik requested changes to this revision.
broulik added a comment.
This revision now requires changes to proceed.


  Notifications don't use `PlasmoidHeading`

INLINE COMMENTS

> PlasmoidHeading.qml:71
> +enabledBorders: {
> +var borders = new Array()
> +borders |= PlasmaCore.FrameSvg.LeftBorder

Use `[]` instead of `new Array()`

> PlasmoidHeading.qml:72
> +var borders = new Array()
> +borders |= PlasmaCore.FrameSvg.LeftBorder
> +borders |= PlasmaCore.FrameSvg.RightBorder

How is operator `|=` supposed to work with an `Array`?!

> PlasmoidHeading.qml:74
> +borders |= PlasmaCore.FrameSvg.RightBorder
> +if (plasmoid.location !== PlasmaCore.Types.TopEdge || location 
> != PlasmoidHeading.Location.Header) {
> +borders |= PlasmaCore.FrameSvg.TopBorder

Where is `plasmoid` defined?

> plasmoidheading.svg:9
> +
> +
> +

You've just added a gazillion non-integer coordinates

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

2020-05-06 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> ahmadsamir wrote in kopenwithdialog.cpp:1008
> Why not QFileInfo::isAbsolute()?

Forgot that also existed, will update

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, #vdg
Cc: ahmadsamir, dfaure, apol, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns


D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-06 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> meven wrote in previewjob.cpp:433
> Maybe make this static, so that apps have to do it once per app sort of like 
> we do with `Qt::AA_UseHighDpiPixmaps`, rather than by KPreviewJob.

Not a fan. You can have different dpi per screen.
Maybe instead we should have a `QWindow*` method or constructor to take dpr from

REPOSITORY
  R241 KIO

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

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


D29445: [KOpenWithDialog] When pointing at a non-executable file print more meaningful error

2020-05-05 Thread Kai Uwe Broulik
broulik created this revision.
broulik added reviewers: Frameworks, VDG.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  `QStandardPaths::findExecutable` only finds executable binaries, so when 
explicitly pointing it at an existing file, it would complain about it not 
existing, rather than telling you it's not executable.

TEST PLAN
  - Tried to associate a non-executable shell script with a file: it now told 
me what was up
  
  Before it would tell me file doesn't exist

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp

To: broulik, #frameworks, #vdg
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-05 Thread Kai Uwe Broulik
broulik added a comment.


  > At the risk of asking a stupid question, why?
  
  The text thumbnailer should be able to produce readable text on high dpi, or 
the folder thumbnailer should render the folder icon sharply and the picture 
frames non-pixelated

REPOSITORY
  R241 KIO

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

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


D29420: Generate DBus interface

2020-05-05 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> notifybypopup.cpp:115
>  
> - bool connected = QDBusConnection::sessionBus().connect(QString(), // 
> from any service
> -   
> QString::fromLatin1(dbusPath),

Previously it would accept the signal from any service which I find odd, 
though. Could you maybe check git logs to see if there was a reason for this?
It should survive restarts anyway and the service name is defined, so I really 
don't see why it used to be a blind connect.

> notifybypopup.cpp:364
> +QDBusPendingReply reply = *watcher;
> +notifications.insert(reply.argumentAt<0>(), notification);
> +});

You have `watcher` (which is parented to `notification`) and `notification` as 
contexts, but in the lambda you also access `this`. This looks dangerous. How 
about using `this` instead of `notification` as context object?

> notifybypopup.cpp:376
> +
> +QDBusPendingCall call = dbusInterface.GetCapabilities();
> +

This is `QDBusPendingReply` (or just `auto`...)

> notifybypopup.cpp:385
> +popupServerCapabilities = capabilities;
> +dbusServiceCapCacheDirty = false;
> +

Unrelated: I was wondering, do we actually monitor the notification service 
changing and make them dirty again?

> notifybypopup.cpp:388
> +// re-run notify() on all enqueued notifications
> +for (int i = 0, total = notificationQueue.size(); i < total; ++i) {
> +q->notify(notificationQueue.at(i).first, 
> notificationQueue.at(i).second);

range for?

REPOSITORY
  R289 KNotifications

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

To: nicolasfella, #frameworks, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29397: KPreviewJob : Support for DeviceRatioPixel

2020-05-05 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> thumbcreator.h:139
>   */
> -virtual bool create(const QString , int width, int height, QImage 
> ) = 0; // KF6 TODO: turn first arg into QUrl (see 
> thumbnail/htmlcreator.cpp)
> +virtual bool create(const QString , int width, int height, QImage 
> , qreal devicePixelRatio = 1.0) = 0; // KF6 TODO: turn first arg into 
> QUrl (see thumbnail/htmlcreator.cpp)
>  

You can't do that

REPOSITORY
  R241 KIO

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

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


D29434: Use small font for ExpandableListItem subtitle

2020-05-04 Thread Kai Uwe Broulik
broulik added a comment.


  Yes, please!

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D29405: [PoC] Make notifications work without a notifyrc file

2020-05-04 Thread Kai Uwe Broulik
broulik added a comment.


  > Maybe we need something similar to `X-GNOME-UsesNotifications=true`
  
  Or we just also use that :D

REPOSITORY
  R289 KNotifications

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

To: nicolasfella, #frameworks, broulik, vkrause
Cc: apol, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29336: Remove galago from method/variable naming

2020-05-04 Thread Kai Uwe Broulik
broulik accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R289 KNotifications

BRANCH
  nogalago

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

To: nicolasfella, #frameworks, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D28499: [LauncherJobs] Emit description

2020-05-04 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:692d9bc81f00: [LauncherJobs] Emit description (authored 
by broulik).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D28499?vs=79700=81849#toc

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28499?vs=79700=81849

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

AFFECTED FILES
  src/gui/applicationlauncherjob.cpp
  src/gui/commandlauncherjob.cpp

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


D29391: Introduce setWindow and CloseWhenWindowActivated

2020-05-03 Thread Kai Uwe Broulik
broulik added a comment.


  Generally +1
  The `QWidget` one has some additional features, like closing the notification 
when the window is destroyed (cf. its docs), which would be useful?
  Not a fan of the timer.
  Also, we might want to deprecate the `QWidget` one now?

REPOSITORY
  R289 KNotifications

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

To: nicolasfella, #frameworks, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-03 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> broulik wrote in openurljob.cpp:274
> I know what you always say when I say what I always say but can we use a 
> mimetype job here? :)

Seems I forgot to remove this when I added the comment above instead :)

REPOSITORY
  R241 KIO

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

To: dfaure, ahmadsamir, broulik, meven, kossebau, davidedmundson, nicolasfella, 
svuorela
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29385: Introduce KIO::OpenUrlJob, a rewrite and replacement for KRun

2020-05-03 Thread Kai Uwe Broulik
broulik added a comment.


  Cool!

INLINE COMMENTS

> openurljob.cpp:261
> +
> +void KIO::OpenUrlJobPrivate::determineLocalMimeType()
> +{

I know what you always say when I say what I always say, but why not just 
always stat/mimetype job?

> openurljob.cpp:274
> +QMimeDatabase db;
> +QMimeType mime = db.mimeTypeForUrl(m_url);
> +//qDebug() << "MIME TYPE is " << mime.name();

I know what you always say when I say what I always say but can we use a 
mimetype job here? :)

> openurljob.cpp:305
> +q->emitResult();
> +} else {
> +if (m_followRedirections) { // Update our URL in case of a 
> redirection

Using an early return here would make the code less nested

> openurljob.cpp:447
> +// X-KDE-LastOpenedWith holds the service desktop entry name that
> +// was should be preferred for opening this URL if possible.
> +// This is used by the Recent Documents menu for instance.

was or should be?

> openurljob.cpp:506
> +q->setError(KJob::UserDefinedError);
> +q->setErrorText(i18n("The file %1 is an executable 
> program. "
> + "For safety it will not be started.", 
> m_url.toDisplayString().toHtmlEscaped()));

While at it, can we clean up/unify those texts? Sometimes it puts the file on a 
new line, sometimes it's bold, sometines in quotes, etc. Generally I wouldn't 
really want any HTML formatting in there.

REPOSITORY
  R241 KIO

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

To: dfaure, ahmadsamir, broulik, meven, kossebau, davidedmundson, nicolasfella, 
svuorela
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29292: Port KKeySequenceItem to QQC2

2020-04-30 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> davidedmundson wrote in KeySequenceItem.qml:57
> > Can we use the non-attached version here please since it's not likely
> 
> It's the worst!
> 
> > Are we able to use some form of units? Hardcoding this seems wrong.
> 
> It's come up before, this isn't ideal, but there's no other consistent 
> alternative. It's the convention followed elsewhere.
> 
> Let's follow that up on https://phabricator.kde.org/T10873

Why is it the worst? It keeps us from having to hardcode magic numbers.

> davidedmundson wrote in KeySequenceItem.qml:58
> We need to specify the domain, but you're right i18nd would work just as well 
> and save a QObject

Assuming `i18n` is always available? Maybe should stick to `qsTr`

REPOSITORY
  R296 KDeclarative

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

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


D29292: Port KKeySequenceItem to QQC2

2020-04-29 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> KeySequenceItem.qml:57
> +ToolTip.visible: hovered
> +ToolTip.delay: 1000
> +ToolTip.text:  _tr.i18n("Click on the button, then enter the 
> shortcut like you would in the program.\nExample for Ctrl+A: hold the Ctrl 
> key and press A.")

Can we use the non-attached version here please since it's not likely (except 
maybe System Tray delegate) to be used in large quantities?

> KeySequenceItem.qml:58
> +ToolTip.delay: 1000
> +ToolTip.text:  _tr.i18n("Click on the button, then enter the 
> shortcut like you would in the program.\nExample for Ctrl+A: hold the Ctrl 
> key and press A.")
> +ToolTip.timeout: 5000

Is this `_tr` thing we could also improve (separately o/c)?

> KeySequenceItem.qml:61
>  
>  onCheckedChanged: {
>  if (checked) {

Probably can now use `onToggled`?

REPOSITORY
  R296 KDeclarative

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

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


D29021: Remove checks for notification service and fallback to KPassivePopup

2020-04-29 Thread Kai Uwe Broulik
broulik accepted this revision.
broulik added a comment.
This revision is now accepted and ready to land.


  Verified that close signal works and that quitting plasma, sending a 
notification, then has the notification show up when plasma is started.
  Please wait until after Frameworks tagging before pushing this

REPOSITORY
  R289 KNotifications

BRANCH
  nofallback2

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

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


D29153: Move handling of untrusted programs to ApplicationLauncherJob.

2020-04-28 Thread Kai Uwe Broulik
broulik accepted this revision.
broulik added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> applicationlauncherjob.cpp:155
> +}
> +proceedAfterSecurityChecks();
> +}

With this it starts to look as hard to follow as KRun :)

REPOSITORY
  R241 KIO

BRANCH
  2020_04_UntrustedProgramHandler

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

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


D28624: Print a warning if runner is incompatible with KRunner

2020-04-28 Thread Kai Uwe Broulik
broulik accepted this revision.
broulik added a comment.
This revision is now accepted and ready to land.


  Would be lovely if we could get this date parsing into `QVersionNumber` of 
`KFormat`, I'm seeing similar code also in kscreenlocker, kwin, libksysguard, 
...

REPOSITORY
  R308 KRunner

BRANCH
  warning (branched from master)

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

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


D29228: [KProcessRunner] Use only executable name for scope

2020-04-27 Thread Kai Uwe Broulik
broulik created this revision.
broulik added reviewers: Frameworks, Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  Slashes aren't allowed in scope names, so when a full executable path is 
launched, e.g. ยด/usr/bin/foo` it would fail to be scoped.
  It's kinda wrong, too, but better than not scoping it at all.

TEST PLAN
  In D29226  I now have my session restored 
apps in scopes, though they're naturally not the same ones as when I have a 
desktop entry associated with the launch job

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/gui/kprocessrunner.cpp

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


Re: changing icon sizes no longer emits signal

2020-04-27 Thread Kai Uwe Broulik
(That's why confirmation prompts are pointless, one gets so used to just 
acknowledge them all that that I sent this mail mid-sentece :)


...so perhaps we should just change it to always emit the dbus signal 
manually when something changed, rather than it being nested inside 
exportToKDE4().


So, basically like David's patch, except maybe just with the explicit 
DBus call, so we can get rid of the emitChange call, which I think might 
be kdelibs4support material?


Cheers
Kai Uwe



Re: changing icon sizes no longer emits signal

2020-04-27 Thread Kai Uwe Broulik

Hi,


A patch like the one attached seems to help, but someone who knows the KCM
better (or has time to dig) should make this conditional on the user actually
changing icon sizes, and only emit for the groups that have changed.


Patch makes sense, though I wonder why we do both emitChange and a 
manual DBus call. From what I gather emitChange does the same.


Plasma-Integration also connects to that signal to update icon sizes.

It looks like a flaw in the previous code, where it would always run 
exportToKDE4() when anything was changed, which would then emit the 
signal, which unbeknownst to me, was also still being used :)


In Plasma 5.17 it used to be:
> if (m_selectedThemeDirty || m_iconSizesDirty || m_revertIconEffects) {
> exportToKDE4();
> }

So perha



D29153: Move handling of untrusted programs to ApplicationLauncherJob.

2020-04-25 Thread Kai Uwe Broulik
broulik added a comment.


  The question is how that will work in conjunction with 
`KNotificationJobUiDelegate`? In principle we could also make it emit a 
`KNotification` with buttons

INLINE COMMENTS

> untrustedprogramhandlerinterface.h:79
> + */
> +bool makeServiceFileExecutable(const QString , QString 
> );
> +

I was wondering if this should be done async? Nested event loops are quite a 
problem when QML is involved.

> applicationlauncherjob.cpp:117
> +// but that gets rid of the command line arguments.
> +QString program = 
> QFileInfo(d->m_service->exec()).canonicalFilePath();
> +if (program.isEmpty()) { // e.g. due to command line arguments

You know I'm not a fan of jobs suddenly blocking on IO :)

REPOSITORY
  R241 KIO

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

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


D29034: Add systemd user service file for kded

2020-04-23 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> bruns wrote in plasma-kded.service.in:5
> How about using some LIBEXEC dir instead, this should never be called 
> directly, or am I missing something?

This is unrelated and kded has been there forever

REPOSITORY
  R297 KDED

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

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


D29062: Port KToolInvocation::kdeinitExecWait to QProcess

2020-04-23 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R297:37137cec3a92: Port KToolInvocation::kdeinitExecWait to 
QProcess (authored by broulik).

REPOSITORY
  R297 KDED

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29062?vs=80785=80973

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

AFFECTED FILES
  src/kded.cpp

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


D29037: Add preferences-desktop-tablet and preferences-desktop-touchpad icons

2020-04-21 Thread Kai Uwe Broulik
broulik added a comment.


  At first glance I thought that drawing tablet icon was about the scroll bar

REPOSITORY
  R266 Breeze Icons

BRANCH
  master

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

To: guoyunhe, #breeze, ngraham
Cc: broulik, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns


D29062: Port KToolInvocation::kdeinitExecWait to QProcess

2020-04-21 Thread Kai Uwe Broulik
broulik created this revision.
broulik added reviewers: davidedmundson, Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

TEST PLAN
  Part of T12185 
  Verified it's calling it and returning with `NormalExit` and `0` exitcode - 
didn't actually test whether it worked :D

REPOSITORY
  R297 KDED

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

AFFECTED FILES
  src/kded.cpp

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


D29051: Add ecm_generate_dbus_service_file

2020-04-21 Thread Kai Uwe Broulik
broulik updated this revision to Diff 80776.
broulik added a comment.


  Add docs rst

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29051?vs=80775=80776

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

AFFECTED FILES
  docs/module/ECMGenerateDBusServiceFile.rst
  modules/ECMGenerateDBusServiceFile.cmake

To: broulik, #frameworks, davidedmundson, kossebau, kfunk, habacker
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, 
michaelh, ngraham, bruns


D29051: Add ecm_generate_dbus_service_file

2020-04-21 Thread Kai Uwe Broulik
broulik updated this revision to Diff 80775.
broulik added a comment.


  - Improve docs
  - Add `DESTINATION` arg

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29051?vs=80766=80775

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

AFFECTED FILES
  modules/ECMGenerateDBusServiceFile.cmake

To: broulik, #frameworks, davidedmundson, kossebau, kfunk, habacker
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, 
michaelh, ngraham, bruns


D29051: Add ecm_generate_dbus_service_file

2020-04-21 Thread Kai Uwe Broulik
broulik edited the summary of this revision.
broulik edited the test plan for this revision.

REPOSITORY
  R240 Extra CMake Modules

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

To: broulik, #frameworks, davidedmundson, kossebau, kfunk, habacker
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, 
michaelh, ngraham, bruns


D29051: Add ecm_generate_dbus_service_file

2020-04-21 Thread Kai Uwe Broulik
broulik edited the test plan for this revision.

REPOSITORY
  R240 Extra CMake Modules

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

To: broulik, #frameworks, davidedmundson, kossebau, kfunk, habacker
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, 
michaelh, ngraham, bruns


D29051: Add ecm_generate_dbus_service_file

2020-04-21 Thread Kai Uwe Broulik
broulik created this revision.
broulik added reviewers: Frameworks, davidedmundson, kossebau, kfunk, habacker.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  It serves as a replacement for `kdbusaddons_generate_dbus_service_file`.
  
  An application can be a DBus-activated service just fine without using 
KDBusAddons.
  Moreover, this new module uses named arguments for future-proofing, and adds 
support for specifying a `SystemdService`.
  It also cleans up the confusion on what the "path" is about: Rather than 
requiring to specify executable and path separately, we just extract the 
executable file name on Windows, if necessary.

TEST PLAN
  - Was able to generate a kded service file
  - Was able to generate a kded service file with `SystemdUnit`
  - Verified that it moaned when executable wasn't an absolute path
  - Untested on Windwos

REPOSITORY
  R240 Extra CMake Modules

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

AFFECTED FILES
  modules/ECMGenerateDBusServiceFile.cmake

To: broulik, #frameworks, davidedmundson, kossebau, kfunk, habacker
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, cblack, bencreasy, 
michaelh, ngraham, bruns


D29034: Add systemd user service file for kded

2020-04-21 Thread Kai Uwe Broulik
broulik planned changes to this revision.

REPOSITORY
  R297 KDED

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

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


D29036: Print meaningful warning when there is no QGuiApplication

2020-04-21 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R278:57e41449ed82: Print meaningful warning when there is no 
QGuiApplication (authored by broulik).

REPOSITORY
  R278 KWindowSystem

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29036?vs=80724=80727

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

AFFECTED FILES
  src/pluginwrapper.cpp

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


D28499: [LauncherJobs] Emit description

2020-04-21 Thread Kai Uwe Broulik
broulik added a comment.


  ping

REPOSITORY
  R241 KIO

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

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


D29036: Print meaningful warning when there is no QGuiApplication

2020-04-21 Thread Kai Uwe Broulik
broulik created this revision.
broulik added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  KWindowSystem plug-in loading (and everything regarding a window system, like 
X or Wayland connection anyway) depends on `QGuiApplication::platformName()` 
which will be empty when there is none, such as in a `QCoreApplication`.
  This would then just print a meaningless error on how now platform plug-in 
was found.

TEST PLAN
  - Created a `QCoreApplication`, called `KWindowSystem::windows()`, got a 
warning that I need a `QGuiApplication` and an empty window list
  - Created a `QGuiApplication`, called `KWindowSystem::windows()`, got no 
warning and a lost of X window ids
  - Created a `QApplication`, called `KWindowSystem::windows()`, got no warning 
and a lost of X window ids

REPOSITORY
  R278 KWindowSystem

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

AFFECTED FILES
  src/pluginwrapper.cpp

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


D29035: Install service files for kwin

2020-04-21 Thread Kai Uwe Broulik
broulik created this revision.
broulik added reviewers: Plasma, Frameworks.
Herald added a project: KWin.
Herald added a subscriber: kwin.
broulik requested review of this revision.

TEST PLAN
  See T11914  and D28305 


REPOSITORY
  R108 KWin

BRANCH
  davidedmundson/systemd_startup

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

AFFECTED FILES
  CMakeLists.txt
  plasma-kwin_wayland.service.in
  plasma-kwin_x11.service.in

To: broulik, #plasma, #frameworks
Cc: kwin, Orage, cacarry, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, mkulinski, ragreen, jackyalcine, iodelay, crozbo, bwowk, 
ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, 
romangg, jensreuterberg, abetts, sebas, apol, ahiemstra, mart


D29034: Add systemd user service file for kded

2020-04-21 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> plasma-kded.service.in:5
> +[Service]
> +ExecStart=@CMAKE_INSTALL_PREFIX@/bin/kded5
> +BusName=org.kde.kded5

Didn't we have a `KDE_INSTALL_BIN_DIR` or something?

REPOSITORY
  R297 KDED

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

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


D29034: Add systemd user service file for kded

2020-04-21 Thread Kai Uwe Broulik
broulik edited the summary of this revision.
broulik edited the test plan for this revision.
broulik added reviewers: Plasma, Frameworks.

REPOSITORY
  R297 KDED

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

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


D29034: Add systemd user service file for kded

2020-04-21 Thread Kai Uwe Broulik
broulik created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  Merge branch 'master' into broulik/systemd_startup
  
  Rename unit to plasma-kded
  
  Merge branch 'master' into broulik/systemd_startup

REPOSITORY
  R297 KDED

BRANCH
  broulik/systemd_startup

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

AFFECTED FILES
  src/CMakeLists.txt
  src/plasma-kded.service.in

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


D28919: Drop delayed second phase

2020-04-20 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R297:093a673f0e59: Drop delayed second phase (authored by 
broulik).

REPOSITORY
  R297 KDED

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28919?vs=80388=80621

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

AFFECTED FILES
  src/kded.cpp
  src/kded.h
  src/kdedadaptor.h

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


D28919: Drop delayed second phase

2020-04-20 Thread Kai Uwe Broulik
broulik requested review of this revision.
broulik added a comment.


  Waiting for @dfaure

REPOSITORY
  R297 KDED

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

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


D28919: Drop delayed second phase

2020-04-17 Thread Kai Uwe Broulik
broulik created this revision.
broulik added reviewers: Plasma, dfaure, davidedmundson.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  kded runs Phase 0 and Phase 1 on startup and then plasma-session Startup 
calls Phase 2 somewhere around the end of startup.
  Investigation and testing shows this has little effect on startup, if any, 
makes it slower.
  More importantly, it complicates startup and makes it harder to recreate the 
startup sequence as systemd units.

TEST PLAN
  - All services are still started
  - @davidedmundson did some bootchart that showed startup is actually 
//faster// without this
  
  Also refer to my lovely table of services we start: T12616 


REPOSITORY
  R297 KDED

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

AFFECTED FILES
  src/kded.cpp
  src/kded.h
  src/kdedadaptor.h

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


D28862: [Extractor] Remove IdleStateMonitor wrapper class

2020-04-16 Thread Kai Uwe Broulik
broulik added a comment.


  It used to be 2 minutes, now it's 1?

INLINE COMMENTS

> app.cpp:59
> +static int s_idleTimeout = 1000 * 60; // 1 min
> +m_idleTime = KIdleTime::instance();
> +m_idleTime->addIdleTimeout(s_idleTimeout);

Why the member variable? It's a singleton after all

> app.cpp:63
> +qCInfo(BALOO) << "Busy, paced indexing";
> +m_isBusy = true; });
> +connect(m_idleTime, QOverload::of(::timeoutReached), 
> this, [this] (int /*identifier*/) {

Coding style

REPOSITORY
  R293 Baloo

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

To: bruns, #baloo, ngraham
Cc: broulik, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams


D26650: Use KService to look for Filelight

2020-04-14 Thread Kai Uwe Broulik
broulik added a comment.


  Superseded by D28266 

REPOSITORY
  R241 KIO

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

To: shubham, broulik, ngraham
Cc: sitter, meven, anthonyfieroni, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns


D28770: Avoid blocking the UI thread

2020-04-13 Thread Kai Uwe Broulik
broulik added a comment.


  I don't fully understand the need for a thread here.
  It merely blocks because the API is synchronous and thus we create a nested 
eventloop.
  I don't think the writing and parsing of the cache file is a real bottleneck 
here.

INLINE COMMENTS

> currency.cpp:57
> +
> +for (const QNetworkInterface  : QNetworkInterface::allInterfaces()) {
> +if (!net.flags().testFlag(QNetworkInterface::IsUp)) {

Can probably just use `QNetworkConfigurationManager::isOnline()`?

> currency.cpp:124
> +
> +static QNetworkAccessManager m_netAccessManager;
> +

Use prefix `s_` since it's static

> currency.cpp:154
> +}
> +m_reply = 
> m_netAccessManager.get(QNetworkRequest(QUrl(QString::fromLatin1(URL;
> +connect(m_reply, ::finished, this, 
> ::onReplyFinished);

Whenever you do networking in KDE, ensure it follows redirects:
`setRedirectPolicy(QNetworkRequest::NoLessSafeRedirectPolicy)` I guess

REPOSITORY
  R292 KUnitConversion

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

To: sandsmark, ngraham, #frameworks, broulik
Cc: broulik, ngraham, kde-frameworks-devel, #frameworks, LeGast00n, cblack, 
michaelh, bruns


D28787: Fix PC3 BusyIndicator binding loop

2020-04-13 Thread Kai Uwe Broulik
broulik accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  fix-busyindicator-binding-loop (branched from master)

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

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


D28778: [FstabWatcher] Use static QStringLiteral instead of macro

2020-04-12 Thread Kai Uwe Broulik
broulik added a comment.


  Don't use global statics in library code

REPOSITORY
  R245 Solid

BRANCH
  submit

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

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


D28753: Add KNotificationJobUiDelegate(KJobUiDelegate::Flags) constructor

2020-04-11 Thread Kai Uwe Broulik
broulik accepted this revision.

REPOSITORY
  R289 KNotifications

BRANCH
  master

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

To: dfaure, broulik, davidedmundson, ervin
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D28741: [KJobUiDelegate] Add AutoHandlingEnabled flag

2020-04-11 Thread Kai Uwe Broulik
broulik added a comment.


  +1

REPOSITORY
  R244 KCoreAddons

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

To: dfaure, broulik, davidedmundson, ervin
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns


D26016: [KeySequenceHelper] Work around Meta modifier behavior

2020-04-09 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R296:4306fb8f4e19: [KeySequenceHelper] Work around Meta 
modifier behavior (authored by broulik).

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26016?vs=71596=79709

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

AFFECTED FILES
  src/qmlcontrols/kquickcontrols/private/keysequencehelper.cpp

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


D26017: [KKeySequenceWidget] Work around Meta modifier behavior

2020-04-09 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R263:577e8cddde7d: [KKeySequenceWidget] Work around Meta 
modifier behavior (authored by broulik).

REPOSITORY
  R263 KXmlGui

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26017?vs=71597=79708

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

AFFECTED FILES
  src/kkeysequencewidget.cpp

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


D28499: [LauncherJobs] Emit description

2020-04-09 Thread Kai Uwe Broulik
broulik updated this revision to Diff 79700.
broulik added a comment.


  - Drop ellipsis

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28499?vs=79105=79700

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

AFFECTED FILES
  src/gui/applicationlauncherjob.cpp
  src/gui/commandlauncherjob.cpp

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


D28531: [KNotificationJobUiDelegate] Append "Failed" for error messages

2020-04-09 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R289:524bdc640dcc: [KNotificationJobUiDelegate] Append 
Failed for error messages (authored by broulik).

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28531?vs=79190=79699

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

AFFECTED FILES
  src/knotificationjobuidelegate.cpp

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


D28673: [PackageUrlInterceptor] Make QRegularExpression static

2020-04-09 Thread Kai Uwe Broulik
broulik planned changes to this revision.
broulik added a comment.


  Yeah, can probably be a `startsWith`

REPOSITORY
  R242 Plasma Framework (Library)

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

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


D28679: [KPropertiesDialog] Disable changing dir icon on samba shares

2020-04-08 Thread Kai Uwe Broulik
broulik added a comment.


  > assuming we have a way of telling which slaves are remote
  
  `KProtocolInfo::protocolClass(url.scheme())` not being `":local"`

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, sitter
Cc: broulik, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns


D28673: [PackageUrlInterceptor] Make QRegularExpression static

2020-04-08 Thread Kai Uwe Broulik
broulik created this revision.
broulik added a reviewer: Plasma.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  Avoids recompiling it every time.

TEST PLAN
  Called 27 times on plasmashell startup for me, saves 90% of time in here

REPOSITORY
  R242 Plasma Framework (Library)

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

AFFECTED FILES
  src/plasmaquick/packageurlinterceptor.cpp

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


D27848: Remove the panel tooltip icon

2020-04-06 Thread Kai Uwe Broulik
broulik added a comment.


  > For all panel widgets
  
  which are all governed by a single place: 
`plasma-desktop/desktoppackage/contents/applet/CompactApplet.qml`
  
  Imho an unacceptable behavior break for a Frameworks component.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: ngraham, #vdg, #plasma, cblack, niccolove, apol
Cc: broulik, davidre, cblack, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns


D28570: Add 3mf thumbnail support

2020-04-04 Thread Kai Uwe Broulik
broulik accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: davidedmundson, broulik
Cc: kde-frameworks-devel, kfm-devel, nikolaik, pberestov, iasensio, fprice, 
LeGast00n, cblack, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, 
michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, mikesomov


D28555: File ioslave : use Better setting for sendfile syscall

2020-04-04 Thread Kai Uwe Broulik
broulik added a comment.


  >   reduce the overhead of processedSize dbus progress notification in each 
iteration since it is called less often, adding some more speed.
  
  From what I recall in copy job the processed size in emitted by a timer and 
not every time a chunk got processed

REPOSITORY
  R241 KIO

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

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


D28552: [CommandLauncherJob] Also mention setDesktopName in constructor

2020-04-04 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:60d7cfec0595: [CommandLauncherJob] Also mention 
setDesktopName in constructor (authored by broulik).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28552?vs=79271=79274

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

AFFECTED FILES
  src/gui/commandlauncherjob.h

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


D28552: [CommandLauncherJob] Also mention setDesktopName in constructor

2020-04-04 Thread Kai Uwe Broulik
broulik created this revision.
broulik added reviewers: Frameworks, dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  I think it's to be preferred over name + icon.

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/gui/commandlauncherjob.h

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


D28515: [CommandLauncherJob] Add constructor taking an executable and argument list

2020-04-04 Thread Kai Uwe Broulik
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:5f2be012c27d: [CommandLauncherJob] Add constructor taking 
an executable and argument list (authored by broulik).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28515?vs=79267=79270

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

AFFECTED FILES
  autotests/commandlauncherjobtest.cpp
  autotests/commandlauncherjobtest.h
  src/gui/commandlauncherjob.cpp
  src/gui/commandlauncherjob.h

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


D28515: [CommandLauncherJob] Add constructor taking an executable and argument list

2020-04-04 Thread Kai Uwe Broulik
broulik requested review of this revision.

REPOSITORY
  R241 KIO

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

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


D28515: [CommandLauncherJob] Add constructor taking an executable and argument list

2020-04-04 Thread Kai Uwe Broulik
broulik updated this revision to Diff 79267.
broulik added a comment.
This revision is now accepted and ready to land.


  - Add test for binary in path with space

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28515?vs=79189=79267

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

AFFECTED FILES
  autotests/commandlauncherjobtest.cpp
  autotests/commandlauncherjobtest.h
  src/gui/commandlauncherjob.cpp
  src/gui/commandlauncherjob.h

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


D28515: [CommandLauncherJob] Add constructor taking an executable and argument list

2020-04-04 Thread Kai Uwe Broulik
broulik planned changes to this revision.
broulik added a comment.


  Good idea

REPOSITORY
  R241 KIO

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

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


D28531: [KNotificationJobUiDelegate] Append "Failed" for error messages

2020-04-03 Thread Kai Uwe Broulik
broulik retitled this revision from "[KNotificationJobUiDelegate] Prepend 
"Failed" for error messages" to "[KNotificationJobUiDelegate] Append "Failed" 
for error messages".

REPOSITORY
  R289 KNotifications

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

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


D28499: [LauncherJobs] Emit description

2020-04-03 Thread Kai Uwe Broulik
broulik added a comment.


  Given we don't add elippsis after any other KIO Job (e.g. "Copying") probably 
shouldn't do it here either?

REPOSITORY
  R241 KIO

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

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


D28531: [KNotificationJobUiDelegate] Prepend "Failed" for error messages

2020-04-03 Thread Kai Uwe Broulik
broulik created this revision.
broulik added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
broulik requested review of this revision.

REVISION SUMMARY
  Makes the resulting error notification resemble one like a KJobTracker 
failure as if I had copied a file.

TEST PLAN
  - Failed to launch an app, got "Launching foo... (Failed)"

REPOSITORY
  R289 KNotifications

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

AFFECTED FILES
  src/knotificationjobuidelegate.cpp

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


D28515: [CommandLauncherJob] Add constructor taking an executable and argument list

2020-04-03 Thread Kai Uwe Broulik
broulik added a comment.


  Not sure how I would test running a command with a space in it.. putting a 
shell script in running it through QFINDTESTDATA? Not sure how to make that 
sort of stuff work on Windows

REPOSITORY
  R241 KIO

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

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


D28515: [CommandLauncherJob] Add constructor taking an executable and argument list

2020-04-03 Thread Kai Uwe Broulik
broulik updated this revision to Diff 79189.
broulik edited the test plan for this revision.
broulik added a comment.


  - Quote executable in commandline
  - Add unittest

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28515?vs=79136=79189

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

AFFECTED FILES
  autotests/commandlauncherjobtest.cpp
  autotests/commandlauncherjobtest.h
  src/gui/commandlauncherjob.cpp
  src/gui/commandlauncherjob.h

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


  1   2   3   4   5   6   7   8   9   10   >