Review Request: sure new name for style is unique

C. Boemann cbr at boemann.dk
Thu May 10 17:59:44 UTC 2012


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



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

    still have a trailing space



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

    it's these i don't understand why we need variables for. You can at any time reference the name this way



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

    it's these i don't understand why we need variables for. You can at any time reference the name this way



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

    Could you please rename this to: checkUniqueStyleName



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

    i don't see why we have to have these checks.
    
    Just check if it is unique



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

    and then we can eliminate the need for these bools too



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

    same comment as abouve naturally



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

    hmm you should only save if it is unique



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

    i think it's better to copy the code than call applyClicked



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

    should be a warning not information



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

    you have too much space betwee "those" and "changes"



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

    "Warning" and this should only be shown in case the name is not unique., and then you should return without closing the dialog
    
    and saving should only be done if unique



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

    if you do the other things i say then you can remove this


- C. Boemann


On May 10, 2012, 12:53 p.m., mojtaba shahi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104882/
> -----------------------------------------------------------
> 
> (Updated May 10, 2012, 12:53 p.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/20120510/98002bf6/attachment-0001.html>


More information about the calligra-devel mailing list