Issue #8557 ("Over 100000 matches" when there are exactly 100000 matches) #9385
Conversation
| @@ -157,6 +161,13 @@ define(function (require, exports, module) { | |||
| this.numMatches += resultInfo.matches.length; | |||
| if (this.numMatches >= SearchModel.MAX_TOTAL_RESULTS) { | |||
There was a problem hiding this comment.
@jacksonweekes I think we can simply change the logic to this.numMatches > SearchModel.MAX_TOTAL_RESULTS so that it matches to the error message we're showing.
peterflynn
changed the title
|
@RaymondLim That was my first thought(and I tried it) however it doesn't work for a couple of reasons. As I understand it(this is my first foray into Brackets and open source) FindInFiles.js passes the results to SearchModel.js, however FindInFiles only finds results up to MAX_TOTAL_RESULTS, which means that without the changes I have added to FindInFiles the condition this.numMatches > SearchModel.MAX_TOTAL_RESULTS never evaluates to true. With my changed FindInFiles it now returns up to MAX_TOTAL_RESULTS+1 which allows SearchModel to evaluate if there are additional results over MAX_TOTAL_RESULTS. |
|
@jacksonweekes Thanks for your detail explanation. |
| /** | ||
| * Clears out the model to an empty state. |
There was a problem hiding this comment.
Why are you removing the existing comment for clear function below?
|
@RaymondLim didn't realise I had done that, I have replaced that comment and fixed the file to conform to JSLint. |
| // Remove final result if there have been over MAX_TOTAL_RESULTS found | ||
| if (this.numMatches > SearchModel.MAX_TOTAL_RESULTS) { | ||
| this.results[fullpath].matches.pop(); | ||
| this.numMatches -= 1; |
There was a problem hiding this comment.
@jacksonweekes You can restore this line to this.numMatches--; since if you use Brackets folder (one level up from "src" folder) as your project root, then you won't get JSLint error on this line. We used to have /*jslint plusplus: true */ comment in most of the brackets source file, but files added lately no longer using it. Instead, .brackets.json file in Brackets folder has all the jslint options defined.
|
@RaymondLim I have made the changes requested |
|
Looks good. Merging. |
Issue #8557 ("Over 100000 matches" when there are exactly 100000 matches)
Solves issue raised in #8557. Code passes all unit tests, and correctly shows results found below, at or above SearchModel.MAX_TOTAL_RESULTS.