Review Request: Configure colors for directory coverage percentages

Daniel Calviño Sánchez danxuliu at gmail.com
Sun Apr 5 01:34:45 UTC 2009


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

(Updated 2009-04-04 18:34:44.798750)


Review request for KDevelop.


Changes
-------

Added dummy StopPointWidget to be used when there is only one stop point in discrete mode. In that case, a StopPointWidget showing only the add button is inserted before the StopPointWidget for percentage 100 so the user can insert another stop point before it.

Also, all the StopPointWidgets are reloaded when a stop point is added or removed. The code is much simpler and there is no perceptible difference from adding and removing specific widgets.

The parent app and componentes in kcm_kdev_lcovsettings.desktop where changed from "kdevelop" to "kdevplatform", so the coverage configuration module only appears in general KDevelop configuration instead of in the general configuration and in each project configuration.

Finally, some two bugs were fixed in LCovSettingsBase. First, when setting and saving the default ColorRange, the saved ColorRange wasn't set to the default ColorRange, it was kept in its previous value. And second, when saving the ColorRange a new group is created, as the previous group could contain garbage stop points (when the previous ColorRange had more stop points than the new one).


Summary (updated)
-------

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 (updated)
-----

  /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





More information about the KDevelop-devel mailing list