D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

[Shubham]

shubham updated this revision to Diff 39163.
shubham added a comment.


  Re factor
  Remove unnecessary indentations
  use only !itemList.supportsMoving()

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14610?vs=39112=39163

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

AFFECTED FILES
  src/widgets/kpropertiesdialog.cpp

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

KDE CI: Frameworks kdelibs4support kf5-qt5 FreeBSDQt5.10 - Build # 18 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kdelibs4support%20kf5-qt5%20FreeBSDQt5.10/18/
 Project:
Frameworks kdelibs4support kf5-qt5 FreeBSDQt5.10
 Date of build:
Mon, 06 Aug 2018 03:33:59 +
 Build duration:
39 min and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 37 test(s), Skipped: 0 test(s), Total: 39 test(s)Failed: TestSuite.kmimetypetestFailed: TestSuite.kstandarddirstest

KDE CI: Frameworks kdelibs4support kf5-qt5 SUSEQt5.10 - Build # 58 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kdelibs4support%20kf5-qt5%20SUSEQt5.10/58/
 Project:
Frameworks kdelibs4support kf5-qt5 SUSEQt5.10
 Date of build:
Mon, 06 Aug 2018 03:33:59 +
 Build duration:
23 min and counting
   JUnit Tests
  Name: (root) Failed: 2 test(s), Passed: 38 test(s), Skipped: 0 test(s), Total: 40 test(s)Failed: TestSuite.kglobalsettingstestFailed: TestSuite.ktabwidget_unittest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report29%
(4/14)43%
(124/291)43%
(124/291)47%
(21588/45958)38%
(13037/34249)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault0%
(0/1)0%
(0/1)0%
(0/87)0%
(0/56)autotests98%
(44/45)98%
(44/45)97%
(11355/11759)50%
(7165/14375)src0%
(0/1)0%
(0/1)0%
(0/4)0%
(0/2)src.kdebugdialog0%
(0/7)0%
(0/7)0%
(0/256)0%
(0/73)src.kdecore76%
(65/86)76%
(65/86)53%
(9476/17864)48%
(5546/11459)src.kdeui19%
(13/68)19%
(13/68)8%
(747/9693)6%
(325/5364)src.kio7%
(2/27)7%
(2/27)0%
(10/2258)0%
(1/1238)src.kioslave.metainfo0%
(0/1)0%
(0/1)0%
(0/32)0%
(0/4)src.kparts0%
(0/1)0%
(0/1)0%
(0/24)0%
(0/12)src.kssl0%
(0/8)0%
(0/8)0%
(0/1807)0%
(0/853)src.kssl.kcm0%
(0/3)0%
(0/3)0%
(0/264)0%
(0/145)src.solid0%
(0/3)0%
(0/3)0%
(0/188)0%
(0/87)src.solid-networkstatus.kded0%
(0/6)0%
(0/6)0%
(0/185)0%
(0/94)tests0%
(0/34)0%
(0/34)0%
(0/1537)0%
(0/487)

KDE CI: Frameworks kdelibs4support kf5-qt5 SUSEQt5.9 - Build # 32 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kdelibs4support%20kf5-qt5%20SUSEQt5.9/32/
 Project:
Frameworks kdelibs4support kf5-qt5 SUSEQt5.9
 Date of build:
Mon, 06 Aug 2018 03:33:59 +
 Build duration:
22 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 39 test(s), Skipped: 0 test(s), Total: 40 test(s)Failed: TestSuite.ktabwidget_unittest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report29%
(4/14)43%
(124/291)43%
(124/291)47%
(21750/45961)38%
(13184/34249)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsdefault0%
(0/1)0%
(0/1)0%
(0/87)0%
(0/56)autotests98%
(44/45)98%
(44/45)98%
(11469/11759)50%
(7254/14375)src0%
(0/1)0%
(0/1)0%
(0/4)0%
(0/2)src.kdebugdialog0%
(0/7)0%
(0/7)0%
(0/256)0%
(0/73)src.kdecore76%
(65/86)76%
(65/86)53%
(9506/17866)49%
(5599/11459)src.kdeui19%
(13/68)19%
(13/68)8%
(765/9694)6%
(330/5364)src.kio7%
(2/27)7%
(2/27)0%
(10/2258)0%
(1/1238)src.kioslave.metainfo0%
(0/1)0%
(0/1)0%
(0/32)0%
(0/4)src.kparts0%
(0/1)0%
(0/1)0%
(0/24)0%
(0/12)src.kssl0%
(0/8)0%
(0/8)0%
(0/1807)0%
(0/853)src.kssl.kcm0%
(0/3)0%
(0/3)0%
(0/264)0%
(0/145)src.solid0%
(0/3)0%
(0/3)0%
(0/188)0%
(0/87)src.solid-networkstatus.kded0%
(0/6)0%
(0/6)0%
(0/185)0%
(0/94)tests0%
(0/34)0%
(0/34)0%
(0/1537)0%
(0/487)

D14449: Modify device usage information

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> kpropertiesdialog.cpp:1189
> +grid->addWidget(l, curRow, 2);
> +l->setText(i18nc("Device usage information","%1 used , %2 free", 
> KIO::convertSize(size - free), KIO::convertSize(free)));
> +

Also, style for i18n strings: there must be no space before comma.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx
Cc: pino, rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14641: Refine wording when a folder with an invalid name could not be created

[Nathaniel Graham]

ngraham added a comment.


  In D14641#304077 , @markuss wrote:
  
  > If you remove the manual new-line, will everything be in a single line with 
a wide window or will Qt just add a line break where it fits?
  
  
  The window becomes really wide, yeah. That's why I added a manual line break.

REPOSITORY
  R241 KIO

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

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

D14641: Refine wording when a folder with an invalid name could not be created

[Markus Slopianka]

markuss added a comment.


  If you remove the manual new-line, will everything be in a single line with a 
wide window or will Qt just add a line break where it fits?
  
  I ask because with a shot name like ".." the line break looks unnatural as if 
the next line is a new sentence.

REPOSITORY
  R241 KIO

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

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

D14643: Bump the minimum logging category to Warning

[Luigi Toscano]

ltoscano created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
ltoscano requested review of this revision.

REVISION SUMMARY
  Also, break the long statements when needed, trying to follow
  the style of the rest of the file.

TEST PLAN
  It compiles.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/core/CMakeLists.txt
  src/gui/CMakeLists.txt
  src/ioslaves/remote/CMakeLists.txt
  src/ioslaves/trash/CMakeLists.txt
  src/ioslaves/trash/tests/CMakeLists.txt
  src/kioexec/CMakeLists.txt
  src/widgets/CMakeLists.txt

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

Re: purpose unittest (AlternativesModelTest::runJobTest) fails randomly

[Aleix Pol]

On Sat, Aug 4, 2018 at 12:31 PM David Faure  wrote:
>
> Any idea about this test that fails sometimes? Timing issue? The wait() 
> should be longer in case of slow network?
>
> https://build.kde.org/view/Frameworks/job/Frameworks%20purpose%20kf5-qt5%20SUSEQt5.10/92/testReport/junit/(root)/TestSuite/alternativesmodeltest/

Hi,
Yes, I looked into fixing it, it seemed like it was but then it wasn't
(yay randomness!).

My guess is that it's because kf5.kio.core.copyjob prints the whole
URL which in this case it's a data one (which I tried to fix in
64492796aa7ca4b63dbae419f47e14da15fbcc51). I failed because it still
prints I added "+
QLoggingCategory::setFilterRules(QStringLiteral("kf5.kio.core.copyjob=false"));"
I'm not sure why this has the effect I hoped it would. Any ideas?

Aleix

D14449: Modify device usage information

[Henrik Fehlauer]

rkflx added a comment.


  In D14449#303766 , @shubham wrote:
  
  > In D14449#300327 , @ngraham 
wrote:
  >
  > > +1 for using the same label but putting the information on more than one 
line like @rkflx suggests.
  >
  
  
  As far as I can see that comment was part of the discussion about what to 
show, which I think we now concluded. I don't think that was meant as an advice 
regarding implementation.
  
  You are already creating a new label (which is fine, also where you're doing 
it is the perfect spot). I was only wondering whether the call to `setText` 
should better be placed elsewhere.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14641: Refine wording when a folder with an invalid name could not be created

[Nathaniel Graham]

ngraham retitled this revision from "Refine wording when a folder with an 
invalid name could not be creted" to "Refine wording when a folder with an 
invalid name could not be created".

REPOSITORY
  R241 KIO

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

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

D14641: Refine wording when a folder with an invalid name could not be creted

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

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

D14641: Refine wording when a folder with an invalid name could not be creted

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Dolphin, Frameworks, VDG, tmarshall.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  This is follow-on for D13805  that 
improves the wording to be less redundant and a little bit clearer.

TEST PLAN
  Before:
  [image goes here]
  After:
  [image goes here]

REPOSITORY
  R241 KIO

BRANCH
  refine-wording (branched from master)

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp

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

D14640: Use appropriate icon for a cancel button that will ask for a new name

[Nathaniel Graham]

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

REPOSITORY
  R241 KIO

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

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

D14640: Use appropriate icon for a cancel button that will ask for a new name

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Frameworks, VDG.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  When a dialog has a cancel button that says, "Enter a different name", give 
that button the `edit-delete` icon instead of the standard center icon.

REPOSITORY
  R241 KIO

BRANCH
  rename-icons (branched from master)

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

AFFECTED FILES
  src/filewidgets/knewfilemenu.cpp

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

KDE CI: Frameworks kwayland kf5-qt5 FreeBSDQt5.10 - Build # 40 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20FreeBSDQt5.10/40/
 Project:
Frameworks kwayland kf5-qt5 FreeBSDQt5.10
 Date of build:
Sun, 05 Aug 2018 16:05:39 +
 Build duration:
5 hr 51 min and counting
   JUnit Tests
  Name: (root) Failed: 38 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 40 test(s)Failed: TestSuite.kwayland-testAppmenuFailed: TestSuite.kwayland-testBlurFailed: TestSuite.kwayland-testCompositorFailed: TestSuite.kwayland-testContrastFailed: TestSuite.kwayland-testDataDeviceFailed: TestSuite.kwayland-testDataSourceFailed: TestSuite.kwayland-testDragAndDropFailed: TestSuite.kwayland-testErrorFailed: TestSuite.kwayland-testFilterFailed: TestSuite.kwayland-testIdleFailed: TestSuite.kwayland-testPlasmaShellFailed: TestSuite.kwayland-testPointerConstraintsFailed: TestSuite.kwayland-testQtSurfaceExtensionFailed: TestSuite.kwayland-testRegionFailed: TestSuite.kwayland-testRemoteAccessFailed: TestSuite.kwayland-testSelectionFailed: TestSuite.kwayland-testServerSideDecorationFailed: TestSuite.kwayland-testServerSideDecorationPaletteFailed: TestSuite.kwayland-testShadowFailed: TestSuite.kwayland-testShmPoolFailed: TestSuite.kwayland-testSlideFailed: TestSuite.kwayland-testSubCompositorFailed: TestSuite.kwayland-testSubSurfaceFailed: TestSuite.kwayland-testTextInputFailed: TestSuite.kwayland-testWaylandConnectionThreadFailed: TestSuite.kwayland-testWaylandOutputFailed: TestSuite.kwayland-testWaylandOutputDeviceFailed: TestSuite.kwayland-testWaylandOutputManagementFailed: TestSuite.kwayland-testWaylandRegistryFailed: TestSuite.kwayland-testWaylandServerDisplayFailed: TestSuite.kwayland-testWaylandShellFailed: TestSuite.kwayland-testWaylandSurfaceFailed: TestSuite.kwayland-testWindowmanagementFailed: TestSuite.kwayland-testXdgForeignFailed: TestSuite.kwayland-testXdgOutputFailed: TestSuite.kwayland-testXdgShellStableFailed: TestSuite.kwayland-testXdgShellV5Failed: TestSuite.kwayland-testXdgShellV6

KDE CI: Frameworks kio kf5-qt5 FreeBSDQt5.10 - Build # 103 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20FreeBSDQt5.10/103/
 Project:
Frameworks kio kf5-qt5 FreeBSDQt5.10
 Date of build:
Sun, 05 Aug 2018 20:31:47 +
 Build duration:
1 hr 11 min and counting
   JUnit Tests
  Name: (root) Failed: 5 test(s), Passed: 53 test(s), Skipped: 0 test(s), Total: 58 test(s)Failed: TestSuite.kiocore-jobtestFailed: TestSuite.kiocore-kmountpointtestFailed: TestSuite.kiofilewidgets-kfileplacesviewtestFailed: TestSuite.kiowidgets-kdirlistertestFailed: TestSuite.kiowidgets-kdirmodeltest

D14639: Add syntax-highlighting for RDoc (R documentation)

[Aaron Puchert]

aaronpuchert created this revision.
aaronpuchert added reviewers: dhaumann, cullmann.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel.
aaronpuchert requested review of this revision.

REVISION SUMMARY
  The R documentation language follows LaTeX-like syntax with occasional
  snippets of R code. Builtin keywords and user-defined macros both begin
  with a backlash, but we try to distinguish them to highlight arguments
  according to their type.
  
  For macros we can't know the argument types (LaTeX/R-like or verbatim),
  so we just assume the type is the same as that of the surrounding code.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  rdoc

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

AFFECTED FILES
  autotests/folding/test.Rd.fold
  autotests/html/test.Rd.html
  autotests/input/test.Rd
  autotests/reference/test.Rd.ref
  data/syntax/rdoc.xml

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

Re: ktextwidgets on FreeBSD

[David Faure]

On dimanche 5 août 2018 01:41:01 CEST Ben Cooksley wrote:
> On Sat, Aug 4, 2018 at 11:06 PM, David Faure  wrote:
> > Anyone running FreeBSD, who could to try and debug this hanging unittest
> > from the ktextwidgets framework?
> > 
> > https://build.kde.org/view/Frameworks/job/Frameworks%20ktextwidgets%20kf5-> 
> > > qt5%20FreeBSDQt5.10/9/testReport/junit/(root)/TestSuite/ktextwidgets_krich
> > textedittest/
> > 
> > There's no waiting of any kind in the code, it's straight QTextDocument
> > usage, this shouldn't deadlock or anything. Yet it times out (after 30
> > seconds!!) quite often in CI.
> 
> This is a rather unusual failure, as the CI system is giving that test
> 5 minutes to timeout, after the test has finished.
> 
> Usually this happens when tests start background daemons and don't
> kill those processes off before the test exits (which CTest doesn't
> like).
> However I don't think it's a background process sticking around in
> this case as those type of hangs are usually
> manual-intervention-required ones, which this isn't.
> 
> Unfortunately it seems that the issue has gone away though so it's not
> possible to dig into this much further...
> https://build.kde.org/view/Frameworks/job/Frameworks%20ktextwidgets%20kf5-qt
> 5%20FreeBSDQt5.10/22/

It went away because the freebsd people made a fix and forgot to CC this list 
;-)

See attached email.

Thanks everyone! One more green (well, blue) in the CI...

-- 
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
--- Begin Message ---
I replaced the backend of qt5-speech with flite instead of
speech-dispatcher which seemed to hang every 20-30th time
let's see if that is enough.

mfg Tobias

On Sat, 4 Aug 2018 at 16:13, Adriaan de Groot  wrote:

> On Saturday, 4 August 2018 15:09:02 CEST Adriaan de Groot wrote:
> > On Saturday, 4 August 2018 13:06:42 CEST David Faure wrote:
> > > There's no waiting of any kind in the code, it's straight QTextDocument
> > > usage, this shouldn't deadlock or anything. Yet it times out (after 30
> > > seconds!!) quite often in CI.
> >
> > I just built master -- after changing the CMakeLists.txt to accept
> > Frameworks 5.46 which I have on my live desktop -- and ran the tests
> twice
> > with "make test".
>
> On the CI nodes themselves, this does lock up, like so: (but I have no
> idea
> what to do about it)
>
> (gdb) thread apply 1 bt
>
> Thread 1 (LWP 101006 of process 16170):
> #0  0x000803a2fc7c in ?? () from /lib/libthr.so.3
> #1  0x000803a2c325 in ?? () from /lib/libthr.so.3
> #2  0x000812e45306 in spd_close () from /usr/local/lib/libspeechd.so.2
> #3  0x000812c377f6 in
> QTextToSpeechEngineSpeechd::~QTextToSpeechEngineSpeechd
> (this=0x60626840)
> at
> /wrkdirs/usr/ports/accessibility/qt5-speech/work/qtspeech-everywhere-
> src-5.10.1/src/plugins/tts/speechdispatcher/qtexttospeech_speechd.cpp:78
> #4  0x000812c37889 in
> QTextToSpeechEngineSpeechd::~QTextToSpeechEngineSpeechd
> (this=0x60626840)
> at
> /wrkdirs/usr/ports/accessibility/qt5-speech/work/qtspeech-everywhere-
> src-5.10.1/src/plugins/tts/speechdispatcher/qtexttospeech_speechd.cpp:74
> #5  0x0008049094f1 in QTextToSpeechPrivate::~QTextToSpeechPrivate
> (this=0x60e12aa0) at /wrkdirs/usr/ports/accessibility/qt5-speech/work/
> qtspeech-everywhere-src-5.10.1/src/tts/qtexttospeech.cpp:88
> #6  0x000804909589 in QTextToSpeechPrivate::~QTextToSpeechPrivate
> (this=0x60e12aa0) at /wrkdirs/usr/ports/accessibility/qt5-speech/work/
> qtspeech-everywhere-src-5.10.1/src/tts/qtexttospeech.cpp:86
> #7  0x000802dfce6b in QScopedPointerDeleter::cleanup
> (pointer=0x60e12aa0) at ../../include/QtCore/../../src/corelib/tools/
> qscopedpointer.h:60
> #8  0x000802df9530 in QScopedPointer QScopedPointerDeleter >::~QScopedPointer
> (this=0x6021ed98) at
> ../../include/QtCore/../../src/corelib/tools/qscopedpointer.h:107
> #9  0x000802dec748 in QObject::~QObject (this=0x6021ed90) at
> kernel/
> qobject.cpp:1033
> #10 0x00080490f405 in QTextToSpeech::~QTextToSpeech
> (this=0x6021ed90)
> at
> .moc/../../../../qtspeech-everywhere-src-5.10.1/src/tts/qtexttospeech.h:54
> #11 0x00080490f429 in QTextToSpeech::~QTextToSpeech
> (this=0x6021ed90)
> at
> .moc/../../../../qtspeech-everywhere-src-5.10.1/src/tts/qtexttospeech.h:54
> #12 0x0008003b2d33 in KTextEdit::Private::~Private
> (this=0x60d16350)
> at /usr/home/jenkins/workspace/Frameworks ktextwidgets kf5-qt5
> FreeBSDQt5.10/
> src/widgets/ktextedit.cpp:99
> #13 0x0008003a0d1a in KTextEdit::~KTextEdit (this=0x7fffce00) at
> /usr/
> home/jenkins/workspace/Frameworks ktextwidgets kf5-qt5 FreeBSDQt5.10/src/
> widgets/ktextedit.cpp:341
> #14 0x000800375fca in KRichTextEdit::~KRichTextEdit
> (this=0x7fffce00)
> at /usr/home/jenkins/workspace/Frameworks ktextwidgets kf5-qt5
> FreeBSDQt5.10/
> src/widgets/krichtextedit.cpp:133
> #15 0x002be6fe in KRichTextEditTest::testUpdateLinkAdd
> 

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

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.9/193/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 05 Aug 2018 20:31:47 +
 Build duration:
24 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 58 test(s), Skipped: 0 test(s), Total: 59 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesviewtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(258/396)65%
(258/396)53%
(31970/59912)38%
(16153/42670)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(55/55)100%
(55/55)95%
(9025/9455)51%
(3931/7716)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.core84%
(98/116)84%
(98/116)58%
(8355/14358)50%
(4663/9289)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3894/7924)34%
(1586/4669)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(528/1015)39%
(316/814)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/1364)0%
(0/1414)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/245)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1770/4320)35%
(1306/3700)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(628/1330)55%
(619/1123)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/257)7%
(14/212)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash56%
(5/9)56%
(5/9)52%
  

D14467: Auth Support: Drop privileges if target is not owned by root

[David Faure]

dfaure added a comment.


  +1, the idea seems sane to me, but I'm no expert with this kind of API so I'd 
like someone else to check it.

REPOSITORY
  R241 KIO

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

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

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

[David Faure]

dfaure added a comment.


  Hmm, well, for IconApplet's use case, if you never want a readonly line-edit 
then a major redesign is needed, switching from the lineedit to the qlabel 
inside of setFileNameReadOnly itself...
  (and using that in the code modified by this patch...). Or adding a ctor 
flag, to avoid creating+destroying widgets right away...
  Given the number of iterations just to get the simple case right, let's do 
that separately, ok? I could do it myself, faster than doing 10 reviews :-)

REPOSITORY
  R241 KIO

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

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

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

[David Faure]

dfaure added a comment.


  `KFilePropsPlugin::setFileNameReadOnly` checks for m_bFromTemplate for the 
case where the properties dialog is used by the "Create New / ..." context menu 
action.
  In that case, the source file (the template from /usr) isn't movable, but we 
still want to let the user type a filename -- for the copied file.
  
  Doing this inside that method won't work anymore, we need to incorporate it 
into the new if() for using the label. For instance like this:
  
  `if (d->bMultiple || isTrash || hasRoot || (!m_bFromTemplate && 
!itemList.supportsMoving()))`

REPOSITORY
  R241 KIO

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

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

KDE CI: Frameworks kio kf5-qt5 SUSEQt5.10 - Build # 348 - Still Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/348/
 Project:
Frameworks kio kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 05 Aug 2018 20:31:47 +
 Build duration:
6 min 12 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 58 test(s), Skipped: 0 test(s), Total: 59 test(s)Failed: TestSuite.kiofilewidgets-kfileplacesviewtest
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report64%
(23/36)65%
(258/396)65%
(258/396)53%
(31881/59911)38%
(16105/42672)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(55/55)100%
(55/55)95%
(9023/9455)51%
(3928/7716)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.core84%
(98/116)84%
(98/116)58%
(8284/14357)50%
(4635/9285)src.core.kssl100%
(1/1)100%
(1/1)40%
(35/88)50%
(3/6)src.filewidgets76%
(28/37)76%
(28/37)49%
(3894/7924)34%
(1586/4669)src.gui100%
(2/2)100%
(2/2)94%
(103/109)74%
(49/66)src.ioslaves.file100%
(5/5)100%
(5/5)52%
(527/1015)39%
(315/814)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/1364)0%
(0/1414)src.ioslaves.help0%
(0/5)0%
(0/5)0%
(0/245)0%
(0/144)src.ioslaves.http88%
(7/8)88%
(7/8)41%
(1775/4320)35%
(1304/3700)src.ioslaves.http.kcookiejar33%
(2/6)33%
(2/6)47%
(629/1330)55%
(620/1123)src.ioslaves.remote100%
(2/2)100%
(2/2)28%
(72/257)7%
(14/212)src.ioslaves.remote.kdedmodule0%
(0/2)0%
(0/2)0%
(0/12)100%
(0/0)src.ioslaves.telnet0%
(0/1)0%
(0/1)0%
(0/43)0%
(0/30)src.ioslaves.trash56%
(5/9)56%

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

[David Faure]

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


  Please re-read your diff and, indeed, revert any unnecessary changes like 
indentation or no-op moving of code.

INLINE COMMENTS

> shubham wrote in kpropertiesdialog.cpp:988
> this test was intended for blocking directories without write access to 
> rename , but as you say it should be done to it's child
> 
> I missed that one & by mistake

You can remove the last condition completely. `!itemList.supportsMoving()` is 
enough to detect the case of files in a non-writable directory. See the 
unittest I just pushed about that, commit 0799ca8f32 
.

REPOSITORY
  R241 KIO

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

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

D13869: [solid] Notify when interface to mounted fs is lost

[Anthony Fieroni]

anthonyfieroni added a comment.


  
  
  In D13869#303905 , @bruns wrote:
  
  > And thinking about it, this very likely makes your code wrong. Have you 
tried mounting a filesystem again after unmounting it, **using via any mean 
implemented via solid**? That is, "solid-hardware mount ...", the device 
notifier, ...
  
  
  So write an example with commands, mounting, umounting flash drive, which is 
a filesystem, works.

REPOSITORY
  R245 Solid

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

To: anthonyfieroni, broulik, cfeck, dfaure, bruns
Cc: ngraham, bcooksley, bruns, kde-frameworks-devel, michaelh

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

[David Faure]

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

INLINE COMMENTS

> batchrenametypestest.h:20
> +
> +#ifndef KIO_RENAMETYPESTEST_H
> +#define KIO_RENAMETYPESTEST_H

Doesn't match the header filename, and anyway, feel free to move this to the 
.cpp file, for qtestlib unittests.

> batchrenamedialog.cpp:193
> +
> +params[QStringLiteral("timestamp")] = QDateTime::currentDateTime();
> +

params.insert(key, value) is faster than params[key] = value, see the book 
Effective C++ (or was it Effective STL?)

> batchrenamedialog.cpp:221
> +bool valid = (newName == itemName) || !(newName.isEmpty() || exists);
> +allItemsOk &= valid;
> +generatedNames.append(newName);

bitfield operation on booleans can lead to unexpected behaviour.
Use `allItemsOk = allItemsOk && valid` instead, or `if (!valid) allItemsOk = 
false;`

> batchrenamedialog.cpp:257
> +
> +if (!job->error()) {
> +m_renamedItems << newUrl;

This makes no sense, you can't test for a job error before the job is done.
Connect to the result signal and do that in the slot (e.g. lambda) if it should 
really only be done on success -- after the move happened.

> batchrenamedialog.cpp:264
> +{
> +for (const auto : m_itemsToBeRenamed) {
> +renameItem(pair.first, pair.second);

qAsConst(m_itemsToBeRenamed)

> batchrenamedialog.cpp:287
> +{
> +Q_UNUSED(checked);
> +

Just remove the argument from the slot then.

> batchrenamedialog.cpp:365
> +for (int i = 0; i < capturedGroups.length(); i++) {
> +if (capturedGroups[i].length() == 0) {
> +continue;

use .isEmpty()

> batchrenamedialog.cpp:377
> +}
> \ No newline at end of file


Please configure your text editor to add newlines at the end of files. 
Surprising that it doesn't do that, most do.

> batchrenamedialog.h:44
> +{
> +Q_OBJECT
> +

indent this line by 4 spaces please

> batchrenamedialogmodel.cpp:28
> +int BatchRenameDialogModel::rowCount(const QModelIndex ) const {
> +Q_UNUSED(parent);
> +return itemData->count();

Technically this should be `if (parent.isValid()) { return 0; }`
in case anyone plugs this model into a QTreeView one day, or plugs a proxy that 
supports trees.

> batchrenamedialogmodel.cpp:79
> +
> +BatchRenameDialogModel::BatchRenameDialogModel(QObject *parent, const 
> KFileItemList ) : QAbstractTableModel(parent) {
> +itemData = new QList;

the '{' that opens a method implementation should go on a separate line 
(repeats below)

> batchrenamedialogmodel.h:35
> +
> +BatchRenameDialogModelData(const KFileItem , QString newName, bool 
> valid)
> +{

This ctor isn't really needed, one could use aggregate initialization instead, 
no?
i.e. BatchRenameDialogModelData{item, newName, valid}

(Not 100% sure)

If you keep this ctor, then use member initialization syntax.

> batchrenamedialogmodel.h:47
> +Q_OBJECT
> +protected:
> +QList *itemData;

private (AFAICS nothing derives from this class), and at the end please.

> batchrenametypes.h:32
> +
> +class KIOWIDGETS_EXPORT BatchRenameTypes
> +{

This looks more like a namespace than an actual class, given that everything is 
static.

Alternatively (and even better), make capturedGroups and the two associated 
methods non-static, meaning that one has to instanciate the class in order to 
use it. This is only used in the dialog, right? So there's no need for this 
"global" variable, it can just be a member of BatchRenameTypes which can be a 
member of the dialog, AFAICS.

> batchrenamevar.h:39
> +
> +static const QString dateYear4Digit;
> +static const QString dateYear2Digit;

Make all these functions rather than variables. It will speed up starting time 
(right now all those QStrings have to be created upfront).

> filenameutils.h:29
> +
> +class KIOWIDGETS_EXPORT FileNameUtils
> +{

This looks more like a namespace than a class, given that it only contains 2 
static methods.

(We don't need a ctor / dtor to be generated)

REPOSITORY
  R241 KIO

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

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

D7487: Make KCMultiDialog scrollable

[Nathaniel Graham]

ngraham added a comment.
Restricted Application added a subscriber: kde-frameworks-devel.


  This has caused some pain with a number of KCMs, most notably plasma-nm and 
kscreen, but also others too (languages, shortcuts, powerdevil, activities). 
See all the duplicates of https://bugs.kde.org/show_bug.cgi?id=389585
  
  It's especially noticeable for plasma-nm because the plasmoid gives you a 
button that opens the full KCM in `kcmshell5`, where it gets the wrong size and 
the controls on the bottom are not visible without scrolling.
  
  KScreen actually does set a sizeHint, but it doesn't seem to get passed on to 
the scroll area.

REPOSITORY
  R295 KCMUtils

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

To: valeriymalov, #frameworks, davidedmundson
Cc: kde-frameworks-devel, ngraham, davidedmundson, wbauer, broulik, michaelh, 
bruns

D14606: KCrash: DrKonqi cancelled = able to start...

[David Faure]

dfaure added a comment.


  The crash recursion counter is increased when a SEGV happens inside the crash 
handler itself.

REPOSITORY
  R285 KCrash

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

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

D13869: [solid] Notify when interface to mounted fs is lost

[Stefan Brüns]

bruns added a comment.


  In D13869#303861 , @anthonyfieroni 
wrote:
  
  > Since you make question to new code I make for old one, did you know why 
Device interfaces is ever used, it's look wrong and why interfaces should be 
empty, after all my tests, it's easy to see that they are never empty.
  
  
  Thats trivial - remove the device if *all* interfaces have been removed. And 
thats correct, otherwise you can no longer call any solid method on the device.
  
  And thinking about it, this very likely makes your code wrong. Have you tried 
mounting a filesystem again after unmounting it, **using via any mean 
implemented via solid**? That is, "solid-hardware mount ...", the device 
notifier, ...

REPOSITORY
  R245 Solid

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

To: anthonyfieroni, broulik, cfeck, dfaure, bruns
Cc: ngraham, bcooksley, bruns, kde-frameworks-devel, michaelh

D14606: KCrash: DrKonqi cancelled = able to start...

[René J . V . Bertin]

rjvbb abandoned this revision.
rjvbb added a comment.


  I'm abandoning this for now because my assumption was wrong even though my 
patch had the effect I intended. I'll reopen if/when I have a real fix, because 
there are still 2 issues here:
  
  - The inappropriate warning. This could be fixed by a version of my current 
patch or else by rewording the warning.
  - The somewhat unorthodox way of coredumping. `/proc/sys/kernel/core_pattern` 
is Linux-specific but the `coredumpsize` "limit" shell setting exits on other 
Unix variants too. IOW, an absent or empty core_pattern file doesn't guarantee 
that a core dump will NOT be created when the caught signal is re-raised. In 
addition, that core_pattern may be set to a crash-reporter facility (apport in 
my case) which is completely irrelevant for code that wasn't installed through 
an official distribution package (the case for all my Qt5/KF5 software).
  
  In short, I think that what's needed here is a configuration variable rather 
than a check of the core_pattern special file (but that file could be used to 
provide the initial value of the config setting).

REPOSITORY
  R285 KCrash

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

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

D14606: KCrash: DrKonqi cancelled = able to start...

[René J . V . Bertin]

rjvbb added a comment.


  Actually, it's even worse than that: I didn't double-check my assumptions 
about the use of DrKonqi's return/exit code. Looking at the code again there is 
actually no way that information is even obtained. The `startProcess` function 
simply starts DrKonqi and then waits for it to exit if so instructed; the only 
information obtained about the crashreporter is its PID.
  
  And indeed, DrKonqi exits with 0 for me too when I cancel/close it.
  
  In a way this is worse not just because my brain was cooked yesterday. It 
means we're getting different behaviour inside KCrash: the value of 
`crashRecursionCounter`. And I'm back to not understanding how that's possible. 
I don't see how `crashRecursionCounter` could ever be `>2` unless you attempt 
to restart the crashed application. But that also doesn't work for me: it 
starts a new application = new PID = `crashRecursionCounter` never increases.
  
  That counter gets set to 2 here:
  
if (crashRecursionCounter < 2) {
if (s_emergencySaveFunction) {
s_emergencySaveFunction(sig);
}
if ((s_flags & AutoRestart) && s_autoRestartCommand) {
QThread::sleep(1);
startProcess(s_autoRestartArgc, const_cast(s_autoRestartCommandLine), false);
}
crashRecursionCounter++;
}
  
  A bit strange that the counter would always be increased and not only when 
there has been an auto-restart attempt, but let's ignore that for now.
  
  I see no other places where the counter is increased, so apparently the code 
assumes that the function itself can be called multiple times. But how? I see 
only 2 possibilities:
  
  - When the application receives an additional signal that is connected to the 
`defaultCrashHandler()` function
  - When the application is restarted without changing its PID and without 
re-initialising all its static variables - is that even possible (after a fatal 
exception)?
  
  In other words, how do you get the `crashRecursionCounter >= 4` required for 
not printing the unable-to-start warning?
  
  Answer: you never get to that check:
  
if (!s_coreConfig->isProcess()) {
// Only exit if we don't forward to core dumps
_exit(253);
}
  
  On my system:
  
%> cat /proc/sys/kernel/core_pattern
|/usr/share/apport/apport %p %s %c %P
  
  Q.E.D. ...

REPOSITORY
  R285 KCrash

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

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

KDE CI: Frameworks attica kf5-qt5 WindowsMSVCQt5.10 - Build # 30 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20attica%20kf5-qt5%20WindowsMSVCQt5.10/30/
 Project:
Frameworks attica kf5-qt5 WindowsMSVCQt5.10
 Date of build:
Sun, 05 Aug 2018 17:59:30 +
 Build duration:
2 min 43 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)

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

[Albert Astals Cid]

aacid added a comment.


  As far as i can see none of these headers get installed so should they all be 
renamed to _p.h ?
  
  Also if the headers don't get installed how do you use the new classes?

REPOSITORY
  R241 KIO

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

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

KDE CI: Frameworks attica kf5-qt5 SUSEQt5.9 - Build # 30 - Fixed!

[CI System]

BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20attica%20kf5-qt5%20SUSEQt5.9/30/
 Project:
Frameworks attica kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 05 Aug 2018 17:59:30 +
 Build duration:
1 min 34 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report67%
(2/3)22%
(17/76)22%
(17/76)12%
(482/3924)8%
(161/1988)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(3/3)100%
(3/3)93%
(74/80)46%
(13/28)src20%
(14/71)20%
(14/71)11%
(408/3551)8%
(148/1858)tests.projecttest0%
(0/2)0%
(0/2)0%
(0/293)0%
(0/102)

D14008: Use QTEST_GUILESS_MAIN

[Heiko Becker]

This revision was automatically updated to reflect the committed changes.
Closed by commit R235:3a82a775b1c3: Use QTEST_GUILESS_MAIN (authored by 
heikobecker).

REPOSITORY
  R235 Attica

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14008?vs=37463=39143

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

AFFECTED FILES
  autotests/configtest.cpp
  autotests/providertest.cpp

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

D13869: [solid] Notify when interface to mounted fs is lost

[Stefan Brüns]

bruns added a comment.


  In D13869#303877 , @anthonyfieroni 
wrote:
  
  > About loop device it should present as well
  >  
http://storaged.org/doc/udisks2-api/2.6.3/gdbus-org.freedesktop.UDisks2.Loop.html
  >  After all you have ony hypothetical arguments, if you know case that this 
code not work let's discuss, till then this is right approach.
  
  
  Of course it is present for a loop device, but this is about **removed** aka 
lost interfaces.
  
  These are not hypothetical arguments, but practical arguments.

REPOSITORY
  R245 Solid

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

To: anthonyfieroni, broulik, cfeck, dfaure, bruns
Cc: ngraham, bcooksley, bruns, kde-frameworks-devel, michaelh

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

[Nathaniel Graham]

ngraham added a comment.


  Awesome! Does Dolphin need a patch to use this?

REPOSITORY
  R241 KIO

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

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

D13869: [solid] Notify when interface to mounted fs is lost

[Anthony Fieroni]

anthonyfieroni added a comment.


  About loop device it should present as well
  
http://storaged.org/doc/udisks2-api/2.6.3/gdbus-org.freedesktop.UDisks2.Loop.html

REPOSITORY
  R245 Solid

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

To: anthonyfieroni, broulik, cfeck, dfaure, bruns
Cc: ngraham, bcooksley, bruns, kde-frameworks-devel, michaelh

D14632: keyword rule: Spport for keywords inclusion from another language/file

[jonathan poelen]

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

REPOSITORY
  R216 Syntax Highlighting

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

To: jpoelen, #framework_syntax_highlighting, cullmann, dhaumann
Cc: kwrite-devel, vkrause, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

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

[Emirald Mateli]

emateli retitled this revision from "Migrate D10698 from Dolphin to KIO" to 
"Adds a new RenameDialog to KIO with more options for batch renaming".
emateli edited the summary of this revision.

REPOSITORY
  R241 KIO

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

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

D13869: [solid] Notify when interface to mounted fs is lost

[Anthony Fieroni]

anthonyfieroni added a comment.


  Since you make question to new code I make for old one, did you why Device 
interfaces is ever used, it's look wrong and they should be empty, after all my 
tests, it's easy to see that they are never empty.

REPOSITORY
  R245 Solid

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

To: anthonyfieroni, broulik, cfeck, dfaure, bruns
Cc: ngraham, bcooksley, bruns, kde-frameworks-devel, michaelh

D14632: keyword rule: Spport for keywords inclusion from another language/file

[jonathan poelen]

jpoelen created this revision.
jpoelen added reviewers: Framework: Syntax Highlighting, cullmann, dhaumann.
Restricted Application added projects: Kate, Frameworks.
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel.
jpoelen requested review of this revision.

REVISION SUMMARY
  The presence of "##" in the name of the keyword rule indicates that it is 
necessary to look for the list in another file (format: 
"listName##languageName")

TEST PLAN
  A quick test was done by removing the "properties" list from scss.xml and 
with "properties##CSS" as the keyword name.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  keyword_rule

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

AFFECTED FILES
  src/indexer/katehighlightingindexer.cpp
  src/lib/rule.cpp

To: jpoelen, #framework_syntax_highlighting, cullmann, dhaumann
Cc: kwrite-devel, vkrause, kde-frameworks-devel, michaelh, kevinapavew, 
ngraham, bruns, demsking, cullmann, sars, dhaumann

D14631: Migrate D10698 from Dolphin to KIO

[Emirald Mateli]

emateli added a reviewer: Frameworks.

REPOSITORY
  R241 KIO

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

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

D14631: Migrate D10698 from Dolphin to KIO

[Emirald Mateli]

emateli created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
emateli requested review of this revision.

REPOSITORY
  R241 KIO

BRANCH
  batchrename2

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/batchrenametypestest.cpp
  autotests/batchrenametypestest.h
  src/widgets/CMakeLists.txt
  src/widgets/rename/batchrenamedialog.cpp
  src/widgets/rename/batchrenamedialog.h
  src/widgets/rename/batchrenamedialogmodel.cpp
  src/widgets/rename/batchrenamedialogmodel.h
  src/widgets/rename/batchrenametypes.cpp
  src/widgets/rename/batchrenametypes.h
  src/widgets/rename/batchrenamevar.cpp
  src/widgets/rename/batchrenamevar.h
  src/widgets/rename/filenameutils.cpp
  src/widgets/rename/filenameutils.h
  tests/CMakeLists.txt
  tests/batchrenamedialogtest_gui.cpp

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

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

[Shubham]

shubham added a comment.


  In D14610#303706 , @rkflx wrote:
  
  > In D14610#303537 , @rkflx wrote:
  >
  > > `KPropertiesDialog::setFileNameReadOnly`
  > > `m_bFromTemplate`
  >
  >
  > @shubham Any comments on that (see my questions to @dfaure above)?
  
  
  sorry, don't know about it

INLINE COMMENTS

> rkflx wrote in kpropertiesdialog.cpp:988
> Without your patch, we only tested for `!itemList.supportsMoving()`. Now you 
> also test for `(itemList.isDirectory() & !itemList.supportsWriting()`. Could 
> you explain in what cases we need that new test? As far as I can see this 
> blocks renaming the folder you removed the write permission from (while only 
> its children should be prevented from being renamed).
> 
> (`&` instead of `&&` also caught my eye.)

this test was intended for blocking directories without write access to rename 
, but as you say it should be done to it's child

I missed that one & by mistake

REPOSITORY
  R241 KIO

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

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

KDE CI: Frameworks kwayland kf5-qt5 SUSEQt5.9 - Build # 48 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20SUSEQt5.9/48/
 Project:
Frameworks kwayland kf5-qt5 SUSEQt5.9
 Date of build:
Sun, 05 Aug 2018 16:05:39 +
 Build duration:
14 min and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 44 test(s), Skipped: 0 test(s), Total: 45 test(s)Failed: TestSuite.kwayland-testWaylandSurface
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report63%
(5/8)92%
(233/254)92%
(233/254)84%
(23660/28247)51%
(9271/18217)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.client98%
(40/41)98%
(40/41)93%
(10820/11653)47%
(5740/12219)autotests.server100%
(5/5)100%
(5/5)99%
(353/356)49%
(169/344)src.client99%
(71/72)99%
(71/72)83%
(5624/6741)64%
(1486/2322)src.compat100%
(2/2)100%
(2/2)100%
(81/81)100%
(0/0)src.server100%
(115/115)100%
(115/115)86%
(6782/7922)64%
(1876/2919)src.tools0%
(0/2)0%
(0/2)0%
(0/693)0%
(0/272)src.tools.testserver0%
(0/3)0%
(0/3)0%
(0/69)0%
(0/10)tests0%
(0/14)0%
(0/14)0%
(0/732)0%
(0/131)

D14008: Use QTEST_GUILESS_MAIN

[David Faure]

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

REPOSITORY
  R235 Attica

BRANCH
  master

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

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

D14606: KCrash: DrKonqi cancelled = able to start...

[David Faure]

dfaure added a comment.


  In D14606#303654 , @rjvbb wrote:
  
  > I think there is no way at this level of interaction to determine if an 
application crashed after start and distinguish that from something like a 
"user-cancelled" return code, or is there?
  
  
  There are some heuristics in kio's KProcessRunner, but aren't we getting 
completely sidetracked?
  Step 1 figuring out if (and why) you get a non-0 exit code, step 2 fixing the 
bug, and only then step 3 thinking about hypothetical situations (drkonqi 
crashing) that we might want to handle better.

REPOSITORY
  R285 KCrash

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

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

KDE CI: Frameworks kwayland kf5-qt5 SUSEQt5.10 - Build # 75 - Unstable!

[CI System]

BUILD UNSTABLE
 Build URL
https://build.kde.org/job/Frameworks%20kwayland%20kf5-qt5%20SUSEQt5.10/75/
 Project:
Frameworks kwayland kf5-qt5 SUSEQt5.10
 Date of build:
Sun, 05 Aug 2018 16:05:39 +
 Build duration:
8 min 18 sec and counting
   JUnit Tests
  Name: (root) Failed: 1 test(s), Passed: 44 test(s), Skipped: 0 test(s), Total: 45 test(s)Failed: TestSuite.kwayland-testWaylandSurface
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report63%
(5/8)92%
(233/254)92%
(233/254)84%
(23662/28248)51%
(9271/18217)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests.client98%
(40/41)98%
(40/41)93%
(10820/11653)47%
(5741/12219)autotests.server100%
(5/5)100%
(5/5)99%
(353/356)49%
(169/344)src.client99%
(71/72)99%
(71/72)83%
(5626/6742)64%
(1485/2322)src.compat100%
(2/2)100%
(2/2)100%
(81/81)100%
(0/0)src.server100%
(115/115)100%
(115/115)86%
(6782/7922)64%
(1876/2919)src.tools0%
(0/2)0%
(0/2)0%
(0/693)0%
(0/272)src.tools.testserver0%
(0/3)0%
(0/3)0%
(0/69)0%
(0/10)tests0%
(0/14)0%
(0/14)0%
(0/732)0%
(0/131)

D13730: Fix memory management in WaylandOutputManagement

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R127:baba76289560: Fix memory management in 
WaylandOutputManagement (authored by davidedmundson).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13730?vs=36663=39133

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

AFFECTED FILES
  autotests/client/test_wayland_outputmanagement.cpp
  src/client/protocols/output-management.xml
  src/server/outputconfiguration_interface.cpp

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

D13729: Isolate every test within WaylandOutputManagement

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R127:d6a250cfb16b: Isolate every test within 
WaylandOutputManagement (authored by davidedmundson).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13729?vs=36662=39132

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

AFFECTED FILES
  autotests/client/test_wayland_outputmanagement.cpp

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

D13601: OutputManagement fractional scaling

[David Edmundson]

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 R127:c981d5406f80: OutputManagement fractional scaling 
(authored by davidedmundson).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D13601?vs=37097=39131#toc

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13601?vs=37097=39131

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

AFFECTED FILES
  autotests/client/test_wayland_outputdevice.cpp
  autotests/client/test_wayland_outputmanagement.cpp
  src/client/outputconfiguration.cpp
  src/client/outputconfiguration.h
  src/client/outputdevice.cpp
  src/client/outputdevice.h
  src/client/protocols/output-management.xml
  src/client/protocols/outputdevice.xml
  src/client/registry.cpp
  src/server/outputchangeset.cpp
  src/server/outputchangeset.h
  src/server/outputchangeset_p.h
  src/server/outputconfiguration_interface.cpp
  src/server/outputdevice_interface.cpp
  src/server/outputdevice_interface.h
  src/server/outputmanagement_interface.cpp

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

D12388: Output device color curves correction

[David Edmundson]

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 R127:3d06279076f1: Output device color curves correction 
(authored by romangg, committed by davidedmundson).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D12388?vs=37879=39134#toc

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12388?vs=37879=39134

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

AFFECTED FILES
  autotests/client/test_wayland_outputdevice.cpp
  autotests/client/test_wayland_outputmanagement.cpp
  src/client/outputconfiguration.cpp
  src/client/outputconfiguration.h
  src/client/outputdevice.cpp
  src/client/outputdevice.h
  src/client/protocols/output-management.xml
  src/client/protocols/outputdevice.xml
  src/server/outputchangeset.cpp
  src/server/outputchangeset.h
  src/server/outputchangeset_p.h
  src/server/outputconfiguration_interface.cpp
  src/server/outputdevice_interface.cpp
  src/server/outputdevice_interface.h

To: davidedmundson, #frameworks, graesslin, romangg
Cc: kde-frameworks-devel, graesslin, davidedmundson, zzag, cfeck, michaelh, 
ngraham, bruns

D13869: [solid] Notify when interface to mounted fs is lost

[Stefan Brüns]

bruns added a comment.


  In D13869#303563 , @ngraham wrote:
  
  > FWIW, we got a thumbs up from someone in 
https://bugs.kde.org/show_bug.cgi?id=394348:
  
  
  A thumbs up is nice, but this does not catch the thumbs down of all the 
persons who are exposed to the changed code later, having a different setup. We 
have seen this in solid several times lately, because every change was only 
checked to "fix" a non-working case, but omitted all the other cases.
  
  Every change should both be checked in a real setup, **and** have a sound 
reasoning behind it why it is correct.

REPOSITORY
  R245 Solid

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

To: anthonyfieroni, broulik, cfeck, dfaure, bruns
Cc: ngraham, bcooksley, bruns, kde-frameworks-devel, michaelh

D13869: [solid] Notify when interface to mounted fs is lost

[Stefan Brüns]

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


  The change needs a proper commit message. A reference to the bug tracker is 
not sufficient.
  
  The commit message should (at least) include:
  
  - Description of the setup
- The UDI of the relevant Solid device
- The UDIs of its parents
- Relevant Solid attributes of the devices, e.g. from solid-hardware details
  - Description of the actions done
  - Old and new behaviour
  
  Think of what happens if anyone changes the code later. The person must be 
able to replicate the setup, to check if any further changes break the code.

INLINE COMMENTS

> udisksmanager.cpp:222
> +if (interfaces.contains(UD2_DBUS_INTERFACE_FILESYSTEM)
> +||  interfaces.contains(UD2_DBUS_INTERFACE_LOOP)) {
>  emit deviceRemoved(udi);

According to your description (which is hard to follow, because you refrain 
from writing complete sentences), what triggers here is the removal of the 
o.f.U2.FileSystem interface. There is no reasoning why the Loop interface 
matters here.

REPOSITORY
  R245 Solid

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

To: anthonyfieroni, broulik, cfeck, dfaure, bruns
Cc: ngraham, bcooksley, bruns, kde-frameworks-devel, michaelh

D14627: KCookieJar: fix wrong timezone conversion.

[David Faure]

dfaure created this revision.
dfaure added a reviewer: stefanocrocco.
Restricted Application added a project: Frameworks.
Restricted Application edited subscribers, added: kde-frameworks-devel; 
removed: Frameworks.
dfaure requested review of this revision.

REVISION SUMMARY
  Those format strings include the word "GMT" but QDateTime was ignoring
  that and setting the timezone to localtime. Set it to UTC so that no
  conversion happens in the toUtc() call below.
  
  See debugging details in https://phabricator.kde.org/D14379

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/ioslaves/http/kcookiejar/kcookiejar.cpp

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

D14449: Modify device usage information

[Shubham]

shubham added a comment.


  In D14449#300327 , @ngraham wrote:
  
  > +1 for using the same label but putting the information on more than one 
line like @rkflx suggests.
  
  
  sorry for causing trouble : (

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D13869: [solid] Notify when interface to mounted fs is lost

[Anthony Fieroni]

anthonyfieroni added a comment.


  I'm using too.

REPOSITORY
  R245 Solid

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

To: anthonyfieroni, broulik, cfeck, dfaure
Cc: ngraham, bcooksley, bruns, kde-frameworks-devel, michaelh

D14449: Modify device usage information

[Henrik Fehlauer]

rkflx added a comment.


  > This comment has been deleted.
  
  You are doing this in every other Diff, which is a bit annoying. Please first 
check what you wrote, then click on Send. Otherwise people will just ignore 
notifications.
  
  > is there any other simpler way to achieve this?
  
  Maybe, maybe not. I added my idea on that topic above, but it's not my job to 
research that in depth.

INLINE COMMENTS

> shubham wrote in kpropertiesdialog.cpp:1184-1186
> If all function calls to setText are to take place inside 
> slotFreeSpaceResult, then we will need to make a new QLabel, so we can't 
> re-use it(which is the main point as @ngraham had earlier said)

Could you point out where @ngraham said something like that? I cannot find it.

> shubham wrote in kpropertiesdialog.cpp:1279
> so we need to remove setMaximumWidth() ?

Yes.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14449: Modify device usage information

[Shubham]

shubham added a comment.


  In D14449#303710 , @rkflx wrote:
  
  > - The vertical spacing between the first and the second line is too big, it 
should be the same as for Size:.
  
  
  is there any other simpler way to achieve this?

INLINE COMMENTS

> rkflx wrote in kpropertiesdialog.cpp:1184-1186
> That's a bit odd: Why would you have to query that information here again, if 
> in the original implementation it is already available?
> 
> Perhaps all calls to `setText` should take place only in one part of the 
> code, e.g. `slotFreeSpaceResult`.

If all function calls to setText are to take place inside slotFreeSpaceResult, 
then we will need to make a new QLabel, so we can't re-use it(which is the main 
point as @ngraham had earlier said)

> rkflx wrote in kpropertiesdialog.cpp:1279
> I don't think we can set a maximum width for the complete bar, because it can 
> conflict with languages with longer translations.
> 
> More importantly, for the Oxygen widget style the text in a `KCapacityBar` 
> might be drawn inline and the bar should span the complete width of the 
> dialog, so I don't think we should fiddle too much with the sizes of 
> `m_capacityBar` or its sub-components.
> 
> I guess we have to live with the longer bar for Breeze, which could already 
> become quite long without your patch if you resized the dialog. For other 
> languages it is also less of an issue.

so we need to remove setMaximumWidth() ?

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14449: Modify device usage information

[Henrik Fehlauer]

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


  Thanks, looking better than before now. There are still some improvements you 
could make:
  
  - There is a superfluous space before the comma in the second line.
  - I'd prefer the bar to be a bit wider by default. However, it turns out 
there is a problem with my original suggestion (see inline comment).
  - The vertical spacing between the first and the second line is too big, it 
should be the same as for Size:. However, there should still be enough spacing 
so it also looks good with the Oxygen style. As far as I can see this is an 
issue with how Breeze renders the `KCapacityBar`, in particular the bounding 
rect contains unnecessary margins ([⇧] + [Ctrl]-click on it in GammaRay and 
compare Breeze and Oxygen). Of course that's material for a patch in a 
different repo, but the "hole" in your current screenshot does not look good 
(the second line is closer to the bottom than to the first line, which is 
bad!), and it would be better to fix the problem there before landing the KIO 
patch.

INLINE COMMENTS

> kpropertiesdialog.cpp:1184-1186
> +QStorageInfo *storage = new QStorageInfo(QLatin1String("/"));
> +const quint64 size = storage->bytesTotal();
> +const quint64 free = storage->bytesAvailable();

That's a bit odd: Why would you have to query that information here again, if 
in the original implementation it is already available?

Perhaps all calls to `setText` should take place only in one part of the code, 
e.g. `slotFreeSpaceResult`.

> kpropertiesdialog.cpp:1188
> +l = new QLabel(d->m_frame);
> +grid->addWidget(l, curRow, 2);
> +l->setText(i18nc("Device usage information","%1 used , %2 free", 
> KIO::convertSize(size - free), KIO::convertSize(free)));

Do you need `curRow++` here, in case additional rows are added later on?

> kpropertiesdialog.cpp:1189
> +grid->addWidget(l, curRow, 2);
> +l->setText(i18nc("Device usage information","%1 used , %2 free", 
> KIO::convertSize(size - free), KIO::convertSize(free)));
> +

Translators will only see the string and the context, so "Device usage 
information" is not enough. It would be better to also describe what the 
parameters are for, see `slotFreeSpaceResult` for an example.

> kpropertiesdialog.cpp:1279
>  
> +d->m_capacityBar->setMaximumWidth(175);
>  d->m_capacityBar->setText(

I don't think we can set a maximum width for the complete bar, because it can 
conflict with languages with longer translations.

More importantly, for the Oxygen widget style the text in a `KCapacityBar` 
might be drawn inline and the bar should span the complete width of the dialog, 
so I don't think we should fiddle too much with the sizes of `m_capacityBar` or 
its sub-components.

I guess we have to live with the longer bar for Breeze, which could already 
become quite long without your patch if you resized the dialog. For other 
languages it is also less of an issue.

> kpropertiesdialog.cpp:1281
>  d->m_capacityBar->setText(
> -i18nc("Available space out of total partition size (percent used)", 
> "%1 free of %2 (%3% used)",
> -  KIO::convertSize(available),
> -  KIO::convertSize(size),
> -  percentUsed));
> +i18nc("Available space out of total partition size (percent used)", 
> "%1% used of %2",
> +  percentUsed,

Please also update the translation context when you change the string to 
translate.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

[Henrik Fehlauer]

rkflx added a comment.


  In D14610#303537 , @rkflx wrote:
  
  > `KPropertiesDialog::setFileNameReadOnly`
  > `m_bFromTemplate`
  
  
  @shubham Any comments on that (see my questions to @dfaure above)?

INLINE COMMENTS

> kpropertiesdialog.cpp:988
> +KFileItemListProperties itemList(KFileItemList() << item);
> +if (d->bMultiple || isTrash || hasRoot || !itemList.supportsMoving() || 
> (itemList.isDirectory() & !itemList.supportsWriting())) {
>  QLabel *lab = new QLabel(d->m_frame);

Without your patch, we only tested for `!itemList.supportsMoving()`. Now you 
also test for `(itemList.isDirectory() & !itemList.supportsWriting()`. Could 
you explain in what cases we need that new test? As far as I can see this 
blocks renaming the folder you removed the write permission from (while only 
its children should be prevented from being renamed).

(`&` instead of `&&` also caught my eye.)

> kpropertiesdialog.cpp:999-1019
> +d->m_lined = new KLineEdit(d->m_frame);
> +d->m_lined->setObjectName("KFilePropsPlugin::nameLineEdit");
> +d->m_lined->setText(filename);
> +d->nameArea = d->m_lined;
> +d->m_lined->setFocus();
> +grid->addWidget(d->nameArea, curRow++, 2);
> +

Please don't add additional indentation here.

> kpropertiesdialog.cpp:1021
>  
> -grid->addWidget(d->nameArea, curRow++, 2);
> -

You are moving that line to both the `if` and the `else` branch. Why not keep 
it here?

REPOSITORY
  R241 KIO

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

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

D14449: Modify device usage information

[Shubham]

shubham edited reviewers, added: rkflx; removed: elvisangelaccio.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, #frameworks, rkflx, elvisangelaccio
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14449: Modify device usage information

[Shubham]

shubham updated this revision to Diff 39120.
shubham added a comment.


  F6176187: 2.png 
  @rkflx is this okay now?

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14449?vs=38784=39120

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

AFFECTED FILES
  src/widgets/kpropertiesdialog.cpp

To: shubham, elvisangelaccio, ngraham, #frameworks
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14449: Modify device usage information

[Shubham]

shubham added a comment.


  > Not if you set a maximum width.
  
  @rkflx  eeh, skipped from my mind
  btw , then it is doable

REPOSITORY
  R241 KIO

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

To: shubham, elvisangelaccio, ngraham, #frameworks
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14008: Use QTEST_GUILESS_MAIN

[Heiko Becker]

heikobecker added a comment.


  Ping?

REPOSITORY
  R235 Attica

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

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

D14449: Modify device usage information

[Henrik Fehlauer]

rkflx added a comment.


  In D14449#303660 , @shubham wrote:
  
  > > Let me plug my suggestion again:
  > > 
  > >   Device capacity: ==- 24% used of 94.4 Gib
  > >22.5 GiB used, 71.9 GiB free
  > >
  >
  > if this design is used , then the capacity bar would look elongated and 
stretched, which doesn't looks good.
  
  
  Not if you set a maximum width.
  
  > Since used space is not of that much importance, so we can just omit it 
like this
  >  Device capacity: ===> 24% full of 90GiB (70GiB free)
  
  That's more or less what we currently have without your patch, but if we want 
to close the bug as WONTFIX I'd prefer the original order.

REPOSITORY
  R241 KIO

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

To: shubham, elvisangelaccio, ngraham, #frameworks
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14449: Modify device usage information

[Shubham]

shubham added a comment.


  > Let me plug my suggestion again:
  > 
  >   Device capacity: ==- 24% used of 94.4 Gib
  >22.5 GiB used, 71.9 GiB free
  >
  
  if this design is used , then the capacity bar would look elongated and 
stretched, which doesn't looks good. 
  Since used space is not of that much importance, so we can just omit it like 
this
  Device capacity: ===> 24% full of 90GiB (70GiB free)

REPOSITORY
  R241 KIO

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

To: shubham, elvisangelaccio, ngraham, #frameworks
Cc: rkflx, dhaumann, kde-frameworks-devel, michaelh, ngraham, bruns

D14606: KCrash: DrKonqi cancelled = able to start...

[René J . V . Bertin]

rjvbb added a comment.


  >   > if DrKonqi gives an exit code that means it was started
  >   
  >   It could have crashed though...
  
  Yeah, that was meant as a snarky remark - even if it crashed it was still 
started :)
  
  I think there is no way at this level of interaction to determine if an 
application crashed after start and distinguish that from something like a 
"user-cancelled" return code, or is there?

REPOSITORY
  R285 KCrash

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

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

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

[Shubham]

shubham updated this revision to Diff 39112.
shubham added a comment.


  Re-use Qlabel

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14610?vs=39082=39112

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

AFFECTED FILES
  src/widgets/kpropertiesdialog.cpp

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