Review Request: Avoid processing parameters if the number of arguments is not met in Text Labels

[Albert Astals Cid]

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

Ship it!



modes/label.cc
<http://git.reviewboard.kde.org/r/102725/#comment6211>

    &= is a bitwise operator, it is working by "change" because of how bools are implemented internally by the compiler, i'd prefer if you changed it to 
    finish = finish && ( *i != 0 );
    
    Other than that the code looks fine, so if you have verified it works, ship it!


- Albert Astals Cid


On Sept. 29, 2011, 4 a.m., David Narváez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102725/
> -----------------------------------------------------------
> 
> (Updated Sept. 29, 2011, 4 a.m.)
> 
> 
> Review request for KDE Edu.
> 
> 
> Description
> -------
> 
> Changed the finished variable to finish, as I think is the way the code was intended to be. Also fixes the second validation that took place (calling canFinish in the finishPressed method) since first validation happens in the validatePage method of the Arguments page.
> 
> 
> This addresses bug 282913.
>     http://bugs.kde.org/show_bug.cgi?id=282913
> 
> 
> Diffs
> -----
> 
>   modes/label.cc d0c855d 
>   modes/textlabelwizard.cc 0721227 
> 
> Diff: http://git.reviewboard.kde.org/r/102725/diff/diff
> 
> 
> Testing
> -------
> 
> 1) Created a new label
> 2) Introduced a parameter %1
> 3) Clicked Next
> 4) Clicked Finish without specifying a parameter
> 5) Clicked Ok in the error popup
> 
> No crash, no second validation.
> 
> 
> Thanks,
> 
> David Narváez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20111003/aa903660/attachment.html>