D7446: [Places panel] Revamp the Recently Saved section

[Méven Car]

meven added a comment.


  In D7446#526506 , @ngraham wrote:
  
  > In D7446#526494 , @meven wrote:
  >
  > > It would be great to have two different icons for recentlyused:/locations 
and recentlyused:/files.
  > >  Also I am not sure recentlyuserd:/ should use 
"document-open-recent-symbolic" since it is monochrone, the kio declares 
currently "document-open-recent".
  > >  Maybe "folder-temp" for /locations.
  > >  Any suggestion @ngraham ?
  >
  >
  > Good point. We probably need a new icon. `document-open-recent` is fine for 
recentlyused:/files, however for recentlyused:/locations we'll need a 
`folder-open-recent`  that uses the same style, but with a folder as the 
background instead of a file. Ideally both would also have appropriate colorful 
versions for the >22px sizes but that's not a hard requirement.
  >
  > There's plenty of time before the 19.12 release. Please file a bug to 
Breeze | Icons requesting the new icon and mention it in the VDG chatroom. 
Thanks!
  
  
  Thanks
  Done: https://bugs.kde.org/show_bug.cgi?id=411635

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, andreaska, gregormi, markg, alexeymin, broulik, elvisangelaccio, 
dfaure, davidedmundson, ltoscano, #konqueror, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

KDE CI: Frameworks » oxygen-icons5 » kf5-qt5 WindowsMSVCQt5.13 - Build # 5 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/oxygen-icons5/job/kf5-qt5%20WindowsMSVCQt5.13/5/
 Project:
kf5-qt5 WindowsMSVCQt5.13
 Date of build:
Fri, 06 Sep 2019 05:13:35 +
 Build duration:
13 min and counting
   JUnit Tests
  Name: projectroot Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)

D7446: [Places panel] Revamp the Recently Saved section

[Nathaniel Graham]

ngraham added a comment.


  In D7446#526494 , @meven wrote:
  
  > It would be great to have two different icons for recentlyused:/locations 
and recentlyused:/files.
  >  Also I am not sure recentlyuserd:/ should use 
"document-open-recent-symbolic" since it is monochrone, the kio declares 
currently "document-open-recent".
  >  Maybe "folder-temp" for /locations.
  >  Any suggestion @ngraham ?
  
  
  Good point. We probably need a new icon. `document-open-recent` is fine for 
recentlyused:/files, however for recentlyused:/locations we'll need a 
`folder-open-recent`  that uses the same style, but with a folder as the 
background instead of a file. Ideally both would also have appropriate colorful 
versions for the >22px sizes but that's not a hard requirement.
  
  There's plenty of time before the 19.12 release. Please file a bug to Breeze 
| Icons requesting the new icon and mention it in the VDG chatroom. Thanks!

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, andreaska, gregormi, markg, alexeymin, broulik, elvisangelaccio, 
dfaure, davidedmundson, ltoscano, #konqueror, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D21760: Add KListOpenFilesJob

[David Hallas]

hallas updated this revision to Diff 65494.
hallas added a comment.


  Review comments, renamed the files to match the class name

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21760?vs=65330&id=65494

BRANCH
  add_list_processes_with_open_files (branched from master)

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/klistopenfilesjobtest_unix.cpp
  autotests/klistopenfilesjobtest_unix.h
  autotests/klistopenfilesjobtest_win.cpp
  autotests/klistopenfilesjobtest_win.h
  src/lib/CMakeLists.txt
  src/lib/util/klistopenfilesjob.h
  src/lib/util/klistopenfilesjob_unix.cpp
  src/lib/util/klistopenfilesjob_win.cpp

To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns, #plasma
Cc: meven, cfeck, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D21760: Add KListOpenFilesJob

[David Hallas]

hallas marked an inline comment as done.
hallas added a comment.


  In D21760#525842 , @dfaure wrote:
  
  > Yes, the filenames should match the classname, obviously :)
  >
  > I did a deeper review of the KJob usage and I have two more comments, sorry 
for not taking the time to do this earlier...
  
  
  No problem ;) I would rather have this change go in slow and correct than 
quick and dirty :)

INLINE COMMENTS

> dfaure wrote in klistopenfiles_unix.cpp:57
> Emitting `result` twice would be a very bad idea, a KJob can only emit 
> `result` once.
> 
> But QProcess can emit `errorOccurred` followed by `finished` (e.g. if the 
> subprocess crashes, from what I can see in qprocess.cpp).
> I think we want to only qWarning() in this method, and let `lsofFinished` 
> take care of finishing the job.

Good catch! Actually it turns out to be a tad more complicated ;) QProcess will 
only emit finished if the process actually started, so if you have the case 
where lsof is not installed you will only get the errorOccurred signal and not 
finished, but if it crashes you get both. I actually added a unit test to test 
the first case (where lsof is not found), and I ended up making a solution 
where we simply remember if we have emitted the result and only emit it if we 
haven't done so before.

> dfaure wrote in klistopenfiles_unix.cpp:73
> The class would be slightly easier to use if this was a getter instead of a 
> signal.
> 
> Because then, in a unittest, you can just do `job->exec()` and then 
> `job->processInfoList()`, no spy needed.
> In (GUI) application code you still need to connect to a signal, but then the 
> usual KJob::result signal is enough, instead of two signals 
> (`processInfoAvailable` in case of success, and `result` to handle failure).
> 
> See `KIO::StatJob::statResult()` for an example.
> 
> Signals emitted by jobs are useful if they happen during the job lifetime 
> (e.g. progress signals). If they happen at the same time as `result`, then 
> it's easier to use `result` as the notification signal.
> 
> The downside is one more member variable, but it'll be very short-lived so I 
> wouldn't worry about memory usage.

Agree, I have refactored the code to do this and it is **much** simpler :)

REPOSITORY
  R244 KCoreAddons

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

To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns, #plasma
Cc: meven, cfeck, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

KDE CI: Frameworks » oxygen-icons5 » kf5-qt5 WindowsMSVCQt5.13 - Build # 4 - Still Failing!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks/job/oxygen-icons5/job/kf5-qt5%20WindowsMSVCQt5.13/4/
 Project:
kf5-qt5 WindowsMSVCQt5.13
 Date of build:
Fri, 06 Sep 2019 04:32:19 +
 Build duration:
7 min 23 sec and counting
   CONSOLE OUTPUT
  [...truncated 9131 lines...][2019-09-06T04:37:48.361Z] **[2019-09-06T04:37:48.361Z] ** Visual Studio 2019 Developer Command Prompt v16.0.3[2019-09-06T04:37:48.361Z] ** Copyright (c) 2019 Microsoft Corporation[2019-09-06T04:37:48.361Z] **[2019-09-06T04:37:50.718Z] [vcvarsall.bat] Environment initialized for: 'x64'[2019-09-06T04:38:21.489Z] 'kdeinit5' is not recognized as an internal or external command,[2019-09-06T04:38:21.489Z] operable program or batch file.[2019-09-06T04:38:22.090Z] Cannot find file: C:/CI/workspace/Frameworks/oxygen-icons5/kf5-qt5 WindowsMSVCQt5.13/build/DartConfiguration.tcl[2019-09-06T04:38:22.592Z]Site: [2019-09-06T04:38:22.592Z]Build name: (empty)[2019-09-06T04:38:22.592Z] Create new tag: 20190906-0438 - Experimental[2019-09-06T04:38:22.592Z] Cannot find file: C:/CI/workspace/Frameworks/oxygen-icons5/kf5-qt5 WindowsMSVCQt5.13/build/DartConfiguration.tcl[2019-09-06T04:38:22.592Z] Test project C:/CI/workspace/Frameworks/oxygen-icons5/kf5-qt5 WindowsMSVCQt5.13/build[2019-09-06T04:38:22.592Z] Start 1: symlink[2019-09-06T04:38:38.511Z] 1/3 Test #1: symlink ..   Passed   14.48 sec[2019-09-06T04:38:38.511Z] Start 2: newline[2019-09-06T04:38:39.011Z] 2/3 Test #2: newline ..   Passed1.45 sec[2019-09-06T04:38:39.011Z] Start 3: dupe[2019-09-06T04:38:39.011Z] 3/3 Test #3: dupe .   Passed0.06 sec[2019-09-06T04:38:39.011Z] [2019-09-06T04:38:39.011Z] 100% tests passed, 0 tests failed out of 3[2019-09-06T04:38:39.011Z] [2019-09-06T04:38:39.011Z] Total Test time (real) =  16.87 sec[2019-09-06T04:38:39.514Z] ERROR: The process "kded5.exe" not found.[2019-09-06T04:38:39.514Z] ERROR: The process "klauncher.exe" not found.[2019-09-06T04:38:40.022Z] ERROR: The process "kdeinit5.exe" not found.[2019-09-06T04:38:40.022Z] ERROR: The process "test_crasher.exe" not found.[2019-09-06T04:38:40.022Z] ERROR: The process "dbus-daemon.exe" not found.[2019-09-06T04:39:41.494Z] ERROR: The process "kioslave.exe" not found.[2019-09-06T04:39:41.494Z] ERROR: The process "vctip.exe" not found.[Pipeline] junit[2019-09-06T04:39:41.948Z] Recording test results[2019-09-06T04:39:42.054Z] Remote call on JNLP4-connect connection from kil-oss-2.ITS.Dal.Ca/192.75.96.253:51112 failed[Pipeline] }[Pipeline] // stage[Pipeline] }[2019-09-06T04:39:42.832Z] Also:   hudson.remoting.Channel$CallSiteStackTrace: Remote call to JNLP4-connect connection from kil-oss-2.ITS.Dal.Ca/192.75.96.253:51112[2019-09-06T04:39:42.832Z] 		at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1741)[2019-09-06T04:39:42.832Z] 		at hudson.remoting.UserRequest$ExceptionResponse.retrieve(UserRequest.java:356)[2019-09-06T04:39:42.832Z] 		at hudson.remoting.Channel.call(Channel.java:955)[2019-09-06T04:39:42.832Z] 		at hudson.FilePath.act(FilePath.java:1072)[2019-09-06T04:39:42.832Z] 		at hudson.FilePath.act(FilePath.java:1061)[2019-09-06T04:39:42.832Z] 		at hudson.tasks.junit.JUnitParser.parseResult(JUnitParser.java:114)[2019-09-06T04:39:42.832Z] 		at hudson.tasks.junit.JUnitResultArchiver.parse(JUnitResultArchiver.java:137)[2019-09-06T04:39:42.832Z] 		at hudson.tasks.junit.JUnitResultArchiver.parseAndAttach(JUnitResultArchiver.java:167)[2019-09-06T04:39:42.832Z] 		at hudson.tasks.junit.pipeline.JUnitResultsStepExecution.run(JUnitResultsStepExecution.java:52)[2019-09-06T04:39:42.832Z] 		at hudson.tasks.junit.pipeline.JUnitResultsStepExecution.run(JUnitResultsStepExecution.java:25)[2019-09-06T04:39:42.832Z] 		at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution.lambda$start$0(SynchronousNonBlockingStepExecution.java:47)[2019-09-06T04:39:42.832Z] 		at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)[2019-09-06T04:39:42.832Z] 		at java.util.concurrent.FutureTask.run(FutureTask.java:266)[2019-09-06T04:39:42.832Z] 		at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)[2019-09-06T04:39:42.832Z] 		at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)[2019-09-06T04:39:42.832Z] 		at java.lang.Thread.run(Thread.java:748)[2019-09-06T04:39:42.832Z] java.lang.NoClassDefFoundError: Could not initialize class jenkins.model.Jenkins[2019-09-06T04:39:42.832Z] 	at hudson.ExtensionList.lookup(ExtensionList.java:433)[2019-09-06T04:39:42.832Z] 	at hudson.tasks.junit.TestNameTransformer.all(TestNameTransformer.java:40)[2019-09-06T04:39:42.832Z] 	at hudson.tasks.junit.TestNameTransformer.getTransformedName(TestNameTransformer.java:33)[2019-09

KDE CI: Frameworks » oxygen-icons5 » kf5-qt5 WindowsMSVCQt5.13 - Build # 3 - Failure!

[CI System]

BUILD FAILURE
 Build URL
https://build.kde.org/job/Frameworks/job/oxygen-icons5/job/kf5-qt5%20WindowsMSVCQt5.13/3/
 Project:
kf5-qt5 WindowsMSVCQt5.13
 Date of build:
Fri, 06 Sep 2019 03:24:17 +
 Build duration:
1 hr 6 min and counting
   CONSOLE OUTPUT
  [...truncated 9133 lines...][2019-09-06T04:28:16.104Z] **[2019-09-06T04:28:16.104Z] ** Visual Studio 2019 Developer Command Prompt v16.0.3[2019-09-06T04:28:16.104Z] ** Copyright (c) 2019 Microsoft Corporation[2019-09-06T04:28:16.104Z] **[2019-09-06T04:28:18.519Z] [vcvarsall.bat] Environment initialized for: 'x64'[2019-09-06T04:29:10.190Z] 'kdeinit5' is not recognized as an internal or external command,[2019-09-06T04:29:10.190Z] operable program or batch file.[2019-09-06T04:29:10.190Z] Cannot find file: C:/CI/workspace/Frameworks/oxygen-icons5/kf5-qt5 WindowsMSVCQt5.13/build/DartConfiguration.tcl[2019-09-06T04:29:10.190Z]Site: [2019-09-06T04:29:10.190Z]Build name: (empty)[2019-09-06T04:29:10.190Z] Create new tag: 20190906-0429 - Experimental[2019-09-06T04:29:10.190Z] Cannot find file: C:/CI/workspace/Frameworks/oxygen-icons5/kf5-qt5 WindowsMSVCQt5.13/build/DartConfiguration.tcl[2019-09-06T04:29:10.190Z] Test project C:/CI/workspace/Frameworks/oxygen-icons5/kf5-qt5 WindowsMSVCQt5.13/build[2019-09-06T04:29:10.190Z] Start 1: symlink[2019-09-06T04:29:28.551Z] 1/3 Test #1: symlink ..   Passed   18.98 sec[2019-09-06T04:29:28.551Z] Start 2: newline[2019-09-06T04:29:29.052Z] 2/3 Test #2: newline ..   Passed1.99 sec[2019-09-06T04:29:29.052Z] Start 3: dupe[2019-09-06T04:29:29.052Z] 3/3 Test #3: dupe .   Passed0.13 sec[2019-09-06T04:29:29.052Z] [2019-09-06T04:29:29.052Z] 100% tests passed, 0 tests failed out of 3[2019-09-06T04:29:29.052Z] [2019-09-06T04:29:29.052Z] Total Test time (real) =  21.22 sec[2019-09-06T04:29:30.115Z] ERROR: The process "kded5.exe" not found.[2019-09-06T04:29:30.115Z] ERROR: The process "klauncher.exe" not found.[2019-09-06T04:29:30.115Z] ERROR: The process "kdeinit5.exe" not found.[2019-09-06T04:29:30.115Z] ERROR: The process "test_crasher.exe" not found.[2019-09-06T04:29:30.624Z] ERROR: The process "dbus-daemon.exe" not found.[2019-09-06T04:30:32.180Z] ERROR: The process "kioslave.exe" not found.[2019-09-06T04:30:32.180Z] ERROR: The process "vctip.exe" not found.[Pipeline] junit[2019-09-06T04:30:32.857Z] Recording test results[2019-09-06T04:30:41.717Z] Remote call on JNLP4-connect connection from kil-oss-2.ITS.Dal.Ca/192.75.96.253:51112 failed[Pipeline] }[Pipeline] // stage[Pipeline] }[2019-09-06T04:30:43.018Z] Also:   hudson.remoting.Channel$CallSiteStackTrace: Remote call to JNLP4-connect connection from kil-oss-2.ITS.Dal.Ca/192.75.96.253:51112[2019-09-06T04:30:43.018Z] 		at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1741)[2019-09-06T04:30:43.018Z] 		at hudson.remoting.UserRequest$ExceptionResponse.retrieve(UserRequest.java:356)[2019-09-06T04:30:43.018Z] 		at hudson.remoting.Channel.call(Channel.java:955)[2019-09-06T04:30:43.018Z] 		at hudson.FilePath.act(FilePath.java:1072)[2019-09-06T04:30:43.018Z] 		at hudson.FilePath.act(FilePath.java:1061)[2019-09-06T04:30:43.018Z] 		at hudson.tasks.junit.JUnitParser.parseResult(JUnitParser.java:114)[2019-09-06T04:30:43.018Z] 		at hudson.tasks.junit.JUnitResultArchiver.parse(JUnitResultArchiver.java:137)[2019-09-06T04:30:43.018Z] 		at hudson.tasks.junit.JUnitResultArchiver.parseAndAttach(JUnitResultArchiver.java:167)[2019-09-06T04:30:43.018Z] 		at hudson.tasks.junit.pipeline.JUnitResultsStepExecution.run(JUnitResultsStepExecution.java:52)[2019-09-06T04:30:43.018Z] 		at hudson.tasks.junit.pipeline.JUnitResultsStepExecution.run(JUnitResultsStepExecution.java:25)[2019-09-06T04:30:43.018Z] 		at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution.lambda$start$0(SynchronousNonBlockingStepExecution.java:47)[2019-09-06T04:30:43.018Z] 		at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)[2019-09-06T04:30:43.018Z] 		at java.util.concurrent.FutureTask.run(FutureTask.java:266)[2019-09-06T04:30:43.018Z] 		at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)[2019-09-06T04:30:43.018Z] 		at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)[2019-09-06T04:30:43.018Z] 		at java.lang.Thread.run(Thread.java:748)[2019-09-06T04:30:43.018Z] java.lang.NoClassDefFoundError: Could not initialize class jenkins.model.Jenkins[2019-09-06T04:30:43.018Z] 	at hudson.ExtensionList.lookup(ExtensionList.java:433)[2019-09-06T04:30:43.018Z] 	at hudson.tasks.junit.TestNameTransformer.all(TestNameTransformer.java:40)[2019-09-06T04:30:43.018Z] 	at hudson.tasks.junit.TestNameTransformer.getTransformedName(TestNameTransformer.java:33)[2019-09-0

KDE CI: Frameworks » kio » kf5-qt5 WindowsMSVCQt5.13 - Build # 28 - Successful!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20WindowsMSVCQt5.13/28/
 Project:
kf5-qt5 WindowsMSVCQt5.13
 Date of build:
Fri, 06 Sep 2019 04:19:57 +
 Build duration:
9 min 59 sec and counting

D7446: [Places panel] Revamp the Recently Saved section

[Méven Car]

meven added a comment.


  It would be great to have two different icons for recentlyused:/locations and 
recentlyused:/files.
  Also I am not sure recentlyuserd:/ should use "document-open-recent-symbolic" 
since it is monochrone, the kio declares currently "document-open-recent".
  Maybe "folder-temp" for /locations.
  Any suggestion @ngraham ?

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, andreaska, gregormi, markg, alexeymin, broulik, elvisangelaccio, 
dfaure, davidedmundson, ltoscano, #konqueror, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

KDE CI: Frameworks » kio » kf5-qt5 WindowsMSVCQt5.13 - Build # 27 - Aborted!

[CI System]

BUILD ABORTED
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20WindowsMSVCQt5.13/27/
 Project:
kf5-qt5 WindowsMSVCQt5.13
 Date of build:
Thu, 05 Sep 2019 13:46:09 +
 Build duration:
14 hr and counting

D23742: Add two special cases url recentlyused:/files and recentlyused:/folders

[Méven Car]

meven updated this revision to Diff 65492.
meven added a comment.


  Use Terms::Type::directories()

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23742?vs=65484&id=65492

BRANCH
  arcpatch-D23742

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

AFFECTED FILES
  recentlyused/recentlyused.cpp
  recentlyused/recentlyused.h

To: meven, ivan, ngraham, #frameworks
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D7446: [Places panel] Revamp the Recently Saved section

[Méven Car]

meven added a comment.


  @ngraham you added some dependencies but D23737 
 and D23741 
 are not direct dependencies, but rather 
related fixes.
  Still, I don't mind leaving them as dependencies.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, andreaska, gregormi, markg, alexeymin, broulik, elvisangelaccio, 
dfaure, davidedmundson, ltoscano, #konqueror, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23736: Add Term::Type::files() and Term::Type::directories() to filter only directories or excluding them

[Méven Car]

meven updated this revision to Diff 65491.
meven added a comment.


  Spacing

REPOSITORY
  R159 KActivities Statistics

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23736?vs=65490&id=65491

BRANCH
  arcpatch-D23736

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

AFFECTED FILES
  src/common/specialvalues.h
  src/resultset.cpp
  src/terms.cpp
  src/terms.h

To: meven, ivan
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D23736: Add Term::Type::files() and Term::Type::directories() to filter only directories or excluding them

[Méven Car]

meven updated this revision to Diff 65490.
meven added a comment.


  Rename constant LOCATIONS_TYPE_TAG -> DIRECTORIES_TYPE_TAG bis

REPOSITORY
  R159 KActivities Statistics

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23736?vs=65489&id=65490

BRANCH
  arcpatch-D23736

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

AFFECTED FILES
  src/common/specialvalues.h
  src/resultset.cpp
  src/terms.cpp
  src/terms.h

To: meven, ivan
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D23736: Add Term::Type::files() and Term::Type::directories() to filter only directories or excluding them

[Méven Car]

meven updated this revision to Diff 65489.
meven added a comment.


  Rename constant LOCATIONS_TYPE_TAG -> DIRECTORIES_TYPE_TAG

REPOSITORY
  R159 KActivities Statistics

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23736?vs=65488&id=65489

BRANCH
  arcpatch-D23736

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

AFFECTED FILES
  src/common/specialvalues.h
  src/resultset.cpp
  src/terms.cpp
  src/terms.h

To: meven, ivan
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D23736: Add Term::Type::files() and Term::Type::directories() to filter only directories or excluding them

[Méven Car]

meven retitled this revision from "Add Term::Type::files() and 
Term::Type::directories() to filter only directories or excluding then" to "Add 
Term::Type::files() and Term::Type::directories() to filter only directories or 
excluding them".

REPOSITORY
  R159 KActivities Statistics

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

To: meven, ivan
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D23736: Add Term::Type::files() and Term::Type::directories() to filter only directories or excluding then

[Méven Car]

meven retitled this revision from "Add Term::Type::files() and 
Term::Type::locations() to filter excluding or only directories" to "Add 
Term::Type::files() and Term::Type::directories() to filter only directories or 
excluding then".

REPOSITORY
  R159 KActivities Statistics

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

To: meven, ivan
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D23736: Add Term::Type::files() and Term::Type::locations() to filter excluding or only directories

[Méven Car]

meven updated this revision to Diff 65488.
meven added a comment.


  Rename locations to directories

REPOSITORY
  R159 KActivities Statistics

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23736?vs=65485&id=65488

BRANCH
  arcpatch-D23736

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

AFFECTED FILES
  src/common/specialvalues.h
  src/resultset.cpp
  src/terms.cpp
  src/terms.h

To: meven, ivan
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

Re: Proposing Quick Charts as a new framework

[Aleix Pol]

On Thu, Sep 5, 2019 at 10:53 PM Arjen Hiemstra  wrote:
>
> On 02-09-2019 19:26, Luigi Toscano wrote:
> > Arjen Hiemstra ha scritto:
> >> Hi,
> >>
> >> I have been working on a library the past few months that provides a
> >> QtQuick
> >> module for rendering gpu-accelerated charts. It currently lives in a
> >> playground
> >> repository, here: https://invent.kde.org/kde/kf5quickcharts . I would
> >> like for
> >> this library to be included as a Tier 1 Framework.
> >
> > Hi,
> > I've noticed that some time ago (I didn't enable the translations yet,
> > but
> > maybe it's not needed).
>
> There are only user-visible strings in the example application. I
> haven't marked them
> to be translated since it is only an example.
>
> > I have a question and a comment:
> >
> > - what is the difference between this one and the existing
> > kqtquickcharts,
> > part of the KDE Applications bundle, from the KDEEdu project? At least
> > their
> > description overlaps. Is there any transition path? Having two
> > duplicate
> > libraries with the exact same goal may not be the best scenario.
>
> The main difference is in the rendering methods: kqtquickcharts uses
> QPainter for
> rendering, Quick Charts uses a technique called Signed Distance Fields
> to render
> certain charts with GPU acceleration and others use pure QtQuick
> scenegraph. I
> don't know if it would have been possible to retrofit that on to
> kqtquickcharts,
> but my guess is it would have been a major overhaul of that library
> anyway.
>
> An additional issue is that kqtquickcharts seems to be unmaintained,
> with the last
> major feature work having been done several years ago. So my personal
> opinion is
> that when Quick Charts is accepted as part of Frameworks, we deprecate
> kqtquickcharts as well as some other bits like the plotter item from
> kdeclarative.

Does it really have the same features? Otherwise it doesn't make sense
to deprecate anything. It will just frustrate the users of the
framework.

> > - I don't think that "5" should be part of the name; in two years from
> > now we
> > may have a Frameworks 6, and having a kf5quickchart as part of it may
> > be
> > confusing.
>
> That's actually a good point, though the kf5 is only in the repository
> name, I
> name it "Quick Charts" everywhere else. Which is probably a good reason
> to have
> the repo name changed in the first place. :) Originally, I put kf5 in
> the repo
> name because that's what is used once installed as part of Frameworks,
> but I
> agree that it can lead to confusing things.

It should be called kquickchart (which indeed is far too similar to
kqtquickcharts). Much like you use KF5CoreAddons, but the repository
is called kcoreaddons.

Aleix

D23708: [CopyJob] Fix crash when copying an already existing dir and pressing "Skip"

[Ahmad Samir]

ahmadsamir marked an inline comment as done.
ahmadsamir added a comment.


  In D23708#525848 , @dfaure wrote:
  
  > Thanks for the fix!
  >
  > Do you feel like writing a unittest for it?
  >  JobTest::chmodFileError shows how to use PredefinedAnswerJobUiDelegate to 
simulate the user pressing "Skip" in the dialog.
  >  Write the test without your fix applied, so that you are sure it triggers 
the crash, before re-applying the fix.
  
  
  That's exactly what I did.
  
  Thanks.

INLINE COMMENTS

> dfaure wrote in copyjob.cpp:1640
> This comment only needs to be in the unittest, not in the code.
> 
> If every bugfix leads to two lines of comments, the code becomes unreadable.

OK.

REPOSITORY
  R241 KIO

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

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

D23708: [CopyJob] Fix crash when copying an already existing dir and pressing "Skip"

[Ahmad Samir]

ahmadsamir updated this revision to Diff 65486.
ahmadsamir retitled this revision from "[CopyJob] Fix crash when copying all 
files is skipped for an already existing dir" to "[CopyJob] Fix crash when 
copying an already existing dir and pressing "Skip"".
ahmadsamir edited the summary of this revision.
ahmadsamir edited the test plan for this revision.
ahmadsamir added a comment.


  Add a unit test
  Move the comments to the unit test

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23708?vs=65336&id=65486

BRANCH
  ahmad/copyjob (branched from master)

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

AFFECTED FILES
  autotests/jobtest.cpp
  autotests/jobtest.h
  src/core/copyjob.cpp

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

D23736: Add Term::Type::files() and Term::Type::locations() to filter excluding or only directories

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> ivan wrote in terms.h:106
> Can we call it `directories` to follow the Qt API? That is the only complaint 
> I have, otherwise thie is quite a nice addition.

+1, directories is fine for the code, and we can just surface the word 
"locations" in the UI.

REPOSITORY
  R159 KActivities Statistics

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

To: meven, ivan
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

Re: Proposing Quick Charts as a new framework

[Arjen Hiemstra]

On 02-09-2019 19:26, Luigi Toscano wrote:

Arjen Hiemstra ha scritto:

Hi,

I have been working on a library the past few months that provides a 
QtQuick
module for rendering gpu-accelerated charts. It currently lives in a 
playground
repository, here: https://invent.kde.org/kde/kf5quickcharts . I would 
like for

this library to be included as a Tier 1 Framework.


Hi,
I've noticed that some time ago (I didn't enable the translations yet, 
but

maybe it's not needed).


There are only user-visible strings in the example application. I 
haven't marked them

to be translated since it is only an example.


I have a question and a comment:

- what is the difference between this one and the existing 
kqtquickcharts,
part of the KDE Applications bundle, from the KDEEdu project? At least 
their
description overlaps. Is there any transition path? Having two 
duplicate

libraries with the exact same goal may not be the best scenario.


The main difference is in the rendering methods: kqtquickcharts uses 
QPainter for
rendering, Quick Charts uses a technique called Signed Distance Fields 
to render
certain charts with GPU acceleration and others use pure QtQuick 
scenegraph. I
don't know if it would have been possible to retrofit that on to 
kqtquickcharts,
but my guess is it would have been a major overhaul of that library 
anyway.


An additional issue is that kqtquickcharts seems to be unmaintained, 
with the last
major feature work having been done several years ago. So my personal 
opinion is

that when Quick Charts is accepted as part of Frameworks, we deprecate
kqtquickcharts as well as some other bits like the plotter item from 
kdeclarative.


- I don't think that "5" should be part of the name; in two years from 
now we
may have a Frameworks 6, and having a kf5quickchart as part of it may 
be

confusing.


That's actually a good point, though the kf5 is only in the repository 
name, I
name it "Quick Charts" everywhere else. Which is probably a good reason 
to have
the repo name changed in the first place. :) Originally, I put kf5 in 
the repo
name because that's what is used once installed as part of Frameworks, 
but I

agree that it can lead to confusing things.

Cheers,
Arjen



Ciao

D23736: Add Term::Type::files() and Term::Type::locations() to filter excluding or only directories

[Ivan Čukić]

ivan added inline comments.

INLINE COMMENTS

> terms.h:106
> + */
> +static Type locations();
>  

Can we call it `directories` to follow the Qt API? That is the only complaint I 
have, otherwise thie is quite a nice addition.

REPOSITORY
  R159 KActivities Statistics

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

To: meven, ivan
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23736: Add Term::Type::files() and Term::Type::locations() to filter excluding or only directories

[Méven Car]

meven retitled this revision from "Add a Term::Type::files() to filter 
excluding directories" to "Add Term::Type::files() and Term::Type::locations() 
to filter excluding or only directories".

REPOSITORY
  R159 KActivities Statistics

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

To: meven, ivan
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23736: Add a Term::Type::files() to filter excluding directories

[Méven Car]

meven updated this revision to Diff 65485.
meven added a comment.


  Add Term::Type::locations

REPOSITORY
  R159 KActivities Statistics

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23736?vs=65446&id=65485

BRANCH
  master

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

AFFECTED FILES
  src/common/specialvalues.h
  src/resultset.cpp
  src/terms.cpp
  src/terms.h

To: meven, ivan
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23742: Add two special cases url recentlyused:/files and recentlyused:/folders

[Méven Car]

meven updated this revision to Diff 65484.
meven added a comment.


  grammar

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23742?vs=65474&id=65484

BRANCH
  master

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

AFFECTED FILES
  recentlyused/recentlyused.cpp
  recentlyused/recentlyused.h

To: meven, ivan, ngraham, #frameworks
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23742: Add two special cases url recentlyused:/files and recentlyused:/folders

[Méven Car]

meven updated this revision to Diff 65474.
meven added a comment.


  typo

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23742?vs=65473&id=65474

BRANCH
  master

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

AFFECTED FILES
  recentlyused/recentlyused.cpp
  recentlyused/recentlyused.h

To: meven, ivan, ngraham, #frameworks
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23742: Add two special cases url recentlyused:/files and recentlyused:/folders

[Méven Car]

meven updated this revision to Diff 65473.
meven added a comment.


  Replace /folders by /locations, add some documentation

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23742?vs=65470&id=65473

BRANCH
  master

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

AFFECTED FILES
  recentlyused/recentlyused.cpp
  recentlyused/recentlyused.h

To: meven, ivan, ngraham, #frameworks
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23745: [server] Make double-buffered properties in xdg-shell double-buffered

[Vlad Zagorodniy]

zzag added a task: T11480: Make xdg-shell properties double-buffered.

REPOSITORY
  R127 KWayland

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

To: zzag, #kwin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23745: [server] Make double-buffered properties in xdg-shell double-buffered

[Vlad Zagorodniy]

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

REVISION SUMMARY
  So far all double-buffered properties in our implementation of xdg-shell
  weren't actually double-buffered. When a property setter is invoked, we
  pray to the God hoping that the client committed associated surface.
  
  This change introduces private SurfaceRole class. The new class provides
  a way for SurfaceInterface to commit pending state of associated shell
  surface.
  
  The chosen architecture allows us to do more in the future. For example,
  we could use SurfaceRole to prevent associating several roles to a single
  wl_surface object, e.g. xdg-toplevel to a pointer surface, etc.

TEST PLAN
  This change breaks support for client-side decorated clients in KWin.

REPOSITORY
  R127 KWayland

BRANCH
  surface-role

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

AFFECTED FILES
  autotests/client/test_xdg_shell_stable.cpp
  autotests/client/test_xdg_shell_v6.cpp
  src/server/CMakeLists.txt
  src/server/generic_shell_surface_p.h
  src/server/shell_interface.cpp
  src/server/surface_interface.cpp
  src/server/surface_interface.h
  src/server/surface_interface_p.h
  src/server/surfacerole.cpp
  src/server/surfacerole_p.h
  src/server/xdgshell_interface.cpp
  src/server/xdgshell_interface.h
  src/server/xdgshell_interface_p.h
  src/server/xdgshell_stable_interface.cpp
  src/server/xdgshell_v5_interface.cpp
  src/server/xdgshell_v6_interface.cpp

To: zzag, #kwin
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23742: Add two special cases url recentlyused:/files and recentlyused:/folders

[Nathaniel Graham]

ngraham added a dependent revision: D7446: [Places panel] Revamp the Recently 
Saved section.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: meven, ivan, ngraham, #frameworks
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D7446: [Places panel] Revamp the Recently Saved section

[Nathaniel Graham]

ngraham edited the summary of this revision.
ngraham added dependencies: D23742: Add two special cases url 
recentlyused:/files and recentlyused:/folders, D23737: Extract mimetype from 
recentlyused.xbel, D23741: Save the Mimetype of opened resources.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, andreaska, gregormi, markg, alexeymin, broulik, elvisangelaccio, 
dfaure, davidedmundson, ltoscano, #konqueror, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23742: Add two special cases url recentlyused:/files and recentlyused:/folders

[Méven Car]

meven updated this revision to Diff 65470.
meven added a comment.


  Add some const

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23742?vs=65467&id=65470

BRANCH
  master

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

AFFECTED FILES
  recentlyused/recentlyused.cpp
  recentlyused/recentlyused.h

To: meven, ivan, ngraham, #frameworks
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23742: Add two special cases url recentlyused:/files and recentlyused:/folders

[Ivan Čukić]

ivan accepted this revision.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: meven, ivan, ngraham, #frameworks
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D22069: Localize long number strings

[Nathaniel Graham]

ngraham added a comment.


  In D22069#526291 , @aacid wrote:
  
  > But i guess @huftis is right, the test is not setting the language "enough" 
to be french, just "a bit" (there's too many layers of stuff to set) and it 
just works for me because i'm also in a locale tht uses , like the french
  
  
  Apologies for my ignorance but how do I do that?

REPOSITORY
  R249 KI18n

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

To: ngraham, #localization, #frameworks, broulik, aacid
Cc: dfaure, aacid, huftis, safaalfulaij, mikeroyal, aspotashev, ilic, 
kde-frameworks-devel, broulik, LeGast00n, GB_2, michaelh, ngraham, bruns

D7446: [Places panel] Revamp the Recently Saved section

[Nathaniel Graham]

ngraham added a comment.


  In D7446#526304 , @meven wrote:
  
  > In D7446#526201 , @ngraham wrote:
  >
  > > Fantastic work. That's exactly what I had in mind and it works perfectly!
  > >
  > > With those, we can have Recent Files and Recent Locations (or maybe 
Recent Folders) in the Recent section, and we can get rid of the existing Today 
entry. That will make this patch //perfect.//
  >
  >
  > +1 for Recent Locations
  >
  > Will replace the entry recentlyused:/ by recentlyused:/files aka "Recent 
files" and recentlyused:/folders aka "Recent Locations".
  
  
  Yay!

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, andreaska, gregormi, markg, alexeymin, broulik, elvisangelaccio, 
dfaure, davidedmundson, ltoscano, #konqueror, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23739: Fix uninitialized memory read

[Albert Astals Cid]

aacid closed this revision.

REPOSITORY
  R287 KImageFormats

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

To: aacid, dfaure, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23742: Add two special cases url recentlyused:/files and recentlyused:/folders

[Méven Car]

meven updated this revision to Diff 65467.
meven added a comment.


  Rephrase 'Recent Folders' to 'Recent Locations'

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23742?vs=65449&id=65467

BRANCH
  master

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

AFFECTED FILES
  recentlyused/recentlyused.cpp
  recentlyused/recentlyused.h

To: meven, ivan, ngraham, #frameworks
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D7446: [Places panel] Revamp the Recently Saved section

[Méven Car]

meven added a comment.


  In D7446#526201 , @ngraham wrote:
  
  > Fantastic work. That's exactly what I had in mind and it works perfectly!
  >
  > With those, we can have Recent Files and Recent Locations (or maybe Recent 
Folders) in the Recent section, and we can get rid of the existing Today entry. 
That will make this patch //perfect.//
  
  
  +1 for Recent Locations
  
  Will replace the entry recentlyused:/ by recentlyused:/files aka "Recent 
files" and recentlyused:/folders aka "Recent Locations".

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, anthonyfieroni, andreaska, gregormi, markg, alexeymin, broulik, 
elvisangelaccio, dfaure, davidedmundson, ltoscano, #konqueror, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D23739: Fix uninitialized memory read

[Volker Krause]

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

REPOSITORY
  R287 KImageFormats

BRANCH
  master

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

To: aacid, dfaure, apol, vkrause
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D22069: Localize long number strings

[Albert Astals Cid]

aacid added a comment.


  But i guess @huftis is right, the test is not setting the language "enough" 
to be french, just "a bit" (there's too many layers of stuff to set) and it 
just works for me because i'm also in a locale tht uses , like the french

REPOSITORY
  R249 KI18n

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

To: ngraham, #localization, #frameworks, broulik, aacid
Cc: dfaure, aacid, huftis, safaalfulaij, mikeroyal, aspotashev, ilic, 
kde-frameworks-devel, broulik, LeGast00n, GB_2, michaelh, ngraham, bruns

D22069: Localize long number strings

[Albert Astals Cid]

aacid added a comment.


  In D22069#526284 , @ngraham wrote:
  
  > In D22069#526278 , @aacid wrote:
  >
  > > In D22069#526243 , @ngraham 
wrote:
  > >
  > > > That's the same failure I have too, but @aacid said it was because my 
system was mis-configured. I don't really understand the issue, I'm afraid.
  > >
  > >
  > > Are you really saying that when i said "this is because you don't have 
the french stuff installed" you didn't install it to check i was right and you 
just trusted me?
  >
  >
  > Yes, I interpreted your "Accepted" status to mean that you had tested the 
patch yourself in your capacity as the patch reviewer and had concluded that it 
worked. My apologies if I misinterpreted this and it was not in fact the case.
  >
  > I actually don't know how to properly install other language stuff. When I 
add French in the languages KCM, I suffer from 
https://bugs.kde.org/show_bug.cgi?id=327757.
  >
  > What is the correct way to install the language?
  
  
  On archlinux you uncomment the language you want in /etc/locale.gen and then 
run locale-gen

REPOSITORY
  R249 KI18n

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

To: ngraham, #localization, #frameworks, broulik, aacid
Cc: dfaure, aacid, huftis, safaalfulaij, mikeroyal, aspotashev, ilic, 
kde-frameworks-devel, broulik, LeGast00n, GB_2, michaelh, ngraham, bruns

D22069: Localize long number strings

[Nathaniel Graham]

ngraham added a comment.


  In D22069#526278 , @aacid wrote:
  
  > In D22069#526243 , @ngraham 
wrote:
  >
  > > That's the same failure I have too, but @aacid said it was because my 
system was mis-configured. I don't really understand the issue, I'm afraid.
  >
  >
  > Are you really saying that when i said "this is because you don't have the 
french stuff installed" you didn't install it to check i was right and you just 
trusted me?
  
  
  Yes, I interpreted your "Accepted" status to mean that you had tested the 
patch yourself in your capacity as the patch reviewer and had concluded that it 
worked. My apologies if I misinterpreted this and it was not in fact the case.
  
  I actually don't know how to properly install other language stuff. When I 
add French in the languages KCM, I suffer from 
https://bugs.kde.org/show_bug.cgi?id=327757.
  
  What is the correct way to install the language?

REPOSITORY
  R249 KI18n

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

To: ngraham, #localization, #frameworks, broulik, aacid
Cc: dfaure, aacid, huftis, safaalfulaij, mikeroyal, aspotashev, ilic, 
kde-frameworks-devel, broulik, LeGast00n, GB_2, michaelh, ngraham, bruns

D22069: Localize long number strings

[Albert Astals Cid]

aacid added a comment.


  In D22069#526243 , @ngraham wrote:
  
  > That's the same failure I have too, but @aacid said it was because my 
system was mis-configured. I don't really understand the issue, I'm afraid.
  
  
  Are you really saying that when i said "this is because you don't have the 
french stuff installed" you didn't install it to check i was right and you just 
trusted me?

REPOSITORY
  R249 KI18n

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

To: ngraham, #localization, #frameworks, broulik, aacid
Cc: dfaure, aacid, huftis, safaalfulaij, mikeroyal, aspotashev, ilic, 
kde-frameworks-devel, broulik, LeGast00n, GB_2, michaelh, ngraham, bruns

D22069: Localize long number strings

[Karl Ove Hufthammer]

huftis added a comment.


  In D22069#526243 , @ngraham wrote:
  
  > That's the same failure I have too, but @aacid said it was because my 
system was mis-configured. I don't really understand the issue, I'm afraid.
  
  
  On my system, the test passes with my default locale, but fails (with the 
same error) if I have `LC_ALL` set to English:
  
LC_ALL=en_US.utf8 ./ki18n-klocalizedstringtest 
  
  Shouldn’t the test override the `LC_ALL` settings? (I have `export 
LC_ALL=nn_NO.utf8` (along with similar commands for `LANG` and `LANGUAGE`) in 
my `.bashrc` to ensure that I get all applications in my language.)

REPOSITORY
  R249 KI18n

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

To: ngraham, #localization, #frameworks, broulik, aacid
Cc: dfaure, aacid, huftis, safaalfulaij, mikeroyal, aspotashev, ilic, 
kde-frameworks-devel, broulik, LeGast00n, GB_2, michaelh, ngraham, bruns

D22069: Localize long number strings

[Nathaniel Graham]

ngraham added a comment.


  That's the same failure I have too, but @aacid said it was because my system 
was mis-configured. I don't really understand the issue, I'm afraid.

REPOSITORY
  R249 KI18n

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

To: ngraham, #localization, #frameworks, broulik, aacid
Cc: dfaure, aacid, huftis, safaalfulaij, mikeroyal, aspotashev, ilic, 
kde-frameworks-devel, broulik, LeGast00n, GB_2, michaelh, ngraham, bruns

D23698: document: Port away from foreach loops over members without calls to owner

[Christoph Cullmann]

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


  Looks ok.

REPOSITORY
  R39 KTextEditor

BRANCH
  portawayfromforeachforitemconstructionestimation_document

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

To: kossebau, #kate, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, 
michaelh, ngraham, bruns, demsking, sars, dhaumann

D7446: [Places panel] Revamp the Recently Saved section

[Nathaniel Graham]

ngraham added a comment.


  Fantastic work. That's exactly what I had in mind and it works perfectly!
  
  With those, we can have Recent Files and Recent Locations (or maybe Recent 
Folders) in the Recent section, and we can get rid of the existing Today entry. 
That will make this patch //perfect.//

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, anthonyfieroni, andreaska, gregormi, markg, alexeymin, broulik, 
elvisangelaccio, dfaure, davidedmundson, ltoscano, #konqueror, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D23742: Add two special cases url recentlyused:/files and recentlyused:/folders

[Nathaniel Graham]

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


  After applying the full patchset, this works great. It's //exactly// what I 
was hoping for! 👏

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

To: meven, ivan, ngraham, #frameworks
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23736: Add a Term::Type::files() to filter excluding directories

[Ivan Čukić]

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


  Add the same for directories for completeness

REPOSITORY
  R159 KActivities Statistics

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

To: meven, ivan
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D7446: [Places panel] Revamp the Recently Saved section

[Méven Car]

meven added a comment.


  In D7446#526039 , @meven wrote:
  
  > In D7446#526032 , @ngraham wrote:
  >
  > > This works great.
  > >
  > > one minor concern I have with the new IOSlave in general is how it shows 
both files and folders. For me, the folders are a distraction from the file 
listing. But I was thinking: how about adding modes that allow it to show only 
files or only folders? Like the IOSlave could support `recentlyused:/files` and 
`recentlyused:/folders`, each of which would limit the display to just those 
things. Then under the "Recent" section, we could have "Recent Files" and 
"Recent Locations", which would allow us to remove the useless old "Today" 
entry without any trouble.
  > >
  > > This would provide a better fix for 
https://bugs.kde.org/show_bug.cgi?id=159299.
  >
  >
  > Great suggestions, I will add those to the kio slave.
  
  
  D23742  is here, depends on D23736 
, also two fix to make sure we have mime 
type of files in the database : fix to kactivitymanagerd D23737 
 when getting history from gtk apps and 
D23741  from gwenview

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, anthonyfieroni, andreaska, gregormi, markg, alexeymin, broulik, 
elvisangelaccio, dfaure, davidedmundson, ltoscano, #konqueror, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D23742: Add two special cases url recentlyused:/files and recentlyused:/folders

[Méven Car]

meven created this revision.
meven added reviewers: ivan, ngraham, Frameworks.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
meven requested review of this revision.

REVISION SUMMARY
  Depends on D23736 

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

AFFECTED FILES
  recentlyused/recentlyused.cpp
  recentlyused/recentlyused.h

To: meven, ivan, ngraham, #frameworks
Cc: kde-frameworks-devel, kfm-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23736: Add a Term::Type::files() to filter excluding directories

[Méven Car]

meven added a dependent revision: D23742: Add two special cases url 
recentlyused:/files and recentlyused:/folders.

REPOSITORY
  R159 KActivities Statistics

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

To: meven, ivan
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23738: Add 22px dialog-scripts, change script actions/places icons to match it

[Noah Davis]

This revision was automatically updated to reflect the committed changes.
Closed by commit R266:939cbf897133: Add 22px dialog-scripts, change script 
actions/places icons to match it (authored by ndavis).

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23738?vs=65438&id=65450

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

AFFECTED FILES
  icons-dark/actions/16/amarok_scripts.svg
  icons-dark/actions/22/amarok_scripts.svg
  icons-dark/actions/22/dialog-scripts.svg
  icons-dark/places/16/folder-script.svg
  icons-dark/places/22/folder-script.svg
  icons/actions/16/amarok_scripts.svg
  icons/actions/22/amarok_scripts.svg
  icons/actions/22/dialog-scripts.svg
  icons/places/16/folder-script.svg
  icons/places/22/folder-script.svg

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

D23736: Add a Term::Type::files() to filter excluding directories

[Méven Car]

meven updated this revision to Diff 65446.
meven added a comment.


  Exclude files whose mimetype is not known from the files filter

REPOSITORY
  R159 KActivities Statistics

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23736?vs=65433&id=65446

BRANCH
  master

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

AFFECTED FILES
  src/common/specialvalues.h
  src/resultset.cpp
  src/terms.cpp
  src/terms.h

To: meven, ivan
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/53/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Thu, 05 Sep 2019 15:38:02 +
 Build duration:
7 min 39 sec and counting
   JUnit Tests
  Name: projectroot Failed: 3 test(s), Passed: 49 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: 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)

D23738: Add 22px dialog-scripts, change script actions/places icons to match it

[Nathaniel Graham]

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

REPOSITORY
  R266 Breeze Icons

BRANCH
  scripts-box-shape (branched from master)

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

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

D23738: Add 22px dialog-scripts, change script actions/places icons to match it

[Noah Davis]

ndavis added a dependent revision: D23740: Change terminal panel icon to 
dialog-scripts.

REPOSITORY
  R266 Breeze Icons

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

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

D23738: Add 22px dialog-scripts, change script actions/places icons to match it

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D23420: Use solid to check if a KFileItem is located on a network mount

[Méven Car]

meven added a comment.


  I didn't know about `KMount`
  
  Gwenview uses it as we are trying here :
  
bool urlIsFastLocalFile(const QUrl &url)
{
if (!url.isLocalFile()) {
return false;
}

KMountPoint::List list = KMountPoint::currentMountPoints();
KMountPoint::Ptr mountPoint = list.findByPath(url.toLocalFile());
if (!mountPoint) {
// We couldn't find a mount point for the url. We are probably in a
// chroot. Assume everything is fast then.
return true;
}

return !mountPoint->probablySlow();
}

REPOSITORY
  R241 KIO

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

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

D23738: Add 22px dialog-scripts, change script actions/places icons to match it

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D23739: Fix uninitialized memory read

[Albert Astals Cid]

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

REVISION SUMMARY
  Make sure whole of pixel_size in pixel has data either because it was
  read or because we set it to 0
  
  oss-fuzz/14565

REPOSITORY
  R287 KImageFormats

BRANCH
  master

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

AFFECTED FILES
  src/imageformats/tga.cpp

To: aacid, dfaure, apol
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23738: Add 22px dialog-scripts, change script actions/places icons to match it

[Noah Davis]

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

REVISION SUMMARY
  The old ">_" style without the box around it looked odd and the margins were 
wrong.

REPOSITORY
  R266 Breeze Icons

BRANCH
  scripts-box-shape (branched from master)

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

AFFECTED FILES
  icons-dark/actions/16/amarok_scripts.svg
  icons-dark/actions/22/amarok_scripts.svg
  icons-dark/actions/22/dialog-scripts.svg
  icons-dark/places/16/folder-script.svg
  icons-dark/places/22/folder-script.svg
  icons/actions/16/amarok_scripts.svg
  icons/actions/22/amarok_scripts.svg
  icons/actions/22/dialog-scripts.svg
  icons/places/16/folder-script.svg
  icons/places/22/folder-script.svg

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

D23730: Add new protocol for 7z archives

[Nathaniel Graham]

ngraham added a comment.


  In D23730#526045 , @nhiga wrote:
  
  > @meven
  >
  > There is a weird bug if I name the protocol as "7z" or "7zip":
  >
  > 1. If I double click on the 7z file, Dolphin gives the "invalid protocol" 
error.
  > 2. If I try to manually enter the URL in Dolphin, i.e. `7z:/tmp/bar.7z` or 
`7zip:/tmp/bar.7z`, it does not work too - it is somehow being treated as a 
host (hostname) instead of a protocol.
  >
  >   I suspect that a KIO protocol should not have a name that starts with a 
number, otherwise there will be bugs.
  
  
  Sounds like we should fix that.

REPOSITORY
  R320 KIO Extras

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

To: nhiga, meven
Cc: ltoscano, meven, ngraham, kfm-devel, kde-frameworks-devel, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, 
mikesomov

D23736: Add a Term::Type::files() to filter excluding directories

[Méven Car]

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

REPOSITORY
  R159 KActivities Statistics

BRANCH
  master

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

AFFECTED FILES
  src/common/specialvalues.h
  src/resultset.cpp
  src/terms.cpp
  src/terms.h

To: meven, ivan
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23730: Add new protocol for 7z archives

[N. Higa]

nhiga added a comment.


  @meven
  
  There is a weird bug if I name the protocol as "7z" or "7zip":
  
  1. If I double click on the 7z file, Dolphin gives the "invalid protocol" 
error.
  2. If I try to manually enter the URL in Dolphin, i.e. `7z:/tmp/bar.7z` or 
`7zip:/tmp/bar.7z`, it does not work too - it is somehow being treated as a 
host (hostname) instead of a protocol.
  
  I suspect that a KIO protocol should not have a name that starts with a 
number, otherwise there will be bugs.

REPOSITORY
  R320 KIO Extras

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

To: nhiga, meven
Cc: ltoscano, meven, ngraham, kfm-devel, kde-frameworks-devel, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, 
mikesomov

D7446: [Places panel] Revamp the Recently Saved section

[Méven Car]

meven added a comment.


  In D7446#526032 , @ngraham wrote:
  
  > This works great.
  >
  > one minor concern I have with the new IOSlave in general is how it shows 
both files and folders. For me, the folders are a distraction from the file 
listing. But I was thinking: how about adding modes that allow it to show only 
files or only folders? Like the IOSlave could support `recentlyused:/files` and 
`recentlyused:/folders`, each of which would limit the display to just those 
things. Then under the "Recent" section, we could have "Recent Files" and 
"Recent Locations", which would allow us to remove the useless old "Today" 
entry without any trouble.
  >
  > This would provide a better fix for 
https://bugs.kde.org/show_bug.cgi?id=159299.
  
  
  Great suggestions, I will add those to the kio slave.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, anthonyfieroni, andreaska, gregormi, markg, alexeymin, broulik, 
elvisangelaccio, dfaure, davidedmundson, ltoscano, #konqueror, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D23730: Add new protocol for 7z archives

[Méven Car]

meven requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R320 KIO Extras

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

To: nhiga, meven
Cc: ltoscano, meven, ngraham, kfm-devel, kde-frameworks-devel, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, 
mikesomov

D23730: Add new protocol for 7z archives

[Méven Car]

meven added a comment.


  In D23730#526024 , @ltoscano wrote:
  
  > Does it work when the older KIO is installed?
  
  
  Yes, I have tested it.

INLINE COMMENTS

> sevenz.protocol:3
> +exec=kf5/kio/archive
> +protocol=sevenz
> +archiveMimetype=application/x-7z-compressed

Was it possible to use 7zip as protocol, it would probably be more user 
intuitive.

REPOSITORY
  R320 KIO Extras

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

To: nhiga, meven
Cc: ltoscano, meven, ngraham, kfm-devel, kde-frameworks-devel, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, 
mikesomov

D7446: [Places panel] Revamp the Recently Saved section

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, anthonyfieroni, andreaska, gregormi, markg, alexeymin, broulik, 
elvisangelaccio, dfaure, davidedmundson, ltoscano, #konqueror, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D7446: [Places panel] Revamp the Recently Saved section

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  This works great.
  
  one minor concern I have with the new IOSlave in general is how it shows both 
files and folders. For me, the folders are a distraction from the file listing. 
But I was thinking: how about adding modes that allow it to show only files or 
only folders? Like the IOSlave could support `recentlyused:/files` and 
`recentlyused:/folders`, each of which would limit the display to just those 
things. Then under the "Recent" section, we could have "Recent Files" and 
"Recent Locations", which would allow us to remove the useless old "Today" 
entry without any trouble.
  
  This would provide a better fix for 
https://bugs.kde.org/show_bug.cgi?id=159299.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, anthonyfieroni, andreaska, gregormi, markg, alexeymin, broulik, 
elvisangelaccio, dfaure, davidedmundson, ltoscano, #konqueror, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20FreeBSDQt5.13/52/
 Project:
kf5-qt5 FreeBSDQt5.13
 Date of build:
Thu, 05 Sep 2019 13:46:09 +
 Build duration:
9 min 44 sec and counting
   JUnit Tests
  Name: projectroot Failed: 4 test(s), Passed: 48 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiocore_jobtestFailed: projectroot.autotests.kiocore_kmountpointtestFailed: projectroot.autotests.kiowidgets_kdirlistertestFailed: projectroot.autotests.kiowidgets_kdirmodeltestName: 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)

D7446: [Places panel] Revamp the Recently Saved section

[Méven Car]

meven added a comment.


  ping @ngraham any luck testing this ?

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, broulik, elvisangelaccio, #vdg, #frameworks, ngraham
Cc: meven, trickyricky26, andreask, huftis, svenmauch, kde-frameworks-devel, 
spoorun, anthonyfieroni, andreaska, gregormi, markg, alexeymin, broulik, 
elvisangelaccio, dfaure, davidedmundson, ltoscano, #konqueror, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, 
mikesomov

D23576: Add property lastScanTime and lastRequestTime to WirelessDevice

[Méven Car]

meven marked 2 inline comments as done.
meven added inline comments.

INLINE COMMENTS

> jgrulich wrote in wirelessdevice.h:143
> Please do, you can push it directly, it's just a function rename.

https://cgit.kde.org/networkmanager-qt.git/commit/?id=af98fdba63e32c38008592774436ae6c9b61cd90

REPOSITORY
  R282 NetworkManagerQt

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

To: meven, jgrulich
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23730: Add new protocol for 7z archives

[Luigi Toscano]

ltoscano added a comment.


  Does it work when the older KIO is installed?

REPOSITORY
  R320 KIO Extras

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

To: nhiga, meven
Cc: ltoscano, meven, ngraham, kfm-devel, kde-frameworks-devel, iasensio, 
fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
michaelh, spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, 
mikesomov

D23490: [KUrlNavigator] Add MIME types supported by krarc to isCompressedPath

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:509640f8e185: [KUrlNavigator] Add MIME types supported by 
krarc to isCompressedPath (authored by nhiga, committed by ngraham).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23490?vs=64731&id=65431

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

AFFECTED FILES
  src/filewidgets/kurlnavigator.cpp

To: nhiga, #frameworks, cfeck, pino, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23730: Add new protocol for 7z archives

[Méven Car]

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


  In D23730#526010 , @ngraham wrote:
  
  > Probably best to submit the KIO patch first.
  
  
  D23490  has been accepted already.
  
  Tested with D23490  and without krusader 
installed, opens 7zip archive in dolphin.
  
  Works like a charm.

REPOSITORY
  R320 KIO Extras

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

To: nhiga, meven
Cc: meven, ngraham, kfm-devel, kde-frameworks-devel, iasensio, fprice, 
LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh, 
spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

D23730: Add new protocol for 7z archives

[Nathaniel Graham]

ngraham added a comment.


  Probably best to submit the KIO patch first.

REPOSITORY
  R320 KIO Extras

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

To: nhiga
Cc: ngraham, kfm-devel, kde-frameworks-devel, iasensio, fprice, LeGast00n, 
MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, 
spoorun, navarromorales, firef, andrebarros, bruns, emmanuelp, mikesomov

D23722: Warn if KPackage is invalid

[Aleix Pol Gonzalez]

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


  Let's consider it an iteration though.

INLINE COMMENTS

> broulik wrote in configmodule.cpp:174
> Is "mainscript" mandatory?
> 
> In any case, if your package has the incorrect folder structure, it 
> previously printed "no QML file provided" which is super misleading as it 
> suggests the config entry is missing which it usually isn't.

¯\_(ツ)_/¯

If that's the case...

Maybe we could see to passing a string ref argument to isValid? Or adding a 
Package::errorString method (that will likely dupe all the logic).

REPOSITORY
  R296 KDeclarative

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

To: broulik, mart, tcanabrava, apol
Cc: apol, ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D23730: Add new protocol for 7z archives

[N. Higa]

nhiga added a comment.


  Notes:
  
  1. This patch requires D23490  and the 
following KIO patch:
  
diff --git a/src/filewidgets/kurlnavigator.cpp 
b/src/filewidgets/kurlnavigator.cpp
index 80aa975..5a6c720 100644
--- a/src/filewidgets/kurlnavigator.cpp
+++ b/src/filewidgets/kurlnavigator.cpp
@@ -1046,8 +1046,8 @@ void KUrlNavigator::setLocationUrl(const QUrl &newUrl)
 // code locationUrl() and url become the same URLs
 QUrl firstChildUrl = KIO::UrlUtil::firstChildUrl(locationUrl(), url);
 
-if ((url.scheme() == QLatin1String("tar")) || (url.scheme() == 
QLatin1String("zip")) || (url.scheme() == QLatin1String("krarc"))) {
-// The URL represents a tar- or zip-file, or an archive file 
supported by krarc.
+if ((url.scheme() == QLatin1String("tar")) || (url.scheme() == 
QLatin1String("zip")) || (url.scheme() == QLatin1String("sevenz")) || 
(url.scheme() == QLatin1String("krarc"))) {
+// The URL represents a tar-, zip- or 7z-file, or an archive file 
supported by krarc.
 // Check whether the URL is really part of the archive file, 
otherwise
 // replace it by the local path again.
 bool insideCompressedPath = d->isCompressedPath(url);
@@ -1064,7 +1064,7 @@ void KUrlNavigator::setLocationUrl(const QUrl &newUrl)
 }
 }
 if (!insideCompressedPath) {
-// drop the tar: or zip: or krarc: protocol since we are not
+// drop the tar:, zip:, sevenz: or krarc: protocol since we 
are not
 // inside the compressed path
 url.setScheme(QStringLiteral("file"));
 firstChildUrl.setScheme(QStringLiteral("file"));
  
  2. If I name the protocol as "7z", "Invalid protocol" error will be thrown, 
so I use "sevenz" instead.
  3. I am not sure whether it is needed to include `k7zip.h` in 
`archive/kio_archivebase.cpp`.
  4. It can take some time to load the 7z archive using this new protocol.

REPOSITORY
  R320 KIO Extras

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

To: nhiga
Cc: kfm-devel, kde-frameworks-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23730: Add new protocol for 7z archives

[N. Higa]

nhiga created this revision.
nhiga added a project: Frameworks.
Herald added a project: Dolphin.
Herald added a subscriber: kfm-devel.
nhiga requested review of this revision.

REVISION SUMMARY
  This patch adds a new protocol, sevenz, for 7z archives by utilizing K7Zip. 
Previously, users have to install Krusader to use its `krarc` protocol to 
navigate a 7z archive using Dolphin.

REPOSITORY
  R320 KIO Extras

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

AFFECTED FILES
  archive/CMakeLists.txt
  archive/kio_archive.cpp
  archive/kio_archivebase.cpp
  archive/sevenz.protocol

To: nhiga
Cc: kfm-devel, kde-frameworks-devel, iasensio, fprice, LeGast00n, MrPepe, 
fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov

D23708: [CopyJob] Fix crash when copying all files is skipped for an already existing dir

[David Faure]

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


  Thanks for the fix!
  
  Do you feel like writing a unittest for it?
  JobTest::chmodFileError shows how to use PredefinedAnswerJobUiDelegate to 
simulate the user pressing "Skip" in the dialog.
  Write the test without your fix applied, so that you are sure it triggers the 
crash, before re-applying the fix.

INLINE COMMENTS

> copyjob.cpp:1640
>  
> -if ((*it).size > ((1ul << 32) - 1)) { // ((1ul << 32) - 1) = 4 GB
> +// if _all_ elements in "files" have been skipped, the last 
> files.erase() will return
> +// files.end(), bug 408350

This comment only needs to be in the unittest, not in the code.

If every bugfix leads to two lines of comments, the code becomes unreadable.

REPOSITORY
  R241 KIO

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

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

D21760: Add KListOpenFilesJob

[David Faure]

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


  Yes, the filenames should match the classname, obviously :)
  
  I did a deeper review of the KJob usage and I have two more comments, sorry 
for not taking the time to do this earlier...

INLINE COMMENTS

> klistopenfiles_unix.cpp:57
> +job->setErrorText(QObject::tr("Failed to execute `lsof' error code 
> %1").arg(processError));
> +job->emitResult();
> +}

Emitting `result` twice would be a very bad idea, a KJob can only emit `result` 
once.

But QProcess can emit `errorOccurred` followed by `finished` (e.g. if the 
subprocess crashes, from what I can see in qprocess.cpp).
I think we want to only qWarning() in this method, and let `lsofFinished` take 
care of finishing the job.

> klistopenfiles_unix.cpp:62
> +QStringList blockApps;
> +QString out(QString::fromLocal8Bit(lsofProcess.readAll()));
> +QStringList pidList = out.split(QRegExp(QStringLiteral("\\s+")), 
> QString::SkipEmptyParts);

const

> klistopenfiles_unix.cpp:73
> +}
> +emit job->processInfoAvailable(path.path(), processInfoList);
> +job->emitResult();

The class would be slightly easier to use if this was a getter instead of a 
signal.

Because then, in a unittest, you can just do `job->exec()` and then 
`job->processInfoList()`, no spy needed.
In (GUI) application code you still need to connect to a signal, but then the 
usual KJob::result signal is enough, instead of two signals 
(`processInfoAvailable` in case of success, and `result` to handle failure).

See `KIO::StatJob::statResult()` for an example.

Signals emitted by jobs are useful if they happen during the job lifetime (e.g. 
progress signals). If they happen at the same time as `result`, then it's 
easier to use `result` as the notification signal.

The downside is one more member variable, but it'll be very short-lived so I 
wouldn't worry about memory usage.

REPOSITORY
  R244 KCoreAddons

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

To: hallas, davidedmundson, broulik, #frameworks, dfaure, bruns, #plasma
Cc: meven, cfeck, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D22069: Localize long number strings

[David Faure]

dfaure added a comment.


  This broke the unittest in CI.
  
https://build.kde.org/job/Frameworks/job/ki18n/job/kf5-qt5%20SUSEQt5.12/35/testReport/junit/projectroot/autotests/ki18n_klocalizedstringtest/
  
  I can reproduce the failure locally.
  
FAIL!  : KLocalizedStringTest::correctSubs() Compared values are not the 
same
   Actual   (ki18n("%1").subs(4.2, 5, 'f', 2).toString()): " 4.20"
   Expected (QString(" 4,20"))   : " 4,20"
   Loc: 
[/d/kde/src/5/frameworks/ki18n/autotests/klocalizedstringtest.cpp(225)]

REPOSITORY
  R249 KI18n

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

To: ngraham, #localization, #frameworks, broulik, aacid
Cc: dfaure, aacid, huftis, safaalfulaij, mikeroyal, aspotashev, ilic, 
kde-frameworks-devel, broulik, LeGast00n, GB_2, michaelh, ngraham, bruns