[Konsole-devel] Review Request: Konsole: Switching tabs resets search

[Jekyll Wu]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105121/#review14319
-----------------------------------------------------------


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 ?


- Jekyll Wu


On May 31, 2012, 8:11 p.m., Lindsay Roberts wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105121/
> -----------------------------------------------------------
> 
> (Updated May 31, 2012, 8:11 p.m.)
> 
> 
> Review request for Konsole.
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> This addresses bug 168769.
>     http://bugs.kde.org/show_bug.cgi?id=168769
> 
> 
> Diffs
> -----
> 
>   src/Screen.h b962dea 
>   src/Screen.cpp cc29cf6 
>   src/ScreenWindow.h adf6182 
>   src/ScreenWindow.cpp e9fa8a0 
>   src/SessionController.h 8c5db37 
>   src/SessionController.cpp 0df29a8 
> 
> Diff: http://git.reviewboard.kde.org/r/105121/diff/
> 
> 
> Testing
> -------
> 
> Tested all combinations of tabs with and without search bars enabled. Changing search contents before switching back, scrolling, deselection searches.
> 
> 
> Thanks,
> 
> Lindsay Roberts
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20120601/a4645bc3/attachment.html>