<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>
</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;">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>
<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>