Review Request: Update block formats on paragraph style change

C. Boemann cbr at boemann.dk
Fri Feb 24 13:07:11 GMT 2012


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

Ship it!


Looks very nice. Kudos for grasping this complex matter.

Just one small issue that can easily be fixed. Let's finalize on irc and then you can commit


libs/kotext/styles/ChangeFollower.cpp
<http://git.reviewboard.kde.org/r/104032/#comment8807>

    Clearing twice like you do is not good enough as applyStyle is (in theory) intelligent and needs the style below to work correctly in all cases.
    The direct formatting on top however we assume is more simple



libs/kotext/styles/ChangeFollower.cpp
<http://git.reviewboard.kde.org/r/104032/#comment8808>

    same here


- C. Boemann


On Feb. 20, 2012, 4:27 p.m., Elvis Stansvik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104032/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2012, 4:27 p.m.)
> 
> 
> Review request for Calligra and C. Boemann.
> 
> 
> Description
> -------
> 
> State of things before the patch:
> 
> Changing properties of a paragraph style does not update
> paragraphs with the corresponding style. The problem was
> that in libs/kotext/styles/ChangeFollower.cpp, the block
> format was simply not set.
> 
> This patch:
> 
> - Adds frameBlockFormat()/setFrameBlockFormat() to
>   KoTextDocument (analog to the existing
>   frameCharFormat()/setFrameCharFormat()). The setter is
>   called from KoTextLoader.
> 
> - Changes ChangeFollower::collectNeededInfo(...) to also
>   collect
> 
>     blockParentFormat
>       The block format of the frame.
>     blockDirectFormat
>       The block format that was due to direct
>       paragraph formatting.
>     blockParentCharFormat
>       The char format of the frame.
>     blockDirectCharFormat
>       The char format that was due to direct
>       paragraph formatting.
> 
>   into its Memento instance.
> 
> - Changes ChangeFollower::processUpdates to apply the
>   aggregation of parent (frame) format + paragraph style
>   + direct formatting to the block (likewise for char
>   formatting properties).
> 
> State of things after the patch:
> 
> Changing properties of a paragraph style in the Style
> Manager will now update paragraphs in the document. If
> local paragraph formatting has been added, it is left
> alone.
> 
> Things that were broken before and still are:
> 
> o The character properties in the Paragraph Format
>   dialog (direct formatting) have no effect.
> 
> o The Line Spacing combo in the Paragraph Format
>   dialog (direct formatting) has no effect (Use
>   Font Metrics checkbox and Minimum spinbox works
>   though).
> 
> I believe these two are bugs elsewhere. I'll take a
> look at it.
> 
> 
> Diffs
> -----
> 
>   libs/kotext/KoTextDocument.h 56f55e6 
>   libs/kotext/KoTextDocument.cpp 5608860 
>   libs/kotext/opendocument/KoTextLoader.cpp 3ea7109 
>   libs/kotext/styles/ChangeFollower.h 8551801 
>   libs/kotext/styles/ChangeFollower.cpp 38aaf8d 
> 
> Diff: http://git.reviewboard.kde.org/r/104032/diff/
> 
> 
> Testing
> -------
> 
> - Typed in two paragraphs of text with the style Standard.
> - Changed things such as Left Indent, Alignment et.c. in
>   the Standard style and made sure the paragraphs updated.
> - Opened the Paragraph Format dialog for direct formatting
>   and overrided some properties.
> - Opened the Standard style again and made sure changing
>   the style properties wouldn't override the direct
>   formatting.
> 
> 
> Thanks,
> 
> Elvis Stansvik
> 
>

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


More information about the calligra-devel mailing list