Re: Review Request 112984: Adjust API in KEmoticons framework: KEmoticonsProvider::save()
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112984/ --- (Updated Oct. 6, 2013, 8:56 p.m.) Review request for KDE Frameworks. Changes --- I address Aurélien's suggestions (I hope I understood them well...) Repository: kdelibs Description --- Adjust API in KEmoticons framework: KEmoticonsProvider::save() -To make KEmoticons API more consistent, deprecate KEmoticonsProvider::save() and prefer saveTheme() instead. -Adjust plugins. -Before the cleanup, KEmoticonsTheme was calling KEmoticonsProvider::save(), which was empty. Now it's a pure virtual function. Therefore, I deprecate it and advice subclassing KEmoticonsProvider. Diffs (updated) - staging/kemoticons/src/core/kemoticonsprovider.cpp d04c76e87b118f5d45717b3b2ccd9dea47dc2526 staging/kemoticons/src/core/kemoticonsprovider.h 85fc7efb8923d76476f0a16f70f8ebb15e451081 staging/kemoticons/src/core/kemoticonstheme.h b8b25f7607d3741dda78d6302c1ed81fbc9211a0 staging/kemoticons/src/core/kemoticonstheme.cpp ed3407cdd45c84d91f2d0057e108b760ff696ff2 staging/kemoticons/src/providers/adium/adium_emoticons.h 039a267679919c968bbe4d12c35abb90cf1bcc9b staging/kemoticons/src/providers/adium/adium_emoticons.cpp a3aaa0fdc0b3dcc862f98865d2e6419e716f4f17 staging/kemoticons/src/providers/kde/kde_emoticons.h 8cba6b194eee1543bf13f1177a0e4092f1a10943 staging/kemoticons/src/providers/kde/kde_emoticons.cpp 5b5114a14dd94a6ebcba8a6f7dd163f73048189a staging/kemoticons/src/providers/pidgin/pidgin_emoticons.h 70bafa3fe4ba25c9d6a69634024b2c157235e674 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp e9f89eecce3d6e6407113a84cb5200ff66564c19 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.h 0873a635a300e0ed8e38b1d59a535d6ec15b99cb staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 0dc92ed092d87a559fe7fa1a40e804843ab73d59 Diff: http://git.reviewboard.kde.org/r/112984/diff/ Testing --- It builds. It installs. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112985: Adjust API in KEmoticons framework: map and index methods
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112985/ --- (Updated Oct. 6, 2013, 9:02 p.m.) Review request for KDE Frameworks. Changes --- Add @since 5.0 in the new methods. Repository: kdelibs Description --- Adjust API in KEmoticons framework: map and index methods -Make map and index KEmoticons API slightly clearer and deprecate the methods that are confusing. -Use the new methods in the plugins. Diffs (updated) - staging/kemoticons/src/core/kemoticonsprovider.h 85fc7efb8923d76476f0a16f70f8ebb15e451081 staging/kemoticons/src/core/kemoticonsprovider.cpp d04c76e87b118f5d45717b3b2ccd9dea47dc2526 staging/kemoticons/src/providers/adium/adium_emoticons.cpp a3aaa0fdc0b3dcc862f98865d2e6419e716f4f17 staging/kemoticons/src/providers/kde/kde_emoticons.cpp 5b5114a14dd94a6ebcba8a6f7dd163f73048189a staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp e9f89eecce3d6e6407113a84cb5200ff66564c19 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 0dc92ed092d87a559fe7fa1a40e804843ab73d59 Diff: http://git.reviewboard.kde.org/r/112985/diff/ Testing --- It builds. It installs. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112984: Adjust API in KEmoticons framework: KEmoticonsProvider::save()
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112984/ --- (Updated Oct. 7, 2013, 10:08 p.m.) Review request for KDE Frameworks. Changes --- Document the save() => saveTheme() change in KDE5PORTING.html Repository: kdelibs Description --- Adjust API in KEmoticons framework: KEmoticonsProvider::save() -To make KEmoticons API more consistent, deprecate KEmoticonsProvider::save() and prefer saveTheme() instead. -Adjust plugins. -Before the cleanup, KEmoticonsTheme was calling KEmoticonsProvider::save(), which was empty. Now it's a pure virtual function. Therefore, I deprecate it and advice subclassing KEmoticonsProvider. Diffs (updated) - staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 0dc92ed092d87a559fe7fa1a40e804843ab73d59 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.h 0873a635a300e0ed8e38b1d59a535d6ec15b99cb staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp e9f89eecce3d6e6407113a84cb5200ff66564c19 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.h 70bafa3fe4ba25c9d6a69634024b2c157235e674 staging/kemoticons/src/providers/kde/kde_emoticons.cpp 5b5114a14dd94a6ebcba8a6f7dd163f73048189a staging/kemoticons/src/providers/kde/kde_emoticons.h 8cba6b194eee1543bf13f1177a0e4092f1a10943 staging/kemoticons/src/providers/adium/adium_emoticons.cpp a3aaa0fdc0b3dcc862f98865d2e6419e716f4f17 staging/kemoticons/src/providers/adium/adium_emoticons.h 039a267679919c968bbe4d12c35abb90cf1bcc9b staging/kemoticons/src/core/kemoticonstheme.cpp ed3407cdd45c84d91f2d0057e108b760ff696ff2 staging/kemoticons/src/core/kemoticonstheme.h b8b25f7607d3741dda78d6302c1ed81fbc9211a0 staging/kemoticons/src/core/kemoticonsprovider.cpp d04c76e87b118f5d45717b3b2ccd9dea47dc2526 staging/kemoticons/src/core/kemoticonsprovider.h 85fc7efb8923d76476f0a16f70f8ebb15e451081 KDE5PORTING.html 57ecf2e007e270b634cc8d16462b73e96553c92d Diff: http://git.reviewboard.kde.org/r/112984/diff/ Testing --- It builds. It installs. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112985: Adjust API in KEmoticons framework: map and index methods
> On Oct. 9, 2013, 5:05 p.m., Kevin Ottens wrote: > > I'm not sure what this patch is trying to achieve. It doesn't look much > > more clearer API to me than what we had before. :-) > > > > Could you share a bit more of the rationale? I guess I'm missing something. Ok, let's see. We have a method in KEmoticonsProvider that creates a map and another method that creates an index: /** * Returns a QHash that contains the emoticon path as keys and the text as values */ QHash emoticonsMap() const; /** * Returns a QHash that contains emoticons indexed by the first char */ QHash > emoticonsIndex() const; We have a method that clears the map: /** * Clears the emoticons map */ void clearEmoticonsMap(); And then we have the original methods that manage the map and the index. The problem I see with them is that addEmoticonsMap(), for example, doesn't actually do what it says. If you don't read the documentation, addEmoticonsMap() seems to add (somewhere) a new (or existing) emoticon map. Instead, it inserts a NEW ITEM in the emoticon map: /** * Inserts a new item in the emoticon map * */ void addEmoticonsMap(QString key, QStringList value); That's why I've changed it to addMapItem(). I could've used addEmoticonsMapItem(), but I thought it was too long and since there's only one map and there's only one index in KEmoticonsProvider, addMapItem() is clear enough (and clearer than clearEmoticonsMap()). Besides, it now points to the documentation of emoticonsMap(), which explains where does that map come from: /** * Inserts a new item in the emoticon map * @since 5.0 * @see emoticonsMap() */ void addMapItem(QString key, QStringList value); The same applies to the rest: Deprecated: /** * Removes an item from the emoticon map * */ void removeEmoticonsMap(QString key); New: /** * Removes an item from the emoticon map * @since 5.0 * @see emoticonsMap() */ void removeMapItem(QString key); -- Deprecated: /** * Adds an emoticon to the index * @param path path to the emoticon * @param emoList list of text associated with this emoticon * */ void addEmoticonIndex(const QString &path, const QStringList &emoList); New: /** * Adds an emoticon to the index * @param path path to the emoticon * @param emoList list of text associated with this emoticon * @since 5.0 * @see emoticonsIndex() */ void addIndexItem(const QString &path, const QStringList &emoList); --- Deprecated: /** * Removes an emoticon from the index * @param path path to the emoticon * @param emoList list of text associated with this emoticon */ void removeEmoticonIndex(const QString &path, const QStringList &emoList); New: /** * Removes an emoticon from the index * @param path path to the emoticon * @param emoList list of text associated with this emoticon * @since 5.0 * @see emoticonsIndex() */ void removeIndexItem(const QString &path, const QStringList &emoList); - When I began to work with KEmoticons I found much code that wasn't very good, so I want to try to make the API better, because is now or never :-) . Anyway, I understand it's a delicate issue. - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112985/#review41450 --- On Oct. 6, 2013, 9:02 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112985/ > --- > > (Updated Oct. 6, 2013, 9:02 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kdelibs > > > Description > --- > > Adjust API in KEmoticons framework: map and index methods > > -Make map and index KEmoticons API slightly clearer and deprecate > the methods that are confusing. > -Use the new methods in the plugins. > > > Diffs > - > > staging/kemoticons/src/core/kemoticonsprovider.h > 85fc7efb8923d76476f0a16f70f8ebb15e451081 > staging/kemoticons/src/core/kemoticonsprovider.cpp > d04c76e87b118f5d45717b3b2ccd9dea47dc2526 > staging/kemoticons/src/providers/adium/adium_emoticons.cpp > a3aaa0fdc0b3dcc862f98865d2e6419e716f4f17 > staging/kemoticons/src/providers/kde/kde_emoticons.cpp > 5b5114a14dd94a6ebcba8a6f7dd163f73048189a > staging/kemoticons/src/p
CMake fails configuring KF5
Hi! I'm getting this error from CMake while building KF5: -- Found KF5: /home/david/devel/kf5-development/share/ECM/find-modules/FindKF5.cmake (found suitable version "5.0.0", minimum required is "5.0.0") found components: CMake Compiler InstallDirs CMake Error at /home/david/devel/kf5-development/qt5/qtbase/lib/cmake/Qt5/Qt5Config.cmake:26 (find_package): Could not find a package configuration file provided by "Qt5X11Extras" with any of the following names: Qt5X11ExtrasConfig.cmake qt5x11extras-config.cmake Add the installation prefix of "Qt5X11Extras" to CMAKE_PREFIX_PATH or set "Qt5X11Extras_DIR" to a directory containing one of the above files. If "Qt5X11Extras" provides a separate development package or SDK, be sure it has been installed. Call Stack (most recent call first): tier1/kidletime/CMakeLists.txt:15 (find_package) -- Configuring incomplete, errors occurred! Anyone can tell me why could this be happening? Maybe is Qt5 not well built or is something KF5-only related? :-/ Thanks in advance David Gil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: CMake fails configuring KF5
Hi! 2013/10/17 Nicolás Alvarez > 2013/10/17 David Gil Oliva : > > Hi! > > > > I'm getting this error from CMake while building KF5: > > > > -- Found KF5: > > /home/david/devel/kf5-development/share/ECM/find-modules/FindKF5.cmake > > (found suitable version "5.0.0", minimum required is "5.0.0") found > > components: CMake Compiler InstallDirs > > CMake Error at > > > /home/david/devel/kf5-development/qt5/qtbase/lib/cmake/Qt5/Qt5Config.cmake:26 > > (find_package): > > Could not find a package configuration file provided by "Qt5X11Extras" > > with > > any of the following names: > > > > Qt5X11ExtrasConfig.cmake > > qt5x11extras-config.cmake > > > > Add the installation prefix of "Qt5X11Extras" to CMAKE_PREFIX_PATH or > set > > "Qt5X11Extras_DIR" to a directory containing one of the above files. > If > > "Qt5X11Extras" provides a separate development package or SDK, be sure > it > > has been installed. > > Call Stack (most recent call first): > > tier1/kidletime/CMakeLists.txt:15 (find_package) > > > > > > -- Configuring incomplete, errors occurred! > > > > Anyone can tell me why could this be happening? Maybe is Qt5 not well > built > > or is something KF5-only related? :-/ > > Did you only compile qtbase? To compile all of KF5, you need qtbase, > qtdeclarative, qtx11extras, qtsvg, and maybe one or two more I'm > forgetting. > > Oh, I see. I'm having a hard time compiling Qt5. It keeps giving me errors, but the last time seemed to compile fine. Obviously, it didn't :-) Cheers! David Gil > -- > Nicolás > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112984: Adjust API in KEmoticons framework: KEmoticonsProvider::save()
> On Oct. 11, 2013, 4:28 p.m., Kevin Ottens wrote: > > Ship It! > > Kevin Ottens wrote: > Any reason why this patch isn't pushed yet? Yes. There are some personal reasons and, besides, I've been trying for days to compile Qt5.2 with no luck till today. I think I'll have both patches pushed this week. - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112984/#review41566 --- On Oct. 7, 2013, 10:08 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112984/ > --- > > (Updated Oct. 7, 2013, 10:08 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kdelibs > > > Description > --- > > Adjust API in KEmoticons framework: KEmoticonsProvider::save() > > -To make KEmoticons API more consistent, deprecate KEmoticonsProvider::save() > and prefer saveTheme() instead. > -Adjust plugins. > -Before the cleanup, KEmoticonsTheme was calling KEmoticonsProvider::save(), > which was empty. Now it's a pure virtual function. Therefore, I deprecate > it and advice subclassing KEmoticonsProvider. > > > Diffs > - > > staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp > 0dc92ed092d87a559fe7fa1a40e804843ab73d59 > staging/kemoticons/src/providers/xmpp/xmpp_emoticons.h > 0873a635a300e0ed8e38b1d59a535d6ec15b99cb > staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp > e9f89eecce3d6e6407113a84cb5200ff66564c19 > staging/kemoticons/src/providers/pidgin/pidgin_emoticons.h > 70bafa3fe4ba25c9d6a69634024b2c157235e674 > staging/kemoticons/src/providers/kde/kde_emoticons.cpp > 5b5114a14dd94a6ebcba8a6f7dd163f73048189a > staging/kemoticons/src/providers/kde/kde_emoticons.h > 8cba6b194eee1543bf13f1177a0e4092f1a10943 > staging/kemoticons/src/providers/adium/adium_emoticons.cpp > a3aaa0fdc0b3dcc862f98865d2e6419e716f4f17 > staging/kemoticons/src/providers/adium/adium_emoticons.h > 039a267679919c968bbe4d12c35abb90cf1bcc9b > staging/kemoticons/src/core/kemoticonstheme.cpp > ed3407cdd45c84d91f2d0057e108b760ff696ff2 > staging/kemoticons/src/core/kemoticonstheme.h > b8b25f7607d3741dda78d6302c1ed81fbc9211a0 > staging/kemoticons/src/core/kemoticonsprovider.cpp > d04c76e87b118f5d45717b3b2ccd9dea47dc2526 > staging/kemoticons/src/core/kemoticonsprovider.h > 85fc7efb8923d76476f0a16f70f8ebb15e451081 > KDE5PORTING.html 57ecf2e007e270b634cc8d16462b73e96553c92d > > Diff: http://git.reviewboard.kde.org/r/112984/diff/ > > > Testing > --- > > It builds. It installs. Tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112984: Adjust API in KEmoticons framework: KEmoticonsProvider::save()
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112984/ --- (Updated Oct. 22, 2013, 9:51 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- Adjust API in KEmoticons framework: KEmoticonsProvider::save() -To make KEmoticons API more consistent, deprecate KEmoticonsProvider::save() and prefer saveTheme() instead. -Adjust plugins. -Before the cleanup, KEmoticonsTheme was calling KEmoticonsProvider::save(), which was empty. Now it's a pure virtual function. Therefore, I deprecate it and advice subclassing KEmoticonsProvider. Diffs - staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 0dc92ed092d87a559fe7fa1a40e804843ab73d59 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.h 0873a635a300e0ed8e38b1d59a535d6ec15b99cb staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp e9f89eecce3d6e6407113a84cb5200ff66564c19 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.h 70bafa3fe4ba25c9d6a69634024b2c157235e674 staging/kemoticons/src/providers/kde/kde_emoticons.cpp 5b5114a14dd94a6ebcba8a6f7dd163f73048189a staging/kemoticons/src/providers/kde/kde_emoticons.h 8cba6b194eee1543bf13f1177a0e4092f1a10943 staging/kemoticons/src/providers/adium/adium_emoticons.cpp a3aaa0fdc0b3dcc862f98865d2e6419e716f4f17 staging/kemoticons/src/providers/adium/adium_emoticons.h 039a267679919c968bbe4d12c35abb90cf1bcc9b staging/kemoticons/src/core/kemoticonstheme.cpp ed3407cdd45c84d91f2d0057e108b760ff696ff2 staging/kemoticons/src/core/kemoticonstheme.h b8b25f7607d3741dda78d6302c1ed81fbc9211a0 staging/kemoticons/src/core/kemoticonsprovider.cpp d04c76e87b118f5d45717b3b2ccd9dea47dc2526 staging/kemoticons/src/core/kemoticonsprovider.h 85fc7efb8923d76476f0a16f70f8ebb15e451081 KDE5PORTING.html 57ecf2e007e270b634cc8d16462b73e96553c92d Diff: http://git.reviewboard.kde.org/r/112984/diff/ Testing --- It builds. It installs. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 112985: Adjust API in KEmoticons framework: map and index methods
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112985/ --- (Updated Oct. 22, 2013, 10:09 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- Adjust API in KEmoticons framework: map and index methods -Make map and index KEmoticons API slightly clearer and deprecate the methods that are confusing. -Use the new methods in the plugins. Diffs - staging/kemoticons/src/core/kemoticonsprovider.h 85fc7efb8923d76476f0a16f70f8ebb15e451081 staging/kemoticons/src/core/kemoticonsprovider.cpp d04c76e87b118f5d45717b3b2ccd9dea47dc2526 staging/kemoticons/src/providers/adium/adium_emoticons.cpp a3aaa0fdc0b3dcc862f98865d2e6419e716f4f17 staging/kemoticons/src/providers/kde/kde_emoticons.cpp 5b5114a14dd94a6ebcba8a6f7dd163f73048189a staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp e9f89eecce3d6e6407113a84cb5200ff66564c19 staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp 0dc92ed092d87a559fe7fa1a40e804843ab73d59 Diff: http://git.reviewboard.kde.org/r/112985/diff/ Testing --- It builds. It installs. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113426: Adjust API in KEmoticons framework: createNew method
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113426/ --- (Updated Oct. 24, 2013, 9:54 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- Adjust API in KEmoticons framework: createNew method -To make KEmoticons API more consistent, deprecate KEmoticonsProvider::createNew() and prefer newTheme() instead, as it appears in KEmoticonsTheme. That way, we have loadTheme(), saveTheme() and newTheme(). -Adjust plugins. -Before the cleanup, KEmoticonsTheme was calling KEmoticonsProvider::createNew(), which was empty. Therefore, I deprecate it and advice subclassing KEmoticonsProvider. Diffs - KDE5PORTING.html ceff2fa13e4a666939dd0a1bb63e967504c31c07 staging/kemoticons/src/core/kemoticons.cpp 43dac6517b77a0d0040912958fe76687b475d85c staging/kemoticons/src/core/kemoticonsprovider.h 2ec0de8d1dfb846188bd458b49a4028fee115431 staging/kemoticons/src/core/kemoticonsprovider.cpp 7374966c65922c3e7a5be881c198a8f8f52fee29 staging/kemoticons/src/core/kemoticonstheme.h 25fc29453535d7e73f4e2d0752ce3f989c83fa96 staging/kemoticons/src/core/kemoticonstheme.cpp e54d015e7f0f866d199d8eed7863fafd28576c13 staging/kemoticons/src/providers/adium/adium_emoticons.h 01d89e13834c345765e696d66560071dc10291af staging/kemoticons/src/providers/adium/adium_emoticons.cpp e6719d112a14478bdfd7f8c47633c18108a5633a staging/kemoticons/src/providers/kde/kde_emoticons.h 0738b79dcf734b7904e061b5eb41807ccaf443ff staging/kemoticons/src/providers/kde/kde_emoticons.cpp a99c6d84f5ab7e0e2f41027c37a97f170333dca8 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.h a51b736f7702d7af1f1367dd1f13271647212fee staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 7596e30e8e5153185a3dd365858567c69477ff4a staging/kemoticons/src/providers/xmpp/xmpp_emoticons.h 4ba706f519cebedaa6c9c3f2f02331e85745e89a staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp afb07b207407b00bbe0d38e0ca6d9e2bf2ccd809 Diff: http://git.reviewboard.kde.org/r/113426/diff/ Testing --- Builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113426: Adjust API in KEmoticons framework: createNew method
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113426/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- Adjust API in KEmoticons framework: createNew method -To make KEmoticons API more consistent, deprecate KEmoticonsProvider::createNew() and prefer newTheme() instead, as it appears in KEmoticonsTheme. That way, we have loadTheme(), saveTheme() and newTheme(). -Adjust plugins. -Before the cleanup, KEmoticonsTheme was calling KEmoticonsProvider::createNew(), which was empty. Therefore, I deprecate it and advice subclassing KEmoticonsProvider. Diffs - KDE5PORTING.html ceff2fa13e4a666939dd0a1bb63e967504c31c07 staging/kemoticons/src/core/kemoticons.cpp 43dac6517b77a0d0040912958fe76687b475d85c staging/kemoticons/src/core/kemoticonsprovider.h 2ec0de8d1dfb846188bd458b49a4028fee115431 staging/kemoticons/src/core/kemoticonsprovider.cpp 7374966c65922c3e7a5be881c198a8f8f52fee29 staging/kemoticons/src/core/kemoticonstheme.h 25fc29453535d7e73f4e2d0752ce3f989c83fa96 staging/kemoticons/src/core/kemoticonstheme.cpp e54d015e7f0f866d199d8eed7863fafd28576c13 staging/kemoticons/src/providers/adium/adium_emoticons.h 01d89e13834c345765e696d66560071dc10291af staging/kemoticons/src/providers/adium/adium_emoticons.cpp e6719d112a14478bdfd7f8c47633c18108a5633a staging/kemoticons/src/providers/kde/kde_emoticons.h 0738b79dcf734b7904e061b5eb41807ccaf443ff staging/kemoticons/src/providers/kde/kde_emoticons.cpp a99c6d84f5ab7e0e2f41027c37a97f170333dca8 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.h a51b736f7702d7af1f1367dd1f13271647212fee staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 7596e30e8e5153185a3dd365858567c69477ff4a staging/kemoticons/src/providers/xmpp/xmpp_emoticons.h 4ba706f519cebedaa6c9c3f2f02331e85745e89a staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp afb07b207407b00bbe0d38e0ca6d9e2bf2ccd809 Diff: http://git.reviewboard.kde.org/r/113426/diff/ Testing --- Builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113426: Adjust API in KEmoticons framework: createNew method
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113426/ --- (Updated Oct. 24, 2013, 9:54 p.m.) Review request for KDE Frameworks. Repository: kdelibs Description --- Adjust API in KEmoticons framework: createNew method -To make KEmoticons API more consistent, deprecate KEmoticonsProvider::createNew() and prefer newTheme() instead, as it appears in KEmoticonsTheme. That way, we have loadTheme(), saveTheme() and newTheme(). -Adjust plugins. -Before the cleanup, KEmoticonsTheme was calling KEmoticonsProvider::createNew(), which was empty. Therefore, I deprecate it and advice subclassing KEmoticonsProvider. Diffs - KDE5PORTING.html ceff2fa13e4a666939dd0a1bb63e967504c31c07 staging/kemoticons/src/core/kemoticons.cpp 43dac6517b77a0d0040912958fe76687b475d85c staging/kemoticons/src/core/kemoticonsprovider.h 2ec0de8d1dfb846188bd458b49a4028fee115431 staging/kemoticons/src/core/kemoticonsprovider.cpp 7374966c65922c3e7a5be881c198a8f8f52fee29 staging/kemoticons/src/core/kemoticonstheme.h 25fc29453535d7e73f4e2d0752ce3f989c83fa96 staging/kemoticons/src/core/kemoticonstheme.cpp e54d015e7f0f866d199d8eed7863fafd28576c13 staging/kemoticons/src/providers/adium/adium_emoticons.h 01d89e13834c345765e696d66560071dc10291af staging/kemoticons/src/providers/adium/adium_emoticons.cpp e6719d112a14478bdfd7f8c47633c18108a5633a staging/kemoticons/src/providers/kde/kde_emoticons.h 0738b79dcf734b7904e061b5eb41807ccaf443ff staging/kemoticons/src/providers/kde/kde_emoticons.cpp a99c6d84f5ab7e0e2f41027c37a97f170333dca8 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.h a51b736f7702d7af1f1367dd1f13271647212fee staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 7596e30e8e5153185a3dd365858567c69477ff4a staging/kemoticons/src/providers/xmpp/xmpp_emoticons.h 4ba706f519cebedaa6c9c3f2f02331e85745e89a staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp afb07b207407b00bbe0d38e0ca6d9e2bf2ccd809 Diff: http://git.reviewboard.kde.org/r/113426/diff/ Testing --- Builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 113436: Deprecate KEmoticonsTheme methods
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113436/ --- Review request for KDE Frameworks. Repository: kdelibs Description --- Deprecate KEmoticonsTheme methods Before the initial cleanup, the loadTheme(), removeEmoticon() and addEmoticon methods of KEmoticonsTheme called the KEmoticonsProvider methods of the same name, which were either empty or useless. Now the KEmoticonsProvider methods are pure virtual and therefore those KEmoticonsTheme methods are deprecated and a subclassing of KEmoticonsProvider is advised. Diffs - KDE5PORTING.html ceff2fa13e4a666939dd0a1bb63e967504c31c07 staging/kemoticons/src/core/kemoticons.cpp 43dac6517b77a0d0040912958fe76687b475d85c staging/kemoticons/src/core/kemoticonstheme.h 25fc29453535d7e73f4e2d0752ce3f989c83fa96 Diff: http://git.reviewboard.kde.org/r/113436/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113426: Adjust API in KEmoticons framework: createNew method
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113426/ --- (Updated Oct. 29, 2013, 7:01 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- Adjust API in KEmoticons framework: createNew method -To make KEmoticons API more consistent, deprecate KEmoticonsProvider::createNew() and prefer newTheme() instead, as it appears in KEmoticonsTheme. That way, we have loadTheme(), saveTheme() and newTheme(). -Adjust plugins. -Before the cleanup, KEmoticonsTheme was calling KEmoticonsProvider::createNew(), which was empty. Therefore, I deprecate it and advice subclassing KEmoticonsProvider. Diffs - KDE5PORTING.html ceff2fa13e4a666939dd0a1bb63e967504c31c07 staging/kemoticons/src/core/kemoticons.cpp 43dac6517b77a0d0040912958fe76687b475d85c staging/kemoticons/src/core/kemoticonsprovider.h 2ec0de8d1dfb846188bd458b49a4028fee115431 staging/kemoticons/src/core/kemoticonsprovider.cpp 7374966c65922c3e7a5be881c198a8f8f52fee29 staging/kemoticons/src/core/kemoticonstheme.h 25fc29453535d7e73f4e2d0752ce3f989c83fa96 staging/kemoticons/src/core/kemoticonstheme.cpp e54d015e7f0f866d199d8eed7863fafd28576c13 staging/kemoticons/src/providers/adium/adium_emoticons.h 01d89e13834c345765e696d66560071dc10291af staging/kemoticons/src/providers/adium/adium_emoticons.cpp e6719d112a14478bdfd7f8c47633c18108a5633a staging/kemoticons/src/providers/kde/kde_emoticons.h 0738b79dcf734b7904e061b5eb41807ccaf443ff staging/kemoticons/src/providers/kde/kde_emoticons.cpp a99c6d84f5ab7e0e2f41027c37a97f170333dca8 staging/kemoticons/src/providers/pidgin/pidgin_emoticons.h a51b736f7702d7af1f1367dd1f13271647212fee staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp 7596e30e8e5153185a3dd365858567c69477ff4a staging/kemoticons/src/providers/xmpp/xmpp_emoticons.h 4ba706f519cebedaa6c9c3f2f02331e85745e89a staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp afb07b207407b00bbe0d38e0ca6d9e2bf2ccd809 Diff: http://git.reviewboard.kde.org/r/113426/diff/ Testing --- Builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 113436: Deprecate KEmoticonsTheme methods
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113436/ --- (Updated Oct. 29, 2013, 7:06 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kdelibs Description --- Deprecate KEmoticonsTheme methods Before the initial cleanup, the loadTheme(), removeEmoticon() and addEmoticon methods of KEmoticonsTheme called the KEmoticonsProvider methods of the same name, which were either empty or useless. Now the KEmoticonsProvider methods are pure virtual and therefore those KEmoticonsTheme methods are deprecated and a subclassing of KEmoticonsProvider is advised. Diffs - KDE5PORTING.html ceff2fa13e4a666939dd0a1bb63e967504c31c07 staging/kemoticons/src/core/kemoticons.cpp 43dac6517b77a0d0040912958fe76687b475d85c staging/kemoticons/src/core/kemoticonstheme.h 25fc29453535d7e73f4e2d0752ce3f989c83fa96 Diff: http://git.reviewboard.kde.org/r/113436/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Dialogs, QPT and Qt5
Hi! 2013/11/29 Albert Astals Cid > El Divendres, 29 de novembre de 2013, a les 12:22:48, Kevin Ottens va > escriure: > > On Friday 29 November 2013 12:00:38 Christoph Feck wrote: > > > On Friday 29 November 2013 08:53:06 Kevin Ottens wrote: > > > > KColorDialog and KFontDialog are out of the picture. We contributed > > > > to the upstream QColorDialog and QFontDialog so that they have the > > > > features we needed out of the box. We only need to do something > > > > for the file dialog. > > > > > > Not sure if we can silently drop the K classes. > > > > > > KFontDialog: > > > - allows the application to specify a list of fonts to show > > > > To upstream if that's used (IIRC we didn't find users of that). > > > > > - allows the application to request fixed-width fonts only > > > > That's upstream. > > > > > - allows limiting to only change family, style, or size > > > > To upstream if that's used (IIRC almost no user again). > > > > > - supports fractional point sizes, e.g. 9.4 pt. > > > > To upstream if needed. > > > > > - word wraps and scrolls big sample texts > > > > To upstream. > > > > > KColorDialog: > > > - allows the application to specify a default color, where the user > > > can revert to > > > > To upstream. > > > > > - supports both RGB and HSV color models > > > > Looks like QColorDialog does too, or I'm missing something? > > > > > - has support for loadable palettes (editable via kcoloredit) > > > > OK, that one is indeed missing. And if we want to upstream that it'll > > require more work than the rest. > > > > > - appearantly the color picker and hex line edit have been upstreamed, > > > but I just tried Qt 5.3 Designer, and those features did not appear. > > > > Odd, I see them here. > > > > > > Overall my point is that it's mostly features which really belongs > upstream. > > Right, but they don't exist at the moment, do they? > > Why do you hate those features and need to kill them now? > > What about apps that use those features? What are we going to tell the > developers? "Sorry but your feature was not really important so we killed > it, > but don't worry KDE loves you!" > > Maybe I'm totally wrong, but If those developers think that those features are really important, perhaps they can wait for the next version of KF5, which I suppose that will be based on Qt5.3, which can have all those features upstreamed. It doesn't have to be "include them right now or kill them forever" :-) The transition to KF5 won't be as fast as to have all the developers changing their code the day after the release of KF5, will they? Is there any possibility to find a solution between the two? Cheers, David Gil Doesn't feel right. > Cheers, > Albert > > > And they're either stuff we can contribute at any point or stuff which is > > unused. The only exception seems to be the loadable palettes, but that's > > not enough to warrant having a full fledged color dialog on our side IMO. > > So if that's really important, it's something to design properly to be > able > > to plug those loadable palettes into QColorDialog. > > > > Regards. > > ___ > Kde-frameworks-devel mailing list > Kde-frameworks-devel@kde.org > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Problem configuring with CMake
Hi! I have begun the building of KF5 from scratch (completely clean) but when configuring kdelibs-frameworks with CMake, I get the following: -- Found GSSAPI: -Wl,-Bsymbolic-functions -Wl,-z,relro -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err CMake Warning at tier3/CMakeLists.txt:25 (find_package): Could not find a package configuration file provided by "Qt5WebKitWidgets" with any of the following names: Qt5WebKitWidgetsConfig.cmake qt5webkitwidgets-config.cmake Add the installation prefix of "Qt5WebKitWidgets" to CMAKE_PREFIX_PATH or set "Qt5WebKitWidgets_DIR" to a directory containing one of the above files. If "Qt5WebKitWidgets" provides a separate development package or SDK, be sure it has been installed. CMake Error at tier4/kde4support/src/CMakeLists.txt:322 (target_compile_definitions): target_compile_definitions can not be used on an ALIAS target. Any hint where are those files? Thank you David Gil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 114716: Separate author name from email addres in KAboutData::processCommandLine
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114716/ --- Review request for KDE Frameworks. Repository: kcoreaddons Description --- Without this patch, the parsing of "kgeography --author" gives: kgeography was written by: Albert Astals cidaa...@kde.org With this patch: kgeography was written by: Albert Astals Cidaa...@kde.org Diffs - src/lib/kaboutdata.cpp 3f08f25 Diff: https://git.reviewboard.kde.org/r/114716/diff/ Testing --- It builds. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114716: Separate author name from email addres in KAboutData::processCommandLine
> On Dec. 29, 2013, 1:05 a.m., Albert Astals Cid wrote: > > src/lib/kaboutdata.cpp, line 969 > > <https://git.reviewboard.kde.org/r/114716/diff/1/?file=227634#file227634line969> > > > > Not related to your patch, but i wonder if we need a \n here too? I don't think so. The output of "kgeography --license" gives the copyright and the license in a good format. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114716/#review46342 ------- On Dec. 29, 2013, 12:55 a.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/114716/ > --- > > (Updated Dec. 29, 2013, 12:55 a.m.) > > > Review request for KDE Frameworks. > > > Repository: kcoreaddons > > > Description > --- > > Without this patch, the parsing of "kgeography --author" gives: > > kgeography was written by: >Albert Astals cidaa...@kde.org > > With this patch: > > kgeography was written by: >Albert Astals Cidaa...@kde.org > > > Diffs > - > > src/lib/kaboutdata.cpp 3f08f25 > > Diff: https://git.reviewboard.kde.org/r/114716/diff/ > > > Testing > --- > > It builds. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114716: Separate author name from email addres in KAboutData::processCommandLine
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114716/ --- (Updated Dec. 29, 2013, 11:50 p.m.) Review request for KDE Frameworks. Changes --- Apply changes suggested by David Faure. Repository: kcoreaddons Description --- Without this patch, the parsing of "kgeography --author" gives: kgeography was written by: Albert Astals cidaa...@kde.org With this patch: kgeography was written by: Albert Astals Cidaa...@kde.org Diffs (updated) - src/lib/kaboutdata.cpp 3f08f25 Diff: https://git.reviewboard.kde.org/r/114716/diff/ Testing --- It builds. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Issues porting KGeography to KF5
Hi! I'm porting KGeography to KF5, and I found some issues. *KConfigDialog::setHelp()* KConfigDialog* dialog = new KConfigDialog(this, "settings", kgeographySettings::self()); dialog->setHelp("configuration", "kgeography"); It gives me the following error: /home/david/devel/kgeography/src/kgeography.cpp:170:13: error: ‘class KConfigDialog’ has no member named ‘setHelp’ make[2]: *** [src/CMakeFiles/kgeography.dir/kgeography.cpp.o] Error 1 make[1]: *** [src/CMakeFiles/kgeography.dir/all] Error 2 make: *** [all] Error 2 What should I subtitute it for? Or should I drop it? *KApplication::setTopWidget()* I substituted KApplication for QApplication in main.cpp. QApplication app(argc, argv); (...) app.setTopWidget(new kgeography()); It gives me the following error: /home/david/devel/kgeography/src/main.cpp: In function ‘int main(int, char**)’: /home/david/devel/kgeography/src/main.cpp:21:9: error: ‘class QApplication’ has no member named ‘setTopWidget’ Should I drop it? Is there anything I can substitute it for? Thanks, David Gil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114716: Separate author name from email addres in KAboutData::processCommandLine
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114716/ --- (Updated Jan. 1, 2014, 6:55 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcoreaddons Description --- Without this patch, the parsing of "kgeography --author" gives: kgeography was written by: Albert Astals cidaa...@kde.org With this patch: kgeography was written by: Albert Astals Cidaa...@kde.org Diffs - src/lib/kaboutdata.cpp 3f08f25 Diff: https://git.reviewboard.kde.org/r/114716/diff/ Testing --- It builds. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Problem with kf5-development/kde-build-metadata/dependency-data-common
Hi! I have tried to build KF5 from scratch. When running ./kdesrc-build, I get the following: david@nimfa:~/devel/kf5-development$ ./kdesrc-build Script started processing at Mon Jan 6 11:53:41 2014 * Downloading projects.kde.org project database... Updating kde-build-metadata (to branch master) * Problems encountered trying to sort modules into correct order: * Unable to open /home/david/devel/kf5-development/kde-build-metadata/dependency-data-common: File or directory doesn't exist at /home/david/devel/kf5-development/extragear/utils/kdesrc-build/modules/ksb/Application.pm line 643. at /home/david/devel/kf5-development/extragear/utils/kdesrc-build/modules/ksb/Application.pm line 643 eval {...} called at /home/david/devel/kf5-development/extragear/utils/kdesrc-build/modules/ksb/Application.pm line 627 ksb::Application::runAllModulePhases('ksb::Application=HASH(0x9830c08)') called at ./kdesrc-build line 232 eval {...} called at ./kdesrc-build line 218 * Will attempt to continue. I have checked that /home/david/devel/kf5-development/kde-build-metadata/dependency-data-common doesn't exist. Many frameworks then don't build properly. What should I do? Should that file actually be there? Thanks in advance David Gil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Problem with kf5-development/kde-build-metadata/dependency-data-common
Yes, it works. Thanks! David Gil El 06/01/2014 13:49, "David Faure" va escriure: > On Monday 06 January 2014 12:03:26 David Gil Oliva wrote: > > I have checked that > > > /home/david/devel/kf5-development/kde-build-metadata/dependency-data-common > > doesn't exist. > > Indeed; chicken and egg problem (I was waiting for kdesrc-build to support > it > before moving stuff to it). > > Created now, does it work better? > > -- > David Faure, fa...@kde.org, http://www.davidfaure.fr > Working on KDE, in particular KDE Frameworks 5 > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
QStringLiteral and QTimer in KCompletion tests
Hi! *kcompletionuitest.cpp* In kcompletionuitest.cpp I have found many uses of QStringLiteral that perhaps aren't necessary, but I'm not sure: Form1::Form1(QWidget *parent) : QWidget(parent) { setAttribute(Qt::WA_DeleteOnClose); *setObjectName(QStringLiteral("Form1"));* resize(559, 465); *setWindowTitle(QStringLiteral("Form1"));* Form1Layout = new QVBoxLayout(this); GroupBox1 = new QGroupBox(this); GroupBox1->setLayout(new QVBoxLayout()); *GroupBox1->setTitle(QStringLiteral("Completion Test"));* Can't it be just GroupBox1->setTitle("Completion Test"); for example? *kcomboboxtest.cpp* Another thing: the test kcomboboxtest has a Enable/Disable button that uses a QTimer, so, when pressed, it takes 5 endless seconds to enable/disable the combo boxes and change the text of the button. May I take off that QTimer or is it useful in some unknown way? :-/ Thanks! David Gil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115206: Correct spelling, grammar and style of kcompletion.h docs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115206/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Correct spelling, grammar and style of kcompletion.h docs Diffs - src/kcompletion.h 46b63a4 Diff: https://git.reviewboard.kde.org/r/115206/diff/ Testing --- Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Split classes into separate files?
Hi! I'm revising KCompletion and I have found that in kcompletion.h you can see the declaration of KCompletionBase (which is defined in a separate file) and KCompletionMatches (which is defined in kcompletion.cpp). I think I have seen that in other frameworks classes are being split into separate files. Should I also do it with KCompletionBase and KCompletionMatches? Cheers David Gil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115206: Correct spelling, grammar and style of kcompletion.h docs
> On Jan. 21, 2014, 11:43 p.m., Alex Merry wrote: > > Thanks for working on apidox! Sorry for the large number of comments... Wow! You were ultra-fast, just 31 minutes for all this! :-) I will see one by one tomorrow (just wanted to comment about something I clearly don't agree). > On Jan. 21, 2014, 11:43 p.m., Alex Merry wrote: > > src/kcompletion.h, line 103 > > <https://git.reviewboard.kde.org/r/115206/diff/1/?file=235092#file235092line103> > > > > You remove the hyphen from "auto-completion" ealier, but not here. Yes, I drop the hyphen from "auto-completion" when it works as a noun. But here (and in many other places) is working as a compound adjective. Some sources about compound adjectives and hyphens: http://writersrelief.com/blog/2008/03/hyphen-rules-dont-let-misused-hyphens-muddle-your-adjectives-or-your-writing/ http://www.grammarbook.com/punctuation/hyphens.asp Nevertheless, I may be wrong. If so, please tell me. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115206/#review47958 ----------- On Jan. 21, 2014, 11:12 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115206/ > --- > > (Updated Jan. 21, 2014, 11:12 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Correct spelling, grammar and style of kcompletion.h docs > > > Diffs > - > > src/kcompletion.h 46b63a4 > > Diff: https://git.reviewboard.kde.org/r/115206/diff/ > > > Testing > --- > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115206: Correct spelling, grammar and style of kcompletion.h docs
> On Jan. 21, 2014, 11:43 p.m., Alex Merry wrote: > > src/kcompletion.h, line 501 > > <https://git.reviewboard.kde.org/r/115206/diff/1/?file=235092#file235092line501> > > > > One sentence: "This signal is emitted by makeCompletion() when a match > > is found. Since more methods emit this signal, I have rephrased it further. makeCompletion() doesn't emit empty strings, but previousMatch() and nextMatch() do emit empty strings when there is no match. In the next days I will update the patch. /** * This signal is emitted by makeCompletion() when a match is found. * @param item the match. In previousMatch() and nextMatch() it can * be a QString() if none is found. */ - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115206/#review47958 ----------- On Jan. 21, 2014, 11:12 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115206/ > --- > > (Updated Jan. 21, 2014, 11:12 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Correct spelling, grammar and style of kcompletion.h docs > > > Diffs > - > > src/kcompletion.h 46b63a4 > > Diff: https://git.reviewboard.kde.org/r/115206/diff/ > > > Testing > --- > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115206: Correct spelling, grammar and style of kcompletion.h docs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115206/ --- (Updated Jan. 26, 2014, 10:46 p.m.) Review request for KDE Frameworks. Changes --- Addressed all issues told by Alex, except the one in the signal matches(), which AFAICS is not necessary. Repository: kcompletion Description --- Correct spelling, grammar and style of kcompletion.h docs Diffs (updated) - src/kcompletion.h 46b63a4 Diff: https://git.reviewboard.kde.org/r/115206/diff/ Testing --- Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115206: Correct spelling, grammar and style of kcompletion.h docs
> On Jan. 21, 2014, 11:43 p.m., Alex Merry wrote: > > src/kcompletion.h, line 501 > > <https://git.reviewboard.kde.org/r/115206/diff/1/?file=235092#file235092line501> > > > > One sentence: "This signal is emitted by makeCompletion() when a match > > is found. > > David Gil Oliva wrote: > Since more methods emit this signal, I have rephrased it further. > makeCompletion() doesn't emit empty strings, but previousMatch() and > nextMatch() do emit empty strings when there is no match. In the next days I > will update the patch. > > /** > * This signal is emitted by makeCompletion() when a match is found. > * @param item the match. In previousMatch() and nextMatch() it can > * be a QString() if none is found. > */ > > Alex Merry wrote: > How about > /** > * This signal is emitted when a match is found. > * > * In particular, makeCompletion(), previousMatch() and nextMatch() > all > * emit this signal; makeCompletion() will only emit it when a match > is > * found, but the other methods will always emit it (and so may emit > it > * with an empty string). > * > * @param item the matching item, or QString() if there were no more > * matching items. > */ > > And a similar thing for matches(). I think matches() is ok, since it is only used by makeCompletion(). - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115206/#review47958 --- On Jan. 26, 2014, 10:46 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115206/ > --- > > (Updated Jan. 26, 2014, 10:46 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Correct spelling, grammar and style of kcompletion.h docs > > > Diffs > - > > src/kcompletion.h 46b63a4 > > Diff: https://git.reviewboard.kde.org/r/115206/diff/ > > > Testing > --- > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115206: Correct spelling, grammar and style of kcompletion.h docs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115206/ --- (Updated Jan. 27, 2014, 7:14 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Correct spelling, grammar and style of kcompletion.h docs Diffs - src/kcompletion.h 46b63a4 Diff: https://git.reviewboard.kde.org/r/115206/diff/ Testing --- Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115206: Correct spelling, grammar and style of kcompletion.h docs
> On Jan. 27, 2014, 12:20 p.m., Alex Merry wrote: > > src/kcompletion.h, line 831 > > <https://git.reviewboard.kde.org/r/115206/diff/2/?file=240658#file240658line831> > > > > Does doxygen properly generate a link for this without the leading > > KCompletion::? Yes, it does. I've checked in downloadmanager.h, in knewstuff, where there's a SortOrder enum and it's referenced as @see SortOrder. And in kxmlguiwindow.h there's a StandardWindowOptions enum referenced as @see StandardWindowOptions. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115206/#review48374 ----------- On Jan. 26, 2014, 10:46 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115206/ > --- > > (Updated Jan. 26, 2014, 10:46 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Correct spelling, grammar and style of kcompletion.h docs > > > Diffs > - > > src/kcompletion.h 46b63a4 > > Diff: https://git.reviewboard.kde.org/r/115206/diff/ > > > Testing > --- > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115370: Fix apidox, fix code style and delete useless includes in KComboBox (KCompletion)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115370/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Fix apidox, fix code style and delete useless includes. Diffs - src/kcombobox.h f34d259 src/kcombobox.cpp 2cfe6e7 Diff: https://git.reviewboard.kde.org/r/115370/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Change the ML default reply-to address
Hi, > > Let's be pragmatic, how many times it happened to you that you actually responded to the author alone while you actually intended to respond to the list? It's just super annoying if you're communicating with lists like plasma-devel which has reply-to-list and dozen more KDE MLs which also have reply-to-list and then you're responding to k-f-devel and everytime it's that "oh wait, I need to change the reply-to address". +1 Cheers, David Gil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115394: Split KCompletionBase from kcompletion.h
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115394/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- To make kcompletion.h smaller, split KCompletionBase class from it into its own header file. Diffs - src/klineedit.h f1107e2 src/kcompletionbase.cpp 252b613 src/kcompletion.h fa0731e src/kcompletionbase.h PRE-CREATION src/CMakeLists.txt 05452ab src/kcombobox.h f34d259 Diff: https://git.reviewboard.kde.org/r/115394/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115394: Split KCompletionBase from kcompletion.h
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115394/ --- (Updated Jan. 30, 2014, 10:59 p.m.) Review request for KDE Frameworks. Changes --- Fix issues noted by Aurélien Gâteau Repository: kcompletion Description --- To make kcompletion.h smaller, split KCompletionBase class from it into its own header file. Diffs (updated) - src/klineedit.h f1107e2 src/kcompletionbase.h PRE-CREATION src/kcompletionbase.cpp 252b613 src/kcompletion.h fa0731e src/kcombobox.h f34d259 src/CMakeLists.txt 05452ab Diff: https://git.reviewboard.kde.org/r/115394/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115418: Hide private methods of KCompletion into d-pointer
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115418/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Hide private methods into d-pointer. Convert d-pointer to a QScopedPointer. There are still things to do next: -Rename d-pointer member variable names: d->myCurrentMatch to d->currentMatch -Move KCompletionPrivate declaration to kcompletion_p.h Diffs - src/kcompletion.cpp 9d727f8 src/kcompletion.h fa0731e Diff: https://git.reviewboard.kde.org/r/115418/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115394: Split KCompletionBase from kcompletion.h
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115394/ --- (Updated Jan. 31, 2014, 10:53 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- To make kcompletion.h smaller, split KCompletionBase class from it into its own header file. Diffs - src/klineedit.h f1107e2 src/kcompletionbase.h PRE-CREATION src/kcompletionbase.cpp 252b613 src/kcompletion.h fa0731e src/kcombobox.h f34d259 src/CMakeLists.txt 05452ab Diff: https://git.reviewboard.kde.org/r/115394/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115419: Split KCompletionMatches declaration from kcompletion.h into own header file
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115419/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- To make kcompletion.h smaller, split KCompletionMatches into own header file Diffs - src/kcompletion.cpp c684727 src/kcompletionmatches.h PRE-CREATION src/kcompletion.h f197fc3 src/CMakeLists.txt ae4a656 Diff: https://git.reviewboard.kde.org/r/115419/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115419: Split KCompletionMatches declaration from kcompletion.h into own header file
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115419/ --- (Updated Feb. 1, 2014, 11 a.m.) Review request for KDE Frameworks. Changes --- Add include guard. Repository: kcompletion Description --- To make kcompletion.h smaller, split KCompletionMatches into own header file Diffs (updated) - src/CMakeLists.txt ae4a656 src/kcompletion.h f197fc3 src/kcompletion.cpp c684727 src/kcompletionmatches.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115419/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115419: Split KCompletionMatches declaration from kcompletion.h into own header file
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115419/ --- (Updated Feb. 1, 2014, 11:02 a.m.) Review request for KDE Frameworks. Changes --- Add include guard. Repository: kcompletion Description --- To make kcompletion.h smaller, split KCompletionMatches into own header file Diffs (updated) - src/CMakeLists.txt ae4a656 src/kcompletion.h f197fc3 src/kcompletion.cpp c684727 src/kcompletionmatches.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115419/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115419: Split KCompletionMatches into own headers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115419/ --- (Updated Feb. 2, 2014, 11:14 p.m.) Review request for KDE Frameworks. Changes --- Fix issues. Split the whole class into own files. Summary (updated) - Split KCompletionMatches into own headers Repository: kcompletion Description (updated) --- To make code clearer, split KCompletionMatches. Diffs (updated) - src/CMakeLists.txt ae4a656 src/kcompletion.cpp c684727 src/kcompletion.h f197fc3 src/kcompletionmatches_p.h PRE-CREATION src/kcompletionmatches.cpp PRE-CREATION src/kcompletionmatches.h PRE-CREATION src/kcompletion_p.h a8dedae Diff: https://git.reviewboard.kde.org/r/115419/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115418: Hide private methods of KCompletion into d-pointer
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115418/ --- (Updated Feb. 4, 2014, 12:30 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Hide private methods into d-pointer. Convert d-pointer to a QScopedPointer. There are still things to do next: -Rename d-pointer member variable names: d->myCurrentMatch to d->currentMatch -Move KCompletionPrivate declaration to kcompletion_p.h Diffs - src/kcompletion.cpp 9d727f8 src/kcompletion.h fa0731e Diff: https://git.reviewboard.kde.org/r/115418/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115419: Split KCompletionMatches class into files of its own
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115419/ --- (Updated Feb. 4, 2014, 12:36 a.m.) Review request for KDE Frameworks. Changes --- Change summary Summary (updated) - Split KCompletionMatches class into files of its own Repository: kcompletion Description --- To make code clearer, split KCompletionMatches. Diffs - src/CMakeLists.txt ae4a656 src/kcompletion.cpp c684727 src/kcompletion.h f197fc3 src/kcompletionmatches_p.h PRE-CREATION src/kcompletionmatches.cpp PRE-CREATION src/kcompletionmatches.h PRE-CREATION src/kcompletion_p.h a8dedae Diff: https://git.reviewboard.kde.org/r/115419/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115419: Split KCompletionMatches class into files of its own
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115419/ --- (Updated Feb. 4, 2014, 11:08 p.m.) Review request for KDE Frameworks. Changes --- Fix issues suggested by Alex Merry. Little by little I'm understanding and learning things I didn't completely understand :-D Thanks! Repository: kcompletion Description --- To make code clearer, split KCompletionMatches. Diffs (updated) - src/CMakeLists.txt ae4a656 src/kcompletion.h f197fc3 src/kcompletion.cpp c684727 src/kcompletion_p.h a8dedae src/kcompletionmatches.h PRE-CREATION src/kcompletionmatches.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115419/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115370: Fix apidox, fix code style and delete useless includes in KComboBox (KCompletion)
> On Jan. 31, 2014, 8:07 p.m., Alex Merry wrote: > > src/kcombobox.h, line 52 > > <https://git.reviewboard.kde.org/r/115370/diff/1/?file=240925#file240925line52> > > > > funtionalities -> features I also change this sentence * @li a completion object that provides both automatic * and manual text completion as well as text rotation features to * @li a completion object that provides both automatic * and manual text completion as well as text rotation Otherwise there are too many "features" in so a short text. Do you agree? > On Jan. 31, 2014, 8:07 p.m., Alex Merry wrote: > > src/kcombobox.h, line 183 > > <https://git.reviewboard.kde.org/r/115370/diff/1/?file=240925#file240925line183> > > > > Why? This is out of keeping with every other method's documentation... The reason is that it is not a sentence because there's no verb. That's why it is more like a title, that therefore wouldn't need any full stop. But I imagine that doxygen wants a full stop, doesn't it? > On Jan. 31, 2014, 8:07 p.m., Alex Merry wrote: > > src/kcombobox.h, line 225 > > <https://git.reviewboard.kde.org/r/115370/diff/1/?file=240925#file240925line225> > > > > While not strictly necessary, a line break after the first sentence > > makes it clearer to read (same goes for dox of other methods). > > > > I would also generally prefer a dox style where the description was like > > "Appends a URL with an icon to the combobox." > > and there were @param entries. But I'm not going to reject the patch > > for not doing this. Fixed the first one. I'll take into account the second one in the future. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115370/#review48708 --- On Jan. 28, 2014, 10:44 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115370/ > --- > > (Updated Jan. 28, 2014, 10:44 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Fix apidox, fix code style and delete useless includes. > > > Diffs > - > > src/kcombobox.h f34d259 > src/kcombobox.cpp 2cfe6e7 > > Diff: https://git.reviewboard.kde.org/r/115370/diff/ > > > Testing > --- > > It builds. Tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115370: Fix apidox, fix code style and delete useless includes in KComboBox (KCompletion)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115370/ --- (Updated Feb. 4, 2014, 11:50 p.m.) Review request for KDE Frameworks. Changes --- Fix issued suggested by Alex Merry. Also: -combobox -> combo box Repository: kcompletion Description --- Fix apidox, fix code style and delete useless includes. Diffs (updated) - src/kcombobox.h f34d259 Diff: https://git.reviewboard.kde.org/r/115370/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115370: Fix apidox, fix code style and delete useless includes in KComboBox (KCompletion)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115370/ --- (Updated Feb. 5, 2014, 12:06 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Fix apidox, fix code style and delete useless includes. Diffs - src/kcombobox.h f34d259 Diff: https://git.reviewboard.kde.org/r/115370/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115419: Split KCompletionMatches class into files of its own
> On Feb. 4, 2014, 11:46 p.m., Alex Merry wrote: > > Hmm... I feel that KCompletionMatchesWrapper is in the wrong place, but I'm > > not entirely sure where it *should* go. Well, it's not in a public header, > > so that's far less urgent (although a potential later task would be to move > > the private methods of KCompletion, some of which take > > KCompletionMatchesWrapper as an argument, to KCompletionMatchesPrivate). > > > > Anyway, resolve the last two issues, and it can go in. Yes, I noticed the same problem. After the whole cleanup everything will hopefully be clearer :-) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115419/#review48988 ----------- On Feb. 4, 2014, 11:08 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115419/ > --- > > (Updated Feb. 4, 2014, 11:08 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > To make code clearer, split KCompletionMatches. > > > Diffs > - > > src/CMakeLists.txt ae4a656 > src/kcompletion.h f197fc3 > src/kcompletion.cpp c684727 > src/kcompletion_p.h a8dedae > src/kcompletionmatches.h PRE-CREATION > src/kcompletionmatches.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/115419/diff/ > > > Testing > --- > > It builds. Tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115419: Split KCompletionMatches class into files of its own
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115419/ --- (Updated Feb. 5, 2014, 12:24 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- To make code clearer, split KCompletionMatches. Diffs - src/CMakeLists.txt ae4a656 src/kcompletion.h f197fc3 src/kcompletion.cpp c684727 src/kcompletion_p.h a8dedae src/kcompletionmatches.h PRE-CREATION src/kcompletionmatches.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115419/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115524: Hide a private method and a private slot of KComboBox behind the d-pointer
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115524/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Hide a private method and a private slot of KComboBox behind the d-pointer Also: --Change lineEdit() for isEditable() for checking whether the kcombobox is editable. Diffs - src/kcombobox.h e59d72b src/kcombobox.cpp 2cfe6e7 Diff: https://git.reviewboard.kde.org/r/115524/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115524: Hide a private method and a private slot of KComboBox behind the d-pointer
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115524/ --- (Updated Feb. 6, 2014, 10:35 p.m.) Review request for KDE Frameworks. Changes --- Uncomment line. Repository: kcompletion Description --- Hide a private method and a private slot of KComboBox behind the d-pointer Also: --Change lineEdit() for isEditable() for checking whether the kcombobox is editable. Diffs (updated) - src/kcombobox.h e59d72b src/kcombobox.cpp 2cfe6e7 Diff: https://git.reviewboard.kde.org/r/115524/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115525: Deprecate slots in KCompletion and convert into slots the methods they call
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115525/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Deprecate three slots (slotFoo()) that the only thing they do is call another method (foo()) Diffs - src/kcompletion.h 2104f1b Diff: https://git.reviewboard.kde.org/r/115525/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115526: Refactor private class member variables of KCompletionBase for readability
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115526/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Refactor private class member variables of KCompletionBase for readability Diffs - src/kcompletionbase.h 1c1f23f src/kcompletionbase.cpp 44d05cb Diff: https://git.reviewboard.kde.org/r/115526/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115525: Deprecate slots in KCompletion and convert into slots the methods they call
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115525/ --- (Updated Feb. 7, 2014, 8:44 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Deprecate three slots (slotFoo()) that the only thing they do is call another method (foo()) Diffs - src/kcompletion.h 2104f1b Diff: https://git.reviewboard.kde.org/r/115525/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115524: Hide a private method and a private slot of KComboBox behind the d-pointer
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115524/ --- (Updated Feb. 8, 2014, 9:59 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Hide a private method and a private slot of KComboBox behind the d-pointer Also: --Change lineEdit() for isEditable() for checking whether the kcombobox is editable. Diffs - src/kcombobox.h e59d72b src/kcombobox.cpp 2cfe6e7 Diff: https://git.reviewboard.kde.org/r/115524/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115610: Hide private method behind the d-pointer in KLineEdit
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115610/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Hide private method behind the d-pointer in KLineEdit Also: --Move declaration of KLineEditPrivate to klineedit_p.h and leave implementation in klineedit.cpp --For coherence, change initWidget() for init() --Un-include headers not used In another patch I will hide other private methods and private slots Diffs - src/klineedit.h bea787a src/klineedit.cpp 73e91df src/klineedit_p.h 333135c Diff: https://git.reviewboard.kde.org/r/115610/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: alpha1 release
El 12/02/2014 09:46, "Kevin Ottens" escribió: > > On Tuesday 11 February 2014 23:19:39 David Faure wrote: > > On Thursday 06 February 2014 23:30:25 David Faure wrote: > > > I have packaged up KF5 alpha 1 and uploaded it for packagers. > > > > And alpha1 is now public ! > > > > (tarballs published, git tags pushed) > > Woohoo! Thanks! > > Reminder: Alpha 2 in 17 days. ;-) Will it be allowed to make API changes during the period from alpha2 to beta1? Or, better said, what will be allowed in each phase? Thanks David Gil > Regards. > -- > Kévin Ottens, http://ervin.ipsquad.net > > KDAB - proud supporter of KDE, http://www.kdab.com > > > ___ > Kde-frameworks-devel mailing list > Kde-frameworks-devel@kde.org > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115710: Hide private methods and slots behind the d-pointer in KHistoryComboBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115710/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Hide private methods and slots behind the d-pointer in KHistoryComboBox Also: -Remove header not used Diffs - src/khistorycombobox.h 3667eb4 src/khistorycombobox.cpp 6f81dda Diff: https://git.reviewboard.kde.org/r/115710/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115610: Hide private method behind the d-pointer in KLineEdit
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115610/#review49684 --- Ping? Should I send this patch differently or something? - David Gil Oliva On Feb. 9, 2014, 11:07 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115610/ > --- > > (Updated Feb. 9, 2014, 11:07 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Hide private method behind the d-pointer in KLineEdit > > Also: > --Move declaration of KLineEditPrivate to klineedit_p.h and leave > implementation in klineedit.cpp > --For coherence, change initWidget() for init() > --Un-include headers not used > > In another patch I will hide other private methods and private slots > > > Diffs > - > > src/klineedit.h bea787a > src/klineedit.cpp 73e91df > src/klineedit_p.h 333135c > > Diff: https://git.reviewboard.kde.org/r/115610/diff/ > > > Testing > --- > > It builds. Tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115526: Refactor private class member variables of KCompletionBase for readability
> On Feb. 12, 2014, 7:26 a.m., David Faure wrote: > > I kind of like the m_ prefix even in Private classes, for the readability > > of methods in that class. But no big deal, won't veto this since Qt doesn't > > use m_. I'm more a Qt-ish kind of man ;-) But I'm glad to know that there are people who think differently. I thought that it was really common to leave out the m_ prefix in d-pointer member variables. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115526/#review49623 ------- On Feb. 7, 2014, 12:08 a.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115526/ > --- > > (Updated Feb. 7, 2014, 12:08 a.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Refactor private class member variables of KCompletionBase for readability > > > Diffs > - > > src/kcompletionbase.h 1c1f23f > src/kcompletionbase.cpp 44d05cb > > Diff: https://git.reviewboard.kde.org/r/115526/diff/ > > > Testing > --- > > It builds. Tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115526: Refactor private class member variables of KCompletionBase for readability
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115526/ --- (Updated Feb. 12, 2014, 11:46 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Refactor private class member variables of KCompletionBase for readability Diffs - src/kcompletionbase.h 1c1f23f src/kcompletionbase.cpp 44d05cb Diff: https://git.reviewboard.kde.org/r/115526/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115610: Hide private method behind the d-pointer in KLineEdit
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115610/ --- (Updated Feb. 18, 2014, 8:59 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Hide private method behind the d-pointer in KLineEdit Also: --Move declaration of KLineEditPrivate to klineedit_p.h and leave implementation in klineedit.cpp --For coherence, change initWidget() for init() --Un-include headers not used In another patch I will hide other private methods and private slots Diffs - src/klineedit.h bea787a src/klineedit.cpp 73e91df src/klineedit_p.h 333135c Diff: https://git.reviewboard.kde.org/r/115610/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115901: Hide private methods and private slots behind the d-pointer in KLineEdit
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115901/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Hide private methods and private slots behind the d-pointer in KLineEdit Diffs - src/klineedit.h b9457c1 src/klineedit.cpp 24b3bf0 src/klineedit_p.h 778e43a Diff: https://git.reviewboard.kde.org/r/115901/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115901: Hide private methods and private slots behind the d-pointer in KLineEdit
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115901/ --- (Updated Feb. 21, 2014, 11:11 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Hide private methods and private slots behind the d-pointer in KLineEdit Diffs - src/klineedit.h b9457c1 src/klineedit.cpp 24b3bf0 src/klineedit_p.h 778e43a Diff: https://git.reviewboard.kde.org/r/115901/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115710: Hide private methods and slots behind the d-pointer in KHistoryComboBox
> On Feb. 20, 2014, 12:21 p.m., Kevin Ottens wrote: > > Please address David's comments, I think this patch is interesting and > > shouldn't stay in limbo too long. Will do as soon as I can. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115710/#review50347 --- On Feb. 12, 2014, 11:28 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115710/ > --- > > (Updated Feb. 12, 2014, 11:28 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Hide private methods and slots behind the d-pointer in KHistoryComboBox > > Also: > -Remove header not used > > > Diffs > - > > src/khistorycombobox.h 3667eb4 > src/khistorycombobox.cpp 6f81dda > > Diff: https://git.reviewboard.kde.org/r/115710/diff/ > > > Testing > --- > > It builds. Tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
CMake error compiling kde-runtime
Hi! I've removed the build and install folders of KF5. When running kdesrc-build, I get the following error in kde-runtime: # kdesrc-build running: 'cmake' '/home/david/devel/kf5-development/kde-runtime' '-DCMAKE_CXX_FLAGS:STRING=-pipe -DQT_STRICT_ITERATORS -DQURL_NO_CAST_FROM_STRING -DQT_NO_HTTP -DQT_NO_FTP -Wformat -Werror=format-security -Werror=return-type -Wno-variadic-macros -Wlogical-op ' '-DCMAKE_INSTALL_PREFIX=/home/david/KF5' # from directory: /home/david/devel/kf5-development/build/kde-runtime -- The C compiler identification is GNU 4.7.3 -- The CXX compiler identification is GNU 4.7.3 -- Check for working C compiler: /usr/bin/cc -- Check for working C compiler: /usr/bin/cc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Check for working CXX compiler: /usr/bin/c++ -- Check for working CXX compiler: /usr/bin/c++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done CMake Error at /home/david/KF5/share/cmake-3.0/Modules/CMakeFindDependencyMacro.cmake:52 (find_package): Could not find a configuration file for package "KF5DesignerPlugin" that is compatible with requested version "5.2". The following configuration files were considered but not accepted: /home/david/KF5/lib/i386-linux-gnu/cmake/KF5DesignerPlugin/KF5DesignerPluginConfig.cmake, version: 4.96.0 Call Stack (most recent call first): /home/david/KF5/lib/i386-linux-gnu/cmake/KF5KDE4Support/KF5KDE4SupportConfig.cmake:62 (find_dependency) CMakeLists.txt:56 (find_package) -- Configuring incomplete, errors occurred! See also "/home/david/devel/kf5-development/build/kde-runtime/CMakeFiles/CMakeOutput.log". This is only to let you know, because I don't need kde-runtime to continue developing. I'd like to have a completely built KF5 and (just perhaps) this needs some fixing somewhere. Thank you David Gil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115710: Hide private methods and slots behind the d-pointer in KHistoryComboBox
> On Feb. 15, 2014, 7:38 p.m., David Faure wrote: > > src/khistorycombobox.cpp, line 508 > > <https://git.reviewboard.kde.org/r/115710/diff/1/?file=243810#file243810line508> > > > > infinite recursion! > > > > Sounds like a unittest for reset() should be added. There are two reset() methods, the private slot and the public method, which only calls the private slot. My error was not calling the d-pointer method: void KHistoryComboBox::reset() { Q_D(KHistoryComboBox); d->reset(); } Maybe the private slot could be substituted by the public method (making it a public slot)? By the way, I would need a hint about the unittest that you think that should be added. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115710/#review49866 ------- On Feb. 12, 2014, 11:28 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115710/ > --- > > (Updated Feb. 12, 2014, 11:28 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Hide private methods and slots behind the d-pointer in KHistoryComboBox > > Also: > -Remove header not used > > > Diffs > - > > src/khistorycombobox.h 3667eb4 > src/khistorycombobox.cpp 6f81dda > > Diff: https://git.reviewboard.kde.org/r/115710/diff/ > > > Testing > --- > > It builds. Tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115710: Hide private methods and slots behind the d-pointer in KHistoryComboBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115710/ --- (Updated Feb. 23, 2014, 11:23 p.m.) Review request for KDE Frameworks. Changes --- Address David Faure's comments. Repository: kcompletion Description --- Hide private methods and slots behind the d-pointer in KHistoryComboBox Also: -Remove header not used Diffs (updated) - src/khistorycombobox.h 3667eb4 src/khistorycombobox.cpp 6f81dda Diff: https://git.reviewboard.kde.org/r/115710/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115984: Implement the d-pointer in KCompletionBase as in the other classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115984/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Implement the d-pointer in KCompletionBase as in the other classes Add two methods: setEmitSignals(bool) and setKeybindingsMap(KeyBindingMap) to be called from setDelegate(). Otherwise, it doesn't seem plausible to reach the private member variables like this (the compiler complains): delegate->d->emitsRotationSignals = d->emitsRotationSignals; Now that has become: delegate->setEmitSignals(d->emitsRotationSignals); For coherence, implement the QScopedPointer and the init() method. Diffs - src/kcompletionbase.h 105a6d0 src/kcompletionbase.cpp 66f9398 Diff: https://git.reviewboard.kde.org/r/115984/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116062: Hide private slots in KCompletionBox and modify d-pointer in KCompletionMatches
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116062/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Hide private slots in KCompletionBox and modify d-pointer in KCompletionMatches for coherence with the other KCompletion classes. Diffs - src/kcompletionbox.h 8ed2b00 src/kcompletionbox.cpp 571b02f src/kcompletionmatches.h 481a4b9 src/kcompletionmatches.cpp bc381b1 Diff: https://git.reviewboard.kde.org/r/116062/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115710: Hide private methods and slots behind the d-pointer in KHistoryComboBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115710/ --- (Updated Feb. 25, 2014, 10:50 p.m.) Review request for KDE Frameworks. Changes --- Add unittest. Repository: kcompletion Description --- Hide private methods and slots behind the d-pointer in KHistoryComboBox Also: -Remove header not used Diffs (updated) - autotests/kcombobox_unittest.cpp 49ef721 src/khistorycombobox.h 3667eb4 src/khistorycombobox.cpp 6f81dda Diff: https://git.reviewboard.kde.org/r/115710/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115984: Implement the d-pointer in KCompletionBase as in the other classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115984/#review50892 --- Ping? - David Gil Oliva On Feb. 24, 2014, 12:06 a.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115984/ > --- > > (Updated Feb. 24, 2014, 12:06 a.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Implement the d-pointer in KCompletionBase as in the other classes > > Add two methods: setEmitSignals(bool) and setKeybindingsMap(KeyBindingMap) > to be called from setDelegate(). Otherwise, it doesn't seem plausible > to reach the private member variables like this (the compiler complains): > > delegate->d->emitsRotationSignals = d->emitsRotationSignals; > > Now that has become: > > delegate->setEmitSignals(d->emitsRotationSignals); > > For coherence, implement the QScopedPointer and the init() method. > > > Diffs > - > > src/kcompletionbase.h 105a6d0 > src/kcompletionbase.cpp 66f9398 > > Diff: https://git.reviewboard.kde.org/r/115984/diff/ > > > Testing > --- > > It builds. Tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116064: @deprecated docs for KUrl methods that duplicate QUrl methods
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116064/#review50906 --- src/kdecore/kurl.h <https://git.reviewboard.kde.org/r/116064/#comment35726> I would say: @deprecated since 5.0, use Foo instead. - David Gil Oliva On Feb. 25, 2014, 11:03 p.m., Alex Merry wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116064/ > --- > > (Updated Feb. 25, 2014, 11:03 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kde4support > > > Description > --- > > @deprecated docs for KUrl methods that duplicate QUrl methods > > Some KUrl methods just forward to their QUrl counterparts (or do > .isEmpty() on a QUrl method result); replace the apidox for these with a > note saying what QUrl method should be used instead. > > > Diffs > - > > src/kdecore/kurl.h 4ac893382c76def83f8e6e12698931df243085f9 > > Diff: https://git.reviewboard.kde.org/r/116064/diff/ > > > Testing > --- > > > Thanks, > > Alex Merry > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115984: Implement the d-pointer in KCompletionBase as in the other classes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115984/ --- (Updated Feb. 26, 2014, 9:05 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Implement the d-pointer in KCompletionBase as in the other classes Add two methods: setEmitSignals(bool) and setKeybindingsMap(KeyBindingMap) to be called from setDelegate(). Otherwise, it doesn't seem plausible to reach the private member variables like this (the compiler complains): delegate->d->emitsRotationSignals = d->emitsRotationSignals; Now that has become: delegate->setEmitSignals(d->emitsRotationSignals); For coherence, implement the QScopedPointer and the init() method. Diffs - src/kcompletionbase.h 105a6d0 src/kcompletionbase.cpp 66f9398 Diff: https://git.reviewboard.kde.org/r/115984/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116062: Hide private slots in KCompletionBox and modify d-pointer in KCompletionMatches
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116062/ --- (Updated Feb. 26, 2014, 9:09 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Hide private slots in KCompletionBox and modify d-pointer in KCompletionMatches for coherence with the other KCompletion classes. Diffs - src/kcompletionbox.h 8ed2b00 src/kcompletionbox.cpp 571b02f src/kcompletionmatches.h 481a4b9 src/kcompletionmatches.cpp bc381b1 Diff: https://git.reviewboard.kde.org/r/116062/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115710: Hide private methods and slots behind the d-pointer in KHistoryComboBox
> On Feb. 27, 2014, 8:09 p.m., David Faure wrote: > > src/khistorycombobox.cpp, line 123 > > <https://git.reviewboard.kde.org/r/115710/diff/3/?file=246152#file246152line123> > > > > q->connect is a strange way to call a static method. QObject::connect > > would be usual (if a bit more verbose). IMHO, it isn't actually so strange: Instead of QObject::connect(sender, SIGNAL, receiver, SLOT); it becomes receiver->connect(sender, SIGNAL, SLOT); http://qt-project.org/doc/qt-5/qobject.html#connect-3 Am I right? - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115710/#review51125 ----------- On Feb. 25, 2014, 10:50 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115710/ > --- > > (Updated Feb. 25, 2014, 10:50 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Hide private methods and slots behind the d-pointer in KHistoryComboBox > > Also: > -Remove header not used > > > Diffs > - > > autotests/kcombobox_unittest.cpp 49ef721 > src/khistorycombobox.h 3667eb4 > src/khistorycombobox.cpp 6f81dda > > Diff: https://git.reviewboard.kde.org/r/115710/diff/ > > > Testing > --- > > It builds. Tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115710: Hide private methods and slots behind the d-pointer in KHistoryComboBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115710/ --- (Updated March 4, 2014, 9:38 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Hide private methods and slots behind the d-pointer in KHistoryComboBox Also: -Remove header not used Diffs - autotests/kcombobox_unittest.cpp 49ef721 src/khistorycombobox.h 3667eb4 src/khistorycombobox.cpp 6f81dda Diff: https://git.reviewboard.kde.org/r/115710/diff/ Testing --- It builds. Tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116684: Improve API in KCompletionBase
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116684/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Improve API in KCompletionBase getKeyBindings -> keyBindingMap getKeyBinding -> keyBinding NOTICE: The following part is commented out because it gives me errors and I need some help to find out what actually happens: /** * @deprecated since 5.0, use keyBindingMap instead */ #ifndef KDE_NO_DEPRECATED KCOMPLETION_DEPRECATED KeyBindingMap getKeyBindings() const { return keyBindingMap(); } #endif Error: In file included from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.cpp:21:0: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h: In member function ‘KCompletionBase::KeyBindingMap KCompletionBase::getKeyBindings() const’: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:336:5: error: return type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ is incomplete /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:337:30: error: invalid use of incomplete type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ In file included from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qvarlengtharray.h:45:0, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qmetatype.h:48, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qobject.h:56, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/QObject:1, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletion.h:25, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:23, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.cpp:21: /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qcontainerfwd.h:54:37: error: declaration of ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ In file included from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.h:27:0, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.cpp:23: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h: In member function ‘KCompletionBase::KeyBindingMap KCompletionBase::getKeyBindings() const’: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:336:5: error: return type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ is incomplete /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:337:30: error: invalid use of incomplete type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ In file included from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qvarlengtharray.h:45:0, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qmetatype.h:48, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qobject.h:56, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/QObject:1, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletion.h:25, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.h:25, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.cpp:23: /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qcontainerfwd.h:54:37: error: declaration of ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ make[2]: *** [src/CMakeFiles/KF5Completion.dir/kcompletionbase.cpp.o] Error 1 make[2]: *** Waiting for unfinished jobs Diffs - src/kcompletionbase.h 1dedd2d src/kcompletionbase.cpp 1e251d1 src/klineedit.cpp ae15093 Diff: https://git.reviewboard.kde.org/r/116684/diff/ Testing --- It compiles and (auto)tests pass without the conflicting part (to be uncommented). Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KF5 Update Meeting Minutes 2014-w11
Hi! 2014-03-11 16:33 GMT+01:00 Kevin Ottens : > Hello everyone, > > This is the minutes of the Week 10 KF5 meeting. As usual it has been held > on > #kde-devel at 4pm Paris time. > > Were present: afiestas, agateau, alexmerry, dfaure, mgraesslin, notmart, > sebas, teo, tosky and myself. > > Announcement: > * Beta 1 will be tagged on March 28 instead of April 1st as planned > earlier; > Why is that? Are we in a hurry? Is the work in all the frameworks so advanced that we can advance the tagging? BTW, is it allowed to deprecate methods and add new methods from beta1 on? (please forgive my ignorance, I didn't find out what "SIC change" means). And, again BTW, the date here isn't updated: http://community.kde.org/Frameworks/Epics Cheers, David Gil > * Two tasks up for adoption on >http://community.kde.org/Frameworks/Epics/KF5.0_Release_Preparation > * Please nail them down before they turn into release blockers! > > * afiestas has been fixing things around; > * he's also working with apol on the kde-runtime split; > > * agateau has been doing mainly build fixes downstream; > * he's also been working on some of the cmake files and on api.kde.org; > * he then fixed a bug in Doxygen waiting for review; > * last but not least he moved the doc policy to community.kde.org; > > * alexmerry is waiting for the maintainers of kdnssd before proceeding > with > the rename, timeout on friday; > * he's also tracking bugs in kimageformats showing up on non-x86_64 > architectures; > * he also proposed ECMFindModuleHelpers macros for ECM and investigates on > how to generate docs; > > * dfaure is optimizing KConfig to minimize reparsing; > * he also plans to improve the KUrl -> QUrl porting scripts; > > * mgraesslin has two open review requests for KWindowSystem which will be > merged after Plasma Next Alpha tagging; > > * notmart is also waiting for Plasma Next Alpha tagging to move QML > imports > in their respective frameworks; > > * sebas is working on kglobalaccel; > * he also has a pending patch to make oxygen the default font; > > * teo ported kwalletmanager to KF5; > * he still has to port away from kde4support; > > * tosky is fixing the fallout of doctools version bump; > * he still has one pending review, still open because it waits on windows > or > mac savvy people feedback: https://git.reviewboard.kde.org/r/116670/ ; > * he also plans to remove all htdig references from kdoctools and > khelpcenter; > > * ervin has been in review frenzy mode; > > If you got questions, feel free to ask. > > Regards. > -- > Kévin Ottens, http://ervin.ipsquad.net > > KDAB - proud supporter of KDE, http://www.kdab.com > > > ___ > Kde-frameworks-devel mailing list > Kde-frameworks-devel@kde.org > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116747: Clean up KCompletionBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() -> cancelled (private method) -Deprecate sizeAndPosition() --> resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KF5 Update Meeting Minutes 2014-w11
El 11/03/2014 22:58, "Martin Klapetek" escribió: > > On Tue, Mar 11, 2014 at 10:19 PM, David Gil Oliva wrote: >>> >>> >>> Announcement: >>> * Beta 1 will be tagged on March 28 instead of April 1st as planned earlier; >> >> >> Why is that? Are we in a hurry? Is the work in all the frameworks so advanced that we can advance the tagging? > > > It's because David Faure, who will do the packages, is travelling on April 1 iirc, so it was moved before his travels. The actual release date should still be April 1st afaik. Thanks! > Cheers > -- > Martin Klapetek | KDE Developer > > ___ > Kde-frameworks-devel mailing list > Kde-frameworks-devel@kde.org > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
> On March 11, 2014, 11:17 p.m., Aleix Pol Gonzalez wrote: > > src/kcompletionbox.h, line 228 > > <https://git.reviewboard.kde.org/r/116747/diff/1/?file=253404#file253404line228> > > > > I wouldn't leave the implementation here. Move it to the .cpp file, > > this way it can be changed in the future, if it's required for some reason. > > > > Also there's a typo in the method name. Alex Merry inlined deprecated methods in https://git.reviewboard.kde.org/r/116012 , so I thought that it was the way to go... On March 11, 2014, 11:17 p.m., David Gil Oliva wrote: > > Have you looked through the uses of the "un-slotted" methods? > > (lxr.kde.org). Maybe there's a reason for that... :/ Maybe I'm totally wrong, but I can't imagine any way that a getter can be useful as a slot. connect (widget, SLOT(valueChanged(), completionBox, SIGNAL(isTabHandling()); Does it make sense?¿?:-/ - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52703 --- On March 11, 2014, 10:32 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116747/ > --- > > (Updated March 11, 2014, 10:32 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Clean up KCompletionBox > > -canceled() -> cancelled (private method) > -Deprecate sizeAndPosition() --> resizeAndReposition() > -Remove old comments and commented-out code > -Move some slots to be normal methods, since they don't seem to be able to > work as slots. > > > Diffs > - > > src/kcompletionbox.h 09b7527 > src/kcompletionbox.cpp 92e87b3 > > Diff: https://git.reviewboard.kde.org/r/116747/diff/ > > > Testing > --- > > It builds and tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116012: Inline deprecated methods, and silence false positives in tests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116012/#review52727 --- src/karchive.h <https://git.reviewboard.kde.org/r/116012/#comment37188> Sorry, I didn't see it. I think it's better @deprecated since 5.0, use blah, blah I know it's been already shipped, but it still can be fixed :-) - David Gil Oliva On Feb. 28, 2014, 12:58 a.m., Alex Merry wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116012/ > --- > > (Updated Feb. 28, 2014, 12:58 a.m.) > > > Review request for KDE Frameworks. > > > Repository: karchive > > > Description > --- > > Four commits (in git log order): > > > Simplify autotests CMakeLists.txt > > > Separate out tests of deprecated functions to their own file > > This allows us to disable deprecation warnings just for that file. > > > Inline deprecated methods > > This both documents how to replace them in the headers and ensures that > enabling or disabling deprecated functions does not affect the ABI. > > > Use KARCHIVE_NO_DEPRECATED instead of KDE_NO_DEPRECATED > > > Diffs > - > > autotests/CMakeLists.txt 1da853fe49ec6fca40ccf417db788340124b41e5 > autotests/deprecatedtest.cpp PRE-CREATION > autotests/karchivetest.cpp 6d6b5848ce9add84736968dd13f529711f895493 > src/karchive.h d4209cf334190dda735fcb4687fa102a4e7a73cd > src/karchive.cpp a25b85e41618d1f7ae62e7e41966afe1a8aaef8e > src/kfilterdev.h 4a35879a15992d19378c7e9baa9b206e187713fd > src/kfilterdev.cpp 60660a0c08922ee0f4ddaf4ad4d1adf0e7f1c5a3 > > Diff: https://git.reviewboard.kde.org/r/116012/diff/ > > > Testing > --- > > Builds, tests pass. No deprecation warnings (before there were two in the > tests). > > > Thanks, > > Alex Merry > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
> On March 11, 2014, 11:17 p.m., Aleix Pol Gonzalez wrote: > > src/kcompletionbox.h, line 228 > > <https://git.reviewboard.kde.org/r/116747/diff/1/?file=253404#file253404line228> > > > > I wouldn't leave the implementation here. Move it to the .cpp file, > > this way it can be changed in the future, if it's required for some reason. > > > > Also there's a typo in the method name. > > David Gil Oliva wrote: > Alex Merry inlined deprecated methods in > https://git.reviewboard.kde.org/r/116012 , so I thought that it was the way > to go... > > Alex Merry wrote: > Well, there's a balance to be struck: putting them in the header ensures > there is no runtime cost to programs that don't use the deprecated methods, > as the code should be optimised away, that the library is always > binary-compatible even if you compile it with deprecated code disabled > (*_NO_DEPRECATED) and makes the header code document how to replace existing > calls. The downsides are an inability to fix the code later and an inability > to access members of a private d-pointer class. Neither of those are an > issue here, as we're just renaming the method. > > tl;dr: I disagree with Aleix, and think it should stay in the header. > > Oh, and Aleix: could you please select the whole method when you're doing > a comment like this, rather than just the first line? Otherwise it's a pain > to see what you're referring to. Thanks :-) > > Aleix Pol Gonzalez wrote: > Well, I wouldn't bother about runtime penalty given that it's deprecated > and we shouldn't be using it anyway. Also we can't make assumptions on how > things are going to be optimized. > > But it's ok, I don't think it's worth discussing further, I doubt this is > going to be a problem in the future. > > Alex Merry wrote: > I mean the runtime penalty for things that *don't* use it. If it's > header-only, it doesn't go in the library, so there is no load-time > symbol-lookup penalty, and no code-size penalty. Admittedly, both of those > are probably negligible... Thank you both of you for the useful hints. With this little discussion I have learned some criteria to decide what to do now and in the future. On March 11, 2014, 11:17 p.m., David Gil Oliva wrote: > > Have you looked through the uses of the "un-slotted" methods? > > (lxr.kde.org). Maybe there's a reason for that... :/ > > David Gil Oliva wrote: > Maybe I'm totally wrong, but I can't imagine any way that a getter can be > useful as a slot. > > connect (widget, SLOT(valueChanged(), completionBox, > SIGNAL(isTabHandling()); > > Does it make sense?¿?:-/ > > Alex Merry wrote: > Non-void slots are only useful if they work as a slot (ie: have some sort > of side-effect) and might be called directly or with > QMetaObject::invokeMethod(). If the method is const (like a getter), there's > no point having it a slot at all; if you want to be able to use it with > invokeMethod, you can just make it Q_INVOKABLE. > > Aleix Pol Gonzalez wrote: > Well, having const methods as slots doesn't make sense indeed, if it's > not for exposing on the meta object system, that's why I said you could do a > fast lxr. I just did, didn't find anything relevant. Thank you for looking it up :-) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52703 --- On March 11, 2014, 10:32 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116747/ > --- > > (Updated March 11, 2014, 10:32 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Clean up KCompletionBox > > -canceled() -> cancelled (private method) > -Deprecate sizeAndPosition() --> resizeAndReposition() > -Remove old comments and commented-out code > -Move some slots to be normal methods, since they don't seem to be able to > work as slots. > > > Diffs > - > > src/kcompletionbox.h 09b7527 > src/kcompletionbox.cpp 92e87b3 > > Diff: https://git.reviewboard.kde.org/r/116747/diff/ > > > Testing > --- > > It builds and tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 12, 2014, 10:54 p.m.) Review request for KDE Frameworks. Changes --- Address Alex and Aleix suggestions. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() -> cancelled (private method) -Deprecate sizeAndPosition() --> resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs (updated) - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 12, 2014, 10:57 p.m.) Review request for KDE Frameworks. Changes --- Fix non-commited change. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() -> cancelled (private method) -Deprecate sizeAndPosition() --> resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs (updated) - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116684: Improve API in KCompletionBase
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116684/ --- (Updated March 12, 2014, 11:06 p.m.) Review request for KDE Frameworks. Changes --- Change include to header, following Kévin's advice. Repository: kcompletion Description --- Improve API in KCompletionBase getKeyBindings -> keyBindingMap getKeyBinding -> keyBinding NOTICE: The following part is commented out because it gives me errors and I need some help to find out what actually happens: /** * @deprecated since 5.0, use keyBindingMap instead */ #ifndef KDE_NO_DEPRECATED KCOMPLETION_DEPRECATED KeyBindingMap getKeyBindings() const { return keyBindingMap(); } #endif Error: In file included from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.cpp:21:0: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h: In member function ‘KCompletionBase::KeyBindingMap KCompletionBase::getKeyBindings() const’: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:336:5: error: return type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ is incomplete /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:337:30: error: invalid use of incomplete type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ In file included from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qvarlengtharray.h:45:0, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qmetatype.h:48, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qobject.h:56, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/QObject:1, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletion.h:25, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:23, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.cpp:21: /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qcontainerfwd.h:54:37: error: declaration of ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ In file included from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.h:27:0, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.cpp:23: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h: In member function ‘KCompletionBase::KeyBindingMap KCompletionBase::getKeyBindings() const’: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:336:5: error: return type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ is incomplete /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:337:30: error: invalid use of incomplete type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ In file included from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qvarlengtharray.h:45:0, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qmetatype.h:48, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qobject.h:56, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/QObject:1, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletion.h:25, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.h:25, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.cpp:23: /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qcontainerfwd.h:54:37: error: declaration of ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ make[2]: *** [src/CMakeFiles/KF5Completion.dir/kcompletionbase.cpp.o] Error 1 make[2]: *** Waiting for unfinished jobs Diffs (updated) - src/kcompletionbase.h 1dedd2d src/kcompletionbase.cpp 1e251d1 src/klineedit.cpp ae15093 Diff: https://git.reviewboard.kde.org/r/116684/diff/ Testing --- It compiles and (auto)tests pass without the conflicting part (to be uncommented). Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 13, 2014, 10:13 p.m.) Review request for KDE Frameworks. Changes --- Restore and rephrase comment in kcompletionbox.cpp, following suggestion of Alex Merry. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() -> cancelled (private method) -Deprecate sizeAndPosition() --> resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs (updated) - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116792: Fix deprecation warning in KComboBox and fix all KDE_NO_DEPRECATED
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116792/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Fix deprecation warning in KComboBox and fix all KDE_NO_DEPRECATED There are other warnings related to the deprecated KComboBox::setUrlDropsEnabled, which calls KLineEdit::setUrlDropsEnabled (also deprecated). I tried to solve it by copying the KLineEdit::setUrlDropsEnabled contents into KComboBox::setUrlDropsEnabled, but it contains private variables which it cannot see. Any hint?? Diffs - autotests/klineedit_unittest.cpp b84e126 src/kcombobox.h 32dc3e9 src/kcombobox.cpp 7195b3e src/kcompletion.h 387a05e src/klineedit.h c7c46b5 src/klineedit.cpp ae15093 Diff: https://git.reviewboard.kde.org/r/116792/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
> On March 14, 2014, 9:18 a.m., David Faure wrote: > > src/kcompletionbox.h, line 229 > > <https://git.reviewboard.kde.org/r/116747/diff/4/?file=253685#file253685line229> > > > > That two 'd's too many. Aaargh!! I added those two d's for refactoring and then I forgot to change it (actually I forgot two times, because one of the reviewers already told me)... The next time I will use something that doesn't compile. I'm sorry, I program at night and lately I'm usually very tired (I just had a baby three months ago). - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/#review52928 ----------- On March 13, 2014, 10:13 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116747/ > --- > > (Updated March 13, 2014, 10:13 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Clean up KCompletionBox > > -canceled() -> cancelled (private method) > -Deprecate sizeAndPosition() --> resizeAndReposition() > -Remove old comments and commented-out code > -Move some slots to be normal methods, since they don't seem to be able to > work as slots. > > > Diffs > - > > src/kcompletionbox.h 09b7527 > src/kcompletionbox.cpp 92e87b3 > > Diff: https://git.reviewboard.kde.org/r/116747/diff/ > > > Testing > --- > > It builds and tests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 14, 2014, 8:46 p.m.) Review request for KDE Frameworks. Changes --- Hopefully everything is fixed now. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() -> cancelled (private method) -Deprecate sizeAndPosition() --> resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs (updated) - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116747: Clean up KCompletionBox
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116747/ --- (Updated March 18, 2014, 10:25 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Clean up KCompletionBox -canceled() -> cancelled (private method) -Deprecate sizeAndPosition() --> resizeAndReposition() -Remove old comments and commented-out code -Move some slots to be normal methods, since they don't seem to be able to work as slots. Diffs - src/kcompletionbox.h 09b7527 src/kcompletionbox.cpp 92e87b3 Diff: https://git.reviewboard.kde.org/r/116747/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116792: Fix deprecation warning in KComboBox and fix all KDE_NO_DEPRECATED
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116792/ --- (Updated March 18, 2014, 10:28 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Fix deprecation warning in KComboBox and fix all KDE_NO_DEPRECATED There are other warnings related to the deprecated KComboBox::setUrlDropsEnabled, which calls KLineEdit::setUrlDropsEnabled (also deprecated). I tried to solve it by copying the KLineEdit::setUrlDropsEnabled contents into KComboBox::setUrlDropsEnabled, but it contains private variables which it cannot see. Any hint?? Diffs - autotests/klineedit_unittest.cpp b84e126 src/kcombobox.h 32dc3e9 src/kcombobox.cpp 7195b3e src/kcompletion.h 387a05e src/klineedit.h c7c46b5 src/klineedit.cpp ae15093 Diff: https://git.reviewboard.kde.org/r/116792/diff/ Testing --- It builds and tests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116684: Improve API in KCompletionBase
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116684/ --- (Updated March 18, 2014, 10:32 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kcompletion Description --- Improve API in KCompletionBase getKeyBindings -> keyBindingMap getKeyBinding -> keyBinding NOTICE: The following part is commented out because it gives me errors and I need some help to find out what actually happens: /** * @deprecated since 5.0, use keyBindingMap instead */ #ifndef KDE_NO_DEPRECATED KCOMPLETION_DEPRECATED KeyBindingMap getKeyBindings() const { return keyBindingMap(); } #endif Error: In file included from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.cpp:21:0: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h: In member function ‘KCompletionBase::KeyBindingMap KCompletionBase::getKeyBindings() const’: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:336:5: error: return type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ is incomplete /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:337:30: error: invalid use of incomplete type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ In file included from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qvarlengtharray.h:45:0, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qmetatype.h:48, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qobject.h:56, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/QObject:1, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletion.h:25, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:23, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.cpp:21: /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qcontainerfwd.h:54:37: error: declaration of ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ In file included from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.h:27:0, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.cpp:23: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h: In member function ‘KCompletionBase::KeyBindingMap KCompletionBase::getKeyBindings() const’: /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:336:5: error: return type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ is incomplete /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletionbase.h:337:30: error: invalid use of incomplete type ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ In file included from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qvarlengtharray.h:45:0, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qmetatype.h:48, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qobject.h:56, from /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/QObject:1, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcompletion.h:25, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.h:25, from /home/david/devel/kf5-development/frameworks/kcompletion/src/kcombobox.cpp:23: /opt/Qt5.2.0-KF5/5.2.0/gcc/include/QtCore/qcontainerfwd.h:54:37: error: declaration of ‘KCompletionBase::KeyBindingMap {aka class QMap >}’ make[2]: *** [src/CMakeFiles/KF5Completion.dir/kcompletionbase.cpp.o] Error 1 make[2]: *** Waiting for unfinished jobs Diffs - src/kcompletionbase.h 1dedd2d src/kcompletionbase.cpp 1e251d1 src/klineedit.cpp ae15093 Diff: https://git.reviewboard.kde.org/r/116684/diff/ Testing --- It compiles and (auto)tests pass without the conflicting part (to be uncommented). Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116886: Refactor private variables of KCompletion
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116886/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Refactor private variables of KCompletion Also: reorder variables declaration to avoid padding Diffs - src/kcompletion_p.h e3fad26 src/kcompletion.cpp 7396029 Diff: https://git.reviewboard.kde.org/r/116886/diff/ Testing --- It builds. Autotests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116886: Refactor private variables of KCompletion
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116886/ --- (Updated March 18, 2014, 11:01 p.m.) Review request for KDE Frameworks. Changes --- Forgot to commit fix. Repository: kcompletion Description --- Refactor private variables of KCompletion Also: reorder variables declaration to avoid padding Diffs (updated) - src/kcompletion_p.h e3fad26 src/kcompletion.cpp 7396029 Diff: https://git.reviewboard.kde.org/r/116886/diff/ Testing --- It builds. Autotests pass. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 116887: Undeprecate setUrlDropsEnabled(bool) in KComboBox and KLineEdit
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116887/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Undeprecate setUrlDropsEnabled(bool) in KComboBox and KLineEdit Apidox say it to be deprecated and use installEventFilter with a LineEditUrlDropEventFilter instead. But that's precisely what setUrlDropsEnabled does, so I don't see the point of making the user do it when there's already a one-line method for that. Diffs - src/klineedit.h 76a1f01 src/kcombobox.h eea930d src/kcombobox.cpp 30edc1b Diff: https://git.reviewboard.kde.org/r/116887/diff/ Testing --- It builds. The compiler warnings about this deprecation disappear. Thanks, David Gil Oliva ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 116886: Refactor private variables of KCompletion
> On March 19, 2014, 11:55 a.m., Frank Reininghaus wrote: > > src/kcompletion_p.h, line 338 > > <https://git.reviewboard.kde.org/r/116886/diff/2/?file=255262#file255262line338> > > > > This is not strictly related to your changes, but it looks a bit > > unusual to have one plain bool and two bool bitfields next to each other. > > Making all bools a bitfield won't make much difference now though because > > the compiler will always use more memory for this class in order to > > preserve the 4-byte or 8-byte alignment. > > > > Another alignment-related issue is caused by your patch though: on a > > 64-bit system, moving the int member away from the bools will most likely > > increase sizeof(KCompletionPrivate) by 8 bytes because the compiler will > > add some padding to both in order to preserve the alignment of the > > neigbouring pointers. > > > > It might not make a big difference because it's probably unusual to > > create thousands of KCompletionPrivate instances, but still, it seems > > unnecessary. > > > > If one really wanted to make use of bitfields to save memory here, one > > could make 'order' a bitfield and move it next to the bools. "This is not strictly related to your changes, but it looks a bit unusual to have one plain bool and two bool bitfields next to each other." I know, but I can't change it without changing the method 'void findAllCompletions(const QString &, KCompletionMatchesWrapper *matches, bool &hasMultipleMatches) const' in kcompletion_p.h. The boolean parameter is passed as a reference and therefore the compiler complains it can't be a bitfield. That method is doomed to be moved elsewhere. Then I will change that bool to bitfield. "If one really wanted to make use of bitfields to save memory here, one could make 'order' a bitfield and move it next to the bools." Ok, let's see if I do it well :-) - David ------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116886/#review53410 --- On March 18, 2014, 11:01 p.m., David Gil Oliva wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/116886/ > --- > > (Updated March 18, 2014, 11:01 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcompletion > > > Description > --- > > Refactor private variables of KCompletion > > Also: reorder variables declaration to avoid padding > > > Diffs > - > > src/kcompletion_p.h e3fad26 > src/kcompletion.cpp 7396029 > > Diff: https://git.reviewboard.kde.org/r/116886/diff/ > > > Testing > --- > > It builds. Autotests pass. > > > Thanks, > > David Gil Oliva > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel