<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/100175/">http://git.reviewboard.kde.org/r/100175/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre>
<br />
<p>- Milian</p>
<br />
<p>On November 27th, 2010, 10:19 a.m., Benjamin Port wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDevelop.</div>
<div>By Benjamin Port.</div>
<p style="color: grey;"><i>Updated 2010-11-27 10:19:28</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Add replace feature to find in files and create new toolview to display results.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">We add some test and they work.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>interfaces/idocumentcontroller.h <span style="color: grey">(d085a62)</span></li>
<li>plugins/grepview/CMakeLists.txt <span style="color: grey">(ce5a084)</span></li>
<li>plugins/grepview/grepdialog.h <span style="color: grey">(5fa2cc0)</span></li>
<li>plugins/grepview/grepdialog.cpp <span style="color: grey">(e6fee0d)</span></li>
<li>plugins/grepview/grepfindthread.h <span style="color: grey">(27fa8bb)</span></li>
<li>plugins/grepview/grepfindthread.cpp <span style="color: grey">(02deeed)</span></li>
<li>plugins/grepview/grepjob.h <span style="color: grey">(184701a)</span></li>
<li>plugins/grepview/grepjob.cpp <span style="color: grey">(faa7fe6)</span></li>
<li>plugins/grepview/grepoutputdelegate.h <span style="color: grey">(45c9c76)</span></li>
<li>plugins/grepview/grepoutputdelegate.cpp <span style="color: grey">(ba3236e)</span></li>
<li>plugins/grepview/grepoutputmodel.h <span style="color: grey">(ef60a78)</span></li>
<li>plugins/grepview/grepoutputmodel.cpp <span style="color: grey">(73ffbb2)</span></li>
<li>plugins/grepview/grepoutputview.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/grepview/grepoutputview.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/grepview/grepoutputview.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/grepview/grepviewplugin.h <span style="color: grey">(915c15e)</span></li>
<li>plugins/grepview/grepviewplugin.cpp <span style="color: grey">(dfad47e)</span></li>
<li>plugins/grepview/grepwidget.ui <span style="color: grey">(768c5ff)</span></li>
<li>plugins/grepview/kdevgrepview.desktop <span style="color: grey">(512230a)</span></li>
<li>plugins/grepview/kdevgrepview.rc <span style="color: grey">(d23a181)</span></li>
<li>plugins/grepview/tests/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/grepview/tests/findreplacetest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>plugins/grepview/tests/findreplacetest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100175/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>