D29101: KNewStuff: Fix file path and process call

[Alexander Lohnau]

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

REPOSITORY
  R304 KNewStuff

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

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

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

[Dan Leinir Turthra Jensen]

leinir requested changes to this revision.
leinir added a comment.
This revision now requires changes to proceed.


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

INLINE COMMENTS

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

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

REPOSITORY
  R304 KNewStuff

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

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

D29149: Fix kio-extras compilation with -DQT_NO_CAST_TO_ASCII

[David Faure]

dfaure created this revision.
dfaure added reviewers: thiago, sitter.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
dfaure requested review of this revision.

REVISION SUMMARY
  D27804  broke compilation for me because 
I have the above flag set ;-)
  It showed rather some rather nasty QString -> QByteArray conversions in this 
code.

TEST PLAN
  Now it builds.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  smb/smburl.cpp

To: dfaure, thiago, sitter
Cc: kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, iasensio, 
aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, rdieter, mikesomov

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Stefan Brüns]

bruns added a comment.


  In D28590#656133 , @meven wrote:
  
  > In D28590#654139 , @bruns wrote:
  >
  > > Do not create m_storageAccess in the constructor
  >
  >
  > Hmm, you told me `the f (m_displayName.isEmpty()) block belongs here`, I 
don't see why instantiating `m_storageAccess` here is bad but mehh.
  
  
  Please read again.
  
  1. m_storageAccess is comparatively expensive
  2. I showed you how to get the mount state without StorageAccess
  3. I told you where to initialize it, yes. Doing it correctly of course still 
applies.

REPOSITORY
  R245 Solid

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

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

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

[Alexander Lohnau]

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

REPOSITORY
  R304 KNewStuff

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

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

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

[Alexander Lohnau]

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


  Emit signalEntryChanged for manually deletet entry

REPOSITORY
  R304 KNewStuff

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

BRANCH
  bugfix_install_uninstall_messages (branched from master)

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

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

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

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

[Alexander Lohnau]

alex added inline comments.

INLINE COMMENTS

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

Yes I get what you mean :-).

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

REPOSITORY
  R304 KNewStuff

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

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

D29101: WIP Fix file path and process call

[Méven Car]

meven retitled this revision from "WIP BUG: 420312 Fix file path and process 
call" to "WIP Fix file path and process call".
meven edited the summary of this revision.

REPOSITORY
  R304 KNewStuff

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

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

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

[Alexander Lohnau]

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


  Merge branch 'bugfix_uninstall'

REPOSITORY
  R304 KNewStuff

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

BRANCH
  bugfix_install_uninstall_messages (branched from master)

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

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

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

D29149: Fix kio-extras compilation with -DQT_NO_CAST_TO_ASCII

[Harald Sitter]

sitter added a comment.


  Shouldn't we just make the entire thing QT_NO_CAST_TO_ASCII by default?

REPOSITORY
  R320 KIO Extras

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

To: dfaure, thiago, sitter
Cc: kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, iasensio, 
aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, rdieter, mikesomov

D29101: KNewStuff: Fix file path and process call

[Alexander Lohnau]

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

REPOSITORY
  R304 KNewStuff

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

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

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

[Alexander Lohnau]

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


  Use KShell to split args

REPOSITORY
  R304 KNewStuff

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

BRANCH
  bugfix_uninstall (branched from master)

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

AFFECTED FILES
  src/core/installation.cpp

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

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

[Alexander Lohnau]

alex edited the summary of this revision.

REPOSITORY
  R304 KNewStuff

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

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

D29149: Fix kio-extras compilation with -DQT_NO_CAST_TO_ASCII

[Harald Sitter]

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


  Sure, I was referring to the smb module, not even kio-extras as a whole. ^^
  
  Anyway, diff looks reasonable.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: dfaure, thiago, sitter
Cc: meven, kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
bruns, emmanuelp, rdieter, mikesomov

D29149: Fix kio-extras compilation with -DQT_NO_CAST_TO_ASCII

[Harald Sitter]

sitter added inline comments.

INLINE COMMENTS

> dfaure wrote in CMakeLists.txt:60
> Isn't that what I did here? Now all of kio-extras gets that flag.
> 
> And BTW everything built by kdesrc-build actually builds with this flag 
> (since I have it in my kdesrc-buildrc). But that doesn't allow setting it in 
> ECM, there is more code using ECM out there...

Huh, I must have scrolled right past it. All is perfect!

REPOSITORY
  R320 KIO Extras

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

To: dfaure, thiago, sitter
Cc: meven, kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
bruns, emmanuelp, rdieter, mikesomov

KDE CI: Frameworks » kio » kf5-qt5 FreeBSDQt5.14 - Build # 72 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.14/72/
 Project:
kf5-qt5 FreeBSDQt5.14
 Date of build:
Fri, 24 Apr 2020 12:07:58 +
 Build duration:
24 min and counting
   JUnit Tests
  Name: projectroot Failed: 1 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_jobtestName: projectroot.autotests Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)Name: projectroot.src.ioslaves.trash Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot.src.kpasswdserver Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)

D29138: [Solid] Replace foreach (deprecated) with range/index for

[Ahmad Samir]

ahmadsamir marked 4 inline comments as done.
ahmadsamir added a comment.


  In D29138#656140 , @meven wrote:
  
  > In D29138#656094 , @dfaure wrote:
  >
  > > Wow those iterations over map.keys() were awful.
  >
  >
  > We still have a few of those in solid :
  
  
  I know. "foreach" marks the place :)

INLINE COMMENTS

> bruns wrote in fakecdrom.cpp:48
> The obviously better solution is to iterate over `supported_medialist` and 
> swap the key and value of the map.

Good point.

REPOSITORY
  R245 Solid

BRANCH
  l-foreach (branched from master)

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

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

D28701: Add KPackage support to KNewStuffCore

[Dan Leinir Turthra Jensen]

leinir added inline comments.

INLINE COMMENTS

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

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

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

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

REPOSITORY
  R304 KNewStuff

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

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

D29054: [Wayland] Add to PlasmaWindowManagement protocol windows stacking order

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> plasmawindowmanagement_interface.h:76
>  
> +
> +/**

A space too many

REPOSITORY
  R127 KWayland

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

To: bport, zzag, davidedmundson
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29054: [Wayland] Add to PlasmaWindowManagement protocol windows stacking order

[Vlad Zahorodnii]

zzag added inline comments.

INLINE COMMENTS

> plasma-window-management.xml:74
>  
> +
> +

To be in-line with upstream standards, please add ``.

REPOSITORY
  R127 KWayland

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

To: bport, zzag, davidedmundson
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29072: Optimize SVG

[Noah Davis]

ndavis added a comment.


  In D29072#654363 , @guoyunhe wrote:
  
  > I do try scour but the result is disappointing. I get 16 times the 
following error:
  >
  >   hge difference of 123 in ./icons-dark/apps/48/hwinfo.svg
  >
  >
  > which means 16 icons are unacceptable after scour optimization
  
  
  That's not true. Huge difference means you should make sure you know what the 
difference is, but most of the time it's perfectly fine.

REPOSITORY
  R266 Breeze Icons

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

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

D24956: Consider desktop files with NoDisplay attribute

[Méven Car]

meven abandoned this revision.
meven added a comment.


  D28856  should be preferred

REPOSITORY
  R268 KGlobalAccel

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

To: meven, mart, #plasma, fvogt, apol, davidedmundson
Cc: davidedmundson, davidre, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D25088: Use X-KDE-NoGlobalShortcut instead of NoDisplay to store that a component is disabled

[Méven Car]

meven abandoned this revision.
meven added a comment.


  D28856  should be preferred

REPOSITORY
  R268 KGlobalAccel

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

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

D27252: add maui index icon in pm style

[Noah Davis]

ndavis added a comment.


  @cblack is right, breeze icons tend to be sharper, but there's no rule 
requiring sharpness and I've accepted similar icons recently (maybe I should 
edit those a bit, but they're still good). However, I'm not sure how we'll 
differentiate this from Dolphin if we make the corners sharper. For now, remove 
the dark blue bit in the top right.

INLINE COMMENTS

> index.svg:29
> +  
> +   y1="10.5268" y2="15" gradientTransform="translate(-391.5697,-499.8)" 
> gradientUnits="userSpaceOnUse">
> +   

The shadow gradient here got moved way out of place: F8257499: 
Screenshot_20200424_074454.png 

That happens sometimes when you use SVG cleaner.

> index.svg:35
> + 
> +  
> +  

This one can't be seen, so get rid of it.

REPOSITORY
  R266 Breeze Icons

BRANCH
  index (branched from master)

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

To: mbruchert, camiloh, #vdg, ndavis
Cc: cblack, ndavis, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D29149: Fix kio-extras compilation with -DQT_NO_CAST_TO_ASCII

[David Faure]

dfaure closed this revision.

REPOSITORY
  R320 KIO Extras

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

To: dfaure, thiago, sitter
Cc: meven, kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
bruns, emmanuelp, rdieter, mikesomov

D28470: [PlasmaCore.IconItem] Refactor source handling for different types

[Konrad Materka]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:5a3fb570feda: [PlasmaCore.IconItem] Refactor source 
handling for different types (authored by kmaterka).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28470?vs=81031=81092

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

AFFECTED FILES
  src/declarativeimports/core/iconitem.cpp
  src/declarativeimports/core/iconitem.h

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

D7563: Add privilegeExecution field to file protocol description

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, dfaure, chinmoyr
Cc: kde-frameworks-devel, feverfew, mreeves, mati865, ngraham, elvisangelaccio, 
LeGast00n, cblack, michaelh, bruns

D7563: Add privilegeExecution field to file protocol description

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, dfaure, chinmoyr
Cc: kde-frameworks-devel, feverfew, mreeves, mati865, ngraham, elvisangelaccio, 
LeGast00n, cblack, michaelh, bruns

D29138: [Solid] Replace foreach (deprecated) with range/index for

[Ahmad Samir]

ahmadsamir updated this revision to Diff 81098.
ahmadsamir marked an inline comment as done.
ahmadsamir added a comment.


  More const
  Optimise by iterating over QStringList, and constFind in QMap

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29138?vs=81040=81098

BRANCH
  l-foreach (branched from master)

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

AFFECTED FILES
  src/imports/devices.cpp
  src/solid/devices/backends/fakehw/fakecdrom.cpp
  src/solid/devices/backends/fakehw/fakedevice.cpp
  src/solid/devices/backends/fakehw/fakemanager.cpp
  src/solid/devices/backends/fakehw/fakeopticaldisc.cpp
  src/solid/devices/backends/fakehw/fakeprocessor.cpp
  src/solid/devices/backends/fakehw/fakestorageaccess.cpp
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabmanager.cpp

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

D29149: Fix kio-extras compilation with -DQT_NO_CAST_TO_ASCII

[Méven Car]

meven added a comment.


  In D29149#656310 , @sitter wrote:
  
  > Shouldn't we just make the entire thing QT_NO_CAST_TO_ASCII by default?
  
  
  It might need some work like here, so one repo at a time I would say.
  Even if that would a goal.
  We could target it for KF6.

REPOSITORY
  R320 KIO Extras

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

To: dfaure, thiago, sitter
Cc: meven, kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
bruns, emmanuelp, rdieter, mikesomov

D29149: Fix kio-extras compilation with -DQT_NO_CAST_TO_ASCII

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> CMakeLists.txt:60
>  
> -add_definitions(-DQT_NO_URL_CAST_FROM_STRING)
> +add_definitions(-DQT_NO_URL_CAST_FROM_STRING -DQT_NO_CAST_TO_ASCII)
>  

Isn't that what I did here? Now all of kio-extras gets that flag.

And BTW everything built by kdesrc-build actually builds with this flag (since 
I have it in my kdesrc-buildrc). But that doesn't allow setting it in ECM, 
there is more code using ECM out there...

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: dfaure, thiago, sitter
Cc: meven, kde-frameworks-devel, kfm-devel, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, LeGast00n, cblack, fbampaloukas, alexde, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
bruns, emmanuelp, rdieter, mikesomov

D27252: add maui index icon in pm style

[Noah Davis]

ndavis added inline comments.

INLINE COMMENTS

> ndavis wrote in index.svg:35
> This one can't be seen, so get rid of it.

Just realized this comment could be easily mistaken for me saying to get rid of 
the shadow because of how the phabricator UI looks. I'm talking about a blue 
rectangle.

REPOSITORY
  R266 Breeze Icons

BRANCH
  index (branched from master)

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

To: mbruchert, camiloh, #vdg, ndavis
Cc: cblack, ndavis, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

[David Faure]

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

REVISION SUMMARY
  This was still in KRun so the porting to ApplicationLauncherJob
  was actually losing that feature along the way.
  
  Move KRun's handling of untrusted programs to a separate class
  and provide it via an interface used by ApplicationLauncherJob
  and implemented in KIOWidgets.
  
  Ideally KIOWidget's JobUiDelegate class would implement it,
  but that's an exported class so it would be BIC.
  So for KF5, the JobUiDelegate registers the handler into kiogui
  using an internal global setter, and in KF6 JobUiDelegate itself
  can implement that interface. The benefit of this approach is
  that the application code stays the same:
  
job->setUiDelegate(new KIO::JobUiDelegate);
  
  We'll have to update all the "new KDialogUiDelegate" to the above
  line instead, where using ApplicationLauncherJob.

TEST PLAN
  Unittest + starting a non-executable desktop file
  and a non-executable copy of dolphin, in dolphin.

REPOSITORY
  R241 KIO

BRANCH
  2020_04_UntrustedProgramHandler

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

AFFECTED FILES
  autotests/applicationlauncherjobtest.cpp
  autotests/applicationlauncherjobtest.h
  src/core/CMakeLists.txt
  src/core/jobuidelegateextension.h
  src/core/untrustedprogramhandlerinterface.cpp
  src/core/untrustedprogramhandlerinterface.h
  src/gui/applicationlauncherjob.cpp
  src/gui/applicationlauncherjob.h
  src/widgets/CMakeLists.txt
  src/widgets/jobuidelegate.cpp
  src/widgets/krun.cpp
  src/widgets/widgetsuntrustedprogramhandler.cpp
  src/widgets/widgetsuntrustedprogramhandler.h

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

D28701: Add KPackage support to KNewStuffCore

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> kpackagejob.cpp:55
> +
> +class KPackageTask : public QObject, public QRunnable
> +{

is it worth encapsulating it in a runnable? the installation of a package is 
already in a separathe thread.

> kpackagejob.cpp:97
> +});
> +loop.exec();
> +} else {

this would freeze a thread waiting another thread is done, while just using the 
job in an async wayshouldn't be much more heavy on the gui thread?

REPOSITORY
  R304 KNewStuff

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

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

D7563: Add privilegeExecution field to file protocol description

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, dfaure, chinmoyr
Cc: kde-frameworks-devel, feverfew, mreeves, mati865, ngraham, elvisangelaccio, 
LeGast00n, cblack, michaelh, bruns

D28590: Add a QString Solid::Device::displayName, used in Fstab Device for network mounts

[Méven Car]

meven marked 3 inline comments as done.
meven added a comment.


  In D28590#656297 , @bruns wrote:
  
  > In D28590#656133 , @meven wrote:
  >
  > > In D28590#654139 , @bruns 
wrote:
  > >
  > > > Do not create m_storageAccess in the constructor
  > >
  > >
  > > Hmm, you told me `the f (m_displayName.isEmpty()) block belongs here`, I 
don't see why instantiating `m_storageAccess` here is bad but mehh.
  >
  >
  > Please read again.
  >
  > 1. m_storageAccess is comparatively expensive
  > 2. I showed you how to get the mount state without StorageAccess
  > 3. I told you where to initialize it, yes. Doing it correctly of course 
still applies.
  
  
  Btw I updated already the code to use `FstabHandling::currentMountPoints` and 
`FstabHandling::currentMountPoints(m_device);` as you suggested (as 
StorageAccess does).

REPOSITORY
  R245 Solid

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

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

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

[David Faure]

dfaure updated this revision to Diff 81108.
dfaure added a comment.


  improve docu, add test in kruntest

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29153?vs=81101=81108

BRANCH
  2020_04_UntrustedProgramHandler

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

AFFECTED FILES
  autotests/applicationlauncherjobtest.cpp
  autotests/applicationlauncherjobtest.h
  src/core/CMakeLists.txt
  src/core/jobuidelegateextension.h
  src/core/untrustedprogramhandlerinterface.cpp
  src/core/untrustedprogramhandlerinterface.h
  src/gui/applicationlauncherjob.cpp
  src/gui/applicationlauncherjob.h
  src/widgets/CMakeLists.txt
  src/widgets/jobuidelegate.cpp
  src/widgets/krun.cpp
  src/widgets/widgetsuntrustedprogramhandler.cpp
  src/widgets/widgetsuntrustedprogramhandler.h
  tests/kruntest.cpp

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

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol updated this revision to Diff 81113.
apol added a comment.


  Refactor the protocol as suggested by David and Vlad

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28882?vs=80984=81113

BRANCH
  master

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

AFFECTED FILES
  autotests/server/CMakeLists.txt
  autotests/server/test_screencasting.cpp
  src/client/CMakeLists.txt
  src/client/protocols/screencast.xml
  src/client/registry.cpp
  src/client/registry.h
  src/client/screencasting.cpp
  src/client/screencasting.h
  src/server/CMakeLists.txt
  src/server/display.cpp
  src/server/display.h
  src/server/screencasting_interface.cpp
  src/server/screencasting_interface.h

To: apol, #kwin, jgrulich, davidedmundson
Cc: davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D29138: [Solid] Replace foreach (deprecated) with range/index for

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> bruns wrote in fakeopticaldisc.cpp:43
> These four lines can be written as
> `content |= map.value(type, Solid::OpticalDisc::ContentType(0));`

Nice. :)

I think Solid::OpticalDisc::NoContent is more readable.

REPOSITORY
  R245 Solid

BRANCH
  l-foreach (branched from master)

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

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

D28883: Add wrapper for wl_global_remove

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R127:c557cdba3a73: Add wrapper for wl_global_remove (authored 
by davidedmundson).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28883?vs=80279=81103

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

AFFECTED FILES
  autotests/client/test_wayland_blur.cpp
  src/server/global.cpp
  src/server/global.h

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

KDE CI: Frameworks » kwayland » kf5-qt5 FreeBSDQt5.14 - Build # 12 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kwayland/job/kf5-qt5%20FreeBSDQt5.14/12/
 Project:
kf5-qt5 FreeBSDQt5.14
 Date of build:
Fri, 24 Apr 2020 14:46:21 +
 Build duration:
11 min and counting
   JUnit Tests
  Name: projectroot.autotests Failed: 12 test(s), Passed: 31 test(s), Skipped: 0 test(s), Total: 43 test(s)Failed: projectroot.autotests.client.kwayland_testCompositorFailed: projectroot.autotests.client.kwayland_testDataDeviceFailed: projectroot.autotests.client.kwayland_testDataSourceFailed: projectroot.autotests.client.kwayland_testRegionFailed: projectroot.autotests.client.kwayland_testShmPoolFailed: projectroot.autotests.client.kwayland_testSubCompositorFailed: projectroot.autotests.client.kwayland_testSubSurfaceFailed: projectroot.autotests.client.kwayland_testWaylandConnectionThreadFailed: projectroot.autotests.client.kwayland_testWaylandRegistryFailed: projectroot.autotests.client.kwayland_testWaylandShellFailed: projectroot.autotests.client.kwayland_testWaylandSurfaceFailed: projectroot.autotests.server.kwayland_testWaylandServerDisplay

D29159: Optimize SVG with sour

[Yunhe Guo]

guoyunhe added a reviewer: Breeze.

REPOSITORY
  R266 Breeze Icons

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

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

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

[David Faure]

dfaure updated this revision to Diff 8.
dfaure added a comment.


  Fix error message box after the user clicks Cancel.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29153?vs=81108=8

BRANCH
  2020_04_UntrustedProgramHandler

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

AFFECTED FILES
  autotests/applicationlauncherjobtest.cpp
  autotests/applicationlauncherjobtest.h
  src/core/CMakeLists.txt
  src/core/jobuidelegateextension.h
  src/core/untrustedprogramhandlerinterface.cpp
  src/core/untrustedprogramhandlerinterface.h
  src/gui/applicationlauncherjob.cpp
  src/gui/applicationlauncherjob.h
  src/widgets/CMakeLists.txt
  src/widgets/jobuidelegate.cpp
  src/widgets/krun.cpp
  src/widgets/widgetsuntrustedprogramhandler.cpp
  src/widgets/widgetsuntrustedprogramhandler.h
  tests/kruntest.cpp

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

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

[David Faure]

dfaure added a reviewer: mdlubakowski.

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

D29138: [Solid] Replace foreach (deprecated) with range/index for

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> fakeopticaldisc.cpp:43
>  }
>  }
>  

These four lines can be written as
`content |= map.value(type, Solid::OpticalDisc::ContentType(0));`

REPOSITORY
  R245 Solid

BRANCH
  l-foreach (branched from master)

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

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

D28882: Create protocol to manage video feeds

[Aleix Pol Gonzalez]

apol marked an inline comment as done.
apol added inline comments.

INLINE COMMENTS

> davidedmundson wrote in screencast.xml:31
> this is racey with create.

Any suggestion on how to do it better?

REPOSITORY
  R127 KWayland

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

To: apol, #kwin, jgrulich, davidedmundson
Cc: davidedmundson, romangg, zzag, kde-frameworks-devel, LeGast00n, cblack, 
michaelh, ngraham, bruns

D29072: Optimize SVG

[Yunhe Guo]

guoyunhe added a comment.


  The diff compare is based on Inkscape PNG output. And you can also check git 
cola's pixel diff, the diff of `icons-dark/actions/22/color-management.svg` 
looks like this (black means no difference):
  
  F8258009: image.png 
  
  The scour solution is D29159 

REPOSITORY
  R266 Breeze Icons

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

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

D29159: Optimize SVG with sour

[Yunhe Guo]

guoyunhe created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
guoyunhe requested review of this revision.

REVISION SUMMARY
  Some icons are failed to optimize due to huge pixel difference though you 
cannot tell by eyes. These files are not updated in this patch.

REPOSITORY
  R266 Breeze Icons

BRANCH
  scour

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

AFFECTED FILES
  icons-dark/actions/22/im-skype.svg
  icons-dark/actions/22/office-chart-polar.svg
  icons-dark/actions/22/run-build-clean.svg
  icons-dark/actions/22/run-build-configure.svg
  icons-dark/actions/22/run-build-file.svg
  icons-dark/actions/22/run-build-prune.svg
  icons-dark/actions/22/run-build.svg
  icons-dark/actions/22/step_object_CircularMotor.svg
  icons-dark/actions/22/step_object_LinearMotor.svg
  icons-dark/actions/22/step_object_SoftBody.svg
  icons-dark/actions/32/colors-luma.svg
  icons-dark/actions/symbolic/pan-down-symbolic.svg
  icons-dark/actions/symbolic/pan-end-symbolic.svg
  icons-dark/actions/symbolic/pan-start-symbolic.svg
  icons-dark/actions/symbolic/pan-up-symbolic.svg
  icons-dark/actions/symbolic/selection-end-symbolic.svg
  icons-dark/actions/symbolic/selection-start-symbolic.svg
  icons-dark/applets/128/user-none.svg
  icons-dark/applets/48/weather-clouds-night.svg
  icons-dark/applets/48/weather-few-clouds-night.svg
  icons-dark/applets/48/weather-showers-day.svg
  icons-dark/applets/48/weather-showers-scattered-night.svg
  icons-dark/applets/48/weather-snow-rain.svg
  icons-dark/applets/48/weather-snow-scattered-day.svg
  icons-dark/applets/48/weather-snow.svg
  icons-dark/apps/48/QtProject-designer.svg
  icons-dark/apps/48/accessories-calculator.svg
  icons-dark/apps/48/acroread.svg
  icons-dark/apps/48/alienarena.svg
  icons-dark/apps/48/anjuta.svg
  icons-dark/apps/48/babe.svg
  icons-dark/apps/48/baloo.svg
  icons-dark/apps/48/bluegriffon.svg
  icons-dark/apps/48/bovo.svg
  icons-dark/apps/48/braindump.svg
  icons-dark/apps/48/choqok.svg
  icons-dark/apps/48/codeblocks.svg
  icons-dark/apps/48/diaspora.svg
  icons-dark/apps/48/digikam.svg
  icons-dark/apps/48/falkon.svg
  icons-dark/apps/48/granatier.svg
  icons-dark/apps/48/kapman.svg
  icons-dark/apps/48/karbon.svg
  icons-dark/apps/48/kbruch.svg
  icons-dark/apps/48/kde-im-log-viewer.svg
  icons-dark/apps/48/kdiff3.svg
  icons-dark/apps/48/kmag.svg
  icons-dark/apps/48/knights.svg
  icons-dark/apps/48/ksnapshot.svg
  icons-dark/apps/48/ktorrent.svg
  icons-dark/apps/48/lokalize.svg
  icons-dark/apps/48/mpv.svg
  icons-dark/apps/48/muondiscover.svg
  icons-dark/apps/48/octave.svg
  icons-dark/apps/48/phonon-gstreamer.svg
  icons-dark/apps/48/planetkde.svg
  icons-dark/apps/48/plasma-media-center.svg
  icons-dark/apps/48/qdbusviewer.svg
  icons-dark/apps/48/qtcreator.svg
  icons-dark/apps/48/showfoto.svg
  icons-dark/apps/48/skanlite.svg
  icons-dark/apps/48/step.svg
  icons-dark/apps/48/umbrello.svg
  icons-dark/apps/48/utilities-log-viewer.svg
  icons-dark/apps/48/words.svg
  icons-dark/categories/32/applications-office.svg
  icons-dark/devices/64/camera-video.svg
  icons-dark/devices/64/input-gaming.svg
  icons-dark/devices/64/media-flash-memory-stick.svg
  icons-dark/devices/64/media-optical-blu-ray.svg
  icons-dark/devices/64/network-rj11-female.svg
  icons-dark/devices/64/network-rj45-female.svg
  icons-dark/mimetypes/16/application-pgp-signature.svg
  icons-dark/mimetypes/16/text-dockerfile.svg
  icons-dark/mimetypes/22/android-package-archive.svg
  icons-dark/mimetypes/22/application-certificate.svg
  icons-dark/mimetypes/22/application-illustrator.svg
  icons-dark/mimetypes/22/application-msonenote.svg
  icons-dark/mimetypes/22/application-msoutlook.svg
  icons-dark/mimetypes/22/application-msword.svg
  icons-dark/mimetypes/22/application-vnd.ms-excel.addin.macroenabled.12.svg
  icons-dark/mimetypes/22/application-vnd.ms-infopath.svg
  icons-dark/mimetypes/22/application-vnd.oasis.opendocument.text-template.svg
  icons-dark/mimetypes/22/application-x-apple-diskimage.svg
  icons-dark/mimetypes/22/application-x-bzip.svg
  icons-dark/mimetypes/22/application-x-gettext-translation.svg
  icons-dark/mimetypes/22/application-x-javascript.svg
  icons-dark/mimetypes/22/application-x-lyx.svg
  icons-dark/mimetypes/22/application-x-rdata.svg
  icons-dark/mimetypes/22/application-x-trash.svg
  icons-dark/mimetypes/22/application-x-xliff.svg
  icons-dark/mimetypes/22/application-xmind.svg
  icons-dark/mimetypes/22/application-zip.svg
  icons-dark/mimetypes/22/image-bmp.svg
  icons-dark/mimetypes/22/image-jpeg.svg
  icons-dark/mimetypes/22/image-tiff.svg
  icons-dark/mimetypes/22/image-x-generic.svg
  icons-dark/mimetypes/22/image-x-psd.svg
  icons-dark/mimetypes/22/image-x-tga.svg
  icons-dark/mimetypes/22/image-x-xcf.svg
  icons-dark/mimetypes/22/package-x-generic.svg
  icons-dark/mimetypes/22/text-css.svg
  icons-dark/mimetypes/22/text-x-qml.svg
  icons-dark/mimetypes/22/video-mp4.svg
  

D29138: [Solid] Replace foreach (deprecated) with range/index for

[Ahmad Samir]

ahmadsamir updated this revision to Diff 81121.
ahmadsamir marked an inline comment as done.
ahmadsamir added a comment.


  Make the code much shorter with map.value(key, defaultvalue), since the 
ContentType enum has a NoContent member

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29138?vs=81098=81121

BRANCH
  l-foreach (branched from master)

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

AFFECTED FILES
  src/imports/devices.cpp
  src/solid/devices/backends/fakehw/fakecdrom.cpp
  src/solid/devices/backends/fakehw/fakedevice.cpp
  src/solid/devices/backends/fakehw/fakemanager.cpp
  src/solid/devices/backends/fakehw/fakeopticaldisc.cpp
  src/solid/devices/backends/fakehw/fakeprocessor.cpp
  src/solid/devices/backends/fakehw/fakestorageaccess.cpp
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabmanager.cpp

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

D29163: WIP: Add document-replace icon (Like for Overwrite action)

[David Hurka]

davidhurka created this revision.
davidhurka added a reviewer: VDG.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidhurka requested review of this revision.

REVISION SUMMARY
  This adds an icon document-replace in 16px, 22px, and 32px size,
  which can be used for the overwrite confirmation box of the Save As dialog.

REPOSITORY
  R266 Breeze Icons

BRANCH
  create-document-replace

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

AFFECTED FILES
  document-replace.svg

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

D29163: WIP: Add document-replace icon (Like for Overwrite action)

[David Hurka]

davidhurka planned changes to this revision.
davidhurka added a comment.


  The current state is as follows:
  F8258336: image.png 
  F8258342: image.png 
  
  All paper sheets are shrunken by 2px, and 16px and 32px got a smaller corner 
fold to look proportional.
  
  The 16px black sheet got its original corner fold back, because it looked too 
small otherwise.
  
  I would like some feedback
  
  - on the overall icon idea
  - Whether I should connect the red sheet to the black sheet. Usually there is 
1px margin arround emblems, but in this case the sheets have different color, 
so no margin would theoretically be possible.
  
  Without margin:
  F8258350: image.png 

REPOSITORY
  R266 Breeze Icons

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

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

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

[David Faure]

dfaure added a comment.


  Thanks for the review! I was getting desperate to get one ;-)

INLINE COMMENTS

> svuorela wrote in kcmoduleinfo.cpp:73
> At a later point, I*m not sure what the purpose is for these members are - 
> but that's probably for another changeset.

Right, I was wondering the same. We could just implement the getters so that 
they call these things directly.

But then I'm also wondering what's the purpose of KCModuleInfo at all and 
whether we could just use KPluginInfo directly instead -- but that's a KF6 
consideration, since it's part of the API for KCMultiDialog and others.

Well, we could add a addModule(KPluginInfo) overload if that's the direction 
we're going for; I just don't have full overview on the KCM stuff.

> svuorela wrote in kcmoduleinfo.h:131
> Can we mark it as deprecated?

It's complicated.

1. If you use the QString constructor, you know service() is usable. That's the 
case for all users of KCModuleInfo except KCModuleLoader. [Not that there are 
many]

2. Even KCModuleLoader calls service(), to test for noDisplay().

The whole concept of NoDisplay only makes sense for desktop files, unless we 
want to have that in JSON metadata as well. I suppose we do, but this currently 
seems to be completely missing (e.g. from KPluginMetaData, if we want this in 
all plugins, not just KCMs). We'd have to duplicate the logic currently in 
KService::noDisplay().

Any volunteers to look into this? :-)

REPOSITORY
  R295 KCMUtils

BRANCH
  master

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

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

D29138: [Solid] Replace foreach (deprecated) with range/index for

[Ahmad Samir]

ahmadsamir updated this revision to Diff 81131.
ahmadsamir added a comment.


  Remove redundant lines...

REPOSITORY
  R245 Solid

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29138?vs=81121=81131

BRANCH
  l-foreach (branched from master)

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

AFFECTED FILES
  src/imports/devices.cpp
  src/solid/devices/backends/fakehw/fakecdrom.cpp
  src/solid/devices/backends/fakehw/fakedevice.cpp
  src/solid/devices/backends/fakehw/fakemanager.cpp
  src/solid/devices/backends/fakehw/fakeopticaldisc.cpp
  src/solid/devices/backends/fakehw/fakeprocessor.cpp
  src/solid/devices/backends/fakehw/fakestorageaccess.cpp
  src/solid/devices/backends/fstab/fstabdevice.cpp
  src/solid/devices/backends/fstab/fstabmanager.cpp

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

D29138: [Solid] Replace foreach (deprecated) with range/index for

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> bruns wrote in fakeopticaldisc.cpp:43
> You forgot to remove the old lines.
> 
> Can also be applied to supportedMedia

/* brown paper bag */, removed the lines.

I don't see a default a-la-NoContent MediumType member.

REPOSITORY
  R245 Solid

BRANCH
  l-foreach (branched from master)

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

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

D29138: [Solid] Replace foreach (deprecated) with range/index for

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> ahmadsamir wrote in fakeopticaldisc.cpp:43
> /* brown paper bag */, removed the lines.
> 
> I don't see a default a-la-NoContent MediumType member.

Just use `0`.

REPOSITORY
  R245 Solid

BRANCH
  l-foreach (branched from master)

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

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

D28765: KSettings::Dialog: add support for KPluginInfos without a KService

[Sune Vuorela]

svuorela accepted this revision.
svuorela added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> kcmoduleinfo.cpp:73
> +lib = pluginInfo.libraryPath();
> +keywords = 
> pluginInfo.property(QStringLiteral("Keywords")).toStringList();
>  }

At a later point, I*m not sure what the purpose is for these members are - but 
that's probably for another changeset.

> kcmoduleinfo.h:131
> + * @warning This will be null if this KCModuleInfo was created from a 
> KPluginInfo coming from KPluginMetaData.
> + * Prefer using pluginInfo() instead, which works for both kinds.
> + */

Can we mark it as deprecated?

REPOSITORY
  R295 KCMUtils

BRANCH
  master

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

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

D29095: New much simpler mouse icon that works in both light and dark breeze

[Chris Escargot]

saligari updated this revision to Diff 81126.
saligari added a comment.


  - a bit darker, with a more pronounced gradient effect

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29095?vs=81041=81126

BRANCH
  input-mouse-patch (branched from master)

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

AFFECTED FILES
  icons-dark/devices/64/input-mouse.svg
  icons/devices/64/input-mouse.svg

To: saligari, #vdg
Cc: bruns, ouwerkerk, ndavis, ngraham, kde-frameworks-devel, LeGast00n, cblack, 
michaelh

D7563: Add privilegeExecution field to file protocol description

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, dfaure, chinmoyr
Cc: kde-frameworks-devel, feverfew, mreeves, mati865, ngraham, elvisangelaccio, 
LeGast00n, cblack, michaelh, bruns

D7563: Add privilegeExecution field to file protocol description

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, dfaure, chinmoyr
Cc: kde-frameworks-devel, feverfew, mreeves, mati865, ngraham, elvisangelaccio, 
LeGast00n, cblack, michaelh, bruns

D29163: RFC: Add document-replace icon (Like for Overwrite action)

[David Hurka]

davidhurka retitled this revision from "WIP: Add document-replace icon (Like 
for Overwrite action)" to "RFC: Add document-replace icon (Like for Overwrite 
action)".
davidhurka edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D29138: [Solid] Replace foreach (deprecated) with range/index for

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> ahmadsamir wrote in fakeopticaldisc.cpp:43
> Nice. :)
> 
> I think Solid::OpticalDisc::NoContent is more readable.

You forgot to remove the old lines.

Can also be applied to supportedMedia

REPOSITORY
  R245 Solid

BRANCH
  l-foreach (branched from master)

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

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

D29095: New much simpler mouse icon that works in both light and dark breeze

[Nathaniel Graham]

ngraham added a comment.


  LGTM now! While you're at it, could you create a 32px version and replace 
`icons/preferences/32/preferences-desktop-mouse.svg`?

REPOSITORY
  R266 Breeze Icons

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

To: saligari, #vdg
Cc: bruns, ouwerkerk, ndavis, ngraham, kde-frameworks-devel, LeGast00n, cblack, 
michaelh

D29095: New much simpler mouse icon that works in both light and dark breeze

[Chris Escargot]

saligari updated this revision to Diff 81138.
saligari added a comment.


  My search on the internet told me LGTM stands for looks good to me.
  I added a 32px with 3px margin

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29095?vs=81126=81138

BRANCH
  input-mouse-patch (branched from master)

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

AFFECTED FILES
  icons-dark/devices/64/input-mouse.svg
  icons-dark/preferences/32/preferences-desktop-mouse.svg
  icons/devices/64/input-mouse.svg
  icons/preferences/32/preferences-desktop-mouse.svg

To: saligari, #vdg
Cc: bruns, ouwerkerk, ndavis, ngraham, kde-frameworks-devel, LeGast00n, cblack, 
michaelh

D29095: New much simpler mouse icon that works in both light and dark breeze

[Noah Davis]

ndavis added a comment.


  It's a bit off center and I'd prefer if lines were more aligned with pixels. 
That way the icon doesn't change in how it looks when you scale it up. You may 
need to lighten the colors of your lines to maintain the original appearance.
  
  You see what I'm seeing? Do you know how to enable a grid with 1px spacing? 
F8258844: Screenshot_20200424_223530.png 
  
  This has some tips: 
https://community.kde.org/Guidelines_and_HOWTOs/Icon_Workflow_Tips
  
  Do check out the Inkscape manual as well. It's actually useful and not very 
difficult to read through.

REPOSITORY
  R266 Breeze Icons

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

To: saligari, #vdg
Cc: bruns, ouwerkerk, ndavis, ngraham, kde-frameworks-devel, LeGast00n, cblack, 
michaelh