Review Request: sure new name for style is unique

[C. Boemann]

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


We are getting there :)

please makse sure you fix all my comments. I've repeated some of them 3-4 times now


plugins/textshape/dialogs/CharacterGeneral.h
<http://git.reviewboard.kde.org/r/104882/#comment10883>

     make the method const



plugins/textshape/dialogs/ParagraphGeneral.h
<http://git.reviewboard.kde.org/r/104882/#comment10884>

    make the method const



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

    still a trailing space here, como on it's the 3rd time i mention it



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

    don't forget to set m_selectedCharStyle to 0



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

    don't forget to set m_selectedParagStyle to 0



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

    please add space after ,



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

    I'm not saying you shouldn't but what made you think trimming the name is needed?



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

    please add space after ,



plugins/textshape/dialogs/StyleManagerDialog.cpp
<http://git.reviewboard.kde.org/r/104882/#comment10892>

    if (m_styleManagerWidget->checkUniqueStyleName()) {
      m_styleManagerWidget->save();
    }



plugins/textshape/dialogs/StyleManagerDialog.cpp
<http://git.reviewboard.kde.org/r/104882/#comment10907>

    you shouldn't save in this case



plugins/textshape/dialogs/StyleManagerDialog.cpp
<http://git.reviewboard.kde.org/r/104882/#comment10894>

    don't check unique here



plugins/textshape/dialogs/StyleManagerDialog.cpp
<http://git.reviewboard.kde.org/r/104882/#comment10895>

    still a space too much.



plugins/textshape/dialogs/StyleManagerDialog.cpp
<http://git.reviewboard.kde.org/r/104882/#comment10896>

    but here you need to check for unique



plugins/textshape/dialogs/StyleManagerDialog.cpp
<http://git.reviewboard.kde.org/r/104882/#comment10897>

    remove this



plugins/textshape/dialogs/StylesModel.h
<http://git.reviewboard.kde.org/r/104882/#comment10898>

    you don't need the original name
    and the method can be made const



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

    please add space after ,



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

    no need for original name



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

    just check against the style being the selected style, then you don't need the originalName



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

    uhm why??



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

    why the count==1 check


- C. Boemann


On May 11, 2012, 8:48 a.m., mojtaba shahi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104882/
> -----------------------------------------------------------
> 
> (Updated May 11, 2012, 8:48 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> when you add new style or when you change name of style, patch checks that the new name for
> 
> style is unique. patch doesnt let you to go another style or close dialog or apply and ok until the name
> 
> that you have choosen for your style be unique.
> 
> 
> Diffs
> -----
> 
>   plugins/textshape/dialogs/CharacterGeneral.h bb72b88 
>   plugins/textshape/dialogs/CharacterGeneral.cpp 4923087 
>   plugins/textshape/dialogs/ParagraphGeneral.h 5ef5c86 
>   plugins/textshape/dialogs/ParagraphGeneral.cpp 8ef5b7f 
>   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/104882/diff/
> 
> 
> Testing
> -------
> 
> it is ok and works right :)
> 
> 
> Thanks,
> 
> mojtaba shahi
> 
>

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