Review Request: Find list items by typing their initial letters.
Peter Penz
peter.penz19 at gmail.com
Sun Aug 28 13:49:12 BST 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102465/#review6096
-----------------------------------------------------------
Ship it!
Looks fine! Please just push it to master after fixing the minor const-comments.
dolphin/src/kitemviews/kitemlistcontroller.cpp
<http://git.reviewboard.kde.org/r/102465/#comment5381>
Please remove the 'const' from searchFromNextItem.
dolphin/src/kitemviews/kitemlistkeyboardsearchmanager.cpp
<http://git.reviewboard.kde.org/r/102465/#comment5382>
const bool
dolphin/src/kitemviews/kitemlistkeyboardsearchmanager.cpp
<http://git.reviewboard.kde.org/r/102465/#comment5383>
const qint64
dolphin/src/kitemviews/kitemlistkeyboardsearchmanager_p.h
<http://git.reviewboard.kde.org/r/102465/#comment5384>
Add a . at the end of the sentence
dolphin/src/kitemviews/kitemlistkeyboardsearchmanager_p.h
<http://git.reviewboard.kde.org/r/102465/#comment5385>
I agree with Frank regarding the bool-parameter. However I'd suggest to get the code pushed to master like this, as it is an internal interface. Probably it might be more readable to have 2 signals here but let's postpone this discussion...
- Peter
On Aug. 28, 2011, 12:21 p.m., Tirtha Chatterjee wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102465/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2011, 12:21 p.m.)
>
>
> Review request for KDE Base Apps and Peter Penz.
>
>
> Summary
> -------
>
> This patch allows finding items by typing on the keyboard while the KItemListController is in focus. Timer and key queuing is handled by KItemListKeyboardManager, and search itself is done by reimplementing indexForKeyboardSearch(QString) from KItemModelBase. This implementation does not, at the moment, take care of the repetitive typing as suggested by Frank. I think we can implement that once this is in.
>
> p.s. Not sure if line 213 in kitemlistcontroller.cpp is the best way to do it. Returning true does not work.
> p.s. I feel the name KItemListKeyboardManager can be changed to KItemListKeyboardSearchManager, although a little too big.
>
>
> Diffs
> -----
>
> dolphin/src/CMakeLists.txt 31d3f89
> dolphin/src/kitemviews/kfileitemmodel.h 654c7db
> dolphin/src/kitemviews/kfileitemmodel.cpp f36ab83
> dolphin/src/kitemviews/kitemlistcontroller.h 134e116
> dolphin/src/kitemviews/kitemlistcontroller.cpp 2e32880
> dolphin/src/kitemviews/kitemlistkeyboardsearchmanager.cpp PRE-CREATION
> dolphin/src/kitemviews/kitemlistkeyboardsearchmanager_p.h PRE-CREATION
> dolphin/src/kitemviews/kitemmodelbase.h 4670469
> dolphin/src/kitemviews/kitemmodelbase.cpp fc604e7
>
> Diff: http://git.reviewboard.kde.org/r/102465/diff
>
>
> Testing
> -------
>
> yes. tested and works.
>
>
> Thanks,
>
> Tirtha
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110828/e8bacddd/attachment.htm>
More information about the kde-core-devel
mailing list