Review Request 115635: Make kconfig_compiler signals actually useful

[Matthew John Dawson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115635/#review50006
-----------------------------------------------------------


Looks good!  I'd prefer to avoid the wrapper, but I can't see any better way.  And this gets the job done.  Once the issues below are fixed, this looks good to go in.

As David Faure mentioned, please squash the two separate review requests (this and 115634) into one commit before pushing!


src/core/kcoreconfigskeleton.h
<https://git.reviewboard.kde.org/r/115635/#comment35114>

    As adding those getters/setters latter is BC , I'd prefer to keep them out of the code until it becomes an issue.



src/core/kcoreconfigskeleton.h
<https://git.reviewboard.kde.org/r/115635/#comment35115>

    If this getter is kept, the QScopedPointer should be changed.  Otherwise this allows the pointer escape unnecessarily.



src/kconfig_compiler/kconfig_compiler.cpp
<https://git.reviewboard.kde.org/r/115635/#comment35113>

    Coding style: please leave the braces for this if statement.



src/kconfig_compiler/kconfig_compiler.cpp
<https://git.reviewboard.kde.org/r/115635/#comment35112>

    This change breaks other unit tests, and doesn't seem to increase the readability of the generated code.  This can probably just be left alone.



src/kconfig_compiler/kconfig_compiler.cpp
<https://git.reviewboard.kde.org/r/115635/#comment35123>

    This change breaks the kconfigcompiler_test, as the reference file for signals (autotests/kconfig_compiler/test_signal.h.ref) no longer matches.  Please update the reference file to match, as the changes are wanted :)



src/kconfig_compiler/kconfig_compiler.cpp
<https://git.reviewboard.kde.org/r/115635/#comment35116>

    This change breaks the kconfigcompiler_test, as the reference file for signals (autotests/kconfig_compiler/test_signal.cpp.ref) no longer matches.  Please update the reference file to match, as the changes are wanted :)


- Matthew John Dawson


On Feb. 10, 2014, 7:48 p.m., Alexander Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115635/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2014, 7:48 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kconfig
> 
> 
> Description
> -------
> 
> Make kconfig_compiler signals actually useful
> 
> Previously the classes generated by kconfig_compiler would only emit
> the defined signals when using the setters provided by that class.
> However, when using e.g. KConfigDialog which uses
> KConfigSkeletonItem::setProperty() to change the items no signal was
> generated.
> This patch fixes this by using a wrapper KConfigSkeletonItem
> subclass that calls a private itemChanged() method in the generated
> class which updates the set of changed properties. As soon as the item
> is saved (usrWriteConfig() in the generated class is called) the signal
> will be emitted
> 
> 
> Diffs
> -----
> 
>   src/core/kcoreconfigskeleton.h c1a158771a785151902cd0a36aa672623618b99e 
>   src/core/kcoreconfigskeleton.cpp d9b95b4b0f236f82b1d4831432d3e7637ef19365 
>   src/kconfig_compiler/kconfig_compiler.cpp 0c4254a296348e02e596e9b10b76ff446f26bb65 
> 
> Diff: https://git.reviewboard.kde.org/r/115635/diff/
> 
> 
> Testing
> -------
> 
> Unit test from https://git.reviewboard.kde.org/r/115634/ passes
> 
> 
> Thanks,
> 
> Alexander Richardson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140217/6fa1cf84/attachment.html>