Review Request: Configure colors for directory coverage percentages

[Andreas Pakulat]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/466/#review829
-----------------------------------------------------------



/trunk/KDE/kdevelop/tools/coverage/kcm_kdev_lcovsettings.desktop
<http://reviewboard.kde.org/r/466/#comment525>

    This shouldn't be changed, the plugin is a kdevelop plugin so it shouldn't show up in other kdevplatform based apps.


Other than that the patch seems fine, so feel free to commit once the above mentioned change has been reverted. If you don't have an svn account let me know and I'll commit the patch.

- Andreas


On 2009-04-04 18:34:44, Daniel Calviño Sánchez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/466/
> -----------------------------------------------------------
> 
> (Updated 2009-04-04 18:34:44)
> 
> 
> Review request for KDevelop.
> 
> 
> Summary
> -------
> 
> I have made a patch for feature request #178470 "Configure colors for directory coverage percentages" (https://bugs.kde.org/show_bug.cgi?id=178470).
> 
> The patch can be seen as two different "modules": the changes to the coverage plugin, and the changes to the coverage configuration plugin.
> 
> The changes to the coverage plugin are easy: adding a ColorRange class that returns a color for a position in the range, and two helper classes (DiscreteColorRange and GradientColorRange) that model the behavior for discrete and gradient ranges. There are unit test for the three classes.
> 
> ColorRange objects can be read from and written to a KConfigGroup using load and save methods. The ReportModel was modified to use a ColorRange object, that is read from configuration if available, or set to a default value if not.
> 
> The changes to the coverage configuration plugin are my main concern, is I have never written before a configuration plugin, and this one contains both KConfigXT and manually managed data.
> 
> I have added a LCovSettingsBase class, which is now used as base class for LCovSettings instead of KConfigSkeleton. LCovSettingsBase class contains the ColorRange that is modified by the user. Through usrReadConfig(), usrSetDefaults() and usrWriteConfig() methods, the ColorRange is loaded, reverted to default value or saved in the configuration. Those methods are (indirectly, through general KCoreConfigSkeleton methods) called in LCovPrefs when needed.
> 
> There is also a widget for the configuration, LCovPrefsWidget, that manages the changes made to the ColorRange by the user. When some change is made, LCovPrefs is notified, telling also whether the ColorRange is, after the change, equal to the saved one or not (so "Apply" button can be disabled or enabled). Also, LCovSettingsBase notifies LCovPrefsWidget when the ColorRange is changed due to being loaded or set to the default value, so LCovPrefsWidget can reload the ColorRange values shown.
> 
> There are also two other helper classes, StopPointWidget and ColorRangeBar. The first manages one specific StopPoint, and the second shows graphically a ColorRange.
> 
> I tried to add unit tests for the coverage configuration plugin, but I didn't found the way to do it. The linker complained that it couldn't found the implementation for the methods of the classes in the plugin, even when linked against kcm_kdev_lcovsettings.
> 
> Finally, I think that the configuration plugin should be moved to its own "config" subdirectory to better differentiate it from the general coverage plugin.
> 
> 
> This addresses bug 178470.
>     https://bugs.kde.org/show_bug.cgi?id=178470
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdevelop/tools/coverage/CMakeLists.txt 943869 
>   /trunk/KDE/kdevelop/tools/coverage/colorrange.h PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/colorrange.cpp PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/colorrangebar.h PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/colorrangebar.cpp PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/discretecolorrange.h PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/discretecolorrange.cpp PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/gradientcolorrange.h PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/gradientcolorrange.cpp PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/kcm_kdev_lcovsettings.desktop 943869 
>   /trunk/KDE/kdevelop/tools/coverage/lcovconfig.kcfgc 943869 
>   /trunk/KDE/kdevelop/tools/coverage/lcovprefs.h 943869 
>   /trunk/KDE/kdevelop/tools/coverage/lcovprefs.cpp 945419 
>   /trunk/KDE/kdevelop/tools/coverage/lcovprefs.ui 943869 
>   /trunk/KDE/kdevelop/tools/coverage/lcovprefswidget.h PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/lcovprefswidget.cpp PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/lcovprefswidget.ui 943869 
>   /trunk/KDE/kdevelop/tools/coverage/lcovsettingsbase.h PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/lcovsettingsbase.cpp PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/reportmodel.h 943869 
>   /trunk/KDE/kdevelop/tools/coverage/reportmodel.cpp 943869 
>   /trunk/KDE/kdevelop/tools/coverage/stoppointwidget.h PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/stoppointwidget.cpp PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/stoppointwidget.ui PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/tests/CMakeLists.txt 943869 
>   /trunk/KDE/kdevelop/tools/coverage/tests/colorrangetest.h PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/tests/colorrangetest.cpp PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/tests/discretecolorrangetest.h PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/tests/discretecolorrangetest.cpp PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/tests/gradientcolorrangetest.h PRE-CREATION 
>   /trunk/KDE/kdevelop/tools/coverage/tests/gradientcolorrangetest.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/466/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Daniel
> 
>