<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/129956/">https://git.reviewboard.kde.org/r/129956/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 11th, 2017, 4:51 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="https://git.reviewboard.kde.org/r/129956/diff/2/?file=491729#file491729line50" style="color: black; font-weight: bold; text-decoration: underline;">ui/findbar.cpp</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">50</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_search</span><span class="o">-></span><span class="n">lineEdit</span><span class="p">()</span><span class="o">-></span><span class="n">setAutomaticSearch</span><span class="p">(</span> <span class="nb">false</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 this line?</p></pre>
</blockquote>
<p>On March 12th, 2017, 9:22 a.m. UTC, <b>Marco Scarpetta</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;">Because when we call <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">m_automaticSearchAct->setChecked( Okular::Settings::searchAutomatic() )</em> and <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Okular::Settings::searchAutomatic()</em> is false, the signal <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">&QAction::toggled</em> won't be emitted since the checked property is false by default. If <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Okular::Settings::searchAutomatic()</em> is true, <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">&QAction::toggled</em> will be emitted and on line 183, <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">m_search->lineEdit()->setAutomaticSearch</em> will be called with <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">true</em> as argument.</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;">Ok, i see, bit convoluted but fair enough.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 11th, 2017, 4:51 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="https://git.reviewboard.kde.org/r/129956/diff/2/?file=491729#file491729line72" style="color: black; font-weight: bold; text-decoration: underline;">ui/findbar.cpp</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">72</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_automaticSearchAct</span> <span class="o">=</span> <span class="n">optionsMenu</span><span class="o">-></span><span class="n">addAction</span><span class="p">(</span> <span class="n">i18n</span><span class="p">(</span> <span class="s">"Automatic search"</span> <span class="p">)</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;">I think "Find as you type" is much better than "Automatic search", what do you think?</p></pre>
</blockquote>
<p>On March 12th, 2017, 9:22 a.m. UTC, <b>Marco Scarpetta</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;">Yes, I think so, too. Do I have to change variables names, functions names etc? (I think yes)</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;">Not mandatory but if you have the time to do it, it's always nice to have the user visible text and the code variables being similar since it makes for easier finding in the source code.</p></pre>
<br />
<p>- Albert</p>
<br />
<p>On February 20th, 2017, 8:50 a.m. UTC, Marco Scarpetta 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 Okular.</div>
<div>By Marco Scarpetta.</div>
<p style="color: grey;"><i>Updated Feb. 20, 2017, 8:50 a.m.</i></p>
<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=326522">326522</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
okular
</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;">Implement an option to disable the automatic search while typing in okular findbar. When the "automatic search" option is disabled the user must press the return key to start searching.</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;">I ran okular and tested that both searching modes work.</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>conf/okular.kcfg <span style="color: grey">(e06dd91b)</span></li>
<li>ui/findbar.h <span style="color: grey">(2c34d0de)</span></li>
<li>ui/findbar.cpp <span style="color: grey">(09201e78)</span></li>
<li>ui/searchlineedit.h <span style="color: grey">(b8230e65)</span></li>
<li>ui/searchlineedit.cpp <span style="color: grey">(baac8be0)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/129956/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>