[prev in list] [next in list] [prev in thread] [next in thread]
List: calligra-devel
Subject: Re: Review Request: Update block formats on paragraph style change
From: "C. Boemann" <cbr () boemann ! dk>
Date: 2012-02-24 13:07:11
Message-ID: 20120224130711.21200.16829 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
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 th=
en 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 theo=
ry) 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
> =
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/104032/">http://git.reviewboard.kde.org/r/104032/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre> <br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/104032/diff/1/?file=50591#file50591line81" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/kotext/styles/ChangeFollower.cpp</a> <span style="font-weight: \
normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void \
ChangeFollower::collectNeededInfo(const QSet<int> \
&changedStyles)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">80</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="n">style</span><span class="o">-></span><span \
class="n">applyStyle</span><span class="p">(</span><span \
class="n">blockFormat</span><span class="p">);</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre> </div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/104032/diff/1/?file=50591#file50591line88" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/kotext/styles/ChangeFollower.cpp</a> <span style="font-weight: \
normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void \
ChangeFollower::collectNeededInfo(const QSet<int> \
&changedStyles)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">87</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="n">style</span><span class="o">-></span><span \
class="n">KoCharacterStyle</span><span class="o">::</span><span \
class="n">applyStyle</span><span class="p">(</span><span \
class="n">charFormat</span><span class="p">);</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">same \
here</pre> </div>
<br />
<p>- C.</p>
<br />
<p>On February 20th, 2012, 4:27 p.m., Elvis Stansvik wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Calligra and C. Boemann.</div>
<div>By Elvis Stansvik.</div>
<p style="color: grey;"><i>Updated Feb. 20, 2012, 4:27 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">- 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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>libs/kotext/KoTextDocument.h <span style="color: grey">(56f55e6)</span></li>
<li>libs/kotext/KoTextDocument.cpp <span style="color: grey">(5608860)</span></li>
<li>libs/kotext/opendocument/KoTextLoader.cpp <span style="color: \
grey">(3ea7109)</span></li>
<li>libs/kotext/styles/ChangeFollower.h <span style="color: \
grey">(8551801)</span></li>
<li>libs/kotext/styles/ChangeFollower.cpp <span style="color: \
grey">(38aaf8d)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104032/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic