Review Request: Add replace feature to find in files.

[Milian Wolff]

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

Ship it!


I'm ok with merging it now, but please do some improvements later on:

- the "next/prev" actions only act on the current index and it's siblings, ignoring children. If the root item (e.g. "5 matches in 1 files") is selected (which is the default) the actions won't have any effect, please fix this. Imo you should change the tooltip to "next match" and "previous match" and ignore the root item and file items, but instead only iterate over matches
- I personally didn't see directly how to actually replace something after a search. The "dialog apply" icon is imo too much disconnected. I'd prefer it if you showed push buttons on the right side in the line of the "Replace PATTERN by REPLACE in X files". Something like this:

Replace asdf by Foobar in 123 files          [replace selected] [cancel]
------------------------------------------------------------------------
| matches ...

The cancel button should just hide the toolview imo.

- I kind of dislike how the workflow is for "search (no replace), decide that you want to replace after all". right now it will just redo the search instead of just do the replace step. not sure this can be improved easily but take a look at it and see whether you want to work on this. Low Priority though.

Actually thinking about it, I'd say this could be improved over all:
- only "search" by default, don't ask for a replacement pattern
- show the tree view with the matches in bold
- show a lineedit "replacement" in the toolview

This would make it easy to search and set the right replacement in the view, maybe even applying the replacment on-the-fly in the view below if there is a pattern provided.

Anyhow, very good start I'll merge it now. Bye

- Milian


On 2010-11-27 10:19:28, Benjamin Port wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100175/
> -----------------------------------------------------------
> 
> (Updated 2010-11-27 10:19:28)
> 
> 
> Review request for KDevelop.
> 
> 
> Summary
> -------
> 
> Add replace feature to find in files and create new toolview to display results.
> 
> 
> Diffs
> -----
> 
>   interfaces/idocumentcontroller.h d085a62 
>   plugins/grepview/CMakeLists.txt ce5a084 
>   plugins/grepview/grepdialog.h 5fa2cc0 
>   plugins/grepview/grepdialog.cpp e6fee0d 
>   plugins/grepview/grepfindthread.h 27fa8bb 
>   plugins/grepview/grepfindthread.cpp 02deeed 
>   plugins/grepview/grepjob.h 184701a 
>   plugins/grepview/grepjob.cpp faa7fe6 
>   plugins/grepview/grepoutputdelegate.h 45c9c76 
>   plugins/grepview/grepoutputdelegate.cpp ba3236e 
>   plugins/grepview/grepoutputmodel.h ef60a78 
>   plugins/grepview/grepoutputmodel.cpp 73ffbb2 
>   plugins/grepview/grepoutputview.h PRE-CREATION 
>   plugins/grepview/grepoutputview.cpp PRE-CREATION 
>   plugins/grepview/grepoutputview.ui PRE-CREATION 
>   plugins/grepview/grepviewplugin.h 915c15e 
>   plugins/grepview/grepviewplugin.cpp dfad47e 
>   plugins/grepview/grepwidget.ui 768c5ff 
>   plugins/grepview/kdevgrepview.desktop 512230a 
>   plugins/grepview/kdevgrepview.rc d23a181 
>   plugins/grepview/tests/CMakeLists.txt PRE-CREATION 
>   plugins/grepview/tests/findreplacetest.h PRE-CREATION 
>   plugins/grepview/tests/findreplacetest.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/100175/diff
> 
> 
> Testing
> -------
> 
> We add some test and they work.
> 
> 
> Thanks,
> 
> Benjamin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20101127/aa44dd30/attachment.html>