D29170: Detect executables without +x permission in $PATH to improve error message

[David Faure]

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


  Resolve relative executables using the directory of the .desktop file 
referring to them
  
  Not useful for /usr/share/applications stuff, but useful for custom setups
  where a custom desktop file refers to a local executable.
  
  Re-reading the thread 
https://lists.freedesktop.org/archives/xdg/2011-April/011883.html
  I'm wondering if this is the right thing to do though.
  
  After all, on the command line "foo" doesn't start a local executable called 
foo,
  only ./foo does that. I was always working under that assumption for Exec= as 
well
  (the fact that the current dir isn't part of the search path). Undecided.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29170?vs=81209=81213

BRANCH
  2020_04_findExecutable

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

AFFECTED FILES
  autotests/applicationlauncherjobtest.cpp
  src/core/desktopexecparser.cpp
  src/gui/kprocessrunner.cpp

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

D29095: Change mouse icon to have better dark theme contrast

[Noah Davis]

ndavis retitled this revision from "Change mouse icon to have better dark theme 
contrast." to "Change mouse icon to have better dark theme contrast".

REPOSITORY
  R266 Breeze Icons

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

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

D29095: Change mouse icon to have better dark theme contrast.

[Noah Davis]

ndavis updated this revision to Diff 81211.
ndavis added a comment.


  - Change style

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29095?vs=81156=81211

BRANCH
  arcpatch-D29095

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: ndavis, #vdg, saligari
Cc: bruns, ouwerkerk, ndavis, ngraham, kde-frameworks-devel, LeGast00n, cblack, 
michaelh

D29095: Change mouse icon to have better dark theme contrast.

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D29095: Change mouse icon to have better dark theme contrast.

[Noah Davis]

ndavis retitled this revision from "New much simpler mouse icon that works in 
both light and dark breeze" to "Change mouse icon to have better dark theme 
contrast.".
ndavis edited the summary of this revision.
ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D29170: Detect executables without +x permission in $PATH to improve error message

[David Faure]

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


  Better support for relative vs absolute executable names, with unittest.
  
  But I'll look into relative-to-desktop-file instead of 
relative-to-QDir::current.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29170?vs=81159=81209

BRANCH
  2020_04_findExecutable

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

AFFECTED FILES
  autotests/applicationlauncherjobtest.cpp
  src/gui/kprocessrunner.cpp

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

D29170: Detect executables without +x permission in $PATH to improve error message

[David Faure]

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

INLINE COMMENTS

> ahmadsamir wrote in kprocessrunner.cpp:53
> KProcessRunner tries finding the executable in the current dir too, so to be 
> precise in the reported error message maybe append currentDir.absolutePath() 
> to searchPaths?
> 
> Also KProcessRunner only checks that the executable exists in the current 
> dir, "!QFileInfo::exists(realExecutable)", it should also check that it's 
> executable. So that KProcessRunner checks the "exists and is +x" and here we 
> check "exists but not +x", if that makes sense.
> 
> I guess you'll have the add an ifdef kludge since QString::SkipEmptyParts is 
> deprecated in Qt 5.15 according to 
> https://lxr.kde.org/source/frameworks/frameworkintegration/src/kpackage-install-handlers/kns/main.cpp#0068
>  (I am still on 5.14).

In a GUI program started graphically, the notion of "current dir" means very 
little. You can't see it, you can't change it.
Granted, when starting it from the command line it does serve a purpose for 
command line arguments, but IMHO not after that.

What's more, here we're executing a KService i.e. usually a desktop file 
(unless it was constructed from an executable name, display name and icon).

Hmm OK I can make a testcase for it with a special case. A copy of dolphin's 
desktop file with Exec=dolphin2 and a copy of dolphin called dolphin2 in the 
same directory, and starting dolphin from that directory. Interesting, it leads 
to "execvp: Permission denied", that's a new one :)
(must be from QProcess).

My next path will fix that testcase, but it remains an unexpected corner case. 
The same dolphin started from $HOME and then navigating to that directory, 
cannot start that desktop file.
Maybe we want to look for executables relative to the desktop file, rather than 
from the hidden-to-the-user current directory... Actually I remember people 
asking for that to work, a VERY long time ago on the freedesktop xdg 
mailing-list. IIRC I even made it work back then.

Thanks for the SkipEmptyParts information and for noticing the weird if() -- 
fixed.

> ahmadsamir wrote in kprocessrunner.cpp:60
> Should be https://

I was counting on the redirection :-)

REPOSITORY
  R241 KIO

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

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

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

[Noah Davis]

ndavis commandeered this revision.
ndavis added a reviewer: saligari.
ndavis added a comment.


  Saligari is Ok with me taking over this patch.

REPOSITORY
  R266 Breeze Icons

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

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

D29190: [FileWatchTest] Extend coverage to config updates

[Stefan Brüns]

bruns updated this revision to Diff 81207.
bruns added a comment.


  whitespace

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29190?vs=81205=81207

BRANCH
  fwtest

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

AFFECTED FILES
  autotests/unit/file/filewatchtest.cpp

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

D29190: [FileWatchTest] Extend coverage to config updates

[Stefan Brüns]

bruns added a dependent revision: D29191: [FileWatch] Fix watch updates on 
config changes.

REPOSITORY
  R293 Baloo

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

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

D29191: [FileWatch] Fix watch updates on config changes

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Track the current list of include/exclude folders, and on a config change
  add/remove watches for the differences. Previously, when an include
  folder was removed the corresponding watches were never removed.
  
  Depends on D29190 

TEST PLAN
  ctest -R filewatchtest

REPOSITORY
  R293 Baloo

BRANCH
  submit

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

AFFECTED FILES
  autotests/unit/file/filewatchtest.cpp
  src/file/filewatch.cpp
  src/file/filewatch.h

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

D29190: [FileWatchTest] Extend coverage to config updates

[Stefan Brüns]

bruns added a dependent revision: D29189: [KInotify] Fix path matching when 
removing watches.

REPOSITORY
  R293 Baloo

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

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

D29189: [KInotify] Fix path matching when removing watches

[Stefan Brüns]

bruns edited the summary of this revision.
bruns added a dependency: D29190: [FileWatchTest] Extend coverage to config 
updates.

REPOSITORY
  R293 Baloo

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

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

D29190: [FileWatchTest] Extend coverage to config updates

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  Currently, config changes are not covered in the
  tests, add a test case

TEST PLAN
  ctest -R filewatchtest

REPOSITORY
  R293 Baloo

BRANCH
  fwtest

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

AFFECTED FILES
  autotests/unit/file/filewatchtest.cpp

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

D29189: [KInotify] Fix path matching when removing watches

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  As in removeWatch the path is passed in with a trailing slash, no path
  is removed.

TEST PLAN
  ctest -R filewatch

REPOSITORY
  R293 Baloo

BRANCH
  fwtest

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

AFFECTED FILES
  autotests/unit/file/filewatchtest.cpp
  src/file/kinotify.cpp

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

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

[Noah Davis]

ndavis added a comment.


  In D29095#656819 , @saligari wrote:
  
  > Oh the shame! I had forgotten to center it (and I do know how to)!
  >
  > One thing I don't know is: since I centered it and used the guidelines my 
1px line centered sits on a 0,5 to 0,5 line. I don't know if that's bad or how 
to change it.
  
  
  That means you need to widen it to 2px. It's between pixels, so it's 2px 
thick when you look at it at the normal size anyway. F8260081: 
Screenshot_20200425_133013.png F8260078: 
Screenshot_20200425_133033.png 

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

D29171: Add 16px konversation icon

[Noah Davis]

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

REPOSITORY
  R266 Breeze Icons

BRANCH
  konversation16px (branched from master)

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

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

D29188: [FileWatch] Remove redundant watchIndexedFolders() slot

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  updateIndexedFoldersWatches() has the same effect, so there is no need
  for an additional function.
  
  Update the updateIndexedFoldersWatches() API documentation to reflect
  the current use.

TEST PLAN
  ctest

REPOSITORY
  R293 Baloo

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

AFFECTED FILES
  autotests/unit/file/filewatchtest.cpp
  src/file/filewatch.cpp
  src/file/filewatch.h
  src/file/mainhub.cpp

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

D29181: Use KFileMetaData for XAttr support instead of private reimplementation

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:774bd228b749: Use KFileMetaData for XAttr support instead 
of private reimplementation (authored by bruns).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29181?vs=81185=81192

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

AFFECTED FILES
  autotests/unit/file/CMakeLists.txt
  autotests/unit/file/filewatchtest.cpp
  autotests/unit/lib/baloo_xattr_p.h
  autotests/unit/lib/xattrdetector.cpp
  autotests/unit/lib/xattrdetector.h

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

D29181: Use KFileMetaData for XAttr support instead of private reimplementation

[Aleix Pol Gonzalez]

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

REPOSITORY
  R293 Baloo

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

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

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

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> ahmadsamir wrote in fakeopticaldisc.cpp:43
> I was talking about something like Solid::OpticalDrive::MediumType(0); 
> ContentType has Solid::OpticalDisc::NoContent whereas MediumType has nothing 
> similar.
> 
> IIUC named casts are preferred over old style casts, so I suggest either we 
> add a MediumType enumerator e.g. NotSupported = 0x0 or use a static_cast.

Thats not a cast but a constructor, see QFlags documentation

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

[Ahmad Samir]

D29181: Use KFileMetaData for XAttr support instead of private reimplementation

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  This removes a lot of duplicate code, and on top it actually works
  as it no longer hardcodes a number of locations (e.g. /tmp/ was always
  blacklisted).

TEST PLAN
  ctest

REPOSITORY
  R293 Baloo

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

AFFECTED FILES
  autotests/unit/file/CMakeLists.txt
  autotests/unit/file/filewatchtest.cpp
  autotests/unit/lib/baloo_xattr_p.h
  autotests/unit/lib/xattrdetector.cpp
  autotests/unit/lib/xattrdetector.h

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

D28980: Revert "add Baloo DBus signals for moved or removed files"

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:c2865b207907: Revert add Baloo DBus signals for 
moved or removed files (authored by bruns).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28980?vs=80540=81180

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

AFFECTED FILES
  autotests/unit/file/CMakeLists.txt
  autotests/unit/file/filewatchtest.cpp
  autotests/unit/file/metadatamovertest.cpp
  autotests/unit/file/metadatamovertest.h
  src/engine/transaction.cpp
  src/engine/transaction.h
  src/file/CMakeLists.txt
  src/file/filewatch.cpp
  src/file/filewatch.h
  src/file/mainhub.cpp
  src/file/mainhub.h
  src/file/metadatamover.cpp
  src/file/metadatamover.h

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

D29170: Detect executables without +x permission in $PATH to improve error message

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> kprocessrunner.cpp:53
> +// This is a *very* simplified version of 
> QStandardPaths::findExecutable
> +const QStringList searchPaths = 
> QString::fromLocal8Bit(qgetenv("PATH")).split(QDir::listSeparator(), 
> QString::SkipEmptyParts);
> +const QDir currentDir = QDir::current();

KProcessRunner tries finding the executable in the current dir too, so to be 
precise in the reported error message maybe append currentDir.absolutePath() to 
searchPaths?

Also KProcessRunner only checks that the executable exists in the current dir, 
"!QFileInfo::exists(realExecutable)", it should also check that it's 
executable. So that KProcessRunner checks the "exists and is +x" and here we 
check "exists but not +x", if that makes sense.

I guess you'll have the add an ifdef kludge since QString::SkipEmptyParts is 
deprecated in Qt 5.15 according to 
https://lxr.kde.org/source/frameworks/frameworkintegration/src/kpackage-install-handlers/kns/main.cpp#0068
 (I am still on 5.14).

> kprocessrunner.cpp:60
> +if (fileInfo.isExecutable()) {
> +qWarning() << "Internal program error. 
> QStandardPaths::findExecutable couldn't find" << executable << "but our own 
> logic found it at" << candidate << ". Please report a bug at 
> http://bugs.kde.org;;
> +} else {

Should be https://

REPOSITORY
  R241 KIO

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

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

D28956: [QML Monitor] Show remaining time as soon as possible

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:60766d57999c: [QML Monitor] Show remaining time as soon 
as possible (authored by bruns).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28956?vs=80489=81179

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

AFFECTED FILES
  src/qml/experimental/monitor.cpp
  src/qml/experimental/monitor.h

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

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

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> ahmadsamir wrote in fakeopticaldisc.cpp:43
> It doesn't make the code more readable; and using QMap::constFind() while 
> iterating is more recognizable/widely-used.

You are contradicting yourself. You use value() for content, but for 
supportedMedia it is a bad idea? Also, `supportedMedia |= 0` is obviously a 
noop, while for any named flag one has to lookup the flag value.

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

D28932: Store filename terms just once

[Stefan Brüns]

bruns added a comment.


  In D28932#657011 , @ngraham wrote:
  
  > I'll get around to reviewing this soon. I'm trying to figure out of I think 
the loss is acceptable.
  
  
  There is no loss, there is even a gain (queries work correctly in all 
constellations).

REPOSITORY
  R293 Baloo

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

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

Re: changing icon sizes no longer emits signal

[Friedrich W. H. Kossebau]

Am Samstag, 25. April 2020, 15:10:37 CEST schrieb Martin Koller:
> Hi,
> 
> in liquidshell I'm using
> 
>   connect(KIconLoader::global(), ::iconLoaderSettingsChanged,
> this, ::adjustIconSize);
> 
> to be informed whenever the icon sizes get changed in systemsettings.
> I don't know since when but at least in version
> 
> Operating System: openSUSE Tumbleweed 20200419
> KDE Plasma Version: 5.18.4
> KDE Frameworks Version: 5.69.0
> Qt Version: 5.14.1
> 
> I no longer receive this signal.

I can confirm that changing icon sizes in Plasma Systemsettings and clicking 
apply seems to have no effect also for all other places which would adapt to 
those values, like KXmlGui-driven toolbars (which also by a quick check seems 
to connect to that signal). The changed sizes are only picked up by newly 
started instances of programs.
I am also on openSUSE TW with same versions. Also not sure when I saw this 
working the last time.

Given the API dox of KIconLoader does not note that signal is deprecated, I 
would assume this signal is still to be used (and main.cpp of the icon kcm 
seems to call KIconLoader::emitChange(...), which should trigger the signal 
via D-Bus in all client side, from a peek view at code path.

Please file a bug, to Plasma Systemsettings I would do. Or debug if you can, 
some developer karma waiting to be picked up for who does :)

Cheers
Friedrich

D26342: Allow overriding to disable auto language detection

[Simon Depiets]

sdepiets updated this revision to Diff 81177.
sdepiets added a comment.


  Bump since + autotest checks the language as well

REPOSITORY
  R246 Sonnet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26342?vs=81176=81177

BRANCH
  master

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

AFFECTED FILES
  autotests/test_highlighter.cpp
  src/core/backgroundchecker.cpp
  src/core/backgroundchecker.h
  src/core/backgroundchecker_p.h
  src/ui/highlighter.cpp
  src/ui/highlighter.h

To: sdepiets, #frameworks, cullmann, mlaurent, mludwig
Cc: aacid, mludwig, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D28956: [QML Monitor] Show remaining time as soon as possible

[Nathaniel Graham]

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

REPOSITORY
  R293 Baloo

BRANCH
  submit

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

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

D26342: Allow overriding to disable auto language detection

[Albert Astals Cid]

aacid added a comment.


  You need to update the \since markers?
  
  After that i guess you can commit unless anyone really disagrees?
  
  @mludwig @cullmann ?

REPOSITORY
  R246 Sonnet

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

To: sdepiets, #frameworks, cullmann, mlaurent, mludwig
Cc: aacid, mludwig, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D26342: Allow overriding to disable auto language detection

[Simon Depiets]

sdepiets updated this revision to Diff 81176.
sdepiets added a comment.


  - Merge branch 'master' of git.kde.org:sonnet

REPOSITORY
  R246 Sonnet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26342?vs=76992=81176

BRANCH
  master

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

AFFECTED FILES
  autotests/test_highlighter.cpp
  src/core/backgroundchecker.cpp
  src/core/backgroundchecker.h
  src/core/backgroundchecker_p.h
  src/ui/highlighter.cpp
  src/ui/highlighter.h

To: sdepiets, #frameworks, cullmann, mlaurent, mludwig
Cc: aacid, mludwig, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, 
bruns

D29050: WIP KRunner fix prepare/teardown signals

[Alexander Lohnau]

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

REPOSITORY
  R308 KRunner

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

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

D28932: Store filename terms just once

[Nathaniel Graham]

ngraham added a comment.


  I'll get around to reviewing this soon. I'm trying to figure out of I think 
the loss is acceptable.

REPOSITORY
  R293 Baloo

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

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

D28909: smb: port to Result system to force serialization of error/finish condition

[Méven Car]

meven added inline comments.

INLINE COMMENTS

> dfaure wrote in kio_smb.h:96
> Excellent idea, but why wait for KF6?
> 
> We could implement a subclass of SlaveBase, in KIO, let's call it 
> SlaveBaseV2, which
> 
> - Reimplements all the virtual methods from SlaveBase, in a private section
> - Defines a new of virtual methods, called differently somehow
> - Implements the signal emission based on the return value of those new 
> virtual methods.
> 
> This allows for incremental porting of the slaves, instead of a big KF6 
> breakage.
> And the V2 class can be developed in a single slave first, before having to 
> freeze its API.

Sounds like a nice idea.

File ioslave would be the best one to work on first IMO, since it is the one 
the most complete, used and tested, albeit one of the biggest probably.

Let's make a task !

But this diff does not need to wait for this.

REPOSITORY
  R320 KIO Extras

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

To: sitter, dfaure
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 » syntax-highlighting » kf5-qt5 WindowsMSVCQt5.14 - Build # 42 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/syntax-highlighting/job/kf5-qt5%20WindowsMSVCQt5.14/42/
 Project:
kf5-qt5 WindowsMSVCQt5.14
 Date of build:
Sat, 25 Apr 2020 13:46:09 +
 Build duration:
6 min 2 sec and counting
   JUnit Tests
  Name: projectroot Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)

KDE CI: Frameworks » syntax-highlighting » kf5-qt5 SUSEQt5.14 - Build # 9 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/syntax-highlighting/job/kf5-qt5%20SUSEQt5.14/9/
 Project:
kf5-qt5 SUSEQt5.14
 Date of build:
Sat, 25 Apr 2020 13:43:55 +
 Build duration:
4 min 7 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5SyntaxHighlighting-5.70.0.xmlcompat_reports/KF5SyntaxHighlighting_compat_report.htmllogs/KF5SyntaxHighlighting/5.70.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report50%
(3/6)76%
(34/45)76%
(34/45)78%
(2742/3524)68%
(1877/2766)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests75%
(6/8)75%
(6/8)91%
(699/765)59%
(414/706)examples.codeeditor0%
(0/2)0%
(0/2)0%
(0/202)0%
(0/88)examples.codepdfprinter0%
(0/3)0%
(0/3)0%
(0/39)0%
(0/6)src.cli0%
(0/1)0%
(0/1)0%
(0/76)0%
(0/28)src.indexer100%
(1/1)100%
(1/1)74%
(258/347)78%
(247/318)src.lib90%
(27/30)90%
(27/30)85%
(1785/2095)75%
(1216/1620)

KDE CI: Frameworks » syntax-highlighting » kf5-qt5 SUSEQt5.12 - Build # 249 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/syntax-highlighting/job/kf5-qt5%20SUSEQt5.12/249/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Sat, 25 Apr 2020 13:42:54 +
 Build duration:
3 min 59 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5SyntaxHighlighting-5.70.0.xmlcompat_reports/KF5SyntaxHighlighting_compat_report.htmllogs/KF5SyntaxHighlighting/5.70.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report50%
(3/6)76%
(34/45)76%
(34/45)78%
(2742/3524)68%
(1877/2766)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests75%
(6/8)75%
(6/8)91%
(699/765)59%
(414/706)examples.codeeditor0%
(0/2)0%
(0/2)0%
(0/202)0%
(0/88)examples.codepdfprinter0%
(0/3)0%
(0/3)0%
(0/39)0%
(0/6)src.cli0%
(0/1)0%
(0/1)0%
(0/76)0%
(0/28)src.indexer100%
(1/1)100%
(1/1)74%
(258/347)78%
(247/318)src.lib90%
(27/30)90%
(27/30)85%
(1785/2095)75%
(1216/1620)

KDE CI: Frameworks » syntax-highlighting » kf5-qt5 WindowsMSVCQt5.14 - Build # 41 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/syntax-highlighting/job/kf5-qt5%20WindowsMSVCQt5.14/41/
 Project:
kf5-qt5 WindowsMSVCQt5.14
 Date of build:
Sat, 25 Apr 2020 13:39:52 +
 Build duration:
6 min 7 sec and counting
   JUnit Tests
  Name: projectroot Failed: 3 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 6 test(s)Failed: projectroot.autotests.folding_testFailed: projectroot.autotests.htmlhighlighter_testFailed: projectroot.autotests.testhighlighter_test

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

[David Faure]

dfaure added a comment.


  > The question is how that will work in conjunction with 
KNotificationJobUiDelegate? In principle we could also make it emit a 
KNotification with buttons
  
  We would need a KIO::NotificationJobUiDelegate subclass of 
KNotificationJobUiDelegate in KIOGui, which also implements 
UntrustedProgramHandlerInterface.
  For KF5, its constructor would register itself using 
setDefaultUntrustedProgramHandler, and the destructor would use that same 
method to reset it to nullptr (to avoid leaving a dangling pointer).
  Alternatively it could do just like KIO::JobUiDelegate and register a 
singleton.
  
  In KF6, it'll inherit the handler interface and it'll just be autodetected by 
qobject_cast on the job's delegate (or we could even already do this as the 
primary way to find the handler, with fallback to checking the singleton --- 
this is all because I can't change what KIO::JobUiDelegate inherits from).

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

KDE CI: Frameworks » syntax-highlighting » kf5-qt5 SUSEQt5.14 - Build # 8 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/syntax-highlighting/job/kf5-qt5%20SUSEQt5.14/8/
 Project:
kf5-qt5 SUSEQt5.14
 Date of build:
Sat, 25 Apr 2020 13:39:52 +
 Build duration:
4 min 0 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5SyntaxHighlighting-5.70.0.xmlcompat_reports/KF5SyntaxHighlighting_compat_report.htmllogs/KF5SyntaxHighlighting/5.70.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 3 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 6 test(s)Failed: projectroot.autotests.folding_testFailed: projectroot.autotests.htmlhighlighter_testFailed: projectroot.autotests.testhighlighter_test
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report50%
(3/6)76%
(34/45)76%
(34/45)78%
(2742/3524)68%
(1876/2766)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests75%
(6/8)75%
(6/8)91%
(699/765)58%
(413/706)examples.codeeditor0%
(0/2)0%
(0/2)0%
(0/202)0%
(0/88)examples.codepdfprinter0%
(0/3)0%
(0/3)0%
(0/39)0%
(0/6)src.cli0%
(0/1)0%
(0/1)0%
(0/76)0%
(0/28)src.indexer100%
(1/1)100%
(1/1)74%
(258/347)78%
(247/318)src.lib90%
(27/30)90%
(27/30)85%
(1785/2095)75%
(1216/1620)

KDE CI: Frameworks » syntax-highlighting » kf5-qt5 FreeBSDQt5.14 - Build # 10 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/syntax-highlighting/job/kf5-qt5%20FreeBSDQt5.14/10/
 Project:
kf5-qt5 FreeBSDQt5.14
 Date of build:
Sat, 25 Apr 2020 13:42:03 +
 Build duration:
2 min 16 sec and counting
   JUnit Tests
  Name: projectroot Failed: 0 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s)

KDE CI: Frameworks » syntax-highlighting » kf5-qt5 SUSEQt5.12 - Build # 248 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/syntax-highlighting/job/kf5-qt5%20SUSEQt5.12/248/
 Project:
kf5-qt5 SUSEQt5.12
 Date of build:
Sat, 25 Apr 2020 13:39:52 +
 Build duration:
3 min 1 sec and counting
   BUILD ARTIFACTS
  abi-compatibility-results.yamlacc/KF5SyntaxHighlighting-5.70.0.xmlcompat_reports/KF5SyntaxHighlighting_compat_report.htmllogs/KF5SyntaxHighlighting/5.70.0/log.txt
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 3 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 6 test(s)Failed: projectroot.autotests.folding_testFailed: projectroot.autotests.htmlhighlighter_testFailed: projectroot.autotests.testhighlighter_test
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report50%
(3/6)76%
(34/45)76%
(34/45)78%
(2742/3524)68%
(1877/2766)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests75%
(6/8)75%
(6/8)91%
(699/765)59%
(414/706)examples.codeeditor0%
(0/2)0%
(0/2)0%
(0/202)0%
(0/88)examples.codepdfprinter0%
(0/3)0%
(0/3)0%
(0/39)0%
(0/6)src.cli0%
(0/1)0%
(0/1)0%
(0/76)0%
(0/28)src.indexer100%
(1/1)100%
(1/1)74%
(258/347)78%
(247/318)src.lib90%
(27/30)90%
(27/30)85%
(1785/2095)75%
(1216/1620)

D28776: FstabDevice: Avoid recurrent construction of emblems QStringList

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R245 Solid

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

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

KDE CI: Frameworks » syntax-highlighting » kf5-qt5 FreeBSDQt5.14 - Build # 9 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/syntax-highlighting/job/kf5-qt5%20FreeBSDQt5.14/9/
 Project:
kf5-qt5 FreeBSDQt5.14
 Date of build:
Sat, 25 Apr 2020 13:39:52 +
 Build duration:
2 min 8 sec and counting
   JUnit Tests
  Name: projectroot Failed: 3 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 6 test(s)Failed: projectroot.autotests.folding_testFailed: projectroot.autotests.htmlhighlighter_testFailed: projectroot.autotests.testhighlighter_test

D28777: FstabDevice: Reevaluate emblems only when state changes

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R245 Solid

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

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

D28870: Updated test file for Logtalk syntax highlighting

[Christoph Cullmann]

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

REPOSITORY
  R216 Syntax Highlighting

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

To: pmoura, cullmann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D28870: Updated test file for Logtalk syntax highlighting

[Christoph Cullmann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:559cda73cf57: Updated test file for Logtalk syntax 
highlighting (authored by cullmann).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28870?vs=80253=81174

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

AFFECTED FILES
  autotests/input/highlight.lgt

To: pmoura, cullmann
Cc: kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

[David Faure]

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


  Make UntrustedProgramHandlerInterface async.
  
  This required a nasty QEventLoop for KRun though, since it has a sync API.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29153?vs=81158=81173

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/krun_p.h
  src/widgets/widgetsuntrustedprogramhandler.cpp
  src/widgets/widgetsuntrustedprogramhandler.h
  tests/kruntest.cpp

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

D28956: [QML Monitor] Show remaining time as soon as possible

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R293 Baloo

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

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

D28932: Store filename terms just once

[Stefan Brüns]

bruns added a comment.


  Ping!

REPOSITORY
  R293 Baloo

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

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

changing icon sizes no longer emits signal

[Martin Koller]

Hi,

in liquidshell I'm using

  connect(KIconLoader::global(), ::iconLoaderSettingsChanged, this, 
::adjustIconSize);

to be informed whenever the icon sizes get changed in systemsettings.
I don't know since when but at least in version

Operating System: openSUSE Tumbleweed 20200419
KDE Plasma Version: 5.18.4
KDE Frameworks Version: 5.69.0
Qt Version: 5.14.1

I no longer receive this signal.
Does it have to do with the changed UI for changing the icon sizes
or something else ?

-- 
Best regards/Schöne Grüße

Martin
A: Because it breaks the logical sequence of discussion
Q: Why is top posting bad?

()  ascii ribbon campaign - against html e-mail 
/\- against proprietary attachments

Frühstück, Geschenkideen, Accessoires, Kulinarisches: www.lillehus.at

D28796: Update Logtalk language syntax support

[Christoph Cullmann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:bf2664ea7907: Update Logtalk language syntax support 
(authored by cullmann).

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28796?vs=80017=81172

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

AFFECTED FILES
  data/syntax/logtalk.xml

To: pmoura, dhaumann, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, 
domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

KAcceleratorManager, Kirigami and accelerator key translations

[Shinjo Park]

Hello,

I have reported Bug #420488 and got to know that similar bugs have been 
reported such as #407386, #417467. This was supposed to be fixed by #406035 but 
the fix was not effective at all.

Inspired by this, I have checked how other languages are translating the 
accelerator keys and found this pattern:
- Languages using Latin, Cyrillic, Arabic (except Uyghur), Hebrew, Greek and 
some Indic scripts are placing accelerator keys within the translated message
- Languages using another Indic scripts, C/J/K are placing accelerator keys 
separate from the message (example: "New " is translated into "નવી 
વિન્ડો 
()", "ಹೊಸ ಕಿಟಕಿ()", "新建窗口()", "නව කවුළුව ()", "새 창()" in Gujarati, 
Kannada, Chinese Simplified, Sinhala, and Korean respectively)
- Source: https://l10n.kde.org/dictionary/compare-translations.php?
package=applications=dolphin.po=New+%26Window

To kde-i18n-doc: if I made some wrong assumptions, please correct them.

For the languages in the first list, I think KAcceleratorManager and Kirigami 
should work fine. For the languages in the second list, there are two problems: 
no accelerator keys are allocated in QWidget based applications when the 
translated string does not contain Latin alphabet within the string or 
followed by an ampersand. Also non typeable characters are misallocated 
accelerator keys in Kirigami based applications.

As a workaround for Kirigami displaying weird accelerator keys, I think 
extending the range of non-typeable characters to include abovementioned 
scripts can at least non-typeable accelerator keys from appearing:
https://cgit.kde.org/kirigami.git/tree/src/mnemonicattached.cpp#n92 However, 
this won't solve the problem that these strings won't have an accelerator key.

For the another problem of having no accelerator keys, I can think of two 
workarounds:
- Manually specify accelerator keys within the translated string. However, 
just by looking PO files I don't know which strings form a group, and some UI 
string which is supposed to have an accelerator key lacks of an accelerator 
key followed by an ampersand, so the suitable accelerator key has to be 
manually examined by translators.
- Allocate accelerator keys based on the original English string: That's what 
used by the current translations. I am not sure whether this workaround is 
possible in KDE Frameworks implementation (both KWidgetsAddons and Kirigami) 
wise or should be addressed in Qt.

I am looking for further opinions and inputs.

Best,
Shinjo

D28909: smb: port to Result system to force serialization of error/finish condition

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> sitter wrote in kio_smb.h:96
> Sure, if you think it's solid enough from an API POV.
> 
> I was thinking that we should amend the slavebase API for KF6 in general. 
> Instead of having error/finished/opened all functions on an API level should 
> return a Result and the slave loop would emit the relevant signal based on 
> the Result. IOW: what currently happens in the derived SlaveBases actually 
> ought to be KIO-internal.
> 
> That would then also allow us to get rid of the two-class split again. And 
> the "fronting" class is actually a much bigger concern than Result to me. The 
> call finalization logic is 100% code copy and so very easy to get wrong (e.g. 
> sftp's special() not finishing when in fact it should).

Excellent idea, but why wait for KF6?

We could implement a subclass of SlaveBase, in KIO, let's call it SlaveBaseV2, 
which

- Reimplements all the virtual methods from SlaveBase, in a private section
- Defines a new of virtual methods, called differently somehow
- Implements the signal emission based on the return value of those new virtual 
methods.

This allows for incremental porting of the slaves, instead of a big KF6 
breakage.
And the V2 class can be developed in a single slave first, before having to 
freeze its API.

REPOSITORY
  R320 KIO Extras

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

To: sitter, dfaure
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

building KF5 projects against a different Qt5 version (than the one the KF5 frameworks were built against)

[René J . V . Bertin]

Hi,

A priori Qt guarantees that you can run binaries against a different, newer Qt 
version than they were built against, as long as no private APIs are used. This 
also works if that newer Qt version is installed elsewhere, provided you set 
LD_LIBRARY_PATH correctly (and possibly LD_PRELOAD). I think this also means 
you can build code against such a newer (test) Qt install even if that code 
uses libraries which were built against the older Qt version (as long as no 
mixups occur at runtime; I've done this successfully with code that uses 
Phonon, for instance).

I'm not having a lot of luck building code that uses KF5 frameworks this way. 
Even after getting cmake to find the intended newer Qt version I still get 
header conflicts because something inserts unwanted header search paths.

I don't expect there to be official support for this kind of tricky things. 
Still, testing code against a new Qt version installed in parallel doesn't seem 
to be such an usual thing to do so if there is a trick to this I'd love to hear 
it. Is there a way to insert a `-isystem /path/to/new/qt/include` BEFORE the 
`-isystem /path/to/system/qt/include` that gets added by cmake, for instance?

Thanks,

R.

D28745: Skip caching thumbnails on encrypted filesystems

[Marcin Gurtowski]

marcingu marked 2 inline comments as done.
marcingu added a comment.


  Unless there's way to get rid of KMountPoint completely, this should reduce 
number of calls to minimum.

REPOSITORY
  R320 KIO Extras

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

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

D28745: Skip caching thumbnails on encrypted filesystems

[Marcin Gurtowski]

marcingu updated this revision to Diff 81167.
marcingu added a comment.


  Limiting usage of KMountPoint and lstat to max once per directory.

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28745?vs=80526=81167

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

AFFECTED FILES
  thumbnail/thumbnail.cpp
  thumbnail/thumbnail.h

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

D28796: Update Logtalk language syntax support

[Christoph Cullmann]

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


  Ok, I see, there is an extra request for the new hl test file.
  Then let's approve this one.

REPOSITORY
  R216 Syntax Highlighting

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

To: pmoura, dhaumann, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, rrosch, LeGast00n, cblack, 
domson, michaelh, ngraham, bruns, demsking, sars, dhaumann

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

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> bruns wrote in fakeopticaldisc.cpp:43
> Just use `0`.

It doesn't make the code more readable; and using QMap::constFind() while 
iterating is more recognizable/widely-used.

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

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

[Alexander Lohnau]

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

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

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

REPOSITORY
  R308 KRunner

BRANCH
  request_actions_once (branched from master)

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

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

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

D29171: Add 16px konversation icon

[Manuel Jesús de la Fuente]

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

REPOSITORY
  R266 Breeze Icons

BRANCH
  konversation16px (branched from master)

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

AFFECTED FILES
  icons-dark/apps/16/konversation.svg
  icons/apps/16/konversation.svg

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

D27216: [KProcessRunner] Improve error messages on failure

[David Faure]

dfaure added a comment.


  Done in D29170 . Now we can start 
discussing the other part of the bug, executable doesn't launch due to missing 
libs.
  
  Ah BTW the unittest leads to "cp" failing because it deletes the temporary 
dir under cp's feet. This could be fixed, but in any case, we don't want to pop 
up an error message on every process that terminates with a non-0 exit code. 
Imagine launching firefox, using it for 2 hours, then it crashes, and you get - 
in addition to drkonqi - that messagebox saying "It terminated abnormally. For 
more details try running it from a terminal emulator (e.g. Konsole)"   I 
think at most we want to let the job run for 5 more seconds to handle "crash on 
startup", but not react on longer-running programs that terminate abnormally.

REPOSITORY
  R241 KIO

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

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

D29170: Detect executables without +x permission in $PATH to improve error message

[David Faure]

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

REVISION SUMMARY
  QStandardPaths::findExecutable will not return to us a non-executable binary.
  So implement our own iteration over $PATH to detect such a case.
  Note: this doesn't handle the case where PATH isn't set at all 
(QStandardPaths implements a fallback)
  nor do we implement this for Windows (where chmod -x doesn't really exist as 
is). I think this is fine,
  in the worst case the user will get the other error message, program not 
found.

TEST PLAN
  'sudo chmod a-x /usr/bin/gwenview' then try opening a picture with gwenview 
from e.g. dolphin, see the error message
  
  CCBUG: 415567

REPOSITORY
  R241 KIO

BRANCH
  2020_04_findExecutable

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

AFFECTED FILES
  src/gui/kprocessrunner.cpp

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

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> broulik wrote in untrustedprogramhandlerinterface.h:79
> I was wondering if this should be done async? Nested event loops are quite a 
> problem when QML is involved.

I don't see a nested event loop in makeServiceFileExecutable.

I guess your comment was for the main method, showUntrustedProgramWarning?

Indeed we could make that one async, if I turn this interface into a QObject 
and add a signal.
Can do.

This kind of turns it into a job, but not really, we don't need this bit to 
have its own delegate etc.
I think a signal is enough?

> broulik wrote in applicationlauncherjob.cpp:117
> You know I'm not a fan of jobs suddenly blocking on IO :)

You know I'm not a fan of network mounts, exactly for this reason

In my opinion it's a crazy requirement to say that we are not allowed to use 
QFile or QFileInfo anywhere anymore, because of network mounts. What's your 
suggestion? I'm not even aware of an asynchronous (but still portable) 
equivalent.

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

D29153: Move handling of untrusted programs to ApplicationLauncherJob.

[Kai Uwe Broulik]

broulik added a comment.


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

INLINE COMMENTS

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

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

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

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

REPOSITORY
  R241 KIO

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

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

D27216: [KProcessRunner] Improve error messages on failure

[David Faure]

dfaure added a comment.


  Please ignore my previous comment. When launching gwenview by clicking on an 
image (and not on the gwenview executable itself), we indeed do end up with 
KProcessRunner failing to find the executable because of the missing -x.
  I'll implement the proper investigation and error message.

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, davidedmundson, broulik
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 81158.
dfaure added a comment.


  Also set a KIO::JobUiDelegate in KRun itself (which simplifies its code)

REPOSITORY
  R241 KIO

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

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/krun_p.h
  src/widgets/widgetsuntrustedprogramhandler.cpp
  src/widgets/widgetsuntrustedprogramhandler.h
  tests/kruntest.cpp

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

D27216: [KProcessRunner] Improve error messages on failure

[David Faure]

dfaure added a comment.


  Note that D29153  affects this since it 
will detect the lack of +x ahead of time in ApplicationLauncherJob and prompt 
the user [if the application uses KIOWidgets' JobUiDelegate -- oh I need to do 
this in KRun, will do right away].
  
  Technically this code path will still exist, but the (first) described 
testcase will not end up there anymore (once D29153 
 lands).

REPOSITORY
  R241 KIO

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

To: ahmadsamir, #frameworks, dfaure, davidedmundson, broulik
Cc: kde-frameworks-devel, LeGast00n, cblack, 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 81156.
saligari added a comment.


  Oh the shame! I had forgotten to center it (and I do know how to)!
  
  One thing I don't know is: since I centered it and used the guidelines my 1px 
line centered sits on a 0,5 to 0,5 line. I don't know if that's bad or how to 
change it. 
  Hope it's ok.
  
  Cleaned up the paths a bit since I noticed I had made a horrible union.
  
  Not including a preview since I don't think anything's actually different 
from my last revision visually

REPOSITORY
  R266 Breeze Icons

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

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