Review Request 114197: EnvironmentWidget: Always display placeholder line

[Milian Wolff]

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


Looking really nice - thanks! Please clean it up a bit more and then push it. Then we can use it in the other places as well :) Nice work!


shell/settings/environmentgroupmodel.cpp
<http://git.reviewboard.kde.org/r/114197/#comment32236>

    please put the braces for conditionals on the same line except for overly long conditions



shell/settings/environmentgroupmodel.cpp
<http://git.reviewboard.kde.org/r/114197/#comment32237>

    same here with the braces



shell/settings/environmentwidget.cpp
<http://git.reviewboard.kde.org/r/114197/#comment32238>

    please put this into a new file in util/ and de-inline the implementation (also pimpl it)



shell/settings/environmentwidget.cpp
<http://git.reviewboard.kde.org/r/114197/#comment32239>

    return !value.toString().isEmpty()


- Milian Wolff


On Dec. 4, 2013, 8:34 a.m., Kevin Funk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114197/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2013, 8:34 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> EnvironmentWidget: Always display placeholder line
> 
> Implement a custom proxy model that just adds another line at the end of
> the list. This line servers as a placeholder item. When setting data on
> that placeholder item, the EnvironmentWidget instance is notified and
> takes care of adding the actual value to the EnvironmentGroupModel
> instance.
> 
> The new PlaceholderItemProxyModel should be useful for similar widgets
> as well. For example, the BreakpointModel uses a similar approach,
> but includes the placeholder item directly in the source QAIM. => Cleanup up
> BreakPointModel and use this proxy to simplify code.
> 
> TODO: Find a nice place to live for PlaceHolderItemProxyModel
> 
> EnvironmentGroupModel: More API fixes
> 
> 
> Diffs
> -----
> 
>   shell/settings/environmentgroupmodel.h 9ddc98f6ebf301c23100de5b20f803d023b2b56c 
>   shell/settings/environmentgroupmodel.cpp fbea12ffb40f24d843a193a3abb8c07608b1a497 
>   shell/settings/environmentwidget.h 9f89ff070c0ea649bbc8c5d73a16c1b027c102bd 
>   shell/settings/environmentwidget.cpp 68d825ccf98ce9a54023107b4a889c06bd56727a 
>   shell/settings/environmentwidget.ui f9683307c0a2d61d42b04ba720be2b4f30143277 
> 
> Diff: http://git.reviewboard.kde.org/r/114197/diff/
> 
> 
> Testing
> -------
> 
> Bump
> 
> 
> Thanks,
> 
> Kevin Funk
> 
>

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