<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/102946/">http://git.reviewboard.kde.org/r/102946/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 31st, 2011, 4:25 p.m., <b>Albert Astals Cid</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="http://git.reviewboard.kde.org/r/102946/diff/2/?file=39710#file39710line779" style="color: black; font-weight: bold; text-decoration: underline;">core/document.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">class OKULAR_EXPORT DocumentViewport</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">777</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">DocumentViewport</span><span class="p">(</span> <span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">description</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">779</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">DocumentViewport</span><span class="p">(</span> <span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">description</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">Okular</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">ViewportType</span></span><span class="hl"> </span><span class="n"><span class="hl">type</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span><span class="hl"> </span><span class="n"><span class="hl">Okular</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">NormalViewport</span></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;">We try to maintain source and binary compatibility in the okular core library. This is source compatible but unfortunately not binary compatible, do you really need to store the viewport type? This information does not really seem to belong to the viewport to me, a viewport is a given position into the view, how you got to create it should not be part of it in my opinion. Maybe you can store something like m_lastViewportChangeBecauseOfBlaBla in document and the use it on notifyviewport?</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;">I thought about adding a flag to the document which indicates that the last viewport results from a source location. But that becomes a bit messy, I think, as one has to take care that other code paths which also manipulate the viewport clear that flag.
Hence, I came up with the idea to add a type to the viewport. I think it is justified as the source location viewports are somewhat special. :)
What do you think of not modifying the constructor but simply adding a new method like 'setFromSourceLocation(bool)' to the viewport class? That should be binary compatible.
Or maybe you have another suggestion?</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 31st, 2011, 4:25 p.m., <b>Albert Astals Cid</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="http://git.reviewboard.kde.org/r/102946/diff/2/?file=39716#file39716line2447" style="color: black; font-weight: bold; text-decoration: underline;">part.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 Part::unsetDummyMode()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">2190</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// ensure history actions are in the correct state</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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;">You sure we do not need this anymore? Doesn't seem much related to the general patch</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;">This was done twice before, I think. Once in 'unsetDummyMode()' and once in the constructor. Now, it's only done in the constructor (line 494).</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 31st, 2011, 4:25 p.m., <b>Albert Astals Cid</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="http://git.reviewboard.kde.org/r/102946/diff/2/?file=39720#file39720line3283" style="color: black; font-weight: bold; text-decoration: underline;">ui/pageview.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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 PageView::updateZoomText()</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">3279</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">d</span><span class="o">-></span><span class="n">aZoom</span><span class="o">-></span><span class="n">setEnabled</span><span class="p">(</span><span class="n">d</span><span class="o">-></span><span class="n">items</span><span class="p">.</span><span class="n">size</span><span class="p">()</span> <span class="o">></span> <span class="mi">0</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;">When will zoom have less than 0 items?</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;">Never, but d->items is a vector of pointers to PageViewItem, which will be empty whenever there are no pages ;)</pre>
<br />
<p>- Michel</p>
<br />
<p>On October 24th, 2011, 8:35 p.m., Michel Ludwig 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 Okular.</div>
<div>By Michel Ludwig.</div>
<p style="color: grey;"><i>Updated Oct. 24, 2011, 8:35 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;">The functionality that I need for Kile should now be implemented, i.e. handling of source references, disabling of certain actions and configuration options in 'ViewerWidgetMode', and drawing of source locations.
Note that currently the drawing of source locations doesn't work correctly for rotated pages. This is due to the fact that locating source references for rotated doesn't seem to be implemented in Okular yet. I'm thinking of disabling the rotate-page actions in 'ViewerWidgetMode' until this is implemented in Okular.
If you want to see the viewer mode in action, you can try it out by following the instructions given here:
http://sourceforge.net/apps/mediawiki/kile/index.php?title=Live_Preview
</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>CMakeLists.txt <span style="color: grey">(f8dcba0)</span></li>
<li>conf/dlggeneral.h <span style="color: grey">(1ee2768)</span></li>
<li>conf/dlggeneral.cpp <span style="color: grey">(80478e6)</span></li>
<li>conf/okular.kcfg <span style="color: grey">(b7d511c)</span></li>
<li>conf/preferencesdialog.h <span style="color: grey">(72a7072)</span></li>
<li>conf/preferencesdialog.cpp <span style="color: grey">(5ea6269)</span></li>
<li>core/document.h <span style="color: grey">(2bcf280)</span></li>
<li>core/document.cpp <span style="color: grey">(a417828)</span></li>
<li>core/global.h <span style="color: grey">(24cef77)</span></li>
<li>interfaces/viewerinterface.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>part-viewermode.rc <span style="color: grey">(PRE-CREATION)</span></li>
<li>part.h <span style="color: grey">(f30eb6d)</span></li>
<li>part.cpp <span style="color: grey">(9219db7)</span></li>
<li>ui/pagepainter.h <span style="color: grey">(4044dd8)</span></li>
<li>ui/pagepainter.cpp <span style="color: grey">(2bd2f23)</span></li>
<li>ui/pageview.h <span style="color: grey">(b89f346)</span></li>
<li>ui/pageview.cpp <span style="color: grey">(b119e82)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/102946/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>