Re: Review Request: Find In Files — Allow depth definition instead of just recursion

Adrián Chaves Fernández adriyetichaves at gmail.com
Mon Sep 17 05:44:35 UTC 2012


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

(Updated Sept. 17, 2012, 5:44 a.m.)


Review request for KDevelop.


Changes
-------

OK, I give up. Same steps but without removing the changes to trailing spaces in chunks that also contain meaningful changes. Since the problem was readability, I guess (hope) those are not that important and can be passed.


Description
-------

I got the Find In Files dialog to allow the user to specify a level of depth for the search instead of just whether it should use full depth (recursion) or none (no recursion).

This is meant to be my first step in the implementation of Find In Files for (current file and) Included Files (https://bugs.kde.org/show_bug.cgi?id=160143). Just in case I give up with the task before I finish the whole of it, I figured out it would be better to at least send you this bit now, since it is already an enhancement over the current behavior.


Diffs (updated)
-----

  plugins/grepview/grepdialog.h d4a1db6 
  plugins/grepview/grepdialog.cpp d1f4bf7 
  plugins/grepview/grepfindthread.h b24520c 
  plugins/grepview/grepfindthread.cpp bc39d87 
  plugins/grepview/grepjob.h c357b77 
  plugins/grepview/grepjob.cpp 66df1a9 
  plugins/grepview/grepwidget.ui 94cfb00 
  plugins/grepview/tests/findreplacetest.cpp ac0687d 

Diff: http://git.reviewboard.kde.org/r/105856/diff/


Testing
-------

I have tested the feature by looking in “kdevplatform” for ‘All Open’ (as in “All Open Files”, one of the optiona locations for Find In Files). I tried with the following depths: Full (-1), 0, 1, 2. Only the first one and the last one gave me a match, for the file kdevplatform(0)/plugins(1)/grepview(2)/grepdialog.cpp.

I did not write an automated test for the feature, although I did updated the current tests so the changes do not break it.

Since the current tests do not actually test for different recursion values (only recursion=true), I didn’t feel like spending time on testing the change (specially since I have never written one in C++/Qt before). But if it were considered a requirement to apply the patch, I would give it a try.


Thanks,

Adrián Chaves Fernández

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


More information about the KDevelop-devel mailing list