Review Request: sure new name for style is unique
C. Boemann
cbr at boemann.dk
Thu May 10 18:59:44 BST 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.htm>
More information about the calligra-devel
mailing list