----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115097/ -----------------------------------------------------------
Review request for KDE Frameworks and David Faure. Repository: kparts Description ------- As discussed in http://lists.kde.org/?l=kde-frameworks-devel&m=138994749530457&w=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 PRE-CREATION src/listingnotificationextension.h PRE-CREATION src/listingnotificationextension.cpp PRE-CREATION src/liveconnectextension.h PRE-CREATION src/liveconnectextension.cpp PRE-CREATION src/mainwindow.h 11d0067 src/mainwindow.cpp 5c3917e src/openurlarguments.h PRE-CREATION src/openurlarguments.cpp PRE-CREATION src/openurlevent.h PRE-CREATION src/openurlevent.cpp PRE-CREATION src/part.h 05194d3 src/part.cpp 5a63777 src/part_p.h PRE-CREATION src/partactivateevent.h PRE-CREATION src/partactivateevent.cpp PRE-CREATION src/partbase.h PRE-CREATION src/partbase.cpp PRE-CREATION src/partbase_p.h PRE-CREATION src/partmanager.h 624a97c src/partmanager.cpp 7c1f3b0 src/partselectevent.h PRE-CREATION src/partselectevent.cpp PRE-CREATION src/plugin.h b7d130f src/plugin.cpp 344f75b src/readonlypart.h PRE-CREATION src/readonlypart.cpp PRE-CREATION src/readonlypart_p.h PRE-CREATION src/readwritepart.h PRE-CREATION src/readwritepart.cpp PRE-CREATION src/readwritepart_p.h PRE-CREATION src/scriptableextension.h 9cec71f src/scriptableextension.cpp d31a558 src/scriptableextension_p.h 82808f7 src/selectorinterface.h PRE-CREATION src/selectorinterface.cpp PRE-CREATION src/statusbarextension.h c46fd55 src/statusbarextension.cpp 2d8de8a src/textextension.h f8c77e4 src/textextension.cpp 3dc5a99 src/windowargs.h PRE-CREATION autotests/parttest.cpp 7505948 src/CMakeLists.txt d987f46 src/browserarguments.h PRE-CREATION src/browserarguments.cpp PRE-CREATION src/browserextension.h 998f7ac src/browserextension.cpp a367de9 src/browserhostextension.h PRE-CREATION src/browserhostextension.cpp PRE-CREATION src/browserinterface.h cf10499 Diff: https://git.reviewboard.kde.org/r/115097/diff/ Testing ------- kdesrc-build/kf5-qt5-build-include builds fine for me with all the patches. Thanks, Friedrich W. H. Kossebau
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
