<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/122278/">https://git.reviewboard.kde.org/r/122278/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Январь 27th, 2015, 7:34 п.п. UTC, <b>Camilla Boemann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="https://git.reviewboard.kde.org/r/122278/diff/1/?file=345390#file345390line120" style="color: black; font-weight: bold; text-decoration: underline;">libs/kotext/KoSectionManager.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="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool KoSectionManager::isValidNewName(const QString &name) const</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool KoSectionManager::isValidNewName(const QString &name)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">85</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">bool</span> <span class="n">KoSectionManager</span><span class="o">::</span><span class="n">isValidNewName</span><span class="p">(</span><span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">name</span><span class="p">)</span><span class="hl"> </span><span class="k"><span class="hl">const</span></span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">115</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">bool</span> <span class="n">KoSectionManager</span><span class="o">::</span><span class="n">isValidNewName</span><span class="p">(</span><span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">name</span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">why no long const ?</p></pre>
</blockquote>
<p>On Январь 27th, 2015, 7:42 п.п. UTC, <b>Denis Kuplyakov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't know why, but QHash<T,Q>::find() isn't const. Thats why I can't make this function const and possibleNewName() too as dependant.</p></pre>
</blockquote>
<p>On Январь 27th, 2015, 7:43 п.п. UTC, <b>Denis Kuplyakov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ohh, I see constFind there. Why they made it such way? Anyway, will fix this.</p></pre>
</blockquote>
<p>On Январь 28th, 2015, 8:05 п.п. UTC, <b>Denis Kuplyakov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There is a sectionNames() call, that updates internal data, to prevent using of invalidated one. So we can't put const here.</p></pre>
</blockquote>
<p>On Январь 28th, 2015, 8:22 п.п. UTC, <b>Friedrich W. H. Kossebau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Could the members which hold that data that is only lazily fetched/updated be tagged with <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">mutable</code> perhaps?
Problem with non-const get-methods is that this escalates up in the call hierarchy, i.e. any other method which uses this method now also needs to be non-const. Which is not nice, because it spoils the usefulness of const, which should protect against unintended data modification. Which lazily updated data surely is not, that's e.g. why there is <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">mutable</code>.</p></pre>
</blockquote>
<p>On Январь 28th, 2015, 8:25 п.п. UTC, <b>Denis Kuplyakov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Can you post me some link, where I can read about what you mean under mutable?</p></pre>
</blockquote>
<p>On Январь 28th, 2015, 8:38 п.п. UTC, <b>Friedrich W. H. Kossebau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Any good physical C++ book, hard to link ;) (no idea about what related ebooks there are)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hm, perhaps this here gives you a start: http://en.cppreference.com/w/cpp/language/cv</p></pre>
</blockquote>
<p>On Январь 28th, 2015, 8:39 п.п. UTC, <b>Elvis Stansvik</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">http://en.cppreference.com/w/cpp/language/cv - "mutable - applies to non-static class members of non-reference non-const type and specifies that the member does not affect the externally visible state of the class. mutable members of const classes are modifiable (Note: the C++ language grammar treats mutable as a storage-class-specifier, but it does not affect storage class)". Wikipedia has a slightly less technical description (second example): http://en.wikipedia.org/wiki/Immutable_object#C.2B.2B</p></pre>
</blockquote>
<p>On Январь 28th, 2015, 9:13 п.п. UTC, <b>Denis Kuplyakov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I have tried to use mutable with no luck. I doesn't understand how can I properly apply it in my case: isValidNewName() calls sectionNames() of its private part, and it calls update() public method. It is absolutely unlogic to make update() const function. And also two subsequent calls of isValidNewName() could give you different results, even if KoSectionManager isn't changed. You can create a section between them without referencing anywhere KoSectionManager (creating KoSection with QTextCursor, will get document associated KoSectionManager by pointer, and update() of it will be called and that will be done regardless of you local entity of KoSectionManager is const or not (which is pointed)). And KoSectionManager isn't supposed to be part of some other class, as I think. It is created dynamically and only links to it are stored.</p></pre>
</blockquote>
<p>On Январь 28th, 2015, 10:28 п.п. UTC, <b>Camilla Boemann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">ah yes, you are right it can't be applied to this case. Hmmm why do sections even have names, and why do they then have to be unique - surely we are not exposing some odf attribute?</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As I remember ODF documents say, that Section element has a Name that should be unique.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Январь 27th, 2015, 7:34 п.п. UTC, <b>Camilla Boemann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="https://git.reviewboard.kde.org/r/122278/diff/1/?file=345394#file345394line321" style="color: black; font-weight: bold; text-decoration: underline;">libs/kotext/commands/DeleteCommand.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; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">317</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">QVariant</span></span><span class="hl"> </span><span class="n">data</span><span class="p">;</span> <span class="c1">// <span class="hl">QVariant version of p</span>ointer to KoSection or KoSectionEnd.</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">266</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt"><span class="hl">void</span></span><span class="hl"> </span><span class="o"><span class="hl">*</span></span><span class="n">data</span><span class="p">;</span> <span class="c1">// <span class="hl"> P</span>ointer to KoSection or KoSectionEnd.</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">yikes</p></pre>
</blockquote>
<p>On Январь 27th, 2015, 7:42 п.п. UTC, <b>Denis Kuplyakov</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QVariant before was used like static_cast<KoSection<em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">>(v.value<void</em>>()). And this structure only for local use, doesn't look dangerous for me.</p></pre>
</blockquote>
<p>On Январь 28th, 2015, 10:31 п.п. UTC, <b>Camilla Boemann</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">well why do we have both KoSection and KoSectionEnd and not just say KoSectionDelimiter that handles both. Just wondering - not saying it has to be like that</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is entirely different entities: KoSection has many things like own style, name and so on (and I can't say that delimiter could have style). And KoSectionEnd is a simple helper class that links to corresponding KoSection.</p></pre>
<br />
<p>- Denis</p>
<br />
<p>On Январь 28th, 2015, 8:21 п.п. UTC, Denis Kuplyakov wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Calligra, Camilla Boemann and Inge Wallin.</div>
<div>By Denis Kuplyakov.</div>
<p style="color: grey;"><i>Updated Янв. 28, 2015, 8:21 п.п.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This all things that I have done after GSoC, but they seems not be in master by now.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">1) Added special functions to retrive startings and endings. Changed all accesses to use this functions from KoSectionUtils. Also extracted some section related functionality to KoSectionUtils.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">2) Fixed wrong indication of section level.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">3) There were no invalidate send to KoSectionManager after paste to update section level.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">4) Added KoSection and KoSectionEnd to Metatype system</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This allowed to reduce number of QVariant casts allwhere
sections are used. Also fixed some missing usings of
KoSectionUtils functions.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">5) Deleted unused code somewhere.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">6) KoSectionManager doesn't store QStandardItemModel now, but can generate it on update. Extracted KoSectionManagerPrivate to cpp file back and changed QScopedPointer to usual pointer for MS VC compiler. Now there is a set of all ever registered sections in KoSectionManager, cuz there are sections that store in memory but cannot be deleted due to possible undo.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">7) Bug with "Incorrect name" is fixed, if you try to rename section with name that existed before.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Have played with sections a bit (deleting, inserting, renaming, undo). And DeleteCommand for sections unittest is passing. Seems to be OK.</p></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>plugins/textshape/dialogs/SimpleInsertWidget.cpp <span style="color: grey">(b5500ae)</span></li>
<li>words/part/dockers/KWDebugWidget.cpp <span style="color: grey">(2aa53c0)</span></li>
<li>libs/kotext/KoSectionManager.h <span style="color: grey">(dc4821f)</span></li>
<li>libs/kotext/KoSectionManager.cpp <span style="color: grey">(df4e535)</span></li>
<li>libs/kotext/KoSectionUtils.h <span style="color: grey">(1bd5eb3)</span></li>
<li>libs/kotext/KoSectionUtils.cpp <span style="color: grey">(cbcefd5)</span></li>
<li>libs/kotext/KoTextPaste.cpp <span style="color: grey">(08c6cf8)</span></li>
<li>libs/kotext/commands/DeleteCommand.cpp <span style="color: grey">(7c69df7)</span></li>
<li>libs/kotext/commands/NewSectionCommand.cpp <span style="color: grey">(50ba14d)</span></li>
<li>libs/kotext/opendocument/KoTextLoader.cpp <span style="color: grey">(fe4713a)</span></li>
<li>libs/kotext/opendocument/KoTextWriter_p.cpp <span style="color: grey">(0e4ea11)</span></li>
<li>libs/kotext/tests/TestKoTextEditor.cpp <span style="color: grey">(a4b02c0)</span></li>
<li>libs/textlayout/KoTextLayoutArea_paint.cpp <span style="color: grey">(d6fa3b8)</span></li>
<li>plugins/textshape/dialogs/SectionFormatDialog.cpp <span style="color: grey">(c825fd2)</span></li>
<li>libs/kotext/KoSection.h <span style="color: grey">(8183958)</span></li>
<li>libs/kotext/KoSection.cpp <span style="color: grey">(a651fd4)</span></li>
<li>libs/kotext/KoSectionEnd.h <span style="color: grey">(0005851)</span></li>
<li>libs/kotext/KoSectionEnd.cpp <span style="color: grey">(0d7cf83)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122278/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>