<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127908/">https://git.reviewboard.kde.org/r/127908/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 24th, 2016, 11:27 p.m. CEST, <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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</p></pre>
</blockquote>
<p>On May 25th, 2016, 10:41 a.m. CEST, <b>René J.V. Bertin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If we ignore opinions on whether there is an issue for an instant, does my patch seem an appropriate and correct solution?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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. </p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On August 2nd, 2017, 12:28 p.m. CEST, <b>Friedrich W. H. Kossebau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I would agree that this is a bug: the exclude filter should only match against the part of the path which is below the root dirs where the search is done (like e.g. the arguments to "find" cmdl tool also apply on the dir names traversed, and not the starting one).
Not yet run into this personally, but I can see how a few people might have "build" in their root dir, or trying to exclude other custom subdirs which are generic enough to also be used for other purposes in high level dir structures ("kde", "docs", "templates", "other", "data", "src"). Chance is low, but there is. And fix should not be too complicated.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Not sure about the patch though, only quickly looked at it. Will have to look into how QDir::match actually works and then at the context code.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks for reviving this. I'm still using the patch (which still applies cleanly to the 5.1 branch at least)</p></pre>
<br />
<p>- René J.V.</p>
<br />
<p>On May 14th, 2016, 8:52 a.m. CEST, René J.V. Bertin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDevelop.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated May 14, 2016, 8:52 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevplatform
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">"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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The standard exclude pattern contains the pattern <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">/build/</code>, 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 <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">build</code>.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch attempts to mask the patch above the search location from the exclude filter.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm not very proud of the expression used to determine the location's <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">dirName</code>, but I saw no better way since it presumably has to be a canonical (normalised) path and there is no <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">dirName()</code> method.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Search now works as I'd expect (when searching the files of a project) regardless of where the search location is stored.</p></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>plugins/grepview/grepfindthread.cpp <span style="color: grey">(9388d5c)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127908/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>