> On Feb. 3, 2014, 9:36 a.m., Martin Gräßlin wrote: > > src/netwm.h, lines 1426-1430 > > <https://git.reviewboard.kde.org/r/115442/diff/1/?file=241482#file241482line1426> > > > > This method should also get adjusted but as it's virtual it will be a > > true API break. > > > > What's the best way? Deprecate this method and add a new one instead? > > Aurélien Gâteau wrote: > The problem with deprecating virtual methods is you can do it that way: > > #1 > > virtual void changeState(NET::States states, NET::WindowTypeMask > mask); > KDE_DEPRECATED virtual void changeState(unsigned long states, > unsigned long mask); > > But then your user has two possible methods to override, which one should > he do? > > Or you can do it that way: > > #2 > > virtual void changeState(NET::States states, NET::WindowTypeMask > mask); > KDE_DEPRECATED void changeState(unsigned long states, unsigned long > mask) > { > changeState(NET::States(states), NET::WindowTypeMask(mask)); > } > > (+) Only one method can be overridden > (-) Existing code which used to override the deprecated version no longer > works. > > Or you can do it that way: > > #3 > > void changeState(NET::States states, NET::WindowTypeMask mask) > { > changeState(long(states), long(mask)); > } > KDE_DEPRECATED virtual void changeState(unsigned long states, > unsigned long mask); > > (+) Only one method can be overridden > (+) Existing code which used to override the deprecated version still > works. > (-) The method to override is the deprecated one, so it's not really > deprecated :/ > > With that in mind, I would go for the future-clean version: change the > method signature and break the ABI. Since this method is not meant to be > called by outside code, it's not going to make any difference with #2.
OK, I'm going for breaking the ABI. It's not that big deal as I think only KWin implements it and we already broke the ABI of NETWinInfo quite a bit and KWin has override flags added to all methods. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115442/#review48791 ----------------------------------------------------------- On Feb. 3, 2014, 9:34 a.m., Martin Gräßlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115442/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2014, 9:34 a.m.) > > > Review request for KDE Frameworks. > > > Repository: kwindowsystem > > > Description > ------- > > Introduce QFlags for NET::Action > > API of NETWinInfo adjusted. > > Introduce QFlags for NET::State > > API in NET* classes and KWindowInfo and KWindowSystem adjusted. > > Introduce QFlags for NET::WindowTypeMasks > > API in NET* classes and KWindowInfo is adjusted to make use > of the new Type instead of using long values. > > Introduce QFlags for NET::Property and NET::Property2 > > API of KWindowInfo is adjusted to take NET::Properties and > NET::Properties2 as arguments instead of usigned long int. > > The ctors of the NET* classes are adjusted which just take > NET::Properties. The other ctors take the properties array > which needs further adjustements first. > > In addition a WMAllProperties and WM2AllProperties enum value > is declared to simplify matching all properties. > > > Diffs > ----- > > src/kwindowinfo_p_x11.h a74ea32aa51407d0a168dd807c29787ce856f2af > src/kwindowinfo_p.h 1ad6cf89e6e9dd8b070d0167410a1a758594f734 > src/kwindowinfo.cpp 790c4458075ff0ddc22102111e2d96f8c636f0ee > src/kwindowinfo.h 36cbbcfede41e00f9c0aaa5ee38bc3708065ffd4 > autotests/netwininfotestwm.cpp 585994fa0cafe63ea88beabe32bf4c73b095b3f1 > autotests/netwininfotestclient.cpp 77b3ab163dcb2bf8a9d0aaf8dd0c7410865ceeca > autotests/kwindowinfox11test.cpp 18cfe39288c280ee9a6afb7b8565215365b114d1 > src/netwm_p.h 41792b330f7405034f4d51fb31a4de5dd674b6d0 > src/netwm_def.h 8b1ccb8bd731aefb9559c8f2b450337b0312ed4d > src/netwm.cpp 84eb137492e0afaaac80e8d26561fd8f8aff9c27 > src/netwm.h 393a29de3153a8b291b9fb249bd3eaeb1ba4e7d5 > src/kwindowsystem_x11.cpp 01c78c1debf95d5a176e2153139da19abf383c41 > src/kwindowsystem.h 3de0fea179dd468a78a265808fc64704027ec30d > src/kwindowinfo_x11.cpp eda31f2d94e23b1dd411a40fd65e96f65c389ac0 > > Diff: https://git.reviewboard.kde.org/r/115442/diff/ > > > Testing > ------- > > Unit tests still succeed. > > Test compiled KWin, it needs adjustments but only where int values are used > (e.g. passing 0). > > > Thanks, > > Martin Gräßlin > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
