[prev in list] [next in list] [prev in thread] [next in thread]
List: calligra-devel
Subject: Re: Review Request 109651: Refactor style manager
From: Pierre Stirnweiss <pstirnweiss () googlemail ! com>
Date: 2013-03-26 9:20:17
Message-ID: CAOjaSmhywczPOTfAsUbvuuDL_x_BGth44_qRWmX9edFPEMm=7w () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
Sorry not to get back to you, I had to reinstall my box and am still not
completely operational.
As Thorsten said, we decided (on the phone) that his patch was a welcomed
fix for a crash. What i am working on is way more intrusive.
All I have left to do on my box is to set back my development environment
and restore my repo from the backup. I'll try to do that during the week.
See you soon,
Pierre
On Mon, Mar 25, 2013 at 11:22 AM, C. Boemann <cbr@boemann.dk> wrote:
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109651/
>
> Ship it!
>
> I have only a small issue. After that you are free to push
>
>
> libs/textlayout/KoStyleThumbnailer.cpp<http://git.reviewboard.kde.org/r/109651/diff/3/?file=121435#file121435line160> \
> (Diff revision 3)
>
> QImage KoStyleThumbnailer::thumbnail(KoCharacterStyle *characterStyle, \
> KoParagraphStyle *paragraphStyle, QSize size, bool recreateThumbnail, \
> KoStyleThumbnailerFlags flags)
> 157
>
> int paragraphStyleId = (paragraphStyle)?paragraphStyle->styleId():0;
>
> 160
>
> unsigned long paragraphStyleId = (paragraphStyle) ? reinterpret_cast<unsigned \
> long>(paragraphStyle) : 0L;
> paragraphStyleId seems a misnomer now
>
>
> - C.
>
> On March 25th, 2013, 5:28 a.m. UTC, Thorsten Zachmann wrote:
> Review request for Calligra and Pierre Stirnweiss.
> By Thorsten Zachmann.
>
> *Updated March 25, 2013, 5:28 a.m.*
> Description
>
> This patch tries to fix the problems/crashes which happen in the style manager.
>
> To get the simple crash open the default document and click on the edit button of \
> the None (character style)
> The patch uses a different model for the style manager and temporary styles that \
> get modified to only apply the changes once the user applies them. The model \
> changes the internal pointer to the temporary files once they are selected and when \
> applied resets to the original ones. However the styles preview in the styles \
> manager list is updated immediately. The patch simplifies the handling quite a bit.
>
> I did not know that Pierre was also working on that so I publish it now so we have \
> something we can discuss.
> If you like this I would like to backport it to 2.6 as it fixes quite some grave \
> bugs compared to what we have now.
> Testing
>
> Tested modification without saving.
> Tested applying.
> Tested Ok.
> Tested Canceling
>
> Diffs
>
> - libs/kotext/styles/KoCharacterStyle.h (2fdde12)
> - libs/textlayout/KoStyleThumbnailer.cpp (436b243)
> - plugins/textshape/CMakeLists.txt (084628c)
> - plugins/textshape/dialogs/CharacterGeneral.h (e6d99db)
> - plugins/textshape/dialogs/CharacterGeneral.cpp (d3eead6)
> - plugins/textshape/dialogs/ParagraphGeneral.h (1a78376)
> - plugins/textshape/dialogs/ParagraphGeneral.cpp (ad30369)
> - plugins/textshape/dialogs/StyleManager.h (dec3b6b)
> - plugins/textshape/dialogs/StyleManager.cpp (9c82c11)
> - plugins/textshape/dialogs/StylesManagerModel.h (PRE-CREATION)
> - plugins/textshape/dialogs/StylesManagerModel.cpp (PRE-CREATION)
> - plugins/textshape/dialogs/StylesSortFilterProxyModel.h (PRE-CREATION)
> - plugins/textshape/dialogs/StylesSortFilterProxyModel.cpp
> (PRE-CREATION)
>
> View Diff <http://git.reviewboard.kde.org/r/109651/diff/>
>
[Attachment #5 (text/html)]
Sorry not to get back to you, I had to reinstall my box and am still not completely \
operational.<br>As Thorsten said, we decided (on the phone) that his patch was a \
welcomed fix for a crash. What i am working on is way more intrusive.<br>
<br>All I have left to do on my box is to set back my development environment and \
restore my repo from the backup. I'll try to do that during the week.<br><br>See \
you soon,<br><br>Pierre<br><br><br><div class="gmail_quote">
On Mon, Mar 25, 2013 at 11:22 AM, C. Boemann <span dir="ltr"><<a \
href="mailto:cbr@boemann.dk" target="_blank">cbr@boemann.dk</a>></span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex">
<div>
<div style="font-family:Verdana,Arial,Helvetica,Sans-Serif"><div class="im">
<table style="border:1px #c9c399 solid" bgcolor="#f9f3c9" cellpadding="8" \
width="100%"> <tbody><tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/109651/" \
target="_blank">http://git.reviewboard.kde.org/r/109651/</a> </td>
</tr>
</tbody></table>
<br>
</div><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">I \
have only a small issue. After that you are free to push</pre> <br>
<div>
<table bgcolor="white" border="0" width="100%">
<thead>
<tr>
<th colspan="4" style="border-bottom:1px solid #c0c0c0;font-size:9pt;padding:4px \
8px;text-align:left" bgcolor="#F0F0F0"> <a \
href="http://git.reviewboard.kde.org/r/109651/diff/3/?file=121435#file121435line160" \
style="text-decoration:underline;font-weight:bold" \
target="_blank">libs/textlayout/KoStyleThumbnailer.cpp</a> <span \
style="font-weight:normal">
(Diff revision 3)
</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">QImage \
KoStyleThumbnailer::thumbnail(KoCharacterStyle *characterStyle, KoParagraphStyle \
*paragraphStyle, QSize size, bool recreateThumbnail, KoStyleThumbnailerFlags \
flags)</pre>
</td>
</tr>
</tbody>
<tbody>
<tr>
<th style="border-right:1px solid #c0c0c0" align="right" \
bgcolor="#e9eaa8"><font>157</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size:8pt;line-height:140%;margin:0"> <span><span>int</span></span> \
<span>paragraphStyleId</span> <span>=</span> \
<span>(</span><span>paragraphStyle</span><span>)</span><span><span>?</span></span><spa \
n>paragraphStyle</span><span><span>-></span></span><span><span>styleId</span></span \
><span><span>()</span></span><span><span>:</span></span><span><span>0</span></span><span>;</span></pre>
>
</td>
<th style="border-left:1px solid #c0c0c0;border-right:1px solid #c0c0c0" \
align="right" bgcolor="#e9eaa8"><font>160</font></th> <td bgcolor="#fdfebc" \
width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> \
<span><span>unsigned</span></span><span> </span><span><span>long</span></span> \
<span>paragraphStyleId</span> <span>=</span> \
<span>(</span><span>paragraphStyle</span><span>)</span> \
<span><span>?</span></span><span> \
</span><span><span>reinterpret_cast</span></span><span><span><</span></span><span><span>unsigned</span></span><span> \
</span><span><span>long</span></span><span><span>></span></span><span><span>(</span></span><span>paragraphStyle</span><span><span>)</span></span><span> \
</span><span><span>:</span></span><span> \
</span><span><span>0L</span></span><span>;</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">paragraphStyleId \
seems a misnomer now</pre> </div>
<br>
<p>- C.</p><div class="im">
<br>
<p>On March 25th, 2013, 5:28 a.m. UTC, Thorsten Zachmann wrote:</p>
</div><table style="background-image:url('');background-repeat:repeat-x;border:1px \
black solid" bgcolor="#fefadf" cellpadding="8" cellspacing="0" width="100%"> \
<tbody><tr> <td><div class="im">
<div>Review request for Calligra and Pierre Stirnweiss.</div>
<div>By Thorsten Zachmann.</div>
<p style="color:grey"><i>Updated March 25, 2013, 5:28 a.m.</i></p>
</div><h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Description </h1><div \
class="im"> <table style="border:1px solid #b8b5a0" bgcolor="#ffffff" \
cellpadding="10" cellspacing="0" width="100%"> <tbody><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">This \
patch tries to fix the problems/crashes which happen in the style manager.
To get the simple crash open the default document and click on the edit button of the \
None (character style)
The patch uses a different model for the style manager and temporary styles that get \
modified to only apply the changes once the user applies them. The model changes the \
internal pointer to the temporary files once they are selected and when applied \
resets to the original ones. However the styles preview in the styles manager list is \
updated immediately. The patch simplifies the handling quite a bit.
I did not know that Pierre was also working on that so I publish it now so we have \
something we can discuss.
If you like this I would like to backport it to 2.6 as it fixes quite some grave bugs \
compared to what we have now.</pre> </td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Testing </h1>
<table style="border:1px solid #b8b5a0" bgcolor="#ffffff" cellpadding="10" \
cellspacing="0" width="100%"> <tbody><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">Tested \
modification without saving. Tested applying.
Tested Ok.
Tested Canceling
</pre>
</td>
</tr>
</tbody></table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Diffs </h1>
</div><div class="im"><ul style="margin-left:3em;padding-left:0">
<li>libs/kotext/styles/KoCharacterStyle.h <span \
style="color:grey">(2fdde12)</span></li>
<li>libs/textlayout/KoStyleThumbnailer.cpp <span \
style="color:grey">(436b243)</span></li>
<li>plugins/textshape/CMakeLists.txt <span style="color:grey">(084628c)</span></li>
<li>plugins/textshape/dialogs/CharacterGeneral.h <span \
style="color:grey">(e6d99db)</span></li>
<li>plugins/textshape/dialogs/CharacterGeneral.cpp <span \
style="color:grey">(d3eead6)</span></li>
<li>plugins/textshape/dialogs/ParagraphGeneral.h <span \
style="color:grey">(1a78376)</span></li>
<li>plugins/textshape/dialogs/ParagraphGeneral.cpp <span \
style="color:grey">(ad30369)</span></li>
<li>plugins/textshape/dialogs/StyleManager.h <span \
style="color:grey">(dec3b6b)</span></li>
<li>plugins/textshape/dialogs/StyleManager.cpp <span \
style="color:grey">(9c82c11)</span></li>
<li>plugins/textshape/dialogs/StylesManagerModel.h <span \
style="color:grey">(PRE-CREATION)</span></li>
<li>plugins/textshape/dialogs/StylesManagerModel.cpp <span \
style="color:grey">(PRE-CREATION)</span></li>
<li>plugins/textshape/dialogs/StylesSortFilterProxyModel.h <span \
style="color:grey">(PRE-CREATION)</span></li>
<li>plugins/textshape/dialogs/StylesSortFilterProxyModel.cpp <span \
style="color:grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109651/diff/" style="margin-left:3em" \
target="_blank">View Diff</a></p>
</div></td>
</tr>
</tbody></table>
</div>
</div>
</blockquote></div><br>
_______________________________________________
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