D17550: Make the open url in tab feature a bit more discoverable

[Aleix Pol Gonzalez]

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


  Remove unrelated change

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17550?vs=47705=47706

BRANCH
  arcpatch-D17550

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

AFFECTED FILES
  src/filewidgets/kurlnavigator.cpp
  src/filewidgets/kurlnavigator.h
  src/filewidgets/kurlnavigatorbutton.cpp
  src/filewidgets/kurlnavigatorbutton_p.h

To: apol, #dolphin, ngraham, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17550: Make the open url in tab feature a bit more discoverable

[Aleix Pol Gonzalez]

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


  add space

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17550?vs=47481=47705

BRANCH
  arcpatch-D17550

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

AFFECTED FILES
  src/filewidgets/kurlnavigator.cpp
  src/filewidgets/kurlnavigator.h
  src/filewidgets/kurlnavigatorbutton.cpp
  src/filewidgets/kurlnavigatorbutton_p.h
  src/widgets/kfileitemactions.cpp

To: apol, #dolphin, ngraham, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17550: Make the open url in tab feature a bit more discoverable

[Aleix Pol Gonzalez]

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

INLINE COMMENTS

> elvisangelaccio wrote in kurlnavigator.cpp:483
> Do we really have to put the folder name in the extry text? This will make 
> the popup menu very large if the folder name is long.

My thinking was that it's the only action that is context-dependent it better 
be explicit.

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: apol, #dolphin, ngraham, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17617: Display error instead of silently failing when asked to create folder that already exists

[Nathaniel Graham]

ngraham updated this revision to Diff 47703.
ngraham marked 3 inline comments as done.
ngraham added a comment.


  Address review comments (thanks @dfaure!)

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17617?vs=47699=47703

BRANCH
  arcpatch-D17617

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp

To: ngraham, #dolphin, elvisangelaccio, dfaure
Cc: dfaure, emateli, elvisangelaccio, Codezela, kde-frameworks-devel, michaelh, 
ngraham, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

[David Faure]

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


  >   "By consulting 
https://lxr.kde.org/search?_filestring=&_string=BatchRenameJob we can see that 
Dolphin is the only application to make use of this BatchRenameJob API."
  
  This isn't a valid reason to break BatchRenameJob. KF5 makes the promise to 
not break API/ABI for third-party applications, which lxr.kde.org wouldn't know 
about.
  Sorry, if I had realized that this meant "break existing API" I would have 
objected earlier to this line of thought.

INLINE COMMENTS

> batchrenamejob.cpp:45
> +QList m_items;
> +QList::const_iterator m_listIterator;
>  const JobFlags m_flags;

I suggest m_itemsIterator so it's independent from the type (which will be 
QVector instead of a QList)

> batchrenamejob.cpp:115
>  {
> -return BatchRenameJobPrivate::newJob(src, newName, index, placeHolder, 
> flags);
> +return batchRenameFiles({}, flags);
> +}

So this breaks the existing functionality completely !?!?!

The new stuff should rather be a whole different job class, leaving old API 
untouched (deprecated, and to be removed in KF6).

> batchrenamejob.h:31
>  
> +struct KioBatchRenameItem {
> +QUrl src;

Needs to be documented since it's public API, including @since 5.54

> batchrenamejob.h:94
>  
> +KIOCORE_EXPORT BatchRenameJob *batchRenameFiles(const 
> QList , JobFlags flags = DefaultFlags);
> +

Needs to be documented + @since 5.54

QList of something bigger than the size of a pointer is a bad idea (see e.g. 
https://marcmutz.wordpress.com/effective-qt/containers/). Please make it a 
QVector.

> batchrenamedialog.cpp:43
> +#include 
> +#include 
> +#include 

remove KI18n prefix, this isn't a namespaced header

> batchrenamedialog.cpp:53
> +
> +class BatchRenameDialogPrivate : public QDialog
> +{

This is very unusual. Any reason why the public class isn't the one that 
inherits from QDialog?

This would allow the caller to call show(), instead of show() happening 
automatically. Just for consistency with all other dialogs.

> batchrenamedialog.cpp:76
> +
> +QList> m_itemsToBeRenamed;
> +QList m_renamedItems;

QVector

> batchrenamedialog.cpp:339
> +
> +job->exec();
> +}

Better connect to the result() signal of the job and do the rest of 
btnOkClicked in the slot (or lambda) connected to that signal.
exec() opens many potential issues (re-entrancy).

> batchrenamedialogmodel.cpp:31
> +{
> +itemData = new QList;
> +itemData->reserve(items.count());

Why is this a pointer instead of just a value member?

> batchrenamedialogmodel_p.h:20
> +
> +#ifndef KIO_BATCHRENAMEDIALOGMODEL
> +#define KIO_BATCHRENAMEDIALOGMODEL

append _P_H, to avoid a clash with a public header of the same name

> batchrenamedialogmodel_p.h:43
> +private:
> +QList *itemData;
> +

QVector

> batchrenametypes.cpp:33
> +
> +QStringList capturedGroups;
> +

static

(and can this global variable be avoided?)

> batchrenamevar_p.h:21
> +
> +#ifndef KIO_BATCHRENAMEVAR_H
> +#define KIO_BATCHRENAMEVAR_H

_P_H

> batchrenamedialogtest_gui.cpp:32
> +
> +QList items = {};
> +

You can remove the `= {}` which serves no purpose.

REPOSITORY
  R241 KIO

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

To: emateli, #frameworks, dfaure, mlaurent
Cc: chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, 
kde-frameworks-devel, michaelh, bruns

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.9 - Build # 381 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.9/381/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 16 Dec 2018 22:27:42 +
 Build duration:
23 min and counting
   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: 52 test(s), Skipped: 0 test(s), Total: 52 test(s)Name: 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)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(262/398)66%
(262/398)53%
(32100/60124)38%
(16594/44104)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(57/57)100%
(57/57)95%
(9099/9539)48%
(4286/8965)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core86%
(100/116)86%
(100/116)58%
(8344/14341)50%
(4678/9267)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3908/7974)34%
(1603/4757)src.gui100%
(2/2)100%
(2/2)94%
(102/108)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)51%
(531/1036)37%
(318/868)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1344)0%
(0/1416)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/248)0%
(0/148)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1765/4288)35%
(1306/3692)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1330)55%
(626/1135)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(73/267)8%
(14/184)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.11 - Build # 20 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.11/20/
 Project:
kf5-qt5 SUSEQt5.11
 Date of build:
Sun, 16 Dec 2018 22:27:42 +
 Build duration:
5 min 48 sec and counting
   BUILD ARTIFACTS
  compat_reports/KF5KIO_compat_report.html
   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: 52 test(s), Skipped: 0 test(s), Total: 52 test(s)Name: 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)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(262/398)66%
(262/398)53%
(32069/60124)38%
(16562/44104)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(57/57)100%
(57/57)95%
(9099/9539)48%
(4284/8965)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core86%
(100/116)86%
(100/116)58%
(8339/14342)50%
(4668/9263)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3908/7974)34%
(1603/4757)src.gui100%
(2/2)100%
(2/2)94%
(102/108)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)51%
(531/1036)37%
(318/868)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1344)0%
(0/1416)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/248)0%
(0/148)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1770/4288)35%
(1304/3692)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1330)55%
(626/1135)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(73/267)8%
(14/184)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
 

D17617: Display error instead of silently failing when asked to create folder that already exists

[David Faure]

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


  BTW if you also want the mkpath case to error-if-already-exists, I guess this 
could be implemented in KIO::mkpath with a flag [and then the if()/else() isn't 
needed anymore, actually]. Not sure it's worth it though.

INLINE COMMENTS

> knewfilemenu.cpp:906
> +// If the name contains any slashes, use mkpath so that a/b/c works.
> +if (name.contains(QStringLiteral("/"))) {
> +job = KIO::mkpath(url, baseUrl);

QLatin1Char('/') would be enough

> knewfilemenu.cpp:908
> +job = KIO::mkpath(url, baseUrl);
> +job->setProperty("mkpathUrl", url);
> +KJobWidgets::setWindow(job, m_parentWidget);

(could have been renamed to something more generic, in all 3 places)

> knewfilemenu.cpp:910
> +KJobWidgets::setWindow(job, m_parentWidget);
> +job->uiDelegate()->setAutoErrorHandlingEnabled(true);
> +
> KIO::FileUndoManager::self()->recordJob(KIO::FileUndoManager::Mkpath, 
> QList(), url, job);

This could be moved out of the if()/else(), no? In fact, same thing for 
setWindow and setProperty. No point in duplicating these lines.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, elvisangelaccio, dfaure
Cc: dfaure, emateli, elvisangelaccio, Codezela, kde-frameworks-devel, michaelh, 
ngraham, bruns

D10446: Add KLanguageName

[Albert Astals Cid]

aacid added inline comments.

INLINE COMMENTS

> kf5_entry.desktop:1
> +[KCM Locale]
> +Name=Catalan

This is going to be a problem, scripty is going to come and whipe these 
.desktop files out and then make the translators translate them again.

Wonder if we could rename them to .desktop.untransltable or something like that 
and then use the cmake file copy command to copy them to the build folder and 
trick the test to find the files there?

Am i making any sense?

> klanguagenametest.cpp:32
> +void initTestCase()
> +{
> +qputenv("LOCALE", "C.UTF-8");

initTestCase is too late and was failing for me

https://paste.kde.org/p8s9js4r3

makes it work

REPOSITORY
  R265 KConfigWidgets

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

To: aacid
Cc: hein, kde-frameworks-devel, sitter, markg, apol, michaelh, ngraham, bruns

D17623: Add YaST icons

[Noah Davis]

ndavis added a comment.


  In D17623#378248 , @ngraham wrote:
  
  > In D17623#378023 , @ndavis wrote:
  >
  > > When the time comes, we can make symlinks with more appropriate names and 
different sizes.
  >
  >
  > If we continue to use this patch to create icons with names that start with 
`yast-`, I would prefer for those to be the symlinks. So for example your new 
`yast-auth-client` icon should be named `auth-kerberos` and `yast-auth-client` 
should be a symlink to it.
  
  
  We can turn the `yast-*` into symlinks and rename the real icons to proper 
names when we need theme. You're asking me to guess what the right names for 
the future would be before we need them. There's no existing pattern for 
authentication related icons like there is for drives and network stuff.

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg, cfeck, broulik, elvisangelaccio, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17623: Add YaST icons

[Noah Davis]

ndavis added a comment.


  LCP, the other openSUSE designer I've been working with, had this to say:
  
  >   I have to disagree with ngraham here, imagine how many potential 
conflicts in hicolor we could get if yast icons had no prefix
  
  and we can't load those icons from resource like everything else
  
  He doesn't currently have a KDE account. Hopefully he will get one so I don't 
need to copy and paste what he says.

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg, cfeck, broulik, elvisangelaccio, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D17623: Add YaST icons

[Nathaniel Graham]

ngraham added reviewers: cfeck, broulik, elvisangelaccio.
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  In D17623#378023 , @ndavis wrote:
  
  > I'd have to make PRs for ~60 GitHub repositories, so I'd like to avoid that 
if possible.
  
  
  I understand that this would be really annoying, but conceptually it seems 
like the correct approach. If YaST wants to icons from icon themes, it doesn't 
seem reasonable to expect every single icon theme in existence to make special 
icons just fro YaST. YaST should use commonly-named icons.
  
  > When the time comes, we can make symlinks with more appropriate names and 
different sizes.
  
  If we continue to use this patch to create icons with names that start with 
`yast-`, I would prefer for those to be the symlinks. So for example your new 
`yast-auth-client` icon should be named `auth-kerberos` and `yast-auth-client` 
should be a symlink to it.

REPOSITORY
  R266 Breeze Icons

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

To: ndavis, #vdg, cfeck, broulik, elvisangelaccio, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

[Nathaniel Graham]

ngraham added a comment.


  It might not be the worst thing in the world if you made this a 
general-purpose rename dialog, so that it handles the simple common case of 
renaming one items as well as the complicated case where multiple items are 
renamed. If you do that, I'll abandon D17595 
.

REPOSITORY
  R241 KIO

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

To: emateli, #frameworks, dfaure, mlaurent
Cc: chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, 
kde-frameworks-devel, michaelh, bruns

D17617: Display error instead of silently failing when asked to create folder that already exists

[Nathaniel Graham]

ngraham marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, elvisangelaccio
Cc: dfaure, emateli, elvisangelaccio, Codezela, kde-frameworks-devel, michaelh, 
ngraham, bruns

D17617: Display error instead of silently failing when asked to create folder that already exists

[Nathaniel Graham]

ngraham updated this revision to Diff 47699.
ngraham edited the test plan for this revision.
ngraham added a comment.


  Use a better approach that works in all cases

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17617?vs=47649=47699

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp

To: ngraham, #dolphin, elvisangelaccio
Cc: dfaure, emateli, elvisangelaccio, Codezela, kde-frameworks-devel, michaelh, 
ngraham, bruns

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.11 - Build # 19 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.11/19/
 Project:
kf5-qt5 SUSEQt5.11
 Date of build:
Sun, 16 Dec 2018 20:58:16 +
 Build duration:
22 min and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 1 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiogui_favicontestName: 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)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(262/398)66%
(262/398)53%
(32092/60124)38%
(16579/44104)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(57/57)100%
(57/57)95%
(9091/9538)48%
(4278/8965)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core86%
(100/116)86%
(100/116)58%
(8356/14342)51%
(4681/9263)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3908/7974)34%
(1603/4757)src.gui100%
(2/2)100%
(2/2)88%
(95/108)68%
(45/66)src.ioslaves.file100%
(5/5)100%
(5/5)51%
(531/1036)37%
(318/868)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1344)0%
(0/1416)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/248)0%
(0/148)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1765/4288)35%
(1306/3692)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1330)55%
(626/1135)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(73/267)8%
(14/184)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
  

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.9 - Build # 380 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.9/380/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 16 Dec 2018 20:58:16 +
 Build duration:
22 min and counting
   BUILD ARTIFACTS
  compat_reports/KF5KIO_compat_report.html
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 1 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiogui_favicontestName: 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)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(262/398)66%
(262/398)53%
(32090/60123)38%
(16584/44104)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(57/57)100%
(57/57)95%
(9091/9538)48%
(4280/8965)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core86%
(100/116)86%
(100/116)58%
(8353/14341)51%
(4683/9267)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3908/7974)34%
(1603/4757)src.gui100%
(2/2)100%
(2/2)88%
(95/108)68%
(45/66)src.ioslaves.file100%
(5/5)100%
(5/5)51%
(532/1036)37%
(319/868)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1344)0%
(0/1416)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/248)0%
(0/148)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1765/4288)35%
(1306/3692)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1330)55%
(626/1135)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(73/267)8%
(14/184)src.ioslaves.remote.kdedmodule0%
(0/2)0%

D16981: [kfilewidget] Return editable URL navigator to breadcrumb mode if it has focus and everything is selected and when Ctrl+L is pressed

[Nathaniel Graham]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:33442c012eb2: [kfilewidget] Return editable URL navigator 
to breadcrumb mode if it has focus… (authored by ngraham).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16981?vs=46932=47698

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

AFFECTED FILES
  src/filewidgets/kfilewidget.cpp

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

D16981: [kfilewidget] Return editable URL navigator to breadcrumb mode if it has focus and everything is selected and when Ctrl+L is pressed

[Nathaniel Graham]

ngraham added a comment.


  Landing this too since its Dolphin companion (D16980 
) has been accepted, and the code change is 
virtually identical.

REPOSITORY
  R241 KIO

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

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

D17617: Display error instead of silently failing when asked to create folder that already exists

[David Faure]

dfaure added a comment.


  What might be easier is:
  
  - if the user entered something without '/', use KIO::mkdir()   (which will 
fail with ERR_DIR_ALREADY_EXIST if the dir already exists)
  - if the user entered something with '/', use KIO::mkpath() as the code 
currently does.
  
  Much simpler and faster than an (async and racy) existence check before hand.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, elvisangelaccio
Cc: dfaure, emateli, elvisangelaccio, Codezela, kde-frameworks-devel, michaelh, 
ngraham, bruns

D17617: Display error instead of silently failing when asked to create folder that already exists

[David Faure]

dfaure added a comment.


  In D17617#378199 , @ngraham wrote:
  
  > Oh darn, now do I check for the existence of a directory in a 
network-transparent, KIO-ish way? Use a `listJob` or something?
  
  
  KIO::stat()

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, elvisangelaccio
Cc: dfaure, emateli, elvisangelaccio, Codezela, kde-frameworks-devel, michaelh, 
ngraham, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

[Emirald Mateli]

emateli added a subscriber: chinmoyr.
emateli added a comment.


  Pinging @chinmoyr as the original author of the BatchRenameJob class.

REPOSITORY
  R241 KIO

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

To: emateli, #frameworks, dfaure, mlaurent
Cc: chinmoyr, mlaurent, asensi, rkflx, dfaure, aacid, ngraham, 
kde-frameworks-devel, michaelh, bruns

D14631: Adds a new RenameDialog to KIO with more options for batch renaming

[Emirald Mateli]

emateli updated this revision to Diff 47694.
emateli added a comment.


  - convert to namespace from class
  - do not show first captured group
  - remove help button
  - Use d-ptr pattern
  - remove show/hide methods and autoshow dialog
  - update doc
  - KFileItemList -> QList; KFileItem usage was unnecessary
  - update doc
  - WIP Change BatchRenameJobPrivate class
  
  This is still a WIP mostly with the use of d-ptr pattern for the exported 
header and the conversion of BatchRenameJob from a class with a very specific 
purpose to a general-purpose batch rename operation.
  
  Looking for comments especially for the repurposing of the BatchRenameJob 
class(currently used only by Doplhin) and other thoughts in general as well.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14631?vs=45438=47694

BRANCH
  batchrename2

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/batchrenametypestest.cpp
  autotests/batchrenametypestest.h
  src/core/batchrenamejob.cpp
  src/core/batchrenamejob.h
  src/widgets/CMakeLists.txt
  src/widgets/rename/batchrenamedialog.cpp
  src/widgets/rename/batchrenamedialog.h
  src/widgets/rename/batchrenamedialogmodel.cpp
  src/widgets/rename/batchrenamedialogmodel_p.h
  src/widgets/rename/batchrenametypes.cpp
  src/widgets/rename/batchrenametypes_p.h
  src/widgets/rename/batchrenamevar.cpp
  src/widgets/rename/batchrenamevar_p.h
  src/widgets/rename/filenameutils.cpp
  src/widgets/rename/filenameutils_p.h
  tests/CMakeLists.txt
  tests/batchrenamedialogtest_gui.cpp
  tests/previewtest.cpp

To: emateli, #frameworks, dfaure, mlaurent
Cc: mlaurent, asensi, rkflx, dfaure, aacid, ngraham, kde-frameworks-devel, 
michaelh, bruns

D17617: Display error instead of silently failing when asked to create folder that already exists

[Emirald Mateli]

emateli added a comment.


  FWIW: The patch over at D11304  changes 
the behavior to select the currently existing folder instead of doing nothing.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, elvisangelaccio
Cc: emateli, elvisangelaccio, Codezela, kde-frameworks-devel, michaelh, 
ngraham, bruns

D17310: Improve Kile icon with LaTeX font

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  I like the current version, FWIW. #kile 
 folks ,what do you think?

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

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

To: trickyricky26, #vdg, ngraham, #kile
Cc: loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17617: Display error instead of silently failing when asked to create folder that already exists

[Nathaniel Graham]

ngraham added a comment.


  Oh darn, now do I check for the existence of a directory in a 
network-transparent, KIO-ish way? Use a `listJob` or something?

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, elvisangelaccio
Cc: elvisangelaccio, Codezela, kde-frameworks-devel, michaelh, ngraham, bruns

D17545: Do not stat move/copy job if the destination file system does not support writing

[David Faure]

dfaure added a comment.


  My proposal is at https://phabricator.kde.org/D17632

REPOSITORY
  R241 KIO

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

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

D17632: Move/copy job: skip stat'ing sources if the destination dir isn't writable

[David Faure]

dfaure retitled this revision from "Unittest only" to "Move/copy job: skip 
stat'ing sources if the destination dir isn't writable".
dfaure edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: dfaure, shubham, broulik, bruns
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17632: Unittest only

[David Faure]

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


  merge the commits

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17632?vs=47688=47689

BRANCH
  master

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

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

To: dfaure, shubham, broulik, bruns
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17632: Unittest only

[David Faure]

dfaure created this revision.
dfaure added reviewers: shubham, broulik, bruns.
Herald added a project: Frameworks.
Herald edited subscribers, added: kde-frameworks-devel; removed: Frameworks.
dfaure requested review of this revision.

REVISION SUMMARY
  Move/copy job: skip stat'ing sources if the destination dir isn't writable
  
  BUG: 141564

TEST PLAN
  Unit test, but also copying to / in dolphin, to check what the error message 
looks like.

REPOSITORY
  R241 KIO

BRANCH
  master

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

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

To: dfaure, shubham, broulik, bruns
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17631: class Message: Use inclass member initialization

[loh tar]

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


  I'm surprised. Obviously have you change something previous (?)

REPOSITORY
  R39 KTextEditor

BRANCH
  message_inclass_member_initializationa

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

To: dhaumann, loh.tar
Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D17545: Do not stat move/copy job if the destination file system does not support writing

[David Faure]

dfaure added a comment.


  In fact this is the wrong location for this code. The checks on the dest dir 
are in CopyJobPrivate::slotResultStating. I'm working on a different fix.

REPOSITORY
  R241 KIO

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

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

D17631: class Message: Use inclass member initialization

[Dominik Haumann]

dhaumann created this revision.
dhaumann added a reviewer: loh.tar.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
dhaumann requested review of this revision.

REVISION SUMMARY
  No change in logic, just some reorganization of initialization code.

TEST PLAN
  make && make test

REPOSITORY
  R39 KTextEditor

BRANCH
  message_inclass_member_initializationa

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

AFFECTED FILES
  src/utils/messageinterface.cpp

To: dhaumann, loh.tar
Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D17624: KTextEditor::Message: Review documentation

[Dominik Haumann]

dhaumann closed this revision.

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

To: loh.tar, #ktexteditor, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars

KDE CI: Frameworks » ktexteditor » kf5-qt5 SUSEQt5.11 - Build # 9 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/ktexteditor/job/kf5-qt5%20SUSEQt5.11/9/
 Project:
kf5-qt5 SUSEQt5.11
 Date of build:
Sun, 16 Dec 2018 18:44:11 +
 Build duration:
6 min 12 sec and counting
   BUILD ARTIFACTS
  compat_reports/KF5TextEditor_compat_report.html
   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: 61 test(s), Skipped: 0 test(s), Total: 61 test(s)Name: projectroot.autotests.src Failed: 1 test(s), Passed: 4 test(s), Skipped: 0 test(s), Total: 5 test(s)Failed: projectroot.autotests.src.vimode.vimode_keys
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report79%
(22/28)88%
(254/289)88%
(254/289)68%
(32960/48239)51%
(15851/30984)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.src100%
(37/37)100%
(37/37)94%
(4431/4730)49%
(1696/3450)autotests.src.vimode100%
(9/9)100%
(9/9)99%
(5483/5523)58%
(996/1712)src.buffer88%
(15/17)88%
(15/17)90%
(1697/1881)76%
(1214/1596)src.completion100%
(11/11)100%
(11/11)57%
(1772/3109)43%
(1160/2717)src.completion.expandingtree100%
(3/3)100%
(3/3)40%
(184/459)22%
(76/348)src.dialogs0%
(0/4)0%
(0/4)0%
(0/732)0%
(0/176)src.document100%
(4/4)100%
(4/4)59%
(1870/3165)47%
(1565/3313)src.export0%
(0/4)0%
(0/4)0%
(0/119)0%
(0/162)src.include.ktexteditor93%
(14/15)93%
(14/15)91%
(221/244)65%
(146/226)src.inputmode100%
(8/8)100%
(8/8)63%
(189/302)51%
(39/77)src.mode83%
(5/6)83%
(5/6)41%
(171/417)40%
(111/275)src.part0%
(0/1)0%
(0/1)0%
(0/7)100%
(0/0)src.printing0%
(0/4)0%
(0/4)0%
(0/830)0%
(0/294)src.render100%
(7/7)100%
(7/7)77%
(953/1244)67%
(627/942)src.schema29%
(2/7)29%
(2/7)1%
(20/1492)1%
(6/673)src.script100%
(16/16)100%
(16/16)68%
(731/1069)58%
(268/465)src.search100%
(6/6)100%
 

D17624: KTextEditor::Message: Review documentation

[Dominik Haumann]

dhaumann added a comment.


  I integrated this now, see a16d082370a44fcbae3a204bfede1db6e6dffe86 - the 
change also includes the rename of autoHide to autoHideDelay. The function 
names cannot be changed of course, since it's public API.

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

To: loh.tar, #ktexteditor, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars

D17620: Make setHelp() public

[Luigi Toscano]

ltoscano added a comment.


  There may be another way: add a new public method which wraps the protected 
one, and when moving to KF6 a) make setHelp public and b) deprecate the new 
wrapper. I think - suggestions needed

REPOSITORY
  R265 KConfigWidgets

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

To: yurchor, #frameworks
Cc: ltoscano, aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, 
bruns

D17620: Make setHelp() public

[Yuri Chornoivan]

yurchor added a comment.


  In D17620#378112 , @aacid wrote:
  
  > In D17620#378110 , @yurchor 
wrote:
  >
  > > Abandoned due to the policy violation. Despite of the handful help pages 
invocation, it is obvious that this change will never happen.
  > >
  > > Just a side note: the protected method is useless and can be removed 
without loss of any functionality because all known setHelp()'s are commented 
now with hopeless messages like "TODO: port to kf5".
  >
  >
  > Yuri, you keep doing aggressive comments like this, your patches are 
interesting, but honestly i don't know if for me it's worthwhile helping you, 
if every so often you complain about how much everything is broken when it is 
not.
  >
  > Of course the method is not useless, this method is trivially usable by 
just having a class that inherits from KConfigDialog like Okular and i'm sure 
lots of other apps do.
  
  
  Ok. Sorry. Thanks for the useful clarification. Please do not help me. You 
need not to do so.

REPOSITORY
  R265 KConfigWidgets

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

To: yurchor, #frameworks
Cc: aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17620: Make setHelp() public

[Albert Astals Cid]

aacid added a comment.


  In D17620#378110 , @yurchor wrote:
  
  > Abandoned due to the policy violation. Despite of the handful help pages 
invocation, it is obvious that this change will never happen.
  >
  > Just a side note: the protected method is useless and can be removed 
without loss of any functionality because all known setHelp()'s are commented 
now with hopeless messages like "TODO: port to kf5".
  
  
  Yuri, you keep doing aggressive comments like this, your patches are 
interesting, but honestly i don't know if for me it's worthwhile helping you, 
if every so often you complain about how much everything is broken when it is 
not.
  
  Of course the method is not useless, this method is trivially usable by just 
having a class that inherits from KConfigDialog like Okular and i'm sure lots 
of other apps do.

REPOSITORY
  R265 KConfigWidgets

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

To: yurchor, #frameworks
Cc: aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17620: Make setHelp() public

[Yuri Chornoivan]

yurchor abandoned this revision.
yurchor added a comment.


  Abandoned due to the policy violation. Despite of the handful help pages 
invocation, it is obvious that this change will never happen.
  
  Just a side note: the protected method is useless and can be removed without 
loss of any functionality because all known setHelp()'s are commented now with 
hopeless messages like "TODO: port to kf5".

REPOSITORY
  R265 KConfigWidgets

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

To: yurchor, #frameworks
Cc: aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17630: Don't re-mark words added/ignored to the dictionary as misspelled

[Ahmad Samir]

ahmadsamir added reviewers: cullmann, davidedmundson.

REPOSITORY
  R39 KTextEditor

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

To: ahmadsamir, cullmann, davidedmundson
Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D17629: Add method to BackgroundChecker to add word to session

[Ahmad Samir]

ahmadsamir added a dependent revision: D17630: Don't re-mark words 
added/ignored to the dictionary as misspelled.

REPOSITORY
  R246 Sonnet

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

To: ahmadsamir, davidedmundson, cullmann
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17630: Don't re-mark words added/ignored to the dictionary as misspelled

[Ahmad Samir]

ahmadsamir created this revision.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  When a word is added to the dictionary or ignored via the spellingMenu,
  the BackgroundChecker instance in ontheflycheck should be updated to
  reflect the changes, otherwise words already added to the dictionary
  (or ignored) are re-highlited as misspelled when the view changes.
  
  Depends on D17629 .
  
  BUG: 387729

TEST PLAN
  - Open a file with misspelled words in kate and enable automatic spellchecking
  - Using the context menu:
- add a misspelled word to the dictionary
- Ignore another word B misspelling highlighting is removed, as expected
  - Scroll up/down so that those particular words are out of view then scroll 
and show them again; notice that those words re-highlighted as misspelled
  - After applying the pacth, words added/ingored to the dictionary aren't 
going to be re-highlighted as misspelled when the view changes

REPOSITORY
  R39 KTextEditor

BRANCH
  fix-ontheflycheck-backgroundchecker

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

AFFECTED FILES
  src/spellcheck/ontheflycheck.cpp
  src/spellcheck/ontheflycheck.h
  src/spellcheck/spellcheck.cpp
  src/spellcheck/spellcheck.h

To: ahmadsamir
Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, 
demsking, cullmann, sars, dhaumann

D17624: KTextEditor::Message: Review documentation

[loh tar]

loh.tar added a comment.


  Sorry for the hassle, will revert the update
  
  > Further, I would prefer autoHideDelay instead of just delay.
  
  Are you refering here to the header or the code part?
  The old name in the header was "autoHideTimeR" which is pretty wrong. So have 
I to change this in the header/docu to "autoHideDelay"? Or perhaps 
"autoHideTime"? Then is the change minimal.

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

To: loh.tar, #ktexteditor, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars

D17620: Make setHelp() public

[Albert Astals Cid]

aacid added a comment.


  You can always just use QMetaObject::invokeMethod it's a bit lame, but slots 
are never really non-public if you use the metaobject system

INLINE COMMENTS

> elvisangelaccio wrote in kconfigdialog.h:176
> If I'm not mistaken, moving a function from `protected` to `public` breaks 
> the ABI

That's what 
https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B 
says yes.

REPOSITORY
  R265 KConfigWidgets

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

To: yurchor, #frameworks
Cc: aacid, elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17629: Add method to BackgroundChecker to add word to session

[Ahmad Samir]

ahmadsamir added reviewers: davidedmundson, cullmann.

REPOSITORY
  R246 Sonnet

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

To: ahmadsamir, davidedmundson, cullmann
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17629: Add method to BackgroundChecker to add word to session

[Ahmad Samir]

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

REVISION SUMMARY
  Add convience method to add a word to BackgroundChecker the session
  of the currentDict speller.

REPOSITORY
  R246 Sonnet

BRANCH
  addWordToSession

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

AFFECTED FILES
  src/core/backgroundchecker.cpp
  src/core/backgroundchecker.h

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

D17624: KTextEditor::Message: Review documentation

[Dominik Haumann]

dhaumann added a comment.


  Now you are mixing documentation changes with code changes. I would have 
preferred to have different review requests, especially since the documentation 
part was already reviewed. Further, I would prefer autoHideDelay instead of 
just delay. If we use "delay" only, then the question immediately arises "the 
delay of what"? With this in mind, the naming of "autoHide" as better, since at 
least the context was clear. I will take care of the documentation now, but 
will not take care of the renaming for the reason given.

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

To: loh.tar, #ktexteditor, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars

D17545: Do not stat move/copy job if the destination file system does not support writing

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> jobtest.cpp:1004
> +
> +QSignalSpy spy(job, ::error);
> +job->setUiDelegate(nullptr);

At runtime, Qt says:

  QSignalSpy: Not a valid signal: ''

Indeed this is the getter, not a signal. `result()` is the signal, but you 
don't really need to connect to it anyway.

> jobtest.cpp:1007
> +QVERIFY(!job->exec());
> +QCOMPARE(job->error(), (int)KIO::ERR_CANNOT_WRITE);
> +QCOMPARE(spy.count(), 1); // one error should be emitted by the move job

FAIL!  : JobTest::moveDirectoryToInaccessibleFilesystem() Compared values are 
not the same

  Actual   (job->error())  : 115
  Expected ((int)KIO::ERR_CANNOT_WRITE): 129

115 is ERR_ACCESS_DENIED, see `grep -w 15 src/core/global.h `

I think that's because you went too far (due to copy/pasting). The dest dir 
shouldn't be inaccessible, it should only be readonly.

Also you forgot the cleanup at the end of the method, which breaks running the 
test multiple times.

OK, even with all this the test doesn't pass.
Stepping into statNextSrc, I see that it goes into startRenameJob on line 846 
so it doesn't get into your new code which is further down.
That's because the src and dest are on the same partition, so a simple rename 
is enough.

Changing dst_dir to use otherTmpDir() ... ok, the error code changes, now it's 
137, i.e. ERR_COULD_NOT_MKDIR :-)
This requires further investigation...

REPOSITORY
  R241 KIO

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

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

D17619: Unit test and fix for bug 401552

[David Faure]

dfaure added a comment.


  What happened to the unittest promised by the commit log? ;)

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17619: Unit test and fix for bug 401552

[David Faure]

dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kcoredirlister.cpp:1580
> +while (!dir->lstItems.isEmpty()) {
> +const KFileItem oldItem = dir->lstItems.takeFirst();
> +KFileItem newItem = oldItem;

Why is this modifying lstItems? That's a somewhat costly operation, if it's not 
needed. Why not a normal readonly iteration?

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17623: Add YaST icons

[Noah Davis]

ndavis added a comment.


  In D17623#378006 , @ngraham wrote:
  
  > Nice work, these are good icons!
  >
  > However, I wonder whether we really want to use the `yast-` prefix for 
these images, especially the ones without explicit opensuse iconography.
  >  I can imagine some of these being useful in other apps' settings dialogs 
as well, but it would feel weird to use an icon that starts with `yast-`. Can 
we not change YaST itself to use different names for its icons? If YaST has 
recently gained the ability to take icons from icon themes, it seems 
unreasonable for it to expect icons named `yast-` rather than just 
using more neutrally-named icons.
  >
  > For example `yast-auth-client` could be be named `auth-kerberos` or 
something so that any kerberos-related things can use that nice icon.
  
  
  I'd have to make PRs for ~60 GitHub repositories, so I'd like to avoid that 
if possible. I agree that some of these would be useful for other programs 
though. When the time comes, we can make symlinks with more appropriate names 
and different sizes.

REPOSITORY
  R266 Breeze Icons

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

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

D17624: KTextEditor::Message: Review documentation

[loh tar]

loh.tar updated this revision to Diff 47678.
loh.tar added a comment.


  -Rename member autoHide->delay to fit accepted change in header file
  
  just an offer

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17624?vs=47673=47678

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

AFFECTED FILES
  src/include/ktexteditor/message.h
  src/utils/messageinterface.cpp

To: loh.tar, #ktexteditor, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars

D17623: Add YaST icons

[Nathaniel Graham]

ngraham added a comment.


  Nice work, these are good icons!
  
  However, I wonder whether we really want to use the `yast-` prefix for these 
images, especially the ones without explicit opensuse iconography. I can 
imagine some of these being useful in other apps' settings dialogs as well, but 
it would feel weird to use an icon that starts with `yast-`. Can we not change 
YaST itself to use different names for its icons? If YaST has recently gained 
the ability to take icons from icon themes, it seems unreasonable for it to 
expect icons named `yast-` rather than just using more 
neutrally-named icons.
  
  For example `yast-auth-client` could be be named `auth-kerberos` or something 
so that any kerberos-related things can use that nice icon.

REPOSITORY
  R266 Breeze Icons

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

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

D17623: Add YaST icons

[Noah Davis]

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


  Change printer and scanner icons to 32px since YaST Control Center doesn't 
want to work with symlinks to 64px device icons.

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17623?vs=47668=47674

BRANCH
  yast-icons (branched from master)

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

AFFECTED FILES
  icons-dark/apps/32/yast-addon.svg
  icons-dark/apps/32/yast-alternatives.svg
  icons-dark/apps/32/yast-apparmor.svg
  icons-dark/apps/32/yast-auth-client.svg
  icons-dark/apps/32/yast-autoyast.svg
  icons-dark/apps/32/yast-bootloader.svg
  icons-dark/apps/32/yast-checkmedia.svg
  icons-dark/apps/32/yast-create-new-vm.svg
  icons-dark/apps/32/yast-device-tree.svg
  icons-dark/apps/32/yast-dhcp-server.svg
  icons-dark/apps/32/yast-disk.svg
  icons-dark/apps/32/yast-dns-server.svg
  icons-dark/apps/32/yast-docker.svg
  icons-dark/apps/32/yast-dsl.svg
  icons-dark/apps/32/yast-fcoe.svg
  icons-dark/apps/32/yast-firewall.svg
  icons-dark/apps/32/yast-fonts.svg
  icons-dark/apps/32/yast-ftp-server.svg
  icons-dark/apps/32/yast-hardware.svg
  icons-dark/apps/32/yast-host.svg
  icons-dark/apps/32/yast-http-server.svg
  icons-dark/apps/32/yast-hwinfo.svg
  icons-dark/apps/32/yast-instserver.svg
  icons-dark/apps/32/yast-iscsi-client.svg
  icons-dark/apps/32/yast-iscsi-server.svg
  icons-dark/apps/32/yast-journal.svg
  icons-dark/apps/32/yast-kerberos-server.svg
  icons-dark/apps/32/yast-kernel.svg
  icons-dark/apps/32/yast-keyboard.svg
  icons-dark/apps/32/yast-lan.svg
  icons-dark/apps/32/yast-language.svg
  icons-dark/apps/32/yast-ldap-server.svg
  icons-dark/apps/32/yast-ldap.svg
  icons-dark/apps/32/yast-mail.svg
  icons-dark/apps/32/yast-messages.svg
  icons-dark/apps/32/yast-misc.svg
  icons-dark/apps/32/yast-network.svg
  icons-dark/apps/32/yast-nfs-server.svg
  icons-dark/apps/32/yast-nfs.svg
  icons-dark/apps/32/yast-nis-server.svg
  icons-dark/apps/32/yast-nis.svg
  icons-dark/apps/32/yast-ntp-client.svg
  icons-dark/apps/32/yast-printer.svg
  icons-dark/apps/32/yast-proxy.svg
  icons-dark/apps/32/yast-release-notes.svg
  icons-dark/apps/32/yast-remote.svg
  icons-dark/apps/32/yast-samba-client.svg
  icons-dark/apps/32/yast-samba-server.svg
  icons-dark/apps/32/yast-scanner.svg
  icons-dark/apps/32/yast-security.svg
  icons-dark/apps/32/yast-services-manager.svg
  icons-dark/apps/32/yast-slp-server.svg
  icons-dark/apps/32/yast-snapper.svg
  icons-dark/apps/32/yast-software.svg
  icons-dark/apps/32/yast-sound.svg
  icons-dark/apps/32/yast-sudo.svg
  icons-dark/apps/32/yast-support.svg
  icons-dark/apps/32/yast-sw_single.svg
  icons-dark/apps/32/yast-sw_source.svg
  icons-dark/apps/32/yast-sysconfig.svg
  icons-dark/apps/32/yast-system.svg
  icons-dark/apps/32/yast-tftp-server.svg
  icons-dark/apps/32/yast-timezone.svg
  icons-dark/apps/32/yast-update.svg
  icons-dark/apps/32/yast-users.svg
  icons-dark/apps/32/yast-vm-install.svg
  icons-dark/apps/32/yast-vm-migrate.svg
  icons-dark/apps/32/yast-vm.svg
  icons-dark/apps/32/yast-vpn.svg
  icons-dark/apps/32/yast-wol.svg
  icons-dark/apps/32/yast.svg
  icons-dark/apps/48/yast-installation.svg
  icons-dark/apps/48/yast-sw_single.svg
  icons-dark/apps/48/yast.svg
  icons/apps/32/yast-addon.svg
  icons/apps/32/yast-alternatives.svg
  icons/apps/32/yast-apparmor.svg
  icons/apps/32/yast-auth-client.svg
  icons/apps/32/yast-autoyast.svg
  icons/apps/32/yast-bootloader.svg
  icons/apps/32/yast-checkmedia.svg
  icons/apps/32/yast-create-new-vm.svg
  icons/apps/32/yast-device-tree.svg
  icons/apps/32/yast-dhcp-server.svg
  icons/apps/32/yast-disk.svg
  icons/apps/32/yast-dns-server.svg
  icons/apps/32/yast-docker.svg
  icons/apps/32/yast-dsl.svg
  icons/apps/32/yast-fcoe.svg
  icons/apps/32/yast-firewall.svg
  icons/apps/32/yast-fonts.svg
  icons/apps/32/yast-ftp-server.svg
  icons/apps/32/yast-hardware.svg
  icons/apps/32/yast-host.svg
  icons/apps/32/yast-http-server.svg
  icons/apps/32/yast-hwinfo.svg
  icons/apps/32/yast-instserver.svg
  icons/apps/32/yast-iscsi-client.svg
  icons/apps/32/yast-iscsi-server.svg
  icons/apps/32/yast-journal.svg
  icons/apps/32/yast-kerberos-server.svg
  icons/apps/32/yast-kernel.svg
  icons/apps/32/yast-keyboard.svg
  icons/apps/32/yast-lan.svg
  icons/apps/32/yast-language.svg
  icons/apps/32/yast-ldap-server.svg
  icons/apps/32/yast-ldap.svg
  icons/apps/32/yast-mail.svg
  icons/apps/32/yast-messages.svg
  icons/apps/32/yast-misc.svg
  icons/apps/32/yast-network.svg
  icons/apps/32/yast-nfs-server.svg
  icons/apps/32/yast-nfs.svg
  icons/apps/32/yast-nis-server.svg
  icons/apps/32/yast-nis.svg
  icons/apps/32/yast-ntp-client.svg
  icons/apps/32/yast-printer.svg
  icons/apps/32/yast-proxy.svg
  icons/apps/32/yast-release-notes.svg
  icons/apps/32/yast-remote.svg
  icons/apps/32/yast-samba-client.svg
  icons/apps/32/yast-samba-server.svg
  icons/apps/32/yast-scanner.svg
  icons/apps/32/yast-security.svg
  

D17624: KTextEditor::Message: Review documentation

[loh tar]

loh.tar updated this revision to Diff 47673.
loh.tar added a comment.


  - Fix optionally/optional + be
  
  I like to suggest that some more eyes take a look at that :-)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17624?vs=47669=47673

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

AFFECTED FILES
  src/include/ktexteditor/message.h

To: loh.tar, #ktexteditor, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars

D17545: Do not stat move/copy job if the destination file system does not support writing

[Shubham]

shubham added a comment.


  Did anyone tried it?

REPOSITORY
  R241 KIO

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

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

D17624: KTextEditor::Message: Review documentation

[Dominik Haumann]

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


  Looks almost good, except for "optional" and a missing "be". Could you update 
again?

INLINE COMMENTS

> message.h:337
>  /**
> - * Optionally set an icon for this notification.
> - * The icon is shown next to the message text.
> + * Add an optionally @p icon for this notification which will shown next 
> to
> + * the message text. If the message was already sent through 
> Document::postMessage(),

Add an optional @p icon ... which will be shown

REPOSITORY
  R39 KTextEditor

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

To: loh.tar, #ktexteditor, dhaumann
Cc: dhaumann, kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, 
ngraham, bruns, demsking, cullmann, sars

D17624: KTextEditor::Message: Review documentation

[loh tar]

loh.tar created this revision.
loh.tar added a reviewer: KTextEditor.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
loh.tar requested review of this revision.

REVISION SUMMARY
  - Reduce redundant text
  - Change some argument names in the hope it's an improvement
  - Update a code example to fit new style
  - Try to improve some wording
  - Add some @see, @p and such, but probably not all will be needed due to 
"auto reference"
  - Fix some outdated info

REPOSITORY
  R39 KTextEditor

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

AFFECTED FILES
  src/include/ktexteditor/message.h

To: loh.tar, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, #ktexteditor, hase, michaelh, ngraham, 
bruns, demsking, cullmann, sars, dhaumann

D17623: Add YaST icons

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D17623: Add YaST icons

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D17623: Add YaST icons

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D17623: Add YaST icons

[Noah Davis]

ndavis edited the summary of this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D17623: Add YaST icons

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D17623: Add YaST icons

[Noah Davis]

ndavis edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

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

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

D17623: Add YaST icons

[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
  Most of these icons are meant to be used in YaST Control Center.
  
  There is currently an issue where there is no good folder to place these. 
`apps/32` is a `Fixed` directory, which means this patch will fail the scalable 
test. `preferences/32` would work, except it's only meant to be used for icons 
that are used by the KDE System Settings. Currently, most of these icons are 
32px because that's the size they will be at in the YaST Control Center. 
Another way to solve the scalable issue would be to also create 48px versions 
of all of these icons, but that would be a lot of work and I want these icons 
to be in KF5.54 so that openSUSE Leap 15.1 will have them.

REPOSITORY
  R266 Breeze Icons

BRANCH
  yast-icons (branched from master)

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

AFFECTED FILES
  icons-dark/apps/32/yast-addon.svg
  icons-dark/apps/32/yast-alternatives.svg
  icons-dark/apps/32/yast-apparmor.svg
  icons-dark/apps/32/yast-auth-client.svg
  icons-dark/apps/32/yast-autoyast.svg
  icons-dark/apps/32/yast-bootloader.svg
  icons-dark/apps/32/yast-checkmedia.svg
  icons-dark/apps/32/yast-create-new-vm.svg
  icons-dark/apps/32/yast-device-tree.svg
  icons-dark/apps/32/yast-dhcp-server.svg
  icons-dark/apps/32/yast-disk.svg
  icons-dark/apps/32/yast-dns-server.svg
  icons-dark/apps/32/yast-docker.svg
  icons-dark/apps/32/yast-dsl.svg
  icons-dark/apps/32/yast-fcoe.svg
  icons-dark/apps/32/yast-firewall.svg
  icons-dark/apps/32/yast-fonts.svg
  icons-dark/apps/32/yast-ftp-server.svg
  icons-dark/apps/32/yast-hardware.svg
  icons-dark/apps/32/yast-host.svg
  icons-dark/apps/32/yast-http-server.svg
  icons-dark/apps/32/yast-hwinfo.svg
  icons-dark/apps/32/yast-instserver.svg
  icons-dark/apps/32/yast-iscsi-client.svg
  icons-dark/apps/32/yast-iscsi-server.svg
  icons-dark/apps/32/yast-journal.svg
  icons-dark/apps/32/yast-kerberos-server.svg
  icons-dark/apps/32/yast-kernel.svg
  icons-dark/apps/32/yast-keyboard.svg
  icons-dark/apps/32/yast-lan.svg
  icons-dark/apps/32/yast-language.svg
  icons-dark/apps/32/yast-ldap-server.svg
  icons-dark/apps/32/yast-ldap.svg
  icons-dark/apps/32/yast-mail.svg
  icons-dark/apps/32/yast-messages.svg
  icons-dark/apps/32/yast-misc.svg
  icons-dark/apps/32/yast-network.svg
  icons-dark/apps/32/yast-nfs-server.svg
  icons-dark/apps/32/yast-nfs.svg
  icons-dark/apps/32/yast-nis-server.svg
  icons-dark/apps/32/yast-nis.svg
  icons-dark/apps/32/yast-ntp-client.svg
  icons-dark/apps/32/yast-proxy.svg
  icons-dark/apps/32/yast-release-notes.svg
  icons-dark/apps/32/yast-remote.svg
  icons-dark/apps/32/yast-samba-client.svg
  icons-dark/apps/32/yast-samba-server.svg
  icons-dark/apps/32/yast-security.svg
  icons-dark/apps/32/yast-services-manager.svg
  icons-dark/apps/32/yast-slp-server.svg
  icons-dark/apps/32/yast-snapper.svg
  icons-dark/apps/32/yast-software.svg
  icons-dark/apps/32/yast-sound.svg
  icons-dark/apps/32/yast-sudo.svg
  icons-dark/apps/32/yast-support.svg
  icons-dark/apps/32/yast-sw_single.svg
  icons-dark/apps/32/yast-sw_source.svg
  icons-dark/apps/32/yast-sysconfig.svg
  icons-dark/apps/32/yast-system.svg
  icons-dark/apps/32/yast-tftp-server.svg
  icons-dark/apps/32/yast-timezone.svg
  icons-dark/apps/32/yast-update.svg
  icons-dark/apps/32/yast-users.svg
  icons-dark/apps/32/yast-vm-install.svg
  icons-dark/apps/32/yast-vm-migrate.svg
  icons-dark/apps/32/yast-vm.svg
  icons-dark/apps/32/yast-vpn.svg
  icons-dark/apps/32/yast-wol.svg
  icons-dark/apps/32/yast.svg
  icons-dark/apps/48/yast-installation.svg
  icons-dark/apps/48/yast-sw_single.svg
  icons-dark/apps/48/yast.svg
  icons-dark/apps/64/yast-printer.svg
  icons-dark/apps/64/yast-scanner.svg
  icons/apps/32/yast-addon.svg
  icons/apps/32/yast-alternatives.svg
  icons/apps/32/yast-apparmor.svg
  icons/apps/32/yast-auth-client.svg
  icons/apps/32/yast-autoyast.svg
  icons/apps/32/yast-bootloader.svg
  icons/apps/32/yast-checkmedia.svg
  icons/apps/32/yast-create-new-vm.svg
  icons/apps/32/yast-device-tree.svg
  icons/apps/32/yast-dhcp-server.svg
  icons/apps/32/yast-disk.svg
  icons/apps/32/yast-dns-server.svg
  icons/apps/32/yast-docker.svg
  icons/apps/32/yast-dsl.svg
  icons/apps/32/yast-fcoe.svg
  icons/apps/32/yast-firewall.svg
  icons/apps/32/yast-fonts.svg
  icons/apps/32/yast-ftp-server.svg
  icons/apps/32/yast-hardware.svg
  icons/apps/32/yast-host.svg
  icons/apps/32/yast-http-server.svg
  icons/apps/32/yast-hwinfo.svg
  icons/apps/32/yast-instserver.svg
  icons/apps/32/yast-iscsi-client.svg
  icons/apps/32/yast-iscsi-server.svg
  icons/apps/32/yast-journal.svg
  icons/apps/32/yast-kerberos-server.svg
  icons/apps/32/yast-kernel.svg
  icons/apps/32/yast-keyboard.svg
  icons/apps/32/yast-lan.svg
  icons/apps/32/yast-language.svg
  icons/apps/32/yast-ldap-server.svg
  

D17310: Improve Kile icon with LaTeX font

[loh tar]

loh.tar added a comment.


  The used traffic cone by VLC is for me also nondescript, but that's their 
icon and therefore we know it.
  My point is only that your new icon should be somehow similar to the stuff 
used by Kile, and not to try to describe what we can do with Kile.
  
  Should I miss that your new icon will be the new Kile icon, I'm sorry for the 
noise :-)

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

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

To: trickyricky26, #vdg, ngraham, #kile
Cc: loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17619: Unit test and fix for bug 401552

[Jaime Torres Amate]

jtamate updated this revision to Diff 47666.
jtamate edited the summary of this revision.
jtamate edited the test plan for this revision.
jtamate added a comment.


  I think this time I got the problem right.
  One of the classics: I was modifying the list while it was being used.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17619?vs=47655=47666

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

AFFECTED FILES
  src/core/kcoredirlister.cpp

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17620: Make setHelp() public

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> kconfigdialog.h:176
>  
> +public Q_SLOTS:
> +/**

If I'm not mistaken, moving a function from `protected` to `public` breaks the 
ABI

REPOSITORY
  R265 KConfigWidgets

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

To: yurchor, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17620: Make setHelp() public

[Yuri Chornoivan]

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

REVISION SUMMARY
  Sometimes it is useful to connect configuration dialogs directly with the 
corresponding section in the manual (KmPlot, KAnagram). It was easy before KF 5:
  
m_settingsDialog->setHelp("configuration", QLatin1String("kmplot"));
  
  but now setHelp is protected and this is an untrivial task.

TEST PLAN
  The code just moved to public slots, should work as expected.

REPOSITORY
  R265 KConfigWidgets

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

AFFECTED FILES
  src/kconfigdialog.h

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

D17270: [KUrlNavigator] List subdirs of a parent folder of an archive

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> kurlnavigator.h:297
> + */
> +bool isInsideCompressedPath(const QUrl ) const;
> +

I'd make this function `static`, since it doesn't depend on the status of a 
specific `KUrlNavigator` instance.

To do so, `isCompressedPath()` needs to become a (static) free function.

> kurlnavigatorbutton.cpp:416
> +auto kurlnavigator = qobject_cast(parent());
> +if 
> (kurlnavigator->isInsideCompressedPath(kurlnavigator->locationUrl())) {
> +// We are in an archive, check whether the subdir we have to 
> list is part of the archive

What if the parent is not a `KUrlNavigator`?

> thsurrel wrote in kurlnavigatorbutton.cpp:414
> What would be the clean way to expose this function ? I wouldn't like to 
> duplicate the code.
> And btw, the "if ((m_url.scheme() == QLatin1String("tar")) || (m_url.scheme() 
> == QLatin1String("zip")))" condition was taken from 
> KUrlNavigator::setLocationUrl, so there must be a bugfix to be made there as 
> well with the 'krarc' case gregormi reports.

> What would be the clean way to expose this function ? I wouldn't like to 
> duplicate the code.

Sorry for the late reply. One way could be a public static function, either in 
`KUrlNavigator` or `KUrlNavigatorButton`.

REPOSITORY
  R241 KIO

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

To: thsurrel, #frameworks
Cc: elvisangelaccio, gregormi, kde-frameworks-devel, michaelh, ngraham, bruns

D17500: Restore mobipocket extractor

[Albert Astals Cid]

aacid added a comment.


  I would really like the module to be in the mobipocket repo, would serve as 
actual proof the extractors outside the main module work if we have an example 
of it :)

INLINE COMMENTS

> FindQMobipocket.cmake:1
> +# - Find QMobipocket
> +# Find the Qmobipocket library.

This is wrong, mobipocket installs its own cmake files so you don't need a find 
module.

REPOSITORY
  R286 KFileMetaData

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

To: astippich, bruns
Cc: ltoscano, mgallien, aacid, kde-frameworks-devel, #baloo, ashaposhnikov, 
michaelh, astippich, spoorun, ngraham, bruns, abrahams

D17245: Add string formatting function to property info

[Alexander Stippich]

astippich added a comment.


  String freeze is approaching next week, and I would like to get this in for 
5.54 so that I can start porting Dolphin and baloo-widgets. Anyone?

REPOSITORY
  R286 KFileMetaData

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

To: astippich, broulik, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D17245: Add string formatting function to property info

[Alexander Stippich]

astippich updated this revision to Diff 47660.
astippich added a comment.


  - improve code readability

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17245?vs=46859=47660

BRANCH
  display_value

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

AFFECTED FILES
  CMakeLists.txt
  autotests/propertyinfotest.cpp
  autotests/propertyinfotest.h
  src/CMakeLists.txt
  src/formatstrings.cpp
  src/formatstrings.h
  src/propertyinfo.cpp
  src/propertyinfo.h

To: astippich, broulik, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D17550: Make the open url in tab feature a bit more discoverable

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> kurlnavigator.cpp:480
> +if (q->receivers(SIGNAL(tabRequested(QUrl))) > 0) {
> +for(auto button : qAsConst(m_navButtons)) {
> +if (button->geometry().contains(p)) {

Space after `for`

> kurlnavigator.cpp:483
> +const auto url = button->url();
> +QAction* openInTab = 
> popup->addAction(QIcon::fromTheme(QStringLiteral("tab-new")), i18n("Open %1 
> in tab", button->text()));
> +q->connect(openInTab, ::triggered, q, [this, url](){ 
> Q_EMIT q->tabRequested(url); });

Do we really have to put the folder name in the extry text? This will make the 
popup menu very large if the folder name is long.

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: apol, #dolphin, ngraham, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

D17548: Fixed link to the coding style wiki page

[Alexander Stippich]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:5d9f4415f7a8: Fixed link to the coding style wiki page 
(authored by janpr, committed by astippich).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17548?vs=47473=47659

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

AFFECTED FILES
  README.md

To: janpr, #baloo, broulik
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17310: Improve Kile icon with LaTeX font

[TrickyRicky]

trickyricky26 edited the summary of this revision.
trickyricky26 edited the test plan for this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

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

To: trickyricky26, #vdg, ngraham, #kile
Cc: loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17310: Improve Kile icon with LaTeX font

[TrickyRicky]

trickyricky26 updated this revision to Diff 47658.
trickyricky26 added a comment.


  - Use Cantor formula design with a bigger pen

REPOSITORY
  R266 Breeze Icons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17310?vs=47591=47658

BRANCH
  improve-kile-icon (branched from master)

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

AFFECTED FILES
  icons-dark/apps/48/kile.svg
  icons/apps/48/kile.svg

To: trickyricky26, #vdg, ngraham, #kile
Cc: loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17310: Improve Kile icon with LaTeX font

[TrickyRicky]

trickyricky26 added a comment.


  In D17310#377145 , @loh.tar wrote:
  
  > For me is these root sign nondescript. When the orig old blue symbol is not 
desired, how about to freshen it up? How? I don't know.
  >  Perhaps Black & White just as all your other offers on these rectangular 
sheet of paper?
  
  
  This icon is supposed to fit that style. 
  However, instead of black lines on a white background which looks more like 
simple text editing, this uses a mathematical formula which is one thing LaTeX 
handles wonderfully and is often used for.
  The pen shows this is an application for editing/generating such files.
  I can't think of a better symbol for LaTeX other than it's "Logo" which is 
just the Text LaTeX.
  
  > Furthermore exist these splash-screen. So maybe in yellow on these fuzzy 
gray paper?
  >  F6477111: kile_splash.png 
  
  This would not work at a small size and imo isn't very visually appealing.

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

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

To: trickyricky26, #vdg, ngraham, #kile
Cc: loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17619: Unit test and fix for bug 401552

[Elvis Angelaccio]

elvisangelaccio added inline comments.

INLINE COMMENTS

> kdirlistertest.cpp:1299-1308
> +const QString dirX = m_tempDir.path() + "/x";
> +QVERIFY(QDir().mkdir(dirX));
> +const QString dirX1 = m_tempDir.path() + "/x/x1";
> +QVERIFY(QDir().mkdir(dirX1));
> +const QString dirX2 = m_tempDir.path() + "/x/x1/x2";
> +QVERIFY(QDir().mkdir(dirX2));
> +const QString dirX3 = m_tempDir.path() + "/x/x1/x2/x3";

I was not able to reproduce the crash with the steps you described in 
https://phabricator.kde.org/D10742#377375.

I can semi-reliably reproduce it by renaming a non-empty folder expanded in the 
dolphin detail view.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.9 - Build # 379 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.9/379/
 Project:
kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 16 Dec 2018 10:37:57 +
 Build duration:
8 min 47 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 1 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiogui_favicontestName: 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)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(262/398)66%
(262/398)53%
(32050/60118)38%
(16554/44090)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(57/57)100%
(57/57)95%
(9091/9538)48%
(4278/8965)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core86%
(100/116)86%
(100/116)58%
(8320/14340)50%
(4662/9267)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3908/7970)34%
(1603/4743)src.gui100%
(2/2)100%
(2/2)88%
(95/108)68%
(45/66)src.ioslaves.file100%
(5/5)100%
(5/5)51%
(531/1036)37%
(318/868)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1344)0%
(0/1416)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/248)0%
(0/148)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1770/4288)35%
(1304/3692)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1330)55%
(626/1135)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(73/267)8%
(14/184)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
 

D17619: Unit test and fix for bug 401552

[Elvis Angelaccio]

elvisangelaccio added a comment.


  Unfortunately it's still crashing for me, even with this patch.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: elvisangelaccio, kde-frameworks-devel, michaelh, ngraham, bruns

KDE CI: Frameworks » kio » kf5-qt5 SUSEQt5.11 - Build # 18 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks/job/kio/job/kf5-qt5%20SUSEQt5.11/18/
 Project:
kf5-qt5 SUSEQt5.11
 Date of build:
Sun, 16 Dec 2018 10:37:57 +
 Build duration:
6 min 49 sec and counting
   BUILD ARTIFACTS
  compat_reports/KF5KIO_compat_report.html
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 1 test(s), Skipped: 0 test(s), Total: 1 test(s)Name: projectroot Failed: 1 test(s), Passed: 51 test(s), Skipped: 0 test(s), Total: 52 test(s)Failed: projectroot.autotests.kiogui_favicontestName: 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)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)66%
(262/398)66%
(262/398)53%
(32054/60120)38%
(16556/44090)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(57/57)100%
(57/57)95%
(9091/9538)48%
(4278/8965)autotests.http100%
(5/5)100%
(5/5)99%
(581/582)68%
(113/166)autotests.kcookiejar100%
(1/1)100%
(1/1)91%
(179/197)72%
(49/68)src100%
(1/1)100%
(1/1)86%
(6/7)67%
(4/6)src.core86%
(100/116)86%
(100/116)58%
(8334/14342)50%
(4668/9263)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3908/7970)34%
(1603/4743)src.gui100%
(2/2)100%
(2/2)88%
(95/108)68%
(45/66)src.ioslaves.file100%
(5/5)100%
(5/5)51%
(531/1036)37%
(318/868)src.ioslaves.file.kauth0%
(0/2)0%
(0/2)0%
(0/106)0%
(0/65)src.ioslaves.ftp0%
(0/1)0%
(0/1)0%
(0/1344)0%
(0/1416)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/248)0%
(0/148)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1770/4288)35%
(1304/3692)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(630/1330)55%
(626/1135)src.ioslaves.remote100%
(2/2)100%
(2/2)27%
(73/267)8%
(14/184)src.ioslaves.remote.kdedmodule0%
(0/2)0%
 

D17617: Display error instead of silently failing when asked to create folder that already exists

[Elvis Angelaccio]

elvisangelaccio added a comment.


  Note that `mkdir` also doesn't warn if the folder already exists. I guess 
`KNewFileMenu` was designed to behave accordingly.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, elvisangelaccio
Cc: elvisangelaccio, Codezela, kde-frameworks-devel, michaelh, ngraham, bruns

T3689: Add abi compliance checker to CI

[Ben Cooksley]

bcooksley added a comment.


  Okay, there have been a number of comments here since i've last read it, so a 
bit to catch up on and go over.
  
  In regards to `akonadi-mime` I have now re-run it, and it seems to work fine.
  As long as you're relying only on the master manifest.yaml, then it should be 
impossible for any publishing failure to cause issues.
  
  In terms of the `akonadi-search` issue, using something like that YAML file 
should be fine yes and is historically how we've solved such issues.

TASK DETAIL
  https://phabricator.kde.org/T3689

To: knauss, bcooksley
Cc: dfaure, kde-frameworks-devel, bcooksley, sysadmin, scarlettclark, aacid, 
knauss, alexeymin, kaning, blazquez

D17619: Unit test and fix for bug 401552

[Jaime Torres Amate]

jtamate edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D10742: get rid of the raw KFileItem pointers in KCoreDirListerCache

[Jaime Torres Amate]

jtamate added a comment.


  Let's continue on D17619 

REPOSITORY
  R241 KIO

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

To: jtamate, #frameworks, dfaure
Cc: elvisangelaccio, bruns, kde-frameworks-devel, mwolff, markg, michaelh, 
ngraham

D17619: Unit test and fix for bug 401552

[Jaime Torres Amate]

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

REVISION SUMMARY
  Add a unit test that fails before applying the patch and doesn't after the 
patch (in skeleton only)
  
  Could someone that was able to reproduce the crash can test the patch to see 
if it really fixes the crash?

TEST PLAN
  Fail in the unit test before applying the patch
  Doesn't fail after applying the patch.

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  autotests/kdirlistertest.cpp
  autotests/kdirlistertest.h
  src/core/kcoredirlister.cpp

To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns