Review Request: [1/3] Refactor EnvironmentSelectionWidget.

[Ivan Shapovalov]

> On Jan. 6, 2013, 11:10 a.m., Milian Wolff wrote:
> > I missed the "why" for this patch set - can you elaborate please why this is required?

Reasons were following:

1. Every user of this class had to write the same code:
    EnvironmentGroupList env( KGlobal::config() );
    environment.addItems( env.groups() );

2. Nobody seemed to use EnvironmentGroupList::defaultGroup(), relying on "default" group being default - so, effectively, "Set group as default" in configuration KCM did nothing.

3. If KConfig value associated with the widget gets empty somehow, widget displays an empty line - which is not user-friendly.

4. Summarizing, EnvironmentSelectionWidget is a wrapper class that does nothing (well, apart from saving item text instead of item index to the KConfig).

Are these reasons ok?


- Ivan


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


On Jan. 5, 2013, 9:22 p.m., Ivan Shapovalov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108212/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2013, 9:22 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> Feature-wise this gets rid of dependency on default KConfigXT value being "default" and allows explicitly specifying to use a default environment group whatever it will be.
> 
> Now an empty value shall be used by default, which has meaning of "use whatever is set by default at runtime".
> Also, a special "Use default" line is now always added to the combo-box which maps to the empty config value, so user won't see empty entries in the combo-box.
> 
> 
> Diffs
> -----
> 
>   plugins/execute/nativeappconfig.cpp 481d56c 
>   plugins/executescript/scriptappconfig.cpp b8f0729 
>   util/CMakeLists.txt 29de126 
>   util/environmentconfigurebutton.cpp 8303c70 
>   util/environmentselectionmodel.h PRE-CREATION 
>   util/environmentselectionmodel.cpp PRE-CREATION 
>   util/environmentselectionwidget.h 0a69afc 
>   util/environmentselectionwidget.cpp 63fdebc 
> 
> Diff: http://git.reviewboard.kde.org/r/108212/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Ivan Shapovalov
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130106/ccfa1ded/attachment.html>