Re: Forwarding headers: current status
On Thursday 02 January 2014 12:14:19 David Faure wrote:
TODO: get agreement on include/KF5/FrameworkName/{everything_here} and
convert all repos.
Done!
Make sure to update everything (ECM, all frameworks, plasma...) otherwise
you'll get compile errors. This is a good time to build from scratch, too,
i.e. removing both builddir and installdir, to avoid old headers left over in
wrong places.
So, fixing ThreadWeaver on Saturday and tagging on Sunday.
Meanwhile maybe someone can test the result of the packaging scripts,
preferrably on a machine without a KF5 development setup?
http://www.davidfaure.fr/2013/extra-cmake-modules-4.95.0.tar.xz
http://www.davidfaure.fr/2013/karchive-4.95.0.tar.xz
Warning to the slashdot crowd: these are not the actual TP1 tarballs :)
--
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
Re: KDE4Support Headers
On Friday 03 January 2014 22:38:28 Christoph Cullmann wrote: Hi, KDE4Support installs still stuff like kmimetype.h in the KF5 directory, shall it not better install all its compat headers in some KDE4Support prefix to avoid that one can use them without using KF4::KDE4Support? Excellent point. Fix coming up. -- 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
Re: KF5 include problems on the build.kde.org?
On Saturday 04 January 2014 15:09:14 Christoph Cullmann wrote:
Hi,
I am currently struggling to have the KF5 port of Okteta not only build
locally (what it does fine), but also on KDE's build server:
could anybody hint to me why on the build server the file KLocalizedString
is
not found for include on building of the static lib kastencoretestio:
I have similar problems with kate on build.kde.org, here it builds, locally,
with a fresh usr/build/src dir,
there it fails unable to find KXMLGUIClient header.
http://build.kde.org/view/Unstable/job/kate_frameworks_qt5/90/console
Strange. Let me log onto the server (slave1) to see why it fails there.
/srv/jenkins/install/linux/x86_64/g++/kf5-qt5/frameworks/kxmlgui/inst/lib64/cmake/KF5XmlGui/KF5XmlGuiTargets.cmake
does say
set_target_properties(KF5::XmlGui PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES
${_IMPORT_PREFIX}/include/KF5/KXmlGui;${_IMPORT_PREFIX}/include/KF5
INTERFACE_LINK_LIBRARIES
Qt5::DBus;Qt5::Xml;Qt5::Widgets;KF5::ConfigCore;KF5::ConfigWidgets
)
and yet include/KF5/KXmlGui is not in part of the command-line for compiling
ktexteditor.cpp.o
Ah yep, this is why:
target_link_libraries(ktexteditor LINK_PUBLIC KF5::Parts
LINK_PRIVATE KF5::I18n)
- KF5::XmlGui missing.
What I don't understand is that it works locally, -isystem
/d/kde/inst/kde_frameworks/include/KF5/KXmlGui
is part of the command line for ktexteditor.cpp.o
The only difference I can think of is that build.kde.org uses cmake next while
I use cmake master
But that would mean master is better than next, it's able to follow the
dependencies of KF5::KParts,
which include KF5::KXmlGui, and parse their Targets.cmake file? CC'ing
kde-buildsystem,
I'm a bit lost about the mechanisms involved here and why they would work
differently in master and in next.
I'll do a full rebuild with cmake next, just to test.
--
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
Re: KF5 include problems on the build.kde.org?
On Saturday 04 January 2014 15:48:39 Friedrich W. H. Kossebau wrote: Am Samstag, 4. Januar 2014, 15:39:52 schrieb Martin Graesslin: On Saturday 04 January 2014 15:04:58 Friedrich W. H. Kossebau wrote: Hi, I am currently struggling to have the KF5 port of Okteta not only build locally (what it does fine), but also on KDE's build server: could anybody hint to me why on the build server the file KLocalizedString is not found for include on building of the static lib kastencoretestio: From http://build.kde.org/job/okteta_master_qt5/8/console : What strikes me there is: 13:22:15 == Build Dependencies: 13:22:15 kdelibs - Branch frameworks That should be the single frameworks I guess, e.g. on kde-workspace it looks like: == Build Dependencies: cmake - Branch master kparts - Branch master kidletime - Branch master sonnet - Branch master extra-cmake-modules - Branch master kwallet-framework - Branch master Good hint possibly. Though strange that the find_package(KF5 [...]) does not fail, it would have guessed some things changed since the frameworks had been just a branch in the kdelibs repo. Ben, could you take a look and see if perhaps the build dependencies of the Okteta KF5 build would need an update to match the new framework repos? I updated kde-build-metadata, let's see if that helps. Using a branch name frameworks instead of making up yet another variant (kf5-port) would have made things a bit simpler and more consistent :) -- 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
Re: KF5 include problems on the build.kde.org?
On Saturday 04 January 2014 15:37:34 David Faure wrote: I'll do a full rebuild with cmake next, just to test. Built just fine locally. I'm stumped. -- 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
Re: KDE-wide Animation settings
On Saturday 04 January 2014 20:19:43 Hugo Pereira Da Costa wrote: The first problem is that with kf5, the style configuration kcm expects a libkstyle_oxygen_config.so *plugin* for this configuration, whereas it was kstyle_oxygen_config.so in kde4 times. now, as the error message indicates: plugins should not start with lib and thus, oxygen still installs kstyle_oxygen_config.so only so that the ill-named plugin is not found, and you get the error dialog. To fix, - either we fix the kcm by changing the looked for plugin name (by removing the lib prefix) Yes. Why does the kcm look for a lib prefix now? Surely there's no good reason for that, the kde4 way was fine. (I think it was added in the first place due to the lack of replacement for kde4_add_plugin) But we can still remove the prefix, can't we? You do so for oxygen :) For the second issue, QStyle::SH_Widget_Animate is set to false not in oxygen style but in KStyle by itself. This is unrelated to the style configuration. I am not too inclined to set it to true, since I am absolutely unclear what this style hint is meant for. Can someone explain ? It should be true on fast local desktops, and false on slow/remote desktops, I suppose. IOW user configurable. Notably, I don't think it should be set to true for _all_ widgets, since most widgets are already animated internally by oxygen (and thus should not be by the app), though it indeed should return true for KMessageWidget, which is not animated internally. So: should I enable the flag only for widgets that oxygen does not animate itself ? No, I think it's rather unrelated. The animations from the widgets themselves are (as can be found by grepping qtbase) - opening trees in QTreeView - moving tabs in QTabBar - dockareas and toolbars Does oxygen animate any of that? I'd be surprised, since these were already animated in Qt4. What's new is letting users turn it off, via kstyle using kconfig. -- 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
Re: KDE-wide Animation settings
On Saturday 04 January 2014 20:56:30 Hugo Pereira Da Costa wrote: KConfigGroup g(KSharedConfig::openConfig(), KDE-Global GUI Settings); return g.readEntry(GraphicEffectsLevel, 0); The default value is wrong. In KDE4 it was enabled by default. (see KGlobalSettings::graphicEffectsLevelDefault()) -- 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
Re: KTextEditor Frameworks question
On Saturday 04 January 2014 19:40:46 Christoph Cullmann wrote: What would be required to have the ktexteditor stuff be frameworks ready? Using all the cmake stuff from other frameworks ;) I just updated and moved the framework template we had in kdelibs to kdeexamples/framework-template. You can use this to generate the entire directory structure if you want. -- 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
Re: KF5 include problems on the build.kde.org?
On Saturday 04 January 2014 16:53:42 David Faure wrote: On Saturday 04 January 2014 15:37:34 David Faure wrote: I'll do a full rebuild with cmake next, just to test. Built just fine locally. I'm stumped. OK it was a kde-build-metadata/dependency-data configuration issue, it was grabbing the old kdelibs-frameworks stuff. Solving it with Ben now, by having a different dependency-data file for each branch group (i.e. a different one for kf5-qt5 than for the kde4/qt4 stuff). -- 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
Re: KTextEditor Frameworks question
On Saturday 04 January 2014 22:40:13 Christoph Cullmann wrote: On Saturday 04 January 2014 19:40:46 Christoph Cullmann wrote: What would be required to have the ktexteditor stuff be frameworks ready? Using all the cmake stuff from other frameworks ;) I just updated and moved the framework template we had in kdelibs to kdeexamples/framework-template. You can use this to generate the entire directory structure if you want. Ok, that shall be not really a problem, given I have already the autotests in the right dir and only ktexteditor = src is needed. This is about the contents of the CMakeLists.txt files too. You should port to ecm_generate_headers if you don't use it yet (you can use my script for that, kde-dev-scripts/kf5/install_forwarding_headers.pl) and compare the cmake stuff with e.g. kcoreaddons or the template. Still, what would be the appropriate way to split the kate.git without loosing the history. Do it just like we did for kdelibs: new repo, old history available via grafting. See kde-dev-scripts/frameworks/split_out_frameworks.sh (change the for loop, obviously - you don't need a loop at all, if you have only one repo to split out). Maybe don't even run the script, just run the commands one by one, to adapt them to your directory structure. Once you have the clean new repo, you'll need to talk to sysadmin for uploading it. -- 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
Re: KDE-wide Animation settings
On Saturday 04 January 2014 23:02:35 Dominik Haumann wrote: On Saturday 04 January 2014 20:56:30 Hugo Pereira Da Costa wrote: ok. So this should go to kstyle (nothing oxygen explicit in there). In fact, kstyle returns: KConfigGroup g(KSharedConfig::openConfig(), KDE-Global GUI Settings); return g.readEntry(GraphicEffectsLevel, 0); mmm. No clue which KCM sets this :) (but that answers Dominik's original question I guess) Yes, that answers my question. In particular, using the code widget-style()-styleHint(QStyle::SH_Widget_Animate, 0, widget) is correct *if* we use a KStyle based style. No, it's correct in all cases :) With the other Qt styles you get true, which is the correct default value. What I personally would like more is to always be able to read this. I of course can use a KSharedConfig::openConfig() and then read the config value myself. However, would it be of interest to have a static accessor for this? Downside is that there are quite a lot of kdeglobals dependent entries... Is the preferred way to read this value manually then in KatePart? Would that also be the preferred way in KMessageWidget? I don't really see what the issue is. Do you care that much for Windows users to be able to turn animations off? Another issue we have in Kate code: kdeglobals right now do not have this effects enabled by default. A unit test from kde4 times now fails in kf5, because the timings are different, because the effects are off. Are there plans to have a kdeglobals that has enabled effects? As I said, it's a bug in the readEntry() call above. The default is supposed to be 1, so you don't need a special kdeglobals. -- 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
Re: KDE-wide Animation settings
On Sunday 05 January 2014 10:04:00 Hugo Pereira Da Costa wrote:
On 01/04/2014 11:21 PM, Dominik Haumann wrote:
On Saturday 04 January 2014 23:11:14 David Faure wrote:
On Saturday 04 January 2014 23:02:35 Dominik Haumann wrote:
On Saturday 04 January 2014 20:56:30 Hugo Pereira Da Costa wrote:
ok. So this should go to kstyle (nothing oxygen explicit in there).
In fact, kstyle returns:
KConfigGroup g(KSharedConfig::openConfig(), KDE-Global GUI Settings);
return g.readEntry(GraphicEffectsLevel, 0);
mmm. No clue which KCM sets this :)
(but that answers Dominik's original question I guess)
Yes, that answers my question. In particular, using the code
widget-style()-styleHint(QStyle::SH_Widget_Animate, 0, widget)
is correct *if* we use a KStyle based style.
No, it's correct in all cases :)
With the other Qt styles you get true, which is the correct default
value.
Ok, I missed this one, thanks for the clarification!
What I personally would like more is to always be able to read this.
I of course can use a KSharedConfig::openConfig() and then read the
config
value myself. However, would it be of interest to have a static accessor
for this? Downside is that there are quite a lot of kdeglobals dependent
entries...
Is the preferred way to read this value manually then in KatePart?
Would that also be the preferred way in KMessageWidget?
I don't really see what the issue is. Do you care that much for Windows
users to be able to turn animations off?
Nevermind, was a misunderstanding then.
Another issue we have in Kate code: kdeglobals right now do not have
this
effects enabled by default. A unit test from kde4 times now fails in
kf5,
because the timings are different, because the effects are off.
Are there plans to have a kdeglobals that has enabled effects?
As I said, it's a bug in the readEntry() call above. The default is
supposed to be 1, so you don't need a special kdeglobals.
Ok, so who is going to fix it? ;) Hugo?
Question
should the default simply be 1
or should kstyle re-introduce the same enumeration that was in
KGlobalSettings ?
enum GraphicEffect {
NoEffects = 0x, /// GUI with no effects at all.
GradientEffects = 0x0001, /// GUI with only gradients enabled.
SimpleAnimationEffects = 0x0002, /// GUI with simple animations enabled.
ComplexAnimationEffects = 0x0006 /// GUI with complex animations enabled.
1) I don't think these multiple levels were ever used anywhere
2) the Qt widgets simply test it as a boolean
If you see a good reason for multiple levels, go for it; but if it's just
because kde4 had it - not a good enough reason, since it wasn't used there.
--
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
Re: KDE-wide Animation settings
On Saturday 04 January 2014 23:04:50 Nicolás Alvarez wrote: 2014/1/4 David Faure fa...@kde.org: On Saturday 04 January 2014 23:02:35 Dominik Haumann wrote: What I personally would like more is to always be able to read this. I of course can use a KSharedConfig::openConfig() and then read the config value myself. However, would it be of interest to have a static accessor for this? Downside is that there are quite a lot of kdeglobals dependent entries... Is the preferred way to read this value manually then in KatePart? Would that also be the preferred way in KMessageWidget? I don't really see what the issue is. Do you care that much for Windows users to be able to turn animations off? Ideally on Windows we'd follow Windows settings rather than KDE apps having their own configuration. SystemParametersInfo(SPI_GETCLIENTAREAANIMATION) seems to get what we'd want. Sounds good, please make a change request for the Qt windows styles :-) -- 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
Re: Review Request 114726: Make sure ktoolbar_unittest does not depend on global icon settings
On Dec. 30, 2013, 12:31 p.m., David Faure wrote: I don't really understand why this fails IMHO it *did* make sense for the KToolBar unittest to check which icon size the toolbars will get by default. The value of 2 on build.kde.org, for instance, should never never happen. 2 pixels for a toolbar icon is just unusable... David Faure wrote: Debugged and fixed in http://commits.kde.org/kxmlgui/94e84ebd02c6ab97e6ac4288f22f17337fc7948a + http://commits.kde.org/frameworkintegration/163282ed294f5c50a1a12be9eb50d8b96d87ab84 Please discard this review request. Alex Merry wrote: I still think these checks shouldn't be there; why is a kxmlgui unit test checking the default icon sizes of KIconTheme? It just makes it fragile if those defaults are changed (especially as they are in different repositories). You see as two different frameworks, I see it as one feature from the user's point of view. Toolbar icons start in a given size, then can be configured to different sizes (and the whole thing is quite complex, with XMLGUI and KConfig and layering of settings etc.). So the full test includes what do I start from, which is the default icon size coming from KIconTheme. I don't want to make that test less useful just for the hypothetical situation where one day we might change the default icon size (which sounds rather unlikely). Yes our autotests aren't all unit tests, some of them are more general feature tests. Anything that prevents regressions is good :) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114726/#review46444 --- On Dec. 29, 2013, 6:04 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114726/ --- (Updated Dec. 29, 2013, 6:04 p.m.) Review request for KDE Frameworks. Repository: kxmlgui Description --- Make sure ktoolbar_unittest does not depend on global icon settings ktoolbar_unittest should not be testing the default settings of KIconTheme anyway. NB: I am away until 2nd Jan, so if it gets a ship it, feel free to commit it in my absence in order to get Jenkins green again. Diffs - autotests/ktoolbar_unittest.cpp 2ee490d35b517a8121aa0aeda0d6ebb4256caad0 Diff: https://git.reviewboard.kde.org/r/114726/diff/ Testing --- Tests pass on my local machine (but they did before as well). Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
TP1 release
I made and uploaded the tarballs (and zips) for the TP1 release. Let's give the packagers a few days, and then we'll publish and announce the release. Meanwhile, no major changes in frameworks please, in case I need to redo tarballs. Bugfixes yes, but nothing that requires updating ECM, or that means changes across the board in all frameworks. -- 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
Re: KTextEditor Frameworks question
On Monday 06 January 2014 08:36:14 Christoph Cullmann wrote: Is it really enough to init a new repository and have that one initial commit + add (and then move the files around inside the new git) to have history via grafting available? There is no other trick behind I just don't see ATM? Yes, you can just try it, see the instructions in that initial commit :-) -- 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
Re: Problem with kf5-development/kde-build-metadata/dependency-data-common
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
Re: TP1 release
On Sunday 05 January 2014 13:42:49 David Faure wrote: I made and uploaded the tarballs (and zips) for the TP1 release. Let's give the packagers a few days, and then we'll publish and announce the release. Meanwhile, no major changes in frameworks please, in case I need to redo tarballs. Bugfixes yes, but nothing that requires updating ECM, or that means changes across the board in all frameworks. TP1 has been released. I just tagged the corresponding sha1s in the git modules. You can push the million of commits you have been doing locally meanwhile :-) -- 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
Re: Review Request 114885: Remove custom build types
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114885/#review46996 --- Ship it! Nice, this also fixes some inconsistencies between compilers. I tested in a pure qt5+cmake test (no ECM) that: * RelWithDebInfo already sets -DQT_NO_DEBUG -O2 -g -DNDEBUG. So this was indeed unnecessary by now. * Release sets -DQT_NO_DEBUG -O3 -DNDEBUG, which is even better (-O3 vs -O2) than what ECM was doing. * Debug sets -g. No -O2, which isn't expected in debug mode. - David Faure On Jan. 7, 2014, 4:30 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114885/ --- (Updated Jan. 7, 2014, 4:30 p.m.) Review request for Build System, KDE Frameworks, David Faure, Kevin Ottens, and Stephen Kelly. Repository: extra-cmake-modules Description --- This is a cleaned-up version of https://git.reviewboard.kde.org/r/113805/ , with documentation fixes. The discussion there appeared to end up being largely in favour of this move. Obviously, this can only go in once TP1 is done. Remove custom build types KDECompilerSettings.cmake no longer alters CMake's built-in build types or adds its own. The debug build type therefore simply sets -g with no additional flags (rather than -O2 and, depending on the compiler, some no-inline/no-reorder flags as previously), the release build types no longer set -DQT_NO_DEBUG and the debugfull, profile and coverage build types no longer exist. QT_NO_DEBUG is set by Qt's CMake scripts depending on the build type of Qt itself. debugfull mostly set -g3, allowing macro expansion when debugging; users can set this flag using environment variables if they wish. RelWithDebugInfo should be used instead of profile (according to dfaure); -fprofile-arcs and -ftest-coverage are easy enough to add to $CXX_FLAGS if they are required (formerly set by profile and coverage). Diffs - kde-modules/KDECompilerSettings.cmake 72824e166d03dcc2d089814dc121f08ba998974a Diff: https://git.reviewboard.kde.org/r/114885/diff/ Testing --- Built kcoreaddons on linux with gcc. -DCMAKE_BUILD_TYPE=debugfull works, but does not set -g. -DCMAKE_BUILD_TYPE=debug does set -g. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KTextEditor Frameworks question
On Tuesday 07 January 2014 19:57:56 Christoph Cullmann wrote:
Hi,
I just tried to fix the naming issues.
Does that try here look better
http://quickgit.kde.org/?p=scratch%2Fcullmann%2Fktexteditor.git
I see a ${FooBar_HEADERS} in src/CMakeLists.txt which is used but not set,
should be removed.
src/include/CMakeList.txt is the one calling ecm_generate_headers, but in a
strange way: it sets KTEXTEDITOR_PUBLIC_HEADERS which was already set,
so it overwrites it.
You can remove the whole list of lowercase headers, ecm_generate_headers
generates it for you from the uppercase ones.
And then just install with the contents of that variable. *after* the call to
ecm_generate_headers, not before ;)
add_library (KF5TextEditor ${ktexteditor_LIB_SRCS}
${KTEXTEDITOR_PUBLIC_HEADERS})
Why pass headers to add_library?
target_link_libraries(KF5TextEditor LINK_PUBLIC KF5::Parts
LINK_PRIVATE KF5::I18n
Qt5::Script
KF5::Archive
KF5::GuiAddons
KF5::I18n
KF5::IconThemes
KF5::ItemViews
KF5::KCMUtils
KF5::KIOFileWidgets
KF5::Notifications
KF5::Parts
KF5::PrintUtils
KF5::SonnetCore)
Are you sure that all of these should be private? The ones that provide
classes that appear in the public API should be under LINK_PUBLIC.
set( katepart_PART_UI
)
unused, remove.
I tested the grafting, works fine.
--
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
Re: KTextEditor Frameworks question
add_library (KF5TextEditor ${ktexteditor_LIB_SRCS}
${KTEXTEDITOR_PUBLIC_HEADERS})
Why pass headers to add_library?
Just for automoc, without that, I get stuff like:
Linking CXX shared library libKF5TextEditor.so
CMakeFiles/KF5TextEditor.dir/view/kateviewinternal.cpp.o: In function
`KateViewInternal::scrollColumns(int)':
/home/cullmann/local/kf5/src/ktexteditor/src/view/kateviewinternal.cpp:522:
undefined reference to
`KTextEditor::View::horizontalScrollPositionChanged(KTextEditor::View*)'
Hmm. I think this comes from the .h and the .cpp files not being in the same
dir. Automoc doesn't like it, and I don't like it very much either ;)
It makes navigation from .h to .cpp harder, depending on the IDE/tools used.
Why not bring them together?
The old reason for include/ktexteditor/*.h (being able to include
ktexteditor/file.h from the cpp files) no longer applies,
ecm_generate_headers takes care of generating local lowercase forwarding
headers when used with PREFIX as you do.
Is it ok then to request moving or are there other constraints, too?
Seems fine to me, go ahead.
The checklist for new frameworks can be extracted from
http://community.kde.org/Frameworks/Epics/KF5.0_Release_Preparation
which gives:
* make new repo using script (done, in your case)
* run astyle-kdelibs (requires some patching, so I can do it if you want)
* adjust kde-build-metadata
* get the job set up on build.kde.org (Ben or Aurélien)
* ensure green ;)
* add to bugs.kde.org (d_ed: how?)
* add to reviewboard.kde.org (sysadmin request I suppose)
Any wiki page where I add this list? Maybe it should go at the end of the list
from definition of done in
http://community.kde.org/Frameworks/Epics/Splitting_kdelibs ?
Seems to me that this is the only useful thing left from that wiki page, which
we can otherwise delete, no?
I guess I shall add all required frameworks to the
KF5TextEditorConfig.cmake.in, too, or?
Yes.
--
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
Re: TODO: Reduce the mentions of KDE4 in the source code to those that are correct/needed
On Wednesday 08 January 2014 01:17:50 Siddharth Sharma wrote: Hiya Folks, I want to give a helping hand to that task, hence added my name. So if i understood correctly does it needs to be done with kde4support, i mean kde4support - kf5support ? Definitely not :) kde4support is kde4support (the stuff that allows compat with kde4 APIs) - much like qt3support and kde3support in the old days. No, this is about KDE4_* in extra-cmake-modules for instance. Or in any CMakeLists.txt files. -- 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
Re: KTextEditor Frameworks question
On Tuesday 07 January 2014 21:12:39 Christoph Cullmann wrote: Yeah, 2 = 4, all right, wanted that anyway since long. Only the autotests/input stuff should be best left untouched. That is no source in that case only input for tests, and yeah, I guess they won't like that. OK, reverted that subdir, and committed. It's a lib that provides API, not an integration plugin. Hmm, but it installs a kpart, too, is that still ok? Sure. This doesn't change the basic rule: Provide non-deprecated API - tier1/2/3. -- 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
Re: Review Request 114904: Remove KDE4 magic from _SET_FANCY in KDEInstallDirs.cmake
On Jan. 7, 2014, 9:43 p.m., Alexander Neundorf wrote: The motivation was that if somebody had set up carefully his install dirs for kdelibs, he simply wanted to point a following project to the same CMAKE_INSTALL_PREFIX, and have all the other install dirs automatically use the same locations as the installed kdelibs. Somebody, or maybe several people, requested this back then, I don't remember right now where or who it was. Alex Merry wrote: OK, thanks for the info. I reckon this doesn't make much sense any more, given we have multiple tier1 frameworks. About the best we could do is maybe follow Qt's layout, but I suspect that isn't worth it (if Qt even gives us all the necessary information). It was probably me. But many things changed since then :) There is no kdelibs anymore, and kdesrc-build makes it easy to specify the same prefix for everything. This leaves the case of compiling an app against system libs for Qt+KF5, but I think we should rather aim at making this work as easily as possible with a different prefix for the app, rather than forcing the installation of the app into the KF5 prefix (e.g. RPATH helps there, KDEDIRS is no more, so at most PATH and XDG_DATA_DIRS need to be set, and not even PATH if there's just one binary invoked by full path). Ah, there was another case I think, kdelibs configured by the distro with lots of custom paths (e.g. prefix of /usr but /etc instead of /usr/share/config for the config dir, and so on... that was more coolo's stuff). I suppose we have to find a solution for such things, but I'm no expert in this area (e.g. what would be convenient for distros). In any case the current stuff for sure doesn't work anymore, so a new solution will have to be used instead, if needed. +1 for removing the old magic. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114904/#review47008 --- On Jan. 7, 2014, 9:18 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114904/ --- (Updated Jan. 7, 2014, 9:18 p.m.) Review request for Build System and KDE Frameworks. Repository: extra-cmake-modules Description --- Remove KDE4 magic from _SET_FANCY in KDEInstallDirs.cmake This appears to be a hangover from the KDE4 days, which would adjust certain paths to match the ones for kdelibs if you installed an application to the same prefix as kdelibs. This was probably to make KStandardDirs work properly in unusual setups. Diffs - kde-modules/KDEInstallDirs.cmake 838a52384b7cbfc84c5bd02c2f40f027f36db169 Diff: https://git.reviewboard.kde.org/r/114904/diff/ Testing --- CMake runs fine on KCoreAddons (clean build dir), and only the install prefix variable I set on the command line (CMAKE_INSTALL_PREFIX) is in the cache. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KDE Frameworks: Moving toward 5.0 final and Governance
On Wednesday 08 January 2014 12:21:10 John Layt wrote: it may be better moved to KDE4Support, along with KPrintPreview, leaving an empty repo. If that's the conclusion, then an option is just to leave everything in the current repo, and mark the whole repo (and its individual classes) as deprecated? -- 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
Re: Review Request 114921: Make KFileItemActions the parent of the actions it creates
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114921/#review47124 --- Ship it! Yep, as discussed. Looks good, except for one invalid URL. autotests/kfileitemactionstest.cpp https://git.reviewboard.kde.org/r/114921/#comment33580 This is an invalid way to create a URL from a local file, you must use QUrl::fromLocalFile(). Then again ... it means this whole code really doesn't care about the URL being used, does it? ;-) So you might just as well use file:/// or whatever. - David Faure On Jan. 9, 2014, 8:15 a.m., Frank Reininghaus wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114921/ --- (Updated Jan. 9, 2014, 8:15 a.m.) Review request for KDE Frameworks and David Faure. Repository: kio Description --- This patch is a result of the discussion in http://lists.kde.org/?t=13868700941r=1w=2 Currently, KFileItemActions makes the widget that is set with setParentWidget(QWidget*) the parent not only of any dialogs that are shown (as advertised by the API docs), but also of the created actions. Nonetheless, KFileItemActions remembers pointers to all created actions and deletes them in the destructor. This can cause problems if the widget is deleted before the KFileItemActions instance - the destructor will then try to delete dangling pointers and cause a crash. This problem can be fixed by making KFileItemActions the parent of the actions. This also makes the code a bit simpler because the m_ownActions member is not needed any more. In fact, this issue is the cause of crashes in Dolphin (see https://bugs.kde.org/show_bug.cgi?id=259089). I still think that we don't really have to change it in kdelibs 4.x because the problem can be worked around (https://git.reviewboard.kde.org/r/114440/ , which isn't pushed yet because it turns out that there is still another source of crashes in the problematic Dolphin use case). Diffs - autotests/CMakeLists.txt 2868327 autotests/kfileitemactionstest.cpp PRE-CREATION src/widgets/kfileitemactions.cpp eee2ebe src/widgets/kfileitemactions_p.h 9f9a701 Diff: https://git.reviewboard.kde.org/r/114921/diff/ Testing --- New unit test crashes with master, and passes if the patch is applied. Existing kio unit tests pass on my system (except for kiocore-kacltest, but I believe that the failure is unrelated to this patch). Thanks, Frank Reininghaus ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114934: Improve dependency specification
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114934/#review47142 --- Ship it! Ship It! - David Faure On Jan. 10, 2014, 11:25 a.m., Michael Palimaka wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114934/ --- (Updated Jan. 10, 2014, 11:25 a.m.) Review request for KDE Frameworks. Repository: threadweaver Description --- Only benchmarks and autotests require QtTest, so move the dependency check there instead of requiring it unconditionally. Diffs - CMakeLists.txt 4165a3b07a983ac5715cc847c91c45a8d1f10003 autotests/CMakeLists.txt 367ae9957b8aceeffd4eb09cfd1322420c7e6bee benchmarks/CMakeLists.txt 1621cec28914745d10de689de13bcbdc8d8ec6dc Diff: https://git.reviewboard.kde.org/r/114934/diff/ Testing --- Framework builds without QtTest, and tests still pass with QtTest. Thanks, Michael Palimaka ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114937: port kconfig_compiler_kf5 to QCommandLineParser
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114937/#review47144 --- What made me suggest this task was that --version was missing :) Please add a addVersionOption() :) src/kconfig_compiler/kconfig_compiler.cpp https://git.reviewboard.kde.org/r/114937/#comment33591 this is main(), return 0 would be enough. src/kconfig_compiler/kconfig_compiler.cpp https://git.reviewboard.kde.org/r/114937/#comment33593 hmm, or too many? Maybe check for 2, or adjust the error ... or check both separately. Also, this should be cerr, for error messages, as the name indicates. src/kconfig_compiler/kconfig_compiler.cpp https://git.reviewboard.kde.org/r/114937/#comment33592 return 1 - David Faure On Jan. 10, 2014, 4:14 p.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114937/ --- (Updated Jan. 10, 2014, 4:14 p.m.) Review request for KDE Frameworks and David Faure. Repository: kconfig Description --- $summary Diffs - src/kconfig_compiler/kconfig_compiler.cpp df17d4c Diff: https://git.reviewboard.kde.org/r/114937/diff/ Testing --- Thanks, Bhushan Shah ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114937: port kconfig_compiler_kf5 to QCommandLineParser
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114937/#review47176 --- Ship it! just two questions, feel free to commit after checking/fixing src/kconfig_compiler/kconfig_compiler.cpp https://git.reviewboard.kde.org/r/114937/#comment33639 wouldn't kconfig_version.h work? seems more robust in case of moving stuff around. src/kconfig_compiler/kconfig_compiler.cpp https://git.reviewboard.kde.org/r/114937/#comment33640 wrong indentation, or is this just reviewboard being buggy? - David Faure On Jan. 10, 2014, 4:59 p.m., Bhushan Shah wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114937/ --- (Updated Jan. 10, 2014, 4:59 p.m.) Review request for KDE Frameworks and David Faure. Repository: kconfig Description --- $summary Diffs - src/kconfig_compiler/kconfig_compiler.cpp df17d4c Diff: https://git.reviewboard.kde.org/r/114937/diff/ Testing --- Thanks, Bhushan Shah ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114988: Fix PREFIX parameter to ecm_generate_headers to match namespace KNS3
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114988/#review47294 --- Well, then it breaks lowercase headers, which have been knewstuff3/entry.h at least since 2009. Looking at kdelibs4: install_manifest.txt:/d/kde/inst/kde4.12/include/knewstuff3/knewstuffaction.h install_manifest.txt:/d/kde/inst/kde4.12/include/knewstuff3/knewstuffbutton.h install_manifest.txt:/d/kde/inst/kde4.12/include/KDE/KNS3/KNewStuffAction install_manifest.txt:/d/kde/inst/kde4.12/include/KDE/KNS3/KNewStuffButton (and no forwarding headers for DownloadDialog, DownloadWidget, DownloadManager, UploadDialog or Entry) This is inconsistent indeed. ecm_generate_prefix assumes that the prefix for camelcase and for lowercase headers is the same, apart from the case. Oh! For sure there's a bug, I meant PREFIX KNewStuff3, rather than knewstuff3. This needs to be fixed. I think this is more consistent than this patch, which will lead to kns3/entry.h etc. This would be a much bigger SIC than changing the prefix for camelcase headers, since KNewStuffButton is now Button anyway, and KNewStuffAction... hmm I didn't generate a forwarding header for it since it doesn't actually define such a class, but we can add that for SC reasons. If we fix the PREFIX to KNewStuff3 then we get -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/KNewStuff3/DownloadDialog -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/KNewStuff3/DownloadWidget -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/KNewStuff3/UploadDialog -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/KNewStuff3/Button -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/KNewStuff3/DownloadManager -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/KNewStuff3/Entry -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/knewstuff3/knewstuff_export.h -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/knewstuff3/knewstuffaction.h -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/knewstuff3/knewstuffbutton.h -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/knewstuff3/downloaddialog.h -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/knewstuff3/downloadwidget.h -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/knewstuff3/downloadmanager.h -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/knewstuff3/uploaddialog.h -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/knewstuff3/entry.h -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KNewStuff3/knewstuff3/button.h Looks nice and consistent to me (including consistent with the other frameworks). - David Faure On Jan. 12, 2014, 8:15 p.m., Friedrich W. H. Kossebau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114988/ --- (Updated Jan. 12, 2014, 8:15 p.m.) Review request for KDE Frameworks, Aleix Pol Gonzalez and David Faure. Repository: knewstuff Description --- Currently the KNewStuff CamelCase forwarding headers are installed in [...]/include/KF5/KNewStuff3/knewstuff3 Which seems wrong, at least does not follow the pattern seen with the other namespaced modules. And breaks all existing #includes if the build does not use KDE4Support with its variants of the CamelCase forwarding headers. Attached patch changes the PREFIX parameter to ecm_generate_headers from knewstuff3 to KNS3, so that the prefix matches the namespace of the classes in the module. Diffs - src/CMakeLists.txt 05cd500 Diff: https://git.reviewboard.kde.org/r/114988/diff/ Testing --- Applied and run make install: CamelCase includes are installed in [...]/include/KF5/KNewStuff3/KNS3 directory, code which #includes KNS3/* without KDE4Support builds. Thanks, Friedrich W. H. Kossebau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KNewStuff3 vs. KNS3 vs. KNewStuff (was: Re: What are the plans with CamelCase includes?)
On Saturday 11 January 2014 02:42:20 Friedrich W. H. Kossebau wrote: There the used namespace does not match the module name: namespace is KNS3, the module name KNewStuff3. That's not a problem, the KIOCore module uses namespace (and therefore prefix) KIO. I just saw this mail, after my reply to reviewboard. It seems that I missed one thing: that the actual C++ namespace is KNS3. Then there is indeed the option of making it KNS3/File and kns3/file.h, for more consistency (this time with the C++ namespace), at the cost of a greater SIC. But you could install knewstuff3/file.h forwarding headers for compatibility. -- 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
Re: kwalletd about to be switched from kde-runtime to kwallet-framework
On Sunday 12 January 2014 19:59:26 Valentin Rusu wrote: Hello, Please be informed that I successfully imported the git history of kwalletd from kde-runtime to kwallet-framework, the I ported it to KF5. The code is ready to be pushed, but there are 430 commits waiting on my local copy. I filed a sysadmin ticket and as soon as it gets handled, I'll push it. After that, and if no one objects, I'll do the git rm -rf kwalletd from frameworks branch of kde-runtime, also adjusting the main CMakeLists.txt. Sounds good, thanks! Do you also plan on doing something with the kwallet repository, i.e. the user-facing application? I guess it makes some sense for it to rename separate, but maybe in that case its git repo should be kwalletmanager and kwallet-framework should be kwallet (the -framework suffix was just a quick hack to solve the name clash when splitting out the repos). -- 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
Re: Review Request 114888: Avoid // in path in generated headers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114888/#review47300 --- Ship it! Ship It! - David Faure On Jan. 13, 2014, 10:17 a.m., Dan Vrátil wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114888/ --- (Updated Jan. 13, 2014, 10:17 a.m.) Review request for Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- When the RELATIVE parameter in ECMGenerateHeaders is empty or not set, the _actualheader variable would be defined to /cmake/current/source/dir//lowercaseclassname.h. The double slash breaks RPM's debug symbols extraction, because it canonicalizes the path, so it's shortened by one /. The shortening is done to avoid double-slash in the beginning of the path, which is non-POSIX. However from my understanding it's not possible to truncate the size of directory table by 1 byte, so the debugedit utility aborts and the entire rpmbuild fails. See a relevant bug report with further information: https://bugzilla.redhat.com/show_bug.cgi?id=304121 (comment #2) This patch simply makes sure that EGH_RELATIVE is either empty, or non-empty and terminated with slash. With this patch we are able to build solid and kdnssd frameworks with RPM build tools. Diffs - modules/ECMGenerateHeaders.cmake f72b1c0 Diff: https://git.reviewboard.kde.org/r/114888/diff/ Testing --- Successfully built solid and kdnssd frameworks with rpmbuild, other frameworks still build too. Thanks, Dan Vrátil ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KNewStuff3 vs. KNS3 vs. KNewStuff (was: Re: What are the plans with CamelCase includes?)
On Monday 13 January 2014 17:50:25 Friedrich W. H. Kossebau wrote: Those knewstuff3/file.h forwarding headers you are proposing, they would be installed from KDE4Support, right? To [...]/include/KF5/knewstuff3, with the pattern... file entry.h contains: #include kns3/entry.h Ah, good idea. There's a bunch of forwarding headers I wrote for SC reasons that should move to kde4support too... Well, including knewstuffbutton.h, for knewstuff. You'll move it too? And all include/KF5/KDE/KNS3/* forwarding headers would be changed from #include knewstuff3/file.h to #include kns3/file.h as well. Yep. Would update/prepare RRs for both kde4support and knewstuff modules, if I got the proposal right and noone objects. Sounds good to me. -- 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
Re: kwalletd about to be switched from kde-runtime to kwallet-framework
On Monday 13 January 2014 16:48:52 Nicolás Alvarez wrote: 2014/1/13 David Faure fa...@kde.org: On Sunday 12 January 2014 19:59:26 Valentin Rusu wrote: Hello, Please be informed that I successfully imported the git history of kwalletd from kde-runtime to kwallet-framework, the I ported it to KF5. The code is ready to be pushed, but there are 430 commits waiting on my local copy. I filed a sysadmin ticket and as soon as it gets handled, I'll push it. After that, and if no one objects, I'll do the git rm -rf kwalletd from frameworks branch of kde-runtime, also adjusting the main CMakeLists.txt. Sounds good, thanks! Do you also plan on doing something with the kwallet repository, i.e. the user-facing application? I guess it makes some sense for it to rename separate, but maybe in that case its git repo should be kwalletmanager and kwallet-framework should be kwallet (the -framework suffix was just a quick hack to solve the name clash when splitting out the repos). If you rename kwallet to kwalletmanager and kwallet-framework to kwallet, people with a clone of kwallet.git will get a mess the next time they pull from the server, as the entire repository/history will have changed. Maybe kwallet - kwalletmanager, then wait for a few months, then kwallet-framework- kwallet? That would give time to anyone having a kwallet clone to get errors and re-clone? -- 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
Re: Review Request 115005: Install forwarding headers for plain knewstuff3 headers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115005/#review47371 --- Ship it! Ship It! - David Faure On Jan. 13, 2014, 11:39 p.m., Friedrich W. H. Kossebau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115005/ --- (Updated Jan. 13, 2014, 11:39 p.m.) Review request for KDE Frameworks, David Faure and Jeremy Whiting. Repository: kde4support Description --- To be seen combined with https://git.reviewboard.kde.org/r/114988/ Installation prefix was changed from knewtuff3/ to kns3/ Also remove no longer needed CamelCase forwarding headers, because they are installed again with the old prefix from the KNewStuff module See also discussion http://lists.kde.org/?l=kde-frameworks-develm=138963692808275w=2 Diffs - src/CMakeLists.txt 241e0c6 src/includes/CMakeLists.txt 8781a9a src/includes/KNS3/DownloadDialog dd7ef3a src/includes/KNS3/Entry cb98675 src/includes/KNS3/KNewStuffAction 48936eb src/includes/KNS3/KNewStuffButton aa033e1 src/knewstuff3/downloaddialog.h PRE-CREATION src/knewstuff3/downloadmanager.h PRE-CREATION src/knewstuff3/downloadwidget.h PRE-CREATION src/knewstuff3/entry.h PRE-CREATION src/knewstuff3/knewstuffaction.h PRE-CREATION src/knewstuff3/knewstuffbutton.h PRE-CREATION src/knewstuff3/uploaddialog.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115005/diff/ Testing --- Thanks, Friedrich W. H. Kossebau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 114988: Fix PREFIX parameter to ecm_generate_headers to match namespace KNS3
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114988/#review47383 --- Ship it! Ship It! - David Faure On Jan. 13, 2014, 11:39 p.m., Friedrich W. H. Kossebau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114988/ --- (Updated Jan. 13, 2014, 11:39 p.m.) Review request for KDE Frameworks, Aleix Pol Gonzalez, David Faure, and Jeremy Whiting. Repository: knewstuff Description --- To be seen combined with https://git.reviewboard.kde.org/r/115005/ Currently the KNewStuff CamelCase forwarding headers are installed in [...]/include/KF5/KNewStuff3/knewstuff3 Which seems wrong, at least does not follow the pattern seen with the other namespaced modules. And breaks all existing #includes if the build does not use KDE4Support with its variants of the CamelCase forwarding headers. Attached patch changes the PREFIX parameter to ecm_generate_headers from knewstuff3 to KNS3, so that the prefix matches the namespace of the classes in the module. This means also a change of the prefix for the normal headers, but as discussed in http://lists.kde.org/?l=kde-frameworks-develm=138963692808275w=2 that is preferred over the old solution. According new backward kde4-style support is proposed in the RR linked above. Patch also * removes knewstuffbutton.h, now installed from kdesupport * removes no longer needed utility include dir cmake code (this could be found also in a few other frameworks, so there will be follow-up patches) Diffs - src/CMakeLists.txt 05cd500 src/knewstuffbutton.h 931a465 Diff: https://git.reviewboard.kde.org/r/114988/diff/ Testing --- Applied and run make install: CamelCase includes are installed in [...]/include/KF5/KNewStuff3/KNS3 directory, code which #includes KNS3/* without KDE4Support builds. Thanks, Friedrich W. H. Kossebau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Q: Rules on inclusion of own headers? how to provide header inclusion kde4-compat? (was: Re: Extending ecm_generate_headers to create cross-forwarding headers?)
On Thursday 16 January 2014 17:57:51 Friedrich W. H. Kossebau wrote: Am Mittwoch, 15. Januar 2014, 12:14:59 schrieb David Faure: On Wednesday 15 January 2014 11:01:55 Friedrich W. H. Kossebau wrote: Guess you already started to tackle that? :) Or should I give it a try tonight? Go ahead :) Some questions while I go ahead: 1. How should own headers be included, what is preferred? #include kparts/part.h or #include part.h And both the same in the public headers and in the normal sources? Myself I also use the second version, but I found a mixture in use, so wonder if one is preferred/recommended in the frameworks. They should both work just the same, I can't see how/when either one would break. I think my preference would be part.h in part.cpp (makes it easy for krazy to check the rule include your own header first), and kparts/part.h in header files -- so that if anyone copies that to their own app, it'll work. 2. How to do KDE4-compatibility for moved header content? E.g. in event.h for KDE4-compatibility the declarations which have been moved to their own headers would need to be pulled in again, at least for builds using KDE4Support Is that done by having includes in the normal file, guarded by a flag, like // KDE4 compat #if KDE4COMPATIBLE #include kparts/guiactivateevent.h #include kparts/partactivateevent.h #include kparts/partselectevent.h #endif I don't think we need a #if. The headers installed by kde4support become unavailable as soon as you stop linking kde4support. So app who want to be clean from compat stuff will eventually have to stop using such compat headers, in order to reach the goal does not link to kde4support anymore. -- 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
Re: Review Request 115097: KParts: Move each class into its own header/source file pair
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115097/#review47626 --- Ship it! Good work, thanks. I agree with the small SC break for the sake of consistency. - David Faure On Jan. 18, 2014, 3:47 a.m., Friedrich W. H. Kossebau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115097/ --- (Updated Jan. 18, 2014, 3:47 a.m.) Review request for KDE Frameworks and David Faure. Repository: kparts Description --- As discussed in http://lists.kde.org/?l=kde-frameworks-develm=138994749530457w=2 this patch ensures that all (exported) classes are declared in header files which match the class name. And while includes had to be touched the patch also does some fixup/cleanup of includes, so the headers only include what is needed and have kparts/part.h in the installed headers and use part.h in the sources. These headers have been split out as new ones: From browserextension.h: * browserarguments.h * windowargs.h * openurlevent.h * browserhostextension.h * liveconnectextension.h From event.h: * guiactivateevent.h * partactivateevent.h * partselectevent.h From part.h: * openurlarguments.h * partbase.h * readonlypart.h * readwritepart.h From htmlextension.h: * selectorinterface.h * htmlsettingsinterface.h listingextension.h has been split up even, as there is no class ListingExtension, into: * listingfilterextension.h * listingnotificationextension.h Matching adaption for KDE4Support: https://git.reviewboard.kde.org/r/115098/ There is one issue: KDE4 code relies on getting all the now moved-out classes when including browserextension.h, event.h, part.h, htmlextension.h. While for CamelCase-forwarding headers, like KParts/Part or KParts/Event in KDE4Support this can be catched by just including all the now needed headers, I found no way yet to also have proper backward support for normal includes like kparts/part.h. There is lots of code which includes that to use KParts::ReadOnlyPart. Because kparts/part.h is now also the correct path for the now-only KParts::Part declaring header from the KParts modul, I could not simply install a kde4-compat part.h from KDE4Support into KDE4Support/kparts/ and then forward include the current headers, because it will fail at least with #include kparts/part.h Would it be acceptable to simply break SC for old code including the lowercase headers (like kparts/part.h) and using classes declared in there not matching the filename (like KParts::ReadOnlyPart)? Or is there a smart workaround for the problem? One workaround I proposed in the email: The real headers would just have the fallback support directly. E.g the new part.h would have // KDE4 compat #if KDE4COMPATIBLE #include kparts/guiactivateevent.h #include kparts/partactivateevent.h #include kparts/partselectevent.h #endif to include the other headers at the places where their classes had been defined before. This would mean having still KDE4 mentioned in KF5 code :) But KDE4COMPATIBLE (or a better name) should be only defined when KDE4Support is linked as well. No strong opinion myself. I would opt for the small SC-break :) Looking at all the stuff build with kdesrc-build/kf5-qt5-build-include there are small adaptions needed for * khtml * kross * kmediaplayer * kdewebkit * ktexteditor * kprintutils * okteta Mainly stuff like -#include kparts/part.h +#include kparts/readonlypart.h Locally all fixed, would commit the adaptions directly to those modules. Diffs - src/windowargs.cpp PRE-CREATION tests/normalktm.h f3bc291 tests/notepad.h 73a6fa2 tests/parts.h 1207c2c tests/parts.cpp 872bdb8 tests/partviewer.h bfe3158 tests/terminal_test.cpp eda318a tests/testmainwindow.h 7ba44e0 src/browserinterface.cpp a008e84 src/browseropenorsavequestion.h cb8d3ed src/browseropenorsavequestion.cpp dd52608 src/browserrun.h 5a0b996 src/browserrun.cpp bcf50df src/browserrun_p.h fbfbea6 src/event.h 056d735 src/event.cpp 1d88c82 src/fileinfoextension.h e1efbc1 src/fileinfoextension.cpp 8ecb234 src/guiactivateevent.h PRE-CREATION src/guiactivateevent.cpp PRE-CREATION src/historyprovider.h f167f30 src/historyprovider.cpp 1e4733a src/htmlextension.h 66966e4 src/htmlextension.cpp 3a6df16 src/htmlsettingsinterface.h PRE-CREATION src/htmlsettingsinterface.cpp PRE-CREATION src/kde_terminal_interface.h d029e02 src/listingextension.h 0b2e1dd src/listingextension.cpp d380584 src/listingfilterextension.h PRE-CREATION src/listingfilterextension.cpp
Re: Review Request 115098: Adapt KParts compat-forwarding headers to new one-header-per-class policy in KParts
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115098/#review47627 --- Why does this remove some forwarding headers? - David Faure On Jan. 18, 2014, 3:48 a.m., Friedrich W. H. Kossebau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115098/ --- (Updated Jan. 18, 2014, 3:48 a.m.) Review request for KDE Frameworks and David Faure. Repository: kde4support Description --- Counterpart to: https://git.reviewboard.kde.org/r/115097/ Main discussion over there, please. Diffs - src/includes/KParts/OpenUrlEvent c3e9e59 src/includes/KParts/Part bcb6c5e src/includes/KParts/PartActivateEvent dabdd2f src/includes/KParts/PartBase bcb6c5e src/includes/KParts/PartManager 2dcfeb3 src/includes/KParts/PartSelectEvent dabdd2f src/includes/KParts/Plugin f73168d src/includes/KParts/ReadOnlyPart bcb6c5e src/includes/KParts/ReadWritePart bcb6c5e src/includes/KParts/StatusBarExtension 8c8a481 src/includes/KParts/TextExtension cb73ab5 src/includes/KParts/WindowArgs c3e9e59 src/kparts/listingextension.h PRE-CREATION src/CMakeLists.txt 23b0d6d src/includes/CMakeLists.txt 2b2130f src/includes/KParts/BrowserExtension c3e9e59 src/includes/KParts/BrowserHostExtension c3e9e59 src/includes/KParts/BrowserInterface 640f47b src/includes/KParts/BrowserRun b63479b src/includes/KParts/Event dabdd2f src/includes/KParts/FileInfoExtension 13c7c41 src/includes/KParts/GUIActivateEvent dabdd2f src/includes/KParts/HistoryProvider b8c3fc9 src/includes/KParts/HtmlExtension aae280e src/includes/KParts/ListingExtension 38598f9 src/includes/KParts/LiveConnectExtension c3e9e59 src/includes/KParts/MainWindow 452e115 Diff: https://git.reviewboard.kde.org/r/115098/diff/ Testing --- Thanks, Friedrich W. H. Kossebau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115024: Remove check for X11
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115024/#review47628 --- Ship it! Windows says Ctrl Alt too, so this seems correct to me. - David Faure On Jan. 15, 2014, 12:21 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115024/ --- (Updated Jan. 15, 2014, 12:21 p.m.) Review request for KDE Frameworks and Andreas Hartmetz. Repository: kxmlgui Description --- Remove check for X11 The only thing that was using it was a preprocessor branch in kkeysequencewidget.cpp, which only had branches for Mac and X11. It appears to be intended to control the order of modifiers in a key sequence description, but there is no explanation anywhere in the logs for the fact that it checks for X11. (Added Andreas as the original author of this code back in the transition to KDE 4). Update: actually, it looks like this originally used KKeyServer to get the modifier descriptions, which was only implemented for X11 and Mac. So that explains that... Diffs - CMakeLists.txt 11a5af110a101a18e4b5a36f1d7e91a34c1b09c5 src/CMakeLists.txt 29e7dfe4aa89c03778bf4137840727fb54c1332b src/config-xmlgui.h.cmake bde7885db30843a0cb241d1ee0ac9e22c762d7b3 src/kkeysequencewidget.cpp 65ff05eec6b99cdcf7db5505475f11918b76a767 Diff: https://git.reviewboard.kde.org/r/115024/diff/ Testing --- Configure, build, run tests, install. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115099: Add function ecm_generate_pri_file() to provide qmake support. Make ECMSetupVersion set PROJECT_VERSION_*
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115099/
---
Review request for Build System, Extra Cmake Modules and KDE Frameworks.
Repository: extra-cmake-modules
Description
---
Two commits:
1) Make ECMSetupVersion set PROJECT_VERSION_*
This makes it easier for other functions to access the project version,
for instance my upcoming ECM_GENERATE_PRI_FILE()
2) This file provides the function ecm_generate_pri_file().
ECM_GENERATE_PRI_FILE() creates a .pri file for a library so that qmake-based
apps can more easily use the library.
It also sets ECM_MKSPECS_INSTALL_DIR as the directory to install the .pri file
to.
Diffs
-
modules/ECMGeneratePriFile.cmake PRE-CREATION
modules/ECMSetupVersion.cmake 6c3a9959be31ee186cf173bb28585dfc52860a55
Diff: https://git.reviewboard.kde.org/r/115099/diff/
Testing
---
Adding these lines to kwidgetaddons/src/CMakeLists.txt:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KWidgetsAddons LIB_NAME KF5WidgetsAddons DEPS
widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And these to kjobwidgets:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KJobWidgets LIB_NAME KF5JobWidgets DEPS
KCoreAddons widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And I added a qmake_test subdir in kf5umbrella with qmake_test.pro saying:
QT += KArchive KJobWidgets KWidgetsAddons
SOURCES += main.cpp
- links to all the mentionned libs, including KCoreAddons (via KJobWidgets).
This requires $QMAKEPATH set to the kf5 install prefix.
Thanks,
David Faure
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115065: kdoctools renames to add 5 namespace to prevent clashes with kdelibs4
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115065/#review47675 --- docs/kde5options/man-kde5options.7.docbook https://git.reviewboard.kde.org/r/115065/#comment33834 Still works with Qt5, but remove one dash. Testcase: kate -caption KATE_KF5 docs/kde5options/man-kde5options.7.docbook https://git.reviewboard.kde.org/r/115065/#comment33835 this is gone [one can use XDG_CONFIG_HOME instead, I suppose] - David Faure On Jan. 17, 2014, 4:48 p.m., Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115065/ --- (Updated Jan. 17, 2014, 4:48 p.m.) Review request for Documentation, KDE Frameworks, Luigi Toscano, and Scarlett Clark. Repository: kdoctools Description --- Rename man pages and checkXML tool to prevent clashes with kdelibs4 credit should go to scarlett Diffs - CMakeLists.txt 74c7af5 checkXML.in.cmake d7a57c7 checkXML5.in.cmake PRE-CREATION docs/CMakeLists.txt 7e9612f docs/checkXML/CMakeLists.txt 7f8226c docs/checkXML/man-checkXML.1.docbook 2bfb3f3 docs/checkXML5/CMakeLists.txt PRE-CREATION docs/checkXML5/man-checkXML5.1.docbook PRE-CREATION docs/kde5options/CMakeLists.txt PRE-CREATION docs/kde5options/man-kde5options.7.docbook PRE-CREATION docs/kdeoptions/CMakeLists.txt a91f451 docs/kdeoptions/man-kdeoptions.7.docbook 7e62f41 docs/qt5options/CMakeLists.txt PRE-CREATION docs/qt5options/man-qt5options.7.docbook PRE-CREATION docs/qtoptions/CMakeLists.txt f1dbb6c docs/qtoptions/man-qtoptions.7.docbook a00677a Diff: https://git.reviewboard.kde.org/r/115065/diff/ Testing --- Thanks, Jonathan Riddell ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115065: kdoctools renames to add 5 namespace to prevent clashes with kdelibs4
On Jan. 19, 2014, 12:39 a.m., Luigi Toscano wrote:
The patch looks ago, I have two remarks:
- I think that kde5options should be kf5options (as it happened with
kde4-config - kf5-config)
- I'm not sure if the content of those manpages still applies. The special
options described into those files were provided (and still provided
through KDE4Support) by KCmdLineArgs, now replaced by the
QCommandLineParser. I don't see those options to be defined into
Frameworks, which means that {kde|kf}5options should be removed; are some
of them provided by Qt directly? If not, also qt5config should disappear.
Can someone from the kde-frameworks-devel list shed some light on this?
You are right, there's some cleanup to do, but also some regressions to fix.
* Some of these options are handled by Qt itself, like -caption (note: single
dash!).
* Some of them have disappeared
* But for some it's a regression, like --nocrashhandler is still parsed by
KCrash, but QCommandLineParser (when used by the app) barfs on it, since the
option isn't defined. We need a method in KCrash.
We need to go through the full list and investigate each one; this work is
necessary but not a blocker for this review request IMHO.
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115065/#review47660
---
On Jan. 17, 2014, 4:48 p.m., Jonathan Riddell wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115065/
---
(Updated Jan. 17, 2014, 4:48 p.m.)
Review request for Documentation, KDE Frameworks, Luigi Toscano, and Scarlett
Clark.
Repository: kdoctools
Description
---
Rename man pages and checkXML tool to prevent clashes with kdelibs4
credit should go to scarlett
Diffs
-
CMakeLists.txt 74c7af5
checkXML.in.cmake d7a57c7
checkXML5.in.cmake PRE-CREATION
docs/CMakeLists.txt 7e9612f
docs/checkXML/CMakeLists.txt 7f8226c
docs/checkXML/man-checkXML.1.docbook 2bfb3f3
docs/checkXML5/CMakeLists.txt PRE-CREATION
docs/checkXML5/man-checkXML5.1.docbook PRE-CREATION
docs/kde5options/CMakeLists.txt PRE-CREATION
docs/kde5options/man-kde5options.7.docbook PRE-CREATION
docs/kdeoptions/CMakeLists.txt a91f451
docs/kdeoptions/man-kdeoptions.7.docbook 7e62f41
docs/qt5options/CMakeLists.txt PRE-CREATION
docs/qt5options/man-qt5options.7.docbook PRE-CREATION
docs/qtoptions/CMakeLists.txt f1dbb6c
docs/qtoptions/man-qtoptions.7.docbook a00677a
Diff: https://git.reviewboard.kde.org/r/115065/diff/
Testing
---
Thanks,
Jonathan Riddell
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115099: Add function ecm_generate_pri_file() to provide qmake support. Make ECMSetupVersion set PROJECT_VERSION_*
On Jan. 18, 2014, 2:52 p.m., Alex Merry wrote:
modules/ECMGeneratePriFile.cmake, line 12
https://git.reviewboard.kde.org/r/115099/diff/1/?file=234571#file234571line12
What if this was TARGET target and you extracted the interface
defines etc. from that target?
That, at least, was the idea I had for generating pkg-config files.
I think this is what I was discussing with Stephen on kde-buildsystem, and it
doesn't work when these thing contain generator expressions.
Maybe not common for definitions, but at least for include dirs it never
worked. We need clean values for qmake, no generator expressions.
But if you see a way to make it work, I'm all for it ;)
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115099/#review47642
---
On Jan. 18, 2014, 11:02 a.m., David Faure wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115099/
---
(Updated Jan. 18, 2014, 11:02 a.m.)
Review request for Build System, Extra Cmake Modules and KDE Frameworks.
Repository: extra-cmake-modules
Description
---
Two commits:
1) Make ECMSetupVersion set PROJECT_VERSION_*
This makes it easier for other functions to access the project version,
for instance my upcoming ECM_GENERATE_PRI_FILE()
2) This file provides the function ecm_generate_pri_file().
ECM_GENERATE_PRI_FILE() creates a .pri file for a library so that qmake-based
apps can more easily use the library.
It also sets ECM_MKSPECS_INSTALL_DIR as the directory to install the .pri
file to.
Diffs
-
modules/ECMGeneratePriFile.cmake PRE-CREATION
modules/ECMSetupVersion.cmake 6c3a9959be31ee186cf173bb28585dfc52860a55
Diff: https://git.reviewboard.kde.org/r/115099/diff/
Testing
---
Adding these lines to kwidgetaddons/src/CMakeLists.txt:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KWidgetsAddons LIB_NAME KF5WidgetsAddons DEPS
widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And these to kjobwidgets:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KJobWidgets LIB_NAME KF5JobWidgets DEPS
KCoreAddons widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And I added a qmake_test subdir in kf5umbrella with qmake_test.pro saying:
QT += KArchive KJobWidgets KWidgetsAddons
SOURCES += main.cpp
- links to all the mentionned libs, including KCoreAddons (via KJobWidgets).
This requires $QMAKEPATH set to the kf5 install prefix.
Thanks,
David Faure
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
KCrash
KCrash automatically initializes itself with the use of Q_COREAPP_STARTUP_FUNCTION, provided that apps link to that lib. This works OK, except for one thing: it gives no chance for KCrash to add --nocrashhandler to QCommandLineParser (in the case where the app uses that). Which makes me reconsider the idea of automatic initialization, and rather adding something explicit like KCrash::initialize(parser); in all apps, when porting away from KApplication. And moving all of the startup function code into that. Otherwise, we run the risk of apps magically using KCrash without knowing it, not defining the option in QCommandLineParser, leading to no such option --nocrashhandler (and an immediate abort) when kcrash restarts the crashed app with that option. Ah! There is one alternative Getting rid of --nocrashhandler altogether and using an environment variable instead. KDE_DEBUG=1 is the existing name for this env var, in the kde4/kf5 code. I don't like it very much though, since it's not very descriptive. So how about - adding support for another env var in kcrash.cpp, say KCRASH_DISABLE=1? - changing kcrash.cpp to set that var in startProcess() (a bit tricky, this can involve kdeinit, but iirc it supports setting env vars...) (Even KCRASH_DISABLE isn't very descriptive technically; we don't disable the fact that a crash handler gets called, but the launching of drkonqi from there. But from a user point of view I guess that's all that matters). -- 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
Re: Review Request 115098: Adapt KParts compat-forwarding headers to new one-header-per-class policy in KParts
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115098/#review47725 --- Ship it! Ship It! - David Faure On Jan. 19, 2014, 5:30 p.m., Friedrich W. H. Kossebau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115098/ --- (Updated Jan. 19, 2014, 5:30 p.m.) Review request for KDE Frameworks and David Faure. Repository: kde4support Description --- Counterpart to: https://git.reviewboard.kde.org/r/115097/ Main discussion over there, please. Diffs - src/includes/KParts/WindowArgs c3e9e59 src/kparts/listingextension.h PRE-CREATION src/CMakeLists.txt 23b0d6d src/includes/CMakeLists.txt 2b2130f src/includes/KParts/BrowserExtension c3e9e59 src/includes/KParts/BrowserHostExtension c3e9e59 src/includes/KParts/Event dabdd2f src/includes/KParts/GUIActivateEvent dabdd2f src/includes/KParts/HtmlExtension aae280e src/includes/KParts/ListingExtension 38598f9 src/includes/KParts/LiveConnectExtension c3e9e59 src/includes/KParts/OpenUrlEvent c3e9e59 src/includes/KParts/Part bcb6c5e src/includes/KParts/PartActivateEvent dabdd2f src/includes/KParts/PartBase bcb6c5e src/includes/KParts/PartSelectEvent dabdd2f src/includes/KParts/ReadOnlyPart bcb6c5e src/includes/KParts/ReadWritePart bcb6c5e Diff: https://git.reviewboard.kde.org/r/115098/diff/ Testing --- Thanks, Friedrich W. H. Kossebau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KCrash
On Sunday 19 January 2014 20:44:48 Kevin Ottens wrote: What about KCRASH_NO_CRASH_HANDLER ? It's long but it makes it at least descriptive. Except that it's not technically correct, a crash handler (= a function called upon crashing) is still called, to handle autorestart functionality (when enabled by the app) and to print out some information (app foo is crashing). Or if the app installs a custom crash handler, it will be called. The only thing that this disables is drkonqi -- the command-line option has always been misnamed, although not as much as the env var :) KCRASH_DISABLE_DRKONQI is probably the only strictly-correct name. Anyhow, naming is the easy part on to actually implementing it... -- 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
Re: Review Request 115120: Clean up the CMake files (and a couple of other bits)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115120/#review47727 --- src/imageformats/CMakeLists.txt https://git.reviewboard.kde.org/r/115120/#comment33852 Remind me, who uses these desktop files? Only the kde4support code, right? - David Faure On Jan. 19, 2014, 12:23 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115120/ --- (Updated Jan. 19, 2014, 12:23 p.m.) Review request for KDE Frameworks. Repository: kimageformats Description --- Clean up the CMake files (and a couple of other bits) src/imageformats/CMakeLists.txt is now much clearer and more consistent. Also, removed an unnecessary/unimplemented method from exr.cpp. Diffs - CMakeLists.txt c39d6f322d335fa5d19e934d0f7c5c602ba1a502 cmake/FindOpenEXR.cmake b4ae656722a78e8f3494415e4583709e9b29065e src/imageformats/CMakeLists.txt 053054a54ef53b7695b244748c3e5c0f252cc8c3 src/imageformats/config-kimgio.h.cmake src/imageformats/exr.h 3ae9e16e3d6e74240e14ce8994a1d9126c4bdd36 src/imageformats/exr.cpp f8c70b7547781614b05936f893a526d12b4a41b1 src/imageformats/jp2.cpp 5be5063d034b048aa47c5491796a89bc4519d3e4 Diff: https://git.reviewboard.kde.org/r/115120/diff/ Testing --- Compiles, installs. 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 114908: Use add_definitions directly, instead of via _KDE4_PLATFORM_DEFINITIONS
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114908/#review47736 --- Looks good to me. (i.e. wait a bit in case of objections, commit in a few days if no other comments) - David Faure On Jan. 19, 2014, 1:21 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114908/ --- (Updated Jan. 19, 2014, 1:21 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Use add_definitions directly, instead of via _KDE4_PLATFORM_DEFINITIONS Setting the variable just leads to set() calls overwriting each other accidentally (as appeared to have happened in the WIN32 block). Diffs - kde-modules/KDECompilerSettings.cmake 6adbc069bf314ae1a462ffbc5abe25264dac0ac2 Diff: https://git.reviewboard.kde.org/r/114908/diff/ Testing --- KCoreAddons still compiles, and make VERBOSE=1 shows that it is setting -D_BSD_SOURCE -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -D_XOPEN_SOURCE=500, as expected. Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: CI environment, kwallet tests that pop windows
On Sunday 19 January 2014 23:02:09 Valentin Rusu wrote: Hello, kwallet-framework tests now (correctly) work when launched from a KF5 session. kwalletd pop-ups password dialogs using KF5/Qt5 (yes!). I also added SKIP for the case where there's no graphical session, thinking that the CI does not have such an environment, but I was wrong :o) QTest has a qWaitForWindowActive, but I think that my tests cannot use it, as the windows are shown by the kwalletd daemon, which is a separate process, launched via dbus activation. Does somebody has an idea how to detect the KNewPasswordDialog and KPasswordDialog instances popping-up from the tests? You can't do that, at least not portably nor easily. It would be better to have a mode where the server replies prepared answers *instead* of popping up a dialog. -- 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
Re: Review Request 115166: Add a CMakeLists.txt which wraps python setup.py
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115166/#review47881 --- Note that I fixed the build failure with kdesrc-build differently - I added a ignore-modules kapidox in kdesrc-build's include files. And I don't like that this will break when using a custom prefix and PYTHONPATH is not set (which is the case here). - David Faure On Jan. 21, 2014, 1:49 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115166/ --- (Updated Jan. 21, 2014, 1:49 p.m.) Review request for Build System, KDE Frameworks and Alex Merry. Repository: kapidox Description --- To fix build failures with kdesrc-build, add a CMakeLists.txt which wraps python setup.py. Diffs - README.md 660e9c3 CMakeLists.txt PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115166/diff/ Testing --- Built and installed with the classic CMake commands. The kapidox tools work fine as long as the installation prefix is either the system one, or defined in $PYTHONPATH. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115166: Add a CMakeLists.txt which wraps python setup.py
On Jan. 21, 2014, 2:57 p.m., David Faure wrote: Note that I fixed the build failure with kdesrc-build differently - I added a ignore-modules kapidox in kdesrc-build's include files. And I don't like that this will break when using a custom prefix and PYTHONPATH is not set (which is the case here). Alex Merry wrote: You can't really avoid that with Python packages; you can install them with --user and make sure your PATH is set correctly, or you can install them with a prefix and make sure your PYTHONPATH is set correctly (or there are a couple of other ways of telling python about custom site package dirs). Well, or you can install them in a standard location, such as for distro packages. Aurélien Gâteau wrote: Note that the breakage is at runtime, not install time. But since kdesrc-build now ignores kapidox, maybe it's simpler to discard this patch? That's one less piece to maintain. Alex Merry wrote: Sure; I have no opinion on it either way, TBH. Ah right, it won't break make install. Then I withdraw my objection, go ahead. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115166/#review47881 --- On Jan. 21, 2014, 1:49 p.m., Aurélien Gâteau wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115166/ --- (Updated Jan. 21, 2014, 1:49 p.m.) Review request for Build System, KDE Frameworks and Alex Merry. Repository: kapidox Description --- To fix build failures with kdesrc-build, add a CMakeLists.txt which wraps python setup.py. Diffs - README.md 660e9c3 CMakeLists.txt PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115166/diff/ Testing --- Built and installed with the classic CMake commands. The kapidox tools work fine as long as the installation prefix is either the system one, or defined in $PYTHONPATH. Thanks, Aurélien Gâteau ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KCrash
On Sunday 19 January 2014 20:56:23 David Faure wrote: On Sunday 19 January 2014 20:44:48 Kevin Ottens wrote: What about KCRASH_NO_CRASH_HANDLER ? It's long but it makes it at least descriptive. Except that it's not technically correct, a crash handler (= a function called upon crashing) is still called, to handle autorestart functionality (when enabled by the app) and to print out some information (app foo is crashing). Or if the app installs a custom crash handler, it will be called. The only thing that this disables is drkonqi -- the command-line option has always been misnamed, although not as much as the env var :) KCRASH_DISABLE_DRKONQI is probably the only strictly-correct name. Anyhow, naming is the easy part on to actually implementing it... Done: commit 6f0059cd976bbd2968eb8d903e797fdcfb23d3af Author: David Faure fa...@kde.org Date: Tue Jan 21 22:52:00 2014 +0100 Replace --nocrashhandler with KCRASH_AUTO_RESTARTED=1 in the kcrash code. This is how we internally detect that we are being restarted after a crash and therefore we must 1) disable drkonqi 2) delay the crashhandler, to avoid infinite respawning in case the app crashes on startup For users, KDE_DEBUG=1 remains the way to disable drkonqi. No cmdline arg anymore. -- 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
Re: Review Request 115198: Fix KDE4Support compilation
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115198/#review47984 --- Indeed - thanks for the notification of breakage. I committed a more complete fix, though. - David Faure On Jan. 21, 2014, 10:23 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115198/ --- (Updated Jan. 21, 2014, 10:23 p.m.) Review request for KDE Frameworks and David Faure. Repository: kde4support Description --- KCrash::setApplicationName and KCrash::setApplicationPath disappeared from the KCrash module 179de6d16fb9be580dbb7b15e0a56fcb990c7516, so I guess that a good fix is just stop using it. I'm unsure what's the best way though. Diffs - src/kdeui/kapplication.cpp 5a7f4c8 Diff: https://git.reviewboard.kde.org/r/115198/diff/ Testing --- Builds. 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 115141: Add a . at the end of command line option descriptions
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115141/#review47986 --- Ship it! Ship It! - David Faure On Jan. 20, 2014, 10:39 a.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115141/ --- (Updated Jan. 20, 2014, 10:39 a.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- Add a . at the end of command line option descriptions According to the QCommandLineOptions docs, this is customary, so this should make the help for these arguments match the ones Qt provides (like --help and --version), as well as application-provided ones. Diffs - src/lib/kaboutdata.cpp f24006b41524e501dc5e402e784425e99aff9ad2 Diff: https://git.reviewboard.kde.org/r/115141/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 115099: Add function ecm_generate_pri_file() to provide qmake support. Make ECMSetupVersion set PROJECT_VERSION_*
On Jan. 21, 2014, 11:02 p.m., Alex Merry wrote:
modules/ECMGeneratePriFile.cmake, lines 6-7
https://git.reviewboard.kde.org/r/115099/diff/1/?file=234571#file234571line6
It also wants PROJECT_VERSION_(MAJOR|MINOR|PATCH) (unless those are
option in the pri file)
No, these are extracted from PROJECT_VERSION_STRING, in lines 55-57.
On Jan. 21, 2014, 11:02 p.m., Alex Merry wrote:
modules/ECMGeneratePriFile.cmake, line 13
https://git.reviewboard.kde.org/r/115099/diff/1/?file=234571#file234571line13
I feel DEPS should really be a list, but CMake apparently doesn't have
a join function, for some bizarre reason (although you probably could do a
string(REPLACE) of semicolons by spaces).
I don't really mind either way, and this way works. Not sure what to do, then :)
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115099/#review47948
---
On Jan. 18, 2014, 11:02 a.m., David Faure wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115099/
---
(Updated Jan. 18, 2014, 11:02 a.m.)
Review request for Build System, Extra Cmake Modules and KDE Frameworks.
Repository: extra-cmake-modules
Description
---
Two commits:
1) Make ECMSetupVersion set PROJECT_VERSION_*
This makes it easier for other functions to access the project version,
for instance my upcoming ECM_GENERATE_PRI_FILE()
2) This file provides the function ecm_generate_pri_file().
ECM_GENERATE_PRI_FILE() creates a .pri file for a library so that qmake-based
apps can more easily use the library.
It also sets ECM_MKSPECS_INSTALL_DIR as the directory to install the .pri
file to.
Diffs
-
modules/ECMGeneratePriFile.cmake PRE-CREATION
modules/ECMSetupVersion.cmake 6c3a9959be31ee186cf173bb28585dfc52860a55
Diff: https://git.reviewboard.kde.org/r/115099/diff/
Testing
---
Adding these lines to kwidgetaddons/src/CMakeLists.txt:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KWidgetsAddons LIB_NAME KF5WidgetsAddons DEPS
widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And these to kjobwidgets:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KJobWidgets LIB_NAME KF5JobWidgets DEPS
KCoreAddons widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And I added a qmake_test subdir in kf5umbrella with qmake_test.pro saying:
QT += KArchive KJobWidgets KWidgetsAddons
SOURCES += main.cpp
- links to all the mentionned libs, including KCoreAddons (via KJobWidgets).
This requires $QMAKEPATH set to the kf5 install prefix.
Thanks,
David Faure
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115099: This file provides the function ecm_generate_pri_file(). Make ECMSetupVersion set PROJECT_VERSION_*
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115099/
---
(Updated Jan. 22, 2014, 9:21 a.m.)
Review request for Build System, Extra Cmake Modules and KDE Frameworks.
Changes
---
Updated diff
Summary (updated)
-
This file provides the function ecm_generate_pri_file(). Make ECMSetupVersion
set PROJECT_VERSION_*
Repository: extra-cmake-modules
Description (updated)
---
This file provides the function ecm_generate_pri_file().
ECM_GENERATE_PRI_FILE() creates a .pri file for a library so that qmake-based
apps can more easily use the library.
It also sets ECM_MKSPECS_INSTALL_DIR as the directory to install the .pri file
to.
REVIEW: 115099
Make ECMSetupVersion set PROJECT_VERSION_*
This makes it easier for other functions to access the project version,
for instance my upcoming ECM_GENERATE_PRI_FILE()
Diffs (updated)
-
modules/ECMGeneratePriFile.cmake PRE-CREATION
modules/ECMSetupVersion.cmake 6c3a9959be31ee186cf173bb28585dfc52860a55
Diff: https://git.reviewboard.kde.org/r/115099/diff/
Testing
---
Adding these lines to kwidgetaddons/src/CMakeLists.txt:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KWidgetsAddons LIB_NAME KF5WidgetsAddons DEPS
widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And these to kjobwidgets:
include(ECMGeneratePriFile)
ecm_generate_pri_file(BASE_NAME KJobWidgets LIB_NAME KF5JobWidgets DEPS
KCoreAddons widgets FILENAME_VAR PRI_FILENAME)
install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
And I added a qmake_test subdir in kf5umbrella with qmake_test.pro saying:
QT += KArchive KJobWidgets KWidgetsAddons
SOURCES += main.cpp
- links to all the mentionned libs, including KCoreAddons (via KJobWidgets).
This requires $QMAKEPATH set to the kf5 install prefix.
Thanks,
David Faure
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115210: Always set DATA_INSTALL_DIR to %ALLUSERSPROFILE% on Windows
On Jan. 22, 2014, 8:22 a.m., Patrick von Reth wrote: Until now we had no problems with the data installed to bin/../share and this setup would make it impossible to have multiple independent kde setups on one system. Alexander Richardson wrote: I know. The problem is QStandardPaths with QStandardPaths::GenericDataLocation only looks in %ALLUSERSPROFILE% and I think %APPDATA%. KF5 based KDE software won't work otherwise since it can't find the data. I think the better way of fixing this is patching Qt, but for now this works. Patrick Spendrin wrote: Can you keep that patch locally for now and we try and come up with a patch for Qt instead? We cannot restrict ourselves at that point I think. Alexander Richardson wrote: Sure no problem. I'll drop this request So what do you recommend instead, for QStandardPaths? Checking some non-standard environment variable? or? - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115210/#review47983 --- On Jan. 22, 2014, 2:53 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115210/ --- (Updated Jan. 22, 2014, 2:53 p.m.) Review request for Build System, Extra Cmake Modules, KDE Frameworks, and kdewin. Repository: extra-cmake-modules Description --- Always set DATA_INSTALL_DIR to %ALLUSERSPROFILE% on Windows Otherwise QStandardPaths will always fail with e.g. GenericDataLocation Diffs - kde-modules/KDEInstallDirs.cmake 46e15c17d488d56f146aba0c2d420f74a22b9152 Diff: https://git.reviewboard.kde.org/r/115210/diff/ Testing --- Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115210: Always set DATA_INSTALL_DIR to %ALLUSERSPROFILE% on Windows
On Jan. 22, 2014, 8:22 a.m., Patrick von Reth wrote: Until now we had no problems with the data installed to bin/../share and this setup would make it impossible to have multiple independent kde setups on one system. Alexander Richardson wrote: I know. The problem is QStandardPaths with QStandardPaths::GenericDataLocation only looks in %ALLUSERSPROFILE% and I think %APPDATA%. KF5 based KDE software won't work otherwise since it can't find the data. I think the better way of fixing this is patching Qt, but for now this works. Patrick Spendrin wrote: Can you keep that patch locally for now and we try and come up with a patch for Qt instead? We cannot restrict ourselves at that point I think. Alexander Richardson wrote: Sure no problem. I'll drop this request David Faure wrote: So what do you recommend instead, for QStandardPaths? Checking some non-standard environment variable? or? Alexander Richardson wrote: I would go for the environment variable. Something like QSTANDARDPATHS_EXTRA_DATA_DIRS that is checked in addition to the default dirs. Would also be useful for other cases: e.g. in the okteta unit tests I set XDG_DATA_DIRS so that my test data gets found by QStandardPaths (I know there is QFINDTESTDATA, but that won't work in that case). It would also be nice if there were some cross-platform solution like QStandardpaths::addDirectory(QStandardPaths::StandardLocation, const QString path) to inject (like KStandardDirs::addResourceDir). Patrick von Reth wrote: I don't like the idea of using the env var as this would require the user to setup the variables or a kde process to set them up. We also would get an undefined behaviour if the env var is not set. I think kde is not the only qt project ported to windows wich uses the bin/../share location on windows, so why not only add this path with a low priority to QStrandardpathes? I agree that the env var would be quite inconvenient, which is why I was dubious about that approach. A method to add paths wouldn't help either (how would all apps do it?) bin/../share means go up one level from the location of the executable and enter share? I thought Windows apps didn't use a bin/ dir actually, but were rather in the toplevel? Anyhow I'd be fine with that, especially if you can find any documentation of this outside of kde (to explain the reasoning in the Qt change request). - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115210/#review47983 --- On Jan. 22, 2014, 2:53 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115210/ --- (Updated Jan. 22, 2014, 2:53 p.m.) Review request for Build System, Extra Cmake Modules, KDE Frameworks, and kdewin. Repository: extra-cmake-modules Description --- Always set DATA_INSTALL_DIR to %ALLUSERSPROFILE% on Windows Otherwise QStandardPaths will always fail with e.g. GenericDataLocation Diffs - kde-modules/KDEInstallDirs.cmake 46e15c17d488d56f146aba0c2d420f74a22b9152 Diff: https://git.reviewboard.kde.org/r/115210/diff/ Testing --- Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115234: Only set QT_STRICT_ITERATORS when not compiling with MSVC
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115234/#review48092 --- Ship it! Oh, OK, didn't know this was broken on MSVC. Worth a Qt fix or at least bugreport? - David Faure On Jan. 22, 2014, 5:51 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115234/ --- (Updated Jan. 22, 2014, 5:51 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Only set QT_STRICT_ITERATORS when not compiling with MSVC On MSVC linker errors will happen when this flag is set. Diffs - kde-modules/KDEFrameworkCompilerSettings.cmake d71c407f9c0b504ebb1c0cf662e69545f7a46371 Diff: https://git.reviewboard.kde.org/r/115234/diff/ Testing --- E.g. KConfigWidgets didn't compile before, compiles now Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115236: Get closer to compiling KIO on windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115236/#review48093 --- Has this been compile-tested on Linux? - David Faure On Jan. 22, 2014, 9:12 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115236/ --- (Updated Jan. 22, 2014, 9:12 p.m.) Review request for KDE Frameworks. Repository: kio Description --- 3 Commits to get closer to compiling on windows: 1. Include qplatformdefs.h where possible unistd.h and others are not available on e.g. Windows, qplatformdefs.h includes the equivalent for each platform 2. add include(CheckLibraryExists) to CMakeLists.txt On Linux this is apparently pulled in by some other file whereas it is missing on Windows 3. Make KIO::MetaData completely inline to prevent linker errors on MSVC Diffs - src/ioslaves/ftp/ftp.h ad94979829a5d0e71ec57177b51f67e115c5445e src/ioslaves/ftp/ftp.cpp 9ea642587bd754f0b4295fcb7d4db1b427f4326e src/ioslaves/help/kio_help.h 0eab4ce1b393b48e552ca94d877f4c069450cee0 src/ioslaves/http/http.cpp 4f97b335c0e206f0a2ff8cfd515d0fae79614ad2 src/ioslaves/http/http_cache_cleaner.cpp ffa8ab9580acf554b27027617cdac06b3f7755bf src/widgets/kdirmodel.cpp ce6e4c9aaddada5715b5aeef36ad163c77d3c635 src/widgets/kpropertiesdialog.cpp 8ddd37f0326e37109ce239a46bfa67d2f4c35411 src/widgets/krun.cpp 92dcfd8da04b9f3d80b632a37758017c088093ab src/widgets/kurlcompletion.cpp ed77fd3213db0524b5a934c94eb7645d98bd27c7 tests/kioslavetest.cpp dc56240dc166dfa93ec099db5cdd262cc06249d4 tests/kruntest.cpp 68720ddf788e6b44468baf18dd04937094741274 CMakeLists.txt 9894ad5e1696fe31d8a3f065d29747cde0474d2c autotests/fileundomanagertest.cpp 4ddee49eed254be6379a1302af2dd17aae28c15c autotests/kurlcompletiontest.cpp 10048e2e15059596e446b03cedb0b302bf038cd5 src/core/chmodjob.cpp b60cb9bf3d3b1a71a795ee5db7c72253dbdf0ad2 src/core/kacl.h 3c9c88d0408c855d02eaacb34d14f45c55343eb2 src/core/kacl.cpp 201b30a02ef93a137b9a8507cabaece6926d8739 src/core/kfileitem.h 553dbf8ccf96983803d0224cf8dc9d72f0107b6a src/core/kfileitem.cpp fdc0fc0279713887dc18ce1da8d3b00d422f6a9b src/core/kprotocolmanager.cpp a8746a4f9a78e6a01eaed6a70e99146ff40129d2 src/core/krecentdocument.cpp ad0a97e7c1af85a94bb7483124b5035f0fca38a6 src/core/metadata.h cd62e3b009c21485537ddd5449a6c347748c3caf src/core/metadata.cpp ad05032dff58314a305256993fcc7fe7b94751bc src/core/slave.h 43b5cd8e8aa11da55d6f8a8d9fdf4eb3d0780d11 src/core/slave.cpp ef9b3c7a57b4f4384343c4d4296ec88add857eb6 src/core/slavebase.cpp a7ac4d5a6a87c256da9a3947b7223649927eba39 src/core/slaveinterface.h d75eb6b02374375a73e1aaa57d5c2979a3bc365f src/core/slaveinterface.cpp faa4bd7dc1b5a733f0060f60626e8a0313bfea8a src/core/slaveinterface_p.h 0ed4980a06f8aa8b3fc8c0717e4a88991483e98a src/filewidgets/kfileplaceeditdialog.cpp a7c433c8baecca8082f8d80077deab68e4b0cec2 src/filewidgets/knewfilemenu.cpp dfc086b6e98b965739c062d7291bbd9139633beb src/ioslaves/file/file.h 453298159791a78d877c4d81d2be73db073db3f2 src/ioslaves/file/file.cpp e3ede0daa8054fc33d12acb9966a707f9484cd98 src/ioslaves/file/file_win.cpp 53e0f8f133bd5c4cbbd07afa945854efffc7297c Diff: https://git.reviewboard.kde.org/r/115236/diff/ Testing --- Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KIO::convertSize(.,.) vs. KFormat::formatByteSize(...)
On Thursday 23 January 2014 23:43:36 Friedrich W. H. Kossebau wrote:
Hi,
I see a few overlappings between methods in KFormat (KCoreAddons) and KIO
(KIOCore), mainly this pair:
namespace KIO
{
typedef qulonglong filesize_t;
KIOCORE_EXPORT QString convertSize(KIO::filesize_t size);
}
and
class KCOREADDONS_EXPORT KFormat Q_DECL_FINAL
{
QString formatByteSize(double size,
int precision = 1,
KFormat::BinaryUnitDialect dialect =
KFormat::DefaultBinaryDialect,
KFormat::BinarySizeUnits units =
KFormat::DefaultBinaryUnits) const;
};
I think it makes sense to have two methods, at two different layers.
The KFormat one is like Qt: the caller of the method decides what they want.
The KIO one is as we like things in KDE, very often: it takes into account the
user's preference, for consistency across all KDE applications.
The KF5/Qt5 trend is to make that happen automatically behind the scenes of
the lowlevel (e.g. Qt) API, but that doesn't work for a method that is so
low-level that it actually takes the settings as parameters :)
Questions:
Q1) What config files can be expected from KF5 modules?
So can KIO::convertSize(...) (which is already KDE4 code) expect such config
files to exist? Only in a Plasma workspace platform, or? How is platform
integration done in the KDE frameworks? E.g. in Unity, GNOME Shell, Win,
etc. I would expect that any matching config data is picked, if there is,
otherwise a hardcoded default. http://community.kde.org/Frameworks/Policies
does not mention that yet, but I guess that has been discussed before?
Well, does a setting exist for how to format byte sizes (including the
choice between GB and GiB) in all these frameworks? I seriously doubt that
So it seems to me that picking this from a KConfig file for which we have a
KCM is the only solution?
Q2) Should KIO::convertSize(...) not use KFormat::formatByteSize(...) behind
the scenes?
Yes, quite probably, unless we're missing something about the details of what
it needs. Feel free to dig into that :)
Q3) Is double as type of parameter size for KFormat::formatByteSize(...)
really a good choice?
I would expect the type to be rather qulonglong, like it is with
KIO::convertSize(...). I have looked at many files with Okteta, but so for
not seen a file with a fraction byte ;)
Indeed. I wonder if this is historical from before qulonglong existed (you
know, a few minutes after the Big Bang, or 2001, in other words).
--
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
Re: Review Request 115186: rename dbus interface file for kjobviewer
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115186/#review48257 --- Ship it! Ship It! - David Faure On Jan. 21, 2014, 5:02 p.m., Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115186/ --- (Updated Jan. 21, 2014, 5:02 p.m.) Review request for KDE Frameworks. Repository: kjobwidgets Description --- This dbus interface file also gets installed in kdelibs4 which overlaps it in kf5, some distros can not handle this. So rename the file on install. The dbus interface remains the same. This is the same technique used for kglobalaccel. Diffs - src/CMakeLists.txt 7527a0a Diff: https://git.reviewboard.kde.org/r/115186/diff/ Testing --- Thanks, Jonathan Riddell ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115300: Fix KKeySequence shortcut types flags
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115300/#review48258 --- Ship it! Hehe, nice find. The bug is in KDE 4.x too, but fixing it would be BIC, so yeah. - David Faure On Jan. 24, 2014, 4:13 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115300/ --- (Updated Jan. 24, 2014, 4:13 p.m.) Review request for KDE Frameworks. Repository: kxmlgui Description --- I assume the code was /trying/ to default to checking against local and global shortcuts, in practice it was only ever going to select local shortcuts. Diffs - src/kkeysequencewidget.cpp cc9016b src/kkeysequencewidget.h d5d70ea Diff: https://git.reviewboard.kde.org/r/115300/diff/ Testing --- Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: kf5options and qt5options manpages
On Tuesday 21 January 2014 18:38:28 Jonathan Riddell wrote: kdeoptions and qtoptions manpages listed the common options to applications using kdelibs4 and qt4. These have just become kf5options and qt5options. But which options are still true? From qt5options: All of these are now single-dash again (like Qt always made them, it's just that kcmdlineargs added double-dash equivalents for them). --display displayname Use the X-server display displayname. -display works --session sessionId Restore the application for the given sessionId. -session is still there. --cmap Causes the application to install a private color map on an 8-bit display. Remove. --ncols count Limits the number of colors allocated in the color cube on an 8-bit display, if the application is using the QApplication::ManyColor color specification. Remove. --nograb Tells Qt(TM) to never grab the mouse or the keyboard. --dograb Running under a debugger can cause an implicit --nograb, use --dograb to override. -nograb and -dograb are still there --sync Switches to synchronous mode for debugging. Seems to be gone, maybe due to the switch to XCB. --fn,--font fontname Defines the application font. --bg,--background color Sets the default background color and an application palette (light and dark shades are calculated). --fg,--foreground color Sets the default foreground color --btn,--button color Sets the default button color. Can't see any of this in the Qt code - remove. --name name Sets the application name. -name exists, in the XCB code, it sets the WM_CLASS. This docu should probably be made much more accurate. --title title Sets the application title (caption). Gone. --visual TrueColor Forces the application to use a TrueColor visual on an 8-bit display. Gone. --inputstyle inputstyle Sets XIM (X Input Method) input style. Possible values are onthespot, overthespot, offthespot and root. --im XIM server Set XIM server. --noxim Disable XIM Can't find anywhere, remove. --reverse mirrors the whole layout of widgets -reverse works --stylesheet file.qss applies the Qt stylesheet to the application widgets New in qt5: -platform qpa_platform_name -qwindowgeometry (as the portable equivalent for -geometry, which only works on xcb) -plugin plugin_name, not sure what it's used for exactly These should be documented in the QGuiApplication ctor docu but they are not, the list there is incomplete... From kf5options: --caption caption Use caption as name in the titlebar. Ah, this is a kde4support thing (kcmdlineargs). If we think this is useful, we should add it to Qt. Is it useful? --icon icon Use icon as the application icon. Same thing. --config filename Use the alternative configuration filename kde4support only - remove [one can use XDG_CONFIG_HOME instead, I suppose]. --nocrashhandler Disable the crash handler, to get core dumps. Removed, set KDE_DEBUG=1 instead. --waitforwm Waits for a WM_NET compatible windowmanager. kde4support only (- remove). --style style Sets the application GUI style. -style (is a Qt option, move it there) Add -stylesheet too See QApplication class docu for these. --geometry geometry Sets the client geometry of the main widget. Was from Qt. -geometry still works, on X11. -- 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
Re: Splitting kde-workspace and kde-runtime proposal
On Monday 27 January 2014 15:21:05 David Edmundson wrote: There is an existing page about slitting runtime here: http://community.kde.org/Frameworks/Epics/New_Runtime_Organization linked to from http://community.kde.org/Frameworks/Epics Alex's wiki page looks far more populated. We should make sure we avoid wiki duplication. Yeah, Alex, can you look into merging the two tables? Then I'll go through and fill in info about the stuff I know about. -- 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
Re: Splitting kde-workspace and kde-runtime proposal
On Thursday 23 January 2014 16:08:51 andrea diamantini wrote: I don't clearly understand why KUriFilter-Plugins should go to plasma- workspace. I noticed KUriFilter is defined in kio and its plugins are used e.g. in kparts (browserextension). Shouldn't these go to kio? Agreed. They are needed for kio-based webbrowsers in any workspace. -- 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
Re: Review Request 115207: Improve integration QCommandLineParser - KAboutData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115207/#review48455 --- I agree with Kévin: this doesn't match what the function name says it does. Better make it a separate function. - David Faure On Jan. 21, 2014, 11:36 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115207/ --- (Updated Jan. 21, 2014, 11:36 p.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- Let the KAboutData set information to QApplication. This way we don't have to duplicate information by passing it to the KAboutData _and_ the QApplication. Diffs - src/lib/kaboutdata.h c9e src/lib/kaboutdata.cpp f24006b Diff: https://git.reviewboard.kde.org/r/115207/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115360: Remove the allocator and visibility check
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115360/#review48514 --- Ship it! Ship It! - David Faure On Jan. 28, 2014, 10:43 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115360/ --- (Updated Jan. 28, 2014, 10:43 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Repository: extra-cmake-modules Description --- Remove the allocator and visibility check I am reasonably sure the allocator check is out of date, given our minimum GCC version, and it was not used for anything interesting anyway. The visibility check will not be performed in practice, as this file will almost always be included before any check for Qt. Diffs - kde-modules/KDECompilerSettings.cmake ba9b03f1c86061dd740960220b6411bbce541617 Diff: https://git.reviewboard.kde.org/r/115360/diff/ Testing --- Everything kdesrc-build knows about builds (GCC 4.8.2 20131219; Linux with glibc 2.18). Thanks, Alex Merry ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: kf5options and qt5options manpages
On Saturday 25 January 2014 16:36:22 Albert Astals Cid wrote: El Dissabte, 25 de gener de 2014, a les 10:20:20, David Faure va escriure: On Tuesday 21 January 2014 18:38:28 Jonathan Riddell wrote: kdeoptions and qtoptions manpages listed the common options to applications using kdelibs4 and qt4. These have just become kf5options and qt5options. But which options are still true? --caption caption Use caption as name in the titlebar. Ah, this is a kde4support thing (kcmdlineargs). If we think this is useful, we should add it to Qt. Is it useful? --icon icon Use icon as the application icon. Isn't this kind of mandated by the desktop entry spec that says that %i will be translated to --icon? Besides ./kio/src/core/desktopexecparser.cpp seems to use it. You're somewhat right. Note that the .desktop file for an app doesn't have to use %i, if the app doesn't support --icon. I can see the idea of the feature, using the Icon field in the .desktop file for both menus and the app window icon, but my question is whether it's really used/useful in practice. In my kde4 applications dir, I see 16 .desktop files using %i, plus the 14 .desktop files for okular but that's just one app, so 17 apps in total. However, I suspect that most of these would work just the same without %i, since they use the default icon name anyway (e.g. ktorrent.desktop, Icon=ktorrent). Plus, the apps need a good default icon anyway, for the case where they started another way (e.g. from the command line). Not sure about -caption but i'd say it may make sense too in some cases. I actually see more possible use cases for -caption, in custom setups (e.g. someone preparing custom desktop files for users to do specific tasks, the window title can make it very clear what a particular window instance is for) I'll see about adding it to Qt. It's also easier, because we can make it single dash... I guess I should also make Qt support double-dash for its builtin options... -- 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
Re: Review Request 115415: Allow kpac_proxyscout to be built again
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115415/#review48737 --- Ship it! Oops, well spotted. - David Faure On Jan. 31, 2014, 5:57 p.m., Michael Palimaka wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115415/ --- (Updated Jan. 31, 2014, 5:57 p.m.) Review request for KDE Frameworks and David Faure. Repository: kio Description --- In 03a83c76f29935e8d11bd7f8470bbc72e821dae5, QtScript was removed from the main Qt find_package because it's optional, but it was never readded anywhere. So, kpac_proxyscout would never be built. Diffs - src/kpac/CMakeLists.txt c3327060bfd6479a008a4ca5e3d2e6f2dc69ceb1 Diff: https://git.reviewboard.kde.org/r/115415/diff/ Testing --- It never built, and now it does. Thanks, Michael Palimaka ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Tier status of attica kwallet
On Friday 24 January 2014 20:33:14 Valentin Rusu wrote: On Saturday, January 25, 2014 01:49:51 AM Michael Palimaka wrote: On 01/24/2014 09:21 AM, Alex Merry wrote: On 23/01/14 21:50, Valentin Rusu wrote: On Thursday, January 23, 2014 11:18:02 PM Michael Palimaka wrote: On 01/23/2014 08:21 AM, Valentin Rusu wrote: On Thursday, January 23, 2014 04:24:37 AM Michael Palimaka wrote: Sure, the framework itself is still tier 2...but the repo also includes kwalletd which definitely is not tier 2, and there does not appear to be any option to control building them independently. I'll add that option asap. Is that useful? Would anyone have any use for the library without the daemon? Once, David F. told on this mailing list that such an option would let an Qt application compile against KF5Walllet. The daemon is a runtime dependency. I can't remember if I said that, and if so what I had in mind. Compiling against a crippled framework and then missing the daemon for it to work at runtime sounds a bit strange. -- 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
Re: Review Request 115148: Add KWINDOWSYSTEM_ namespace to HAVE_FOO defines
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115148/#review48745 --- This is the only framework that installs a config-foo.h file, yes. And yes, that is a hack, it would be much better not to do that in the first place... - David Faure On Jan. 27, 2014, 12:30 p.m., Alex Merry wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115148/ --- (Updated Jan. 27, 2014, 12:30 p.m.) Review request for KDE Frameworks. Repository: kwindowsystem Description --- Add KWINDOWSYSTEM_ namespace to HAVE_FOO defines config-kwindowsystem.h is installed and included by public headers, so it should not define things as generic as HAVE_X11, which are likely to also be defined by users of the framework. Diffs - src/netwm.cpp 266afccaf111e6707493b18dad1d9c03dde1d912 src/netwm.h aee6cea5e3b65cf6252b50515e4920cb9c96f240 src/kxerrorhandler.cpp 612e62a345ec9c8503bab9fb12afa08426974828 src/kxerrorhandler_p.h ad670fec6ff35e1c9df81b91517c02e6e0893ad1 src/kxmessages.h 5d57e06d702afb5ceaed05de97ca40116b7f3aa3 src/kxmessages.cpp b409ef2114c18190188ba5503d2f357bd9336e76 src/kxutils.cpp e47ba68c08a14597e49fd33901b0a7c079924112 src/kxutils_p.h 1e229801aac929bc596685dbfa5260d55d0f5298 src/kusertimestamp.cpp de8ca61e7e9dd0ae9492ccf61883560d80501e2b src/kstartupinfo.cpp 5dbf47cb666fbed17c943491efe93e17f27d581e src/kkeyserver.h 8869fe7331e98846183ab37814b1d95e75ab9647 src/config-kwindowsystem.h.cmake 58949d87f4254531ba57de86b6303cba05053222 src/CMakeLists.txt b453af11a46615ecd94911f0d2f86922087bde0e CMakeLists.txt 2479237e574e86c9094b557dfa146c85b7b19b65 src/kwindoweffects.cpp 91ef52f149a781df9308bb1587629dbaaf571b8e src/kwindoweffects_p.h 266b5f3e7f24bdca6ce467138ee067335225da78 src/kwindowsystem.h 5bcfd062dcca42d282b70d0683d4ee1da88cc814 src/kwindowsystem_x11.cpp 8634240cabc708a608277b34f21c41cee295e48a Diff: https://git.reviewboard.kde.org/r/115148/diff/ Testing --- Clean configure-build-test-install on a system with X11. KWindowSystem already failed to build on a non-windows, non-mac system when X11 is not found (one of the tests fails to link if you comment out the find_package(X11) calls, as KWindowSystem::activeWindow() is never defined). 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 115424: Improve kparts dependencies
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115424/#review48758 --- tests/CMakeLists.txt https://git.reviewboard.kde.org/r/115424/#comment34420 There's never any reason for the tests/ subdir to use Qt5Test. Qt5Test = library for autotests. - David Faure On Feb. 1, 2014, 6:28 p.m., Michael Palimaka wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115424/ --- (Updated Feb. 1, 2014, 6:28 p.m.) Review request for KDE Frameworks and David Faure. Repository: kparts Description --- - QtNetwork is not used - QtTest is only required for tests - Remove transitive dependencies Diffs - tests/CMakeLists.txt d7bca988b59022bb0731a38a5d7e872217541810 autotests/CMakeLists.txt d62107e51d88b0ea46be113b57c318197f3ebaa9 CMakeLists.txt fe6de305d28c2cd073ae1bc5d171f1244d2ed29c Diff: https://git.reviewboard.kde.org/r/115424/diff/ Testing --- Inspection of source. Builds. Tests pass. Thanks, Michael Palimaka ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115424: Improve kparts dependencies
On Feb. 2, 2014, 9:36 a.m., David Faure wrote: tests/CMakeLists.txt, lines 1-2 https://git.reviewboard.kde.org/r/115424/diff/1/?file=241204#file241204line1 There's never any reason for the tests/ subdir to use Qt5Test. Qt5Test = library for autotests. Michael Palimaka wrote: It uses QFINDTESTDATA: notepad.cpp:setXMLFile(QFINDTESTDATA(notepadpart.rc)); parts.cpp:setXMLFile(QFINDTESTDATA(kpartstest_part1.rc)); parts.cpp:setXMLFile(QFINDTESTDATA(kpartstest_part2.rc)); partviewer.cpp:setXMLFile(QFINDTESTDATA(partviewer_shell.rc)); testmainwindow.cpp:setXMLFile(QFINDTESTDATA(kpartstest_shell.rc)) Oh I was wrong, there is *one* reason to use qtestlib ;) OK. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115424/#review48758 --- On Feb. 2, 2014, 3:09 p.m., Michael Palimaka wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115424/ --- (Updated Feb. 2, 2014, 3:09 p.m.) Review request for KDE Frameworks and David Faure. Repository: kparts Description --- - QtNetwork is not used - QtTest is only required for tests - Remove transitive dependencies Diffs - CMakeLists.txt fe6de305d28c2cd073ae1bc5d171f1244d2ed29c autotests/CMakeLists.txt d62107e51d88b0ea46be113b57c318197f3ebaa9 tests/CMakeLists.txt d7bca988b59022bb0731a38a5d7e872217541810 Diff: https://git.reviewboard.kde.org/r/115424/diff/ Testing --- Inspection of source. Builds. Tests pass. Thanks, Michael Palimaka ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
kf5 alpha 1 : modules, versions
Any new module that should be added to this release, compared to TP1? Should I include attica? Any version number that should be upgraded in the modules themselves? I realize now that it's all called 5.0.0 everywhere already. The packages are properly numbered, but not the cmake variable containing the version or the *version.h files, which all say 5.0.0 already. Option 1 - it's too late, due to the tech preview saying 5.0.0 already. Option 1 bis - this was intended from the start, no point in apps doing version checking on pre-release versions of kf5 Option 2 - tech previews don't count, let's downgrade to 4.96.0 everywhere. -- 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
Re: kf5 alpha 1 : attica?
On Monday 03 February 2014 11:34:44 Kevin Ottens wrote: On Monday 03 February 2014 10:17:49 David Faure wrote: Any new module that should be added to this release, compared to TP1? Should I include attica? Since this question keeps popping up, let's integrate it. It should also be added to the list: http://community.kde.org/Frameworks/List Yes, but see what I wrote in the Tier status of attica kwallet thread: there's some buildsystem work to be done for attica to be a proper framework (making it use ECM, so it can integrate better with the other frameworks and be fully consistent with them, including installing camelcase forwarding headers etc.), which also means moving the qt4 support into a separate branch first. Which brings us to the next topic: who as maintainer should approve this. Also, since no one stepped up to say if it should be in or out, I'd say it should be with no declared maintainer until someone claims it. I was under the impression that it had a maintainer, although right now I can't remember if that was Jeremy Whiting or Frederik Gladhorn or someone else. Cc'ing them. Guys, any input? (Note that overall this would lower the future maintainance work on attica's buildsystem, since it will just be maintained together with the other frameworks, by anyone who makes changes to ECM or across all frameworks.) -- 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
Re: kf5 alpha 1 : attica?
Frederik wrote: From my point of view, please just go ahead and change it as you think is sensible. OK, thanks for the green lights, I went ahead: * Qt4 support for attica is now in the qt4 branch * Attica master is now qt5 only, and requires ECM. * It gained all the bells and whistles of being a proper framework: camelcase forwarding headers, version upgrade to 4.96.0, .pri file, etc. And, being released together with the other frameworks. The only thing that makes attica an odd duck compared to the other frameworks is that we can't yet move it under the frameworks/ hierarchy because that would break the Qt4 build scripts. So, all KF5 hackers, please note that whenever making a change across all frameworks you should also remember attica, outside your frameworks/ subdir. -- 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
Re: kf5 alpha 1 : attica?
On Thursday 06 February 2014 00:53:53 Frederik Gladhorn wrote:
On Thursday 6. February 2014 00.29.45 David Faure wrote:
Frederik wrote:
From my point of view, please just go ahead and change it as you think
is
sensible.
OK, thanks for the green lights, I went ahead:
* Qt4 support for attica is now in the qt4 branch
* Attica master is now qt5 only, and requires ECM.
* It gained all the bells and whistles of being a proper framework:
camelcase forwarding headers, version upgrade to 4.96.0, .pri file, etc.
And, being released together with the other frameworks.
The only thing that makes attica an odd duck compared to the other
frameworks is that we can't yet move it under the frameworks/ hierarchy
because that would break the Qt4 build scripts.
So, all KF5 hackers, please note that whenever making a change across all
frameworks you should also remember attica, outside your frameworks/
subdir.
Thanks a lot, I really appreciate it.
Some issues left:
- who do we write down as maintainer for attica?
- can I run astyle on the code to make it consistent with all other
frameworks? ('kdelibs' coding style, almost like the qt one)
- does it have a component on bugs.kde.org?
- does it exist on reviewboard.kde.org?
--
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
Re: kf5 alpha 1 : attica?
[resent with Frederik's correct email address]
On Thursday 06 February 2014 00:53:53 Frederik Gladhorn wrote:
Thanks a lot, I really appreciate it.
Some issues left:
- who do we write down as maintainer for attica?
- can I run astyle on the code to make it consistent with all other
frameworks? ('kdelibs' coding style, almost like the qt one)
- does it have a component on bugs.kde.org?
- does it exist on reviewboard.kde.org?
--
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
alpha1 release
I have packaged up KF5 alpha 1 and uploaded it for packagers. Can we consider KF5 frozen all of tomorrow (Friday 7) ? This way if there's any showstopper bugfix I can redo the tarballs without bringing in unrelated changes which might create other problems. Sorry for the productivity loss :-) -- 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
Re: Review Request 115398: rename dbus interface to prevent clashes with khtml from kdelibs4
On Thursday 30 January 2014 12:38:15 Jonathan Riddell wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115398/ --- Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark. Repository: khtml Description --- rename dbus interface to prevent clashes with khtml from kdelibs4 alas I have no idea what applications provide this interface src/khtml_iface.h 07fbf32 IIRC I wrote this dcop interface (became dbus later) to be able to script khtml (to automate lotto websites :). It's fine to not install the dbus xml indeed, since I'm not aware of any app using it. -- 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
Re: Review Request 115421: Clean up the CMakeLists.txt files
On Saturday 01 February 2014 13:27:49 Alex Merry wrote: - KStyle can once again be built standalone Is there any reason for this? I ask because I noticed that this was another place where the KF5 version had to be updated at release time, the only one that is not in frameworks/*/CMakeLists.txt. If it's needed that's fine, I adapted my scripts, but I'm just surprised. -- 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
Re: Building frameworks: docbook problems
On Monday 03 February 2014 22:08:20 Andriy Rysin wrote: I am trying to build frameworks using http://community.kde.org/Frameworks/Building on Fedora 20 and it failes in several modules due to some docbook problem, e.g. in kconfigwidgets: Scanning dependencies of target kstandardactiontest_automoc man-preparetips5.1.docbook:5: warning: failed to load external entity dtd/kdex.dtd ] ^ man-preparetips5.1.docbook:7: parser error : Entity 'language' not defined refentry lang=language; Did you set XDG_DATA_DIRS to contain yourkf5installprefix/share ? -- 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
Re: kf5 alpha 1 : modules, versions
On Thursday 06 February 2014 23:32:11 David Faure wrote: On Wednesday 05 February 2014 22:42:00 Michael Palimaka wrote: On 02/03/2014 08:17 PM, David Faure wrote: Any new module that should be added to this release, compared to TP1? I see that plasma-framework.yaml says tier 3, is that correct - wouldn't it mean it should be released with the others? Excellent question. Plasma people: should I release plasma-framework with the other frameworks? Two things make it stand out a little bit, we might want to fix that, if the answer is yes: * it's not under frameworks/ in the projects.kde.org hierarchy. Shall we move it? * it declares itself to be version 2.0.0. Can I change that to 4.96.0 like all other frameworks? Ping? It's now or never, for alpha1 :) (I'm not on plasma-devel, please cc k-f-d) -- 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
Re: kf5 alpha 1 : modules, versions
On Saturday 08 February 2014 10:55:25 Marco Martin wrote: On Saturday 08 February 2014 10:31:10 David Faure wrote: * it's not under frameworks/ in the projects.kde.org hierarchy. Shall we move it? * it declares itself to be version 2.0.0. Can I change that to 4.96.0 like all other frameworks? Ping? It's now or never, for alpha1 :) (I'm not on plasma-devel, please cc k-f-d) To me the main issue is that there will still be several changes in there (freeze is at the end of march) would be this ok? Yes, it's just an alpha, any kind of change can still happen after that. -- 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
Re: Review Request 115541: Build fix for Mac OS X
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115541/#review49240 --- Urgh, we were hoping this wouldn't be an issue. This commit would break #include attica/event.h, so it cannot go in. All namespaced frameworks do it this way already btw, see kparts for instance: -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KParts/KParts/ReadWritePart -- Up-to-date: /d/kde/inst/kde_frameworks/include/KF5/KParts/kparts/readwritepart.h Since there is no filename clash, what is the issue if these end up in the same folder on Mac OSX? - David Faure On Feb. 7, 2014, 7:37 p.m., Harald Fernengel wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115541/ --- (Updated Feb. 7, 2014, 7:37 p.m.) Review request for KDE Frameworks. Repository: attica Description --- Case-insensitive filesystems don't like the Attica vs. attica pathes, when installing, the headers would all be messed up. Instead, install everything to include/KF5/Attica to be in line with the other frameworks. Note - this might not be the best solution, but we need one in order to deploy on Mac OS X :) Diffs - src/CMakeLists.txt 676c8a8e78420371bba19414b3f090180a49758d Diff: https://git.reviewboard.kde.org/r/115541/diff/ Testing --- Only on Mac OS X Thanks, Harald Fernengel ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 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/#review49241 --- Ship it! Ship It! - David Faure On Feb. 6, 2014, 10:35 p.m., David Gil Oliva wrote: --- 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. 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: KIO::convertSize(.,.) vs. KFormat::formatByteSize(...)
to KCoreAddons BTW. I just think both are needed. The current KF5 policies seem to not yet mention the issue of platform/system integration. Has there been some discussion on this before already? Well, for things coming from Qt we have KStyle and the QPA theme plugin, and for things coming from KF5 that we don't feel belong in Qt we have interfaces implemented by frameworkintegrationplugin. -- 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
Re: kf5 alpha 1 : modules, versions
On Saturday 08 February 2014 11:36:26 Marco Martin wrote: On Saturday 08 February 2014 10:59:19 David Faure wrote: On Saturday 08 February 2014 10:55:25 Marco Martin wrote: On Saturday 08 February 2014 10:31:10 David Faure wrote: * it's not under frameworks/ in the projects.kde.org hierarchy. Shall we move it? * it declares itself to be version 2.0.0. Can I change that to 4.96.0 like all other frameworks? Ping? It's now or never, for alpha1 :) (I'm not on plasma-devel, please cc k-f-d) To me the main issue is that there will still be several changes in there (freeze is at the end of march) would be this ok? Yes, it's just an alpha, any kind of change can still happen after that. ok, soo, what we should have to do on our part? :) + First, answering the questions below: * plasma-framework depends on kactivities which is also not a proper framework. Shall we make it one and release it together with the others? It needs a bit of updating in the cmake stuff (it has too many of the usually toplevel stuff like ecm_setup_version etc. in the subdir src/lib/core). * OK if I run astyle-kdelibs in both, to harmonize coding style? (drawback: it makes forward-porting changes from 4.x harder) + Can you add both to http://community.kde.org/Frameworks/List? This includes figuring out who to write down as maintainer :) + plasma-framework/README.md should be completed with a description + according to http://community.kde.org/Frameworks/Policies, the autotests and tests in plasma-framework should move to the toplevel. + In kactivities, the actual autotests like ./tests/core should move to an autotests subdir. + kactivites needs a README.md, a kactivities.yaml and a .reviewboardrc Both frameworks need to be ported to ecm_generate_headers and ecm_generate_pri_file. Do you want to look at that too? Of course I can release 4.96.0 without any of the above apart from an answer to the first question :) -- 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
Re: kf5 alpha 1 : modules, versions
Also, kactivities is the only framework that still uses a branch named frameworks. Can we switch to master = kf5 and KDE/4.13 for the current master, assuming a 4.13 release of it is planned? [otherwise what do we do with master?] The only commit that is in master and not in 4.12 is this one btw: commit 2702febc7474537b19d816a941261755b04798f4 Author: Aaron Seigo ase...@kde.org cut off adding/removing activities at the source -- 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
Re: kf5 alpha 1 : modules, versions
On Saturday 08 February 2014 14:13:40 Ivan Čukić wrote: On Saturday, 8. February 2014. 12.58.53 David Faure wrote: Also, kactivities is the only framework that still uses a branch named frameworks. Can we switch to master = kf5 and KDE/4.13 for the current master, assuming a 4.13 release of it is planned? [otherwise what do we do with master?] I wanted that, but got a frowny face from Albert: Personally I'd suggest against it since seems that even if we dicussed for that happening to kde-workspace people did not get the memo and got angry, Well, that was before we had tools to abstract branch names, like kde-build- metadata/logical-module-structure. These days such changes are a lot more transparent, which puts the matter to rest (I was one of the unhappy people when things started to get inconsistent). Since then, kdelibs got splitted, so as I said, KF5 stuff is master in more and more places - and especially all of the frameworks themselves. -- 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
Re: Review Request 115424: Improve kparts dependencies
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115424/#review49264 --- Ship it! Looks good, but then you have 56 other frameworks where to add a if(BUILD_TESTING) around add_subdirectory(tests) and add_subdirectory(autotests) :-) - David Faure On Feb. 8, 2014, 5:32 p.m., Michael Palimaka wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115424/ --- (Updated Feb. 8, 2014, 5:32 p.m.) Review request for KDE Frameworks and David Faure. Repository: kparts Description --- - QtNetwork is not used - QtTest is only required for tests - Remove transitive dependencies Diffs - CMakeLists.txt 8d7a76250a3977120f014eb735a2ef66ca5d59ea autotests/CMakeLists.txt d62107e51d88b0ea46be113b57c318197f3ebaa9 tests/CMakeLists.txt d7bca988b59022bb0731a38a5d7e872217541810 Diff: https://git.reviewboard.kde.org/r/115424/diff/ Testing --- Inspection of source. Builds. Tests pass. Thanks, Michael Palimaka ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115424: Improve kparts dependencies
On Feb. 8, 2014, 5:34 p.m., David Faure wrote: Looks good, but then you have 56 other frameworks where to add a if(BUILD_TESTING) around add_subdirectory(tests) and add_subdirectory(autotests) :-) Michael Palimaka wrote: That's fine, do I need to do a review request for all of those that have just that change? No, if that's the only change, it doesn't need to be reviewed, go ahead. Review your own diff though :-) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115424/#review49264 --- On Feb. 8, 2014, 5:39 p.m., Michael Palimaka wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115424/ --- (Updated Feb. 8, 2014, 5:39 p.m.) Review request for KDE Frameworks and David Faure. Repository: kparts Description --- - QtNetwork is not used - QtTest is only required for tests - Remove transitive dependencies Diffs - CMakeLists.txt 8d7a76250a3977120f014eb735a2ef66ca5d59ea autotests/CMakeLists.txt d62107e51d88b0ea46be113b57c318197f3ebaa9 tests/CMakeLists.txt d7bca988b59022bb0731a38a5d7e872217541810 Diff: https://git.reviewboard.kde.org/r/115424/diff/ Testing --- Inspection of source. Builds. Tests pass. Thanks, Michael Palimaka ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115424: Improve kparts dependencies
On Feb. 8, 2014, 5:34 p.m., David Faure wrote:
Looks good, but then you have 56 other frameworks where to add a
if(BUILD_TESTING) around add_subdirectory(tests) and
add_subdirectory(autotests) :-)
Michael Palimaka wrote:
That's fine, do I need to do a review request for all of those that have
just that change?
David Faure wrote:
No, if that's the only change, it doesn't need to be reviewed, go ahead.
Review your own diff though :-)
Michael Palimaka wrote:
Actually, it looks like a lot of frameworks already do something like
this in autotests/CMakeLists.txt:
find_package(Qt5Test ${REQUIRED_QT_VERSION} CONFIG QUIET)
if(NOT Qt5Test_FOUND)
message(STATUS Qt5Test not found, autotests will not be built.)
return()
endif()
Should we punt this in favour of BUILD_TESTING check?
I would say so (it's more common to not want to build tests, to save time, than
to be missing Qt5Test in the first place).
So explicit control with -DBUILD_TESTING seems better.
But please check with Alex Richardson who committed such code, to be sure
everyone's ok with this.
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115424/#review49264
---
On Feb. 8, 2014, 5:39 p.m., Michael Palimaka wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115424/
---
(Updated Feb. 8, 2014, 5:39 p.m.)
Review request for KDE Frameworks and David Faure.
Repository: kparts
Description
---
- QtNetwork is not used
- QtTest is only required for tests
- Remove transitive dependencies
Diffs
-
CMakeLists.txt 8d7a76250a3977120f014eb735a2ef66ca5d59ea
autotests/CMakeLists.txt d62107e51d88b0ea46be113b57c318197f3ebaa9
tests/CMakeLists.txt d7bca988b59022bb0731a38a5d7e872217541810
Diff: https://git.reviewboard.kde.org/r/115424/diff/
Testing
---
Inspection of source. Builds. Tests pass.
Thanks,
Michael Palimaka
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115541: Build fix for Mac OS X
On Feb. 8, 2014, 10:07 a.m., David Faure wrote:
Urgh, we were hoping this wouldn't be an issue.
This commit would break #include attica/event.h, so it cannot go in.
All namespaced frameworks do it this way already btw, see kparts for
instance:
-- Up-to-date:
/d/kde/inst/kde_frameworks/include/KF5/KParts/KParts/ReadWritePart
-- Up-to-date:
/d/kde/inst/kde_frameworks/include/KF5/KParts/kparts/readwritepart.h
Since there is no filename clash, what is the issue if these end up in the
same folder on Mac OSX?
Harald Fernengel wrote:
Here's the layout after make install on OS X:
include/KF5/KParts/textextension.h contains:
#include /tmp/kf5-kparts-ty2Y/src/textextension.h
^^^ this is broken, points to the temporary build dir...? What should
this include?
Then, we have include/KF5/KParts/KParts/ which contains both lower case
and upper case headers.
include/KF5/KParts/KParts/textextension.h is the actual header
include/KF5/KParts/KParts/TextExtension contains:
#include kparts/textextension.h
Ah, I see. The local forwarding includes which are supposed to only be used
during compilation of kparts, get installed because they end up in KParts/
instead of kparts/ (and the cmakelists.txt file just installs the whole
directory).
I can think of two solutions...
1) put local forwarders into ./local/kparts instead of ./kparts, to ensure they
stay out of ./KParts
2) change cmakelists.txt to install a list of camelcase headers instead of just
the whole directory (which gives surprises with an unclean builddir,
installing old stuff still lying around)
The first one seems simpler.
In kparts/src:
- target_include_directories(KF5Parts PUBLIC
$BUILD_INTERFACE:${KParts_BINARY_DIR})
+ target_include_directories(KF5Parts PUBLIC
$BUILD_INTERFACE:${KParts_BINARY_DIR}/local)
And in CEM:
diff --git a/modules/ECMGenerateHeaders.cmake b/modules/ECMGenerateHeaders.cmake
index e98a22e..38839f2 100644
--- a/modules/ECMGenerateHeaders.cmake
+++ b/modules/ECMGenerateHeaders.cmake
@@ -50,7 +50,7 @@ function(ECM_GENERATE_HEADERS)
endif()
if(NOT EGH_OUTPUT_DIR)
-set(EGH_OUTPUT_DIR ${CMAKE_CURRENT_BINARY_DIR})
+set(EGH_OUTPUT_DIR ${CMAKE_CURRENT_BINARY_DIR}/local)
endif()
# Make sure EGH_RELATIVE is /-terminated when it's not empty
Can you try it?
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115541/#review49240
---
On Feb. 7, 2014, 7:37 p.m., Harald Fernengel wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115541/
---
(Updated Feb. 7, 2014, 7:37 p.m.)
Review request for KDE Frameworks.
Repository: attica
Description
---
Case-insensitive filesystems don't like the Attica vs. attica pathes, when
installing, the headers would all be messed up. Instead, install everything
to include/KF5/Attica to be in line with the other frameworks.
Note - this might not be the best solution, but we need one in order to
deploy on Mac OS X :)
Diffs
-
src/CMakeLists.txt 676c8a8e78420371bba19414b3f090180a49758d
Diff: https://git.reviewboard.kde.org/r/115541/diff/
Testing
---
Only on Mac OS X
Thanks,
Harald Fernengel
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115586: Improve attica tests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115586/#review49343 --- All looks good, but I'm not sure about removing the broken PrivateDataParser test (the class still exists, but is not exported and has a private parseXml method; it could be exported for the test and the test class marked as friend, for instance?). sandsmark, can you confirm or deny removal of the autotest? - David Faure On Feb. 8, 2014, 7:52 p.m., Michael Palimaka wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115586/ --- (Updated Feb. 8, 2014, 7:52 p.m.) Review request for KDE Frameworks. Repository: attica Description --- - Use BUILD_TESTING from ECM instead of custom ATTICA_ENABLE_TESTS - Remove enable_testing since this is done automatically by KDECMakeSettings - Mark tests - Remove old test that doesn't build, which tests API that no longer exists Diffs - CMakeLists.txt 445503efba2b49546137ce1548b3ddcddb2a0ebb autotests/CMakeLists.txt e4a455730fd1faddf547b6c91d4477d707f41551 autotests/privatedatatest.cpp e5c2d2675b1f33631f2b31a2ce0eb5db28f3406a tests/projecttest/CMakeLists.txt 0fce1bf8b2f9f93b9ee5b7961a12aeecdc8828a8 Diff: https://git.reviewboard.kde.org/r/115586/diff/ Testing --- Builds. Tests pass when enabled, and do not run when not. Thanks, Michael Palimaka ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115613: Drop platform name from default user agent string
On Monday 10 February 2014 09:15:23 Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115613/ --- (Updated Feb. 10, 2014, 9:15 a.m.) Review request for KDE Frameworks, Dawit Alemayehu and Bernhard Beschow. Changes --- Adding more people for review. IMHO Dawit has final say on what the UA string should look like. Reviewboard is weird. I added that comment, but the mail sent by reviewboard doesn't show that anywhere. It makes it look like Martin made that change. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE, in particular KDE Frameworks 5 ---BeginMessage--- --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115613/ --- (Updated Feb. 10, 2014, 9:15 a.m.) Review request for KDE Frameworks, Dawit Alemayehu and Bernhard Beschow. Changes --- Adding more people for review. IMHO Dawit has final say on what the UA string should look like. Repository: kio Description --- Drop platform name from default user agent string The platform name (e.g. X11) was currently broken on compile time. On Linux it returned unknown and on all other platforms the same name as already included in the OS name. We cannot really determine the platform name as this is a core application and the Qt's platform name is only available in a GUI application. Compile time is no solution as we cannot know whether the binary is executed on X11, Wayland, Android or whatever. Diffs - src/core/kprotocolmanager.cpp f81b6797887eebd868c36b98e867eb055b05a1e2 Diff: https://git.reviewboard.kde.org/r/115613/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 ---End Message--- ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Fwd: Re: Re: HAVE_X11 usage in KIO/core
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5
---BeginMessage---
Well that was done for compatibility with what Firefox/Chromium.
The latest stable version (version 27) sends the following user agent
string:
Mozilla/5.0 (X11; Linux x86_64; rv:27.0) Gecko/20100101 Firefox/27.0
And the latest Chromium (version 32) seems to send the following string by
default:
Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko)
Chrome/32.0.1700.107 Safari/537.36
So removing the platform name from the user-agent string might have
consequences for those that check for it?
Regards,
Dawit A.
On Sat, Feb 8, 2014 at 4:29 AM, David Faure fa...@kde.org wrote:
Any opinion?
-- Forwarded Message --
Subject: Re: HAVE_X11 usage in KIO/core
Date: Saturday 08 February 2014, 00:55:22
From: Albert Astals Cid aa...@kde.org
To: kde-frameworks-devel@kde.org
El Divendres, 7 de febrer de 2014, a les 15:25:42, Kevin Krammer va
escriure:
On Friday, 2014-02-07, 09:51:27, Martin Gräßlin wrote:
On Friday 07 February 2014 09:38:41 Kevin Krammer wrote:
On Friday, 2014-02-07, 08:53:54, Martin Gräßlin wrote:
I'm wondering what to do about it. The best would be to use
QGuiApplication::platformName, but it's a core app. Also finding
X11
in
CMakeLists to get the HAVE_X11 defined looks very wrong to me and
not
future safe (Wayland).
My guess is that platform() in this context means operating system,
not
windowing/display system.
See the comment I pasted, it's explicitly saying it's the windowing
system
and not the OS...
Ah, didn't see that. Does it actually make sense?
If yes than this obviously has be to be done at runtime, at least for
platforms with multiple UI systems:
#if defined(Q_OS_MAC)
return QL1S(Macintosh)
#elfi defined(Q_OS_WINDOWS)
return QL1S(Windows)
#else
const QVariant platformName = qApp ? qApp-property(platformName) :
QVariant();
if (platformName.isValid()) {
const QString name = platformName.toString();
if (!name.isEmpty())
return name;
}
#endif
return QL1S(Unknown);
Sincerely, just drop it.
We will change from sending
Mozilla/5.0 (compatible; Konqueror/4.0; Linux; X11; i686; en_US)
KHTML/4.0.1
(like Gecko)
to
Mozilla/5.0 (compatible; Konqueror/4.0; Linux; i686; en_US) KHTML/4.0.1
(like
Gecko)
will anyone ever care?
Cheers,
Albert
Cheers,
Kevin
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
-
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5
---End Message---
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
