D26564: Clean kbuildsycoca4 support

[Eike Hein]

hein added a comment.


  Let's wait until branching, there's still some KDE 4 apps people use.

REPOSITORY
  R241 KIO

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

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

D23036: Fix incorrect initial tabs position for vertical tab bars

[Eike Hein]

hein accepted this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: lisin, #plasma, ngraham, hein
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22960: Add handle icons

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:29dffaf4cd72: Add handle icons (authored by hein).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22960?vs=63155=63327

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

AFFECTED FILES
  KF5Kirigami2Macros.cmake

To: hein, #frameworks, apol, mart
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22958: Add initial Android support to ecm_add_app_icon

[Eike Hein]

hein abandoned this revision.
hein added a comment.


  Marco explained privately he doesn't want this so I'll move it to my codebase 
for now until the alternative better thing appears.

REPOSITORY
  R240 Extra CMake Modules

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

To: hein, #frameworks, mart, leinir, apol
Cc: nicolasfella, kde-frameworks-devel, kde-buildsystem, LeGast00n, sbergeron, 
bencreasy, michaelh, ngraham, bruns

D22958: Add initial Android support to ecm_add_app_icon

[Eike Hein]

hein added a comment.


  In D22958#507299 , @nicolasfella 
wrote:
  
  > Why do you restrict it to SVGs? Wouldn't you be able to use PNGs as well?
  
  
  Mostly because I don't know what assumptions Kirigami makes. Its own macro is 
hardcoded to only work with Breeze, which is implicitly SVG, so I didn't want 
to throw anything else at it in its special folder.

REPOSITORY
  R240 Extra CMake Modules

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

To: hein, #frameworks, mart, leinir, apol
Cc: nicolasfella, kde-frameworks-devel, kde-buildsystem, LeGast00n, sbergeron, 
bencreasy, michaelh, ngraham, bruns

D22960: Add handle icons

[Eike Hein]

hein created this revision.
hein added reviewers: Frameworks, apol, mart.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
hein requested review of this revision.

REPOSITORY
  R169 Kirigami

BRANCH
  master

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

AFFECTED FILES
  KF5Kirigami2Macros.cmake

To: hein, #frameworks, apol, mart
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22958: Add initial Android support to ecm_add_app_icon

[Eike Hein]

hein added inline comments.

INLINE COMMENTS

> apol wrote in ECMAddAppIcon.cmake:154
> this if could go up to where we're checking the icon name in an elseif up at 
> line 146.

We'd need to do a `get_filename_component` there and end up either pulling the 
`ext` thing out twice (unless CMake doesn't do block scoping) or ahead of the 
`ABSOLUTE` one and then possibly unnecessarily. FWIW I copied that entire regex 
block out of ECMInstallIcons.cmake to play it safe so I'd prefer not touching 
it ... :-)

REPOSITORY
  R240 Extra CMake Modules

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

To: hein, #frameworks, mart, leinir, apol
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, sbergeron, bencreasy, 
michaelh, ngraham, bruns

D22955: Add icon used by Kirigami.AboutPage

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:c17c048ec2f7: Add icon used by Kirigami.AboutPage 
(authored by hein).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22955?vs=63146=63152

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

AFFECTED FILES
  KF5Kirigami2Macros.cmake

To: hein, #frameworks, mart, apol
Cc: mart, plasma-devel, fbampaloukas, domson, dkardarakos, apol, 
davidedmundson, hein

D22958: Add initial Android support to ecm_add_app_icon

[Eike Hein]

hein added a comment.


  This is the culmination of a patch series that makes About dialogs in 
Kirigami work reasonably across platforms:
  
  F7168840: about.jpg 

REPOSITORY
  R240 Extra CMake Modules

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

To: hein, #frameworks, mart, leinir, apol
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, sbergeron, bencreasy, 
michaelh, ngraham, bruns

D22958: Add initial Android support to ecm_add_app_icon

[Eike Hein]

hein added a comment.


  The code is mostly copy-and-pasted together from ECMAddAppIcon.cmake and 
ECMInstallIcons.cmake.

REPOSITORY
  R240 Extra CMake Modules

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

To: hein, #frameworks, mart, leinir, apol
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, sbergeron, bencreasy, 
michaelh, ngraham, bruns

D22958: Add initial Android support to ecm_add_app_icon

[Eike Hein]

hein created this revision.
hein added reviewers: Frameworks, mart, leinir, apol.
Herald added projects: Frameworks, Build System.
Herald added subscribers: kde-buildsystem, kde-frameworks-devel.
hein requested review of this revision.

REVISION SUMMARY
  Kirigami has a `kirigami_package_breeze_icons` CMake macro that
  KDE Frameworks apps targeting Android use to extract icons from
  the Breeze theme into a folder that will get picked up by our
  SDK for inclusion in the eventual .apk binary package.
  
  Applications that bundle their own application icon that is not
  included in the Breeze theme do not benefit from this, as that
  macro relies on making a clone of breeze-icons.git to work from,
  and as those apps install their icons into the "hicolor" theme
  instead anyway, as is spec-conformant.
  
  This e.g. means that the application icon won't show on the
  Kirigami.AboutPage Qt Quick component, which is used to display
  the About dialogs in Kirigami apps.
  
  This patch makes ecm_app_icon put app icons into the same
  folder, if the constraints of being on Android and being a SVG
  icon are satisfied, causing the icon to be picked up into the
  .apk.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  modules/ECMAddAppIcon.cmake

To: hein, #frameworks, mart, leinir, apol
Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, sbergeron, bencreasy, 
michaelh, ngraham, bruns

D22955: Add icon used by Kirigami.AboutPage

[Eike Hein]

hein created this revision.
hein added reviewers: Frameworks, mart.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
hein requested review of this revision.

REPOSITORY
  R169 Kirigami

BRANCH
  master

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

AFFECTED FILES
  KF5Kirigami2Macros.cmake

To: hein, #frameworks, mart
Cc: mart, plasma-devel, fbampaloukas, domson, dkardarakos, apol, 
davidedmundson, hein

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein added a comment.


  I'll remove that part of the patch later tonight and talk Marco into fixing 
it differently.

REPOSITORY
  R169 Kirigami

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

To: hein, #frameworks, #vdg, mart, apol, ngraham, leinir, nicolasfella
Cc: nicolasfella, ngraham, plasma-devel, fbampaloukas, domson, dkardarakos, 
apol, davidedmundson, mart, hein

D22867: Consistently use Units.toolTipDelay instead of hardcoded values

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:a99c14d1eac9: Consistently use Units.toolTipDelay instead 
of hardcoded values (authored by hein).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22867?vs=62879=62912

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

AFFECTED FILES
  src/controls/private/GlobalDrawerActionItem.qml
  src/controls/private/PrivateActionToolButton.qml
  src/controls/templates/SwipeListItem.qml
  src/controls/templates/private/BackButton.qml
  src/controls/templates/private/ForwardButton.qml

To: hein, #frameworks, mart
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein updated this revision to Diff 62883.
hein added a comment.


  Don't hit Kirigami.Label (deprecated)

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22868?vs=62880=62883

BRANCH
  aboutpage

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

AFFECTED FILES
  src/controls/AboutPage.qml
  src/controls/Page.qml

To: hein, #frameworks, #vdg, mart, apol, ngraham, leinir
Cc: ngraham, plasma-devel, fbampaloukas, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein added a reviewer: leinir.

REPOSITORY
  R169 Kirigami

BRANCH
  aboutpage

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

To: hein, #frameworks, #vdg, mart, apol, ngraham, leinir
Cc: ngraham, plasma-devel, fbampaloukas, domson, dkardarakos, apol, 
davidedmundson, mart, hein

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein added a comment.


  Discover:
  
  F7130049: Screenshot_20190801_100601.png 

  
  Material style:
  
  F7130053: Screenshot_20190801_100645.png 


REPOSITORY
  R169 Kirigami

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

To: hein, #frameworks, #vdg, mart, apol
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein added a comment.


  Before:
  
  F7130036: Screenshot_20190801_15.png 

  
  After:
  
  F7130037: Screenshot_20190801_100033.png 


REPOSITORY
  R169 Kirigami

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

To: hein, #frameworks, #vdg, mart, apol
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22868: Revamp Kirigami.AboutPage

[Eike Hein]

hein created this revision.
hein added reviewers: Frameworks, VDG, mart, apol.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
hein requested review of this revision.

REVISION SUMMARY
  - Use a FormLayout to work nicely in both portrait and landscape orientations
  - Don't use an AbstractCard for the person delegate, it's very jarring in 
this design
  - Fix app description not having word wrap (breaks on portrait)
  - Improve inter-section spacing
  - Fix empty author email addresses showing as "Name <>"
  - Fix scrollbar length if a `Kirigami.Page` doesn't set any actions (and 
don't load a gigantic subtree of unnecessary objects into the page)
  - If an app uses `Kirigami.AbstractApplicationWindow` and doesn't have a 
`pageStack`, don't have the license delegate be a `LinkButton` that operates on 
one and causes an error
  - Add a fallback to KAboutData::componentName for the program icon - 
`KAboutData::programIconName` is deprecated, there's no QML accessor for its 
replacement `QGuiApplication::setWindowIcon` yet, but 
`KAboutData::programIconName` falls back to `componentName` already (it fails 
because `KAboutData::programIconName` isn't a qprop, but I don't feel like 
adding it to KCoreAddons for a deprecated thing while `componentName` is not 
deprecated)

REPOSITORY
  R169 Kirigami

BRANCH
  aboutpage

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

AFFECTED FILES
  src/controls/AboutPage.qml
  src/controls/Page.qml

To: hein, #frameworks, #vdg, mart, apol
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D22867: Consistently use Units.toolTipDelay instead of hardcoded values

[Eike Hein]

hein created this revision.
hein added reviewers: Frameworks, mart.
Herald added a project: Kirigami.
hein requested review of this revision.

REPOSITORY
  R169 Kirigami

BRANCH
  master

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

AFFECTED FILES
  src/controls/private/GlobalDrawerActionItem.qml
  src/controls/private/PrivateActionToolButton.qml
  src/controls/templates/SwipeListItem.qml
  src/controls/templates/private/BackButton.qml
  src/controls/templates/private/ForwardButton.qml

To: hein, #frameworks, mart
Cc: plasma-devel, fbampaloukas, domson, dkardarakos, apol, davidedmundson, 
mart, hein

D21169: Add app icon for the Kirogi Drone control app

[Eike Hein]

hein added a comment.


  I'd say it's up to the Breeze maintainers. From my end I'm very interested in 
the app having a Breeze-compatible icon, because I'd like it to integrate well 
with KDE's workspaces and other apps. Review by the Breeze community is very 
appreciated to help with that. Application code needs to ship the icon anyway 
though for systems that don't have the theme and can't be reasonably expected 
to provide every app icon, so it's not vital to be included with the theme.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, hein, #vdg
Cc: ngraham, apol, kde-frameworks-devel, hein, michaelh, bruns

D21169: Add app icon for the Kirogi Drone control app

[Eike Hein]

hein added a comment.


  @trickyricky26 It's great! What do you think about adding the detailed SVG as 
256 version?

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, hein, #vdg
Cc: apol, kde-frameworks-devel, hein, michaelh, ngraham, bruns

D20428: separe mobile text selection to avoid recursive imports

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  Works here now, thx!

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

To: mart, #plasma, hein
Cc: hein, kde-frameworks-devel, michaelh, ngraham, bruns

D18986: [breeze-icons] Add video camera icons

[Eike Hein]

hein accepted this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  arcpatch-D18986

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

To: trickyricky26, #vdg, hein, ndavis, rooty, ngraham
Cc: filipf, trickyricky26, ngraham, kde-frameworks-devel, michaelh, bruns

D18986: [breeze-icons] Add video camera icons

[Eike Hein]

hein added a comment.


  Would it be possible to also add the lineart versions as -symbolic? In my new 
app I rely on Kirigami's icon recoloring feature and it only works with 
monochrome icons, so I rely on -symbolic to request them.

REPOSITORY
  R266 Breeze Icons

BRANCH
  arcpatch-D18986

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

To: trickyricky26, #vdg, hein, ndavis, rooty, ngraham
Cc: filipf, trickyricky26, ngraham, kde-frameworks-devel, michaelh, bruns

D19030: Add 16 px and 22 px versions of the gamepad icon to devices/

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R266 Breeze Icons

BRANCH
  gamepad-icon-16-22-px (branched from master)

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

To: rooty, hein, #vdg
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D19030: Add 16 px and 22 px versions of the gamepad icon to devices/

[Eike Hein]

hein added a comment.


  LGTM, thanks!

REPOSITORY
  R266 Breeze Icons

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

To: rooty, hein, #vdg
Cc: ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D19030: Add 16 px and 22 px versions of the gamepad icon to devices/

[Eike Hein]

hein added a comment.


  Could this maybe be input-games-symbolic (too)? Because I want a monochrome 
version always (my app recolors icons using the Kirigami feature, and that only 
works with monochrome icons).

REPOSITORY
  R266 Breeze Icons

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

To: rooty, hein, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D18901: Add device icons for drones

[Eike Hein]

hein added a comment.


  Thank you! Just what I needed.

REPOSITORY
  R266 Breeze Icons

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

To: trickyricky26, #vdg, hein, ngraham, rooty
Cc: ngraham, hein, kde-frameworks-devel, michaelh, bruns

D17691: Add rows info to the plasma virtual desktop protocol

[Eike Hein]

hein added a task: T4457: [kwayland] Virtual Desktop protocol.

REPOSITORY
  R127 KWayland

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

To: mart, #plasma, #kwin
Cc: zzag, davidedmundson, hein, kde-frameworks-devel, michaelh, ngraham, bruns

D18187: [KNewFileMenu] Don't emit fileCreated when creating a directory

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: chinmoyr, ngraham, hein
Cc: kde-frameworks-devel, dfaure, michaelh, ngraham, bruns

D17691: Add rows info to the plasma virtual desktop protocol

[Eike Hein]

hein added a comment.


  > The desktop cube shows everything flat, desktop grid is configurable (and 
hardcoded).
  
  The desktop grid effect uses the rows setting from the KCM for me.
  
  I don't think it's easy for users to understand that the KCM is configuring 
KWin only, instead of all of Plasma. Especially because the Pager can't escape 
KWin's spatial notions anyway: If you use the Pager to switch desktops, by 
default KWin does a slide animation that's directional based on the rows 
setting. If the Pager's layout differs from KWin's, the animation played won't 
fit what's visible on the Pager. Considering the Pager is all about providing a 
spatial overview into the system, this seems weird.
  
  Plus this protocol is meant to be a private one for use between KWin and 
plasmashell. I think it's OK for them to integrate tightly, it's not the same 
as exposing layout information to regular clients. I'd argue the entire 
PlasmaWindowManagement protocol already does so after all.

REPOSITORY
  R127 KWayland

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

To: mart, #plasma, #kwin
Cc: davidedmundson, hein, kde-frameworks-devel, michaelh, ngraham, bruns

D17691: Add rows info to the plasma virtual desktop protocol

[Eike Hein]

hein added a comment.


  As per this , progress on this is 
currently blocking wrapping up Wayland virtual desktops support for 5.15.

REPOSITORY
  R127 KWayland

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

To: mart, #plasma, #kwin
Cc: hein, kde-frameworks-devel, michaelh, ngraham, bruns

D16643: Correct the accept flag of the event object on DragMove

[Eike Hein]

hein added a comment.


  @mart ping

REPOSITORY
  R296 KDeclarative

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

To: trmdi, mart, broulik, #plasma, hein, davidedmundson
Cc: anthonyfieroni, bruns, dkorth, ngraham, kde-frameworks-devel, michaelh

D15970: Properly look for fallback themes

[Eike Hein]

hein accepted this revision.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

To: apol, #plasma, davidedmundson, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15074: [server] Do not try to create data offers without source

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R127 KWayland

BRANCH
  offerWithSourceOnly

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

To: romangg, #kwin, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Eike Hein]

hein added a task: T4457: [kwayland] Virtual Desktop protocol.

REPOSITORY
  R127 KWayland

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

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Eike Hein]

hein added a comment.


  I have a problem: When I restart plasmashell, I don't get any desktops again. 
I think the code may not be re-sending the data when the client goes away and 
comes a second time.

REPOSITORY
  R127 KWayland

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

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D14042: Support special URLs for finding files of a certain type

[Eike Hein]

hein added a comment.


  Does it make sense to name the queries after XDG_USER_DIRS, so e.g. pictures 
instead of images?
  
  https://www.freedesktop.org/wiki/Software/xdg-user-dirs/

REPOSITORY
  R293 Baloo

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

To: broulik, #frameworks, bruns, dfaure, renatoo, franckarrecot, hein, ngraham
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D14043: [KFilePlacesModel] Support pretty baloosearch URLs

[Eike Hein]

hein accepted this revision.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, bruns, dfaure, renatoo, franckarrecot, ngraham, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13866: Fix ResultModel item moving

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  It works for me.

REPOSITORY
  R159 KActivities Statistics

BRANCH
  master

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

To: davidedmundson, ivan, hein
Cc: hein, kde-frameworks-devel, michaelh, ngraham, bruns

D12820: Add KWayland virtual desktop protocol

[Eike Hein]

hein added a dependent revision: D13745: Implement support for virtual desktops 
on Wayland.

REPOSITORY
  R127 KWayland

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

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns

D13275: Teach ContainmentInterface::processMimeData how to handle Task Manager drops

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:e108d7e90e36: Teach ContainmentInterface::processMimeData 
how to handle Task Manager drops (authored by hein).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13275?vs=35463=35656

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp

To: hein, davidedmundson, Zren, mart
Cc: ngraham, zzag, kde-frameworks-devel, plasma-devel, michaelh, bruns

D13275: Teach ContainmentInterface::processMimeData how to handle Task Manager drops

[Eike Hein]

hein created this revision.
hein added reviewers: davidedmundson, Zren, mart.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
hein requested review of this revision.

REVISION SUMMARY
  To explicitly confine Task Manager drops to Plasma and avoid accidental
  drops on other apps, D13274  makes it 
store task launcher URLs in a special
  internal MIME type instead of the generic text/url one. This change to
  processMimeData() adds the necessary conversion back.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp

To: hein, davidedmundson, Zren, mart
Cc: kde-frameworks-devel, plasma-devel, michaelh, ngraham, bruns

D13275: Teach ContainmentInterface::processMimeData how to handle Task Manager drops

[Eike Hein]

hein updated this revision to Diff 35463.
hein added a comment.


  Fix last minute build error.

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13275?vs=35371=35463

BRANCH
  master

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp

To: hein, davidedmundson, Zren, mart
Cc: ngraham, zzag, kde-frameworks-devel, plasma-devel, michaelh, bruns

D13223: Set kcm context property on the correct context

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R296 KDeclarative

BRANCH
  master

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

To: davidedmundson, #plasma, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13137: Updated handles and overflow-menu-*

[Eike Hein]

hein accepted this revision.

REPOSITORY
  R266 Breeze Icons

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

To: alex-l, mart, andreask, andreaska, hein
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D12392: Fix the "Default" color scheme to match Breeze again

[Eike Hein]

hein accepted this revision.
hein added a comment.
This revision is now accepted and ready to land.


  This looks good, but I think there might be additional copies of these values 
in (a) the color KCM code and (b) the kdeglobals code in kdelibs4support, did 
you have a chance to look into those?

REPOSITORY
  R265 KConfigWidgets

BRANCH
  fix-default-color-scheme (branched from master)

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

To: ngraham, #breeze, #plasma, hein
Cc: kde-frameworks-devel, cfeck, abetts, michaelh, ngraham, bruns

D12508: Make KMessageWidget match Kirigami inlineMessage's style

[Eike Hein]

hein accepted this revision.
hein added a comment.


  Looks good to me, thanks for working on this.

REPOSITORY
  R236 KWidgetsAddons

BRANCH
  arcpatch-D12508

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

To: ngraham, hein, #plasma, #vdg, cfeck
Cc: broulik, anemeth, abetts, cfeck, mart, fabianr, elvisangelaccio, jnoack, 
#frameworks, michaelh, ngraham, bruns

D12512: ScrollViewKCM

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R296 KDeclarative

BRANCH
  mart/ScrollViewKCM

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

To: mart, #plasma, hein
Cc: hein, #frameworks, michaelh, bruns

D12512: ScrollViewKCM

[Eike Hein]

hein added a comment.


  It looks good otherwise and I really want to accept this so I can progress on 
the Language KCM, but this magic value thing gives me a headache. It's going to 
break with other QStyles with a different value. We need some sort of fix for 
that ...

REPOSITORY
  R296 KDeclarative

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

To: mart, #plasma
Cc: hein, #frameworks, michaelh, bruns

D12512: ScrollViewKCM

[Eike Hein]

hein added a comment.


  Does the magic pixel size number for the padding get scaled correctly on 
hidpi?
  
  And where does System Settings have the magic pixel number from - is it 
hardcoded there as well or is it a QStyle thing that could change with the 
style engine?
  
  Could System Settings inject the magic number as a context prop?

REPOSITORY
  R296 KDeclarative

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

To: mart, #plasma
Cc: hein, #frameworks, michaelh, bruns

D12215: Add a "Reload" menu item to KDirOperator's context menu

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: ngraham, #frameworks, hein
Cc: maximilianocuria, dhaumann, rkflx, michaelh, bruns

D12097: Make room for footer and header.

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R296:c6c41b4a97cc: Make room for footer and header. (authored 
by hein).

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12097?vs=31836=32594

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

AFFECTED FILES
  src/qmlcontrols/kcmcontrols/qml/SimpleKCM.qml

To: hein, #kirigami, mart
Cc: plasma-devel, #frameworks, michaelh, bruns, apol, davidedmundson, mart, hein

D12034: Use the more appropriate "two sliders" icon for "configure"

[Eike Hein]

hein added a comment.


  In D12034#244019 , @hein wrote:
  
  > In principle I totally agree with this, but I'm not sure about the 
`input-keyboard` icon it extends to. Do you know where it's used currently?
  
  
  It's sometimes even used as window icon: 
https://lxr.kde.org/search?_filestring=&_string=input-keyboard

REPOSITORY
  R266 Breeze Icons

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

To: ngraham, #vdg, #plasma, #breeze
Cc: hein, bruns, abetts, sharvey, #frameworks, michaelh, ngraham

D12034: Use the more appropriate "two sliders" icon for "configure"

[Eike Hein]

hein added a comment.


  In principle I totally agree with this, but I'm not sure about the 
`input-keyboard` icon it extends to. Do you know where it's used currently?

REPOSITORY
  R266 Breeze Icons

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

To: ngraham, #vdg, #plasma, #breeze
Cc: hein, bruns, abetts, sharvey, #frameworks, michaelh, ngraham

D12097: Make room for footer and header.

[Eike Hein]

hein added a dependent revision: D12102: Port Language KCM to Qt Quick.

REPOSITORY
  R296 KDeclarative

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

To: hein, #kirigami, mart
Cc: plasma-devel, #frameworks, michaelh, ngraham, bruns, apol, davidedmundson, 
mart, hein

D12097: Make room for footer and header.

[Eike Hein]

hein created this revision.
hein added reviewers: Kirigami, mart.
Restricted Application added projects: Kirigami, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.
hein requested review of this revision.

REPOSITORY
  R296 KDeclarative

BRANCH
  master

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

AFFECTED FILES
  src/qmlcontrols/kcmcontrols/qml/SimpleKCM.qml

To: hein, #kirigami, mart
Cc: plasma-devel, #frameworks, michaelh, ngraham, bruns, apol, davidedmundson, 
mart, hein

D11960: Support nvidia reset notifications in QtQuickViews

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R296 KDeclarative

BRANCH
  master

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

To: davidedmundson, #plasma, hein
Cc: #frameworks, michaelh, ngraham

D6313: Support Icon Scale from Icon naming specification 0.13

[Eike Hein]

hein added a comment.


  I also prefer the "Before" images.

REPOSITORY
  R302 KIconThemes

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

To: broulik, kde-frameworks-devel, #plasma, #vdg
Cc: hein, rkflx, acrouthamel, ngraham, elvisangelaccio, mart, kvermette, cfeck, 
davidedmundson, plasma-devel, #frameworks, ragreen, michaelh, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D11552: [WIP] Handle CJK characters

[Eike Hein]

hein added a comment.


  For the record though - a better way to do this is to use QTextBoundaryFinder 
which will operate e.g. on grapheme cluster boundaries. This still isn't super 
great for Chinese though. If you want to really-properly do it you'll end up 
depending on ICU and using its BreakIterator combined with dict-based support 
for Chinese, which isn't terribly fast however.

REPOSITORY
  R293 Baloo

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

To: michaelh, hein
Cc: bruns, lbeltrame, #frameworks, alexeymin, cfeck, ashaposhnikov, michaelh, 
astippich, spoorun, nicolasfella, ngraham

D11552: [WIP] Handle CJK characters

[Eike Hein]

hein added a comment.


  If you're going to loop over a QString and break it down into QChars anyway, 
why don't you just use QChar::script?

REPOSITORY
  R293 Baloo

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

To: michaelh, hein
Cc: bruns, lbeltrame, #frameworks, alexeymin, cfeck, ashaposhnikov, michaelh, 
astippich, spoorun, nicolasfella, ngraham

D10670: Reduce plasmashell frozen time

[Eike Hein]

hein added a comment.


  I think it's because mwolff did "Requesting changes" and he didn't accept.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: jtamate, #frameworks, #plasma, mwolff, davidedmundson
Cc: hein, mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, ngraham, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10670: Reduce plasmashell frozen time

[Eike Hein]

hein added a comment.


  > This revision was not accepted when it landed; it landed in state Needs 
Review.
  
  Weird, David accepted it though?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: jtamate, #frameworks, #plasma, mwolff, davidedmundson
Cc: hein, mwolff, plasma-devel, davidedmundson, michaelh, ZrenBot, ngraham, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D11510: [KPropertiesDialog] Support Checksum tab for any local path

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, dfaure, elvisangelaccio, hein
Cc: michaelh, ngraham

D11278: [KateCompletionWidget] Create configuration interface on demand

[Eike Hein]

hein added a comment.


  A backtrace for the crash @martinkostolny mentioned can be found here: 
https://bugs.kde.org/show_bug.cgi?id=391955

REPOSITORY
  R39 KTextEditor

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

To: broulik, #ktexteditor, dhaumann, kfunk, cullmann
Cc: hein, martinkostolny, cullmann, kfunk, #frameworks, michaelh, kevinapavew, 
ngraham, demsking, sars, dhaumann

D11107: [KProtocolInfoFactory] Don't clear cache if it had just been built

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:a9625a3be6b1: [KProtocolInfoFactory] Dont clear 
cache if it had just been built (authored by hein).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11107?vs=28916=29202

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

AFFECTED FILES
  src/core/kprotocolinfofactory.cpp
  src/core/kprotocolinfofactory_p.h

To: hein, #frameworks, broulik, dfaure
Cc: michaelh

D11107: [KProtocolInfoFactory] Don't clear cache if it had just been built

[Eike Hein]

hein added a reviewer: dfaure.

REPOSITORY
  R241 KIO

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

To: hein, #frameworks, broulik, dfaure
Cc: michaelh

D11153: Use correct item URL for Create New context menu

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:032709c17f00: Use correct item URL for Create New context 
menu (authored by prukgaber, committed by hein).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11153?vs=29006=29163

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

AFFECTED FILES
  src/filewidgets/kdiroperator.cpp

To: prukgaber, #frameworks, dfaure
Cc: hein, #frameworks, michaelh

D11153: Use correct item URL for Create New context menu

[Eike Hein]

hein added reviewers: Frameworks, dfaure.
hein added a comment.


  Looks good to me. I'll let David have a look too and then it can go in.

REPOSITORY
  R241 KIO

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

To: prukgaber, #frameworks, dfaure
Cc: hein, #frameworks, michaelh

D11107: [KProtocolInfoFactory] Don't clear cache if it had just been built

[Eike Hein]

hein updated this revision to Diff 28916.
hein added a comment.


  Slightly cleaner approach.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11107?vs=28885=28916

BRANCH
  master

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

AFFECTED FILES
  src/core/kprotocolinfofactory.cpp
  src/core/kprotocolinfofactory_p.h

To: hein, #frameworks, broulik
Cc: michaelh

D11107: [KProtocolInfoFactory] Don't clear cache if it had just been built

[Eike Hein]

hein commandeered this revision.
hein edited reviewers, added: broulik; removed: hein.
hein added a comment.


  Commandeering as agreed upon with Kai, new rev in a moment.

REPOSITORY
  R241 KIO

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

To: hein, #frameworks, broulik
Cc: michaelh

D11107: [KProtocolInfoFactory] Don't clear cache if it had just been built

[Eike Hein]

hein added a comment.


  Good idea, but every time one of those state tracking bools get copied my 
mind screams "BAD PATTERN". It's not the fault of your patch but I don't like 
how this cache handling is written - it'd be cleaner if fillCache had a return 
value for "I actually did work" and if the meaning of m_allProtocolsLoaded got 
inverted to being a dirty bit, so fillCache() itself can take care of the cache 
eviction code (qDeleteAll etc.) if dirty is set. Doing this instead creates a 
"now every time you want to change things you have to read site first and copy 
some boiler plate" situation.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, hein
Cc: michaelh

D10870: [PackageUrlInterceptor] Don't rewrite "inline"

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: broulik, #plasma, hein
Cc: #frameworks, michaelh

D10863: [KRecentDocument] Use QGuiApplication::desktopFileName()

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, #plasma, hein, aleksejshilin
Cc: michaelh

D10691: [kactivities-stats] Fix plasmashell crash when database is broken

[Eike Hein]

hein added a comment.


  @ivan So you wrote a method that can return null, but the call sites aren't 
actually prepared to handle that gracefully? :) But I was waiting for your 
review for that context.

REPOSITORY
  R159 KActivities Statistics

BRANCH
  master

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

To: kpiwowarski, #plasma, #frameworks, hein
Cc: ivan, plasma-devel, #frameworks, michaelh, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10691: [kactivities-stats] Fix plasmashell crash when database is broken

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R159 KActivities Statistics

BRANCH
  master

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

To: kpiwowarski, #plasma, #frameworks, hein
Cc: plasma-devel, #frameworks, michaelh, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10684: [Drop Job] Create proper trash file on linking

[Eike Hein]

hein accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

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

To: broulik, #frameworks, #plasma, hein
Cc: plasma-devel, michaelh, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D10405: Stuff the "Couldn't find executable" message box into a queued lambda

[Eike Hein]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:e3a80661c9cd: Stuff the Couldnt find 
executable message box into a queued lambda (authored by hein).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D10405?vs=27048=27285#toc

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10405?vs=27048=27285

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

AFFECTED FILES
  src/widgets/krun.cpp

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Stuff the "Couldn't find executable" message box into a queued lambda

[Eike Hein]

hein added a comment.


  Thanks!

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Stuff the "Couldn't find executable" message box into a queued lambda

[Eike Hein]

hein added a comment.


  ping

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Stuff the "Couldn't find executable" message box into a queued lambda

[Eike Hein]

hein added a comment.


  (Side note: I like the idea of a KRun job - something like that could even be 
added without needing a KIO 6.)

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Stuff the "Couldn't find executable" message box into a queued lambda

[Eike Hein]

hein updated this revision to Diff 27048.
hein added a comment.


  - Fall back to qWarning if !qGuiApp
  - Use QTimer::singleShot to work on older Qt

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10405?vs=27046=27048

BRANCH
  master

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

AFFECTED FILES
  src/widgets/krun.cpp

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Stuff the "Couldn't find executable" message box into a queued lambda

[Eike Hein]

hein added a comment.


  There's a problem with my patch :(. QMetaObject::invokeMethod(context, 
functor) is new in Qt 5.10. I don't think we can depend on that yet, right?
  
  Ah, but QTimer::singleShot added an overload w/ context in 5.4 ..

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Stuff the "Couldn't find executable" message box into a queued lambda

[Eike Hein]

hein retitled this revision from "Don't proceed in runCommandInternal if the 
executable doesn't exit" to "Stuff the "Couldn't find executable" message box 
into a queued lambda".
hein edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Don't proceed in runCommandInternal if the executable doesn't exit

[Eike Hein]

hein updated this revision to Diff 27046.
hein added a comment.


  New approach.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10405?vs=26821=27046

BRANCH
  master

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

AFFECTED FILES
  src/widgets/krun.cpp

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Don't proceed in runCommandInternal if the executable doesn't exit

[Eike Hein]

hein added a comment.


  It's totally conceivable for e.g. a KPart or a Plasma plugin to open a 
QWidget-based window where the KMessageBox is appropriate, and if there's only 
a global interface instance and Plasma overrides it to hide all message boxes 
it's going to break KIO users in plugins. So I don't think going to a single 
global is good enough, it would have to be able to set more narrowly. This also 
bubbles up actually - the KRun calls we're talking about are in libtaskmanager 
which is technically meant to be UI-agnostic, so it'd have to also expose some 
way to set the interface instance. I agree all of this is in theory good (we 
need to make KIO more toolkit-agnostic - I have another giant patch sitting 
around that's almost-unfinishable that adds QWindow support to some APIs that 
currently only accept QWidgets I don't even have), but I think it's KIO 6 
material and not the quick fix I'm looking for.
  
  I found a cludge that's ugly but works to fix the crash - I'll update this in 
a moment to see how much you hate it. :)

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Don't proceed in runCommandInternal if the executable doesn't exit

[Eike Hein]

hein added a comment.


  There's a Job::setUiDelegateExtension though in addition to 
KIO::setDefaultJobUiDelegateExtension. A global would wreak havoc with stuff 
like plugins, no?

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Don't proceed in runCommandInternal if the executable doesn't exit

[Eike Hein]

hein added a comment.


  Thanks for the solution for sure, but - it requires writing a million lines 
of boilerplate, extensively refactoring KRun and adding reams of new overloads 
to its API (there would need to be some way to pass an instance of that 
interface to all these - not internal - statics). It's more than I have 
time/energy for currently and I'm not really sure it's viable without a KIO6. I 
have another idea, though, let me see if it's viable ...

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10420: Make sure the FrameSvg and Svg instances have the right devicePixelRatio

[Eike Hein]

hein 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/D10420

To: apol, #plasma, hein
Cc: hein, plasma-devel, #frameworks, michaelh, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10405: Don't proceed in runCommandInternal if the executable doesn't exit

[Eike Hein]

hein added a comment.


  /o\ I don't like life anymore

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Don't proceed in runCommandInternal if the executable doesn't exit

[Eike Hein]

hein added a comment.


  @dfaure I don't see how the virtual thing is going to work - there's numerous 
static affected codepaths so there's no 'this' to pass down to call the 
non-static error handler on.
  
  I could add a new "silent" RunFlag and pass that around to quiet the message 
box altogether, but this would mean zero error handling. I'm unhappy about that.

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10405: Don't proceed in runCommandInternal if the executable doesn't exit

[Eike Hein]

hein added a comment.


  Yeah, I had a feeling there were other design considerations here, but 
they're not documented/commented in the code so the only way to find out was to 
show you a patch :)
  
  You want this virtual method where? In KRun? And then Plasma should subclass 
KRun?

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart, ngraham
Cc: #frameworks, michaelh

D10420: Make sure the FrameSvg and Svg instances have the right devicePixelRatio

[Eike Hein]

hein added a comment.


  Sanity check question: Did you make sure this doesn't introduce an extra 
render when things start up? (Initial vs. change.)
  
  Otherwise makes sense (and finally fixes some Wayland uglyness I've been 
annoyed by ...).

REPOSITORY
  R242 Plasma Framework (Library)

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

To: apol, #plasma
Cc: hein, plasma-devel, #frameworks, michaelh, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10405: Don't proceed in runCommandInternal if the executable doesn't exit

[Eike Hein]

hein updated this revision to Diff 26821.
hein added a comment.


  Remove unrelated changes.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10405?vs=26820=26821

BRANCH
  master

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

AFFECTED FILES
  src/widgets/krun.cpp

To: hein, dfaure, davidedmundson, mart
Cc: #frameworks, michaelh, ngraham

D10405: Don't proceed in runCommandInternal if the executable doesn't exit

[Eike Hein]

hein added a comment.


  Argh, sorry for the noisy diff, I didn't do commit -a but arc did I guess ... 
I'll clean this up, one sec.

REPOSITORY
  R241 KIO

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

To: hein, dfaure, davidedmundson, mart
Cc: #frameworks, michaelh, ngraham

D10405: Don't proceed in runCommandInternal if the executable doesn't exit

[Eike Hein]

hein created this revision.
hein added reviewers: dfaure, davidedmundson, mart.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
hein requested review of this revision.

REVISION SUMMARY
  Without this patch, runCommandInternal tries running first and
  handles a missing executable in slotProcessExited after process
  execution ended with a non-zero exit code.
  
  One of the side-effects is that a startup notification is created
  even if nothing is ultimately run.
  
  With this patch, we check for executable existence first and
  abort if we can't find it.
  
  This fixes a Plasma shell crash in the Task Manager applet: We
  use KRun from inside the onReleased handler of a MouseArea. In
  this error case this leads to a KMessageBox spinning the event
  loop. The delegate hosting the MouseArea is in the meanwhile
  deleted because it's replaced by a different one for the startup
  creation. After closing the dialog we're back in the destroyed
  delegate and crash.
  
  BUG:385942

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/core/kprotocolinfofactory.cpp
  src/widgets/jobuidelegate.cpp
  src/widgets/jobuidelegate.h
  src/widgets/krun.cpp

To: hein, dfaure, davidedmundson, mart
Cc: #frameworks, michaelh, ngraham

D9207: Don't load KDE platform module in kglobalaccel5

[Eike Hein]

hein abandoned this revision.
hein added a comment.


  As mentioned, I'm OK with abandoning it. I think the change is hygienic, but 
it's also a micro-optimization.

REPOSITORY
  R268 KGlobalAccel

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

To: hein, #plasma, davidedmundson, mart, graesslin
Cc: plasma-devel, #frameworks, michaelh, ZrenBot, ngraham, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D9207: Don't load KDE platform module in kglobalaccel5

[Eike Hein]

hein added a comment.


  Well, the maintainer spoke out against it, so not much I can do.

REPOSITORY
  R268 KGlobalAccel

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

To: hein, #plasma, davidedmundson, mart, graesslin
Cc: plasma-devel, #frameworks, michaelh, ZrenBot, ngraham, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10251: [RFC] Use mipmap filtering in window thumbnails

[Eike Hein]

hein added a comment.


  It's definitely nicer, but could we go for the full thing and make it crisper 
too?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: fredrik, #plasma, #vdg
Cc: hein, plasma-devel, #frameworks, michaelh, ZrenBot, ngraham, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart