<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/109021/">http://git.reviewboard.kde.org/r/109021/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 8th, 2013, 6:02 p.m. UTC, <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/109021/diff/2/?file=142647#file142647line29" style="color: black; font-weight: bold; text-decoration: underline;">core/textdocumentsettings.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">29</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">OKULAR_EXPORT</span> <span class="n">TextDocumentSettings</span> <span class="o">:</span> <span class="n">public</span> <span class="n">QWidget</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 need to install this class so generators outside the okular repo can use it. This means you need to add it to the install list somewhere in the CMakeLists.txt and that you should add a d-pointer so it's possible to keep Binary Compatibility when we need to add more fields, etc. Also some documentation at least mentioning an @since is needed</pre>
</blockquote>
<p>On May 11th, 2013, 4:24 p.m. UTC, <b>Azat Khuzhin</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;">It is already in install()
https://git.reviewboard.kde.org/r/109021/diff/2/?file=142647#file142647line29
What about private class - I agree, will be done.</pre>
</blockquote>
<p>On May 11th, 2013, 7:44 p.m. UTC, <b>Azat Khuzhin</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;">I think here we must add map of widgets/objects into TextDocumentSettings/TextDocumentSettingsSkeleton
To allow manually adding fields from generators.
Don't you think so?</pre>
</blockquote>
<p>On May 11th, 2013, 7:45 p.m. UTC, <b>Azat Khuzhin</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;">Also we can avoid of private classes, if we did like this.</pre>
</blockquote>
<p>On May 13th, 2013, 10:48 p.m. UTC, <b>Albert Astals Cid</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;">You pointed me to a line that contains no install() call, i'm confused.
My idea about adding fields in generators is just calling generalSettings()/generalSettingsWidgets() and adding there the needed stuff.
How does your solution work?</pre>
</blockquote>
<p>On May 13th, 2013, 11:13 p.m. UTC, <b>Azat Khuzhin</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;">Sorry, something wrong happened.
See here if you are still interested in this:
http://quickgit.kde.org/?p=clones%2Fokular%2Fazatkhuzhin%2Fokular.git&a=blob&h=64c4c2a277099bfa3e220a8bbcdeaa355051046e&hb=17e6bca0bbf2094351ff13055165563ffe96bbe3&f=CMakeLists.txt#l101
Seems that in current implementation we can do this, so no need in what I was wrote before.
You just add widget and config item.</pre>
</blockquote>
<p>On May 13th, 2013, 11:31 p.m. UTC, <b>Azat Khuzhin</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;">If you agree with me, please close the issue.</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;">Not sure i follow here. Do you want to have "add map of widgets/objects into TextDocumentSettings/TextDocumentSettingsSkeleton" or just do what i suggested "adding fields in generators is just calling generalSettings()/generalSettingsWidgets() and adding there the needed stuff."?
I can't close issues btw, I think it's only for the person that crated the review request.</pre>
<br />
<p>- Albert</p>
<br />
<p>On May 13th, 2013, 11:30 p.m. UTC, Azat Khuzhin 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, Albert Astals Cid and Eike Hein.</div>
<div>By Azat Khuzhin.</div>
<p style="color: grey;"><i>Updated May 13, 2013, 11:30 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;">Development history:
https://github.com/azat/okular/compare/master...font-selector-for-plain-text-formats
http://quickgit.kde.org/?p=clones%2Fokular%2Fazatkhuzhin%2Fokular.git&a=commitdiff&h=font-selector-for-plain-text-formats&hp=master
(but it has some delay, about a day or so while this clone will be updated, maybe because of mirrors?)
Link to thread from mailing list:
http://comments.gmane.org/gmane.comp.kde.devel.okular/13279</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;">Tested manually</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">(543e6df)</span></li>
<li>conf/textdocumentsettings.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>core/textdocumentgenerator.h <span style="color: grey">(dd75c5c)</span></li>
<li>core/textdocumentgenerator.cpp <span style="color: grey">(f370ded)</span></li>
<li>core/textdocumentgenerator_p.h <span style="color: grey">(749d6f2)</span></li>
<li>core/textdocumentsettings.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>core/textdocumentsettings.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>core/textdocumentsettings_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>generators/epub/CMakeLists.txt <span style="color: grey">(f076ed9)</span></li>
<li>generators/epub/generator_epub.h <span style="color: grey">(cef2879)</span></li>
<li>generators/epub/generator_epub.cpp <span style="color: grey">(59bb2bf)</span></li>
<li>generators/epub/libokularGenerator_epub.desktop <span style="color: grey">(5c853a3)</span></li>
<li>generators/fictionbook/generator_fb.h <span style="color: grey">(a898397)</span></li>
<li>generators/fictionbook/generator_fb.cpp <span style="color: grey">(2317083)</span></li>
<li>generators/fictionbook/libokularGenerator_fb.desktop <span style="color: grey">(099268c)</span></li>
<li>generators/ooo/converter.cpp <span style="color: grey">(1124e2a)</span></li>
<li>generators/ooo/generator_ooo.h <span style="color: grey">(3441c7a)</span></li>
<li>generators/ooo/generator_ooo.cpp <span style="color: grey">(793ee58)</span></li>
<li>generators/ooo/libokularGenerator_ooo.desktop <span style="color: grey">(328ae26)</span></li>
<li>generators/txt/CMakeLists.txt <span style="color: grey">(5a126b7)</span></li>
<li>generators/txt/generator_txt.h <span style="color: grey">(5c15ec4)</span></li>
<li>generators/txt/generator_txt.cpp <span style="color: grey">(93ca4aa)</span></li>
<li>generators/txt/libokularGenerator_txt.desktop <span style="color: grey">(235e23d)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109021/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>