<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;">Thanks for your research and patch. Looks good to me.
Since this patch introduces two changes, it is better to split it into two separate patches and only commit the first at the moment. The thing is we are currently feature frozen for KDE SC 4.9, and the second change looks more like changing existing behavior than bug fixing. So I would suggest opening another review or bug report for the second change and explain/discuss it there first.
By the way, do you have the committing permission ?
</pre>
<br />
<p>- Jekyll</p>
<br />
<p>On May 31st, 2012, 8:11 p.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 May 31, 2012, 8:11 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;">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,
c2) Searching without a selection now searches from the opposite extent of the visible terminal area, instead of the latest line.
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.
f2) Searches with no selection search from the opposite extent of the visible window (c2).
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.</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/Screen.h <span style="color: grey">(b962dea)</span></li>
<li>src/Screen.cpp <span style="color: grey">(cc29cf6)</span></li>
<li>src/ScreenWindow.h <span style="color: grey">(adf6182)</span></li>
<li>src/ScreenWindow.cpp <span style="color: grey">(e9fa8a0)</span></li>
<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>