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
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
if (matches.length > SearchModel.MAX_TOTAL_RESULTS) {
queryExpr.lastIndex = 0;
break;
}
Expand Down
16 changes: 15 additions & 1 deletion src/search/SearchModel.js
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--;
this.exceedsMaximum = true;
}
}
};

Expand Down
10 changes: 5 additions & 5 deletions src/search/SearchResultsView.js
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