[Konsole-devel] Review Request 111766: Incremental search bar improvements

[Kurt Hindenburg]

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



src/IncrementalSearchBar.cpp
<http://git.reviewboard.kde.org/r/111766/#comment28284>

    Try to follow the kdelibs style
    
    so 
    if () {
    } else {
    }
    http://techbase.kde.org/Policies/Kdelibs_Coding_Style



src/SessionController.cpp
<http://git.reviewboard.kde.org/r/111766/#comment28285>

    use extra () for stuff like this



src/SessionController.cpp
<http://git.reviewboard.kde.org/r/111766/#comment28286>

    and here ()



src/TerminalDisplay.cpp
<http://git.reviewboard.kde.org/r/111766/#comment28287>

    () and throw it on 1 line


A few minor code issues which can be fixed later if necessary.  Let me double-check the code and do some testing.  Likely we should break this up into smaller patches but I'm OK with it.

One thing that bothers me is that w/ the search buttons, my terminal smallest width is 56 chars (depending on font size) mainly due to the 'Search from bottom'.  Also, if you have a 40 char width and toggle the search, it will increase the width of the terminal window. With the current code it is 41 characters.  I don't really like this but not sure how to shorten the buttons.

- Kurt Hindenburg


On Aug. 17, 2013, 8:22 a.m., Harald Hvaal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111766/
> -----------------------------------------------------------
> 
> (Updated Aug. 17, 2013, 8:22 a.m.)
> 
> 
> Review request for Konsole and Kurt Hindenburg.
> 
> 
> Description
> -------
> 
> commit 4f3ac690828c4d7a707ddae38d977798a6e2c13d
> Author: Harald Hvaal <harald.hvaal at gmail.com>
> Date:   Sun Jul 28 11:24:37 2013
> 
>     Various search-related improvements
>     
>     - Add "search from beginning" feature
>         This will scroll to the top and search from there.
>         Ctrl+return is also bound to this action
>     - Text Highlighted by mouse will be set as the current search text
>     - Add "Search backwards" to search bar options
> 
> commit 36e59043ba41b4ea9fd31bd961b0b88e20da10e1
> Author: Harald Hvaal <harald.hvaal at gmail.com>
> Date:   Sat Jul 27 10:07:21 2013
> 
>     When showing the search bar, do not invoke a search, only apply the highlight filters
> 
> commit fb2202fd4f54d0e1a82a861d0d998582cc08780c
> Author: Harald Hvaal <harald.hvaal at gmail.com>
> Date:   Sat Jul 27 19:51:32 2013
> 
>     Scroll with the result centered
> 
> commit 7b8f805fd15e18bb6cdba520292dd7afb0f7e4e3
> Author: Harald Hvaal <harald.hvaal at gmail.com>
> Date:   Sat Jul 27 09:39:27 2013
> 
>     Do not automatically reset the search start line on search hits
>     
>     This was causing the annoying behavior that if you were to pause while typing
>     in a search term, and it would actually find a hit, then you would be searching
>     for the term a second time once you finish typing.
>     
>     Example console output:
>     
>      ***
>     1 usb
>     2 hdmi
>     3 usb
>     4 hdmi
>     5 usb
>      ***
>     
>     if you were to search for this by quickly typing "usb" you would get the hit on
>     line 1.  If you type "us", wait a moment, then type "b", you will end up on
>     line 3. When searching through large console output, this is frustrating as you
>     would never really be sure whether you are at the first search result without
>     double-checking.
>     
>     This commit introduces two new behaviors:
>     1. When you show the search bar, all searching will be done from the first
>         visible line in the terminal.
>     2. This start position is only reset when you
>         advance to the next result, by pressing "next", "previous", or the shortcuts
>         RETURN or SHIFT-RETURN
>     
>     In the example above, this would ensure that you would end up at the hit on
>     line 1 as expected.
> 
> 
> Diffs
> -----
> 
>   src/TerminalDisplay.cpp acfddc176bc68ae9b0f3a9ea52c958e37d14dff8 
>   src/TerminalDisplay.h 100ffccc3f8210d5f07d9d24aa14032766415205 
>   src/SessionController.cpp 88fc50c6a164fed94eefc72a71e4460f15ede57e 
>   src/SessionController.h 036e0534960fe7a1bd32be04136ba162c0469413 
>   src/ScreenWindow.cpp 92804302c7df6658a10ee0b8000eae9ec5617df0 
>   src/ScreenWindow.h 89fe7bed73b41749fae03f24c1868aee62369b18 
>   src/IncrementalSearchBar.cpp 79a28773c64513ccc95375eef9a51b0e13b81a9f 
>   src/IncrementalSearchBar.h d0661a5ae539fe6e11800e2367d6a55660b3028b 
> 
> Diff: http://git.reviewboard.kde.org/r/111766/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Harald Hvaal
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20130821/165a19af/attachment-0001.html>