Re: Review Request 127209: Fix some issues found by Coverity

2016-02-28 Thread David Faure

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



You lost the changes to the other files, in the diff. (missing --amend?)

- David Faure


On Feb. 29, 2016, 12:13 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127209/
> ---
> 
> (Updated Feb. 29, 2016, 12:13 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> Mostly initializing values and using printf() correctly.
> 
> 
> Diffs
> -
> 
>   src/lib/kaboutdata.cpp a220977 
> 
> Diff: https://git.reviewboard.kde.org/r/127209/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127216: [KStatusNotifierItem] MinimizeRestore does not "run" over the desktop on X11

2016-02-28 Thread Martin Gräßlin

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



sorry, but I don't understand what this review request is about and what it 
tries to solve.


src/kstatusnotifieritem.cpp (line 938)


please use NET::Properties() instead of relying on implicit cast.


- Martin Gräßlin


On Feb. 29, 2016, 6:18 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127216/
> ---
> 
> (Updated Feb. 29, 2016, 6:18 a.m.)
> 
> 
> Review request for KDE Frameworks, Martin Gräßlin, Thomas Lübking, and Martin 
> Klapetek.
> 
> 
> Bugs: 356523
> https://bugs.kde.org/show_bug.cgi?id=356523
> 
> 
> Repository: knotifications
> 
> 
> Description
> ---
> 
> I add specific X11 code till we have proper move in KWindowSystem
> 
> 
> Diffs
> -
> 
>   src/kstatusnotifieritem.cpp cf3e7b5 
> 
> Diff: https://git.reviewboard.kde.org/r/127216/diff/
> 
> 
> Testing
> ---
> 
> Tested on pixel ration = 1
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127211: Properly use static QMaps

2016-02-28 Thread Kevin Funk

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


Ship it!




Ship It!

- Kevin Funk


On Feb. 28, 2016, 9:52 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127211/
> ---
> 
> (Updated Feb. 28, 2016, 9:52 p.m.)
> 
> 
> Review request for Kate and KDE Frameworks.
> 
> 
> Repository: ktexteditor
> 
> 
> Description
> ---
> 
> Only initialize them once.
> Since they're not const, use ::value rather than ::operator[], because the 
> latter will provide a default-constructed object and add it to the map.
> 
> 
> Diffs
> -
> 
>   src/vimode/cmds.cpp 7804af4 
> 
> Diff: https://git.reviewboard.kde.org/r/127211/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127197: [kcm_colors] Port away from KDELibs4Support

2016-02-28 Thread Martin Gräßlin

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


Ship it!




Please put the change into master not into the Plasma/5.5. bracnh.

Concerning the leak: I noticed in the past that KTextEditor based applications 
run amok once you change the color scheme. No idea whether it's related, but it 
could be.

- Martin Gräßlin


On Feb. 27, 2016, 9:27 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127197/
> ---
> 
> (Updated Feb. 27, 2016, 9:27 a.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, David Edmundson, David Faure, Hugo 
> Parente Lima, and Thomas Lübking.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Start porting to find leaks on X in radeon driver.
> 
> 
> Diffs
> -
> 
>   kcms/colors/CMakeLists.txt f02e0eb 
>   kcms/colors/colorscm.h 8e74e8a 
>   kcms/colors/colorscm.cpp f3c4f05 
> 
> Diff: https://git.reviewboard.kde.org/r/127197/diff/
> 
> 
> Testing
> ---
> 
> 1. Open dolphin, kate, etc...
> 2. kcmschell5 colors
> 3. Apply color scheme
> 4. Extremly leak on X
> The bug is in this peace of code
> QDBusMessage message = 
> QDBusMessage::createSignal(QStringLiteral("/KGlobalSettings"), 
> QStringLiteral("org.kde.KGlobalSettings"), QStringLiteral("notifyChange") );
> QList args;
> args.append(0);//previous KGlobalSettings::PaletteChanged. This is now 
> private API in khintsettings
> args.append(0);//unused in palette changed but needed for the DBus 
> signature
> message.setArguments(args);
> QDBusConnection::sessionBus().send(message);
> 
> 
> File Attachments
> 
> 
> Screenshot_20160227_102609.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/02/27/a23bfdad-7465-422e-9cef-25fe7991e795__Screenshot_20160227_102609.png
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 127216: [KStatusNotifierItem] MinimizeRestore does not "run" over the desktop on X11

2016-02-28 Thread Anthony Fieroni

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

Review request for KDE Frameworks, Martin Gräßlin, Thomas Lübking, and Martin 
Klapetek.


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


Repository: knotifications


Description
---

I add specific X11 code till we have proper move in KWindowSystem


Diffs
-

  src/kstatusnotifieritem.cpp cf3e7b5 

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


Testing
---

Tested on pixel ration = 1


Thanks,

Anthony Fieroni

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127215: simplify code, reduce pointer dereferences

2016-02-28 Thread Aleix Pol Gonzalez

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




src/sycoca/kbuildsycoca.cpp (line 236)


foreach like you did above?



src/sycoca/ksycocadict.cpp (line 316)


Maybe it's better to leave the * there, as it makes it more clear that it's 
an output argument.


- Aleix Pol Gonzalez


On Feb. 29, 2016, 1:43 a.m., Nick Shaforostoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127215/
> ---
> 
> (Updated Feb. 29, 2016, 1:43 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> Changes are mostly related to containers/iterators, but there are also few 
> QString related optimizations
> 
> note that i have made a switch from QLinkedList to just QList because it 
> performs better for T=
> also i have changed QStringList to QSet in one place to make the 
> code run faster
> 
> 
> Diffs
> -
> 
>   src/sycoca/kbuildmimetypefactory.cpp 340f76a 
>   src/sycoca/kbuildservicefactory.cpp a74f2e8 
>   src/sycoca/kbuildsycoca.cpp e99e906 
>   src/sycoca/kmimeassociations.cpp 9423b27 
>   src/sycoca/ksycocadict.cpp f33d01a 
>   src/sycoca/ksycocafactory.cpp e410581 
> 
> Diff: https://git.reviewboard.kde.org/r/127215/diff/
> 
> 
> Testing
> ---
> 
> all tests pass, except kmimeassociationstest (fakeApplicationService is 
> NULL), but it fails also without my changes
> 
> 
> Thanks,
> 
> Nick Shaforostoff
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 127215: simplify code, reduce pointer dereferences

2016-02-28 Thread Nick Shaforostoff

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

Review request for KDE Frameworks.


Repository: kservice


Description
---

Changes are mostly related to containers/iterators, but there are also few 
QString related optimizations

note that i have made a switch from QLinkedList to just QList because it 
performs better for T=
also i have changed QStringList to QSet in one place to make the code 
run faster


Diffs
-

  src/sycoca/kbuildmimetypefactory.cpp 340f76a 
  src/sycoca/kbuildservicefactory.cpp a74f2e8 
  src/sycoca/kbuildsycoca.cpp e99e906 
  src/sycoca/kmimeassociations.cpp 9423b27 
  src/sycoca/ksycocadict.cpp f33d01a 
  src/sycoca/ksycocafactory.cpp e410581 

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


Testing
---

all tests pass, except kmimeassociationstest (fakeApplicationService is NULL), 
but it fails also without my changes


Thanks,

Nick Shaforostoff

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127209: Fix some issues found by Coverity

2016-02-28 Thread Aleix Pol Gonzalez

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

(Updated Feb. 29, 2016, 1:13 a.m.)


Review request for KDE Frameworks.


Changes
---

Address issue as pointed out by Lamarque.


Repository: kcoreaddons


Description
---

Mostly initializing values and using printf() correctly.


Diffs (updated)
-

  src/lib/kaboutdata.cpp a220977 

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


Testing
---


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127214: Remove some warnings

2016-02-28 Thread Aleix Pol Gonzalez

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

(Updated Feb. 29, 2016, 1:06 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 06c7b9c050aa57d4d7ac8b3302ef7b09a6436b55 by Aleix Pol to 
branch master.


Repository: plasma-framework


Description
---

* Unused variables
* Unused code
* Unused arguments
* Unnecessarily complex code
* Initialization order
* Duplicated properties

It would be _very_ good if we could get rid of Plasma::Package usage within 
Plasma Framework. I'm quite confident most of these warnings would have been 
fixed already if it wasn't because we're flooded with deprecated API usage 
warnings.


Diffs
-

  autotests/pluginloadertest.cpp b75dd03 
  src/declarativeimports/calendar/daysmodel.cpp 1faf43b 
  src/declarativeimports/calendar/eventpluginsmanager.cpp 343d586 
  src/declarativeimports/core/quicktheme.h b8017d2 
  src/declarativeimports/platformcomponents/application.cpp bdcc9b4 
  src/plasma/private/applet_p.cpp 1c0515a 
  src/plasma/private/theme_p.cpp b18e1fa 
  src/plasma/service.cpp a512569 
  src/plasmapkg/plasmapkg.cpp 138b2fe 

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


Testing
---

Builds, tests pass.


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127214: Remove some warnings

2016-02-28 Thread David Edmundson

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


Ship it!




Ship It!

- David Edmundson


On Feb. 28, 2016, 11:36 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127214/
> ---
> 
> (Updated Feb. 28, 2016, 11:36 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> * Unused variables
> * Unused code
> * Unused arguments
> * Unnecessarily complex code
> * Initialization order
> * Duplicated properties
> 
> It would be _very_ good if we could get rid of Plasma::Package usage within 
> Plasma Framework. I'm quite confident most of these warnings would have been 
> fixed already if it wasn't because we're flooded with deprecated API usage 
> warnings.
> 
> 
> Diffs
> -
> 
>   autotests/pluginloadertest.cpp b75dd03 
>   src/declarativeimports/calendar/daysmodel.cpp 1faf43b 
>   src/declarativeimports/calendar/eventpluginsmanager.cpp 343d586 
>   src/declarativeimports/core/quicktheme.h b8017d2 
>   src/declarativeimports/platformcomponents/application.cpp bdcc9b4 
>   src/plasma/private/applet_p.cpp 1c0515a 
>   src/plasma/private/theme_p.cpp b18e1fa 
>   src/plasma/service.cpp a512569 
>   src/plasmapkg/plasmapkg.cpp 138b2fe 
> 
> Diff: https://git.reviewboard.kde.org/r/127214/diff/
> 
> 
> Testing
> ---
> 
> Builds, tests pass.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 127214: Remove some warnings

2016-02-28 Thread Aleix Pol Gonzalez

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

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

* Unused variables
* Unused code
* Unused arguments
* Unnecessarily complex code
* Initialization order
* Duplicated properties

It would be _very_ good if we could get rid of Plasma::Package usage within 
Plasma Framework. I'm quite confident most of these warnings would have been 
fixed already if it wasn't because we're flooded with deprecated API usage 
warnings.


Diffs
-

  autotests/pluginloadertest.cpp b75dd03 
  src/declarativeimports/calendar/daysmodel.cpp 1faf43b 
  src/declarativeimports/calendar/eventpluginsmanager.cpp 343d586 
  src/declarativeimports/core/quicktheme.h b8017d2 
  src/declarativeimports/platformcomponents/application.cpp bdcc9b4 
  src/plasma/private/applet_p.cpp 1c0515a 
  src/plasma/private/theme_p.cpp b18e1fa 
  src/plasma/service.cpp a512569 
  src/plasmapkg/plasmapkg.cpp 138b2fe 

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


Testing
---

Builds, tests pass.


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126672: Fix most of Clazy warnings in plasma-framework

2016-02-28 Thread Sergey Popov

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

(Updated Feb. 28, 2016, 11:22 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Plasma and Aleix Pol Gonzalez.


Changes
---

Submitted with commit 9f625326745369b289952b3d83a2f3478cca4748 by Aleix Pol to 
branch master.


Repository: plasma-framework


Description
---

Fix almost all the Clazy warnings in plasma-framework.

Related GCI-2015 task: 
[https://codein.withgoogle.com/tasks/6272625345036288/](https://codein.withgoogle.com/tasks/6272625345036288/)


Diffs
-

  autotests/coronatest.cpp 378a4b7 
  autotests/dialogqmltest.cpp 618e64d 
  autotests/fallbackpackagetest.cpp 91bc6e9 
  autotests/packagestructuretest.cpp 67cdb4f 
  autotests/pluginloadertest.cpp 868d5f8 
  autotests/sortfiltermodeltest.cpp 6ee0e35 
  autotests/storagetest.cpp 8a7dbd0 
  src/declarativeimports/calendar/calendar.cpp 5515550 
  src/declarativeimports/calendar/daysmodel.cpp 0f81b5a 
  src/declarativeimports/core/corebindingsplugin.cpp adfdc29 
  src/declarativeimports/core/datamodel.cpp 4449f30 
  src/declarativeimports/core/datasource.cpp 4fe5dc5 
  src/declarativeimports/core/tooltip.cpp a5e223b 
  src/declarativeimports/core/tooltipdialog.cpp 6c3712e 
  src/declarativeimports/core/units.cpp 1798727 
  src/declarativeimports/core/windowthumbnail.cpp 21e655e 
  src/declarativeimports/plasmacomponents/plasmacomponentsplugin.cpp 9e924b3 
  src/declarativeimports/plasmacomponents/qmenuitem.cpp 287e9b3 
  src/declarativeimports/plasmaextracomponents/plasmaextracomponentsplugin.cpp 
1a25f06 
  src/plasma/applet.cpp 4ce2d28 
  src/plasma/containment.cpp 0beb196 
  src/plasma/containmentactions.cpp f42807f 
  src/plasma/corona.cpp bae9244 
  src/plasma/datacontainer.cpp 396bc6d 
  src/plasma/dataengine.cpp dd56807 
  src/plasma/dataengineconsumer.cpp 7c9b5f9 
  src/plasma/framesvg.cpp 81187dc 
  src/plasma/package.cpp 09f36f3 
  src/plasma/packagestructure.cpp 09ea698 
  src/plasma/pluginloader.cpp d7c49f2 
  src/plasma/private/applet_p.cpp 949c92e 
  src/plasma/private/associatedapplicationmanager.cpp e1620e9 
  src/plasma/private/componentinstaller.cpp 8fbef24 
  src/plasma/private/containment_p.cpp 09ed2cd 
  src/plasma/private/dataenginemanager.cpp 7862171 
  src/plasma/private/packages.cpp 1edd55a 
  src/plasma/private/service_p.h 8a48487 
  src/plasma/private/storage.cpp bc6992e 
  src/plasma/private/storagethread.cpp 91b490b 
  src/plasma/private/svg_p.h 1d1000d 
  src/plasma/private/theme_p.cpp 18419bb 
  src/plasma/private/timetracker.cpp cdfe94b 
  src/plasma/scripting/scriptengine.cpp b9f43fe 
  src/plasma/service.cpp d603cf2 
  src/plasma/svg.cpp 28abd00 
  src/plasma/theme.cpp e9420e6 
  src/plasmapkg/main.cpp 336b14e 
  src/plasmapkg/plasmapkg.cpp 4626323 
  src/plasmaquick/appletquickitem.cpp ec2ed24 
  src/plasmaquick/configmodel.cpp df537c1 
  src/plasmaquick/configview.cpp c4ab518 
  src/plasmaquick/dialog.cpp 8f4ee57 
  src/plasmaquick/dialogshadows.cpp db408ae 
  src/plasmaquick/dialogshadows_p.h 7e17c12 
  src/plasmaquick/packageurlinterceptor.cpp 5e349d2 
  src/plasmaquick/private/packages.h aa08b11 
  src/plasmaquick/private/packages.cpp 5275848 
  src/scriptengines/qml/plasmoid/appletinterface.cpp 8e4979a 
  src/scriptengines/qml/plasmoid/containmentinterface.cpp 31285a2 
  src/scriptengines/qml/plasmoid/declarativeappletscript.cpp b15695b 
  src/scriptengines/qml/plasmoid/wallpaperinterface.cpp 9ecd62b 
  tests/dpi/main.cpp 6767b2d 
  tests/kplugins/main.cpp 421e3fb 
  tests/kplugins/plugintest.h a99701a 
  tests/kplugins/plugintest.cpp 3d98dec 
  tests/testengine/testengine.cpp 76947c3 

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


Testing
---

Tests passed


Thanks,

Sergey Popov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126672: Fix most of Clazy warnings in plasma-framework

2016-02-28 Thread Aleix Pol Gonzalez

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


Ship it!




I'll fix the issues pointed out by Kai and commit it, since Sergey seems AFK.

- Aleix Pol Gonzalez


On Jan. 17, 2016, 9:50 p.m., Sergey Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126672/
> ---
> 
> (Updated Jan. 17, 2016, 9:50 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Aleix Pol Gonzalez.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Fix almost all the Clazy warnings in plasma-framework.
> 
> Related GCI-2015 task: 
> [https://codein.withgoogle.com/tasks/6272625345036288/](https://codein.withgoogle.com/tasks/6272625345036288/)
> 
> 
> Diffs
> -
> 
>   autotests/coronatest.cpp 378a4b7 
>   autotests/dialogqmltest.cpp 618e64d 
>   autotests/fallbackpackagetest.cpp 91bc6e9 
>   autotests/packagestructuretest.cpp 67cdb4f 
>   autotests/pluginloadertest.cpp 868d5f8 
>   autotests/sortfiltermodeltest.cpp 6ee0e35 
>   autotests/storagetest.cpp 8a7dbd0 
>   src/declarativeimports/calendar/calendar.cpp 5515550 
>   src/declarativeimports/calendar/daysmodel.cpp 0f81b5a 
>   src/declarativeimports/core/corebindingsplugin.cpp adfdc29 
>   src/declarativeimports/core/datamodel.cpp 4449f30 
>   src/declarativeimports/core/datasource.cpp 4fe5dc5 
>   src/declarativeimports/core/tooltip.cpp a5e223b 
>   src/declarativeimports/core/tooltipdialog.cpp 6c3712e 
>   src/declarativeimports/core/units.cpp 1798727 
>   src/declarativeimports/core/windowthumbnail.cpp 21e655e 
>   src/declarativeimports/plasmacomponents/plasmacomponentsplugin.cpp 9e924b3 
>   src/declarativeimports/plasmacomponents/qmenuitem.cpp 287e9b3 
>   
> src/declarativeimports/plasmaextracomponents/plasmaextracomponentsplugin.cpp 
> 1a25f06 
>   src/plasma/applet.cpp 4ce2d28 
>   src/plasma/containment.cpp 0beb196 
>   src/plasma/containmentactions.cpp f42807f 
>   src/plasma/corona.cpp bae9244 
>   src/plasma/datacontainer.cpp 396bc6d 
>   src/plasma/dataengine.cpp dd56807 
>   src/plasma/dataengineconsumer.cpp 7c9b5f9 
>   src/plasma/framesvg.cpp 81187dc 
>   src/plasma/package.cpp 09f36f3 
>   src/plasma/packagestructure.cpp 09ea698 
>   src/plasma/pluginloader.cpp d7c49f2 
>   src/plasma/private/applet_p.cpp 949c92e 
>   src/plasma/private/associatedapplicationmanager.cpp e1620e9 
>   src/plasma/private/componentinstaller.cpp 8fbef24 
>   src/plasma/private/containment_p.cpp 09ed2cd 
>   src/plasma/private/dataenginemanager.cpp 7862171 
>   src/plasma/private/packages.cpp 1edd55a 
>   src/plasma/private/service_p.h 8a48487 
>   src/plasma/private/storage.cpp bc6992e 
>   src/plasma/private/storagethread.cpp 91b490b 
>   src/plasma/private/svg_p.h 1d1000d 
>   src/plasma/private/theme_p.cpp 18419bb 
>   src/plasma/private/timetracker.cpp cdfe94b 
>   src/plasma/scripting/scriptengine.cpp b9f43fe 
>   src/plasma/service.cpp d603cf2 
>   src/plasma/svg.cpp 28abd00 
>   src/plasma/theme.cpp e9420e6 
>   src/plasmapkg/main.cpp 336b14e 
>   src/plasmapkg/plasmapkg.cpp 4626323 
>   src/plasmaquick/appletquickitem.cpp ec2ed24 
>   src/plasmaquick/configmodel.cpp df537c1 
>   src/plasmaquick/configview.cpp c4ab518 
>   src/plasmaquick/dialog.cpp 8f4ee57 
>   src/plasmaquick/dialogshadows.cpp db408ae 
>   src/plasmaquick/dialogshadows_p.h 7e17c12 
>   src/plasmaquick/packageurlinterceptor.cpp 5e349d2 
>   src/plasmaquick/private/packages.h aa08b11 
>   src/plasmaquick/private/packages.cpp 5275848 
>   src/scriptengines/qml/plasmoid/appletinterface.cpp 8e4979a 
>   src/scriptengines/qml/plasmoid/containmentinterface.cpp 31285a2 
>   src/scriptengines/qml/plasmoid/declarativeappletscript.cpp b15695b 
>   src/scriptengines/qml/plasmoid/wallpaperinterface.cpp 9ecd62b 
>   tests/dpi/main.cpp 6767b2d 
>   tests/kplugins/main.cpp 421e3fb 
>   tests/kplugins/plugintest.h a99701a 
>   tests/kplugins/plugintest.cpp 3d98dec 
>   tests/testengine/testengine.cpp 76947c3 
> 
> Diff: https://git.reviewboard.kde.org/r/126672/diff/
> 
> 
> Testing
> ---
> 
> Tests passed
> 
> 
> Thanks,
> 
> Sergey Popov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126672: Fix most of Clazy warnings in plasma-framework

2016-02-28 Thread Aleix Pol Gonzalez


> On Feb. 1, 2016, 10 p.m., Kai Uwe Broulik wrote:
> > src/plasma/private/packages.cpp, line 99
> > 
> >
> > QByteArrayLiteral and below

QByteArrayLiteral is a weird thing, let's not get there.


- Aleix


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


On Jan. 17, 2016, 9:50 p.m., Sergey Popov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126672/
> ---
> 
> (Updated Jan. 17, 2016, 9:50 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Aleix Pol Gonzalez.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Fix almost all the Clazy warnings in plasma-framework.
> 
> Related GCI-2015 task: 
> [https://codein.withgoogle.com/tasks/6272625345036288/](https://codein.withgoogle.com/tasks/6272625345036288/)
> 
> 
> Diffs
> -
> 
>   autotests/coronatest.cpp 378a4b7 
>   autotests/dialogqmltest.cpp 618e64d 
>   autotests/fallbackpackagetest.cpp 91bc6e9 
>   autotests/packagestructuretest.cpp 67cdb4f 
>   autotests/pluginloadertest.cpp 868d5f8 
>   autotests/sortfiltermodeltest.cpp 6ee0e35 
>   autotests/storagetest.cpp 8a7dbd0 
>   src/declarativeimports/calendar/calendar.cpp 5515550 
>   src/declarativeimports/calendar/daysmodel.cpp 0f81b5a 
>   src/declarativeimports/core/corebindingsplugin.cpp adfdc29 
>   src/declarativeimports/core/datamodel.cpp 4449f30 
>   src/declarativeimports/core/datasource.cpp 4fe5dc5 
>   src/declarativeimports/core/tooltip.cpp a5e223b 
>   src/declarativeimports/core/tooltipdialog.cpp 6c3712e 
>   src/declarativeimports/core/units.cpp 1798727 
>   src/declarativeimports/core/windowthumbnail.cpp 21e655e 
>   src/declarativeimports/plasmacomponents/plasmacomponentsplugin.cpp 9e924b3 
>   src/declarativeimports/plasmacomponents/qmenuitem.cpp 287e9b3 
>   
> src/declarativeimports/plasmaextracomponents/plasmaextracomponentsplugin.cpp 
> 1a25f06 
>   src/plasma/applet.cpp 4ce2d28 
>   src/plasma/containment.cpp 0beb196 
>   src/plasma/containmentactions.cpp f42807f 
>   src/plasma/corona.cpp bae9244 
>   src/plasma/datacontainer.cpp 396bc6d 
>   src/plasma/dataengine.cpp dd56807 
>   src/plasma/dataengineconsumer.cpp 7c9b5f9 
>   src/plasma/framesvg.cpp 81187dc 
>   src/plasma/package.cpp 09f36f3 
>   src/plasma/packagestructure.cpp 09ea698 
>   src/plasma/pluginloader.cpp d7c49f2 
>   src/plasma/private/applet_p.cpp 949c92e 
>   src/plasma/private/associatedapplicationmanager.cpp e1620e9 
>   src/plasma/private/componentinstaller.cpp 8fbef24 
>   src/plasma/private/containment_p.cpp 09ed2cd 
>   src/plasma/private/dataenginemanager.cpp 7862171 
>   src/plasma/private/packages.cpp 1edd55a 
>   src/plasma/private/service_p.h 8a48487 
>   src/plasma/private/storage.cpp bc6992e 
>   src/plasma/private/storagethread.cpp 91b490b 
>   src/plasma/private/svg_p.h 1d1000d 
>   src/plasma/private/theme_p.cpp 18419bb 
>   src/plasma/private/timetracker.cpp cdfe94b 
>   src/plasma/scripting/scriptengine.cpp b9f43fe 
>   src/plasma/service.cpp d603cf2 
>   src/plasma/svg.cpp 28abd00 
>   src/plasma/theme.cpp e9420e6 
>   src/plasmapkg/main.cpp 336b14e 
>   src/plasmapkg/plasmapkg.cpp 4626323 
>   src/plasmaquick/appletquickitem.cpp ec2ed24 
>   src/plasmaquick/configmodel.cpp df537c1 
>   src/plasmaquick/configview.cpp c4ab518 
>   src/plasmaquick/dialog.cpp 8f4ee57 
>   src/plasmaquick/dialogshadows.cpp db408ae 
>   src/plasmaquick/dialogshadows_p.h 7e17c12 
>   src/plasmaquick/packageurlinterceptor.cpp 5e349d2 
>   src/plasmaquick/private/packages.h aa08b11 
>   src/plasmaquick/private/packages.cpp 5275848 
>   src/scriptengines/qml/plasmoid/appletinterface.cpp 8e4979a 
>   src/scriptengines/qml/plasmoid/containmentinterface.cpp 31285a2 
>   src/scriptengines/qml/plasmoid/declarativeappletscript.cpp b15695b 
>   src/scriptengines/qml/plasmoid/wallpaperinterface.cpp 9ecd62b 
>   tests/dpi/main.cpp 6767b2d 
>   tests/kplugins/main.cpp 421e3fb 
>   tests/kplugins/plugintest.h a99701a 
>   tests/kplugins/plugintest.cpp 3d98dec 
>   tests/testengine/testengine.cpp 76947c3 
> 
> Diff: https://git.reviewboard.kde.org/r/126672/diff/
> 
> 
> Testing
> ---
> 
> Tests passed
> 
> 
> Thanks,
> 
> Sergey Popov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: QString -> QStringLiteral conversions might make applications crash on exit

2016-02-28 Thread Frank Reininghaus
Hi Albert,

2016-02-28 11:54 GMT+01:00 Albert Astals Cid:
> El Friday 26 February 2016, a les 01:37:57, Frank Reininghaus va escriure:
>> Hi everyone,
>>
>> sorry if most of you know about this already, but since it seems that
>> QStringLiterals are being introduced everywhere right now, I think
>> that it is important to raise awareness about the fact that this might
>> be more dangerous that it seems at first sight.
>>
>> QStringLiteral has the nice property that it stores the string data in
>> read-only memory and avoids heap allocations when it is used to
>> construct a QString. The QString, and any copies that are made, just
>> contain a pointer to read-only data. There is a very nice overview by
>> Olivier at
>>
>> https://woboq.com/blog/qstringliteral.html
>>
>> However, QString is still a non-POD type, even if it has been
>> constructed with QStringLiteral (or copied from such a QString), so
>> its destructor is being run, which accesses the read-only data, then
>> finds out that it's been made with QStringLiteral, such that no
>> refcount updates and deallocations are needed.
>>
>> This becomes a problem if the read-only data that the QString refers
>> to are not there any more, which can happen if the QString was stored
>> in a global static object from one library, and the QStringLiteral is
>> from another library, which might have been unloaded before the global
>> static object was destroyed.
>>
>> I believe that this is just what happens right now with a global
>> static KIconLoader from kicontnkhemes and a QStringLiteral from
>> frameworkintegration:
>>
>> https://bugs.kde.org/show_bug.cgi?id=359758
>>
>> If my analysis is wrong, please let me know!
>
> If you know what commit causes it I'd say you have two options:
>  * Find exactly which of the changes in the patch is causing the problem, add
> a test case that crashes and then commit the smallest change that fixes the
> crash
>  * Revert the commit and CC the person that did the commit asking him to be
> more careful.

I could go for option 1 because I already found which QStringLiteral
caused the problem, but I'll try to find out in the next few days if
the workaround that Jan showed can also be used in KIconLoader. This
would prevent that the same problem happens again if QStringLiterals
are also used elsewhere to load icons.

But I'm not 100% sure if there aren't more places in KF5 and KDE
Applications where using QStringLiteral could cause similar problems.

Best regards,
Frank
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126765: Fix some Clazy warnings in KTextEditor.

2016-02-28 Thread Andrey Cygankov

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

(Updated Feb. 28, 2016, 10:18 p.m.)


Status
--

This change has been marked as submitted.


Review request for Kate, KDE Frameworks and Aleix Pol Gonzalez.


Changes
---

Submitted with commit a15145c2c473a4df806d81cf1e3c34dd4763fd2f by Aleix Pol to 
branch master.


Repository: ktexteditor


Description
---

Fix some Clazy warnings in KTextEditor.


Diffs
-

  autotests/src/bug205447.cpp a6c6c23 
  autotests/src/bug313759.cpp 9763454 
  autotests/src/bug313769.cpp c011345 
  autotests/src/bug317111.cpp ec6a5d1 
  autotests/src/commands_test.cpp 5a0e4c4 
  autotests/src/completion_test.cpp 919c2cb 
  autotests/src/testutils.cpp 06c22bf 
  src/buffer/katetextbuffer.cpp e7397c9 
  src/buffer/katetextfolding.cpp 54d407a 
  src/buffer/katetextloader.h 84d420a 
  src/completion/expandingtree/expandingdelegate.cpp 7eb03c5 
  src/completion/expandingtree/expandingwidgetmodel.cpp f4cad7a 
  src/completion/katecompletionconfig.cpp c0494ff 
  src/completion/katecompletionmodel.cpp b48c91c 
  src/completion/katekeywordcompletion.cpp f1ad42c 
  src/completion/katewordcompletion.cpp 925b381 
  src/dialogs/katedialogs.cpp 0d44f97 
  src/document/katedocument.cpp ef96a17 
  src/export/abstractexporter.h cc014dd 
  src/export/htmlexporter.cpp 7736a71 
  src/inputmode/kateviinputmode.cpp 1954b2e 
  src/mode/katemodeconfigpage.cpp 42a75de 
  src/mode/katemodemanager.cpp 641067f 
  src/printing/printconfigwidgets.cpp b082983 
  src/printing/printpainter.cpp 8368f9e 
  src/render/katelayoutcache.cpp 957006c 
  src/render/katerenderer.cpp b108e10 
  src/schema/katecolortreewidget.cpp 5b6c32c 
  src/schema/kateschema.cpp 90c91ef 
  src/schema/kateschemaconfig.cpp 082fce6 
  src/schema/katestyletreewidget.cpp e167ceb 
  src/script/katecommandlinescript.cpp afa3efa 
  src/script/kateindentscript.cpp 2050d4f 
  src/script/katescript.cpp 3dbc2e1 
  src/script/katescriptaction.cpp bf0984e 
  src/script/katescriptdocument.cpp c5a1784 
  src/script/katescripthelpers.cpp c078614 
  src/script/katescriptmanager.cpp 4c416d4 
  src/search/kateplaintextsearch.cpp 5a36577 
  src/search/kateregexp.cpp 1431f95 
  src/search/kateregexpsearch.cpp 8eb374b 
  src/search/katesearchbar.cpp 2e1ee99 
  src/spellcheck/spellcheckdialog.cpp 531d24f 
  src/spellcheck/spellingmenu.cpp 2319680 
  src/swapfile/kateswapdiffcreator.cpp ac09363 
  src/swapfile/kateswapfile.cpp f716af9 
  src/syntax/data/katehighlightingindexer.cpp d9b0d65 
  src/syntax/katehighlight.cpp c3412f7 
  src/syntax/katehighlighthelpers.cpp e953d23 
  src/syntax/katehighlightingcmds.h 802c0f9 
  src/syntax/katesyntaxdocument.cpp 15d629b 
  src/syntax/katesyntaxmanager.cpp 4d09fa3 
  src/undo/kateundomanager.cpp cc7366c 
  src/utils/attribute.cpp 130c496 
  src/utils/katebookmarks.cpp 2fcf924 
  src/utils/katecmd.cpp 462d4fd 
  src/utils/katecmds.h d02e700 
  src/utils/katecmds.cpp 2fd3a38 
  src/utils/katecommandrangeexpressionparser.cpp f57cb45 
  src/utils/kateconfig.cpp abf30cc 
  src/utils/kateglobal.cpp 66b70e2 
  src/utils/katesedcmd.h eb61d32 
  src/utils/katesedcmd.cpp 062a65e 
  src/utils/ktexteditor.cpp 815a40e 
  src/variableeditor/katehelpbutton.cpp a0e1914 
  src/variableeditor/variableitem.cpp b0e0a7b 
  src/variableeditor/variablelineedit.cpp 2a1af83 
  src/variableeditor/variablelistview.cpp 7316a7f 
  src/view/kateview.cpp 7e828fe 
  src/view/kateviewhelpers.cpp fc171f3 
  src/view/kateviewinternal.cpp f968a87 
  src/vimode/appcommands.cpp 2708487 
  src/vimode/cmds.h 03fc39f 
  src/vimode/cmds.cpp 7804af4 
  src/vimode/commandrangeexpressionparser.cpp 94e00c5 
  src/vimode/config/configtab.cpp 8074327 
  src/vimode/emulatedcommandbar.cpp e876b7a 
  src/vimode/inputmodemanager.cpp 5f797a0 
  src/vimode/keyparser.cpp 99f6fbf 
  src/vimode/lastchangerecorder.cpp e7b4d43 
  src/vimode/macros.cpp b96c099 
  src/vimode/mappings.cpp 82388cf 
  src/vimode/modes/insertvimode.cpp ed71cd4 
  src/vimode/modes/modebase.cpp 5016b96 
  src/vimode/modes/normalvimode.cpp c0eaf40 
  src/vimode/modes/visualvimode.cpp c58277b 
  src/vimode/searcher.cpp 133ff9b 

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


Testing
---

Built without errors.
All tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126765: Fix some Clazy warnings in KTextEditor.

2016-02-28 Thread Aleix Pol Gonzalez

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


Ship it!




Ship It!

- Aleix Pol Gonzalez


On Jan. 16, 2016, 3:15 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126765/
> ---
> 
> (Updated Jan. 16, 2016, 3:15 p.m.)
> 
> 
> Review request for Kate, KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: ktexteditor
> 
> 
> Description
> ---
> 
> Fix some Clazy warnings in KTextEditor.
> 
> 
> Diffs
> -
> 
>   autotests/src/bug205447.cpp a6c6c23 
>   autotests/src/bug313759.cpp 9763454 
>   autotests/src/bug313769.cpp c011345 
>   autotests/src/bug317111.cpp ec6a5d1 
>   autotests/src/commands_test.cpp 5a0e4c4 
>   autotests/src/completion_test.cpp 919c2cb 
>   autotests/src/testutils.cpp 06c22bf 
>   src/buffer/katetextbuffer.cpp e7397c9 
>   src/buffer/katetextfolding.cpp 54d407a 
>   src/buffer/katetextloader.h 84d420a 
>   src/completion/expandingtree/expandingdelegate.cpp 7eb03c5 
>   src/completion/expandingtree/expandingwidgetmodel.cpp f4cad7a 
>   src/completion/katecompletionconfig.cpp c0494ff 
>   src/completion/katecompletionmodel.cpp b48c91c 
>   src/completion/katekeywordcompletion.cpp f1ad42c 
>   src/completion/katewordcompletion.cpp 925b381 
>   src/dialogs/katedialogs.cpp 0d44f97 
>   src/document/katedocument.cpp ef96a17 
>   src/export/abstractexporter.h cc014dd 
>   src/export/htmlexporter.cpp 7736a71 
>   src/inputmode/kateviinputmode.cpp 1954b2e 
>   src/mode/katemodeconfigpage.cpp 42a75de 
>   src/mode/katemodemanager.cpp 641067f 
>   src/printing/printconfigwidgets.cpp b082983 
>   src/printing/printpainter.cpp 8368f9e 
>   src/render/katelayoutcache.cpp 957006c 
>   src/render/katerenderer.cpp b108e10 
>   src/schema/katecolortreewidget.cpp 5b6c32c 
>   src/schema/kateschema.cpp 90c91ef 
>   src/schema/kateschemaconfig.cpp 082fce6 
>   src/schema/katestyletreewidget.cpp e167ceb 
>   src/script/katecommandlinescript.cpp afa3efa 
>   src/script/kateindentscript.cpp 2050d4f 
>   src/script/katescript.cpp 3dbc2e1 
>   src/script/katescriptaction.cpp bf0984e 
>   src/script/katescriptdocument.cpp c5a1784 
>   src/script/katescripthelpers.cpp c078614 
>   src/script/katescriptmanager.cpp 4c416d4 
>   src/search/kateplaintextsearch.cpp 5a36577 
>   src/search/kateregexp.cpp 1431f95 
>   src/search/kateregexpsearch.cpp 8eb374b 
>   src/search/katesearchbar.cpp 2e1ee99 
>   src/spellcheck/spellcheckdialog.cpp 531d24f 
>   src/spellcheck/spellingmenu.cpp 2319680 
>   src/swapfile/kateswapdiffcreator.cpp ac09363 
>   src/swapfile/kateswapfile.cpp f716af9 
>   src/syntax/data/katehighlightingindexer.cpp d9b0d65 
>   src/syntax/katehighlight.cpp c3412f7 
>   src/syntax/katehighlighthelpers.cpp e953d23 
>   src/syntax/katehighlightingcmds.h 802c0f9 
>   src/syntax/katesyntaxdocument.cpp 15d629b 
>   src/syntax/katesyntaxmanager.cpp 4d09fa3 
>   src/undo/kateundomanager.cpp cc7366c 
>   src/utils/attribute.cpp 130c496 
>   src/utils/katebookmarks.cpp 2fcf924 
>   src/utils/katecmd.cpp 462d4fd 
>   src/utils/katecmds.h d02e700 
>   src/utils/katecmds.cpp 2fd3a38 
>   src/utils/katecommandrangeexpressionparser.cpp f57cb45 
>   src/utils/kateconfig.cpp abf30cc 
>   src/utils/kateglobal.cpp 66b70e2 
>   src/utils/katesedcmd.h eb61d32 
>   src/utils/katesedcmd.cpp 062a65e 
>   src/utils/ktexteditor.cpp 815a40e 
>   src/variableeditor/katehelpbutton.cpp a0e1914 
>   src/variableeditor/variableitem.cpp b0e0a7b 
>   src/variableeditor/variablelineedit.cpp 2a1af83 
>   src/variableeditor/variablelistview.cpp 7316a7f 
>   src/view/kateview.cpp 7e828fe 
>   src/view/kateviewhelpers.cpp fc171f3 
>   src/view/kateviewinternal.cpp f968a87 
>   src/vimode/appcommands.cpp 2708487 
>   src/vimode/cmds.h 03fc39f 
>   src/vimode/cmds.cpp 7804af4 
>   src/vimode/commandrangeexpressionparser.cpp 94e00c5 
>   src/vimode/config/configtab.cpp 8074327 
>   src/vimode/emulatedcommandbar.cpp e876b7a 
>   src/vimode/inputmodemanager.cpp 5f797a0 
>   src/vimode/keyparser.cpp 99f6fbf 
>   src/vimode/lastchangerecorder.cpp e7b4d43 
>   src/vimode/macros.cpp b96c099 
>   src/vimode/mappings.cpp 82388cf 
>   src/vimode/modes/insertvimode.cpp ed71cd4 
>   src/vimode/modes/modebase.cpp 5016b96 
>   src/vimode/modes/normalvimode.cpp c0eaf40 
>   src/vimode/modes/visualvimode.cpp c58277b 
>   src/vimode/searcher.cpp 133ff9b 
> 
> Diff: https://git.reviewboard.kde.org/r/126765/diff/
> 
> 
> Testing
> ---
> 
> Built without errors.
> All tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org

Review Request 127211: Properly use static QMaps

2016-02-28 Thread Aleix Pol Gonzalez

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

Review request for Kate and KDE Frameworks.


Repository: ktexteditor


Description
---

Only initialize them once.
Since they're not const, use ::value rather than ::operator[], because the 
latter will provide a default-constructed object and add it to the map.


Diffs
-

  src/vimode/cmds.cpp 7804af4 

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


Testing
---


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126769: Fix all Clazy warnings in KDED.

2016-02-28 Thread Andrey Cygankov

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

(Updated Feb. 28, 2016, 9:38 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Aleix Pol Gonzalez.


Changes
---

Submitted with commit 706bf6f39319f6c2b79ad82c689d7506747b98c7 by Aleix Pol on 
behalf of Andrey Cygankov to branch master.


Repository: kded


Description
---

Fix all Clazy warnings in KDED.


Diffs
-

  src/kded.cpp 0eb6380 

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


Testing
---

Built without errors.
No tests available.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126772: Fix some Clazy warnings in KService

2016-02-28 Thread Andrey Cygankov

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

(Updated Feb. 28, 2016, 9:30 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Aleix Pol Gonzalez.


Changes
---

Submitted with commit 1939360c50cba1241b01108fc32e73ef88c1d83d by Aleix Pol on 
behalf of Andrey Cygankov to branch master.


Repository: kservice


Description
---

Fix some Clazy warnings in KService.


Diffs
-

  autotests/kautostarttest.cpp 1949baa 
  autotests/kmimeassociationstest.cpp 60818f7 
  autotests/kplugininfotest.cpp 9ddc8cf 
  autotests/kservicetest.cpp 6dc6f98 
  autotests/ksycoca_xdgdirstest.cpp f879959 
  autotests/ksycocadicttest.cpp 302853d 
  autotests/ksycocatest.cpp 4af759c 
  autotests/ksycocathreadtest.cpp fba9cae 
  autotests/pluginlocatortest.cpp 528a2a5 
  src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
  src/kdeinit/ktoolinvocation_x11.cpp 58f2c7f 
  src/services/kautostart.cpp 86f0270 
  src/services/kmimetypetrader.h 6882b75 
  src/services/kplugininfo.cpp 3627fe8 
  src/services/kservice.cpp 5bbca9a 
  src/services/kservicegroup.cpp 54cace5 
  src/services/kservicetype.cpp 5f66f51 
  src/services/kservicetypeprofile.cpp c22645b 
  src/sycoca/kbuildmimetypefactory.cpp 5d32b35 
  src/sycoca/kbuildservicefactory.cpp dd81860 
  src/sycoca/kbuildservicetypefactory.cpp bf5e663 
  src/sycoca/kbuildsycoca.cpp 650526b 
  src/sycoca/kmimeassociations.cpp 922a447 
  src/sycoca/ksycoca.cpp 5744e95 
  src/sycoca/ksycocautils_p.h caeb0e0 
  src/sycoca/vfolder_menu.cpp d3e31c3 
  tests/kdbusservicestartertest.cpp 8174485 
  tests/pluginlocator/main.cpp f1dcfae 
  tests/pluginlocator/plugintest.h ab41c7c 
  tests/pluginlocator/plugintest.cpp 82df4dc 
  tests/startserviceby.cpp 1a683b3 

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


Testing
---

Built without errors.

Without patch: Failed kmimeassociationstest
With patch: Failed kmimeassociationstest


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127209: Fix some issues found by Coverity

2016-02-28 Thread Lamarque Souza

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




src/lib/kaboutdata.cpp (line 993)


I think you must change the second "%s" to "%1" here, otherwise the value 
returned by qAppName() will be added to the string created by 
QCoreApplication::translate().



src/lib/kaboutdata.cpp (line 1006)


Same here.


- Lamarque Souza


On Feb. 28, 2016, 7:46 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127209/
> ---
> 
> (Updated Feb. 28, 2016, 7:46 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> Mostly initializing values and using printf() correctly.
> 
> 
> Diffs
> -
> 
>   src/lib/io/kdirwatch.cpp f2fd3b8 
>   src/lib/io/kprocess_p.h 70fe91f 
>   src/lib/kaboutdata.cpp a220977 
>   src/lib/randomness/krandom.cpp bdbdec6 
>   src/lib/util/kformatprivate.cpp 3d98007 
> 
> Diff: https://git.reviewboard.kde.org/r/127209/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 127210: Fix warnings

2016-02-28 Thread Aleix Pol Gonzalez

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

Review request for KDE Frameworks and KDEPIM.


Repository: kpeople


Description
---

* Fixes Coverity issue that notifies that the factory might be null by checking 
and adding a warning in case it is.
* Uses the correct overload for ::create, this way we don't get a _using 
deprecated API_ warning anymore.


Diffs
-

  src/widgets/actions.cpp 5ac56a2 

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


Testing
---

Builds, tests pass.


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 127209: Fix some issues found by Coverity

2016-02-28 Thread Aleix Pol Gonzalez

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

Review request for KDE Frameworks.


Repository: kcoreaddons


Description
---

Mostly initializing values and using printf() correctly.


Diffs
-

  src/lib/io/kdirwatch.cpp f2fd3b8 
  src/lib/io/kprocess_p.h 70fe91f 
  src/lib/kaboutdata.cpp a220977 
  src/lib/randomness/krandom.cpp bdbdec6 
  src/lib/util/kformatprivate.cpp 3d98007 

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


Testing
---


Thanks,

Aleix Pol Gonzalez

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127201: Fix svg icon path resolving in IconItem

2016-02-28 Thread Xuetian Weng


> On Feb. 28, 2016, 9:58 a.m., David Rosca wrote:
> > autotests/iconitemtest.cpp, line 234
> > 
> >
> > Why?
> 
> Xuetian Weng wrote:
> Actually this check is not so reliable if the icon is not rendered with 
> the same svg backend but it will pass for now.
> 
> David Rosca wrote:
> But both items use exactly the same icon, so they should be rendered the 
> same. Do you have example when it fails?
> 
> Xuetian Weng wrote:
> You could try to move bug359388.svg to data/icons/test-theme/apps/22/ , 
> and comment out the QSKIP.
> 
> I guess the problem is that it might just render a svg scaled in some 
> cases when using QIcon, in some other cases, it will just render the svg 
> directly to the correct size.

Ah, ok , the problem here is my test environment has QT_DEVICE_PIXEL_RATIO set.


- Xuetian


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


On Feb. 28, 2016, 5:15 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127201/
> ---
> 
> (Updated Feb. 28, 2016, 5:15 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
> Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Well.. the bug should be obvious:
> 1. QString iconPath overrides the iconPath in a higher scope. introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 2. iconPath not assigned, also introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/
> 
> 
> Diffs
> -
> 
>   autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
>   autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
>   autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
>   autotests/data/icons/test-theme/index.theme PRE-CREATION 
>   autotests/iconitemtest.h 9fd3063 
>   autotests/iconitemtest.cpp ea98c9d 
>   src/declarativeimports/core/iconitem.cpp c65a9a7 
> 
> Diff: https://git.reviewboard.kde.org/r/127201/diff/
> 
> 
> Testing
> ---
> 
> tested with qmlscene with Plasma.IconItem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127201: Fix svg icon path resolving in IconItem

2016-02-28 Thread Xuetian Weng


> On Feb. 28, 2016, 9:58 a.m., David Rosca wrote:
> > autotests/iconitemtest.cpp, line 234
> > 
> >
> > Why?
> 
> Xuetian Weng wrote:
> Actually this check is not so reliable if the icon is not rendered with 
> the same svg backend but it will pass for now.
> 
> David Rosca wrote:
> But both items use exactly the same icon, so they should be rendered the 
> same. Do you have example when it fails?

You could try to move bug359388.svg to data/icons/test-theme/apps/22/ , and 
comment out the QSKIP.

I guess the problem is that it might just render a svg scaled in some cases 
when using QIcon, in some other cases, it will just render the svg directly to 
the correct size.


- Xuetian


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


On Feb. 28, 2016, 5:15 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127201/
> ---
> 
> (Updated Feb. 28, 2016, 5:15 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
> Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Well.. the bug should be obvious:
> 1. QString iconPath overrides the iconPath in a higher scope. introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 2. iconPath not assigned, also introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/
> 
> 
> Diffs
> -
> 
>   autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
>   autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
>   autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
>   autotests/data/icons/test-theme/index.theme PRE-CREATION 
>   autotests/iconitemtest.h 9fd3063 
>   autotests/iconitemtest.cpp ea98c9d 
>   src/declarativeimports/core/iconitem.cpp c65a9a7 
> 
> Diff: https://git.reviewboard.kde.org/r/127201/diff/
> 
> 
> Testing
> ---
> 
> tested with qmlscene with Plasma.IconItem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127201: Fix svg icon path resolving in IconItem

2016-02-28 Thread David Rosca


> On Feb. 28, 2016, 9:58 a.m., David Rosca wrote:
> > autotests/iconitemtest.cpp, line 234
> > 
> >
> > Why?
> 
> Xuetian Weng wrote:
> Actually this check is not so reliable if the icon is not rendered with 
> the same svg backend but it will pass for now.

But both items use exactly the same icon, so they should be rendered the same. 
Do you have example when it fails?


- David


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


On Feb. 28, 2016, 5:15 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127201/
> ---
> 
> (Updated Feb. 28, 2016, 5:15 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
> Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Well.. the bug should be obvious:
> 1. QString iconPath overrides the iconPath in a higher scope. introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 2. iconPath not assigned, also introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/
> 
> 
> Diffs
> -
> 
>   autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
>   autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
>   autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
>   autotests/data/icons/test-theme/index.theme PRE-CREATION 
>   autotests/iconitemtest.h 9fd3063 
>   autotests/iconitemtest.cpp ea98c9d 
>   src/declarativeimports/core/iconitem.cpp c65a9a7 
> 
> Diff: https://git.reviewboard.kde.org/r/127201/diff/
> 
> 
> Testing
> ---
> 
> tested with qmlscene with Plasma.IconItem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126291: initial implementation of a platform plugin for OS X (WIP)

2016-02-28 Thread René J . V . Bertin

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

(Updated Feb. 28, 2016, 6:15 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

I've activated the only autotest that can be ported with only trivial changes. 
I've also made some progress with the internal WId registry; it doesn't update 
yet when new windows are created so not all of the tests from 
kwindowsystem_threadtest succeed ATM.

I had a look at moving `setMainWindow()` method to the plugin architecture (and 
calling it through the plugin on OS X only). I got stuck on a compiler error 
I'd never seen before, about `connect()` being undefined. I've reverted those 
changes for now for lack of time to pursue that issue.


Repository: kwindowsystem


Description
---

KWindowSystem has been lacking a platform plugin for OS X. This RR presents a 
"backport" of the modified KDE4 KWindowSystem implementation that has been used 
in the MacPorts kdelibs4 port for the last 2 or 3 (or more) years.

I have made some initial steps to remove deprecated Carbon API calls, but this 
is clearly a work in progress.

Open questions include
- is there any justification to run an event handler (or Cocoa observer) to 
keep track of running applications, possibly even listing all their open 
windows?
- is there any use for the Qt event listener framework (cf. the NETEventFilter 
in the X11 plugin)? I haven't yet had time to try to figure out what this 
"could be good for", and am very open to suggestions in this departments.
- one application for such an event filter would be a listener that catches the 
opening and closing of all windows by the running process, and keeps track of 
their `WId`s. A new method could then be added (to `KWindowInfo`?) to 
distinguish `WId`s created by the running application from "foreign" ones 
(useful also on Wayland and MS Windows).

`KWindowSystem::setMainWindow` should become a front for payload provided by 
the plugins. I'll leave that to the regular/official maintainer(s) of this 
framework.

This code could probably do with *lots* of comments; I'll try to add them as 
questions about this or that bit of code arise.


Diffs (updated)
-

  autotests/CMakeLists.txt 65ed8d4 
  autotests/kwindowsystem_threadtest.cpp a142bae 
  src/debug_p.h 5ef8996 
  src/debug_p.cpp 72abfb7 
  src/kwindowsystem.cpp 407a67d 
  src/platforms/osx/CMakeLists.txt 4fc3347 
  src/platforms/osx/SCRAPBOOK PRE-CREATION 
  src/platforms/osx/cocoa.json PRE-CREATION 
  src/platforms/osx/kkeyserver.cpp 3ddb921 
  src/platforms/osx/kwindowinfo.cpp e8555bb 
  src/platforms/osx/kwindowinfo.mm PRE-CREATION 
  src/platforms/osx/kwindowinfo_mac_p.h c8f307e 
  src/platforms/osx/kwindowinfo_p_cocoa.h PRE-CREATION 
  src/platforms/osx/kwindowsystem.cpp 1758829 
  src/platforms/osx/kwindowsystem_mac_p.h PRE-CREATION 
  src/platforms/osx/kwindowsystem_macobjc.mm PRE-CREATION 
  src/platforms/osx/kwindowsystem_p_cocoa.h PRE-CREATION 
  src/platforms/osx/plugin.h PRE-CREATION 
  src/platforms/osx/plugin.cpp PRE-CREATION 

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


Testing
---

On OS X 10.9.5 with Qt 5.5.1 and frameworks 5.16.0 .


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127201: Fix svg icon path resolving in IconItem

2016-02-28 Thread Xuetian Weng

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

(Updated Feb. 28, 2016, 5:15 p.m.)


Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
Marco Martin.


Changes
---

Fixes for David Rosca's comment


Repository: plasma-framework


Description
---

Well.. the bug should be obvious:
1. QString iconPath overrides the iconPath in a higher scope. introduced in 
https://git.reviewboard.kde.org/r/126168/
2. iconPath not assigned, also introduced in 
https://git.reviewboard.kde.org/r/126168/
3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/


Diffs (updated)
-

  autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
  autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
  autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
PRE-CREATION 
  autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
PRE-CREATION 
  autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
  autotests/data/icons/test-theme/index.theme PRE-CREATION 
  autotests/iconitemtest.h 9fd3063 
  autotests/iconitemtest.cpp ea98c9d 
  src/declarativeimports/core/iconitem.cpp c65a9a7 

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


Testing
---

tested with qmlscene with Plasma.IconItem.


Thanks,

Xuetian Weng

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127201: Fix svg icon path resolving in IconItem

2016-02-28 Thread Xuetian Weng


> On Feb. 28, 2016, 9:58 a.m., David Rosca wrote:
> > autotests/iconitemtest.cpp, line 253
> > 
> >
> > This should be `QSize(32, 32)` as the comment says?

I indented to use a size without exact match.


> On Feb. 28, 2016, 9:58 a.m., David Rosca wrote:
> > autotests/iconitemtest.cpp, line 234
> > 
> >
> > Why?

Actually this check is not so reliable if the icon is not rendered with the 
same svg backend but it will pass for now.


- Xuetian


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


On Feb. 28, 2016, 1:18 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127201/
> ---
> 
> (Updated Feb. 28, 2016, 1:18 a.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
> Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Well.. the bug should be obvious:
> 1. QString iconPath overrides the iconPath in a higher scope. introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 2. iconPath not assigned, also introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/
> 
> 
> Diffs
> -
> 
>   autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
>   autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
>   autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
>   autotests/data/icons/test-theme/index.theme PRE-CREATION 
>   autotests/iconitemtest.h 9fd3063 
>   autotests/iconitemtest.cpp f675465 
>   src/declarativeimports/core/iconitem.cpp c65a9a7 
> 
> Diff: https://git.reviewboard.kde.org/r/127201/diff/
> 
> 
> Testing
> ---
> 
> tested with qmlscene with Plasma.IconItem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127169: By default, make KDE_INSTALL_USE_QT_SYS_PATHS share the same directory scheme as Qt if they share the prefix

2016-02-28 Thread Aleix Pol
On Sun, Feb 28, 2016 at 9:50 AM, Stephen Kelly  wrote:
> Aleix Pol Gonzalez wrote:
>>> I think it would be good to have changes be better understood,
>>> otherwise we end up with lots of mess. However, it looks like several
>>> people really want this, and I don't want to stand in the way.
>>
>> I don't mind spending some time to explain it better.
>>
>> Currently what we're doing from ECM, by default, is to _guess_ where Qt
>> will have placed things. Nowadays, `KDE_INSTALL_QTQUICKIMPORTSDIR` points
>> to `${KDE_INSTALL_QTPLUGINDIR}/imports`. See `KDEInstallDirs.cmake:440`.
>>
>> If the Qt installation is configured to get the Qt imports to go to
>> `/usr/lib/IKnowBetter/imports` and we don't change our guess, we'll get
>> both the Qt directory and a `/usr/lib64/plugins/imports` directory where
>> the cmake projects will naïvely place the plugins.
>>
>> An example of a case where we can see people suffering this (I found over
>> a fast google search) is this:
>> https://forum.kde.org/viewtopic.php?f=25=130498
>
>
> Thanks for explaining!
>
> So, this isn't being done at the request of packagers? I've asked a few
> times if packagers have complained and no answer has come back. From now I'm
> just going to assume that no packager has complained about the default of
> the variable.

No packager complained because they just know they have to enable the
variable and be done with it.

> Instead this is motivated my issues like the one in the forum post?

Correct. Also I've been bugged about the issue a couple of times.

> What actually is the problem in the forum post? Will the plugin be found at
> runtime? Does the location simply not match the expectations of the person
> writing it, or is there some bad effect resulting from this?

The problem is that Qt won't find the plugin and the user will have to
define QT_PLUGIN_PATH.

> If this is about users installing self-built things to /usr? Should that
> person be installing things to /usr? My understanding up to now has been
> 'no, they should not'.
It's about easily co-installing with Qt. It will work better if they
install to /usr, but that doesn't mean they have to.

> So, I still don't understand. Maybe this is about whether or not the plugin
> can be found at runtime, but that has not been said, unless I missed it. I
> could understand this discussion being tedious for you, and still I don't
> want to stand in the way!

I've got the ship it, I'll commit later today, we can continue
discussing though. I think it's a good discussion, especially
considering it's at the core on the experience the newcomers receive
when developing on KF5 and by extension any KDE software.

And yes, it's mainly about getting Qt to find the run-time resources
(i.e. plugins, yes) without starting to push down environment
variables to let Qt find what ECM placed naively.

Aleix
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127204: [KHintSettings] Disable setting the palette on-the-fly

2016-02-28 Thread David Faure

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



Why not fix this in Qt? What's the associated Qt bug report, at least?

- David Faure


On Feb. 28, 2016, 10:19 a.m., Anthony Fieroni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127204/
> ---
> 
> (Updated Feb. 28, 2016, 10:19 a.m.)
> 
> 
> Review request for KDE Frameworks, David Edmundson and Hugo Pereira Da Costa.
> 
> 
> Bugs: 341940 and 357800
> https://bugs.kde.org/show_bug.cgi?id=341940
> https://bugs.kde.org/show_bug.cgi?id=357800
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> [QApplication/QGuiApplication] setPalette is too buggy in current state (Qt 
> 5.5.1)
> 
> 
> Diffs
> -
> 
>   src/platformtheme/khintssettings.cpp 8adf6c5 
> 
> Diff: https://git.reviewboard.kde.org/r/127204/diff/
> 
> 
> Testing
> ---
> 
> 1. Open Dolphin, Kate any other KDE/Qt5 app
> 2. kcmshell5 colors
> 3. Look at Xorg ram usage
> 3. Apply new colors
> 4. Look again Xorg ram - it's pro rata on opened apps
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: QString -> QStringLiteral conversions might make applications crash on exit

2016-02-28 Thread Albert Astals Cid
El Friday 26 February 2016, a les 01:37:57, Frank Reininghaus va escriure:
> Hi everyone,
> 
> sorry if most of you know about this already, but since it seems that
> QStringLiterals are being introduced everywhere right now, I think
> that it is important to raise awareness about the fact that this might
> be more dangerous that it seems at first sight.
> 
> QStringLiteral has the nice property that it stores the string data in
> read-only memory and avoids heap allocations when it is used to
> construct a QString. The QString, and any copies that are made, just
> contain a pointer to read-only data. There is a very nice overview by
> Olivier at
> 
> https://woboq.com/blog/qstringliteral.html
> 
> However, QString is still a non-POD type, even if it has been
> constructed with QStringLiteral (or copied from such a QString), so
> its destructor is being run, which accesses the read-only data, then
> finds out that it's been made with QStringLiteral, such that no
> refcount updates and deallocations are needed.
> 
> This becomes a problem if the read-only data that the QString refers
> to are not there any more, which can happen if the QString was stored
> in a global static object from one library, and the QStringLiteral is
> from another library, which might have been unloaded before the global
> static object was destroyed.
> 
> I believe that this is just what happens right now with a global
> static KIconLoader from kicontnkhemes and a QStringLiteral from
> frameworkintegration:
> 
> https://bugs.kde.org/show_bug.cgi?id=359758
> 
> If my analysis is wrong, please let me know!

If you know what commit causes it I'd say you have two options:
 * Find exactly which of the changes in the patch is causing the problem, add 
a test case that crashes and then commit the smallest change that fixes the 
crash
 * Revert the commit and CC the person that did the commit asking him to be 
more careful.

Cheers,
  Albert
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 127204: [KHintSettings] Disable setting the palette on-the-fly

2016-02-28 Thread Anthony Fieroni

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

Review request for KDE Frameworks, David Edmundson and Hugo Pereira Da Costa.


Bugs: 341940 and 357800
https://bugs.kde.org/show_bug.cgi?id=341940
https://bugs.kde.org/show_bug.cgi?id=357800


Repository: frameworkintegration


Description
---

[QApplication/QGuiApplication] setPalette is too buggy in current state (Qt 
5.5.1)


Diffs
-

  src/platformtheme/khintssettings.cpp 8adf6c5 

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


Testing
---

1. Open Dolphin, Kate any other KDE/Qt5 app
2. kcmshell5 colors
3. Look at Xorg ram usage
3. Apply new colors
4. Look again Xorg ram - it's pro rata on opened apps


Thanks,

Anthony Fieroni

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 127205: Add stubs to allow compilation on Android.

2016-02-28 Thread Andreas Cord-Landwehr

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

Review request for KDE Frameworks, Aleix Pol Gonzalez and Alex Richardson.


Repository: kcoreaddons


Description
---

Android's libc implementation lacks several features required by the Unix 
backend implementation. This patch allows compilation by adding stubs for all 
missing struct members and functions. Result of this patch is that on Android 
KUser returns empty GUID and UID lists.


Diffs
-

  src/lib/CMakeLists.txt cf57f0947f13b126f658774209363480013a2e2a 
  src/lib/util/kuser_unix.cpp de5acde07f4cb8425f03c455bc55d03dfd2579e9 

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


Testing
---

Compilation tested on Linux and Android.


Thanks,

Andreas Cord-Landwehr

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127201: Fix svg icon path resolving in IconItem

2016-02-28 Thread David Rosca

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




autotests/iconitemtest.cpp (line 133)


Now that you've enabled QStandardPaths test mode, maybe you can write 
plasma theme name to plasmarc and remove this check?



autotests/iconitemtest.cpp (line 220)


Indent



autotests/iconitemtest.cpp (line 234)


Why?



autotests/iconitemtest.cpp (line 253)


This should be `QSize(32, 32)` as the comment says?


- David Rosca


On Feb. 28, 2016, 1:18 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127201/
> ---
> 
> (Updated Feb. 28, 2016, 1:18 a.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Kai Uwe Broulik, David Rosca, and 
> Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Well.. the bug should be obvious:
> 1. QString iconPath overrides the iconPath in a higher scope. introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 2. iconPath not assigned, also introduced in 
> https://git.reviewboard.kde.org/r/126168/
> 3. svgz -> svg?? introduced in https://git.reviewboard.kde.org/r/126557/
> 
> 
> Diffs
> -
> 
>   autotests/data/bug359388/hicolor/22x22/apps/bug359388.svg PRE-CREATION 
>   autotests/data/icons/hicolor/22x22/apps/bug359388.svg 8739af5 
>   autotests/data/icons/test-theme/apps/22/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/32/tst-plasma-framework-test-icon.svg 
> PRE-CREATION 
>   autotests/data/icons/test-theme/apps/48/konversation.svg PRE-CREATION 
>   autotests/data/icons/test-theme/index.theme PRE-CREATION 
>   autotests/iconitemtest.h 9fd3063 
>   autotests/iconitemtest.cpp f675465 
>   src/declarativeimports/core/iconitem.cpp c65a9a7 
> 
> Diff: https://git.reviewboard.kde.org/r/127201/diff/
> 
> 
> Testing
> ---
> 
> tested with qmlscene with Plasma.IconItem.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 127169: By default, make KDE_INSTALL_USE_QT_SYS_PATHS share the same directory scheme as Qt if they share the prefix

2016-02-28 Thread Stephen Kelly
Aleix Pol Gonzalez wrote: 
>> I think it would be good to have changes be better understood,
>> otherwise we end up with lots of mess. However, it looks like several
>> people really want this, and I don't want to stand in the way.
> 
> I don't mind spending some time to explain it better.
> 
> Currently what we're doing from ECM, by default, is to _guess_ where Qt
> will have placed things. Nowadays, `KDE_INSTALL_QTQUICKIMPORTSDIR` points
> to `${KDE_INSTALL_QTPLUGINDIR}/imports`. See `KDEInstallDirs.cmake:440`.
> 
> If the Qt installation is configured to get the Qt imports to go to
> `/usr/lib/IKnowBetter/imports` and we don't change our guess, we'll get
> both the Qt directory and a `/usr/lib64/plugins/imports` directory where
> the cmake projects will naïvely place the plugins.
> 
> An example of a case where we can see people suffering this (I found over
> a fast google search) is this:
> https://forum.kde.org/viewtopic.php?f=25=130498


Thanks for explaining! 

So, this isn't being done at the request of packagers? I've asked a few 
times if packagers have complained and no answer has come back. From now I'm 
just going to assume that no packager has complained about the default of 
the variable.

Instead this is motivated my issues like the one in the forum post? 

What actually is the problem in the forum post? Will the plugin be found at 
runtime? Does the location simply not match the expectations of the person 
writing it, or is there some bad effect resulting from this?

If this is about users installing self-built things to /usr? Should that 
person be installing things to /usr? My understanding up to now has been 
'no, they should not'.

So, I still don't understand. Maybe this is about whether or not the plugin 
can be found at runtime, but that has not been said, unless I missed it. I 
could understand this discussion being tedious for you, and still I don't 
want to stand in the way!

Thanks,

Steve.


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel