Review Request: renaming style and add new style

[C. Boemann]

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


As I asked you submit midway through you work i know it's not done, so i'm not going to do a deep review.

However I've added some comments on what i think you should change


plugins/textshape/dialogs/StyleManager.cpp
<http://git.reviewboard.kde.org/r/104962/#comment11128>

    you new lines are correct, just keep the setName and selectName) from above



plugins/textshape/dialogs/StylesModel.cpp
<http://git.reviewboard.kde.org/r/104962/#comment11130>

    please follow this by
    
    if (!paragStyle && m_newStyleList.contains(styleId)) {
       paragStyle = m_newStyleList[styleId];
    }



plugins/textshape/dialogs/StylesModel.cpp
<http://git.reviewboard.kde.org/r/104962/#comment11129>

    not needed if you do the above



plugins/textshape/dialogs/StylesModel.cpp
<http://git.reviewboard.kde.org/r/104962/#comment11124>

    this change looks wrong. It could cause a crash. better to write in the line above
    
    if (!paragStyle && m_newStyleList.contains(styleId)) {
       paragStyle = m_newStyleList[styleId];
    }



plugins/textshape/dialogs/StylesModel.cpp
<http://git.reviewboard.kde.org/r/104962/#comment11125>

    and again here you need to add something similar



plugins/textshape/dialogs/StylesModel.cpp
<http://git.reviewboard.kde.org/r/104962/#comment11127>

    giving it a name is not the responsibility of the stylemodel



plugins/textshape/dialogs/StylesModel.cpp
<http://git.reviewboard.kde.org/r/104962/#comment11126>

    obviously you will need to assign different numbers, so we can insert more than one new style, before applying


- C. Boemann


On May 23, 2012, 12:12 p.m., mojtaba shahi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104962/
> -----------------------------------------------------------
> 
> (Updated May 23, 2012, 12:12 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> a new review for adding new style and renamimg styles
> 
> 
> Diffs
> -----
> 
>   plugins/textshape/dialogs/CharacterGeneral.h eed6a02 
>   plugins/textshape/dialogs/CharacterGeneral.cpp c951b27 
>   plugins/textshape/dialogs/ParagraphGeneral.h 3cf9824 
>   plugins/textshape/dialogs/ParagraphGeneral.cpp 2fdb272 
>   plugins/textshape/dialogs/StyleManager.h 44dff97 
>   plugins/textshape/dialogs/StyleManager.cpp c318dd7 
>   plugins/textshape/dialogs/StyleManagerDialog.h 56e36b4 
>   plugins/textshape/dialogs/StyleManagerDialog.cpp d423ae0 
>   plugins/textshape/dialogs/StylesModel.h 53c0225 
>   plugins/textshape/dialogs/StylesModel.cpp 3b03f1b 
> 
> Diff: http://git.reviewboard.kde.org/r/104962/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> mojtaba shahi
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120523/f6dad9bb/attachment.htm>