Re: Review Request 122149: Fix QCommandLineParser warnings in kcmshell5
On Jan. 19, 2015, 6:08 p.m., David Faure wrote: That's one option, and a good one for compat. The other option is to change the callers to pass a single dash instead, then Qt will process such options automatically - right? Aleix Pol Gonzalez wrote: Yes, but then we can't integrate them in our code, or can we? Also it feels weird to rely on something that isn't listed in --help. David Faure wrote: The integration is automatic, given that argc,argv is passed to the QApp constructor before it even gets to your code with QCommandLineParser etc. If all you want is to set the icon and title of the mainwindow, then Qt will take care of that. It just doesn't work with --icon/--title, it requires -icon/-title (on X11) or -qwindowicon/-qwindowtitle (on all platforms). Suboptimal, I know, but I didn't manage to get something better (due to compat). (We could do some s/--icon/-qwindowicon/ and s/--title/-qwindowtitle/ before the qapp ctor, but that would require calling some helper func in all main()s -- and given int argc, char ** argv it would be horrible to write such a function.) Well, to be honest all I want is to resolve the warning. Maybe I can just remove the code that checks whether the user is passing it as an argument and let Qt do the correct thing if those arguments are passed. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122149/#review74347 --- On Jan. 19, 2015, 4:21 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122149/ --- (Updated Jan. 19, 2015, 4:21 p.m.) Review request for KDE Frameworks and David Faure. Repository: kde-cli-tools Description --- KCmdLineArgs used to define many arguments. In this case it was using --icon and --caption. At the moment, since we don't have these options we are getting warnings such as: kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: QCommandLineParser: option not defined: caption kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: QCommandLineParser: option not defined: icon This patch addresses this by adding them explicitly in here. I'm unsure if we want to do any further engineering or that's good enough. Diffs - kcmshell/main.cpp 98e646b Diff: https://git.reviewboard.kde.org/r/122149/diff/ Testing --- Ran it again, now it doesn't complain. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 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/#review74293
---
Ship it!
Let's see. This is a namespaced framework (namespace KPackage { class Foo ...
}) so it should install headers the same way e.g. KParts does.
Indeed that means this patch is correct for the DESTINATION paths. I checked
target_include_directories, looks ok already. All good.
- David Faure
On Jan. 16, 2015, 6:47 p.m., Hrvoje Senjan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122093/
---
(Updated Jan. 16, 2015, 6:47 p.m.)
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: Split kde-baseapps?
On Sunday 18 January 2015 14:35:53 David Narvaez wrote: On Mon, Jan 5, 2015 at 3:27 AM, David Faure fa...@kde.org wrote: AFAIK there are two ways. One where you replay the history using git filter- branch, there's a wiki page about how to do this (iirc from Alex Merry). This is up at g...@git.kde.org:clones/kio/narvaez/kio-about-protocol Please review everything is in order. It says it is 49 commits different from kio: 47 commits from the git filter-branch'ed original repo, then the git mv, then the merge commit. You can check the history with $ git log --follow src/ioslaves/about/about.protocol I thought kio-about was going into kio-extras? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122105: Fix KgDifficulty saving on app close
On Saturday 17 January 2015 21:48:29 Albert Astals Cid wrote: KF5 people: Is this something worth mentioning on the porting notes or using KSharedConfig::openConfig from a static destructor is a bit of a corner case anyway? It's not the first time we hit this (IIRC), so yeah, it's definitely worth mentionning in the porting notes and in the KSharedConfig documentation. Or maybe we can look into fixing this somehow? If just the app name is needed, we could keep a copy of it -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Move thumbnail kioslave and thumbcreator service type to kio?
On Sun, Jan 18, 2015 at 2:38 PM, David Narvaez david.narv...@computer.org wrote: Coincidentially, I am transplanting a protocol into KIO right now, so I can do this move also. So this transplant required some heavier porting to KF5 and now I have a couple of questions about how to post this for review. Should I: A) 1) Post a clone repo with the transplant and wait for approval 2) Post a single RR with the KF5 port B) 1) Post a clone repo with the transplant and wait for approval 2) Post several RR with specific changes towards KF5 C) 1) Post a clone repo with the transplant and KF5 port all together A and B would mean I post a clone repo that has the thumbnail code but doesn't build it, the include_directory CMake instructions would come in A|B.2). David E. Narvaez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122140: Support reading IconPixmap and IconMask from WM_HINTS in NETWinInfo
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122140/ --- (Updated Jan. 20, 2015, 8:53 a.m.) Review request for KDE Frameworks. Changes --- renamed to icccmFoo Repository: kwindowsystem Description --- WM_HINTS property is already read and this provides the icon pixmap and icon mask hint as read from WM_HINTS. This can be used for example in KWindowSystem::icon which currently uses an XLib call. Use NETWinInfo's WM2WindowClass and WM2IconPixmap in KWindowSystemPrivateX11::icon Instead of using an XLib call the wrapper from NETWinInfo is used. This reduces the number of roundtrips to the X server in case flags includes both NETWM, ClassHint and WMHints. In additon we don't need the deprecated x error eater any more. Diffs (updated) - autotests/netwininfotestclient.cpp a0fdc403bb8329ea9fde786494bc0ccf88a8ebfd src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a src/netwm.h 382ab460867905fdf0a4106a271e6614478fb438 src/netwm.cpp 1c96aaf9e480842c0b5379c89722f2e91a664476 src/netwm_def.h 49d02035eaec402a61e0b17b377b27c80d605b7b src/netwm_p.h 0f1dd62ecd9c856e028dd33d28d461ed7099f60b Diff: https://git.reviewboard.kde.org/r/122140/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 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/ --- (Updated Jan. 19, 2015, 11:32 a.m.) Status -- This change has been marked as submitted. 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
Review Request 122144: Add new overload to KWindowSystem::icon
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122144/ --- Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- The new overload is a special solution for the X11 platform. It takes the NETWinInfo as argument to read the information from. This significantly reduces the time to fetch icons for users who already have a NETWinInfo with the required data. E.g. for the case of KWin it can reduce the number of roundtrips to the X Server from up to 15 to 0. For non-X11 platforms the method just delegates to the method not taking the NETWinInfo argument. CHANGELOG: New overload to KWindowSystem::icon which reduces roundtrips to X-Server Diffs - src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 Diff: https://git.reviewboard.kde.org/r/122144/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
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/ --- (Updated Jan. 19, 2015, 2:50 p.m.) Review request for KDE Frameworks. Changes --- Add an overload to read data from NETWinInfo. 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 122144: Add new overload to KWindowSystem::icon
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122144/ --- (Updated Jan. 19, 2015, 2:45 p.m.) Review request for KDE Frameworks. Changes --- fixed an unused variable warning. Repository: kwindowsystem Description --- The new overload is a special solution for the X11 platform. It takes the NETWinInfo as argument to read the information from. This significantly reduces the time to fetch icons for users who already have a NETWinInfo with the required data. E.g. for the case of KWin it can reduce the number of roundtrips to the X Server from up to 15 to 0. For non-X11 platforms the method just delegates to the method not taking the NETWinInfo argument. CHANGELOG: New overload to KWindowSystem::icon which reduces roundtrips to X-Server Diffs (updated) - src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b Diff: https://git.reviewboard.kde.org/r/122144/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
Review Request 122149: Fix QCommandLineParser warnings in kcmshell5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122149/ --- Review request for KDE Frameworks and David Faure. Repository: kde-cli-tools Description --- KCmdLineArgs used to define many arguments. In this case it was using --icon and --caption. At the moment, since we don't have these options we are getting warnings such as: kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: QCommandLineParser: option not defined: caption kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: QCommandLineParser: option not defined: icon This patch addresses this by adding them explicitly in here. I'm unsure if we want to do any further engineering or that's good enough. Diffs - kcmshell/main.cpp 98e646b Diff: https://git.reviewboard.kde.org/r/122149/diff/ Testing --- Ran it again, now it doesn't complain. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122140: Support reading IconPixmap and IconMask from WM_HINTS in NETWinInfo
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122140/#review74333 --- Ship it! src/netwm.h https://git.reviewboard.kde.org/r/122140/#comment51587 icccmIconPixmap() and a comment to in doubt preferably use the ::icon(int, int) function (for this is a legacy feature w/ reduced functionality)? - Thomas Lübking On Jan. 19, 2015, 11:03 vorm., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122140/ --- (Updated Jan. 19, 2015, 11:03 vorm.) Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- WM_HINTS property is already read and this provides the icon pixmap and icon mask hint as read from WM_HINTS. This can be used for example in KWindowSystem::icon which currently uses an XLib call. Use NETWinInfo's WM2WindowClass and WM2IconPixmap in KWindowSystemPrivateX11::icon Instead of using an XLib call the wrapper from NETWinInfo is used. This reduces the number of roundtrips to the X server in case flags includes both NETWM, ClassHint and WMHints. In additon we don't need the deprecated x error eater any more. Diffs - autotests/netwininfotestclient.cpp a0fdc403bb8329ea9fde786494bc0ccf88a8ebfd src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a src/netwm.h 382ab460867905fdf0a4106a271e6614478fb438 src/netwm.cpp 1c96aaf9e480842c0b5379c89722f2e91a664476 src/netwm_def.h 49d02035eaec402a61e0b17b377b27c80d605b7b src/netwm_p.h 0f1dd62ecd9c856e028dd33d28d461ed7099f60b Diff: https://git.reviewboard.kde.org/r/122140/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 122144: Add new overload to KWindowSystem::icon
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122144/#review74335 --- Ship it! Ship It! - Thomas Lübking On Jan. 19, 2015, 1:45 nachm., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122144/ --- (Updated Jan. 19, 2015, 1:45 nachm.) Review request for KDE Frameworks. Repository: kwindowsystem Description --- The new overload is a special solution for the X11 platform. It takes the NETWinInfo as argument to read the information from. This significantly reduces the time to fetch icons for users who already have a NETWinInfo with the required data. E.g. for the case of KWin it can reduce the number of roundtrips to the X Server from up to 15 to 0. For non-X11 platforms the method just delegates to the method not taking the NETWinInfo argument. CHANGELOG: New overload to KWindowSystem::icon which reduces roundtrips to X-Server Diffs - src/kwindowsystem_p_x11.h 0d4b6ba551776d2fa08030f78226ecdb3c80c889 src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a src/kwindowsystem.h 322322f12dda7279567be8420533ed22cd52 src/kwindowsystem.cpp 65d215b6dfbf4df22e871fd7187fface75abb61b Diff: https://git.reviewboard.kde.org/r/122144/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: Split kde-baseapps?
On Mon, Jan 19, 2015 at 3:05 AM, David Faure fa...@kde.org wrote: I thought kio-about was going into kio-extras? Indeed, I just realized. David E. Narvaez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122105: Fix KgDifficulty saving on app close
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122105/#review74330 --- Can't you `Q_ASSERT(qApp)` in `KSharedConfig::openConfig()` in order to make sure people don't do that? - Kevin Funk On Jan. 17, 2015, 9:48 p.m., Albert Astals Cid wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122105/ --- (Updated Jan. 17, 2015, 9:48 p.m.) Review request for KDE Frameworks and KDE Games. Repository: libkdegames Description --- Since you can't use KSharedConfig::openConfig from a static destructor anymore (the QCoreApplication is gone and thus can't find the name of the thing) use a post routine to save the level before the QCoreApplication is gone. KF5 people: Is this something worth mentioning on the porting notes or using KSharedConfig::openConfig from a static destructor is a bit of a corner case anyway? Diffs - kgdifficulty.cpp 94489c7 Diff: https://git.reviewboard.kde.org/r/122105/diff/ Testing --- Thanks, Albert Astals Cid ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122086: Add new method KWindowSystem::icons
On Jan. 16, 2015, 5: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)
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.
Thomas Lübking wrote:
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)?
Martin Gräßlin wrote:
QIcon KWindowSystem::completeIcon(WId win, int flags, NETWinInfo nwi =
nullptr, XWMHints xhints = nullptr, QString className = QString());
I thought about it when implementing it, but I fear we cannot do it as
NETWinInfo is only available on X11. So it's unsuited for the cross-platform
API. Though maybe one could do something with forward declaring it. XWMHints
and the classname shouldn't be needed as both should be possible to take from
NETWinInfo (at least we have a replacement for XWMHints in NETWinInfo, we
just need to add reading out the pixmap hint).
Thomas Lübking wrote:
#if Q_OS_USELESS
typedef NETWinInfo void;
#endif
/** NETWinInfo XWMHints are ignored on non-X11 systems - and so is
likely className */
kwindowsystem_useless_os.cpp
QIcon KWindowSystem::completeIcon(WId win, int flags, NETWinInfo nwi =
nullptr, XWMHints xhints = nullptr, QString className = QString())
{
Q_UNUSED(nwi);
Q_UNUSED(hints);
...
}
Ok, maybe I should have read the third sentence as well ;-)
About the classname:
I meant to be able to override the plain one, resp. to pass one despite
not having a sufficient NETWinInfo (so it doesn't have to be fetched again)
tried a variant for one icon: https://git.reviewboard.kde.org/r/122144/
for this review I suggest that I rebase it on top of the other and add a
similar overload.
- Martin
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122086/#review74142
---
On Jan. 16, 2015, 1: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, 1: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
Review Request 122140: Support reading IconPixmap and IconMask from WM_HINTS in NETWinInfo
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122140/ --- Review request for KDE Frameworks and kwin. Repository: kwindowsystem Description --- WM_HINTS property is already read and this provides the icon pixmap and icon mask hint as read from WM_HINTS. This can be used for example in KWindowSystem::icon which currently uses an XLib call. Use NETWinInfo's WM2WindowClass and WM2IconPixmap in KWindowSystemPrivateX11::icon Instead of using an XLib call the wrapper from NETWinInfo is used. This reduces the number of roundtrips to the X server in case flags includes both NETWM, ClassHint and WMHints. In additon we don't need the deprecated x error eater any more. Diffs - autotests/netwininfotestclient.cpp a0fdc403bb8329ea9fde786494bc0ccf88a8ebfd src/kwindowsystem_x11.cpp bf958ae63b48424fc412405259f082b740928f9a src/netwm.h 382ab460867905fdf0a4106a271e6614478fb438 src/netwm.cpp 1c96aaf9e480842c0b5379c89722f2e91a664476 src/netwm_def.h 49d02035eaec402a61e0b17b377b27c80d605b7b src/netwm_p.h 0f1dd62ecd9c856e028dd33d28d461ed7099f60b Diff: https://git.reviewboard.kde.org/r/122140/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_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) Martin Gräßlin wrote: yes in deed, the other one can be simplified and I also intend to do that in another review request. addressed in https://git.reviewboard.kde.org/r/122140/ - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122086/#review74132 --- On Jan. 16, 2015, 1: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, 1: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 122149: Fix QCommandLineParser warnings in kcmshell5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122149/#review74347 --- That's one option, and a good one for compat. The other option is to change the callers to pass a single dash instead, then Qt will process such options automatically - right? - David Faure On Jan. 19, 2015, 3:21 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122149/ --- (Updated Jan. 19, 2015, 3:21 p.m.) Review request for KDE Frameworks and David Faure. Repository: kde-cli-tools Description --- KCmdLineArgs used to define many arguments. In this case it was using --icon and --caption. At the moment, since we don't have these options we are getting warnings such as: kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: QCommandLineParser: option not defined: caption kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: QCommandLineParser: option not defined: icon This patch addresses this by adding them explicitly in here. I'm unsure if we want to do any further engineering or that's good enough. Diffs - kcmshell/main.cpp 98e646b Diff: https://git.reviewboard.kde.org/r/122149/diff/ Testing --- Ran it again, now it doesn't complain. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122149: Fix QCommandLineParser warnings in kcmshell5
On Jan. 19, 2015, 5:08 p.m., David Faure wrote: That's one option, and a good one for compat. The other option is to change the callers to pass a single dash instead, then Qt will process such options automatically - right? Aleix Pol Gonzalez wrote: Yes, but then we can't integrate them in our code, or can we? Also it feels weird to rely on something that isn't listed in --help. The integration is automatic, given that argc,argv is passed to the QApp constructor before it even gets to your code with QCommandLineParser etc. If all you want is to set the icon and title of the mainwindow, then Qt will take care of that. It just doesn't work with --icon/--title, it requires -icon/-title (on X11) or -qwindowicon/-qwindowtitle (on all platforms). Suboptimal, I know, but I didn't manage to get something better (due to compat). (We could do some s/--icon/-qwindowicon/ and s/--title/-qwindowtitle/ before the qapp ctor, but that would require calling some helper func in all main()s -- and given int argc, char ** argv it would be horrible to write such a function.) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122149/#review74347 --- On Jan. 19, 2015, 3:21 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122149/ --- (Updated Jan. 19, 2015, 3:21 p.m.) Review request for KDE Frameworks and David Faure. Repository: kde-cli-tools Description --- KCmdLineArgs used to define many arguments. In this case it was using --icon and --caption. At the moment, since we don't have these options we are getting warnings such as: kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: QCommandLineParser: option not defined: caption kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: QCommandLineParser: option not defined: icon This patch addresses this by adding them explicitly in here. I'm unsure if we want to do any further engineering or that's good enough. Diffs - kcmshell/main.cpp 98e646b Diff: https://git.reviewboard.kde.org/r/122149/diff/ Testing --- Ran it again, now it doesn't complain. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122149: Fix QCommandLineParser warnings in kcmshell5
On Jan. 19, 2015, 6:08 p.m., David Faure wrote: That's one option, and a good one for compat. The other option is to change the callers to pass a single dash instead, then Qt will process such options automatically - right? Yes, but then we can't integrate them in our code, or can we? Also it feels weird to rely on something that isn't listed in --help. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122149/#review74347 --- On Jan. 19, 2015, 4:21 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122149/ --- (Updated Jan. 19, 2015, 4:21 p.m.) Review request for KDE Frameworks and David Faure. Repository: kde-cli-tools Description --- KCmdLineArgs used to define many arguments. In this case it was using --icon and --caption. At the moment, since we don't have these options we are getting warnings such as: kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: QCommandLineParser: option not defined: caption kcmshell5(19712)/(default) QCommandLineParserPrivate::aliases: QCommandLineParser: option not defined: icon This patch addresses this by adding them explicitly in here. I'm unsure if we want to do any further engineering or that's good enough. Diffs - kcmshell/main.cpp 98e646b Diff: https://git.reviewboard.kde.org/r/122149/diff/ Testing --- Ran it again, now it doesn't complain. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122105: Fix KgDifficulty saving on app close
On gen. 19, 2015, 2:43 p.m., Kevin Funk wrote: Can't you `Q_ASSERT(qApp)` in `KSharedConfig::openConfig()` in order to make sure people don't do that? There's already a warning. - Albert --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122105/#review74330 --- On gen. 17, 2015, 9:48 p.m., Albert Astals Cid wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122105/ --- (Updated gen. 17, 2015, 9:48 p.m.) Review request for KDE Frameworks and KDE Games. Repository: libkdegames Description --- Since you can't use KSharedConfig::openConfig from a static destructor anymore (the QCoreApplication is gone and thus can't find the name of the thing) use a post routine to save the level before the QCoreApplication is gone. KF5 people: Is this something worth mentioning on the porting notes or using KSharedConfig::openConfig from a static destructor is a bit of a corner case anyway? Diffs - kgdifficulty.cpp 94489c7 Diff: https://git.reviewboard.kde.org/r/122105/diff/ Testing --- Thanks, Albert Astals Cid ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122105: Fix KgDifficulty saving on app close
El Dilluns, 19 de gener de 2015, a les 09:02:07, David Faure va escriure: On Saturday 17 January 2015 21:48:29 Albert Astals Cid wrote: KF5 people: Is this something worth mentioning on the porting notes or using KSharedConfig::openConfig from a static destructor is a bit of a corner case anyway? It's not the first time we hit this (IIRC), so yeah, it's definitely worth mentionning in the porting notes and in the KSharedConfig documentation. Or maybe we can look into fixing this somehow? If just the app name is needed, we could keep a copy of it I had a quick look, but could not find anywhere to copy it, what's going to live more time that my statics? I actually don't even understand how this used to work :D Cheers, Albert ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122105: Fix KgDifficulty saving on app close
El Dilluns, 19 de gener de 2015, a les 21:15:42, David Faure va escriure: On Monday 19 January 2015 19:54:46 Albert Astals Cid wrote: El Dilluns, 19 de gener de 2015, a les 09:02:07, David Faure va escriure: On Saturday 17 January 2015 21:48:29 Albert Astals Cid wrote: KF5 people: Is this something worth mentioning on the porting notes or using KSharedConfig::openConfig from a static destructor is a bit of a corner case anyway? It's not the first time we hit this (IIRC), so yeah, it's definitely worth mentionning in the porting notes and in the KSharedConfig documentation. Or maybe we can look into fixing this somehow? If just the app name is needed, we could keep a copy of it I had a quick look, but could not find anywhere to copy it, what's going to live more time that my statics? I actually don't even understand how this used to work :D OK I had a look, and I discovered that * there are related unittests in kdelibs4support, I am moving (copying) them to kconfig (without deprecated stuff). * there are a number of issues here 1) KConfig::mainConfigName uses QCoreApplication::arguments() to check for --config. Doing this after qApp is destroyed leads to a runtime waning. KApplication was storing this in its componentdata. Even if we didn't have the issue described below in kdelibs4, I'm not sure we ever tested that using KConfig in a global-object-destructor worked in combination with the very- rarely-used --config command-line-option 2) KConfig::mainConfigName then uses its own global object, globalMainConfigName(), which might be already deleted as well, leading to a crash. I actually don't even understand how this used to work :D Well, it didn't. I just tried my testcase in kdelibs4, and I got this: Fatal Error: Accessed global static 'KGlobalPrivate *globalData()' after destruction. Defined at kdelibs/kdecore/kernel/kglobal.cpp:128 Here's the testcase http://www.davidfaure.fr/2015/ksharedconfig_in_global_object.cpp Fails with both kdelibs4 and kconfig5, everything's consistent :-) So the real question is: what did kgdifficulty really do in the kdelibs4 world, that worked? See http://quickgit.kde.org/?p=libkdegames.gita=blobh=b94cce00ef338cdfd3224e3542074b699ead0b29hb=63855d630c928e46554885f6d022d0091dbe252bf=kgdifficulty.cpp It has a K_GLOBAL_STATIC(KgDifficulty, g_difficulty) and a KConfigGroup cg(KGlobal::config(), KgDifficulty); cg.writeEntry(Level, currentLevel()-key()); in KgDifficulty destructor. And it did work :D Cheers, Albert ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122105: Fix KgDifficulty saving on app close
On Monday 19 January 2015 19:54:46 Albert Astals Cid wrote: El Dilluns, 19 de gener de 2015, a les 09:02:07, David Faure va escriure: On Saturday 17 January 2015 21:48:29 Albert Astals Cid wrote: KF5 people: Is this something worth mentioning on the porting notes or using KSharedConfig::openConfig from a static destructor is a bit of a corner case anyway? It's not the first time we hit this (IIRC), so yeah, it's definitely worth mentionning in the porting notes and in the KSharedConfig documentation. Or maybe we can look into fixing this somehow? If just the app name is needed, we could keep a copy of it I had a quick look, but could not find anywhere to copy it, what's going to live more time that my statics? I actually don't even understand how this used to work :D OK I had a look, and I discovered that * there are related unittests in kdelibs4support, I am moving (copying) them to kconfig (without deprecated stuff). * there are a number of issues here 1) KConfig::mainConfigName uses QCoreApplication::arguments() to check for --config. Doing this after qApp is destroyed leads to a runtime waning. KApplication was storing this in its componentdata. Even if we didn't have the issue described below in kdelibs4, I'm not sure we ever tested that using KConfig in a global-object-destructor worked in combination with the very- rarely-used --config command-line-option 2) KConfig::mainConfigName then uses its own global object, globalMainConfigName(), which might be already deleted as well, leading to a crash. I actually don't even understand how this used to work :D Well, it didn't. I just tried my testcase in kdelibs4, and I got this: Fatal Error: Accessed global static 'KGlobalPrivate *globalData()' after destruction. Defined at kdelibs/kdecore/kernel/kglobal.cpp:128 Here's the testcase http://www.davidfaure.fr/2015/ksharedconfig_in_global_object.cpp Fails with both kdelibs4 and kconfig5, everything's consistent :-) So the real question is: what did kgdifficulty really do in the kdelibs4 world, that worked? -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
