D4923: [desktop:/ KIO] Strip superfluous slashes and fixup local root url

[David Faure]

dfaure added a comment.


  KFileItem::localPath() should not ever ever be a URL like desktop:/. That 
would be very wrong.
  It just returns what you set in UDS_LOCAL_PATH, so it's kio_desktop you 
should debug, not kio ;)
  Well except for KIO::ForwardingSlaveBase, called by kio_desktop -- note that 
ForwardingSlaveBase::prepareUDSEntry already sets UDS_LOCAL_PATH, but ok you 
call insert again which replaces that value.
  
  The code looks ok to me, are you 100% sure about your diagnostic that 
localPath() returns desktop:/... ?

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, hein, dfaure
Cc: anthonyfieroni, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D4923: [desktop:/ KIO] Strip superfluous slashes and fixup local root url

[Kai Uwe Broulik]

broulik added a comment.


  Taking for example the context menu code in FolderView:
  It does `KFileItem = m_dirModel->itemForIndex(index);`
  
  `item.localPath()`: `""`
  `item.isLocalFile()`: `false`
  `item.mostLocalUrl()`: `QUrl("desktop:/foo")`
  `item.targetUrl():` `QUrl("desktop:/foo")`
  `item.entry().stringValue(KIO::UDSEntry::UDS_LOCAL_PATH)`: `""`
  
  If you right click an item in desktop:/ the "Move to Trash" option will not 
be available, both in FolderView and Dolphin because it doesn't consider it a 
local file.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, hein, dfaure
Cc: anthonyfieroni, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D4956: [Folder View] Lower toolbox when an item is hovered

[Kai Uwe Broulik]

broulik added a comment.


  > You are still unable to hover the mouse and move it to click the selection 
and folder preview buttons. The toolbox raises or activates and blocks you.
  
  I can't reproduce. Notice how I was able to interact with the controls 
despite the ToolBox being there.
  F2753717: Screenshot_20170307_102345.png 

  
  Of course if you throw your mouse at the corner you will still raise the 
toolbox but that is somewhat to be expected. In any way it is an improvement 
over the status quo. It might indeed be beneficial if we place the tool box at 
the top right corner by default.

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, #vdg, hein, mart
Cc: rikmills, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D4956: [Folder View] Lower toolbox when an item is hovered

[Rik Mills]

rikmills added a comment.


  No, not throwing my mouse cursor at it at all.
  
  Starting at:
  
  F2753737: c691ba2eef.png 
  
  then very very very slowly edging towards the toolbox, it triggers about here:
  
  F2753741: 80fd4c7b7a.png 

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, #vdg, hein, mart
Cc: rikmills, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D4956: [Folder View] Lower toolbox when an item is hovered

[Kai Uwe Broulik]

broulik added a comment.


  Are you running plasma-desktop master? There have been many improvements in 
highlight handling as well as the icon grid layout itself.

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, #vdg, hein, mart
Cc: rikmills, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D4961: [Folder View] Allow querying currentItem

[Kai Uwe Broulik]

broulik abandoned this revision.

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, hein
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4962: [Folder View] Open context menu when pressing Menu key

[Kai Uwe Broulik]

broulik updated this revision to Diff 12256.
broulik edited the test plan for this revision.
broulik added a comment.


  - Use GridView currentItem
  - Cannot reproduce placement problem, I might have forgotten to actually 
compile after changing it to use `currentItem.frame` :) The context menu now 
properly touches the frame, even with one line of label.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4962?vs=12246&id=12256

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

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/plugins/folder/foldermodel.cpp
  containments/desktop/plugins/folder/foldermodel.h

To: broulik, #plasma, hein
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4964: [Folder View] Remove local copy of PlasmaQuick/Dialog

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:186f1386b6be: [Folder View] Remove local copy of 
PlasmaQuick/Dialog (authored by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4964?vs=12248&id=12258

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

AFFECTED FILES
  containments/desktop/plugins/folder/CMakeLists.txt
  containments/desktop/plugins/folder/plasmaquick/dialog.h
  containments/desktop/plugins/folder/subdialog.h

To: broulik, #plasma, hein, davidedmundson
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4959: [Folder View] Turn heading into a "hyperlink" and reduce hit area

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:138184afe09a: [Folder View] Turn heading into a 
"hyperlink" and reduce hit area (authored by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4959?vs=12240&id=12257

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

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderViewLayer.qml

To: broulik, #plasma, hein
Cc: mart, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

Jenkins-kde-ci: plasma-desktop master kf5-qt5 » Linux,gcc - Build # 655 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/plasma-desktop%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/655/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Mar 2017 09:51:55 +
Build duration: 6 min 48 sec

CHANGE SET
Revision 138184afe09ac868683a792249ad54a493b43bab by kde: ([Folder View] Turn 
heading into a "hyperlink" and reduce hit area)
  change: edit containments/desktop/package/contents/ui/FolderViewLayer.qml
Revision 186f1386b6bec7a1680a95975779a327801c675a by kde: ([Folder View] Remove 
local copy of PlasmaQuick/Dialog)
  change: edit containments/desktop/plugins/folder/CMakeLists.txt
  change: edit containments/desktop/plugins/folder/subdialog.h
  change: delete containments/desktop/plugins/folder/plasmaquick/dialog.h


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 7 
test(s)Failed: TestSuite.appstreamtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 7/7 (100%)FILES 36/39 (92%)CLASSES 36/39 (92%)LINE 2306/3420 
(67%)CONDITIONAL 1546/3761 (41%)

By packages
  
kcms.cursortheme.xcursor
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 99/192 (52%)CONDITIONAL 
22/98 (22%)
kcms.keyboard
FILES 20/23 (87%)CLASSES 20/23 (87%)LINE 743/1511 
(49%)CONDITIONAL 600/1672 (36%)
kcms.keyboard.preview
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 500/582 (86%)CONDITIONAL 
431/1110 (39%)
kcms.keyboard.tests
FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 229/231 (99%)CONDITIONAL 
236/358 (66%)
kcms.krdb
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 348/401 (87%)CONDITIONAL 
108/196 (55%)
kcms.lookandfeel
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 282/398 (71%)CONDITIONAL 
95/219 (43%)
kcms.lookandfeel.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 105/105 
(100%)CONDITIONAL 54/108 (50%)

T5412: Improve Plasma PA usability

[David Kahles]

davidk added a comment.


  >> When having many devices, the list quickly becomes longer than the 
available space and you need to scroll. Which is difficult, because you can't 
use the scrollwheel for that on most of the applet because of the sliders.
  > 
  > That's an issue in systray and not in plasma-pa, you can always use it 
outside of systray to have bigger popup.
  > 
  > I've tried disabling the wheel scrolling on sliders (because you can't 
disable it with QtQuickControls) with various hacks, but I failed. There was a 
patch to Qt to make it configurable, but it was rejected.
  
  The Qt patch has been merged and will be available in Qt 5.10: 
https://codereview.qt-project.org/#/c/187392/

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

To: davidk
Cc: davidk, vpilo, Zren, broulik, #vdg, plasma-devel, #plasma, subdiff, drosca, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mbohlender

D4917: [Breeze Plasma Theme] Improve action-overlay icons

[Kai Uwe Broulik]

broulik added a comment.


  > only a draft based on the old design with the new semantic.
  
  The graphic is missing some states (add-hover, add-pressed, open-normal, 
open-hovered, remove-hover, remove-pressed). Not a fan of having a blue remove 
icon. Also, I was told the arrow isn't a particular good icon for the idea of 
opening the folder in a popup, maybe there's a better icon for it.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: broulik, #plasma, #vdg, andreask, alex-l, mart
Cc: andreaska, hein, plasma-devel, #frameworks, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D4956: [Folder View] Lower toolbox when an item is hovered

[Rik Mills]

rikmills added a comment.


  In https://phabricator.kde.org/D4956#93275, @broulik wrote:
  
  > Are you running plasma-desktop master? There have been many improvements in 
highlight handling as well as the icon grid layout itself.
  
  
  Just updated and re-applied the patch to make sure.
  
  i.e. http://build.neon.kde.org/job/xenial_unstable_plasma_plasma-desktop/280/
  
  (which was tip of master until you pushed some changes 30 mins ago)
  
  with the patch applied on top.
  
  Tested with a new user once the patch was applied to FolderView.qml
  
  F2753814: 34dbd70c5a.png 

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, #vdg, hein, mart
Cc: rikmills, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D4939: Get rid of KDELibs4Support

[Alexander Volkov]

volkov added inline comments.

INLINE COMMENTS

> CMakeLists.txt:111
> +qt5_add_dbus_adaptor(powerdevil_SRCS 
> ${SOLID_DBUS_INTERFACES_DIR}/kf5_org.freedesktop.PowerManagement.xml 
> powerdevilfdoconnector.h PowerDevil::FdoConnector powermanagementfdoadaptor 
> PowerManagementFdoAdaptor)
> +qt5_add_dbus_adaptor(powerdevil_SRCS 
> ${SOLID_DBUS_INTERFACES_DIR}/kf5_org.freedesktop.PowerManagement.Inhibit.xml 
> powerdevilfdoconnector.h PowerDevil::FdoConnector 
> powermanagementinhibitadaptor PowerManagementInhibitAdaptor)
>  

I guess it can be just removed. These kf5_org.* files were dropped from solid 
three years ago.

> CMakeLists.txt:22
>  add_powerdevil_bundled_action(runscript KF5::KIOCore KF5::KIOWidgets)
> -add_powerdevil_bundled_action(suspendsession KF5::KIOCore KF5::KIOWidgets 
> KF5::Solid KF5::KDELibs4Support)
> +add_powerdevil_bundled_action(suspendsession KF5::KIOCore KF5::KIOWidgets 
> KF5::Solid)
>  if(HAVE_WIRELESS_SUPPORT)

Is it necessary to add KF5::Solid?

> powerdevilcore.cpp:102
> +{
> +if (!m_backend)
> +return BackendInterface::UnknownSuspendMethod;

And who will set backend for Core?

Now I think that it was a bad idea to request supported suspend methods from a 
backend, 
because GUI doesn't depend on it. You need to load the backend in GUI, it will 
do some initialization...

It seems to be more reasonable to request supported methods from powerdevil 
daemon by dbus calllings
org.freedesktop.PowerManagement.{CanSuspend, CanHibernate, CanHybridSuspend}

> powerdevilcore.cpp:107
> +
> +void Core::generateDefaultProfiles()
> +{

unrelated

> activitypage.cpp:76
>  // Message widget
> -m_messageWidget = new KMessageWidget(i18n("The activity service is 
> running with bare functionalities.\n"
> -  "Names and icons 
> of the activities might not be available."));
> +m_messageWidget = QSharedPointer(new 
> KMessageWidget(i18n("The activity service is running with bare 
> functionalities.\n"
> + 
> "Names and icons of the activities might not be available.")));

Shouldn't it be in a separate change?

REPOSITORY
  R122 Powerdevil

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

To: denisshienkov, volkov, afiestas
Cc: graesslin, davidedmundson, broulik, plasma-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D4956: [Folder View] Lower toolbox when an item is hovered

[Rik Mills]

rikmills added a comment.


  > In https://phabricator.kde.org/D4956#93270, @broulik wrote:
  >  In any way it is an improvement over the status quo.
  
  No doubt. Just annoying I can't replicate your result.
  
  > It might indeed be beneficial if we place the tool box at the top right 
corner by default.
  
  Well.. I personally am in favour of that change, unless there is 
particular/reason or objection.

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, #vdg, hein, mart
Cc: rikmills, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

drawing on the X11 root window with Qt5

[René J . V . Bertin]

Hello,

May I ask a question not directly related to Plasma but that hopefully concerns 
a trick used in some Plasma component?

Basically, is it still possible to get a QWidget corresponding to the X11 root 
window and draw on it as on any other QWidget, and if so, how does one do that?
A few pointers to relevant Plasma code should already help a lot.

The Qt4 approach I found that works is something like

class Foo : public QWidget
{
public:
Foo(WId id)
: QWidget()
{
if (id) {
create(id, false, true);
}
}
};

where Foo(QX11Info::appScreen()) would correspond to a subclassed QWidget that 
displays on the root window.

The suggested replacement method using 
QWidget::fromWindowContainer(QWindow::fromWinId(id)) doesn't allow subclassing 
but I also cannot get it to work. I get 2 standard windows when I pass in the 
root window WId, plus paintEngine errors that don't come from my own code. It 
probably doesn't help that it's been a long time since I coded for X11 and I 
never used the XCB APIs.

Thanks,
R.

Re: Review Request 129995: Fix KillRunner Memory leak

[David Edmundson]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129995/#review102746
---


Fix it, then Ship it!





runners/kill/killrunner.cpp (line 185)


just put it on the stack, that way you don't have to worry about any manual 
deletions.

Or use the static QProcess::execute which wraps it all anyway.


- David Edmundson


On March 7, 2017, 7:15 a.m., Leslie Zhai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129995/
> ---
> 
> (Updated March 7, 2017, 7:15 a.m.)
> 
> 
> Review request for Plasma, Aleix Pol Gonzalez and Kai Uwe Broulik.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Hi KDE developers,
> 
> Bug reported by the clang static analyzer.
> 
> Description: Potential leak of memory pointed to by 'process'
> File: plasma-workspace/runners/kill/killrunner.cpp
> Line: 186
> 
> ```
> 168   void KillRunner::run(const Plasma::RunnerContext &context, const 
> Plasma::QueryMatch &match)
> 169   {
> 170   Q_UNUSED(context)
> 171
> 172   QVariantList data = match.data().value();
> 173   quint64 pid = data[0].toUInt();
> 174   // QString user = data[1].toString();
> 175
> 176   int signal;
> 177   if (match.selectedAction() != NULL) {
> 
> Assuming the condition is false   
> ?
> 
> ?
> Taking false branch   
> ?
> 178   signal = match.selectedAction()->data().toInt();
> 179   } else {
> 180   signal = 9; //default: SIGKILL
> 181   }
> 182
> 183   QStringList args;
> 184   args << QStringLiteral("-%1").arg(signal) << 
> QStringLiteral("%1").arg(pid);
> 185   KProcess *process = new KProcess(this);
> 
> ?
> Memory is allocated   
> ?
> 186   int returnCode = process->execute(QStringLiteral("kill"), args);
> 
> ?
> Within the expansion of the macro 'QStringLiteral':
> a
> Potential leak of memory pointed to by 'process'
> 
> 187
> 188   if (returnCode == 0)
> ```
> 
> So I simply add ```delete process``` to free the allocated memory.
> 
> Regards,
> Leslie Zhai
> 
> 
> Diffs
> -
> 
>   runners/kill/killrunner.cpp 5c2e8529 
> 
> Diff: https://git.reviewboard.kde.org/r/129995/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Leslie Zhai
> 
>

D4383: Fix memory leak

[David Edmundson]

davidedmundson abandoned this revision.
davidedmundson added a comment.


  someone else wrote just put the same thing on RB. May as well let them ship 
theirs.

REPOSITORY
  R120 Plasma Workspace

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

To: davidedmundson, #plasma
Cc: apol, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas

Re: Review Request 129994: Fix AppMenuApplet Called C++ object pointer is null

[Kai Uwe Broulik]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129994/#review102749
---




applets/appmenu/lib/appmenuapplet.cpp (line 167)


Prefer !ctx instead of comparing with nullptr

Also, could you move the check up a bit so we don't do unneccessary work, 
ie.

if (m_currentIndex == idx) {
return;
}

if (!ctx || !ctx->window() ) {
return;
}

QMenu *actionMenu = createMenu(idx);


- Kai Uwe Broulik


On März 7, 2017, 6:48 vorm., Leslie Zhai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129994/
> ---
> 
> (Updated März 7, 2017, 6:48 vorm.)
> 
> 
> Review request for Plasma, Kai Uwe Broulik and David Edmundson.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Hi KDE developer,
> 
> Bug reported by the clang static analyzer.
> 
> Description: Called C++ object pointer is null
> File: plasma-workspace/applets/appmenu/lib/appmenuapplet.cpp
> Line: 167
> 
> ```
> 153   void AppMenuApplet::trigger(QQuickItem *ctx, int idx)
> 154   {
> 155   if (m_currentIndex == idx) {
> 
> Taking false branch   
> ?
> 156   return;
> 157   }
> 158
> 159   QMenu *actionMenu = createMenu(idx);
> 160   if (actionMenu) {
> 
> ?
> Assuming 'actionMenu' is non-null 
> ?
> 
> ?
> Taking true branch
> ?
> 161
> 162   if (ctx && ctx->window() && ctx->window()->mouseGrabberItem()) {
> 4
> ?
> Assuming 'ctx' is null
> ?
> 163   // FIXME event forge thing enters press and hold move mode 
> :/
> 164   ctx->window()->mouseGrabberItem()->ungrabMouse();
> 165   }
> 166
> 167   const auto &geo = 
> ctx->window()->screen()->availableVirtualGeometry();
> 5
> ?
> Called C++ object pointer is null
> ```
> 
> So I simply add some checker for ctx, ctx->window() and 
> ctx->window()->screen() pointers.
> 
> Regards,
> Leslie Zhai
> 
> 
> Diffs
> -
> 
>   applets/appmenu/lib/appmenuapplet.cpp 9aaf5cc6 
> 
> Diff: https://git.reviewboard.kde.org/r/129994/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Leslie Zhai
> 
>

D4965: Remove ifdef round qCDebugs

[David Edmundson]

davidedmundson added a comment.


  In https://phabricator.kde.org/D4965#93205, @graesslin wrote:
  
  > The ifdefs are from kde4 times. There it was also not needed, but 
nevertheless there. When working on the qt5 port I kept that under the 
assumption that the previous maintainer had a good reason for ifdefing it. 
Looking at the code I doubt it is side effect free. E.g. lines 694 following 
look dangerous to me. In fact I wouldn't guarantee that this won't result in a 
crash on wayland.
  
  
  I'll remove the ones using m_clip, you're right that they're not const 
methods, so could potentially throw off any debug anyway.
  
  > The code paths have not been used in production for years.
  
  That will also still effectively be the case here.
  qCDebug() isn't a function like qDebug() but a very clever macro. The rest of 
the line is only evaluated if that category logging is enabled.

REPOSITORY
  R120 Plasma Workspace

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

To: davidedmundson, #plasma, graesslin
Cc: graesslin, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D4956: [Folder View] Lower toolbox when an item is hovered

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:d1e81740d372: [Folder View] Lower toolbox when an item is 
hovered (authored by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4956?vs=12229&id=12261

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

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderView.qml

To: broulik, #plasma, #vdg, hein, mart
Cc: rikmills, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D4939: Get rid of KDELibs4Support

[Denis Shienkov]

denisshienkov added inline comments.

INLINE COMMENTS

> volkov wrote in CMakeLists.txt:111
> I guess it can be just removed. These kf5_org.* files were dropped from solid 
> three years ago.

Where I need to take an appropriate XML files then?

> volkov wrote in CMakeLists.txt:22
> Is it necessary to add KF5::Solid?

I will try later.

> volkov wrote in powerdevilcore.cpp:102
> And who will set backend for Core?
> 
> Now I think that it was a bad idea to request supported suspend methods from 
> a backend, 
> because GUI doesn't depend on it. You need to load the backend in GUI, it 
> will do some initialization...
> 
> It seems to be more reasonable to request supported methods from powerdevil 
> daemon by dbus calllings
> org.freedesktop.PowerManagement.{CanSuspend, CanHibernate, CanHybridSuspend}

> It seems to be more reasonable to request supported methods from powerdevil 
> daemon by dbus calllings

Ok, maybe then I can take a suggestion from @graesslin ?

"Just as an FYI: I reimplemented parts of the API in kscreenlocker. This code 
could be used here as well."

> volkov wrote in powerdevilcore.cpp:107
> unrelated

ok

> volkov wrote in activitypage.cpp:76
> Shouldn't it be in a separate change?

Then it does not compiled with Qt5 (without of KDELibs4Support).

REPOSITORY
  R122 Powerdevil

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

To: denisshienkov, volkov, afiestas
Cc: graesslin, davidedmundson, broulik, plasma-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D4925: actually delete old theme caches

[Marco Martin]

mart updated this revision to Diff 12262.
mart added a comment.


  use QFileInfo

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4925?vs=12133&id=12262

BRANCH
  arcpatch-D4925

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

AFFECTED FILES
  src/plasma/private/theme_p.cpp

To: mart, #plasma
Cc: apol, plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas

D4925: actually delete old theme caches

[Harald Sitter]

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


  lgtm

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  arcpatch-D4925

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

To: mart, #plasma, sitter
Cc: sitter, apol, plasma-devel, #frameworks, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas

D4955: [QDBusMenuBar] Connect to popupRequested signal

[Kai Uwe Broulik]

broulik updated this revision to Diff 12263.
broulik added a comment.


  - Rebase to 5.9

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4955?vs=12228&id=12263

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

AFFECTED FILES
  src/platformtheme/qdbusmenubar.cpp

To: broulik, #plasma, davidedmundson, fvogt
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4955: [QDBusMenuBar] Connect to popupRequested signal

[Kai Uwe Broulik]

broulik edited the summary of this revision.

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: broulik, #plasma, davidedmundson, fvogt
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4955: [QDBusMenuBar] Connect to popupRequested signal

[Fabian Vogt]

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


  Tested this with Qt 5.6.2 + backports, the signal gets triggered fine. So far 
it only seems to work with a few select apps (dolphin), some (e.g. konsole) do 
not assign shortcuts at all and others add shortcuts but assign them more than 
once...

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: broulik, #plasma, davidedmundson, fvogt
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4925: actually delete old theme caches

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> theme_p.cpp:271
> +for (const QFileInfo &file : cacheDir.entryInfoList()) {
> +if (!file.absoluteFilePath().endsWith(svgElementsFileName)) {
> +QFile::remove(file.absoluteFilePath());

do we not want

if (! file.fileName() ==

?

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  arcpatch-D4925

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

To: mart, #plasma, sitter
Cc: davidedmundson, sitter, apol, plasma-devel, #frameworks, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

Jenkins-kde-ci: plasma-desktop master kf5-qt5 » Linux,gcc - Build # 656 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/plasma-desktop%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/656/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Mar 2017 12:44:34 +
Build duration: 6 min 45 sec

CHANGE SET
Revision d1e81740d37289ac62dce56ceac860f6fd302718 by kde: ([Folder View] Lower 
toolbox when an item is hovered)
  change: edit containments/desktop/package/contents/ui/FolderView.qml


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 7 
test(s)Failed: TestSuite.appstreamtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 7/7 (100%)FILES 36/39 (92%)CLASSES 36/39 (92%)LINE 2306/3420 
(67%)CONDITIONAL 1546/3761 (41%)

By packages
  
kcms.cursortheme.xcursor
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 99/192 (52%)CONDITIONAL 
22/98 (22%)
kcms.keyboard
FILES 20/23 (87%)CLASSES 20/23 (87%)LINE 743/1511 
(49%)CONDITIONAL 600/1672 (36%)
kcms.keyboard.preview
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 500/582 (86%)CONDITIONAL 
431/1110 (39%)
kcms.keyboard.tests
FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 229/231 (99%)CONDITIONAL 
236/358 (66%)
kcms.krdb
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 348/401 (87%)CONDITIONAL 
108/196 (55%)
kcms.lookandfeel
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 282/398 (71%)CONDITIONAL 
95/219 (43%)
kcms.lookandfeel.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 105/105 
(100%)CONDITIONAL 54/108 (50%)

D4925: actually delete old theme caches

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
mart marked 2 inline comments as done.
Closed by commit R242:bbcb217b4812: actually delete old theme caches (authored 
by mart).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4925?vs=12262&id=12264

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

AFFECTED FILES
  src/plasma/private/theme_p.cpp

To: mart, #plasma, sitter
Cc: davidedmundson, sitter, apol, plasma-devel, #frameworks, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas

D4955: [QDBusMenuBar] Connect to popupRequested signal

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R135:aef74e97e2ed: [QDBusMenuBar] Connect to popupRequested 
signal (authored by broulik).

REPOSITORY
  R135 Integration for Qt applications in Plasma

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4955?vs=12263&id=12265

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

AFFECTED FILES
  src/platformtheme/qdbusmenubar.cpp

To: broulik, #plasma, davidedmundson, fvogt
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4939: Get rid of KDELibs4Support

[Alexander Volkov]

volkov added inline comments.

INLINE COMMENTS

> denisshienkov wrote in CMakeLists.txt:111
> Where I need to take an appropriate XML files then?

Copy them here.

> denisshienkov wrote in powerdevilcore.cpp:102
> > It seems to be more reasonable to request supported methods from powerdevil 
> > daemon by dbus calllings
> 
> Ok, maybe then I can take a suggestion from @graesslin ?
> 
> "Just as an FYI: I reimplemented parts of the API in kscreenlocker. This code 
> could be used here as well."

Yes, powermanagement.{h, cpp} from kscreenlocker is what you need.
The question is whether to create a library from it?

> denisshienkov wrote in activitypage.cpp:76
> Then it does not compiled with Qt5 (without of KDELibs4Support).

Why don't make QWeakPointer -> QPointer commit first and base this one on it?

REPOSITORY
  R122 Powerdevil

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

To: denisshienkov, volkov, afiestas
Cc: graesslin, davidedmundson, broulik, plasma-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D4967: Make difference between download and installed size

[Jan Grulich]

jgrulich created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  Currently in discover we show app size, but we don't make difference between 
download size and installed
  size which can be different (e.g. in flatpak backend). This patch adds 
download/installed size labels into
  the UI instead the original "size" label. The download size label is shown 
only when the application is not
  installed or there is an update available. The installed size label is shown 
all the time, except if the size
  is zero which doesn't make sense. If backend doesn't have download/installed 
size properties implemented then
  the original size label is shown instead so current backends remains 
unchtouched

REPOSITORY
  R134 Discover Software Store

BRANCH
  master

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

AFFECTED FILES
  discover/qml/ApplicationPage.qml
  libdiscover/backends/FlatpakBackend/FlatpakResource.cpp
  libdiscover/backends/FlatpakBackend/FlatpakResource.h
  libdiscover/resources/AbstractResource.cpp
  libdiscover/resources/AbstractResource.h
  libdiscover/resources/ResourcesProxyModel.cpp
  libdiscover/resources/ResourcesProxyModel.h

To: jgrulich
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4967: Make difference between download and installed size

[Jan Grulich]

jgrulich added a reviewer: apol.

REPOSITORY
  R134 Discover Software Store

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

To: jgrulich, apol
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

Jenkins-kde-ci: plasma-workspace Plasma-5.9 stable-kf5-qt5 » Linux,gcc - Build # 89 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/plasma-workspace%20Plasma-5.9%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/89/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Mar 2017 13:40:13 +
Build duration: 22 min

CHANGE SET
Revision 280ec4581b47611db654ebdd12fe324e3eee88b7 by scripty: (SVN_SILENT made 
messages (.desktop file) - always resolve ours)
  change: edit templates/ion-dataengine/ion-dataengine.kdevtemplate
  change: edit applets/appmenu/package/metadata.desktop


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 
11 test(s)Failed: TestSuite.appstreamtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 15/15 (100%)FILES 55/76 (72%)CLASSES 55/76 (72%)LINE 2333/5964 
(39%)CONDITIONAL 1634/5920 (28%)

By packages
  
drkonqi.parser
FILES 6/10 (60%)CLASSES 6/10 (60%)LINE 303/423 (72%)CONDITIONAL 
478/616 (78%)
drkonqi.tests.backtraceparsertest
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 74/74 (100%)CONDITIONAL 
33/50 (66%)
kioslave.desktop
FILES 2/3 (67%)CLASSES 2/3 (67%)LINE 107/141 (76%)CONDITIONAL 
42/72 (58%)
kioslave.desktop.tests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 84/84 (100%)CONDITIONAL 
37/72 (51%)
klipper
FILES 12/13 (92%)CLASSES 12/13 (92%)LINE 256/384 
(67%)CONDITIONAL 109/210 (52%)
klipper.autotests
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 630/693 (91%)CONDITIONAL 
377/820 (46%)
libtaskmanager
FILES 6/21 (29%)CLASSES 6/21 (29%)LINE 196/3298 (6%)CONDITIONAL 
122/3243 (4%)
libtaskmanager.autotests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 151/151 
(100%)CONDITIONAL 85/170 (50%)
runners.bookmarks
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 87/157 (55%)CONDITIONAL 
34/96 (35%)
runners.bookmarks.browsers
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 88/93 (95%)CONDITIONAL 
84/107 (79%)
runners.bookmarks.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 65/65 (100%)CONDITIONAL 
31/62 (50%)
runners.services
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 128/201 (64%)CONDITIONAL 
117/206 (57%)
runners.services.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 67/70 (96%)CONDITIONAL 
50/90 (56%)
shell
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 57/90 (63%)CONDITIONAL 
20/76 (26%)
shell.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 40/40 (100%)CONDITIONAL 
15/30 (50%)

D4956: [Folder View] Lower toolbox when an item is hovered

[Kai Uwe Broulik]

broulik added a subscriber: graesslin.
broulik added a comment.


  @graesslin The issue @rikmills described happens when there's a screen corner 
configured in KWin. Could this magic window with the glow interfere in hover 
handling in some way?

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, #vdg, hein, mart
Cc: graesslin, rikmills, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D4962: [Folder View] Open context menu when pressing Menu key

[Eike Hein]

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

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, hein
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4962: [Folder View] Open context menu when pressing Menu key

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:895f617f0887: [Folder View] Open context menu when 
pressing Menu key (authored by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D4962?vs=12256&id=12267

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

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderView.qml
  containments/desktop/plugins/folder/foldermodel.cpp
  containments/desktop/plugins/folder/foldermodel.h

To: broulik, #plasma, hein
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

Jenkins-kde-ci: plasma-desktop master kf5-qt5 » Linux,gcc - Build # 657 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/plasma-desktop%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/657/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Mar 2017 15:00:32 +
Build duration: 15 min

CHANGE SET
Revision 895f617f08876e6d02185514a00d9597c56bf455 by kde: ([Folder View] Open 
context menu when pressing Menu key)
  change: edit containments/desktop/package/contents/ui/FolderView.qml
  change: edit containments/desktop/plugins/folder/foldermodel.cpp
  change: edit containments/desktop/plugins/folder/foldermodel.h


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 7 
test(s)Failed: TestSuite.appstreamtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 7/7 (100%)FILES 36/39 (92%)CLASSES 36/39 (92%)LINE 2306/3420 
(67%)CONDITIONAL 1546/3761 (41%)

By packages
  
kcms.cursortheme.xcursor
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 99/192 (52%)CONDITIONAL 
22/98 (22%)
kcms.keyboard
FILES 20/23 (87%)CLASSES 20/23 (87%)LINE 743/1511 
(49%)CONDITIONAL 600/1672 (36%)
kcms.keyboard.preview
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 500/582 (86%)CONDITIONAL 
431/1110 (39%)
kcms.keyboard.tests
FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 229/231 (99%)CONDITIONAL 
236/358 (66%)
kcms.krdb
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 348/401 (87%)CONDITIONAL 
108/196 (55%)
kcms.lookandfeel
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 282/398 (71%)CONDITIONAL 
95/219 (43%)
kcms.lookandfeel.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 105/105 
(100%)CONDITIONAL 54/108 (50%)

D4967: Make difference between download and installed size

[Aleix Pol Gonzalez]

apol added a comment.


  All in all, +1.
  Would you be interested in implementing it for PackageKit? It's probably more 
interesting and complex there, as we need to compute the dependencies.

INLINE COMMENTS

> FlatpakResource.cpp:124
>  
> -int FlatpakResource::downloadSize() const
> +int FlatpakResource::downloadSize()
>  {

Why are you dropping the const?

REPOSITORY
  R134 Discover Software Store

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

To: jgrulich, apol
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4967: Make difference between download and installed size

[Jan Grulich]

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


  In https://phabricator.kde.org/D4967#93376, @apol wrote:
  
  > All in all, +1.
  >  Would you be interested in implementing it for PackageKit? It's probably 
more interesting and complex there, as we need to compute the dependencies.
  
  
  I can definitely take a look once I have some time.

INLINE COMMENTS

> apol wrote in FlatpakResource.cpp:124
> Why are you dropping the const?

Because I made downloadSize() and installedSize() non const in 
AbstractResource, same as size() in case you need to compute the size andf if 
you want to override it you have to follow the exact signature.

REPOSITORY
  R134 Discover Software Store

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

To: jgrulich, apol
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4963: Add scaling support into BlurEffect::doBlur

[Fredrik Höglund]

fredrik added a comment.


  The blur shader is not designed to blur and scale simultaneously.
  
  The sampled offsets are computed so that the shader samples between pixels, 
and the kernel weights are the sums of the gaussian function for the 
interpolated pixel values at each offset.
  
  This only works when the source and destinations have the same scale.

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

To: davidedmundson, #plasma
Cc: fredrik, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

Re: drawing on the X11 root window with Qt5

[Kai Uwe Broulik]

Note that KWIN doesn't render the X11 root window when compositing is enabled 
so this would be of little use.
‎
‎

D4939: Get rid of KDELibs4Support

[Denis Shienkov]

denisshienkov added inline comments.

INLINE COMMENTS

> volkov wrote in powerdevilcore.cpp:102
> Yes, powermanagement.{h, cpp} from kscreenlocker is what you need.
> The question is whether to create a library from it?

> The question is whether to create a library from it?

I don't know. It is not in my competence. I just can do copy/paste  a code from 
kscreenlocker.

> volkov wrote in activitypage.cpp:76
> Why don't make QWeakPointer -> QPointer commit first and base this one on it?

> Why don't make QWeakPointer -> QPointer



1. Then, I suggest do not use QPointer at all, as it is overhead for this case. 
I suggest to use the pointers directly..

> commit first and base this one on it?

I don't know how to do it with phabricator. It is my first issue in here. :)

REPOSITORY
  R122 Powerdevil

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

To: denisshienkov, volkov, afiestas
Cc: graesslin, davidedmundson, broulik, plasma-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D4969: open applet handles when the toolbox is open

[Marco Martin]

mart created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  this makes a mini "edit mode" when the toolbox is open, opening
  all the applet handles.
  It is coherent with the panel and partly solves the unintuitiveness
  of the press and hold for the handle

TEST PLAN
  open the toolbox, clicking on an empty area or on another window closes it
  clicking anywhere on an applet handle executes the action and doesn't close 
it.
  is possible to resize, move and close applets without closing any handle or
  the toolbox.
  
  Unfortunately there is a timer more in the toolbox that i tried really hard 
to kill
  but i didn't manage to have the correct behavior otherwise (ideas welcome)

REPOSITORY
  R119 Plasma Desktop

BRANCH
  phab/toolboxeditmode

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

AFFECTED FILES
  containments/desktop/package/contents/ui/AppletAppearance.qml
  containments/desktop/package/contents/ui/main.qml
  toolboxes/desktoptoolbox/contents/ui/ToolBoxButton.qml
  toolboxes/desktoptoolbox/contents/ui/ToolBoxItem.qml
  toolboxes/desktoptoolbox/contents/ui/ToolBoxRoot.qml

To: mart, #plasma
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4970: Get rid of QWeakPointer

[Denis Shienkov]

denisshienkov created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  As it is overhead to use QWeakPointer and QPointer in this simple code. Also, 
now we delete the ErrorOverlay objects directly instead of calling 
deleteLater() methods.

REPOSITORY
  R122 Powerdevil

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

AFFECTED FILES
  kcmodule/activities/activitypage.cpp
  kcmodule/activities/activitypage.h
  kcmodule/global/GeneralPage.cpp
  kcmodule/global/GeneralPage.h
  kcmodule/profiles/EditPage.cpp
  kcmodule/profiles/EditPage.h

To: denisshienkov
Cc: plasma-devel, volkov, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D4970: Get rid of QWeakPointer

[Denis Shienkov]

denisshienkov added a reviewer: volkov.

REPOSITORY
  R122 Powerdevil

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

To: denisshienkov, volkov
Cc: plasma-devel, volkov, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D4970: Get rid of QWeakPointer

[Denis Shienkov]

denisshienkov added a reviewer: afiestas.

REPOSITORY
  R122 Powerdevil

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

To: denisshienkov, volkov, afiestas
Cc: plasma-devel, volkov, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D4970: Get rid of QWeakPointer

[Denis Shienkov]

denisshienkov added a reviewer: ltoscano.

REPOSITORY
  R122 Powerdevil

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

To: denisshienkov, volkov, afiestas, ltoscano
Cc: plasma-devel, volkov, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D4939: Get rid of KDELibs4Support

[Denis Shienkov]

denisshienkov added inline comments.

INLINE COMMENTS

> denisshienkov wrote in activitypage.cpp:76
> > Why don't make QWeakPointer -> QPointer
> 
> 
> 
> 1. Then, I suggest do not use QPointer at all, as it is overhead for this 
> case. I suggest to use the pointers directly..
> 
> > commit first and base this one on it?
> 
> I don't know how to do it with phabricator. It is my first issue in here. :)

> I don't know how to do it with phabricator. It is my first issue in here. :)

Please see https://phabricator.kde.org/D4970

REPOSITORY
  R122 Powerdevil

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

To: denisshienkov, volkov, afiestas
Cc: graesslin, davidedmundson, broulik, plasma-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D4918: [Folder View] Increase delegate action size to smallMedium in icon mode

[Eike Hein]

hein added a comment.


  After trying it I still don't like smallMedium for icon mode :(. Just seems 
too huge (also rather dislike the icons, and the press color clashes with the 
selected frame state ...).

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, hein
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4969: open applet handles when the toolbox is open

[David Edmundson]

davidedmundson added a comment.


  Tried it out, expecting to find it difficult to use, acutally quite liked it. 
Surprisingly simple too.
  
  One problem:
  
  - have multiscreen
  - click toolbox on left
  - drag widget from left screen to right
  - handles disappear mid drag
  
  Maybe we can just set temporaryShowAppletHandle on a drag start too?
  
  I'd also like to see this setting going via Plasma::Contianment instead of 
AppletHandle sniffing random properties in the parent component scope. 
  That will allow us to throw this in a context menu too - and some applets 
should disable their own popups and tooltips when in this mode.
  
  Can we re-use isUserConfiguring?

REPOSITORY
  R119 Plasma Desktop

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

To: mart, #plasma
Cc: davidedmundson, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

Jenkins-kde-ci: plasma-desktop master kf5-qt5 » Linux,gcc - Build # 658 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/plasma-desktop%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/658/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Mar 2017 20:23:44 +
Build duration: 6 min 55 sec

CHANGE SET
Revision cb975ddc52c45c7a41cc937b2a5ae7b73c46eebd by scripty: (SVN_SILENT made 
messages (.desktop file) - always resolve ours)
  change: edit kcms/colors/colors.desktop
Revision d37b57dae09d1d9d0e2b26101514ad216139c063 by hein: (Do apply margin if 
in right-to-left mode.)
  change: edit containments/desktop/package/contents/ui/main.qml


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 7 
test(s)Failed: TestSuite.appstreamtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 7/7 (100%)FILES 36/39 (92%)CLASSES 36/39 (92%)LINE 2306/3420 
(67%)CONDITIONAL 1546/3761 (41%)

By packages
  
kcms.cursortheme.xcursor
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 99/192 (52%)CONDITIONAL 
22/98 (22%)
kcms.keyboard
FILES 20/23 (87%)CLASSES 20/23 (87%)LINE 743/1511 
(49%)CONDITIONAL 600/1672 (36%)
kcms.keyboard.preview
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 500/582 (86%)CONDITIONAL 
431/1110 (39%)
kcms.keyboard.tests
FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 229/231 (99%)CONDITIONAL 
236/358 (66%)
kcms.krdb
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 348/401 (87%)CONDITIONAL 
108/196 (55%)
kcms.lookandfeel
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 282/398 (71%)CONDITIONAL 
95/219 (43%)
kcms.lookandfeel.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 105/105 
(100%)CONDITIONAL 54/108 (50%)

Jenkins-kde-ci: plasma-tests master kf5-qt5 » Linux,gcc - Build # 816 - Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/plasma-tests%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/816/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Mar 2017 20:31:24 +
Build duration: 3 min 21 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 2 test(s), Skipped: 0 test(s), Total: 3 
test(s)Failed: TestSuite.org.kde.plasma.analogclock-test

COBERTURA RESULTS

Cobertura Coverage Report
  

By packages
  

D4929: DrKonqi : lldb support

[Kevin Funk]

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


  So, plasma-workspace is usually way out of my comfort zone, I'm commenting 
anyway since RJVB urged me to do it.
  
  See my notes.

INLINE COMMENTS

> backtracegenerator.cpp:62
> +// than waiting a potentially very long time for it to heed the 
> kill() request.
> +m_proc->deleteLater();
> +} else {

Please do this only if `lldb` is used then. No need  to change code which 
apparently worked fine for years. Also, did you try `QProcess::terminate` 
instead?

PS: `QProcess` gets unhappy when being deleted while still running (=> runtime 
warnings).

PPS: Please read my other advice about quitting LLDB below, too

> backtracegenerator.cpp:144
>  emit newLine(line);
> +line = line.simplified();
> +if (line.startsWith(QLatin1String("Process ")) && 
> line.endsWith(QLatin1String(" detached"))) {

This whole logic looks really cumbersome.

Is there really no way to exit LLDB cleanly after detach?

http://stackoverflow.com/questions/26267289/how-can-i-exit-lldb-after-running-commands-with-o
 suggests there is:

  lldb /bin/ls -o "run" -o "script import os; os._exit(1)"

I take it not everyone's got Python on they system, but this works for me as 
well:

  lldb -p $(pidof kate) -o detach -o quit

Just put that into the `-o quit` in the lldbrc?

> backtracegenerator.h:87
>  QString   m_parsedBacktrace;
> +bool  m_lldbDetached;
>  

Looks pretty unclean to have a backend-specific variable around here.

> AppleTerminal:1
> +#!/bin/sh
> +

Please explain why this file is needed(?)

> lldbrc:9
> +ExecInputFile=%tempfile
> +BatchCommands=set set term-width 200\nthread info\nbt all\ndetach

`set set`? Really?

> drkonqibackends.cpp:165
>  KConfigGroup config(KSharedConfig::openConfig(), "DrKonqi");
> -#ifndef Q_OS_WIN
> +#if defined(__MAC_OS_X_VERSION_MAX_ALLOWED) && 
> __MAC_OS_X_VERSION_MAX_ALLOWED > 1070
> +QString defaultDebuggerName = config.readEntry("Debugger", 
> QString("lldb"));

Why these special conditions? Needs comments.

Whether to use or not to use lldb on a particular OS X version should be 
runtime decision, too.

> backtraceparserlldb.cpp:26
> +public:
> +BacktraceLineLldb(const QString & line);
> +};

Style: Use `const QString &line`

More of these issues in other lines

> backtraceparserlldb.h:28
> +public:
> +explicit BacktraceParserLldb(QObject *parent = 0);
> +

`nullptr`

> backtraceparserlldb.h:31
> +protected Q_SLOTS:
> +virtual void newLine(const QString & lineStr);
> +

Use `override`, strip `virtual`

REPOSITORY
  R120 Plasma Workspace

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

To: rjvbb, #plasma_workspaces, kfunk
Cc: kfunk, mart, broulik, kde-mac, plasma-devel, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

Jenkins-kde-ci: plasma-desktop master kf5-qt5 » Linux,gcc - Build # 659 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/plasma-desktop%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/659/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Mar 2017 20:39:24 +
Build duration: 5 min 47 sec

CHANGE SET
Revision ab89da7d1d0547dc98f423e16d1dd28d3d9830c7 by wengxt: (kimpanel: add 
show/hide buttons functionality.)
  change: edit applets/kimpanel/package/contents/ui/StatusIcon.qml
  change: edit applets/kimpanel/package/contents/ui/main.qml
  change: add applets/kimpanel/package/contents/ui/ContextMenu.qml
  change: edit applets/kicker/package/contents/ui/ActionMenu.qml
  change: edit applets/kimpanel/package/contents/config/main.xml


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 7 
test(s)Failed: TestSuite.appstreamtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 7/7 (100%)FILES 36/39 (92%)CLASSES 36/39 (92%)LINE 2306/3420 
(67%)CONDITIONAL 1546/3761 (41%)

By packages
  
kcms.cursortheme.xcursor
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 99/192 (52%)CONDITIONAL 
22/98 (22%)
kcms.keyboard
FILES 20/23 (87%)CLASSES 20/23 (87%)LINE 743/1511 
(49%)CONDITIONAL 600/1672 (36%)
kcms.keyboard.preview
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 500/582 (86%)CONDITIONAL 
431/1110 (39%)
kcms.keyboard.tests
FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 229/231 (99%)CONDITIONAL 
236/358 (66%)
kcms.krdb
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 348/401 (87%)CONDITIONAL 
108/196 (55%)
kcms.lookandfeel
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 282/398 (71%)CONDITIONAL 
95/219 (43%)
kcms.lookandfeel.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 105/105 
(100%)CONDITIONAL 54/108 (50%)

Jenkins-kde-ci: plasma-tests master kf5-qt5 » Linux,gcc - Build # 817 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/plasma-tests%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/817/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Mar 2017 20:48:04 +
Build duration: 1 min 29 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  

By packages
  

Jenkins-kde-ci: plasma-tests master kf5-qt5 » Linux,gcc - Build # 817 - Fixed!

[no-reply]

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/plasma-tests%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/817/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Mar 2017 20:48:04 +
Build duration: 1 min 29 sec

CHANGE SET
No changes


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 
test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  

By packages
  

Re: drawing on the X11 root window with Qt5

[René J . V . Bertin]

Kai Uwe Broulik wrote:

> Note that KWIN doesn't render the X11 root window when compositing is enabled
> so this would be of little use. ‎

Who says this is for use when KWin is active too? :) Even with KWin 5.9.3 
running applications like xscreensaver somehow manage to make the root window 
(and anything drawn thereupon) visible.

R.

Jenkins-kde-ci: plasma-desktop Plasma-5.9 stable-kf5-qt5 » Linux,gcc - Build # 91 - Still Unstable!

[no-reply]

GENERAL INFO

BUILD UNSTABLE
Build URL: 
https://build.kde.org/job/plasma-desktop%20Plasma-5.9%20stable-kf5-qt5/PLATFORM=Linux,compiler=gcc/91/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Tue, 07 Mar 2017 20:22:54 +
Build duration: 28 min

CHANGE SET
Revision cb975ddc52c45c7a41cc937b2a5ae7b73c46eebd by scripty: (SVN_SILENT made 
messages (.desktop file) - always resolve ours)
  change: edit kcms/colors/colors.desktop
Revision d37b57dae09d1d9d0e2b26101514ad216139c063 by hein: (Do apply margin if 
in right-to-left mode.)
  change: edit containments/desktop/package/contents/ui/main.qml


JUNIT RESULTS

Name: (root) Failed: 1 test(s), Passed: 6 test(s), Skipped: 0 test(s), Total: 7 
test(s)Failed: TestSuite.appstreamtest

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 7/7 (100%)FILES 36/39 (92%)CLASSES 36/39 (92%)LINE 2306/3420 
(67%)CONDITIONAL 1566/3802 (41%)

By packages
  
kcms.cursortheme.xcursor
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 99/192 (52%)CONDITIONAL 
22/98 (22%)
kcms.keyboard
FILES 20/23 (87%)CLASSES 20/23 (87%)LINE 743/1511 
(49%)CONDITIONAL 619/1711 (36%)
kcms.keyboard.preview
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 500/582 (86%)CONDITIONAL 
432/1112 (39%)
kcms.keyboard.tests
FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 229/231 (99%)CONDITIONAL 
236/358 (66%)
kcms.krdb
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 348/401 (87%)CONDITIONAL 
108/196 (55%)
kcms.lookandfeel
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 282/398 (71%)CONDITIONAL 
95/219 (43%)
kcms.lookandfeel.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 105/105 
(100%)CONDITIONAL 54/108 (50%)

D3805: Per-activity favorites (Final?)

[Eike Hein]

hein added a comment.


  ‎IRC log of review session:
  
  [05:37] ‎<‎Sho_‎>‎ ivan|home: currently compiling fav patch
  ‎[05:38] ‎<‎Sho_‎>‎ ivan|home: i may have one request
  ‎[05:38] ‎<‎ivan|home‎>‎ shoot
  ‎[05:39] ‎<‎Sho_‎>‎ ivan|home: we might have to renege on renaming favmodel 
to simplefavmodel - we technically don't really promise any api stability for 
the kicker backend, but we do have quite a few widgets on the KDE Store now 
that will break when the class changes name
  ‎[05:40] ‎<‎Sho_‎>‎ hmm actually it looks like Simple Menu wouldn't break
  ‎[05:40] ‎<‎Sho_‎>‎ maybe I should check the others first
  ‎[05:40] ‎<‎ivan|home‎>‎ Sho_: that is fine, the reason for the rename was 
for me to make sure I don't forget to replace the old model somewhere it should 
be replaced - to make the compiler complain :)
  ‎[05:41] ‎<‎Sho_‎>‎ oh, qmlRegisterType(uri, 0, 1, 
"FavoritesModel");
  ‎[05:41] ‎<‎Sho_‎>‎ you didn't actually change the export name
  ‎[05:41] ‎<‎Sho_‎>‎ so it won't even break Kickoff
  ‎[05:41] ‎<‎Sho_‎>‎ I guess we can just swing it that way, too
  ‎[05:41] ‎<‎ivan|home‎>‎ yes, for qml - everything is the same
  ‎[05:42] ‎<‎Sho_‎>‎ alright
  ‎[05:42] ‎<‎ivan|home‎>‎ the KAStatsFavoritesModel might use a better name 
though
  ‎[05:42] ‎<‎Sho_‎>‎ restarting plasma
  ‎[05:42] ‎<‎Sho_‎>‎ "ActivityFavoritesModel" works for me
  ‎[05:43] ‎<‎Sho_‎>‎ Application Dashboard didn't migrate favorites for me
  ‎[05:43] ‎<‎Sho_‎>‎ but it's possible plasma hadn't flushed to config yet
  ‎[05:43] ‎<‎ivan|home‎>‎ That name might be a bit problematic since those are 
not favourite activities
  ‎[05:44] ‎<‎Sho_‎>‎ i think i just saw a bug
  ‎[05:44] ‎<‎ivan|home‎>‎ cool :)
  ‎[05:44] ‎<‎Sho_‎>‎ i had five favorites, and i added one to the current 
activity, and it appeared at index 4
  ‎[05:45] ‎<‎Sho_‎>‎ so not at the end, one item before the previously last one
  ‎[05:45] ‎<‎ivan|home‎>‎ one potential problem with the transitioning mech is 
that it will get only the items from the first menu that initializes
  ‎[05:46] ‎<‎Sho_‎>‎ interestingly after some activity switching it changed 
order
  ‎[05:46] ‎<‎ivan|home‎>‎ is your .config/kactivitymanagerd-statsrc empty?
  ‎[05:46] ‎<‎Sho_‎>‎ no
  ‎[05:47] ‎<‎Sho_‎>‎ plasma just crashed during favorites dnd
  ‎[05:47] ‎<‎ivan|home‎>‎ The ordering is a bit tricky - handling between 
global favs and per-activity favs - I'll check what are the things that can go 
wrong
  ‎[05:47] ‎<‎ivan|home‎>‎ bt?
  ‎[05:47] ‎<‎Sho_‎>‎ https://paste.kde.org/pnqer4fz7
  ‎[05:48] ‎<‎Sho_‎>‎ i was dragging and nto running though, hmm
  ‎[05:48] ‎<‎Sho_‎>‎ crashed again
  ‎[05:48] ‎<‎Sho_‎>‎ press, move, crash on release
  ‎[05:48] ‎<‎Sho_‎>‎ meanwhile in kicker no crash, but favorites also do not 
move
  ‎[05:49] ‎<‎ivan|home‎>‎ in AppEntry::run!?
  ‎[05:49] ‎<‎Sho_‎>‎ did you test favorites dnd with dashboard only or also 
with kicker?
  ‎[05:49] ‎<‎ivan|home‎>‎ KSycocaEntry pointer is null
  ‎[05:50] ‎<‎Sho_‎>‎ about the transitioning stuff
  ‎[05:50] ‎<‎Sho_‎>‎ is there some way we can change it to "combine all 
favorites from all menus and eliminate dupes"?
  ‎[05:50] ‎<‎Sho_‎>‎ i feel like users will be angry if they lose some faves
  ‎[05:50] ‎<‎ivan|home‎>‎ dnd with dashobard
  ‎[05:50] ‎<‎Sho_‎>‎ dnd in kicker doesn't seem to work at all
  ‎[05:51] ‎<‎ivan|home‎>‎ Sho_: I haven't used kicker in a while - will re-test
  ‎[05:51] ‎<‎Sho_‎>‎ also small nitpick (sorry if this is chaotic, best take 
notes) - should probably be "Show in Favorites", not capital I
  ‎[05:51] ‎<‎ivan|home‎>‎ The favs combining might be problematic, I'll see
  ‎[05:52] ‎<‎ivan|home‎>‎ We have Move To Desktop, Move To Activity in TM
  ‎[05:53] ‎<‎Sho_‎>‎ but also "Add to ..." in Kicker :D
  ‎[05:53] ‎<‎Sho_‎>‎ I'm also not sure what proper is :(
  ‎[05:53] ‎<‎Sho_‎>‎ but capitalizing articles and prepositions feels weird to 
me
  ‎[05:53] ‎<‎ivan|home‎>‎ We are as unified as ... to me as well - I did it 
because I saw it in other places :)
  ‎[05:54] ‎<‎ivan|home‎>‎ Sho_: why would dnd call trigger?
  ‎[05:55] ‎<‎Sho_‎>‎ it might be a bug in the ui code's mouse event handling
  ‎[05:55] ‎<‎Sho_‎>‎ it shouldn't call trigger 
  ‎[05:55] ‎<‎ivan|home‎>‎ for me, it does not
  ‎[05:55] ‎<‎Sho_‎>‎ but i also don't get any dnd happening btw
  ‎[05:56] ‎<‎Sho_‎>‎ oh wait
  ‎[05:56] ‎<‎Sho_‎>‎ i'm wondering if we're totally not on the same page here
  ‎[05:56] ‎<‎Sho_‎>‎ by "dnd" in both UIs I am referring to reordering 
favorites by drag
  ‎[05:56] ‎<‎Sho_‎>‎ so i move pointer to item, press, move while holding, 
icon doesn't move position, i release, crash in trigger
  ‎[05:57] ‎<‎ivan|home‎>‎ Kicker reordering works for me, and the dashboard 
... :)
  ‎[05:57] ‎<‎ivan|home‎>‎ Do you have the latest master of everything?
  ‎[05:58] ‎<‎Sho_‎>‎ yep
  ‎[05:58] ‎<‎Sho_‎>‎ actually
  ‎[05:58] ‎<‎Sho_‎>‎ wait
  ‎[05:58] ‎<‎Sho_‎>‎ i might have forgotten to update kactivities
  ‎[05:58] ‎<‎Sho_‎>‎ also i didn't

D4070: Changes to global menu applet

[Luigi Toscano]

ltoscano added inline comments.

INLINE COMMENTS

> main.qml:55
> +checked: checkable
> +iconSource: appletEnabled ? i18n("application-menu") : 
> i18n("emblem-warning")
>  onClicked: {

A bit late to the party, but if this is the freedesktop name of an icon, then 
it should not have i18n.

REPOSITORY
  R120 Plasma Workspace

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

To: chinmoyr, davidedmundson, broulik
Cc: ltoscano, broulik, plasma-devel, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

[Powerdevil] [Bug 377357] New: configurable timer setting to turn off the keyboard's backlight

[bugzilla_noreply]

https://bugs.kde.org/show_bug.cgi?id=377357

Bug ID: 377357
   Summary: configurable timer setting to turn off the keyboard's
backlight
   Product: Powerdevil
   Version: unspecified
  Platform: Mint (Ubuntu based)
OS: Linux
Status: UNCONFIRMED
  Severity: wishlist
  Priority: NOR
 Component: general
  Assignee: plasma-devel@kde.org
  Reporter: ar...@protonmail.com
  Target Milestone: ---

I'm running Mint 18.1 KDE on a 2012 MacBook Pro and would love a setting
allowing the keyboard to stay lit for, say 15s of inactivity (no keyboard or
mouse/trackpad input) and then turn off.

Thanks :D

-- 
You are receiving this mail because:
You are the assignee for the bug.

D4918: [Folder View] Increase delegate action size to smallMedium in icon mode

[Kai Uwe Broulik]

broulik abandoned this revision.

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, hein
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4972: Start drag with press and hold on touch events

[David Edmundson]

davidedmundson created this revision.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.

REVISION SUMMARY
  This patch consists of two changes:
  
  - starting drags on press and hold
  - not starting drags when the event is from a touch event
  
  This means a user can scroll on a widget explorer with a touch screen
  correctly without it immediately starting a drag, but can still drag an
  item using a press and hold pattern.
  
  BUG: 368698

TEST PLAN
  Scrolled in Widget Explorer
  Added widget with press and hold
  
  Surprisingly that's the only Plasma usage of DragArea.

REPOSITORY
  R296 KDeclarative

BRANCH
  master

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

AFFECTED FILES
  src/qmlcontrols/draganddrop/DeclarativeDragArea.cpp
  src/qmlcontrols/draganddrop/DeclarativeDragArea.h

To: davidedmundson, #plasma
Cc: plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

Re: Review Request 129994: Fix AppMenuApplet Called C++ object pointer is null

[Leslie Zhai]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129994/
---

(Updated 三月 8, 2017, 10:17 a.m.)


Review request for Plasma, Kai Uwe Broulik and David Edmundson.


Changes
---

Fix as Kai suggest!


Repository: plasma-workspace


Description
---

Hi KDE developer,

Bug reported by the clang static analyzer.

Description: Called C++ object pointer is null
File: plasma-workspace/applets/appmenu/lib/appmenuapplet.cpp
Line: 167

```
153 void AppMenuApplet::trigger(QQuickItem *ctx, int idx)
154 {
155 if (m_currentIndex == idx) {

Taking false branch 
?
156 return;
157 }
158  
159 QMenu *actionMenu = createMenu(idx);
160 if (actionMenu) {

?
Assuming 'actionMenu' is non-null   
?

?
Taking true branch  
?
161  
162 if (ctx && ctx->window() && ctx->window()->mouseGrabberItem()) {
4
?
Assuming 'ctx' is null  
?
163 // FIXME event forge thing enters press and hold move mode 
:/
164 ctx->window()->mouseGrabberItem()->ungrabMouse();
165 }
166  
167 const auto &geo = 
ctx->window()->screen()->availableVirtualGeometry();
5
?
Called C++ object pointer is null
```

So I simply add some checker for ctx, ctx->window() and ctx->window()->screen() 
pointers.

Regards,
Leslie Zhai


Diffs (updated)
-

  applets/appmenu/lib/appmenuapplet.cpp 9aaf5cc6 

Diff: https://git.reviewboard.kde.org/r/129994/diff/


Testing
---


Thanks,

Leslie Zhai

Re: Review Request 129994: Fix AppMenuApplet Called C++ object pointer is null

[Leslie Zhai]

> On 三月 7, 2017, 7:45 p.m., Kai Uwe Broulik wrote:
> > applets/appmenu/lib/appmenuapplet.cpp, line 167
> > 
> >
> > Prefer !ctx instead of comparing with nullptr
> > 
> > Also, could you move the check up a bit so we don't do unneccessary 
> > work, ie.
> > 
> > if (m_currentIndex == idx) {
> > return;
> > }
> > 
> > if (!ctx || !ctx->window() ) {
> > return;
> > }
> > 
> > QMenu *actionMenu = createMenu(idx);

Hi Kai,

Why use nullptr everywhere? it is my understanding 
http://www.leetcode.cn/2017/02/use-nullptr-everywhere.html please point out my 
fault, thanks!

Regards,
Leslie Zhai


- Leslie


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129994/#review102749
---


On 三月 8, 2017, 10:17 a.m., Leslie Zhai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129994/
> ---
> 
> (Updated 三月 8, 2017, 10:17 a.m.)
> 
> 
> Review request for Plasma, Kai Uwe Broulik and David Edmundson.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Hi KDE developer,
> 
> Bug reported by the clang static analyzer.
> 
> Description: Called C++ object pointer is null
> File: plasma-workspace/applets/appmenu/lib/appmenuapplet.cpp
> Line: 167
> 
> ```
> 153   void AppMenuApplet::trigger(QQuickItem *ctx, int idx)
> 154   {
> 155   if (m_currentIndex == idx) {
> 
> Taking false branch   
> ?
> 156   return;
> 157   }
> 158
> 159   QMenu *actionMenu = createMenu(idx);
> 160   if (actionMenu) {
> 
> ?
> Assuming 'actionMenu' is non-null 
> ?
> 
> ?
> Taking true branch
> ?
> 161
> 162   if (ctx && ctx->window() && ctx->window()->mouseGrabberItem()) {
> 4
> ?
> Assuming 'ctx' is null
> ?
> 163   // FIXME event forge thing enters press and hold move mode 
> :/
> 164   ctx->window()->mouseGrabberItem()->ungrabMouse();
> 165   }
> 166
> 167   const auto &geo = 
> ctx->window()->screen()->availableVirtualGeometry();
> 5
> ?
> Called C++ object pointer is null
> ```
> 
> So I simply add some checker for ctx, ctx->window() and 
> ctx->window()->screen() pointers.
> 
> Regards,
> Leslie Zhai
> 
> 
> Diffs
> -
> 
>   applets/appmenu/lib/appmenuapplet.cpp 9aaf5cc6 
> 
> Diff: https://git.reviewboard.kde.org/r/129994/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Leslie Zhai
> 
>

Re: Review Request 129995: Fix KillRunner Memory leak

[Leslie Zhai]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129995/
---

(Updated 三月 8, 2017, 10:42 a.m.)


Review request for Plasma, Aleix Pol Gonzalez and Kai Uwe Broulik.


Changes
---

Fix as David suggest!


Repository: plasma-workspace


Description
---

Hi KDE developers,

Bug reported by the clang static analyzer.

Description: Potential leak of memory pointed to by 'process'
File: plasma-workspace/runners/kill/killrunner.cpp
Line: 186

```
168 void KillRunner::run(const Plasma::RunnerContext &context, const 
Plasma::QueryMatch &match)
169 {
170 Q_UNUSED(context)
171  
172 QVariantList data = match.data().value();
173 quint64 pid = data[0].toUInt();
174 // QString user = data[1].toString();
175  
176 int signal;
177 if (match.selectedAction() != NULL) {

Assuming the condition is false 
?

?
Taking false branch 
?
178 signal = match.selectedAction()->data().toInt();
179 } else {
180 signal = 9; //default: SIGKILL
181 }
182  
183 QStringList args;
184 args << QStringLiteral("-%1").arg(signal) << 
QStringLiteral("%1").arg(pid);
185 KProcess *process = new KProcess(this);

?
Memory is allocated 
?
186 int returnCode = process->execute(QStringLiteral("kill"), args);

?
Within the expansion of the macro 'QStringLiteral':
a
Potential leak of memory pointed to by 'process'

187  
188 if (returnCode == 0)
```

So I simply add ```delete process``` to free the allocated memory.

Regards,
Leslie Zhai


Diffs (updated)
-

  runners/kill/killrunner.cpp 5c2e8529 

Diff: https://git.reviewboard.kde.org/r/129995/diff/


Testing
---


Thanks,

Leslie Zhai

Re: Review Request 129995: Fix KillRunner Memory leak

[Leslie Zhai]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129995/
---

(Updated March 8, 2017, 2:43 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma, Aleix Pol Gonzalez and Kai Uwe Broulik.


Changes
---

Submitted with commit 55237430bdab1bfbbfb442281ca5e34f5eb5e212 by Leslie Zhai 
to branch master.


Repository: plasma-workspace


Description
---

Hi KDE developers,

Bug reported by the clang static analyzer.

Description: Potential leak of memory pointed to by 'process'
File: plasma-workspace/runners/kill/killrunner.cpp
Line: 186

```
168 void KillRunner::run(const Plasma::RunnerContext &context, const 
Plasma::QueryMatch &match)
169 {
170 Q_UNUSED(context)
171  
172 QVariantList data = match.data().value();
173 quint64 pid = data[0].toUInt();
174 // QString user = data[1].toString();
175  
176 int signal;
177 if (match.selectedAction() != NULL) {

Assuming the condition is false 
?

?
Taking false branch 
?
178 signal = match.selectedAction()->data().toInt();
179 } else {
180 signal = 9; //default: SIGKILL
181 }
182  
183 QStringList args;
184 args << QStringLiteral("-%1").arg(signal) << 
QStringLiteral("%1").arg(pid);
185 KProcess *process = new KProcess(this);

?
Memory is allocated 
?
186 int returnCode = process->execute(QStringLiteral("kill"), args);

?
Within the expansion of the macro 'QStringLiteral':
a
Potential leak of memory pointed to by 'process'

187  
188 if (returnCode == 0)
```

So I simply add ```delete process``` to free the allocated memory.

Regards,
Leslie Zhai


Diffs
-

  runners/kill/killrunner.cpp 5c2e8529 

Diff: https://git.reviewboard.kde.org/r/129995/diff/


Testing
---


Thanks,

Leslie Zhai

D4718: support for auto-hidden windows to resize

[Martin Gräßlin]

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


  Could you please extend autotests/integration/screenedge_client_show_test.cpp 
with a test case for your change?

REPOSITORY
  R108 KWin

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

To: mart, #plasma, graesslin
Cc: luebking, plasma-devel, kwin, #kwin, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol

D4759: [kcheckpass] Drop command line option for username

[Martin Gräßlin]

graesslin added a comment.


  ping

REPOSITORY
  R133 KScreenLocker

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

To: graesslin, #plasma
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4773: [kcheckpass] Drop the caller command line option

[Martin Gräßlin]

graesslin added a comment.


  ping

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

To: graesslin, #plasma
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4768: Require PAM by default and provide an option to not require it

[Martin Gräßlin]

graesslin added a comment.


  ping

REPOSITORY
  R133 KScreenLocker

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

To: graesslin, #plasma, erichameleers
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4780: [kcheckpass] Drop the conv_legacy support

[Martin Gräßlin]

graesslin added a comment.


  ping

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

To: graesslin, #plasma
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol

D4806: [greeter] Send the auth result through the server instead return value

[Martin Gräßlin]

graesslin added a comment.


  ping

REPOSITORY
  R133 KScreenLocker

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

To: graesslin, #plasma
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol