Re: Review Request 125274: KBuildSycoca: remove writing of the ksycoca5stamp file.

[Aleix Pol Gonzalez]

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


+1

- Aleix Pol Gonzalez


On Sept. 17, 2015, 1:32 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125274/
> ---
> 
> (Updated Sept. 17, 2015, 1:32 a.m.)
> 
> 
> Review request for KDE Frameworks and Albert Astals Cid.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> The code for reading it was already removed as part of b0c8fd8e64f,
> the removal of the --checkstamps option.
> 
> The feature still exists anyway, a timestamp of "at least everything until
> that time is in the DB" is in the sycoca header, and it used by the timestamp
> check in ksycoca.cpp. This was an unnecessary separate file.
> 
> REVIEW: 125274
> 
> 
> Diffs
> -
> 
>   docs/kbuildsycoca5/man-kbuildsycoca5.8.docbook 
> 3419e42cb29c6699bc17e6dd46bbc523139c59eb 
>   src/sycoca/kbuildsycoca.cpp 3685211e9da68f14516ec2b3d9a7e6b4f559b6f3 
> 
> Diff: https://git.reviewboard.kde.org/r/125274/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125278: KSycoca: add a q pointer to remove more singleton usage

[Aleix Pol Gonzalez]

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


+1

- Aleix Pol Gonzalez


On Sept. 17, 2015, 1:28 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125278/
> ---
> 
> (Updated Sept. 17, 2015, 1:28 a.m.)
> 
> 
> Review request for KDE Frameworks and Albert Astals Cid.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> This change is needed for an upcoming unittest which needs to create
> another KSycoca instance.
> 
> 
> Diffs
> -
> 
>   src/sycoca/ksycoca.cpp c5465a828da615e87220304e3f8b160d471edbc7 
>   src/sycoca/ksycoca_p.h a86148a20fccf038e2d46f5d2d6d460be94910da 
> 
> Diff: https://git.reviewboard.kde.org/r/125278/diff/
> 
> 
> Testing
> ---
> 
> ctest.
> 
> Albert: don't despair, this is the one-before-last patch ;)
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125279: KSycoca: change DB filename to include language and sha1 of the dirs it's built from.

[Aleix Pol Gonzalez]

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


+1

- Aleix Pol Gonzalez


On Sept. 17, 2015, 1:30 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125279/
> ---
> 
> (Updated Sept. 17, 2015, 1:30 a.m.)
> 
> 
> Review request for KDE Frameworks and Albert Astals Cid.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> This will prevent sycoca-rebuild ping-pong if two apps with different settings
> would share the same file (and keep finding that it's wrong for them),
> and it fixes Albert's bug that "LANG=de kcmshell5 --list" doesn't show German
> translations for the strings coming from desktop files.
> 
> 
> Diffs
> -
> 
>   autotests/ksycocatest.cpp 7c2d91e056726540b8a3a5c679d9b2a93f023c50 
>   docs/kbuildsycoca5/man-kbuildsycoca5.8.docbook 
> 3419e42cb29c6699bc17e6dd46bbc523139c59eb 
>   src/sycoca/kbuildsycoca.cpp 3685211e9da68f14516ec2b3d9a7e6b4f559b6f3 
>   src/sycoca/ksycoca.h c561dfd1550fd28e73144af6d3b2fa9008b17f59 
>   src/sycoca/ksycoca.cpp c5465a828da615e87220304e3f8b160d471edbc7 
> 
> Diff: https://git.reviewboard.kde.org/r/125279/diff/
> 
> 
> Testing
> ---
> 
> unittests, after adjusting ksycocatest which was checking for the old 
> behavior (same file).
> 
> Albert: there we are, finally ;) Your bug should be fixed (I don't have 
> translations installed to test it, though). Thanks for all the reviews!
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125270: KBuildSycoca: use qCWarning rather than fprintf(stderr, ...) or qWarning

[Aleix Pol Gonzalez]

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

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Sept. 17, 2015, 8:36 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125270/
> ---
> 
> (Updated Sept. 17, 2015, 8:36 a.m.)
> 
> 
> Review request for KDE Frameworks and Albert Astals Cid.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> REVIEW: 125270
> 
> 
> Diffs
> -
> 
>   src/sycoca/kbuildservicefactory.cpp 
> bebb76947672d0e2c100c7149c642406c30355f1 
>   src/sycoca/kbuildservicegroupfactory.cpp 
> 6ac0fc491f42bb117954ba8488b2847c8954e6b3 
>   src/sycoca/kbuildservicetypefactory.cpp 
> 2610c8d99beb12e02070365b078deb4455a6472a 
>   src/sycoca/kbuildsycoca.cpp 3685211e9da68f14516ec2b3d9a7e6b4f559b6f3 
>   src/sycoca/ksycoca.cpp c5465a828da615e87220304e3f8b160d471edbc7 
>   src/sycoca/ksycocadict.cpp e90969c9acb7ef94ae8da2098400c7207e0aeb67 
>   src/sycoca/ksycocafactory.cpp bc55fb54f0836f492ad04519ca1c9f93e9af3880 
>   src/sycoca/vfolder_menu.cpp b17ef2d371a2804ebe6029a0b7abfbebe4298d50 
> 
> Diff: https://git.reviewboard.kde.org/r/125270/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125289: Use Q_SLOTS/Q_SIGNALS instead of slots/signals in all headers from include dir

[Aleix Pol Gonzalez]

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


+1 this was a problem in KDE Connect as well.

- Aleix Pol Gonzalez


On Sept. 17, 2015, 2:26 p.m., Jan Grulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125289/
> ---
> 
> (Updated Sept. 17, 2015, 2:26 p.m.)
> 
> 
> Review request for KDE Frameworks and Ivan Romanov.
> 
> 
> Repository: qca
> 
> 
> Description
> ---
> 
> This is used also in other header files from include folder. I'm trying to 
> use QCA in plasma-nm, but I have to use - add_definitions(-DQT_NO_KEYWORDS) 
> in CMake due to NetworkManager and thus plasma-nm will not compile with QCA 
> if signals/slots are used.
> 
> 
> Diffs
> -
> 
>   include/QtCrypto/qca_safetimer.h 68dde5c 
> 
> Diff: https://git.reviewboard.kde.org/r/125289/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>

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

Re: Review Request 125289: Use Q_SLOTS/Q_SIGNALS instead of slots/signals in all headers from include dir

[Aleix Pol Gonzalez]

> On Sept. 17, 2015, 4:13 p.m., Andrea Scarpino wrote:
> > Inviala!
> 
> Andrea Scarpino wrote:
> And in Choqok too, thanks!
> 
> (please do not consider my "Ship It" :-)

Oops... xD


- Aleix


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


On Sept. 17, 2015, 4:14 p.m., Jan Grulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125289/
> ---
> 
> (Updated Sept. 17, 2015, 4:14 p.m.)
> 
> 
> Review request for KDE Frameworks and Ivan Romanov.
> 
> 
> Repository: qca
> 
> 
> Description
> ---
> 
> This is used also in other header files from include folder. I'm trying to 
> use QCA in plasma-nm, but I have to use - add_definitions(-DQT_NO_KEYWORDS) 
> in CMake due to NetworkManager and thus plasma-nm will not compile with QCA 
> if signals/slots are used.
> 
> 
> Diffs
> -
> 
>   include/QtCrypto/qca_safetimer.h 68dde5c 
> 
> Diff: https://git.reviewboard.kde.org/r/125289/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jan Grulich
> 
>

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

Review Request 125300: Fix lock in KProtocolManager

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks.


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


Repository: kio


Description
---

We had a non-recursive mutex and we kept requesting it. Eventually it locked 
because we were locking the mutex when already locked.

An alternative fix could be to rearrange the code and release it sooner, but it 
doesn't seem worth it. (although I had to remove 2 asserts and I feel uneasy 
about it).


Diffs
-

  src/core/kprotocolmanager.cpp 294ebdf 

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


Testing
---

Tests pass, cannot reproduce the attached bug anymore.


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 125332: Use the more advanced version of uriencode.cmake from kdoctools which supports Windows.

[Aleix Pol Gonzalez]

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



cmake/FindPerlModules.cmake (line 2)
<https://git.reviewboard.kde.org/r/125332/#comment59239>

How is that new file related?



cmake/uriencode.cmake (line 2)
<https://git.reviewboard.kde.org/r/125332/#comment59237>

should be kdelibs4support_encode_uri



src/CMakeLists.txt (line 645)
<https://git.reviewboard.kde.org/r/125332/#comment59238>

That doesn't seem to make a lot of sense...


- Aleix Pol Gonzalez


On Sept. 21, 2015, 2:41 p.m., Patrick von Reth wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125332/
> ---
> 
> (Updated Sept. 21, 2015, 2:41 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kdelibs4support
> 
> 
> Description
> ---
> 
> Use the more advanced version of uriencode.cmake from kdoctools which 
> supports Windows.
> 
> 
> Diffs
> -
> 
>   cmake/FindPerlModules.cmake PRE-CREATION 
>   cmake/uriencode.cmake fde06d3ccf14212122bbee583b44158a2411fbcf 
>   src/CMakeLists.txt 14cc57413b827286ba1d29ccf31e308b05c52413 
> 
> Diff: https://git.reviewboard.kde.org/r/125332/diff/
> 
> 
> Testing
> ---
> 
> Windows
> 
> 
> Thanks,
> 
> Patrick von Reth
> 
>

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

Re: Review Request 125316: KProtocolManager: fix deadlock when using EnvVarProxy.

[Aleix Pol Gonzalez]

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

Ship it!


Looks good to me, seems to fix the issue. I'll discard my patch.

- Aleix Pol Gonzalez


On Sept. 21, 2015, 3:23 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125316/
> ---
> 
> (Updated Sept. 21, 2015, 3:23 p.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Bugs: 350890
> https://bugs.kde.org/show_bug.cgi?id=350890
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> BUG: 350890
> Change-Id: I8471c01d68054c7dccb9109524f0433465219aff
> FIXED-IN: 5.15
> 
> 
> Diffs
> -
> 
>   autotests/kprotocolinfotest.cpp 1efd91094db17a7f2c5a008f35de8f6a86a91f12 
>   src/core/kprotocolmanager.cpp 294ebdfd9de53aeb560211fbd0df17a590650a27 
> 
> Diff: https://git.reviewboard.kde.org/r/125316/diff/
> 
> 
> Testing
> ---
> 
> unittest
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125300: Fix lock in KProtocolManager

[Aleix Pol Gonzalez]

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

(Updated Sept. 21, 2015, 4:34 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


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


Repository: kio


Description
---

We had a non-recursive mutex and we kept requesting it. Eventually it locked 
because we were locking the mutex when already locked.

An alternative fix could be to rearrange the code and release it sooner, but it 
doesn't seem worth it. (although I had to remove 2 asserts and I feel uneasy 
about it).


Diffs
-

  src/core/kprotocolmanager.cpp 294ebdf 

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


Testing
---

Tests pass, cannot reproduce the attached bug anymore.


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 125021: Fix uninitialized value valgrind error

[Aleix Pol Gonzalez]

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

(Updated Sept. 21, 2015, 2:36 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 8bd2d9010da0269c7a633d1c19bad70aae10a34f by Aleix Pol to 
branch master.


Repository: kdeclarative


Description
---

==25580== Conditional jump or move depends on uninitialised value(s)
==25580==at 0x5C94636: 
KQuickAddons::QuickViewSharedEngine::setResizeMode(KQuickAddons::QuickViewSharedEngine::ResizeMode)
 (quickviewsharedengine.cpp:212)
==25580==by 0x506F6BF: 
PlasmaQuick::ContainmentView::ContainmentView(Plasma::Corona*, QWindow*) 
(containmentview.cpp:222)
==25580==by 0x47E7FD: PanelView::PanelView(ShellCorona*, QScreen*, 
QWindow*) (panelview.cpp:64)
==25580==by 0x4971AE: ShellCorona::createWaitingPanels() 
(shellcorona.cpp:998)
==25580==by 0x4AC7E7: QtPrivate::FunctorCall, 
QtPrivate::List<>, void, void (ShellCorona::*)()>::call(void 
(ShellCorona::*)(), ShellCorona*, void**) (qobjectdefs_impl.h:501)
==25580==by 0x4AB119: void QtPrivate::FunctionPointer::call, void>(void (ShellCorona::*)(), 
ShellCorona*, void**) (qobjectdefs_impl.h:520)
==25580==by 0x4A7F1E: QtPrivate::QSlotObject, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, 
void**, bool*) (qobject_impl.h:143)
==25580==by 0xB194686: call (qobject_impl.h:124)
==25580==by 0xB194686: QMetaObject::activate(QObject*, int, int, void**) 
(qobject.cpp:3708)
==25580==by 0xB1A1647: QTimer::timerEvent(QTimerEvent*) (qtimer.cpp:247)
==25580==by 0xB19559A: QObject::event(QEvent*) (qobject.cpp:1271)
==25580==by 0xA17DB7B: QApplicationPrivate::notify_helper(QObject*, 
QEvent*) (qapplication.cpp:3717)
==25580==by 0xA182B45: QApplication::notify(QObject*, QEvent*) 
(qapplication.cpp:3498)
==25580==


Diffs
-

  src/quickaddons/quickviewsharedengine.cpp 0c8edc6 

Diff: https://git.reviewboard.kde.org/r/125021/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 124563: Make it possible to import/export shortcut schemes symmetrically

[Aleix Pol Gonzalez]

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

(Updated Sept. 21, 2015, 2:40 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Alexander Dymo.


Changes
---

Submitted with commit cffb5232246e3059d048c55a1a1eab0d0b2a9b17 by Aleix Pol to 
branch master.


Repository: kxmlgui


Description
---

I wanted to make sure that it's possible to provide shortcut files easily. We 
already had an export function but not an import one.

* Instead of exporting all of the shortcuts into a directory full of xml files, 
use KShortcutsEditor::exportConfiguration, that uses KConfig files.
* Add an action to import using KShortcutsEditor::importConfiguration.


Diffs
-

  src/kshortcutschemeseditor.cpp e9402bc 
  src/kshortcutsdialog.h 0e2bdaf 
  src/kshortcutsdialog.cpp 2c16b16 
  src/kshortcutsdialog_p.h fb5007b 

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


Testing
---

Tests still pass.
I played around importing and exporting Kate shortcuts.


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 125334: Small improvements in ColumnProxyModel

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks.


Repository: kdeclarative


Description
---

* Add a check for a property set so that it doesn't re-compute if the new value 
is the same as the former.
* Implement ::setData to the source model.


Diffs
-

  autotests/columnproxymodeltest.h 46b8d40 
  autotests/columnproxymodeltest.cpp 8844651 
  src/qmlcontrols/kquickcontrolsaddons/columnproxymodel.h 83d65d1 
  src/qmlcontrols/kquickcontrolsaddons/columnproxymodel.cpp b4f7973 

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


Testing
---

added test, does what I need it to do on muon.


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 125337: Mark kconfig-compiler as non-gui tool

[Aleix Pol Gonzalez]

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

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Sept. 21, 2015, 8:42 p.m., Harald Fernengel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125337/
> ---
> 
> (Updated Sept. 21, 2015, 8:42 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Makes sure that it doens't create an app bundle on Mac OS X
> 
> 
> Diffs
> -
> 
>   src/kconfig_compiler/CMakeLists.txt 
> 368a4d84d032878ff56234e0c540463670a94ec7 
> 
> Diff: https://git.reviewboard.kde.org/r/125337/diff/
> 
> 
> Testing
> ---
> 
> Builds with homebrew on Mac OS X
> 
> 
> Thanks,
> 
> Harald Fernengel
> 
>

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

Re: Review Request 125338: Fix out of bounds memory access in KNTLM::getAuth

[Aleix Pol Gonzalez]

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



src/kntlm/kntlm.cpp (line 246)
<https://git.reviewboard.kde.org/r/125338/#comment59289>

Isn't this already checked in line 230? Or is `sizeof(Challenge)!=32`?


- Aleix Pol Gonzalez


On Sept. 21, 2015, 9:56 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125338/
> ---
> 
> (Updated Sept. 21, 2015, 9:56 p.m.)
> 
> 
> Review request for KDE Frameworks and Dawit Alemayehu.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Make sure the size of the byte array we just dumped into the struct is big 
> enough before calculating the targetInfo, otherwise we're accessing memory 
> that doesn't belong to us
> 
> Fix out of bounds memory access 
> https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/102/testReport/junit/%28root%29/TestSuite/kioslave_httpauthenticationtest/
> 
> Also remove a cast to quint32 that is not necessary since the member is 
> already a quint32
> 
> 
> Diffs
> -
> 
>   src/kntlm/kntlm.cpp 77526dd 
> 
> Diff: https://git.reviewboard.kde.org/r/125338/diff/
> 
> 
> Testing
> ---
> 
> Valgrind doesn't complain anymore.
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

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

Re: Review Request 125479: KFileWidget: parent argument should default to 0 like in all widgets.

[Aleix Pol Gonzalez]

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

Ship it!


- Aleix Pol Gonzalez


On Oct. 2, 2015, 9:56 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125479/
> ---
> 
> (Updated Oct. 2, 2015, 9:56 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is the only ctor, so as far as I can see, this is SC.
> 
> Change-Id: Iab7c725a400802f7f14ddde9b4a8f95822be66f2
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kfilewidget.h 5b59ea9a57f38da3cb0e94da509d3943ff5a672e 
> 
> Diff: https://git.reviewboard.kde.org/r/125479/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125480: KFileWidget: provide a "bigger than minimum" sizeHint.

[Aleix Pol Gonzalez]

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


+1 lgtm

- Aleix Pol Gonzalez


On Oct. 2, 2015, 9:58 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125480/
> ---
> 
> (Updated Oct. 2, 2015, 9:58 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This code is taken from kdelibs4 KFileDialog.
> 
> The too small size hint can be tested by the new kfilewidgettest_gui.cpp.
> I originally hit this when inserting a picture shape into calligra stage,
> the "shape options dialog" is mostly just a KFileWidget, and it was tiny
> every time.
> 
> Change-Id: I7618af127619f0fe0204310d28b6b5ac07bfc5b8
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kfilewidget.h 5b59ea9a57f38da3cb0e94da509d3943ff5a672e 
>   src/filewidgets/kfilewidget.cpp d66040974d244c8dd54649615c9aae05eb3356c8 
> 
> Diff: https://git.reviewboard.kde.org/r/125480/diff/
> 
> 
> Testing
> ---
> 
> Tested with kfilewidgettest_gui
> 
> I actually expected the file dialog to be a little bigger.
> 
> Also I wonder why the config saving doesn't seem to work? IIRC this widget 
> was supposed to remember its own size.
> 
> Also it seems kfilewidgettest_gui doesn't exit when closing the 
> KFileWidget... weird.
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125338: Fix out of bounds memory access in KNTLM::getAuth

[Aleix Pol Gonzalez]

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


+1 Makes sense to me.

- Aleix Pol Gonzalez


On Sept. 21, 2015, 9:56 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125338/
> ---
> 
> (Updated Sept. 21, 2015, 9:56 p.m.)
> 
> 
> Review request for KDE Frameworks and Dawit Alemayehu.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Make sure the size of the byte array we just dumped into the struct is big 
> enough before calculating the targetInfo, otherwise we're accessing memory 
> that doesn't belong to us
> 
> Fix out of bounds memory access 
> https://build.kde.org/job/kio%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/102/testReport/junit/%28root%29/TestSuite/kioslave_httpauthenticationtest/
> 
> Also remove a cast to quint32 that is not necessary since the member is 
> already a quint32
> 
> 
> Diffs
> -
> 
>   src/kntlm/kntlm.cpp 77526dd 
> 
> Diff: https://git.reviewboard.kde.org/r/125338/diff/
> 
> 
> Testing
> ---
> 
> Valgrind doesn't complain anymore.
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

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

Re: Review Request 125511: Add missing documentation for argument to LAUNCHER_EXEC_NEW

[Aleix Pol Gonzalez]

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

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Oct. 3, 2015, 8:25 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125511/
> ---
> 
> (Updated Oct. 3, 2015, 8:25 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> Code already exists was just missing the docs for this argument
> 
> 
> Diffs
> -
> 
>   src/klauncher_cmds.h 35e0d9f352d459b724452758ac3d564d73cd4472 
> 
> Diff: https://git.reviewboard.kde.org/r/125511/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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

Re: Review Request 125334: Small improvements in ColumnProxyModel

[Aleix Pol Gonzalez]

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

(Updated Oct. 4, 2015, 4:27 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit c0b8ee043e8fbf1edd7b4e3223dedb00960f2e0a by Aleix Pol to 
branch master.


Repository: kdeclarative


Description
---

* Add a check for a property set so that it doesn't re-compute if the new value 
is the same as the former.
* Implement ::setData to the source model.


Diffs
-

  autotests/columnproxymodeltest.h 46b8d40 
  autotests/columnproxymodeltest.cpp 8844651 
  src/qmlcontrols/kquickcontrolsaddons/columnproxymodel.h 83d65d1 
  src/qmlcontrols/kquickcontrolsaddons/columnproxymodel.cpp b4f7973 

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


Testing
---

added test, does what I need it to do on muon.


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 125535: Minor optimizations

[Aleix Pol Gonzalez]

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



src/k7zip.cpp (line 2567)
<https://git.reviewboard.kde.org/r/125535/#comment59521>

.fill() and remove the loop



src/k7zip.cpp (line 2574)
<https://git.reviewboard.kde.org/r/125535/#comment59522>

.fill



src/k7zip.cpp (line 2580)
<https://git.reviewboard.kde.org/r/125535/#comment59523>

    .fill


- Aleix Pol Gonzalez


On Oct. 6, 2015, 12:34 a.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125535/
> ---
> 
> (Updated Oct. 6, 2015, 12:34 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: karchive
> 
> 
> Description
> ---
> 
> A few reserve calls
> A missing &
> QLatin1String -> QStringLiteral
> 
> 
> Diffs
> -
> 
>   src/kzip.cpp 6053a3b 
>   src/kfilterdev.cpp 85fa940 
>   src/k7zip.cpp 64c6b52 
>   src/ktar.cpp 49a6791 
>   src/karchive.cpp a1cd6a8 
> 
> Diff: https://git.reviewboard.kde.org/r/125535/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

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

Re: Review Request 125538: [kio-trash] Add file system freespace retrieval to trash kioslave

[Aleix Pol Gonzalez]

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



src/ioslaves/trash/kio_trash.cpp (line 617)
<https://git.reviewboard.kde.org/r/125538/#comment59537>

Maybe use qCDebug? So it can be disabled and so.


- Aleix Pol Gonzalez


On Oct. 6, 2015, 9:40 a.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125538/
> ---
> 
> (Updated Oct. 6, 2015, 9:40 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Gives the user info about the trash occupancy when opening the trash
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/kio_trash.h e6160c4 
>   src/ioslaves/trash/kio_trash.cpp 7ca8ad0 
>   src/ioslaves/trash/trashimpl.h 9881d8e 
>   src/ioslaves/trash/trashimpl.cpp 5a2832a 
> 
> Diff: https://git.reviewboard.kde.org/r/125538/diff/
> 
> 
> Testing
> ---
> 
> Works fine (with and without maximum size limit)
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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

Re: Review Request 125538: [kio-trash] Add file system freespace retrieval to trash kioslave

[Aleix Pol Gonzalez]

> On Oct. 6, 2015, 10:27 a.m., Aleix Pol Gonzalez wrote:
> > src/ioslaves/trash/kio_trash.cpp, line 617
> > <https://git.reviewboard.kde.org/r/125538/diff/1/?file=410023#file410023line617>
> >
> > Maybe use qCDebug? So it can be disabled and so.
> 
> Emmanuel Pescosta wrote:
> Trash doesn't use categorized logging atm so I decided to use qDebug.
> 
> IMHO categorized logging should be added in a separate patch instead.

Ok, looking forward to it. :)


- Aleix


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


On Oct. 6, 2015, 9:40 a.m., Emmanuel Pescosta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125538/
> ---
> 
> (Updated Oct. 6, 2015, 9:40 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Gives the user info about the trash occupancy when opening the trash
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/kio_trash.h e6160c4 
>   src/ioslaves/trash/kio_trash.cpp 7ca8ad0 
>   src/ioslaves/trash/trashimpl.h 9881d8e 
>   src/ioslaves/trash/trashimpl.cpp 5a2832a 
> 
> Diff: https://git.reviewboard.kde.org/r/125538/diff/
> 
> 
> Testing
> ---
> 
> Works fine (with and without maximum size limit)
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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

Re: Review Request 125535: Minor optimizations

[Aleix Pol Gonzalez]

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


+1 LGTM.

- Aleix Pol Gonzalez


On Oct. 6, 2015, 2:44 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125535/
> ---
> 
> (Updated Oct. 6, 2015, 2:44 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: karchive
> 
> 
> Description
> ---
> 
> A few reserve calls
> A missing &
> QLatin1String -> QStringLiteral
> 
> 
> Diffs
> -
> 
>   src/k7zip.cpp 64c6b52 
>   src/karchive.cpp a1cd6a8 
>   src/kfilterdev.cpp 85fa940 
>   src/ktar.cpp 49a6791 
>   src/kzip.cpp 6053a3b 
> 
> Diff: https://git.reviewboard.kde.org/r/125535/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

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

Re: Review Request 125332: Use the more advanced version of uriencode.cmake from kdoctools which supports Windows.

[Aleix Pol Gonzalez]

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


+1

- Aleix Pol Gonzalez


On Oct. 8, 2015, 2:30 p.m., Hannah von Reth wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125332/
> ---
> 
> (Updated Oct. 8, 2015, 2:30 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kdelibs4support
> 
> 
> Description
> ---
> 
> Use the more advanced version of uriencode.cmake from kdoctools which 
> supports Windows.
> 
> 
> Diffs
> -
> 
>   cmake/uriencode.cmake fde06d3ccf14212122bbee583b44158a2411fbcf 
>   src/CMakeLists.txt 14cc57413b827286ba1d29ccf31e308b05c52413 
> 
> Diff: https://git.reviewboard.kde.org/r/125332/diff/
> 
> 
> Testing
> ---
> 
> Windows
> 
> 
> Thanks,
> 
> Hannah von Reth
> 
>

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

Re: Review Request 125559: Fix usage with cmake < 3.0.0

[Aleix Pol Gonzalez]

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

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Oct. 8, 2015, 7:33 p.m., Antonio Larrosa Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125559/
> ---
> 
> (Updated Oct. 8, 2015, 7:33 p.m.)
> 
> 
> Review request for KDE Frameworks and Alex Merry.
> 
> 
> Repository: kdesignerplugin
> 
> 
> Description
> ---
> 
> Qt5UiPlugin [requires cmake 
> 3.0.0](http://code.qt.io/cgit/qt/qtbase.git/commit/mkspecs/features/data/cmake/Qt5BasicConfig.cmake.in?id=884679a7cc74b85d6c565316520304a9c73b5f04)
>  so don't try to use it if we're
> running with cmake < 3.0.0
> 
> 
> Diffs
> -
> 
>   KF5DesignerPluginMacros.cmake 0dfa9cdfc3f05d477b30dbee67585bf407a32613 
> 
> Diff: https://git.reviewboard.kde.org/r/125559/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Antonio Larrosa Jimenez
> 
>

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

Re: Review Request 125480: KFileWidget: provide a "bigger than minimum" sizeHint.

[Aleix Pol Gonzalez]

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

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Oct. 2, 2015, 9:58 a.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125480/
> ---
> 
> (Updated Oct. 2, 2015, 9:58 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This code is taken from kdelibs4 KFileDialog.
> 
> The too small size hint can be tested by the new kfilewidgettest_gui.cpp.
> I originally hit this when inserting a picture shape into calligra stage,
> the "shape options dialog" is mostly just a KFileWidget, and it was tiny
> every time.
> 
> Change-Id: I7618af127619f0fe0204310d28b6b5ac07bfc5b8
> 
> 
> Diffs
> -
> 
>   src/filewidgets/kfilewidget.h 5b59ea9a57f38da3cb0e94da509d3943ff5a672e 
>   src/filewidgets/kfilewidget.cpp d66040974d244c8dd54649615c9aae05eb3356c8 
> 
> Diff: https://git.reviewboard.kde.org/r/125480/diff/
> 
> 
> Testing
> ---
> 
> Tested with kfilewidgettest_gui
> 
> I actually expected the file dialog to be a little bigger.
> 
> Also I wonder why the config saving doesn't seem to work? IIRC this widget 
> was supposed to remember its own size.
> 
> Also it seems kfilewidgettest_gui doesn't exit when closing the 
> KFileWidget... weird.
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 125619: Refactor KNewPasswordDialog class

[Aleix Pol Gonzalez]

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



File Attachment: knewpassworddialog4.png - knewpassworddialog4.png
<https://git.reviewboard.kde.org//r/125619/#fcomment486>

Shouldn't it have a red background when empty as well?



File Attachment: knewpassworddialog4.png - knewpassworddialog4.png
<https://git.reviewboard.kde.org//r/125619/#fcomment487>

Maybe the error message can go here?


- Aleix Pol Gonzalez


On Oct. 13, 2015, 3:30 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125619/
> ---
> 
> (Updated Oct. 13, 2015, 3:30 p.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Feck and David Faure.
> 
> 
> Repository: kwidgetsaddons
> 
> 
> Description
> ---
> 
> Follow-up of the KNewPasswordWidget review.
> By introducing a KNewPasswordWidget in the .ui file, we can:
> 
> - remove the code that has been moved to KNewPasswordWidget
> - enable the KNewPasswordWidget additional features
> 
> The only feature that is currently not available is the ability to remove the 
> strength bar. Do we want to allow developers to show a KNewPasswordDialog 
> without a strenght bar?
> 
> 
> Diffs
> -
> 
>   src/knewpassworddialog.h 1ac7b2151f1e88681a15ce21465449cedb871f1e 
>   src/knewpassworddialog.cpp bd7459acf3c72bc6dc0711da6086213d5111d5c3 
>   src/knewpassworddialog.ui 55a1f62cf4ba6d6c87b2c47742ba3bcd531ebfe8 
> 
> Diff: https://git.reviewboard.kde.org/r/125619/diff/
> 
> 
> Testing
> ---
> 
> Trying to insert a password which is too short or too weak or empty, works as 
> expected.
> 
> 
> File Attachments
> 
> 
> knewpassworddialog1.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/13/62bf21f6-1dcf-46c3-8b1d-146500b5f629__knewpassworddialog1.png
> knewpassworddialog2.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/13/b2e2666f-ce19-4df3-b6ba-25c13fc370ae__knewpassworddialog2.png
> knewpassworddialog3.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/13/b9536088-0cc4-470b-a211-6db4ca79aa2f__knewpassworddialog3.png
> knewpassworddialog4.png
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/13/4f826716-9ed2-4ecb-8cbe-4fb50abf1086__knewpassworddialog4.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

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

Review Request 125628: KTempDir: Make sense out of s_umask initialization

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks.


Repository: kdelibs4support


Description
---

Since my Qt rebuild from today, I started having crashes when starting some Qt 
processes (namely kcminit and another I can't remember). Both KCrash backtraces 
pointed to this code.

I don't know why it didn't crash before, but the code doesn't make much sense 
to me. This patch tailors kStoreUmask to what it is requested to do. I guess 
this code was refactored and the return is an ancient, unfortunate leftover.


Diffs
-

  src/kdecore/ktempdir.cpp 1240ac7 

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


Testing
---

Tests still pass, my system starts reliably.


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 125628: KTempDir: Make sense out of s_umask initialization

[Aleix Pol Gonzalez]

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

(Updated Oct. 15, 2015, 1:33 a.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


Repository: kdelibs4support


Description
---

Since my Qt rebuild from today, I started having crashes when starting some Qt 
processes (namely kcminit and another I can't remember). Both KCrash backtraces 
pointed to this code.

I don't know why it didn't crash before, but the code doesn't make much sense 
to me. This patch tailors kStoreUmask to what it is requested to do. I guess 
this code was refactored and the return is an ancient, unfortunate leftover.


Diffs
-

  src/kdecore/ktempdir.cpp 1240ac7 

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


Testing
---

Tests still pass, my system starts reliably.


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 125647: Fix KArchiveTest::testTarCopyTo on NTFS

[Aleix Pol Gonzalez]

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


Maybe such code should go in QFileInfo::permissions, no?

- Aleix Pol Gonzalez


On Oct. 15, 2015, 4:29 p.m., Eric Lemanissier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125647/
> ---
> 
> (Updated Oct. 15, 2015, 4:29 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: karchive
> 
> 
> Description
> ---
> 
> KArchiveTest::testTarCopyTo fails on NTFS, because qt_ntfs_permission_lookup 
> variable is not updated before checking file permissions 
> (http://doc.qt.io/qt-5/qfileinfo.html#ntfs-permissions)
> 
> 
> Diffs
> -
> 
>   autotests/karchivetest.cpp b1d 
> 
> Diff: https://git.reviewboard.kde.org/r/125647/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran test successfully
> 
> 
> Thanks,
> 
> Eric Lemanissier
> 
>

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

Re: Review Request 125658: Do not treat the context menu button as modifier

[Aleix Pol Gonzalez]

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

Ship it!


Makes sense to me.

- Aleix Pol Gonzalez


On Oct. 18, 2015, 4:45 p.m., Thomas Lübking wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125658/
> ---
> 
> (Updated Oct. 18, 2015, 4:45 p.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Feck and Hans Chen.
> 
> 
> Bugs: 165542
> https://bugs.kde.org/show_bug.cgi?id=165542
> 
> 
> Repository: kxmlgui
> 
> 
> Description
> ---
> 
> It's not and not treated as such by Qt either.
> 
> In addition suck context events while recording
> (so it doesn't activate, rather cosmetic)
> 
> BUG: 165542
> FIXED-IN: 5.16
> CHANGELOG: Allow to bind the contextmenu key (lower right) to shortcuts
> 
> 
> Diffs
> -
> 
>   src/kkeysequencewidget.cpp 2385f20 
> 
> Diff: https://git.reviewboard.kde.org/r/125658/diff/
> 
> 
> Testing
> ---
> 
> yupp.
> 
> 
> Thanks,
> 
> Thomas Lübking
> 
>

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

Re: Review Request 125164: Fix misbehavior when canceling a job

[Aleix Pol Gonzalez]

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


bump?

- Aleix Pol Gonzalez


On Sept. 11, 2015, 3:24 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125164/
> ---
> 
> (Updated Sept. 11, 2015, 3:24 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Catch that the job has been cancelled and emit the job as finished.
> 
> Still, I'm not too sure it's the correct fix.
> 
> 
> Diffs
> -
> 
>   autotests/jobtest.h ef8c3e1 
>   autotests/jobtest.cpp c24bcea 
>   src/core/transferjob.cpp 0c38070 
> 
> Diff: https://git.reviewboard.kde.org/r/125164/diff/
> 
> 
> Testing
> ---
> 
> Added a test and made it 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 125725: Make KCrash optional for kservice

[Aleix Pol Gonzalez]

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



src/kbuildsycoca/kbuildsycoca_main.cpp (line 121)
<https://git.reviewboard.kde.org/r/125725/#comment59874>

Maybe it would make sense to handle it using signal() directly here, rather 
than just ifdef'ing KCrash?


- Aleix Pol Gonzalez


On Oct. 20, 2015, 3:41 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125725/
> ---
> 
> (Updated Oct. 20, 2015, 3:41 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> kservice depends on KCrash only for kbuildsycoca.
> make this optional and link only against it, if around. Move check to 
> kbuildsyscoca file.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4c0f269 
>   src/kbuildsycoca/CMakeLists.txt 19bdc84 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
> 
> Diff: https://git.reviewboard.kde.org/r/125725/diff/
> 
> 
> Testing
> ---
> 
> Seems to compile fine without it.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Re: Review Request 125725: Make KCrash optional for kservice

[Aleix Pol Gonzalez]

> On Oct. 20, 2015, 3:52 p.m., Aleix Pol Gonzalez wrote:
> > src/kbuildsycoca/kbuildsycoca_main.cpp, line 122
> > <https://git.reviewboard.kde.org/r/125725/diff/1/?file=411919#file411919line122>
> >
> > Maybe it would make sense to handle it using signal() directly here, 
> > rather than just ifdef'ing KCrash?
> 
> Christoph Cullmann wrote:
> Perhaps, but then I would have to reimplement more or less what 
> setEmergencySaveFunction does, or? I am not sure if that makes sense, if you 
> want crash recovery, you can have KCrash.
> 
> Christoph Cullmann wrote:
> On the other side: Ok, that was anyway not that useful, given 
> kglobalaccel + kinit use KCrash, too.
> Perhaps I should better take a look at KCrash to make it less verbose, 
> e.g. not warn about missing drkonqui which won't be there on mac or win in 
> the normal case.

Sure.

Or maybe some of the code can become part of KCoreAddons?


- Aleix


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


On Oct. 20, 2015, 4 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125725/
> ---
> 
> (Updated Oct. 20, 2015, 4 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> kservice depends on KCrash only for kbuildsycoca.
> make this optional and link only against it, if around. Move check to 
> kbuildsyscoca file.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4c0f269 
>   src/kbuildsycoca/CMakeLists.txt 19bdc84 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
> 
> Diff: https://git.reviewboard.kde.org/r/125725/diff/
> 
> 
> Testing
> ---
> 
> Seems to compile fine without it.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Review Request 125750: Reduce some allocations

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks and Sebastian Kügler.


Repository: kcoreaddons


Description
---

I know this could be done so much better and there's a comment saying it's 
transitional (heh), but this happens to be called a ton of times on every 
plasma boot through [1].
Actually, why is it transitional? Are we supposed to move from metadata.desktop 
to metadata.json? Or is it because we should all be using `kpackagetool5 
--generate-index`?

[1] KPluginMetaData::loadFromDesktopFile(QString const&, QStringList const&)


Diffs
-

  src/lib/plugin/desktopfileparser.cpp 7a4556a 

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


Testing
---

Tests pass, plasma still boots.


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 125750: Reduce some allocations

[Aleix Pol Gonzalez]

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

(Updated Oct. 22, 2015, 4:05 a.m.)


Review request for KDE Frameworks and Sebastian Kügler.


Repository: kcoreaddons


Description
---

I know this could be done so much better and there's a comment saying it's 
transitional (heh), but this happens to be called a ton of times on every 
plasma boot through [1].
Actually, why is it transitional? Are we supposed to move from metadata.desktop 
to metadata.json? Or is it because we should all be using `kpackagetool5 
--generate-index`?

[1] KPluginMetaData::loadFromDesktopFile(QString const&, QStringList const&)


Diffs (updated)
-

  src/lib/plugin/desktopfileparser.cpp 7a4556a 

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


Testing
---

Tests pass, plasma still boots.


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 125750: Reduce some allocations

[Aleix Pol Gonzalez]

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

(Updated Oct. 22, 2015, 1:20 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Sebastian Kügler.


Changes
---

Submitted with commit 32e48f1911103537645c77d50ea3ddc192b0befe by Aleix Pol to 
branch master.


Repository: kcoreaddons


Description
---

I know this could be done so much better and there's a comment saying it's 
transitional (heh), but this happens to be called a ton of times on every 
plasma boot through [1].
Actually, why is it transitional? Are we supposed to move from metadata.desktop 
to metadata.json? Or is it because we should all be using `kpackagetool5 
--generate-index`?

[1] KPluginMetaData::loadFromDesktopFile(QString const&, QStringList const&)


Diffs
-

  src/lib/plugin/desktopfileparser.cpp 7a4556a 

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


Testing
---

Tests pass, plasma still boots.


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 125164: Fix misbehavior when canceling a job

[Aleix Pol Gonzalez]

> On Oct. 22, 2015, 9:39 a.m., David Faure wrote:
> > autotests/jobtest.cpp, line 309
> > <https://git.reviewboard.kde.org/r/125164/diff/1/?file=402693#file402693line309>
> >
> > What is the purpose of changing the currently-being-uploaded data 
> > buffer? Ah, from the fix I can see the purpose is to make it emit 
> > readyRead. Can you add a comment here like  "// make it emit readyRead..." 
> > if that's indeed the idea?
> > 
> > (I assume this is to emulate something more like a socket?)

Correct, we originally spot this when testing KDE Connect transfers.


> On Oct. 22, 2015, 9:39 a.m., David Faure wrote:
> > src/core/transferjob.cpp, line 400
> > <https://git.reviewboard.kde.org/r/125164/diff/1/?file=402694#file402694line400>
> >
> > Why slotFinished? Wouldn't just return be enough?
> > kill() is supposed to take care of emitting finished already, and you 
> > don't want to emit it twice.

slotFinished is never called, if I remove the slotFinished call the test never 
ends.


- Aleix


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


On Sept. 11, 2015, 3:24 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125164/
> ---
> 
> (Updated Sept. 11, 2015, 3:24 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Catch that the job has been cancelled and emit the job as finished.
> 
> Still, I'm not too sure it's the correct fix.
> 
> 
> Diffs
> -
> 
>   autotests/jobtest.h ef8c3e1 
>   autotests/jobtest.cpp c24bcea 
>   src/core/transferjob.cpp 0c38070 
> 
> Diff: https://git.reviewboard.kde.org/r/125164/diff/
> 
> 
> Testing
> ---
> 
> Added a test and made it 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 125164: Fix misbehavior when canceling a job

[Aleix Pol Gonzalez]

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

(Updated Oct. 23, 2015, 2:19 a.m.)


Review request for KDE Frameworks.


Changes
---

Simplified test.


Repository: kio


Description
---

Catch that the job has been cancelled and emit the job as finished.

Still, I'm not too sure it's the correct fix.


Diffs (updated)
-

  autotests/jobtest.h ef8c3e1 
  autotests/jobtest.cpp c24bcea 
  src/core/transferjob.cpp 0c38070 

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


Testing
---

Added a test and made it 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 125164: Fix misbehavior when canceling a job

[Aleix Pol Gonzalez]

> On Oct. 22, 2015, 9:39 a.m., David Faure wrote:
> > src/core/transferjob.cpp, line 400
> > <https://git.reviewboard.kde.org/r/125164/diff/1/?file=402694#file402694line400>
> >
> > Why slotFinished? Wouldn't just return be enough?
> > kill() is supposed to take care of emitting finished already, and you 
> > don't want to emit it twice.
> 
> Aleix Pol Gonzalez wrote:
> slotFinished is never called, if I remove the slotFinished call the test 
> never ends.

I got deep to the issue. Here's the catch:
SchedulerPrivate::jobFinished did finish the job, but it didn't emit anything. 
slotFinished actually needed to be called after killing the job.

Then the modification in TransferJob isn't needed, which restores all of our 
karma and gives us an extra life and 10 coins. *happy*


- Aleix


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


On Oct. 23, 2015, 2:19 a.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125164/
> ---
> 
> (Updated Oct. 23, 2015, 2:19 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Catch that the job has been cancelled and emit the job as finished.
> 
> Still, I'm not too sure it's the correct fix.
> 
> 
> Diffs
> -
> 
>   autotests/jobtest.h ef8c3e1 
>   autotests/jobtest.cpp c24bcea 
>   src/core/transferjob.cpp 0c38070 
> 
> Diff: https://git.reviewboard.kde.org/r/125164/diff/
> 
> 
> Testing
> ---
> 
> Added a test and made it 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 125164: Finish killed KIO jobs

[Aleix Pol Gonzalez]

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

(Updated Oct. 23, 2015, 2:48 a.m.)


Review request for KDE Frameworks.


Changes
---

When killing a job, report it as finished.


Summary (updated)
-

Finish killed KIO jobs


Repository: kio


Description (updated)
---

Scheduler::jobFinished cleans up the job but doesn't report it as such.


Diffs (updated)
-

  src/core/simplejob.cpp 3380cbe 
  autotests/jobtest.cpp c24bcea 
  autotests/jobtest.h ef8c3e1 

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


Testing
---

Added a test and made it 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 125760: Allow local embedded themes, like Qt does a default search in :/icons

[Aleix Pol Gonzalez]

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


Looks good to me. Maybe some testing would be useful?

- Aleix Pol Gonzalez


On Oct. 23, 2015, 8:25 a.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125760/
> ---
> 
> (Updated Oct. 23, 2015, 8:25 a.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Feck and Boudewijn Rempt.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Allow icon themes to be embedded in applications and prefer the embedded 
> ones, if e.g. Krita or Kate ships an embedded theme, that shall be found and 
> used.
> Qt does already do so, by searching in :/icons.
> 
> 
> Diffs
> -
> 
>   src/kicontheme.cpp d0ab4b9 
> 
> Diff: https://git.reviewboard.kde.org/r/125760/diff/
> 
> 
> Testing
> ---
> 
> Themes still work, if you have the right theme in :/icons it is found, in 
> contrast to before this patch.
> Still a problem, that I need to takle in a different patch: How to enforce 
> the use of some theme, Qt is able to do so via setThemeName, KIconLoader and 
> Co. ignore that.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Re: Review Request 125766: Changed the manual command line parsing to use QCommandLineParser.

[Aleix Pol Gonzalez]

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

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Oct. 23, 2015, 4:36 p.m., Antonio Larrosa Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125766/
> ---
> 
> (Updated Oct. 23, 2015, 4:36 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kded
> 
> 
> Description
> ---
> 
> Changed the manual command line parsing to use QCommandLineParser instead of
> parsing it manually. Also, now kded5 --version and kded5 --help show the
> version information and help on options.
> 
> There's a new human readable text ("Check cache validity") that maybe
> should be inside i18n, but since kded doesn't user i18n at all and
> I wasn't sure it's desired to do so, I just left it untranslated.
> 
> Also, unset SESSION_MANAGER environment variable instead of setting it empty.
> This removes a warning from Qt when parsing the (wrong, empty)
> value of SESSION_MANAGER
> 
> 
> Diffs
> -
> 
>   src/kded.cpp 6929d7d3f24d3556f1b227d5a9bfb5b02b1c295e 
> 
> Diff: https://git.reviewboard.kde.org/r/125766/diff/
> 
> 
> Testing
> ---
> 
> I tested kded5 runs, and the --check, --version and --help options work as 
> expected.
> Also, now it doesn't show a warning every time it's run. I didn't really 
> check it's
> not doing any session management, but SESSION_MANAGER is only used in one 
> place in qtbase,
> at 
> https://github.com/qtproject/qtbase/blob/dev/src/plugins/platforms/xcb/qxcbsessionmanager.cpp#L350
> and reading that, I think unsetting the environment variable makes more sense.
> 
> 
> Thanks,
> 
> Antonio Larrosa Jimenez
> 
>

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

Re: Review Request 125766: Changed the manual command line parsing to use QCommandLineParser.

[Aleix Pol Gonzalez]

> On Oct. 23, 2015, 5:09 p.m., Aleix Pol Gonzalez wrote:
> > Ship It!

Eh sorry I pressed that by fault, I don't know how it got confirmed. My actual 
review follows.


- Aleix


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


On Oct. 23, 2015, 4:36 p.m., Antonio Larrosa Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125766/
> ---
> 
> (Updated Oct. 23, 2015, 4:36 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kded
> 
> 
> Description
> ---
> 
> Changed the manual command line parsing to use QCommandLineParser instead of
> parsing it manually. Also, now kded5 --version and kded5 --help show the
> version information and help on options.
> 
> There's a new human readable text ("Check cache validity") that maybe
> should be inside i18n, but since kded doesn't user i18n at all and
> I wasn't sure it's desired to do so, I just left it untranslated.
> 
> Also, unset SESSION_MANAGER environment variable instead of setting it empty.
> This removes a warning from Qt when parsing the (wrong, empty)
> value of SESSION_MANAGER
> 
> 
> Diffs
> -
> 
>   src/kded.cpp 6929d7d3f24d3556f1b227d5a9bfb5b02b1c295e 
> 
> Diff: https://git.reviewboard.kde.org/r/125766/diff/
> 
> 
> Testing
> ---
> 
> I tested kded5 runs, and the --check, --version and --help options work as 
> expected.
> Also, now it doesn't show a warning every time it's run. I didn't really 
> check it's
> not doing any session management, but SESSION_MANAGER is only used in one 
> place in qtbase,
> at 
> https://github.com/qtproject/qtbase/blob/dev/src/plugins/platforms/xcb/qxcbsessionmanager.cpp#L350
> and reading that, I think unsetting the environment variable makes more sense.
> 
> 
> Thanks,
> 
> Antonio Larrosa Jimenez
> 
>

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

Re: Review Request 125766: Changed the manual command line parsing to use QCommandLineParser.

[Aleix Pol Gonzalez]

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



src/kded.cpp (line 684)
<https://git.reviewboard.kde.org/r/125766/#comment59966>

Use qputenv, also then the strdup isn't necessary.



src/kded.cpp (line 686)
<https://git.reviewboard.kde.org/r/125766/#comment59967>

Maybe it should set up KAboutData as well?


- Aleix Pol Gonzalez


On Oct. 23, 2015, 4:36 p.m., Antonio Larrosa Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125766/
> ---
> 
> (Updated Oct. 23, 2015, 4:36 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kded
> 
> 
> Description
> ---
> 
> Changed the manual command line parsing to use QCommandLineParser instead of
> parsing it manually. Also, now kded5 --version and kded5 --help show the
> version information and help on options.
> 
> There's a new human readable text ("Check cache validity") that maybe
> should be inside i18n, but since kded doesn't user i18n at all and
> I wasn't sure it's desired to do so, I just left it untranslated.
> 
> Also, unset SESSION_MANAGER environment variable instead of setting it empty.
> This removes a warning from Qt when parsing the (wrong, empty)
> value of SESSION_MANAGER
> 
> 
> Diffs
> -
> 
>   src/kded.cpp 6929d7d3f24d3556f1b227d5a9bfb5b02b1c295e 
> 
> Diff: https://git.reviewboard.kde.org/r/125766/diff/
> 
> 
> Testing
> ---
> 
> I tested kded5 runs, and the --check, --version and --help options work as 
> expected.
> Also, now it doesn't show a warning every time it's run. I didn't really 
> check it's
> not doing any session management, but SESSION_MANAGER is only used in one 
> place in qtbase,
> at 
> https://github.com/qtproject/qtbase/blob/dev/src/plugins/platforms/xcb/qxcbsessionmanager.cpp#L350
> and reading that, I think unsetting the environment variable makes more sense.
> 
> 
> Thanks,
> 
> Antonio Larrosa Jimenez
> 
>

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

Re: Review Request 125760: Allow local embedded themes, like Qt does a default search in :/icons

[Aleix Pol Gonzalez]

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


+1 looks good to me.

- Aleix Pol Gonzalez


On Oct. 23, 2015, 7:28 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125760/
> ---
> 
> (Updated Oct. 23, 2015, 7:28 p.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Feck and Boudewijn Rempt.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> ---
> 
> Allow icon themes to be embedded in applications and prefer the embedded 
> ones, if e.g. Krita or Kate ships an embedded theme, that shall be found and 
> used.
> Qt does already do so, by searching in :/icons.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 707f39b 
>   autotests/kiconloader_resourcethemetest.cpp PRE-CREATION 
>   autotests/resources.qrc 5385977 
>   src/kicontheme.cpp d0ab4b9 
> 
> Diff: https://git.reviewboard.kde.org/r/125760/diff/
> 
> 
> Testing
> ---
> 
> Themes still work, if you have the right theme in :/icons it is found, in 
> contrast to before this patch.
> Still a problem, that I need to takle in a different patch: How to enforce 
> the use of some theme, Qt is able to do so via setThemeName, KIconLoader and 
> Co. ignore that.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Review Request 125809: Fix logic (and warnings) in ModelContextMenu.qml

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

It was checking that a variable existed, then it used it. Now it works like the 
rest of the properties.


Diffs
-

  src/declarativeimports/plasmacomponents/qml/ModelContextMenu.qml f8fddb7 

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


Testing
---

Booted again, tests still pass, no warnings.


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 125809: Fix logic (and warnings) in ModelContextMenu.qml

[Aleix Pol Gonzalez]

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

(Updated Oct. 26, 2015, 5:22 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 36288df5ac48901134ca98ff8131d87ad434e2a1 by Aleix Pol to 
branch master.


Repository: plasma-framework


Description
---

It was checking that a variable existed, then it used it. Now it works like the 
rest of the properties.


Diffs
-

  src/declarativeimports/plasmacomponents/qml/ModelContextMenu.qml f8fddb7 

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


Testing
---

Booted again, tests still pass, no warnings.


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 125815: RFC: Attempt to provide an alternative when khelpcenter is not available

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks, Christoph Cullmann and Luigi Toscano.


Repository: kguiaddons


Description
---

Tries to reach out to docs.kde.org when khelpcenter is not available.


Diffs
-

  src/util/urlhandler.cpp 5b46be2 

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


Testing
---

I tried it with a couple of applications. For the main documentation just works.

Needs figuring out for more complex cases, I'm unsure if applications are 
opening the documentation in specific pages. In fact, I couldn't find the 
documentation for docs.kde.org url scheme, and I just made up the `path` part, 
although it seems to work.


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 125164: Finish killed KIO jobs

[Aleix Pol Gonzalez]

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

(Updated Oct. 29, 2015, 12:35 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 22b4cc54e1ecdc4dce89dbe2ba81de4b9786754a by Aleix Pol to 
branch master.


Repository: kio


Description
---

Scheduler::jobFinished cleans up the job but doesn't report it as such.


Diffs
-

  src/core/simplejob.cpp 3380cbe 
  autotests/jobtest.cpp c24bcea 
  autotests/jobtest.h ef8c3e1 

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


Testing
---

Added a test and made it 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 125164: Finish killed KIO jobs

[Aleix Pol Gonzalez]

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


Reopened because now everyone has the crash I was complaining on the reviews 
there:
https://git.reviewboard.kde.org/r/125613/

Will have to look into how to fix properly, whatever that means.

- Aleix Pol Gonzalez


On Oct. 29, 2015, 1:35 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125164/
> ---
> 
> (Updated Oct. 29, 2015, 1:35 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Scheduler::jobFinished cleans up the job but doesn't report it as such.
> 
> 
> Diffs
> -
> 
>   src/core/simplejob.cpp 3380cbe 
>   autotests/jobtest.cpp c24bcea 
>   autotests/jobtest.h ef8c3e1 
> 
> Diff: https://git.reviewboard.kde.org/r/125164/diff/
> 
> 
> Testing
> ---
> 
> Added a test and made it 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 125877: Fix kdeplatformtheme_unittest after last commit

[Aleix Pol Gonzalez]

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



autotests/kdeplatformtheme_unittest.cpp (line 237)
<https://git.reviewboard.kde.org/r/125877/#comment60168>

What does `qApp->wheelScrollLines()` return now?


- Aleix Pol Gonzalez


On Oct. 29, 2015, 9:22 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125877/
> ---
> 
> (Updated Oct. 29, 2015, 9:22 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> see summary
> 
> 
> Diffs
> -
> 
>   autotests/kdeplatformtheme_unittest.cpp f660ffd 
> 
> Diff: https://git.reviewboard.kde.org/r/125877/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125877: Fix kdeplatformtheme_unittest after last commit

[Aleix Pol Gonzalez]

> On Oct. 29, 2015, 9:26 p.m., Aleix Pol Gonzalez wrote:
> > autotests/kdeplatformtheme_unittest.cpp, line 237
> > <https://git.reviewboard.kde.org/r/125877/diff/1/?file=413361#file413361line237>
> >
> > What does `qApp->wheelScrollLines()` return now?
> 
> David Rosca wrote:
> It returned the value I have set in mouse kcm, because the test 
> KdePlatformTheme is not installed to qApp (and WheelScrollLines is now set as 
> themeHint instead of directly calling qApp->setWheelScrollLines)

So the behavior on all applications that used qApp->wheelScrollLines() changed 
now?


- Aleix


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


On Oct. 29, 2015, 9:22 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125877/
> ---
> 
> (Updated Oct. 29, 2015, 9:22 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> see summary
> 
> 
> Diffs
> -
> 
>   autotests/kdeplatformtheme_unittest.cpp f660ffd 
> 
> Diff: https://git.reviewboard.kde.org/r/125877/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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

Re: Review Request 125164: Finish killed KIO jobs

[Aleix Pol Gonzalez]

1f0) at 
/home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:3717
#28 0x73aadd96 in QApplication::notify (this=0x7fffde60, 
receiver=0xb93530, e=0xeaf1f0) at 
/home/kde-devel/frameworks/qt5/qtbase/src/widgets/kernel/qapplication.cpp:3498
```


Repository: kio


Description
---

Scheduler::jobFinished cleans up the job but doesn't report it as such.


Diffs (updated)
-

  src/core/simplejob.cpp 3380cbe 
  autotests/jobtest.h 7641131 
  autotests/jobtest.cpp 7da92d5 

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


Testing
---

Added a test and made it 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 125904: Save proxy url with correct scheme.

[Aleix Pol Gonzalez]

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



src/kcms/kio/kproxydlg.cpp (line 106)
<https://git.reviewboard.kde.org/r/125904/#comment60243>

QString() instead of ""



src/kcms/kio/kproxydlg.cpp (line 207)
<https://git.reviewboard.kde.org/r/125904/#comment60245>

remove space between url and (



src/kcms/kio/kproxydlg.cpp (line 294)
<https://git.reviewboard.kde.org/r/125904/#comment60244>

does that even compile? it should be QStringLiteral("http"), same with the 
rest.


- Aleix Pol Gonzalez


On Oct. 31, 2015, 11:55 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125904/
> ---
> 
> (Updated Oct. 31, 2015, 11:55 p.m.)
> 
> 
> Review request for KDE Frameworks and Eike Hein.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently, if user type a ip address/domain name in socks 5 proxy field, the 
> saved proxy url in configuration file will start with http:// instead of 
> socks:// . Same applies to ftp proxy.
> 
> The reason is that KUrlFilter append http scheme to url without scheme. This 
> patch uses setDefaultUrlScheme to make sure the correct scheme is used.
> 
> Also make necessary change to make sure that loading url configuration will 
> hide url scheme correctly.
> 
> 
> Diffs
> -
> 
>   src/kcms/kio/kproxydlg.cpp c8fd0cd 
> 
> Diff: https://git.reviewboard.kde.org/r/125904/diff/
> 
> 
> Testing
> ---
> 
> Save socks proxy and ftp proxy will result in "ftp://"; and "socks://".
> 
> 
> 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 125915: Drop runtime dependency on QApplication

[Aleix Pol Gonzalez]

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



src/platforms/xcb/kwindowsystem.cpp (line 585)
<https://git.reviewboard.kde.org/r/125915/#comment60267>

Just compute once and use the same QRect instance all long this function.


- Aleix Pol Gonzalez


On Nov. 1, 2015, 11:34 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125915/
> ---
> 
> (Updated Nov. 1, 2015, 11:34 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> QDesktopWidget was used which is broken for applications using
> QGuiApplication.
> 
> QDesktopWidget is now just a thin wrapper over QScreen so we can use that 
> directly.
> 
> QApplication::activeWindow is ported to QGuiApplication::topLevelWindows
> and then itterating to see if one is active.
> 
> 
> Diffs
> -
> 
>   src/platforms/xcb/kwindowsystem.cpp 
> 9d287043c24894ca3c29c439c7939b139da055e8 
> 
> Diff: https://git.reviewboard.kde.org/r/125915/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

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

Review Request 125924: KJob::kill(Quiet) should also exit the event loop

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks.


Repository: kcoreaddons


Description
---

To that end, this patch unifies both duplicated codes so it keeps consistent.


Diffs
-

  src/lib/jobs/kjob.h cfb6909 
  src/lib/jobs/kjob.cpp ff5a171 

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


Testing
---

Tests still pass, addresses issue in KIO: 
https://git.reviewboard.kde.org/r/125164/


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 125924: KJob::kill(Quiet) should also exit the event loop

[Aleix Pol Gonzalez]

> On Nov. 3, 2015, 1:40 p.m., Milian Wolff wrote:
> > src/lib/jobs/kjob.h, line 644
> > <https://git.reviewboard.kde.org/r/125924/diff/1/?file=414449#file414449line644>
> >
> > move this into KJobPrivate, no?

I don't think I can. I tried and I wasn't able to call the QPrivateSignal from 
there.


- Aleix


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


On Nov. 2, 2015, 8:31 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125924/
> ---
> 
> (Updated Nov. 2, 2015, 8:31 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> To that end, this patch unifies both duplicated codes so it keeps consistent.
> 
> 
> Diffs
> -
> 
>   src/lib/jobs/kjob.h cfb6909 
>   src/lib/jobs/kjob.cpp ff5a171 
> 
> Diff: https://git.reviewboard.kde.org/r/125924/diff/
> 
> 
> Testing
> ---
> 
> Tests still pass, addresses issue in KIO: 
> https://git.reviewboard.kde.org/r/125164/
> 
> 
> 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 125164: Finish killed KIO jobs

[Aleix Pol Gonzalez]

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

(Updated Nov. 3, 2015, 11:44 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


Repository: kio


Description
---

Scheduler::jobFinished cleans up the job but doesn't report it as such.


Diffs
-

  src/core/simplejob.cpp 3380cbe 
  autotests/jobtest.h 7641131 
  autotests/jobtest.cpp 7da92d5 

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


Testing
---

Added a test and made it 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 125924: KJob::kill(Quiet) should also exit the event loop

[Aleix Pol Gonzalez]

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

(Updated Nov. 3, 2015, 10:44 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 17a1ff06f234de4e763494101254542ff49a8659 by Aleix Pol to 
branch master.


Repository: kcoreaddons


Description
---

To that end, this patch unifies both duplicated codes so it keeps consistent.


Diffs
-

  src/lib/jobs/kjob.h cfb6909 
  src/lib/jobs/kjob.cpp ff5a171 

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


Testing
---

Tests still pass, addresses issue in KIO: 
https://git.reviewboard.kde.org/r/125164/


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 125815: RFC: Attempt to provide an alternative when khelpcenter is not available

[Aleix Pol Gonzalez]

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


Bump? Anyone with docs.kde.org knowledge around?

- Aleix Pol Gonzalez


On Oct. 26, 2015, 7:04 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125815/
> ---
> 
> (Updated Oct. 26, 2015, 7:04 p.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Cullmann and Luigi Toscano.
> 
> 
> Repository: kguiaddons
> 
> 
> Description
> ---
> 
> Tries to reach out to docs.kde.org when khelpcenter is not available.
> 
> 
> Diffs
> -
> 
>   src/util/urlhandler.cpp 5b46be2 
> 
> Diff: https://git.reviewboard.kde.org/r/125815/diff/
> 
> 
> Testing
> ---
> 
> I tried it with a couple of applications. For the main documentation just 
> works.
> 
> Needs figuring out for more complex cases, I'm unsure if applications are 
> opening the documentation in specific pages. In fact, I couldn't find the 
> documentation for docs.kde.org url scheme, and I just made up the `path` 
> part, although it seems to work.
> 
> 
> 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 125941: Add KCompressionDevice tests to KArchive

[Aleix Pol Gonzalez]

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



autotests/kcompressiondevicetest.cpp (line 29)
<https://git.reviewboard.kde.org/r/125941/#comment60359>

Can't KArchive do that already?



autotests/kcompressiondevicetest.cpp (line 69)
<https://git.reviewboard.kde.org/r/125941/#comment60358>

Use QTemporaryDir.



autotests/kcompressiondevicetest.cpp (line 79)
<https://git.reviewboard.kde.org/r/125941/#comment60357>

You should compare to the file size, otherwise this all will have to be 
changed every time these files are modified.


Good first step!

- Aleix Pol Gonzalez


On Nov. 4, 2015, 3:08 a.m., Romário Rios wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125941/
> ---
> 
> (Updated Nov. 4, 2015, 3:08 a.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: karchive
> 
> 
> Description
> ---
> 
> I recently noticed that using KTar with KCompressionDevice and QNetworkReply 
> did not work, so I'm adding some tests to confirm that
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1b2e819 
>   autotests/kcompressiondevicetest.h PRE-CREATION 
>   autotests/kcompressiondevicetest.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125941/diff/
> 
> 
> Testing
> ---
> 
> The tests run and show that KCompressionDevice isn't working as expected
> 
> 
> Thanks,
> 
> Romário Rios
> 
>

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

Re: Review Request 125815: Provide an alternative when khelpcenter is not available

[Aleix Pol Gonzalez]

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

(Updated Nov. 4, 2015, 6:55 p.m.)


Review request for KDE Frameworks, Christoph Cullmann and Luigi Toscano.


Changes
---

The apache redirections do something similar to my patch 
https://quickgit.kde.org/?p=websites%2Fdocs-kde-org.git&a=blob&h=ae03e951c8d7cb8dda070e1a9592ea0a15ecc0f9&f=apache-docs.kde.org.conf&o=plain.

I suggest to have it merged, as it will always work better than the current 
state which is just show an error.


Summary (updated)
-

Provide an alternative when khelpcenter is not available


Repository: kguiaddons


Description
---

Tries to reach out to docs.kde.org when khelpcenter is not available.


Diffs
-

  src/util/urlhandler.cpp 5b46be2 

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


Testing
---

I tried it with a couple of applications. For the main documentation just works.

Needs figuring out for more complex cases, I'm unsure if applications are 
opening the documentation in specific pages. In fact, I couldn't find the 
documentation for docs.kde.org url scheme, and I just made up the `path` part, 
although it seems to work.


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 125960: Fix build of some projects using ecm_create_qm_loader()

[Aleix Pol Gonzalez]

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

Review request for Extra Cmake Modules and KDE Frameworks.


Repository: extra-cmake-modules


Description
---

Makes sure the variable is properly initialized.


Diffs
-

  modules/ECMPoQmTools.cmake 22258dc 

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


Testing
---

This error won't happen anymore:
https://build.kde.org/job/analitza%20master%20kf5-qt5/12/PLATFORM=Linux,compiler=gcc/console


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 125965: Add declarative plugin to KHolidays

[Aleix Pol Gonzalez]

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


I don't know enough about the module, so I don't know about the exact case, but 
usually it's good to develop such API outside then when it stabilizes merge to 
the framework, I'd say. Otherwise changes in the API will be very hard.

- Aleix Pol Gonzalez


On Nov. 5, 2015, 8:57 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125965/
> ---
> 
> (Updated Nov. 5, 2015, 8:57 p.m.)
> 
> 
> Review request for KDE Frameworks and John Layt.
> 
> 
> Repository: kholidays
> 
> 
> Description
> ---
> 
> For now it contains just the model of holiday regions
> which will be used in the Plasma calendar events
> configuration.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt e8b7970 
>   src/CMakeLists.txt c067b6c 
>   src/declarative/CMakeLists.txt PRE-CREATION 
>   src/declarative/holidayregionsmodel.h PRE-CREATION 
>   src/declarative/holidayregionsmodel.cpp PRE-CREATION 
>   src/declarative/kholidaysdeclarativeplugin.h PRE-CREATION 
>   src/declarative/kholidaysdeclarativeplugin.cpp PRE-CREATION 
>   src/declarative/qmldir PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125965/diff/
> 
> 
> Testing
> ---
> 
> Applet configuration contains proper list of available
> holiday regions.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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

Re: Review Request 125975: X11 build system cleanup

[Aleix Pol Gonzalez]

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

Ship it!


Ship It!

- Aleix Pol Gonzalez


On Nov. 6, 2015, 10:42 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125975/
> ---
> 
> (Updated Nov. 6, 2015, 10:42 a.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Marco Martin.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> Use newer cmake syntax for XCB dependencies
> 
> 
> [declarativeimports/core] Specify HAVE_XCB_COMPOSITE in config-x11.h
> 
> 
> [plasmaquick] Drop XCB::COMPOSITE and DAMAGE dependency
> 
> Not used
> 
> [plasmaquick] Don't link OpenGL explicitly
> 
> Not needed and anyway pulled in from Qt.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/CMakeLists.txt 
> 0265d5999dcec533b04f6b38ac2f345ea57f966d 
>   src/declarativeimports/core/config-x11.h.cmake 
> 89858d17de239cfc7eed1f40a8b828803de3299c 
>   src/declarativeimports/core/windowthumbnail.h 
> 576b053229a8cfc15b6fadd8d4e6ff07f937565d 
>   src/plasma/CMakeLists.txt 73f308cf5ac8371c1d259c335edcb069117c5c11 
>   src/plasmaquick/CMakeLists.txt 1ddc61123e2a061b42d14cbd1065a70458fdd2d7 
> 
> Diff: https://git.reviewboard.kde.org/r/125975/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

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

Re: Review Request 125965: Add declarative plugin to KHolidays

[Aleix Pol Gonzalez]

> On Nov. 6, 2015, 12:49 a.m., Aleix Pol Gonzalez wrote:
> > I don't know enough about the module, so I don't know about the exact case, 
> > but usually it's good to develop such API outside then when it stabilizes 
> > merge to the framework, I'd say. Otherwise changes in the API will be very 
> > hard.
> 
> Martin Klapetek wrote:
> Well it's a simple non-exported model, I don't see any API changes coming 
> to that.

ok, +1.


- Aleix


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


On Nov. 5, 2015, 8:57 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125965/
> ---
> 
> (Updated Nov. 5, 2015, 8:57 p.m.)
> 
> 
> Review request for KDE Frameworks and John Layt.
> 
> 
> Repository: kholidays
> 
> 
> Description
> ---
> 
> For now it contains just the model of holiday regions
> which will be used in the Plasma calendar events
> configuration.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt e8b7970 
>   src/CMakeLists.txt c067b6c 
>   src/declarative/CMakeLists.txt PRE-CREATION 
>   src/declarative/holidayregionsmodel.h PRE-CREATION 
>   src/declarative/holidayregionsmodel.cpp PRE-CREATION 
>   src/declarative/kholidaysdeclarativeplugin.h PRE-CREATION 
>   src/declarative/kholidaysdeclarativeplugin.cpp PRE-CREATION 
>   src/declarative/qmldir PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125965/diff/
> 
> 
> Testing
> ---
> 
> Applet configuration contains proper list of available
> holiday regions.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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

Re: Review Request 125815: Provide an alternative when khelpcenter is not available

[Aleix Pol Gonzalez]

> On Nov. 4, 2015, 1:43 a.m., Aleix Pol Gonzalez wrote:
> > Bump? Anyone with docs.kde.org knowledge around?
> 
> Ben Cooksley wrote:
> As long as the application name is being submitted in lower case, it 
> should work fine.
> 
> Examples: 
> https://docs.kde.org/index.php?branch=stable5&application=gwenview
> https://docs.kde.org/index.php?branch=stable5&application=konversation
> https://docs.kde.org/index.php?branch=stable5&application=dolphin
> 
> Luigi Toscano wrote:
> There are few exceptions for kioslaves and kcm modules:
> application=kioslave5/ftp for kioslaves (or kioslave/ftp for the kdelibs4 
> based version)
> application=kcontrol/powerdevil
> Not sure if they need to be handled there or those values are already 
> specified as applicationName()
> 
> For the record, all the source code is here:
> https://quickgit.kde.org/?p=websites%2Fdocs-kde-org.git&a=tree
> 
> Ralf Habacker wrote:
> What about language selection ? khelpcenter also covers to select the 
> language currently used in the KDE environment.

I introduced it in the committed version. It will need some love in the 
server-side though, since it needs to understand that "fr_FR" is "fr".
We can do the cutting before the _ because there's some exceptions, for example 
zh_CN.

Help there is welcome.


- Aleix


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


On Nov. 4, 2015, 6:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125815/
> ---
> 
> (Updated Nov. 4, 2015, 6:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Christoph Cullmann and Luigi Toscano.
> 
> 
> Repository: kguiaddons
> 
> 
> Description
> ---
> 
> Tries to reach out to docs.kde.org when khelpcenter is not available.
> 
> 
> Diffs
> -
> 
>   src/util/urlhandler.cpp 5b46be2 
> 
> Diff: https://git.reviewboard.kde.org/r/125815/diff/
> 
> 
> Testing
> ---
> 
> I tried it with a couple of applications. For the main documentation just 
> works.
> 
> Needs figuring out for more complex cases, I'm unsure if applications are 
> opening the documentation in specific pages. In fact, I couldn't find the 
> documentation for docs.kde.org url scheme, and I just made up the `path` 
> part, although it seems to work.
> 
> 
> 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 125815: Provide an alternative when khelpcenter is not available

[Aleix Pol Gonzalez]

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

(Updated Nov. 6, 2015, 4:48 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Christoph Cullmann and Luigi Toscano.


Changes
---

Submitted with commit c61b3a8acfd013e50219e62da68f300ab43acfaf by Aleix Pol to 
branch master.


Repository: kguiaddons


Description
---

Tries to reach out to docs.kde.org when khelpcenter is not available.


Diffs
-

  src/util/urlhandler.cpp 5b46be2 

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


Testing
---

I tried it with a couple of applications. For the main documentation just works.

Needs figuring out for more complex cases, I'm unsure if applications are 
opening the documentation in specific pages. In fact, I couldn't find the 
documentation for docs.kde.org url scheme, and I just made up the `path` part, 
although it seems to work.


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 126054: Missed doctool usage.

[Aleix Pol Gonzalez]

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

Ship it!


Ship It!

- Aleix Pol Gonzalez


On nov. 13, 2015, 5:01 p.m., Michael Palimaka wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126054/
> ---
> 
> (Updated nov. 13, 2015, 5:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Cullmann.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> ---
> 
> kdoctools_install fails if KF5DocTools is missing or disabled.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt e7ce4fe3e6e7537d675bb475734b19c97a2b4db5 
> 
> Diff: https://git.reviewboard.kde.org/r/126054/diff/
> 
> 
> Testing
> ---
> 
> Configures with and without KF5DocTools.
> 
> 
> Thanks,
> 
> Michael Palimaka
> 
>

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

Re: Review Request 126069: [OS X] better integration of the cross-platform wallet runtime

[Aleix Pol Gonzalez]

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



src/runtime/kwallet-query/src/CMakeLists.txt (line 20)
<https://git.reviewboard.kde.org/r/126069/#comment60584>

Why only for APPLE?


I wouldn't add if(APPLE) unless there's something very specific about the 
platform.

- Aleix Pol Gonzalez


On Nov. 14, 2015, 10:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126069/
> ---
> 
> (Updated Nov. 14, 2015, 10:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> Pending an overdue update to my OS X Keychain backend, this patch provides 
> better integration of the runtime:
> 
> - kwallet-query and kwalletd5 are built as regular executables, not app 
> bundles, making them easier to find via the shell and KService
> - kwalletd5 is modified to run as an agent, meaning it doesn't show up in the 
> Dock or App Switcher.
> 
> 
> Diffs
> -
> 
>   src/runtime/kwallet-query/src/CMakeLists.txt 9ab9d9d 
>   src/runtime/kwalletd/CMakeLists.txt ba9e7ba 
>   src/runtime/kwalletd/main.cpp dd959b6 
> 
> Diff: https://git.reviewboard.kde.org/r/126069/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.0, in the 5.15.0 frameworks release version.
> 
> 
> 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 126069: [OS X] better integration of the cross-platform wallet runtime

[Aleix Pol Gonzalez]

> On Nov. 15, 2015, 12:33 a.m., Aleix Pol Gonzalez wrote:
> > src/runtime/kwallet-query/src/CMakeLists.txt, line 20
> > <https://git.reviewboard.kde.org/r/126069/diff/1/?file=416871#file416871line20>
> >
> > Why only for APPLE?
> 
> René J.V. Bertin wrote:
> Mostly because I didn't want to pretend to know the side-effects this 
> could have on platforms other than Linux and OS X ...

That's not how it works. You don't improve kwallet on OS X, you improve it 
everywhere. That's why it's a framework.

Also given the nature of `ecm_mark_nongui_executable` I don't see a problem 
with having it on every platform. I can see how it can be useful on other 
platforms, in fact. We need to minimize platfrom-specific code if we want to 
have a viable cross-platform solution.


- Aleix


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


On Nov. 14, 2015, 10:29 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126069/
> ---
> 
> (Updated Nov. 14, 2015, 10:29 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> Pending an overdue update to my OS X Keychain backend, this patch provides 
> better integration of the runtime:
> 
> - kwallet-query and kwalletd5 are built as regular executables, not app 
> bundles, making them easier to find via the shell and KService
> - kwalletd5 is modified to run as an agent, meaning it doesn't show up in the 
> Dock or App Switcher.
> 
> 
> Diffs
> -
> 
>   src/runtime/kwallet-query/src/CMakeLists.txt 9ab9d9d 
>   src/runtime/kwalletd/CMakeLists.txt ba9e7ba 
>   src/runtime/kwalletd/main.cpp dd959b6 
> 
> Diff: https://git.reviewboard.kde.org/r/126069/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.0, in the 5.15.0 frameworks release version.
> 
> 
> 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 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Aleix Pol Gonzalez]

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



src/plugins/osx/macpoller.h (line 31)
<https://git.reviewboard.kde.org/r/126078/#comment60588>

You removed Carbon from cmake, no?

I'm not sure why you're keeping both really. If IOKit is the way to go, 
then do it.


In fact, I'm quite sure that as is it already is broken without the #define

- Aleix Pol Gonzalez


On Nov. 15, 2015, 11:58 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 15, 2015, 11:58 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor why this 
> doesn't happen on Linux.
> 
> Surely I'm missing something, but what?
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

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

Review Request 126084: Small KDeclarative cleanup

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks.


Repository: kdeclarative


Description
---

* Remove unread attribute.
* Remove unneeded include.
* Remove unneeded constructor.


Diffs
-

  src/kdeclarative/kdeclarative.cpp be795b2 
  src/kdeclarative/private/kdeclarative_p.h 5bb0241 
  src/kdeclarative/qmlobjectsharedengine.cpp 9a05e26 

Diff: https://git.reviewboard.kde.org/r/126084/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 126084: Small KDeclarative cleanup

[Aleix Pol Gonzalez]

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

(Updated Nov. 16, 2015, 12:26 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit c213aae48ce169a4f8cb7272335e9b0138d7206f by Aleix Pol to 
branch master.


Repository: kdeclarative


Description
---

* Remove unread attribute.
* Remove unneeded include.
* Remove unneeded constructor.


Diffs
-

  src/kdeclarative/kdeclarative.cpp be795b2 
  src/kdeclarative/private/kdeclarative_p.h 5bb0241 
  src/kdeclarative/qmlobjectsharedengine.cpp 9a05e26 

Diff: https://git.reviewboard.kde.org/r/126084/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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Repository: ki18n


Description
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs
-

  src/CMakeLists.txt 818595e 
  src/klocalizedcontext.h PRE-CREATION 
  src/klocalizedcontext.cpp PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126088: Port KDeclarative to use KI18n directly, rather than adding its own bindings

[Aleix Pol Gonzalez]

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

Review request for KDE Frameworks.


Repository: kdeclarative


Description
---

`i18n*()` bindings were moved upstream to ki18n so they can be used by a wider 
audience.


Diffs
-

  src/kdeclarative/CMakeLists.txt ce12546 
  src/kdeclarative/kdeclarative.cpp 820250d 
  src/kdeclarative/private/kdeclarative_p.h 0e48ca3 
  src/kdeclarative/private/rootcontext.cpp d88b566 
  src/kdeclarative/private/rootcontext_p.h 94df09f 

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


Testing
---

Tests still pass, apps still work, plasma still works as well.


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 126086: kauth-policy-gen should not be an app bundle on OS X

[Aleix Pol Gonzalez]

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



src/CMakeLists.txt (line 1)
<https://git.reviewboard.kde.org/r/126086/#comment60602>

Move it in the upper CMakeLists.txt file together with 
`include(ECMGenerateHeaders)`.


Otherwise looks good to me. +1

- Aleix Pol Gonzalez


On Nov. 16, 2015, 1:47 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126086/
> ---
> 
> (Updated Nov. 16, 2015, 1:47 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kauth
> 
> 
> Description
> ---
> 
> kauth-policy-gen should not be built as an app bundle on OS X.
> This can be achieved by using `ecm_mark_nongui-executable` and apparently 
> there are advantages and no contra-indications to doing that on all platforms.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1b6930d 
> 
> Diff: https://git.reviewboard.kde.org/r/126086/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

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

(Updated Nov. 16, 2015, 3:55 p.m.)


Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Changes
---

Address Milian's concerns.


Repository: ki18n


Description
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs (updated)
-

  src/klocalizedcontext.cpp PRE-CREATION 
  src/CMakeLists.txt 818595e 
  src/klocalizedcontext.h PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

> On Nov. 17, 2015, 10:58 a.m., Marco Martin wrote:
> > Here's the reason I don't like it:
> > In origin KDeclarative was intended to be a library that only depended from 
> > Qt stuff, to be at most tier 2 (and imports there were supposed to be 
> > qt-only as well) and frameworks applications using QML were supposed to do 
> > it trough KDeclarative..
> > This just highlights a problem happened afterwards, when no frameworks 
> > maintainer wanted any qml binding it the framework's repository, the 
> > kdeclarative repo ended up being pretty much a dumpster where all the 
> > binding of all the frameworks went, making it depend from pretty much 
> > everything.
> > This also means that whoever will need the binding of $framework, it will 
> > probably rather rewrite them, since using kdeclarative means pretty much 
> > depending from all of them.
> > So, fine for making the i18n context independent, but iff the whole 
> > situation gets resolved and each single import that depends from more than 
> > Qt stuff goes either in its framework repository or gets its own.
> 
> Martin Gräßlin wrote:
> I agree with Marco: we should try to split this up again. Somehow the 
> framework starts to remind me of kdelibs4 and in applications I maintain it 
> takes a lot of strength to buy into using it. (Why would I want KIO in KWin?)

Isn't it what this patch is about?

To me it's clear that KIconProvider could go to KIconThemes and 
KIOAccessManagerFactory could go to KIO.


- Aleix


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

> On Nov. 16, 2015, 3:08 p.m., Milian Wolff wrote:
> > src/klocalizedcontext.cpp, line 53
> > <https://git.reviewboard.kde.org/r/126087/diff/1/?file=417069#file417069line53>
> >
> > isNull or isEmpty? Don't we usually want to check against isEmpty and 
> > never against isNull? Isn't the latter even quasi-deprecated as it's 
> > internal state is somewhat undocumented and one must not depend on it? I 
> > think I remember something like that from the qt devel mailing list... 
> > Should ask Thiago again maybe?

QString::isNull is correct. Qml will give us a null string if the argument 
isn't there, AFAIK.


- Aleix


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

> On Nov. 17, 2015, 9:43 a.m., Chusslove Illich wrote:
> > Could you also document the usage in docs/programmers-guide.md (section 
> > #link_cat)? I'm not much into QML, so it would help me understand the 
> > implications of the usage.
> > 
> > It seems to me other three series of calls (xi18n*, ki18n*, kxi18n*) should 
> > be made available as well.

I don't think we want the `k*i18n*` version of the calls, as they return 
KLocalizedString and we probably don't need to deal with it from QML.

Regarding `xi18n*`, I see that we have counter-parts for every of the `i18n*` 
calls. Could you explain a bit what does it do differently? Is it to support 
the xml-like markup?


- Aleix


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

> On Nov. 17, 2015, 10:58 a.m., Marco Martin wrote:
> > Here's the reason I don't like it:
> > In origin KDeclarative was intended to be a library that only depended from 
> > Qt stuff, to be at most tier 2 (and imports there were supposed to be 
> > qt-only as well) and frameworks applications using QML were supposed to do 
> > it trough KDeclarative..
> > This just highlights a problem happened afterwards, when no frameworks 
> > maintainer wanted any qml binding it the framework's repository, the 
> > kdeclarative repo ended up being pretty much a dumpster where all the 
> > binding of all the frameworks went, making it depend from pretty much 
> > everything.
> > This also means that whoever will need the binding of $framework, it will 
> > probably rather rewrite them, since using kdeclarative means pretty much 
> > depending from all of them.
> > So, fine for making the i18n context independent, but iff the whole 
> > situation gets resolved and each single import that depends from more than 
> > Qt stuff goes either in its framework repository or gets its own.
> 
> Martin Gräßlin wrote:
> I agree with Marco: we should try to split this up again. Somehow the 
> framework starts to remind me of kdelibs4 and in applications I maintain it 
> takes a lot of strength to buy into using it. (Why would I want KIO in KWin?)
> 
> Aleix Pol Gonzalez wrote:
> Isn't it what this patch is about?
> 
> To me it's clear that KIconProvider could go to KIconThemes and 
> KIOAccessManagerFactory could go to KIO.
> 
> Marco Martin wrote:
> we could push that right at the start of the 5.18 cycle so that's done 
> well in advance
> 
> Marco Martin wrote:
> yes, and most of the linking luckily is private. the only thing that is 
> public and a bit out of tune unfortunately is configpropertymap
> 
> Marco Martin wrote:
> in general i'm in favor, i would like to try to make it an import instead 
> tough (the qobject inserted as context object if a custom one wasn't set 
> already and made a QML singleton as well, so that can get accessed by 
> KI18n.i18n("") and such).
> it should work if it can be injected somehow from KDeclarative c++

If we do it in a different way (e.g. a singleton), we'll have to get the code 
ported. That's not backwards-compatible.


- Aleix


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

> On Nov. 17, 2015, 9:43 a.m., Chusslove Illich wrote:
> > Could you also document the usage in docs/programmers-guide.md (section 
> > #link_cat)? I'm not much into QML, so it would help me understand the 
> > implications of the usage.
> > 
> > It seems to me other three series of calls (xi18n*, ki18n*, kxi18n*) should 
> > be made available as well.
> 
> Aleix Pol Gonzalez wrote:
> I don't think we want the `k*i18n*` version of the calls, as they return 
> KLocalizedString and we probably don't need to deal with it from QML.
> 
> Regarding `xi18n*`, I see that we have counter-parts for every of the 
> `i18n*` calls. Could you explain a bit what does it do differently? Is it to 
> support the xml-like markup?
> 
> Chusslove Illich wrote:
> KLocalizedString exposes the lower-level interface, and more possible 
> argument combinations, so in principle k*i18n* can be useful even in 
> one-liners ending with .toString(). Is there any particular reason not to 
> have it?
> 
> Yes, xi18n* are for ki18n's own XML markup. So if someone likes to use 
> xi18n* generally in the whole program, he should want to use it in QML files 
> as well.

I added xi18n*.

Regarding KLocalizedString, I would wait and assess how we want it to be used 
later on. It largely relies on type system overloads and I'm not sure how well 
Qt would be able to bind to it.


- Aleix


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


On Nov. 16, 2015, 3:55 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 16, 2015, 3:55 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

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

(Updated Nov. 18, 2015, 1:26 p.m.)


Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Changes
---

Updated with `ki18n*` calls.


Repository: ki18n


Description (updated)
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs (updated)
-

  docs/programmers-guide.md 13a5f9d 
  src/CMakeLists.txt 818595e 
  src/klocalizedcontext.h PRE-CREATION 
  src/klocalizedcontext.cpp PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

> On Nov. 17, 2015, 11:05 a.m., Marco Martin wrote:
> > a public symbol could even be spared (and in the process made possibleto 
> > use it from pure qml) by making an import instead that does the 
> > setContextObject() injection in its setupEngine (that would happen 
> > immediately after an import org.kde.i18n statement)
> > 
> > the problem tough would be that then could be hard maintaining 
> > retrocompatibility in kdeclarative (tough an explicit 
> > QQmlEngine::importPlugin *may* work)

What if the application already has a ContextObject?

In fact, then we wouldn't be able to port KDeclarative, AFAIU: 
https://git.reviewboard.kde.org/r/126088/


- Aleix


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


On Nov. 18, 2015, 1:26 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 18, 2015, 1:26 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Aleix Pol Gonzalez]

> On Nov. 18, 2015, 8:22 a.m., Martin Gräßlin wrote:
> > src/plugins/osx/macpoller.cpp, lines 222-227
> > 
> >
> > seriously? You care about idle timeouts below 5 msec? This is a 
> > framework to tell the application whether the user doesn't use input 
> > devices. I don't know how fast you type, but I'm relatively certain that it 
> > takes more than 5 msec to move my finger from one key to another. What 
> > exactly is that you want to detect here? An event after each key press, 
> > maybe even between a key press and a key release? Because that's probably 
> > already more about 5 msec.
> 
> René J.V. Bertin wrote:
> The timer is used only for the detection of idle timeouts (so *absence* 
> of user input), and the precision with which it fires determines the accuracy 
> with which those events can be timed. This is currently inaccessible anyway, 
> but I don't believe in doing things half-bakedly. One might question whether 
> it makes sense to allow the framework to work at high precision (I more or 
> less agree that remains to be seen). But I don't think it makes sense to do 
> that and then leave the QTimer in a mode where the requested precision cannot 
> be met.
> 
> Martin Gräßlin wrote:
> To get this quite clear: you care about a precision of below 5 msec, this 
> is a third of the time it takes at least till the next frame is rendered. If 
> you are lucky you get the result of a key press or mouse move represented on 
> screen after 16 msec, more likely it's more. And you care about a wrong timer 
> precision below 5 msec. Sorry that's ridiculous. Please don't include such 
> non-sense code. You make that code more difficult to all other developers to 
> maintain. This code has cost if it's included.

It's not a matter of doing things "right" or "wrong". It's a matter of 
priorities.

KIdleTime is a framework for figuring out whether the system is idle. I don't 
consider 5ms not using a system as it's being idling.


- Aleix


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


On Nov. 17, 2015, 10:13 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 17, 2015, 10:13 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/macpoller_helper.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor why this 
> doesn't happen on Linux.
> 
> Surely I'm missing something, but what?
> 
> 
> 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 125960: Fix build of some projects using ecm_create_qm_loader()

[Aleix Pol Gonzalez]

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

(Updated Nov. 18, 2015, 4:11 p.m.)


Status
--

This change has been discarded.


Review request for Extra Cmake Modules and KDE Frameworks.


Repository: extra-cmake-modules


Description
---

Makes sure the variable is properly initialized.


Diffs
-

  modules/ECMPoQmTools.cmake 22258dc 

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


Testing
---

This error won't happen anymore:
https://build.kde.org/job/analitza%20master%20kf5-qt5/12/PLATFORM=Linux,compiler=gcc/console


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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

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

(Updated Nov. 19, 2015, 5 p.m.)


Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Changes
---

Added a tiny test.

I'd say exporting this class should be ok. Introducing a module that can be 
imported from within QML would be nice as well. I can work on it next if you 
like.


Repository: ki18n


Description
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs (updated)
-

  autotests/CMakeLists.txt 1cf0f7a 
  autotests/ki18ndeclarativetest.cpp PRE-CREATION 
  autotests/test.qml PRE-CREATION 
  docs/programmers-guide.md 13a5f9d 
  src/CMakeLists.txt 818595e 
  src/klocalizedcontext.h PRE-CREATION 
  src/klocalizedcontext.cpp PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

> On Nov. 19, 2015, 5:21 p.m., Marco Martin wrote:
> > so let's go for that and then add the import with the singleton as well, if 
> > is okay for framework maintainer are fine with the qtqml dependency.. but 
> > they should be as the "everything in kdeclarative" situation has to be 
> > solved (maybe optional dep at build time?)

Are you sure you want to consider Qt5::Qml as an optional dependency? I don't 
have a problem with it, but it seems odd as a first step.

Personally I'd like to propose it as is and if there's a reason not to have it, 
then consider it.


- Aleix


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


On Nov. 19, 2015, 5 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 19, 2015, 5 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

> On Nov. 19, 2015, 5:21 p.m., Marco Martin wrote:
> > so let's go for that and then add the import with the singleton as well, if 
> > is okay for framework maintainer are fine with the qtqml dependency.. but 
> > they should be as the "everything in kdeclarative" situation has to be 
> > solved (maybe optional dep at build time?)
> 
> Aleix Pol Gonzalez wrote:
> Are you sure you want to consider Qt5::Qml as an optional dependency? I 
> don't have a problem with it, but it seems odd as a first step.
> 
> Personally I'd like to propose it as is and if there's a reason not to 
> have it, then consider it.

For completion, at the moment the Qml dependency is only on the test. The full 
Qml dependency we'd get it if/when we do the qml runtime plugin, which I'd 
introduce in a later patch.


- Aleix


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


On Nov. 19, 2015, 5 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 19, 2015, 5 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126113: make event filters static to decrease installed filters on qApp

[Aleix Pol Gonzalez]

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



src/plasma/svg.cpp (line 581)
<https://git.reviewboard.kde.org/r/126113/#comment60769>

It could be useful to use qGuiApp here, so that software that doesn't use 
QApplication can use it as well.


- Aleix Pol Gonzalez


On Nov. 20, 2015, 2:27 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126113/
> ---
> 
> (Updated Nov. 20, 2015, 2:27 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Bugs: 351923
> http://bugs.kde.org/show_bug.cgi?id=351923
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> since seems there are so many event filters installed on the QApplication 
> installed that may give performance issues, try to use separate, singleton 
> watchers for them to decrease the amount of eventfilters called
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/core/units.h fa2256e 
>   src/declarativeimports/core/units.cpp 4e2adae 
>   src/plasma/private/svg_p.h 597465e 
>   src/plasma/private/theme_p.h 5b8f71c 
>   src/plasma/private/theme_p.cpp f346343 
>   src/plasma/svg.cpp bcceaf7 
> 
> Diff: https://git.reviewboard.kde.org/r/126113/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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

Re: Review Request 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

> On Nov. 20, 2015, 2:33 p.m., Chusslove Illich wrote:
> > src/klocalizedcontext.cpp, line 87
> > <https://git.reviewboard.kde.org/r/126087/diff/4/?file=417454#file417454line87>
> >
> > How are numbers converted to strings, before being passed here? Because 
> > when it receives actual numeric types, KI18n will take care to represent 
> > them in locale-specific way. Also it will let translators (through 
> > "translation scripting" feature) operate on numbers, e.g. to handle 
> > multi-plural cases.
> > 
> > Is it possible to get back to actual numbers, or the originating data 
> > type is irrevocably lost? If not, at least we should document this mungling 
> > somewhere.

At the moment it's QML itself doing the conversion and now that you point it 
out I see how it could be a problem... I'll try if it's possible to use 
QVariant there instead of QString directly.


- Aleix


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


On Nov. 19, 2015, 5 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 19, 2015, 5 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> 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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

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

(Updated Nov. 20, 2015, 3:03 p.m.)


Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Changes
---

Change the API into QVariant so we can have more control on the types, instead 
of leaving up to QML to do the type conversion.

Maybe it would be better to use QJSValue here, so that we didn't need the 
QJSValue -> QVariant conversion. This would require having the QML dependency 
though (and a separate object as well, probably).


Repository: ki18n


Description
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs (updated)
-

  autotests/CMakeLists.txt 1cf0f7a 
  autotests/ki18ndeclarativetest.cpp PRE-CREATION 
  autotests/test.qml PRE-CREATION 
  docs/programmers-guide.md 13a5f9d 
  src/CMakeLists.txt 818595e 
  src/klocalizedcontext.h PRE-CREATION 
  src/klocalizedcontext.cpp PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126123: [Linux/Kubuntu] find udev when installing to a parallel prefix

[Aleix Pol Gonzalez]

> On Nov. 20, 2015, 1:44 p.m., Martin Gräßlin wrote:
> > The need for the change surprises me. I have been using a /opt setup for 
> > years and udev was always found. Can you show the error you hit?
> 
> René J.V. Bertin wrote:
> Have you been using a Linux distro like Ubuntu that has multi-arch, with 
> the actual libraries installed in `/[usr/]lib/x86_64-linux-gnu`? I guess it's 
> also not for nothing that most FindFoo scripts appear to use the 2-stage 
> approach where pkgconfig is queried first for hints about where to look in 
> addition to the default locations. BTW, I'm using cmake 3.3.1 . That's not 
> the host's default cmake which *may* have been patched to look in the 
> multi-arch locations, but in my experience it works better to use a cmake 
> from the target prefix (Ubuntu's Project Neon5 did the same, IIRC).
> 
> I don't have access to the Linux system right now, but the error was that 
> the library wasn't being found, IIRC it did find the headerfile. The error 
> wasn't more explicit than that, my initial reaction was to check if I had the 
> libudev-dev package installed.
> 
> René J.V. Bertin wrote:
> Actually, my memory was wrong, the error is more perverse. Or rather, 
> was, because I cannot seem to reproduce it now :-/
> 
> Somehow, the configure step completed without error, set `UDEV_LIBS` (to 
> `udev`, I think), but for whatever reason the library wasn't added to the 
> link command where it should be added. So I got a link error.
> 
> Of course I tried to rule out operator error before starting to hack the 
> FindUDev.cmake script, but I must have missed something, or something else 
> changed on my system since those few days ago. I seem to recall that I found 
> a libudev.so symlink in a generic (as opposed to a multi-arch) location, but 
> that one I can no longer find either.
> 
> So I can either discard this RR, or else it could serve as a reason 
> investigate whether some FindFoo scripts need `pkg_check_modules` to obtain 
> hints for `find_path` and `find_library`.

I'd suggest discarding it until we can properly reproduce.

Removing the build directory helps to reproduce this kind of issues, by the way.


- Aleix


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


On Nov. 20, 2015, 1:41 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126123/
> ---
> 
> (Updated Nov. 20, 2015, 1:41 p.m.)
> 
> 
> Review request for KDE Frameworks and Kubuntu.
> 
> 
> Repository: solid
> 
> 
> Description
> ---
> 
> Configuring KF5 Solid under KUbuntu 14.04 for installation in a parallel 
> prefix (/opt/local) it failed to find libudev which is installed "normally".
> 
> Modifying FindUDev.cmake to follow the same approach as other Find`*.cmake 
> files (use pkgconfig first, and the results as hints for `find_path` and 
> `find_library`) corrected that failure.
> 
> 
> Diffs
> -
> 
>   cmake/FindUDev.cmake 9d0f21d 
> 
> Diff: https://git.reviewboard.kde.org/r/126123/diff/
> 
> 
> Testing
> ---
> 
> On Kubuntu 14.04.3 with Qt 5.5.1 and all of Solid 5.16.0's KF5 dependencies 
> installed in /opt/local
> 
> 
> 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 126125: [OS X] make KDE's trash use the OS X trash

[Aleix Pol Gonzalez]

> On Nov. 20, 2015, 10:06 p.m., René J.V. Bertin wrote:
> > src/ioslaves/trash/trashimpl.cpp, line 947
> > 
> >
> > These were `kDebug()` statements; I guess they should become 
> > `qWarning()` now that `kDebug` is gone. I've left the qDebug() statements 
> > in because I noticed there are plenty of those in the code; shouldn't they 
> > be turned into `qCWarning()` or `qCDebug` to avoid polluting the terminal 
> > or (on OS X) the `system.log` ?

Yes, they should, eventually.


- Aleix


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


On Nov. 20, 2015, 10:09 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 20, 2015, 10:09 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *KDE on OS X does not handle the desktop session (no "Plasma") nor can it 
> rely on XDG to obtain the proper paths to use for something like the trash. 
> As a result, all applications that propose to move things they manage to the 
> wastebin (Dolphin, but also digiKam) will store those items in a place that 
> has no particular meaning on OS X, and that will thus tend to fill up.*
> 
> *OS X stores trash in one of several locations. Files trashed from the boot 
> volume (and/or the volume containing $HOME, I don't actually know that) end 
> up in ~/.Trash. Files deleted from other volumes end up in 
> /Volumes/volName/.Trashes/uid, where volName is the volume name (regardless 
> whether it's an external or a remote drive; only mounted NFS shares are 
> handled differently) and uid the numerical user id. Permissions on .Trashes 
> are the same as those expected by KDE.*
> 
> The patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> 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 126125: [OS X] make KDE's trash use the OS X trash

[Aleix Pol Gonzalez]

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



src/ioslaves/trash/trashimpl.cpp (line 129)
<https://git.reviewboard.kde.org/r/126125/#comment60780>

Infrastructure is 1 word


I'm wondering if ifdef'ing the shit out of the code is the best solution. Would 
it make sense to split it in a couple of files or get an `_apple.cpp` file? Or 
even get a completely different trash:/ kio implementation?

- Aleix Pol Gonzalez


On Nov. 20, 2015, 10:09 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126125/
> ---
> 
> (Updated Nov. 20, 2015, 10:09 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This is a "backport" of the [kde-runtime modification reviewed 
> here](https://git.reviewboard.kde.org/r/120573/) that makes the trash ioslave 
> use a subdirectory in the OS X trashcan.
> 
> From the original review:
> *KDE on OS X does not handle the desktop session (no "Plasma") nor can it 
> rely on XDG to obtain the proper paths to use for something like the trash. 
> As a result, all applications that propose to move things they manage to the 
> wastebin (Dolphin, but also digiKam) will store those items in a place that 
> has no particular meaning on OS X, and that will thus tend to fill up.*
> 
> *OS X stores trash in one of several locations. Files trashed from the boot 
> volume (and/or the volume containing $HOME, I don't actually know that) end 
> up in ~/.Trash. Files deleted from other volumes end up in 
> /Volumes/volName/.Trashes/uid, where volName is the volume name (regardless 
> whether it's an external or a remote drive; only mounted NFS shares are 
> handled differently) and uid the numerical user id. Permissions on .Trashes 
> are the same as those expected by KDE.*
> 
> The patch from KDE4 was straightforward to apply as the trash implementation 
> has only had what seem to be cosmetic changes and "Qt4 -> Qt5" adaptations.
> Therefore, I have ported my code "as is", so as not to change anything that 
> was discussed and negotiated during the rather lengthy previous review 
> process. The only place where I've had to introduce a small additional change 
> is in `trashForMountPoint`, where KF5 made `trashDir` a const. Appending the 
> KDE trash-subdir in the `trashDir` declaration might seem the appropriate 
> approach, but since there is no guarantee at that point that the subdirectory 
> actually (still) exists, the `QT_LSTAT` test could fail, and the the 
> appending has to be done after the check on the actual host trash directory.
> 
> I have also introduced a small change in `testtrash.cpp` allowing it to run 
> instead of failing because the 1st location returned `trashDirectories()` 
> doesn't match the one determined from QSP.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/trash/CMakeLists.txt 05161cd 
>   src/ioslaves/trash/kcmtrash.cpp 79c2ca7 
>   src/ioslaves/trash/tests/CMakeLists.txt 3e138f7 
>   src/ioslaves/trash/tests/testtrash.cpp 339aa19 
>   src/ioslaves/trash/trashimpl.h 9886011 
>   src/ioslaves/trash/trashimpl.cpp 26d9ea8 
> 
> Diff: https://git.reviewboard.kde.org/r/126125/diff/
> 
> 
> Testing
> ---
> 
> OS X 10.9 with Qt 5.5.1 and KF5rameworks 5.16.0 . The code builds, and the 
> unittests from testtrash succeed (with the Trash open in the Finder one sees 
> the `KDE.trash` directory show up while the test runs, and disappear 
> afterwards).
> 
> 
> 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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

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

(Updated Nov. 23, 2015, 12:54 p.m.)


Review request for KDE Frameworks, Chusslove Illich and Marco Martin.


Changes
---

Address milian's issues.


Repository: ki18n


Description
---

The only way to use `i18n*()` so far in QML is by depending on all 
KDeclarative. This patch moves the necessary bits there so it can be adopted by 
an application or framework just by offering the needed bits within KI18n.
This is done by offering an object with methods that map to the `i18n*()` C++ 
counter-parts.


Diffs (updated)
-

  autotests/CMakeLists.txt 1cf0f7a 
  autotests/ki18ndeclarativetest.cpp PRE-CREATION 
  autotests/test.qml PRE-CREATION 
  docs/programmers-guide.md 13a5f9d 
  src/CMakeLists.txt 818595e 
  src/klocalizedcontext.h PRE-CREATION 
  src/klocalizedcontext.cpp PRE-CREATION 

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


Testing
---

Ported KDeclarative, everything still seems to work.


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 126087: Move the i18n context from KDeclarative

[Aleix Pol Gonzalez]

> On Nov. 22, 2015, 7:51 p.m., Milian Wolff wrote:
> > src/klocalizedcontext.h, line 59
> > <https://git.reviewboard.kde.org/r/126087/diff/5/?file=417518#file417518line59>
> >
> > virtual

It already is, I'll mark it as override.


> On Nov. 22, 2015, 7:51 p.m., Milian Wolff wrote:
> > src/klocalizedcontext.h, line 132
> > <https://git.reviewboard.kde.org/r/126087/diff/5/?file=417518#file417518line132>
> >
> > are we sure we'll never add anything else? i.e. is pimpling a safer 
> > choice?

Good point.


- Aleix


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


On Nov. 20, 2015, 3:03 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Nov. 20, 2015, 3:03 p.m.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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