Review Request: Refactor of KoFind to a general interface for searching

Arjen Hiemstra djfreestyler at gmail.com
Thu Apr 7 14:43:37 BST 2011



> On April 7, 2011, 12:29 p.m., Casper Boemann wrote:
> > Hi it looks nicely coded. Just a few small issues listed below.
> > 
> > I've not actually tried to build aand use it
> > 
> > One question though: Don't KDE have some find toolbar already we could have resued. Okay it keeps us free of relying on KDE but on the other hand this is ui.
> > 
> > As far as I'm concerned it can go in, but I'm not pressing ship it since i'v not actually tried it.

There are actually at least two implementations of the find bar in KDE: There's the one used in Kate, and the one in Konqueror. Kate's seems to be embedded deeply within the KatePart code. Konqueror's is embedded deeply within KHTML code, so both do not seem to be too useable from outside of those libraries.


> On April 7, 2011, 12:29 p.m., Casper Boemann wrote:
> > libs/main/KoFindToolbar.cpp, line 75
> > <http://git.reviewboard.kde.org/r/101045/diff/1/?file=13894#file13894line75>
> >
> >     What, no ui file but hardcoding ??
> >     
> >     oh well.
> >

I did not really consider it important to use a UI file for what essentially amounts to some widgets next to each other. I can change it to make use of one, though.


- Arjen


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


On April 7, 2011, 12:11 p.m., Arjen Hiemstra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101045/
> -----------------------------------------------------------
> 
> (Updated April 7, 2011, 12:11 p.m.)
> 
> 
> Review request for Calligra, Marijn Kruisselbrink, Boudewijn Rempt, Thorsten Zachmann, and Casper Boemann.
> 
> 
> Summary
> -------
> 
> This is the diff of the branch libs-kofind_refactor-ahiemstra and master. It encompasses the entire refactor of KoFind to a more generic structure that can be used by different interfaces and by different document types.
> 
> I consider this to be done API-wise, though several features are currently missing. This review therefore is mostly a request for comments about the current API.
> 
> Still to be done feature-wise:
> - Add support for more options (search in selection, search from cursor, regex, etc)
> - Properly implement replacing, including implementing an interface for it.
> - Add support for searching through multiple text shapes to KoFindText.
> - Implement a searching backend for tables.
> 
> 
> Diffs
> -----
> 
>   libs/kotext/KoTextDocument.h 6d755f8 
>   libs/kotext/KoTextDocument.cpp 1517953 
>   libs/main/CMakeLists.txt a7f3077 
>   libs/main/KoFindBase.h PRE-CREATION 
>   libs/main/KoFindBase.cpp PRE-CREATION 
>   libs/main/KoFindMatch.h PRE-CREATION 
>   libs/main/KoFindMatch.cpp PRE-CREATION 
>   libs/main/KoFindOption.h PRE-CREATION 
>   libs/main/KoFindOption.cpp PRE-CREATION 
>   libs/main/KoFindOptionSet.h PRE-CREATION 
>   libs/main/KoFindOptionSet.cpp PRE-CREATION 
>   libs/main/KoFindText.h PRE-CREATION 
>   libs/main/KoFindText.cpp PRE-CREATION 
>   libs/main/KoFindToolbar.h PRE-CREATION 
>   libs/main/KoFindToolbar.cpp PRE-CREATION 
>   libs/main/tests/CMakeLists.txt 6f13036 
>   libs/main/tests/testfindmatch.h PRE-CREATION 
>   libs/main/tests/testfindmatch.cpp PRE-CREATION 
>   plugins/textshape/TextShape.cpp 1c8f2c5 
>   words/part/KWView.h a8eec16 
>   words/part/KWView.cpp 539e6b1 
> 
> Diff: http://git.reviewboard.kde.org/r/101045/diff
> 
> 
> Testing
> -------
> 
> I have implemented a toolbar interface for Words, which is enabled in this review. It works very well, though it does not yet feature all the options the current dialog has. Furthermore, I have implemented unit tests for KoFindMatch, the most important class in this code, though arguably at least KoFindOption/KoFindOptionSet also could do with some unit testing.
> 
> 
> Thanks,
> 
> Arjen
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20110407/c115ebf9/attachment.htm>


More information about the calligra-devel mailing list