Review Request 127908: Find/Replace in Files: don't exclude items that trigger the exclude filter above the search location

[René J.V. Bertin]

> On May 24, 2016, 11:27 p.m., Milian Wolff wrote:
> > personally, I think you should instead check the exclude filter against the start location of the search. If that is already excluded, show a helpful message to the user explaining that nothing can possibly be matched and that he needs to change the filter then

If we ignore opinions on whether there is an issue for an instant, does my patch seem an appropriate and correct solution?

I don't get why you would apply an exclude filter on data that is irrelevant. You're searching in a location, anything outside that location is excluded by default and it simply doesn't make sense to apply filters on it IMHO. There are also performance implications to it, though that's probably only a theoretical argument.
Take the case of users who have to work on say, a shared partition called "build", on which each has his/her own space for development. Supposing they put a proper build directory inside the source tree, they will just have to put up with a search that includes that directory if we follow your suggestion. 

IIRC there's also a discrepancy between the regular and the "search only project files" modes, I'm pretty sure that latter mode wasn't affected (I'm running my own patch so cannot verify this quickly).

There is of course a way to work around the particular instance of the issue that made me realise there is an issue to begin with. If the CMake project manager uses out-of-source build directories by default the default exclude filter no longer needs to include the build directory.
I think this is a safe thing to do. At MacPorts we've made out-of-source building the default for all ports that use CMake because it's something that CMake supports inherently. If there are projects where this is not supported they are so few that I didn't pick up any echos of it. It's always been the default for all KDE projects.


- René J.V.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127908/#review95760
-----------------------------------------------------------


On May 14, 2016, 8:52 a.m., René J.V. Bertin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127908/
> -----------------------------------------------------------
> 
> (Updated May 14, 2016, 8:52 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> "Find/Replace in Files" has an include and an exclude filter. The former serves to consider only files matching a pattern under the search location. The latter serves to exclude files matching a pattern, but considers the full path.
> 
> The standard exclude pattern contains the pattern `/build/`, to exclude the build directory. Filtering on the full path means that it is not possible to search in a location that has an ancestor called `build`.
> 
> This patch attempts to mask the patch above the search location from the exclude filter.
> 
> It could even mask the search location itself from that filter, which would allow to search e.g. in a typical build directory by setting the location to that directory, and without having to edit the exclude filter.
> 
> I'm not very proud of the expression used to determine the location's `dirName`, but I saw no better way since it presumably has to be a canonical (normalised) path and there is no `dirName()` method.
> 
> 
> Diffs
> -----
> 
>   plugins/grepview/grepfindthread.cpp 9388d5c 
> 
> Diff: https://git.reviewboard.kde.org/r/127908/diff/
> 
> 
> Testing
> -------
> 
> Search now works as I'd expect (when searching the files of a project) regardless of where the search location is stored.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

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