<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/105121/">http://git.reviewboard.kde.org/r/105121/</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;">This review has been submitted with commit cc7af6655625242a4dda2ed5c4c24d26b37f88ea by Jekyll Wu to branch master.</pre>
<br />
<p>- Commit</p>
<br />
<p>On June 1st, 2012, 7:17 a.m., Lindsay Roberts 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 Konsole.</div>
<div>By Lindsay Roberts.</div>
<p style="color: grey;"><i>Updated June 1, 2012, 7:17 a.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;">Switching to a tab with history search currently produces unexpected results.
This patch makes the following functional changes:
c1) Switching tabs back to a tab with search no longer resets the view to the first top down search result,
Technical issue details:
t1) Switching tab was clearing search state and restarting.
t2) Search position state is based entirely on terminal selection.
t3) Terminal selection is cleared when the window size changes.
t4) The window size changes whenever the search bar is shown/hidden.
t5) The search bar is shown/hidden when swapping between tabs that have differing enablement of the search bar.
t6) Search start position without selection was the most recent line.
t7) Default search direction is (Old -> New).
Combined, every tab switch in most circumstances was resetting the view to the oldest match against the current search bar contents.
Issues fixed:
f1) Switching tabs now only handles re-enable of the search bar, search highlights and buffer position do not change.
Issues unaddressed:
u1) Retaining selection - believe out of scope.
u2) Default search direction. I believe it would be more in line with user expectation to search from newest to oldest by default, but didn't want to see the current patch blocked by controversy.
u3) Per-tab search text - again out of scope.
u4) [Moved out for separate review] Searches with no selection search from the opposite extent of the visible window.</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 all combinations of tabs with and without search bars enabled. Changing search contents before switching back, scrolling, deselection searches.</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=168769">168769</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>src/SessionController.h <span style="color: grey">(8c5db37)</span></li>
<li>src/SessionController.cpp <span style="color: grey">(0df29a8)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105121/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>