<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/110566/">http://git.reviewboard.kde.org/r/110566/</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;">Looks good overall, it's good taht it doesn't touch core/ at all so if we need to change something taht was horribly wrong and we didn't realize we can still do it in a minor revision. OTOH I'm kind of thinking that some of this code might make more sense in core/ (like the code that "creates" the definitions of the tools, etc) but we are always on time to move it "down" later if we need.
Some minor stuff coming as comments</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/110566/diff/1/?file=145416#file145416line62" style="color: black; font-weight: bold; text-decoration: underline;">conf/okular.kcfg</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">62</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> }</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;">Maybe add a kWarning here if annotationTools content is empty? The old code seemed to have them, no?</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/110566/diff/1/?file=145418#file145418line51" style="color: black; font-weight: bold; text-decoration: underline;">conf/preferencesdialog.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; ">PreferencesDialog::PreferencesDialog( QWidget * parent, KConfigSkeleton * skeleton, Okular::EmbedMode embedMode )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">51</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">addPage</span><span class="p">(</span> <span class="n">m_identity</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Identity"</span><span class="p">),</span> <span class="s">"preferences-desktop-personal"</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">51</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">addPage</span><span class="p">(</span> <span class="n">m_annotations</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Annotations"</span><span class="p">),</span> <span class="s">"draw-freehand"</span><span class="p">,</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Annotation Options"</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;">Instead of draw-freehand maybe use the same icon we use in the side panel?</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/110566/diff/1/?file=145421#file145421line291" style="color: black; font-weight: bold; text-decoration: underline;">conf/widgetannottools.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">291</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_type</span><span class="o">-></span><span class="n">addItem</span><span class="p">(</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Pop-up Note"</span><span class="p">),</span> <span class="n">qVariantFromValue</span><span class="p">(</span> <span class="n">ToolNoteLinked</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;">Would it make sense to try to reuse the defaultToolName function?</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/110566/diff/1/?file=145421#file145421line298" style="color: black; font-weight: bold; text-decoration: underline;">conf/widgetannottools.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">298</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_type</span><span class="o">-></span><span class="n">addItem</span><span class="p">(</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Stamp"</span><span class="p">),</span> <span class="n">qVariantFromValue</span><span class="p">(</span> <span class="n">ToolStamp</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;">maybe i18nc here so it is clear that this is a noun and not a verb?</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/110566/diff/1/?file=145453#file145453line1083" style="color: black; font-weight: bold; text-decoration: underline;">ui/pageviewannotator.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">1077</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">i18n</span><span class="p">(</span> <span class="s">"Stamp"</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;">Maybe some i18nc here too?</pre>
</div>
<br />
<p>- Albert</p>
<br />
<p>On May 20th, 2013, 7 p.m. UTC, Fabio D'Urso 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 Fabio D'Urso.</div>
<p style="color: grey;"><i>Updated May 20, 2013, 7 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;">Diff dump from the configurable-review-tools branch, as requested in http://mail.kde.org/pipermail/okular-devel/2013-May/015009.html
This patch mainly addresses bug 159601, but it also adds GUI control to configure some annotation properties (text alignment in inline notes, stroke width in freehand lines, background color in polygons) and changes some texts.
Use
gitk origin/configurable-review-tools ^origin/master --no-merges
for a detailed changelog.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=159601">159601</a>
</div>
<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">(64c4c2a)</span></li>
<li>Messages.sh <span style="color: grey">(6d0d0b0)</span></li>
<li>conf/dlgannotations.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/dlgannotations.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/dlgannotationsbase.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/dlgidentity.h <span style="color: grey">(1bbd937)</span></li>
<li>conf/dlgidentity.cpp <span style="color: grey">(8585716)</span></li>
<li>conf/dlgidentitybase.ui <span style="color: grey">(15752eb)</span></li>
<li>conf/okular.kcfg <span style="color: grey">(4a2aaf3)</span></li>
<li>conf/preferencesdialog.h <span style="color: grey">(3340487)</span></li>
<li>conf/preferencesdialog.cpp <span style="color: grey">(9f6d339)</span></li>
<li>conf/settings.kcfgc <span style="color: grey">(060f260)</span></li>
<li>conf/widgetannottools.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>conf/widgetannottools.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/annotationpropertiesdialog.h <span style="color: grey">(d1a1c27)</span></li>
<li>ui/annotationpropertiesdialog.cpp <span style="color: grey">(5d86d79)</span></li>
<li>ui/annotationwidgets.h <span style="color: grey">(1832876)</span></li>
<li>ui/annotationwidgets.cpp <span style="color: grey">(ce8a91b)</span></li>
<li>ui/data/CMakeLists.txt <span style="color: grey">(6501be5)</span></li>
<li>ui/data/sources/tool-base-okular.svgz <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/data/sources/tool-highlighter-okular-colorizable.svgz <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/data/sources/tool-ink-okular-colorizable.svgz <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/data/sources/tool-note-inline-okular-colorizable.svgz <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/data/sources/tool-note-okular-colorizable.svgz <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/data/tool-base-okular.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/data/tool-ellipse-okular.png <span style="color: grey">(6a3260e)</span></li>
<li>ui/data/tool-highlighter-okular-colorizable.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/data/tool-highlighter-okular.png <span style="color: grey">(594ba41)</span></li>
<li>ui/data/tool-ink-okular-colorizable.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/data/tool-ink-okular.png <span style="color: grey">(8a2eeb0)</span></li>
<li>ui/data/tool-line-okular.png <span style="color: grey">(a2dda94)</span></li>
<li>ui/data/tool-note-inline-okular-colorizable.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/data/tool-note-inline-okular.png <span style="color: grey">(4d8187f)</span></li>
<li>ui/data/tool-note-okular-colorizable.png <span style="color: grey">(PRE-CREATION)</span></li>
<li>ui/data/tool-note-okular.png <span style="color: grey">(a89c91b)</span></li>
<li>ui/data/tool-polygon-okular.png <span style="color: grey">(66ba2cb)</span></li>
<li>ui/data/tool-stamp-okular.png <span style="color: grey">(e53a04a)</span></li>
<li>ui/data/tool-underline-okular.png <span style="color: grey">(924772f)</span></li>
<li>ui/data/tools.xml <span style="color: grey">(5e3cb84)</span></li>
<li>ui/guiutils.h <span style="color: grey">(73c0838)</span></li>
<li>ui/guiutils.cpp <span style="color: grey">(af06000)</span></li>
<li>ui/pagepainter.h <span style="color: grey">(23ac845)</span></li>
<li>ui/pagepainter.cpp <span style="color: grey">(2890b56)</span></li>
<li>ui/pageview.cpp <span style="color: grey">(26f5516)</span></li>
<li>ui/pageviewannotator.h <span style="color: grey">(850d887)</span></li>
<li>ui/pageviewannotator.cpp <span style="color: grey">(035c1f3)</span></li>
<li>ui/pageviewutils.h <span style="color: grey">(0aaf057)</span></li>
<li>ui/pageviewutils.cpp <span style="color: grey">(c2e0388)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110566/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>