<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/107047/">http://git.reviewboard.kde.org/r/107047/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The patch looks ok minor the two small inline comments.
Having a look at the patch though i see we have those *special* ids, and i think it would make much more sense not to have any, that would mean having a look at what those special ids do and splitting it into separate options of either observer, or the function calls or whatever. But maybe this is too much for this and we should just commit this first and then try to generalize those?
What do you all think?</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/107047/diff/3/?file=113487#file113487line4316" style="color: black; font-weight: bold; text-decoration: underline;">core/document.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<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">4316</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="nb">false</span> <span class="o">==</span> <span class="p">((</span><span class="mi">0</span> <span class="o"><=</span> <span class="n">pObserver</span><span class="o">-></span><span class="n">observerId</span><span class="p">())</span> <span class="o">&&</span> <span class="p">(</span><span class="n">OBS_LENGTH</span> <span class="o">></span> <span class="n">pObserver</span><span class="o">-></span><span class="n">observerId</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;">No yoda syntax for ifs please :-)</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/107047/diff/3/?file=113505#file113505line429" style="color: black; font-weight: bold; text-decoration: underline;">ui/presentationwidget.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; ">void PresentationWidget::notifyCurrentPageChanged( int previousPage, int currentPage )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">427</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span> <span class="o">!</span><span class="n">frame</span><span class="o">-></span><span class="n">page</span><span class="o">-></span><span class="n">hasPixmap</span><span class="p">(</span> <span class="n"><span class="hl">PRESENTATION_ID</span></span><span class="p">,</span> <span class="n">pixW</span><span class="p">,</span> <span class="n">pixH</span> <span class="p">)</span> <span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">429</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span> <span class="o">!</span><span class="n">frame</span><span class="o">-></span><span class="n">page</span><span class="o">-></span><span class="n">hasPixmap</span><span class="p">(</span> <span class="n"><span class="hl">Okular</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">DocumentObserver</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">observerId</span></span><span class="p"><span class="hl">()</span>,</span> <span class="n">pixW</span><span class="p">,</span> <span class="n">pixH</span> <span class="p">)</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;">Does the shorted "observerId()" work here?</pre>
</div>
<br />
<p>- Albert</p>
<br />
<p>On February 10th, 2013, 9:51 p.m. UTC, Bogdan Cristea wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Okular.</div>
<div>By Bogdan Cristea.</div>
<p style="color: grey;"><i>Updated Feb. 10, 2013, 9:51 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;">This patch is related to settings separation for the frontend and the backend. It proposes the installation of core/observer.h and corrects compilation of okular on Windows (using KDE on windows):
- after separation, settings.h and settings_core.h need to use different precompiler switches for exporting/importing symbols
- add definitions needed to activate these switches on Windows
</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;">no</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>ui/minibar.h <span style="color: grey">(a0c0514)</span></li>
<li>ui/pagepainter.cpp <span style="color: grey">(91ae211)</span></li>
<li>ui/pagesizelabel.h <span style="color: grey">(7c4a1e2)</span></li>
<li>ui/pageview.h <span style="color: grey">(d8a7653)</span></li>
<li>ui/pageview.cpp <span style="color: grey">(60a273d)</span></li>
<li>ui/presentationwidget.h <span style="color: grey">(1608ef8)</span></li>
<li>ui/presentationwidget.cpp <span style="color: grey">(35b9d34)</span></li>
<li>ui/side_reviews.h <span style="color: grey">(bbd8324)</span></li>
<li>ui/thumbnaillist.h <span style="color: grey">(20c434f)</span></li>
<li>ui/thumbnaillist.cpp <span style="color: grey">(33a5431)</span></li>
<li>ui/toc.h <span style="color: grey">(8679648)</span></li>
<li>ui/toc.cpp <span style="color: grey">(bde7c97)</span></li>
<li>CMakeLists.txt <span style="color: grey">(e40cfd6)</span></li>
<li>active/components/pageitem.cpp <span style="color: grey">(a04a8dc)</span></li>
<li>core/document.h <span style="color: grey">(1d825e1)</span></li>
<li>core/document.cpp <span style="color: grey">(372af56)</span></li>
<li>core/generator.cpp <span style="color: grey">(402c881)</span></li>
<li>core/observer.h <span style="color: grey">(f7189be)</span></li>
<li>core/observer.cpp <span style="color: grey">(59bbb11)</span></li>
<li>core/page.h <span style="color: grey">(6bc60c5)</span></li>
<li>core/page.cpp <span style="color: grey">(4df58e0)</span></li>
<li>core/page_p.h <span style="color: grey">(75575a7)</span></li>
<li>generators/chm/generator_chm.cpp <span style="color: grey">(c342a10)</span></li>
<li>part.h <span style="color: grey">(e3a9418)</span></li>
<li>ui/annotationmodel.cpp <span style="color: grey">(d6d234d)</span></li>
<li>ui/bookmarklist.h <span style="color: grey">(cb8fcc3)</span></li>
<li>ui/bookmarklist.cpp <span style="color: grey">(e1b3869)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107047/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>