Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Issue #8557 ("Over 100000 matches" when there are exactly 100000 matches) #9385

Merged
merged 5 commits into from
Oct 8, 2014
Merged

Issue #8557 ("Over 100000 matches" when there are exactly 100000 matches) #9385

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c78541c
Issue #8557- fixed to show correct result count
jacksonweekes Oct 1, 2014
5eaaee8
Removed unused reference to SearchModel
jacksonweekes Oct 1, 2014
35f7280
Add comment back to clear() function
jacksonweekes Oct 3, 2014
d63a124
Changed -- to conform to JSLint
jacksonweekes Oct 3, 2014
0c83a92
Conform code to conventions
jacksonweekes Oct 8, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/search/FindInFiles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ define(function (require, exports, module) {

// We have the max hits in just this 1 file. Stop searching this file.
// This fixed issue #1829 where code hangs on too many hits.
if (matches.length >= SearchModel.MAX_TOTAL_RESULTS) {
// ** Adds one over MAX_TOTAL_RESULTS in order to know if the search has exceeded
// or is equal to MAX_TOTAL_RESULTS. Additional result removed in SearchModel **
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Would you mind to remove ** at the beginning and end of your comment? I know that our coding conventions for comments do not explicitly mention what can be and can't be in the comments. And I know you want to highlight the issue you want to solve in your comment, but it's better we don't have any comment with different forms of highlighting with extra *.

if (matches.length > SearchModel.MAX_TOTAL_RESULTS) {
queryExpr.lastIndex = 0;
break;
}
Expand Down
16 changes: 15 additions & 1 deletion src/search/SearchModel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,13 @@ define(function (require, exports, module) {
* @type {boolean}
*/
SearchModel.prototype.foundMaximum = false;


/**
* Whether or not we exceeded the maximum number of results in the search we did.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing the existing comment for clear function below?

* @type {boolean}
*/
SearchModel.prototype.exceedsMaximum = false;

/**
* Clears out the model to an empty state.
*/
Expand All @@ -112,6 +118,7 @@ define(function (require, exports, module) {
this.scope = null;
this.numMatches = 0;
this.foundMaximum = false;
this.exceedsMaximum = false;
this.fireChanged();
};

Expand Down Expand Up @@ -157,6 +164,13 @@ define(function (require, exports, module) {
this.numMatches += resultInfo.matches.length;
if (this.numMatches >= SearchModel.MAX_TOTAL_RESULTS) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

this.foundMaximum = true;

// 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

this.exceedsMaximum = true;
}
}
};

Expand Down
10 changes: 5 additions & 5 deletions src/search/SearchResultsView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
*/
define(function (require, exports, module) {
"use strict";

var CommandManager = require("command/CommandManager"),
Commands = require("command/Commands"),
DocumentManager = require("document/DocumentManager"),
Expand All @@ -41,12 +41,12 @@ define(function (require, exports, module) {
StringUtils = require("utils/StringUtils"),
Strings = require("strings"),
_ = require("thirdparty/lodash"),

searchPanelTemplate = require("text!htmlContent/search-panel.html"),
searchResultsTemplate = require("text!htmlContent/search-results.html"),
searchSummaryTemplate = require("text!htmlContent/search-summary.html");


/**
* @const
* The maximum results to show per page.
Expand Down Expand Up @@ -348,7 +348,7 @@ define(function (require, exports, module) {
// This text contains some formatting, so all the strings are assumed to be already escaped
summary = StringUtils.format(
Strings.FIND_TITLE_SUMMARY,
this._model.foundMaximum ? Strings.FIND_IN_FILES_MORE_THAN : "",
this._model.exceedsMaximum ? Strings.FIND_IN_FILES_MORE_THAN : "",
String(count.matches),
(count.matches > 1) ? Strings.FIND_IN_FILES_MATCHES : Strings.FIND_IN_FILES_MATCH,
filesStr
Expand Down