Review Request 122086: Add new method KWindowSystem::icons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs - src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b Diff: https://git.reviewboard.kde.org/r/122086/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: OSX/CI: ark fails to build on branch frameworks
Marko Käning mk-li...@email.de writes: Hi Raphael, On 14 Jan 2015, at 22:11 , Raphael Kubo da Costa rak...@freebsd.org wrote: Is the full build log available somewhere, preferably with `make VERBOSE=1'? This really smells like OS X having an older ( 3.0) version of libarchive in its base system, a more recent version being installed by `port' (found by CMake) and the older version being picked up for linking. yes, that’s correct. The system’s libarchive is being found. Will send details as PM. So to summarize: my hunch was correct and a libarchive shipped by OS X itself in /usr/lib/libarchive.dynlib was being picked up by CMake (why it got recognized as being recent enough I do not know) even though there was a more recent libarchive version installed by MacPorts into /opt/local. Since CMake itself was installed into a different prefix, it wasn't looking at /opt/local by default, so we've fixed this by setting CMAKE_PREFIX_PATH to that location. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122086: Add new method KWindowSystem::icons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/#review74132 --- src/kwindowsystem_p.h https://git.reviewboard.kde.org/r/122086/#comment51491 this is pure virtual and you haven't provided a win or mac implementation that will break the build? src/kwindowsystem_x11.cpp https://git.reviewboard.kde.org/r/122086/#comment51492 this className code is very different from the one in pixmap; either this won't work, or the one in pixmap can be simplified in the same way. (I suspect the latter) - David Edmundson On Jan. 16, 2015, 10:01 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 16, 2015, 10:01 a.m.) Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs - src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b Diff: https://git.reviewboard.kde.org/r/122086/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122086: Add new method KWindowSystem::icons
On Jan. 16, 2015, 12:47 p.m., David Edmundson wrote: src/kwindowsystem_p.h, line 48 https://git.reviewboard.kde.org/r/122086/diff/1/?file=342288#file342288line48 this is pure virtual and you haven't provided a win or mac implementation that will break the build? no, there are no win or mac implementations yet :-( Nothing to break. On Jan. 16, 2015, 12:47 p.m., David Edmundson wrote: src/kwindowsystem_x11.cpp, line 798 https://git.reviewboard.kde.org/r/122086/diff/1/?file=342290#file342290line798 this className code is very different from the one in pixmap; either this won't work, or the one in pixmap can be simplified in the same way. (I suspect the latter) yes in deed, the other one can be simplified and I also intend to do that in another review request. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/#review74132 --- On Jan. 16, 2015, 11:01 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 16, 2015, 11:01 a.m.) Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs - src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b Diff: https://git.reviewboard.kde.org/r/122086/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build is back to stable : plasma-framework_master_qt5 » All,LINBUILDER #955
See http://build.kde.org/job/plasma-framework_master_qt5/Variation=All,label=LINBUILDER/955/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121448: Introduce ECMAddAppIcon.
On Dez. 12, 2014, 3:08 nachm., Alex Merry wrote: modules/ECMAddAppIcon.cmake, line 15 https://git.reviewboard.kde.org/r/121448/diff/1/?file=332663#file332663line15 Would this actually work? The code looks to me like pattern_rx would just be the filename again, so fn would be empty, and the icon would never be appended to _list. Ralf Habacker wrote: The doc in the origin cmake macro file seems to be out of sync with the official doc located in FindKDE4Internals.cmake, which is: --- # adds an application icon to target source list. # Make sure you have a 128x128 icon, or the icon won't display on Mac OS X. # Mac OSX notes : the application icon is added to a Mac OS X bundle so that Finder and friends show the right thing. # Win32 notes: the application icon(s) are compiled into the application # There is some workaround in kde4_add_kdeinit_executable to make it possible for those applications as well. # Parameters: # SRCS_VAR - specifies the list of source files # pattern - regular expression for searching application icons # Example: KDE4_ADD_APP_ICON( myapp_SOURCES pics/cr*-myapp.png) # Example: KDE4_ADD_APP_ICON( myapp_KDEINIT_SRCS icons/oxygen/*/apps/myapp.png) --- normal file mode is not documentated and does not work Alex Merry wrote: I would still much prefer an API to match ecm_add_icons, as I noted at the start of this review. Would you be willing to do that? If not, I can probably find some time over Christmas to look at it. Ralf Habacker wrote: Just one question: As it turns out that the macro used in KDE4 only supports a simple pattern style and no single files, what is different or has been extended with KDE5 to require this now ? Alex Merry wrote: Best CMake practices include listing all source files in the CMakeLists.txt file, rather than globbing them. That way, CMake knows when a file has been added (or removed), because the CMakeLists.txt file will change, and so it will automatically reconfigure when you run `make`. The icon installation function (`ecm_add_icons`) was changed to support and encourage that approach, and I think this function should also change to match it. Additionally, I would like consistency between interfaces. Having ecm_add_app_icon behave completely differently to ecm_add_icons just makes things that much harder for users of e-c-m. They should use the same file naming scheme (or compatible schemes, at least), and similar arguments, in so far as they need similar information. I'll admit it makes porting a little harder, but it should be possible to create a porting script without too much difficulty. Jeremy Whiting wrote: Ralf any updates on this patch? @Alex, you wrote The icon installation function (ecm_add_icons) you are refering to ecm_install_icons ? was changed to support and encourage that approach, and I think this function should also change to match it. What could be matched ? As far as I can see only the ICONS parameter, all other parameter do not match by design. :-( Merging the ICONS parameter with recent ecm_add_app_icon parameter list will have the following syntax: 1. ecm_add_app_icon(source_var ICONS icon [icon [...]]) the additional mentioned syntax forms are not supported by ecm_install_icons so we are free to choose what we like. According to http://www.cmake.org/cmake/help/v3.0/command/install.html the mentioned term 'PATTERN' only supports '*' at the end of files and therefore does not fit into the recent implementation. So the term 'GLOB' which is defined in the cmake file command http://www.cmake.org/cmake/help/v3.0/command/file.html remains and fit into what is implemented in the patch 2. ecm_add_app_icons(source_var GLOB icon-glob-expression) [1] and 3.ecm_add_app_icon(source_var icon-glob-expression) [1] because ecm_install_icons also have a compatibility mode to support old style parameter list: From https://projects.kde.org/projects/kdesupport/extra-cmake-modules/repository/revisions/master/entry/modules/ECMInstallIcons.cmake An old form of arguments will also be accepted:: # ecm_install_icons(icon_install_dir [l10n_code]) syntax 3 is what is already implemented by the recent patch. Can we agree ? @Jeremey: 1. Syntax 3 is already implemented and need only a few spelling fixed before commit. 2. Currently I do not have any access to a Mac/OSX development machine, so I can only cover the windows part. Is there anyone having access to a Mac OSX development machine ? 3. If not I would remove the Mac OSX support for now. - Ralf --- This is an automatically generated e-mail. To reply, visit:
Re: Review Request 121448: Introduce ECMAddAppIcon.
On Dez. 12, 2014, 3:08 nachm., Alex Merry wrote: modules/ECMAddAppIcon.cmake, line 15 https://git.reviewboard.kde.org/r/121448/diff/1/?file=332663#file332663line15 Would this actually work? The code looks to me like pattern_rx would just be the filename again, so fn would be empty, and the icon would never be appended to _list. Ralf Habacker wrote: The doc in the origin cmake macro file seems to be out of sync with the official doc located in FindKDE4Internals.cmake, which is: --- # adds an application icon to target source list. # Make sure you have a 128x128 icon, or the icon won't display on Mac OS X. # Mac OSX notes : the application icon is added to a Mac OS X bundle so that Finder and friends show the right thing. # Win32 notes: the application icon(s) are compiled into the application # There is some workaround in kde4_add_kdeinit_executable to make it possible for those applications as well. # Parameters: # SRCS_VAR - specifies the list of source files # pattern - regular expression for searching application icons # Example: KDE4_ADD_APP_ICON( myapp_SOURCES pics/cr*-myapp.png) # Example: KDE4_ADD_APP_ICON( myapp_KDEINIT_SRCS icons/oxygen/*/apps/myapp.png) --- normal file mode is not documentated and does not work Alex Merry wrote: I would still much prefer an API to match ecm_add_icons, as I noted at the start of this review. Would you be willing to do that? If not, I can probably find some time over Christmas to look at it. Ralf Habacker wrote: Just one question: As it turns out that the macro used in KDE4 only supports a simple pattern style and no single files, what is different or has been extended with KDE5 to require this now ? Alex Merry wrote: Best CMake practices include listing all source files in the CMakeLists.txt file, rather than globbing them. That way, CMake knows when a file has been added (or removed), because the CMakeLists.txt file will change, and so it will automatically reconfigure when you run `make`. The icon installation function (`ecm_add_icons`) was changed to support and encourage that approach, and I think this function should also change to match it. Additionally, I would like consistency between interfaces. Having ecm_add_app_icon behave completely differently to ecm_add_icons just makes things that much harder for users of e-c-m. They should use the same file naming scheme (or compatible schemes, at least), and similar arguments, in so far as they need similar information. I'll admit it makes porting a little harder, but it should be possible to create a porting script without too much difficulty. Jeremy Whiting wrote: Ralf any updates on this patch? Ralf Habacker wrote: @Alex, you wrote The icon installation function (ecm_add_icons) you are refering to ecm_install_icons ? was changed to support and encourage that approach, and I think this function should also change to match it. What could be matched ? As far as I can see only the ICONS parameter, all other parameter do not match by design. :-( Merging the ICONS parameter with recent ecm_add_app_icon parameter list will have the following syntax: 1. ecm_add_app_icon(source_var ICONS icon [icon [...]]) the additional mentioned syntax forms are not supported by ecm_install_icons so we are free to choose what we like. According to http://www.cmake.org/cmake/help/v3.0/command/install.html the mentioned term 'PATTERN' only supports '*' at the end of files and therefore does not fit into the recent implementation. So the term 'GLOB' which is defined in the cmake file command http://www.cmake.org/cmake/help/v3.0/command/file.html remains and fit into what is implemented in the patch 2. ecm_add_app_icons(source_var GLOB icon-glob-expression) [1] and 3.ecm_add_app_icon(source_var icon-glob-expression) [1] because ecm_install_icons also have a compatibility mode to support old style parameter list: From https://projects.kde.org/projects/kdesupport/extra-cmake-modules/repository/revisions/master/entry/modules/ECMInstallIcons.cmake An old form of arguments will also be accepted:: # ecm_install_icons(icon_install_dir [l10n_code]) syntax 3 is what is already implemented by the recent patch. Can we agree ? @Jeremey: 1. Syntax 3 is already implemented and need only a few spelling fixed before commit. 2. Currently I do not have any access to a Mac/OSX development machine, so I can only cover the windows part. Is there anyone having access to a Mac OSX development
Re: Review Request 122086: Add new method KWindowSystem::icons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 16, 2015, 1:55 p.m.) Review request for KDE Frameworks. Changes --- add default value for flags. Same as for ::icon without flags Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs (updated) - src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122086/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121448: Introduce ECMAddAppIcon.
On Dec. 12, 2014, 7:08 a.m., Alex Merry wrote: modules/ECMAddAppIcon.cmake, line 15 https://git.reviewboard.kde.org/r/121448/diff/1/?file=332663#file332663line15 Would this actually work? The code looks to me like pattern_rx would just be the filename again, so fn would be empty, and the icon would never be appended to _list. Ralf Habacker wrote: The doc in the origin cmake macro file seems to be out of sync with the official doc located in FindKDE4Internals.cmake, which is: --- # adds an application icon to target source list. # Make sure you have a 128x128 icon, or the icon won't display on Mac OS X. # Mac OSX notes : the application icon is added to a Mac OS X bundle so that Finder and friends show the right thing. # Win32 notes: the application icon(s) are compiled into the application # There is some workaround in kde4_add_kdeinit_executable to make it possible for those applications as well. # Parameters: # SRCS_VAR - specifies the list of source files # pattern - regular expression for searching application icons # Example: KDE4_ADD_APP_ICON( myapp_SOURCES pics/cr*-myapp.png) # Example: KDE4_ADD_APP_ICON( myapp_KDEINIT_SRCS icons/oxygen/*/apps/myapp.png) --- normal file mode is not documentated and does not work Alex Merry wrote: I would still much prefer an API to match ecm_add_icons, as I noted at the start of this review. Would you be willing to do that? If not, I can probably find some time over Christmas to look at it. Ralf Habacker wrote: Just one question: As it turns out that the macro used in KDE4 only supports a simple pattern style and no single files, what is different or has been extended with KDE5 to require this now ? Alex Merry wrote: Best CMake practices include listing all source files in the CMakeLists.txt file, rather than globbing them. That way, CMake knows when a file has been added (or removed), because the CMakeLists.txt file will change, and so it will automatically reconfigure when you run `make`. The icon installation function (`ecm_add_icons`) was changed to support and encourage that approach, and I think this function should also change to match it. Additionally, I would like consistency between interfaces. Having ecm_add_app_icon behave completely differently to ecm_add_icons just makes things that much harder for users of e-c-m. They should use the same file naming scheme (or compatible schemes, at least), and similar arguments, in so far as they need similar information. I'll admit it makes porting a little harder, but it should be possible to create a porting script without too much difficulty. Jeremy Whiting wrote: Ralf any updates on this patch? Ralf Habacker wrote: @Alex, you wrote The icon installation function (ecm_add_icons) you are refering to ecm_install_icons ? was changed to support and encourage that approach, and I think this function should also change to match it. What could be matched ? As far as I can see only the ICONS parameter, all other parameter do not match by design. :-( Merging the ICONS parameter with recent ecm_add_app_icon parameter list will have the following syntax: 1. ecm_add_app_icon(source_var ICONS icon [icon [...]]) the additional mentioned syntax forms are not supported by ecm_install_icons so we are free to choose what we like. According to http://www.cmake.org/cmake/help/v3.0/command/install.html the mentioned term 'PATTERN' only supports '*' at the end of files and therefore does not fit into the recent implementation. So the term 'GLOB' which is defined in the cmake file command http://www.cmake.org/cmake/help/v3.0/command/file.html remains and fit into what is implemented in the patch 2. ecm_add_app_icons(source_var GLOB icon-glob-expression) [1] and 3.ecm_add_app_icon(source_var icon-glob-expression) [1] because ecm_install_icons also have a compatibility mode to support old style parameter list: From https://projects.kde.org/projects/kdesupport/extra-cmake-modules/repository/revisions/master/entry/modules/ECMInstallIcons.cmake An old form of arguments will also be accepted:: # ecm_install_icons(icon_install_dir [l10n_code]) syntax 3 is what is already implemented by the recent patch. Can we agree ? @Jeremey: 1. Syntax 3 is already implemented and need only a few spelling fixed before commit. 2. Currently I do not have any access to a Mac/OSX development machine, so I can only cover the windows part. Is there anyone having access to a Mac OSX development machine
Re: Review Request 122086: Add new method KWindowSystem::icons
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/#review74142 --- Wrt the other patches, using NETIcon over there, the requirement in KWin and likely libtaskbar (ie. keep this in sync): What do you think about extending the API by allowing to optionally feed in the required elements? QIcon KWindowSystem::completeIcon(WId win, int flags, NETWinInfo *nwi = nullptr, XWMHints *xhints = nullptr, QString className = QString()); The function would then (in case nwi is supplied) check passedProperties() to see whether icons and/or classname can be fetched from there (unless classname is explicitly trumped by the last parameter) - Thomas Lübking On Jan. 16, 2015, 12:55 nachm., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 16, 2015, 12:55 nachm.) Review request for KDE Frameworks. Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs - src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122086/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build became unstable: plasma-framework_master_qt5 » All,LINBUILDER #956
See http://build.kde.org/job/plasma-framework_master_qt5/Variation=All,label=LINBUILDER/956/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122086: Add new method KWindowSystem::icons
On Jan. 16, 2015, 4:13 p.m., Thomas Lübking wrote: Wrt the other patches, using NETIcon over there, the requirement in KWin and likely libtaskbar (ie. keep this in sync): What do you think about extending the API by allowing to optionally feed in the required elements? QIcon KWindowSystem::completeIcon(WId win, int flags, NETWinInfo *nwi = nullptr, XWMHints *xhints = nullptr, QString className = QString()); The function would then (in case nwi is supplied) check passedProperties() to see whether icons and/or classname can be fetched from there (unless classname is explicitly trumped by the last parameter) Regarding keeping things in sync: I've prepared a changeset to libtaskmanager that deletes around 90% of stunningly redundant icon code and will replace it with a call to KWindowSystem::icons(). It won't pass the ClassHint flag though to avoid an additional XGetClassHint in KWS since it has the ClassHint already, so it will do the fallback itself. - Eike --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/#review74142 --- On Jan. 16, 2015, 12:55 p.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 16, 2015, 12:55 p.m.) Review request for KDE Frameworks. Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs - src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122086/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122082: Rename jpegcreatorsettings to jpegcreatorsettings5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122082/ --- (Updated Jan. 16, 2015, 6:21 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Lukáš Tinkl. Repository: kio-extras Description --- same file is also provided by kde4 runtime, which will lead to file conflict. rename jpegcreatorsettings - jpegcreatorsettings5 to make packager life a little bit easier. Diffs - thumbnail/jpegcreatorsettings.kcfgc 3f6cdab thumbnail/jpegcreatorsettings5.kcfg PRE-CREATION thumbnail/jpegcreatorsettings5.kcfgc PRE-CREATION thumbnail/CMakeLists.txt e9ab79b thumbnail/jpegcreator.cpp de4902b thumbnail/jpegcreatorsettings.kcfg 8f68f46 Diff: https://git.reviewboard.kde.org/r/122082/diff/ Testing --- compiles. Thanks, Xuetian Weng ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122093: Install lower- and camelcase headers to different subdirs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122093/ --- Review request for KDE Frameworks and Marco Martin. Bugs: 342899 https://bugs.kde.org/show_bug.cgi?id=342899 Repository: kpackage Description --- otherwise they get overwriten on case insensitive FS's.. Diffs - src/kpackage/CMakeLists.txt cd1ce00 Diff: https://git.reviewboard.kde.org/r/122093/diff/ Testing --- builds... i expect plasma-framework shall still build Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121448: Introduce ECMAddAppIcon.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121448/#review74164 --- Ralf, Can you update the diff rather than uploading the file? It's much easier to apply a diff than to figure out where to put this file and if any other changes are needed to use it, etc. - Jeremy Whiting On Dec. 15, 2014, 1:01 a.m., Ralf Habacker wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121448/ --- (Updated Dec. 15, 2014, 1:01 a.m.) Review request for Extra Cmake Modules, KDE Frameworks and Laurent Navet. Repository: extra-cmake-modules Description --- This module, which has been migrated from the related KDE4 macto kde4_app_app_icon, supports platform specific application icon for Windows and Mac OSX. On Windows this function depends on the external tool png2ico, which is provided by the kdewin-tools binary package. Sources are available at https://projects.kde.org/projects/kdesupport/kdewin. Diffs - modules/ECMAddAppIcon.cmake PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121448/diff/ Testing --- File Attachments ECMAddAppIcon.cmake https://git.reviewboard.kde.org/media/uploaded/files/2014/12/12/a05ee2b5-64e3-4e44-ae34-4e1b7110e5f1__ECMAddAppIcon.cmake ECMAddAppIcon.cmake https://git.reviewboard.kde.org/media/uploaded/files/2014/12/15/8b3e226f-a70b-4998-983a-813730a436bf__ECMAddAppIcon.cmake ECMAddAppIcon.cmake https://git.reviewboard.kde.org/media/uploaded/files/2014/12/15/8433995f-b88f-426d-af54-46aba635ae1e__ECMAddAppIcon.cmake Thanks, Ralf Habacker ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122086: Add new method KWindowSystem::icons
On Jan. 16, 2015, 4:13 nachm., Thomas Lübking wrote: Wrt the other patches, using NETIcon over there, the requirement in KWin and likely libtaskbar (ie. keep this in sync): What do you think about extending the API by allowing to optionally feed in the required elements? QIcon KWindowSystem::completeIcon(WId win, int flags, NETWinInfo *nwi = nullptr, XWMHints *xhints = nullptr, QString className = QString()); The function would then (in case nwi is supplied) check passedProperties() to see whether icons and/or classname can be fetched from there (unless classname is explicitly trumped by the last parameter) Eike Hein wrote: Regarding keeping things in sync: I've prepared a changeset to libtaskmanager that deletes around 90% of stunningly redundant icon code and will replace it with a call to KWindowSystem::icons(). It won't pass the ClassHint flag though to avoid an additional XGetClassHint in KWS since it has the ClassHint already, so it will do the fallback itself. Ie. you'd benefit from such extended API? Do you have a NETWindowInfo or the XWMHints around as well (for a different purpose, like the input flag or the window group)? - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/#review74142 --- On Jan. 16, 2015, 12:55 nachm., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 16, 2015, 12:55 nachm.) Review request for KDE Frameworks. Repository: kwindowsystem Description --- ::icons tries to retrieve all available icon sizes and combine them into a single QIcon. On the X11 platform this can significantly reduce the time needed to fetch all available icons compared to the already existing ::icon methods which return a QPixmap of a specific size. The change includes a new test application icontest which shows all available icons in all available sizes for a window id passed as command line argument. CHANGELOG: New method QIcon KWindowSystem::icons(WId win, int flags) Diffs - src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b src/kwindowsystem_p.h 1f01145b5c7efe925fcb8242f92af17b704e01c9 src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a tests/CMakeLists.txt ce68cc505a69ea9a3cf645e9ae587bd89abe1648 tests/icontest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122086/diff/ Testing --- Thanks, Martin Gräßlin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121737: KPluginInfo: Fix properties not being copied from KService::Ptr
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121737/ --- (Updated Jan. 16, 2015, 11:52 nachm.) Review request for KDE Frameworks and David Faure. Changes --- Added test Note: KService::Ptr merges MimeTypes and ServicesTypes, is this intended or a bug? Makes the unit test unnecessarily complicated Repository: kservice Description --- This should fix property() returning empty QVariants even if it existed in the .desktop file Diffs (updated) - autotests/fakeplugin.desktop 9c6df7ffdb95f9e1b6e6f921040db00e8e5565ff autotests/fakeplugin.json 067a302505956c34e940ce0f6facd8a1745384ec autotests/kplugininfotest.cpp b520a9ca83e88fe5e4a2d53d4e05d7eb9764beea src/services/kplugininfo.cpp 35e09fa49603a3c9fd5b534cacf46c18c848eaf4 Diff: https://git.reviewboard.kde.org/r/121737/diff/ Testing --- KDED now correctly reads X-Kded-* in https://git.reviewboard.kde.org/r/121315/ instead of reading the default value. Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122096: Fix build if epoxy headers are not installed to the default include dir
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122096/ --- Review request for KDE Frameworks. Repository: kdeclarative Description --- Fix build if epoxy headers are installed to the default include dir Diffs - src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt ce5091e5ff0911d5bf511973cacbcbcfd90e1774 Diff: https://git.reviewboard.kde.org/r/122096/diff/ Testing --- compiles now Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
