D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Christoph Feck
cfeck added inline comments. INLINE COMMENTS > cfeck wrote in kpasswordlineedit.cpp:30 > Does this class need to be a QObject? If not, remove the QObject stuff. Ping? > kpasswordlineedit.h:53 > +Q_PROPERTY(bool clearButtonEnabled READ isClearButtonEnabled WRITE > setClearButtonEnabled) >

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Christoph Feck
cfeck added inline comments. INLINE COMMENTS > elvisangelaccio wrote in knewpasswordwidget.cpp:118 > Then please rename the slot to `_k_echoModeToggled`, otherwise is confusing. Ah, now I understand. Also, use the passed echoMode, instead of accessing lineEdit(). REVISION DETAIL

D7085: Fix issue where notifications will show as 1 pixel line

2017-08-03 Thread David Edmundson
davidedmundson added a comment. Apparently doesn't fix the bug that brought me here. To me the code still makes sense, but equally I don't want to touch this clearly cursed class any more than possible. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Christoph Feck
cfeck added inline comments. INLINE COMMENTS > cfeck wrote in knewpasswordwidget.cpp:75 > While you are at renaming slots: -> _k_passwordChanged Ah wait, you are using the same slot for the verify widget, so maybe scratch that comment. REVISION DETAIL https://phabricator.kde.org/D7011 To:

D6197: Add kauth helper to file ioslave

2017-08-03 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > chinmoyr wrote in filehelper.cpp:86 > In case of OPEN and OPENDIR, I can see why we need error checking, opendir() > can return null and there are some

KDE CI: Frameworks bluez-qt kf5-qt5 XenialQt5.7 - Build # 16 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20bluez-qt%20kf5-qt5%20XenialQt5.7/16/ Project: Frameworks bluez-qt kf5-qt5 XenialQt5.7 Date of build: Thu, 03 Aug 2017 07:11:40 + Build duration: 2 min 58 sec and counting JUnit Tests

D7071: Fix issue where notifications will show as 1 pixel line if primary screen wasn't the leftmost one

2017-08-03 Thread Matan Keren
matank updated this revision to Diff 17609. matank added a comment. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. - Fix issue where notifications will show as 1 pixel line Sets the dialog to his correct size to prevent it showing

D7071: Fix issue where notifications will show as 1 pixel line if primary screen wasn't the leftmost one

2017-08-03 Thread David Edmundson
davidedmundson added a comment. Can you explain the logic behind adding the code you've added. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D7071 To: matank, #plasma, davidedmundson Cc: plasma-devel, davidedmundson, ltoscano, #frameworks,

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Elvis Angelaccio
elvisangelaccio added inline comments. INLINE COMMENTS > mlaurent wrote in knewpasswordwidget.cpp:118 > Nope it's ok as code is called after changing echomode > I created an autotest for testing it Then please rename the slot to `_k_echoModeToggled`, otherwise is confusing. REVISION DETAIL

D7071: Fix issue where notifications will show as 1 pixel line if primary screen wasn't the leftmost one

2017-08-03 Thread Matan Keren
matank added a comment. Calling syncToMainItemSize() in componentComplete() seems to solve the issue , so I checked what is being done in syncToMainItemSize() that affects the issue. I found that resizing the dialog is what seems to resolve the issue so I copied the relevant code from

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Kevin Funk
kfunk added inline comments. INLINE COMMENTS > cfeck wrote in kpasswordlineedit.cpp:30 > That's fishy. I just checked a single class (KColumnResizer), and it has just > "class KColumnResizerPrivate" (no namespace, no QObject), and still has a > Q_PRIVATE_SLOT in its KColumnResizer class. +1.

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Laurent Montel
mlaurent added inline comments. INLINE COMMENTS > cfeck wrote in kpasswordlineedit.cpp:30 > That's fishy. I just checked a single class (KColumnResizer), and it has just > "class KColumnResizerPrivate" (no namespace, no QObject), and still has a > Q_PRIVATE_SLOT in its KColumnResizer class.

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Laurent Montel
mlaurent updated this revision to Diff 17615. mlaurent added a comment. Fix all comment reported CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7011?vs=17606=17615 REVISION DETAIL https://phabricator.kde.org/D7011 AFFECTED FILES autotests/CMakeLists.txt

D7069: kpackagetool can now write appstream data to a file

2017-08-03 Thread Harald Sitter
This revision was automatically updated to reflect the committed changes. sitter marked an inline comment as done. Closed by commit R290:ef15ef0889d2: kpackagetool can now write appstream data to a file (authored by sitter). CHANGED PRIOR TO COMMIT

KDE CI: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 - Build # 26 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/26/ Project: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 Date of build: Thu, 03 Aug 2017 11:20:24 + Build duration: 48 sec and counting JUnit Tests

KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 28 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/28/ Project: Frameworks kirigami kf5-qt5 XenialQt5.7 Date of build: Thu, 03 Aug 2017 11:20:24 + Build duration: 1 min 9 sec and counting JUnit Tests

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Christoph Feck
cfeck added inline comments. INLINE COMMENTS > cfeck wrote in kpasswordlineedit.cpp:30 > Ping? You made it a namespaced class again. Why? REVISION DETAIL https://phabricator.kde.org/D7011 To: mlaurent, cfeck, dfaure, elvisangelaccio Cc: kossebau, elvisangelaccio, #frameworks

D7069: kpackagetool can now write appstream data to a file

2017-08-03 Thread Harald Sitter
sitter marked an inline comment as done. sitter added inline comments. INLINE COMMENTS > apol wrote in kpackagetool.cpp:473 > Why the `? true : false`? lol, brain fart that. REPOSITORY R290 KPackage BRANCH master REVISION DETAIL https://phabricator.kde.org/D7069 To: sitter, apol,

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Laurent Montel
mlaurent marked 4 inline comments as done. mlaurent added inline comments. INLINE COMMENTS > cfeck wrote in knewpasswordwidget.cpp:118 > Ah, now I understand. Also, use the passed echoMode, instead of accessing > lineEdit(). we can't as we use it in : connect(toggleEchoModeAction, ::triggered,

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Christoph Feck
cfeck added inline comments. INLINE COMMENTS > mlaurent wrote in kpasswordlineedit.cpp:30 > Otherwise it failed to using Q_PRIVATE_SLOT... That's fishy. I just checked a single class (KColumnResizer), and it has just "class KColumnResizerPrivate" (no namespace, no QObject), and still has a

KDE CI: Frameworks kpackage kf5-qt5 XenialQt5.7 - Build # 17 - Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kpackage%20kf5-qt5%20XenialQt5.7/17/ Project: Frameworks kpackage kf5-qt5 XenialQt5.7 Date of build: Thu, 03 Aug 2017 10:15:14 + Build duration: 2 min 20 sec and counting JUnit Tests

KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 31 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/31/ Project: Frameworks kirigami kf5-qt5 XenialQt5.7 Date of build: Thu, 03 Aug 2017 14:42:46 + Build duration: 52 sec and counting JUnit Tests

KDE CI: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 - Build # 29 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/29/ Project: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 Date of build: Thu, 03 Aug 2017 14:42:46 + Build duration: 26 sec and counting JUnit Tests

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

2017-08-03 Thread Sven Brauch
brauch updated this revision to Diff 17638. brauch added a comment. Right. Better call it here. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7010?vs=17411=17638 REVISION DETAIL https://phabricator.kde.org/D7010 AFFECTED FILES src/ksqueezedtextlabel.cpp To: brauch, cfeck Cc:

KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 32 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/32/ Project: Frameworks kirigami kf5-qt5 XenialQt5.7 Date of build: Thu, 03 Aug 2017 14:49:51 + Build duration: 58 sec and counting JUnit Tests

KDE CI: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 - Build # 31 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/31/ Project: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 Date of build: Thu, 03 Aug 2017 15:30:46 + Build duration: 1 min 8 sec and counting JUnit Tests

KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 33 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/33/ Project: Frameworks kirigami kf5-qt5 XenialQt5.7 Date of build: Thu, 03 Aug 2017 15:30:46 + Build duration: 1 min 0 sec and counting JUnit Tests

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Christoph Feck
cfeck accepted this revision. cfeck added a comment. Together with the Designer stuff, please. REVISION DETAIL https://phabricator.kde.org/D7011 To: mlaurent, cfeck, dfaure, elvisangelaccio Cc: kfunk, kossebau, elvisangelaccio, #frameworks

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Christoph Feck
cfeck removed a reviewer: elvisangelaccio. This revision is now accepted and ready to land. REVISION DETAIL https://phabricator.kde.org/D7011 To: mlaurent, cfeck, dfaure Cc: kfunk, kossebau, elvisangelaccio, #frameworks

KDE CI: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 - Build # 27 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/27/ Project: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 Date of build: Thu, 03 Aug 2017 14:15:24 + Build duration: 48 sec and counting JUnit Tests

D7094: Include a module for finding qml imports as runtime dependencies

2017-08-03 Thread Aleix Pol Gonzalez
apol created this revision. Restricted Application added projects: Frameworks, Build System. REVISION SUMMARY Allows to check if a module is available on the system and sets it as a runtime dependency. This is useful for projects so that they can specify their qml dependencies easily and

KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 29 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/29/ Project: Frameworks kirigami kf5-qt5 XenialQt5.7 Date of build: Thu, 03 Aug 2017 14:15:24 + Build duration: 4 min 23 sec and counting JUnit Tests

KDE CI: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 - Build # 28 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20FreeBSDQt5.7/28/ Project: Frameworks kirigami kf5-qt5 FreeBSDQt5.7 Date of build: Thu, 03 Aug 2017 14:28:44 + Build duration: 26 sec and counting JUnit Tests

D7058: Initialize the "Pause" button state in the widget tracker

2017-08-03 Thread Aleix Pol Gonzalez
apol added a comment. I just tried now. It works.  REPOSITORY R288 KJobWidgets REVISION DETAIL https://phabricator.kde.org/D7058 To: apol, #frameworks Cc: aacid

KDE CI: Frameworks kirigami kf5-qt5 XenialQt5.7 - Build # 30 - Still Unstable!

2017-08-03 Thread no-reply
BUILD UNSTABLE Build URL https://build.kde.org/job/Frameworks%20kirigami%20kf5-qt5%20XenialQt5.7/30/ Project: Frameworks kirigami kf5-qt5 XenialQt5.7 Date of build: Thu, 03 Aug 2017 14:28:44 + Build duration: 4 min 11 sec and counting JUnit Tests

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

2017-08-03 Thread Christoph Feck
cfeck requested changes to this revision. cfeck added a comment. This revision now requires changes to proceed. The title says "when text changes", but you call it even if the text does not change. Note that squeezeTextToLabel() is called from many places, including resizeEvent(), so avoid

D7102: Add cross-compilation support for the highlighting indexer

2017-08-03 Thread Volker Krause
vkrause created this revision. Restricted Application added a project: Frameworks. REPOSITORY R216 Syntax Highlighting BRANCH hl-cross-compile REVISION DETAIL https://phabricator.kde.org/D7102 AFFECTED FILES src/indexer/CMakeLists.txt To: vkrause, #frameworks

D7102: Add cross-compilation support for the highlighting indexer

2017-08-03 Thread Kevin Funk
kfunk accepted this revision. kfunk added a comment. This revision is now accepted and ready to land. TIL you can use `IMPORTED` on `add_executable`... LGTM REPOSITORY R216 Syntax Highlighting BRANCH hl-cross-compile REVISION DETAIL https://phabricator.kde.org/D7102 To: vkrause,

D7117: Fix compilation on Yocto

2017-08-03 Thread Volker Krause
vkrause created this revision. Restricted Application added a project: Frameworks. REVISION SUMMARY By default, Qt is built without accessibility there, which is what would indirectly include the needed QColor with a "normal" Qt build here. REPOSITORY R242 Plasma Framework (Library)

D7103: Allow to build KSyntaxHighlighter without Qt5Gui

2017-08-03 Thread Kevin Funk
kfunk accepted this revision. kfunk added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > CMakeLists.txt:3 > +if(TARGET Qt5::Gui) > +add_subdirectory(lib) > +add_subdirectory(cli) Too bad it needs QtGui. Just b/c it uses `QColor` as far as I can

D7062: Create a designer plugin for kpasswordlineedit

2017-08-03 Thread Laurent Montel
mlaurent closed this revision. REVISION DETAIL https://phabricator.kde.org/D7062 To: mlaurent, dfaure, kossebau, cfeck Cc: cfeck, elvisangelaccio, #frameworks

D7117: Fix compilation on Yocto

2017-08-03 Thread Volker Krause
This revision was automatically updated to reflect the committed changes. Closed by commit R242:0a8d1f9cd474: Fix compilation on Yocto (authored by vkrause). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7117?vs=17661=17677 REVISION

D7117: Fix compilation on Yocto

2017-08-03 Thread Kai Uwe Broulik
broulik accepted this revision. This revision is now accepted and ready to land. REPOSITORY R242 Plasma Framework (Library) BRANCH master REVISION DETAIL https://phabricator.kde.org/D7117 To: vkrause, #frameworks, broulik

D6820: Add QValidator to KTimeCombobox

2017-08-03 Thread Christoph Feck
cfeck added a comment. Are you sure the patch is complete? I remember there was code which was supposed to be replaced by the new validator. INLINE COMMENTS > ktimecombobox.cpp:608 > #include "moc_ktimecombobox.cpp" > +#include "ktimecombobox.moc" This does not look right. REPOSITORY

D6820: Add QValidator to KTimeCombobox

2017-08-03 Thread Christoph Feck
cfeck requested changes to this revision. cfeck added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > ktimecombobox.cpp:33 > +public: > +KTimeValidator(QString timeFormat, QObject *parent = Q_NULLPTR); > +KTimeValidator(QString timeFormat, QTime min,

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Elvis Angelaccio
elvisangelaccio added a comment. +1, looks good to me now. REVISION DETAIL https://phabricator.kde.org/D7011 To: mlaurent, cfeck, dfaure, elvisangelaccio Cc: kfunk, kossebau, elvisangelaccio, #frameworks

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Christoph Feck
cfeck added inline comments. INLINE COMMENTS > kpasswordlineedit.cpp:3 > + Copyright (c) 2017 Montel Laurent > + > + This library is free software; you can redistribute it and/or modify Elvis, you wrote the "visibilityAction" stuff, right? If yes, needs to be mentioned

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Christoph Feck
cfeck added inline comments. INLINE COMMENTS > kfunk wrote in kpasswordlineedit.cpp:30 > +1. Don't use nested private classes. Nested classes/structs inherit the > symbols visibility. > > Also see: https://phabricator.kde.org/D6927 > > PS: In times of Qt5 & lambda-connects, you don't need

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Laurent Montel
mlaurent updated this revision to Diff 17621. mlaurent added a comment. +1 CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7011?vs=17620=17621 REVISION DETAIL https://phabricator.kde.org/D7011 AFFECTED FILES autotests/CMakeLists.txt autotests/knewpasswordwidgettest.cpp

D7011: Extract KPasswordLineEdit class

2017-08-03 Thread Christoph Feck
cfeck added a comment. Looks good to go from my side. Let's wait a bit if someone has more objections. Thanks Laurent, it was a needed change considering the code duplication that was present before. REVISION DETAIL https://phabricator.kde.org/D7011 To: mlaurent, cfeck, dfaure,