diff --git a/src/document/Document.js b/src/document/Document.js index c928a288639..1a48c53be81 100644 --- a/src/document/Document.js +++ b/src/document/Document.js @@ -275,6 +275,17 @@ define(function (require, exports, module) { // _handleEditorChange() triggers "change" event }; + /** + * @private + * Triggers the appropriate events when a change occurs: "change" on the Document instance + * and "documentChange" on the Document module. + * @param {Object} changeList Changelist in CodeMirror format + */ + Document.prototype._notifyDocumentChange = function (changeList) { + $(this).triggerHandler("change", [this, changeList]); + $(exports).triggerHandler("documentChange", [this, changeList]); + }; + /** * Sets the contents of the document. Treated as reloading the document from disk: the document * will be marked clean with a new timestamp, the undo/redo history is cleared, and we re-check @@ -303,9 +314,7 @@ define(function (require, exports, module) { // TODO: Dumb to split it here just to join it again in the change handler, but this is // the CodeMirror change format. Should we document our change format to allow this to // either be an array of lines or a single string? - var fakeChangeList = [{text: text.split(/\r?\n/)}]; - $(this).triggerHandler("change", [this, fakeChangeList]); - $(exports).triggerHandler("documentChange", [this, fakeChangeList]); + this._notifyDocumentChange([{text: text.split(/\r?\n/)}]); } } this._updateTimestamp(newTimestamp); @@ -398,6 +407,9 @@ define(function (require, exports, module) { * @private */ Document.prototype._handleEditorChange = function (event, editor, changeList) { + // TODO: This needs to be kept in sync with SpecRunnerUtils.createMockActiveDocument(). In the + // future, we should fix things so that we either don't need mock documents or that this + // is factored so it will just run in both. if (!this._refreshInProgress) { // Sync isDirty from CodeMirror state var wasDirty = this.isDirty; @@ -410,11 +422,7 @@ define(function (require, exports, module) { } // Notify that Document's text has changed - // TODO: This needs to be kept in sync with SpecRunnerUtils.createMockDocument(). In the - // future, we should fix things so that we either don't need mock documents or that this - // is factored so it will just run in both. - $(this).triggerHandler("change", [this, changeList]); - $(exports).triggerHandler("documentChange", [this, changeList]); + this._notifyDocumentChange(changeList); }; /** diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index f36fd35f4a4..dfc9312ac93 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -1174,7 +1174,7 @@ define(function (require, exports, module) { } else { // Multiple unsaved files: show a single bulk prompt listing all files - var message = Strings.SAVE_CLOSE_MULTI_MESSAGE + StringUtils.makeDialogFileList(_.map(unsavedDocs, _shortTitleForDocument)); + var message = Strings.SAVE_CLOSE_MULTI_MESSAGE + FileUtils.makeDialogFileList(_.map(unsavedDocs, _shortTitleForDocument)); Dialogs.showModalDialog( DefaultDialogs.DIALOG_ID_SAVE_CLOSE, @@ -1232,9 +1232,12 @@ define(function (require, exports, module) { /** * Closes all open documents; equivalent to calling handleFileClose() for each document, except * that unsaved changes are confirmed once, in bulk. - * @param {?{promptOnly: boolean}} If true, only displays the relevant confirmation UI and does NOT + * @param {?{promptOnly: boolean, _forceClose: boolean}} + * If promptOnly is true, only displays the relevant confirmation UI and does NOT * actually close any documents. This is useful when chaining close-all together with * other user prompts that may be cancelable. + * If _forceClose is true, forces the files to close with no confirmation even if dirty. + * Should only be used for unit test cleanup. * @return {$.Promise} a promise that is resolved when all files are closed */ function handleFileCloseAll(commandData) { diff --git a/src/file/FileUtils.js b/src/file/FileUtils.js index 76356c6c8ee..4e503326e2c 100644 --- a/src/file/FileUtils.js +++ b/src/file/FileUtils.js @@ -183,6 +183,21 @@ define(function (require, exports, module) { ); } + /** + * Creates an HTML string for a list of files to be reported on, suitable for use in a dialog. + * @param {Array.} Array of filenames or paths to display. + */ + function makeDialogFileList(paths) { + var result = ""; + return result; + } + /** * Convert a URI path to a native path. * On both platforms, this unescapes the URI @@ -446,11 +461,12 @@ define(function (require, exports, module) { } /** - * Compares two paths. Useful for sorting. - * @param {string} filename1 - * @param {string} filename2 - * @param {boolean} extFirst If true it compares the extensions first and then the file names. - * @return {number} The result of the local compare function + * Compares two paths segment-by-segment, used for sorting. Sorts folders before files, + * and sorts files based on `compareFilenames()`. + * @param {string} path1 + * @param {string} path2 + * @return {number} -1, 0, or 1 depending on whether path1 is less than, equal to, or greater than + * path2 according to this ordering. */ function comparePaths(path1, path2) { var entryName1, entryName2, @@ -486,6 +502,7 @@ define(function (require, exports, module) { exports.translateLineEndings = translateLineEndings; exports.showFileOpenError = showFileOpenError; exports.getFileErrorString = getFileErrorString; + exports.makeDialogFileList = makeDialogFileList; exports.readAsText = readAsText; exports.writeText = writeText; exports.convertToNativePath = convertToNativePath; diff --git a/src/htmlContent/findreplace-bar.html b/src/htmlContent/findreplace-bar.html index fcb7aaf716e..c7f1f3dc1b4 100644 --- a/src/htmlContent/findreplace-bar.html +++ b/src/htmlContent/findreplace-bar.html @@ -3,29 +3,30 @@ -->
{{#replace}}
+ {{/multifile}} + --> {{/replace}} -{{#scope}} +{{#multifile}}
{{Strings.FIND_NO_RESULTS}}
{{{scopeLabel}}}
+
-{{/scope}} +{{/multifile}} diff --git a/src/htmlContent/search-results.html b/src/htmlContent/search-results.html index 8f8126e633a..7c5a671b9cf 100644 --- a/src/htmlContent/search-results.html +++ b/src/htmlContent/search-results.html @@ -1,14 +1,14 @@ {{#searchList}} - + {{#items}} - + {{#replace}}{{/replace}} diff --git a/src/htmlContent/search-summary-paging.html b/src/htmlContent/search-summary-paging.html deleted file mode 100644 index 5d4a88d9f6d..00000000000 --- a/src/htmlContent/search-summary-paging.html +++ /dev/null @@ -1,9 +0,0 @@ -{{#hasPages}} -
- - - {{{results}}} - - -
-{{/hasPages}} diff --git a/src/htmlContent/search-summary.html b/src/htmlContent/search-summary.html index cd22ad3d897..3160d952a6f 100644 --- a/src/htmlContent/search-summary.html +++ b/src/htmlContent/search-summary.html @@ -13,7 +13,15 @@
{{{scope}}}
{{/scope}}
{{{summary}}}
-{{>paging}} +{{#hasPages}} +
+ + + {{{results}}} + + +
+{{/hasPages}} {{#replace}}
diff --git a/src/search/FindBar.js b/src/search/FindBar.js index 1ad77536b00..f50c4babe77 100644 --- a/src/search/FindBar.js +++ b/src/search/FindBar.js @@ -24,6 +24,9 @@ /*jslint vars: true, plusplus: true, devel: true, nomen: true, regexp: true, indent: 4, maxerr: 50 */ /*global define, $, window, Mustache */ +/* + * UI for the Find/Replace and Find in Files modal bar. + */ define(function (require, exports, module) { "use strict"; @@ -51,30 +54,26 @@ define(function (require, exports, module) { * * Dispatches these events: * - * queryChange - when the user types in the input field or sets a query option. Use getQuery() + * - queryChange - when the user types in the input field or sets a query option. Use getQueryInfo() * to get the current query state. - * doFind - when the user hits enter/shift-enter in an input field or clicks the Find Previous or Find Next button. + * - doFind - when the user chooses to do a Find Previous or Find Next. * Parameters are: - * shiftKey - boolean, false for Find Next, true for Find Previous. - * replace - boolean, true if they hit enter in the Replace field - * doReplace - when the user clicks on the Replace or Replace All button. Parameter is a boolean, - * false for single Replace, true for Replace All. Use getReplaceText() to get the current - * replacement text. - * close - when the find bar is closed + * shiftKey - boolean, false for Find Next, true for Find Previous + * - doReplace - when the user chooses to do a single replace. Use getReplaceText() to get the current replacement text. + * - doReplaceAll - when the user chooses to initiate a Replace All. Use getReplaceText() to get the current replacement text. + *- close - when the find bar is closed * - * @param {{navigator: boolean, replace: boolean, queryPlaceholder: string, initialQuery: string}} options - * Options for the Find bar. - * navigator - true to show the Find Previous/Find Next buttons - default false - * replace - true to show the Replace controls - default false - * replaceAllOnly - true to show only a Replace All button (no Replace button) - default false - * scope - true to show the scope filter controls - default false - * queryPlaceholder - label to show in the Find field - default empty string - * initialQuery - query to populate in the Find field on open - default empty string - * scopeLabel - label to show for the scope of the search - default empty string + * @param {boolean=} options.multifile - true if this is a Find/Replace in Files (changes the behavior of Enter in + * the fields, hides the navigator controls, shows the scope/filter controls, and if in replace mode, hides the + * Replace button (so there's only Replace All) + * @param {boolean=} options.replace - true to show the Replace controls - default false + * @param {string=} options.queryPlaceholder - label to show in the Find field - default empty string + * @param {string=} options.initialQuery - query to populate in the Find field on open - default empty string + * @param {string=} scopeLabel - HTML label to show for the scope of the search, expected to be already escaped - default empty string */ function FindBar(options) { var defaults = { - navigator: false, + multifile: false, replace: false, queryPlaceholder: "", initialQuery: "", @@ -97,6 +96,7 @@ define(function (require, exports, module) { * @private * Register a find bar so we can close it later if another one tries to open. * Note that this is a global function, not an instance function. + * @param {!FindBar} findBar The find bar to register. */ FindBar._addFindBar = function (findBar) { FindBar._bars = FindBar._bars || []; @@ -125,7 +125,7 @@ define(function (require, exports, module) { var bars = FindBar._bars; if (bars) { bars.forEach(function (bar) { - bar.close(true); + bar.close(true, false); }); bars = []; } @@ -138,7 +138,7 @@ define(function (require, exports, module) { /** * @private * Options passed into the FindBar. - * @type {?{navigator: boolean, replace: boolean, queryPlaceholder: string, initialQuery: string}} + * @type {!{multifile: boolean, replace: boolean, queryPlaceholder: string, initialQuery: string, scopeLabel: string}} */ FindBar.prototype._options = null; @@ -184,8 +184,10 @@ define(function (require, exports, module) { * Set the state of the toggles in the Find bar to the saved prefs state. */ FindBar.prototype._updateSearchBarFromPrefs = function () { - this.$("#find-case-sensitive").toggleClass("active", PreferencesManager.getViewState("caseSensitive")); - this.$("#find-regexp").toggleClass("active", PreferencesManager.getViewState("regexp")); + // Have to make sure we explicitly cast the second parameter to a boolean, because + // toggleClass expects literal true/false. + this.$("#find-case-sensitive").toggleClass("active", !!PreferencesManager.getViewState("caseSensitive")); + this.$("#find-regexp").toggleClass("active", !!PreferencesManager.getViewState("regexp")); }; /** @@ -227,7 +229,7 @@ define(function (require, exports, module) { var templateVars = _.clone(this._options); templateVars.Strings = Strings; - templateVars.replaceAllLabel = (templateVars.replaceAllOnly ? Strings.BUTTON_REPLACE_ALL_IN_FILES : Strings.BUTTON_REPLACE_ALL); + templateVars.replaceAllLabel = (templateVars.multifile ? Strings.BUTTON_REPLACE_ALL_IN_FILES : Strings.BUTTON_REPLACE_ALL); this._modalBar = new ModalBar(Mustache.render(_searchBarTemplate, templateVars), true); // 2nd arg = auto-close on Esc/blur @@ -258,11 +260,27 @@ define(function (require, exports, module) { if (e.keyCode === KeyEvent.DOM_VK_RETURN) { e.preventDefault(); e.stopPropagation(); - $(self).triggerHandler("doFind", [e.shiftKey, (e.target.id === "replace-with")]); + if (self._options.multifile) { + if ($(e.target).is("#find-what")) { + if (self._options.replace) { + // Just set focus to the Replace field. + self.focusReplace(); + } else { + // Trigger a Find (which really means "Find All" in this context). + $(self).triggerHandler("doFind"); + } + } else { + $(self).triggerHandler("doReplaceAll"); + } + } else { + // In the single file case, we just want to trigger a Find Next (or Find Previous + // if Shift is held down). + $(self).triggerHandler("doFind", [e.shiftKey]); + } } }); - if (this._options.navigator) { + if (!this._options.multifile) { this._addShortcutToTooltip($("#find-next"), Commands.CMD_FIND_NEXT); this._addShortcutToTooltip($("#find-prev"), Commands.CMD_FIND_PREVIOUS); $root @@ -278,10 +296,10 @@ define(function (require, exports, module) { this._addShortcutToTooltip($("#replace-yes"), Commands.CMD_REPLACE); $root .on("click", "#replace-yes", function (e) { - $(self).triggerHandler("doReplace", false); + $(self).triggerHandler("doReplace"); }) .on("click", "#replace-all", function (e) { - $(self).triggerHandler("doReplace", true); + $(self).triggerHandler("doReplaceAll"); }) // One-off hack to make Find/Replace fields a self-contained tab cycle // TODO: remove once https://trello.com/c/lTSJgOS2 implemented @@ -400,9 +418,7 @@ define(function (require, exports, module) { */ FindBar.prototype.enable = function (enable) { var self = this; - ["#find-what", "#replace-with", "#find-prev", "#find-next", "#find-case-sensitive", "#find-regexp"].forEach(function (selector) { - self.$(selector).prop("disabled", !enable); - }); + this.$("#find-what, #replace-with, #find-prev, #find-next, #find-case-sensitive, #find-regexp").prop("disabled", !enable); this._enabled = enable; }; @@ -413,6 +429,13 @@ define(function (require, exports, module) { return this._enabled; }; + /** + * @return {boolean} true if the Replace button is enabled. + */ + FindBar.prototype.isReplaceEnabled = function () { + return this.$("#replace-yes").is(":enabled"); + }; + /** * Enable or disable the navigation controls if present. Note that if the Find bar is currently disabled * (i.e. isEnabled() returns false), this will have no effect. diff --git a/src/search/FindInFiles.js b/src/search/FindInFiles.js index d3243bdebf0..00e1fb02df7 100644 --- a/src/search/FindInFiles.js +++ b/src/search/FindInFiles.js @@ -79,15 +79,15 @@ define(function (require, exports, module) { /** * @private - * Searches through the contents an returns an array of matches + * Searches through the contents and returns an array of matches * @param {string} contents * @param {RegExp} queryExpr - * @return {Array.<{start: {line:number,ch:number}, end: {line:number,ch:number}, line: string}>} + * @return {!Array.<{start: {line:number,ch:number}, end: {line:number,ch:number}, line: string}>} */ function _getSearchMatches(contents, queryExpr) { // Quick exit if not found or if we hit the limit if (searchModel.foundMaximum || contents.search(queryExpr) === -1) { - return null; + return []; } var match, lineNum, line, ch, matchLength, @@ -106,8 +106,16 @@ define(function (require, exports, module) { matches.push({ start: {line: lineNum, ch: ch}, end: {line: lineNum, ch: ch + matchLength}, + + // Note that the following offsets from the beginning of the file are *not* updated if the search + // results change. These are currently only used for multi-file replacement, and we always + // abort the replace (by shutting the results panel) if we detect any result changes, so we don't + // need to keep them up to date. Eventually, we should either get rid of the need for these (by + // doing everything in terms of line/ch offsets, though that will require re-splitting files when + // doing a replace) or properly update them. startOffset: match.index, endOffset: match.index + matchLength, + line: line, result: match, isChecked: true @@ -124,35 +132,21 @@ define(function (require, exports, module) { return matches; } - /** - * @private - * Searches and stores the match results for the given file, if there are matches - * @param {string} fullPath - * @param {string} contents - * @param {RegExp} queryExpr - * @return {boolean} True iff the matches were added to the search results - */ - function _addSearchMatches(fullPath, contents, queryExpr, timestamp) { - var matches = _getSearchMatches(contents, queryExpr); - - if (matches && matches.length) { - searchModel.addResultMatches(fullPath, matches, timestamp); - return true; - } - return false; - } - /** * @private * Update the search results using the given list of changes for the given document * @param {Document} doc The Document that changed, should be the current one - * @param {Array.<{from: {line:number,ch:number}, to: {line:number,ch:number}, text: string, next: change}>} changeList + * @param {Array.<{from: {line:number,ch:number}, to: {line:number,ch:number}, text: !Array.}>} changeList * An array of changes as described in the Document constructor */ function _updateResults(doc, changeList) { var i, diff, matches, lines, start, howMany, resultsChanged = false, - fullPath = doc.file.fullPath; + fullPath = doc.file.fullPath, + resultInfo = searchModel.results[fullPath]; + + // Remove the results before we make any changes, so the SearchModel can accurately update its count. + searchModel.removeResults(fullPath); changeList.forEach(function (change) { lines = []; @@ -162,7 +156,9 @@ define(function (require, exports, module) { // There is no from or to positions, so the entire file changed, we must search all over again if (!change.from || !change.to) { // TODO: add unit test exercising timestamp logic in this case - _addSearchMatches(fullPath, doc.getText(), searchModel.queryExpr, doc.diskTimestamp); + // We don't just call _updateSearchMatches() here because we want to continue iterating through changes in + // the list and update at the end. + resultInfo = {matches: _getSearchMatches(doc.getText(), searchModel.queryExpr), timestamp: doc.diskTimestamp}; resultsChanged = true; } else { @@ -171,17 +167,16 @@ define(function (require, exports, module) { lines.push(doc.getLine(change.from.line + i)); } - // We need to know how many lines changed to update the rest of the lines - if (change.from.line !== change.to.line) { - diff = change.from.line - change.to.line; - } else { - diff = lines.length - 1; - } + // We need to know how many newlines were inserted/deleted in order to update the rest of the line indices; + // this is the total number of newlines inserted (which is the length of the lines array minus + // 1, since the last line in the array is inserted without a newline after it) minus the + // number of original newlines being removed. + diff = lines.length - 1 - (change.to.line - change.from.line); - if (searchModel.results[fullPath]) { + if (resultInfo) { // Search the last match before a replacement, the amount of matches deleted and update // the lines values for all the matches after the change - searchModel.results[fullPath].matches.forEach(function (item) { + resultInfo.matches.forEach(function (item) { if (item.end.line < change.from.line) { start++; } else if (item.end.line <= change.to.line) { @@ -194,14 +189,14 @@ define(function (require, exports, module) { // Delete the lines that where deleted or replaced if (howMany > 0) { - searchModel.results[fullPath].matches.splice(start, howMany); + resultInfo.matches.splice(start, howMany); } resultsChanged = true; } // Searches only over the lines that changed matches = _getSearchMatches(lines.join("\r\n"), searchModel.queryExpr); - if (matches && matches.length) { + if (matches.length) { // Updates the line numbers, since we only searched part of the file matches.forEach(function (value, key) { matches[key].start.line += change.from.line; @@ -209,12 +204,12 @@ define(function (require, exports, module) { }); // If the file index exists, add the new matches to the file at the start index found before - if (searchModel.results[fullPath]) { - Array.prototype.splice.apply(searchModel.results[fullPath].matches, [start, 0].concat(matches)); + if (resultInfo) { + Array.prototype.splice.apply(resultInfo.matches, [start, 0].concat(matches)); // If not, add the matches to a new file index } else { // TODO: add unit test exercising timestamp logic in self case - searchModel.results[fullPath] = { + resultInfo = { matches: matches, collapsed: false, timestamp: doc.diskTimestamp @@ -222,17 +217,18 @@ define(function (require, exports, module) { } resultsChanged = true; } - - // All the matches where deleted, remove the file from the results - if (searchModel.results[fullPath] && !searchModel.results[fullPath].matches.length) { - delete searchModel.results[fullPath]; - resultsChanged = true; - } } }); + // Always re-add the results, even if nothing changed. + if (resultInfo && resultInfo.matches.length) { + searchModel.setResults(fullPath, resultInfo); + } + if (resultsChanged) { - searchModel.fireChanged(); + // Pass `true` for quickChange here. This will make listeners debounce the change event, + // avoiding lots of updates if the user types quickly. + searchModel.fireChanged(true); } } @@ -270,6 +266,8 @@ define(function (require, exports, module) { /** * Finds all candidate files to search in the given scope's subtree that are not binary content. Does NOT apply * the current filter yet. + * @param {?FileSystemEntry} scope Search scope, or null if whole project + * @return {$.Promise} A promise that will be resolved with the list of files in the scope. Never rejected. */ function getCandidateFiles(scope) { function filter(file) { @@ -278,8 +276,7 @@ define(function (require, exports, module) { // If the scope is a single file, just check if the file passes the filter directly rather than // trying to use ProjectManager.getAllFiles(), both for performance and because an individual - // in-memory file might be an untitled document or external file that doesn't show up in - // getAllFiles(). + // in-memory file might be an untitled document that doesn't show up in getAllFiles(). if (scope && scope.isFile) { return new $.Deferred().resolve(filter(scope) ? [scope] : []).promise(); } else { @@ -328,8 +325,8 @@ define(function (require, exports, module) { * Tries to update the search result on document changes * @param {$.Event} event * @param {Document} document - * @param {{from: {line:number,ch:number}, to: {line:number,ch:number}, text: string, next: change}} change - * A linked list as described in the Document constructor + * @param {<{from: {line:number,ch:number}, to: {line:number,ch:number}, text: !Array.}>} change + * A change list as described in the Document constructor */ _documentChangeHandler = function (event, document, change) { if (_inSearchScope(document.file)) { @@ -356,13 +353,14 @@ define(function (require, exports, module) { .done(function (text, timestamp) { // Note that we don't fire a model change here, since this is always called by some outer batch // operation that will fire it once it's done. - var foundMatches = _addSearchMatches(file.fullPath, text, searchModel.queryExpr, timestamp); - result.resolve(foundMatches); + var matches = _getSearchMatches(text, searchModel.queryExpr); + searchModel.setResults(file.fullPath, {matches: matches, timestamp: timestamp}); + result.resolve(!!matches.length); }) .fail(function () { // Always resolve. If there is an error, this file // is skipped and we move on to the next file. - result.resolve(); + result.resolve(false); }); return result.promise(); @@ -437,7 +435,8 @@ define(function (require, exports, module) { * @param {{query: string, caseSensitive: boolean, isRegexp: boolean}} queryInfo Query info object * @param {?Entry} scope Project file/subfolder to search within; else searches whole project. * @param {?string} filter A "compiled" filter as returned by FileFilters.compile(), or null for no filter - * @param {?string} replaceText If this is a replacement, the text to replace matches with. + * @param {?string} replaceText If this is a replacement, the text to replace matches with. This is just + * stored in the model for later use - the replacement is not actually performed right now. * @param {?$.Promise} candidateFilesPromise If specified, a promise that should resolve with the same set of files that * getCandidateFiles(scope) would return. * @return {$.Promise} A promise that's resolved with the search results or rejected when the find competes. @@ -487,14 +486,13 @@ define(function (require, exports, module) { // Update the search results _.forEach(searchModel.results, function (item, fullPath) { - if (fullPath.match(oldName)) { - searchModel.results[fullPath.replace(oldName, newName)] = item; - delete searchModel.results[fullPath]; + if (fullPath.indexOf(oldName) === 0) { + searchModel.removeResults(fullPath); + searchModel.setResults(fullPath.replace(oldName, newName), item); resultsChanged = true; } }); - // Restore the results if needed if (resultsChanged) { searchModel.fireChanged(); } @@ -518,7 +516,7 @@ define(function (require, exports, module) { function _removeSearchResultsForEntry(entry) { Object.keys(searchModel.results).forEach(function (fullPath) { if (fullPath.indexOf(entry.fullPath) === 0) { - delete searchModel.results[fullPath]; + searchModel.removeResults(fullPath); resultsChanged = true; } }); @@ -607,4 +605,9 @@ define(function (require, exports, module) { exports.getCandidateFiles = getCandidateFiles; exports.clearSearch = clearSearch; exports.ZERO_FILES_TO_SEARCH = ZERO_FILES_TO_SEARCH; + + // For unit tests only + exports._documentChangeHandler = _documentChangeHandler; + exports._fileNameChangeHandler = _fileNameChangeHandler; + exports._fileSystemChangeHandler = _fileSystemChangeHandler; }); diff --git a/src/search/FindInFilesUI.js b/src/search/FindInFilesUI.js index 4054d5d3a89..16c3ef05cb9 100644 --- a/src/search/FindInFilesUI.js +++ b/src/search/FindInFilesUI.js @@ -25,13 +25,9 @@ /*global define, $, window, Mustache */ /* - * Adds a "find in files" command to allow the user to find all occurrences of a string in all files in - * the project. - * - * The keyboard shortcut is Cmd(Ctrl)-Shift-F. + * UI and controller logic for find/replace across multiple files within the project. * * FUTURE: - * - Search files in working set that are *not* in the project * - Handle matches that span multiple lines */ define(function (require, exports, module) { @@ -44,6 +40,7 @@ define(function (require, exports, module) { DefaultDialogs = require("widgets/DefaultDialogs"), EditorManager = require("editor/EditorManager"), FileFilters = require("search/FileFilters"), + FileUtils = require("file/FileUtils"), FindBar = require("search/FindBar").FindBar, FindInFiles = require("search/FindInFiles"), FindUtils = require("search/FindUtils"), @@ -76,7 +73,7 @@ define(function (require, exports, module) { * @return {$.Promise} A promise that's resolved with the search results or rejected when the find competes. */ function searchAndShowResults(queryInfo, scope, filter, replaceText, candidateFilesPromise) { - FindInFiles.doSearchInScope(queryInfo, scope, filter, replaceText, candidateFilesPromise) + return FindInFiles.doSearchInScope(queryInfo, scope, filter, replaceText, candidateFilesPromise) .done(function (zeroFilesToken) { // Done searching all files: show results if (FindInFiles.searchModel.hasResults()) { @@ -150,10 +147,8 @@ define(function (require, exports, module) { } _findBar = new FindBar({ - navigator: false, + multifile: true, replace: showReplace, - replaceAllOnly: showReplace, - scope: true, initialQuery: initialString, queryPlaceholder: (showReplace ? Strings.CMD_REPLACE_IN_SUBTREE : Strings.CMD_FIND_IN_SUBTREE), scopeLabel: FindUtils.labelForScope(scope) @@ -175,25 +170,23 @@ define(function (require, exports, module) { function handleQueryChange() { // Check the query expression on every input event. This way the user is alerted // to any RegEx syntax errors immediately. - if (_findBar) { - var queryInfo = _findBar.getQueryInfo(), - queryResult = FindInFiles.searchModel.setQueryInfo(queryInfo); + var queryInfo = _findBar.getQueryInfo(), + queryResult = FindInFiles.searchModel.setQueryInfo(queryInfo); - // Enable the replace button appropriately. - _findBar.enableReplace(queryResult.valid); - - if (queryResult.valid || queryResult.empty) { - _findBar.showNoResults(false); - _findBar.showError(null); - } else { - _findBar.showNoResults(true, false); - _findBar.showError(queryResult.error); - } + // Enable the replace button appropriately. + _findBar.enableReplace(queryResult.valid); + + if (queryResult.valid || queryResult.empty) { + _findBar.showNoResults(false); + _findBar.showError(null); + } else { + _findBar.showNoResults(true, false); + _findBar.showError(queryResult.error); } } function startSearch(replaceText) { - var queryInfo = _findBar && _findBar.getQueryInfo(); + var queryInfo = _findBar.getQueryInfo(); if (queryInfo && queryInfo.query) { _findBar.enable(false); StatusBar.showBusyIndicator(true); @@ -215,15 +208,10 @@ define(function (require, exports, module) { } $(_findBar) - .on("doFind.FindInFiles", function (e, shiftKey, replace) { - // If in Replace mode, just set focus to the Replace field. - if (replace) { - startReplace(); - } else if (showReplace) { - _findBar.focusReplace(); - } else { - startSearch(); - } + .on("doFind.FindInFiles", function () { + // Subtle issue: we can't just pass startSearch directly as the handler, because + // we don't want it to get the event object as an argument. + startSearch(); }) .on("queryChange.FindInFiles", handleQueryChange) .on("close.FindInFiles", function (e) { @@ -232,9 +220,9 @@ define(function (require, exports, module) { }); if (showReplace) { - $(_findBar).on("doReplace.FindInFiles", function (e, all) { - startReplace(); - }); + // We shouldn't get a "doReplace" in this case, since the Replace button + // is hidden when we set options.multifile. + $(_findBar).on("doReplaceAll.FindInFiles", startReplace); } // Show file-exclusion UI *unless* search scope is just a single file @@ -265,7 +253,7 @@ define(function (require, exports, module) { // Clone the search results so that they don't get updated in the middle of the replacement. var resultsClone = _.cloneDeep(model.results), - replacedFiles = _.filter(Object.keys(resultsClone), function (path) { + replacedFiles = Object.keys(resultsClone).filter(function (path) { return FindUtils.hasCheckedMatches(resultsClone[path]); }), isRegexp = model.queryInfo.isRegexp, @@ -275,8 +263,8 @@ define(function (require, exports, module) { StatusBar.showBusyIndicator(true); FindInFiles.doReplace(resultsClone, replaceText, { forceFilesOpen: forceFilesOpen, isRegexp: isRegexp }) .fail(function (errors) { - var message = Strings.REPLACE_IN_FILES_ERRORS + StringUtils.makeDialogFileList( - _.map(errors, function (errorInfo) { + var message = Strings.REPLACE_IN_FILES_ERRORS + FileUtils.makeDialogFileList( + errors.map(function (errorInfo) { return ProjectManager.makeProjectRelativeIfPossible(errorInfo.item); }) ); @@ -372,7 +360,7 @@ define(function (require, exports, module) { var model = FindInFiles.searchModel; _resultsView = new SearchResultsView(model, "find-in-files-results", "find-in-files.results"); $(_resultsView) - .on("doReplaceAll", function () { + .on("replaceAll", function () { _finishReplaceAll(model); }) .on("close", function () { diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js index 529c5579ed9..9426e741026 100644 --- a/src/search/FindReplace.js +++ b/src/search/FindReplace.js @@ -66,12 +66,6 @@ define(function (require, exports, module) { */ var FIND_HIGHLIGHT_MAX = 2000; - /** - * Maximum number of matches to collect for Replace All; any additional matches are not listed in the panel & are not replaced - * @const {number} - */ - var REPLACE_ALL_MAX = 10000; - /** * Instance of the currently opened document when replaceAllPanel is visible * @type {?Document} @@ -86,6 +80,7 @@ define(function (require, exports, module) { function SearchState() { this.searchStartPos = null; + this.parsedQuery = null; this.queryInfo = null; this.foundAny = false; this.marked = []; @@ -98,9 +93,9 @@ define(function (require, exports, module) { return cm._searchState; } - function getSearchCursor(cm, query, queryInfo, pos) { + function getSearchCursor(cm, state, pos) { // Heuristic: if the query string is all lowercase, do a case insensitive search. - return cm.getSearchCursor(query, pos, !queryInfo.isCaseSensitive); + return cm.getSearchCursor(state.parsedQuery, pos, !state.queryInfo.isCaseSensitive); } function parseQuery(queryInfo) { @@ -131,16 +126,16 @@ define(function (require, exports, module) { /** * @private * Determine the query from the given info and store it in the state. - * @param {SearchState} state The state to store the query in + * @param {SearchState} state The state to store the parsed query in * @param {{query: string, caseSensitive: boolean, isRegexp: boolean}} queryInfo * The query info object as returned by FindBar.getQueryInfo() */ function setQueryInfo(state, queryInfo) { state.queryInfo = queryInfo; if (!queryInfo) { - state.query = null; + state.parsedQuery = null; } else { - state.query = parseQuery(queryInfo); + state.parsedQuery = parseQuery(queryInfo); } } @@ -160,12 +155,12 @@ define(function (require, exports, module) { function _getNextMatch(editor, rev, pos, wrap) { var cm = editor._codeMirror; var state = getSearchState(cm); - var cursor = getSearchCursor(cm, state.query, state.queryInfo, pos || editor.getCursorPos(false, rev ? "start" : "end")); + var cursor = getSearchCursor(cm, state, pos || editor.getCursorPos(false, rev ? "start" : "end")); state.lastMatch = cursor.find(rev); if (!state.lastMatch && wrap !== false) { // If no result found before hitting edge of file, try wrapping around - cursor = getSearchCursor(cm, state.query, state.queryInfo, rev ? {line: cm.lineCount() - 1} : {line: 0, ch: 0}); + cursor = getSearchCursor(cm, state, rev ? {line: cm.lineCount() - 1} : {line: 0, ch: 0}); state.lastMatch = cursor.find(rev); } if (!state.lastMatch) { @@ -410,7 +405,7 @@ define(function (require, exports, module) { function clearSearch(cm) { cm.operation(function () { var state = getSearchState(cm); - if (!state.query || !state.queryInfo) { + if (!state.parsedQuery) { return; } setQueryInfo(state, null); @@ -439,14 +434,12 @@ define(function (require, exports, module) { state = getSearchState(cm); function indicateHasMatches(numResults) { - if (findBar) { - // Make the field red if it's not blank and it has no matches (which also covers invalid regexes) - findBar.showNoResults(!state.foundAny && findBar.getQueryInfo().query); + // Make the field red if it's not blank and it has no matches (which also covers invalid regexes) + findBar.showNoResults(!state.foundAny && findBar.getQueryInfo().query); - // Navigation buttons enabled if we have a query and more than one match - findBar.enableNavigation(state.foundAny && numResults > 1); - findBar.enableReplace(state.foundAny); - } + // Navigation buttons enabled if we have a query and more than one match + findBar.enableNavigation(state.foundAny && numResults > 1); + findBar.enableReplace(state.foundAny); } cm.operation(function () { @@ -455,11 +448,9 @@ define(function (require, exports, module) { clearHighlights(cm, state); } - if (!state.query || !state.queryInfo) { + if (!state.parsedQuery) { // Search field is empty - no results - if (findBar) { - findBar.showFindCount(""); - } + findBar.showFindCount(""); state.foundAny = false; indicateHasMatches(); return; @@ -467,7 +458,7 @@ define(function (require, exports, module) { // Find *all* matches, searching from start of document // (Except on huge documents, where this is too expensive) - var cursor = getSearchCursor(cm, state.query, state.queryInfo); + var cursor = getSearchCursor(cm, state); if (cm.getValue().length <= FIND_MAX_FILE_SIZE) { // FUTURE: if last query was prefix of this one, could optimize by filtering last result set var resultSet = []; @@ -490,25 +481,21 @@ define(function (require, exports, module) { ScrollTrackMarkers.addTickmarks(editor, scrollTrackPositions); } - if (findBar) { - var countInfo; - if (resultSet.length === 0) { - countInfo = Strings.FIND_NO_RESULTS; - } else if (resultSet.length === 1) { - countInfo = Strings.FIND_RESULT_COUNT_SINGLE; - } else { - countInfo = StringUtils.format(Strings.FIND_RESULT_COUNT, resultSet.length); - } - findBar.showFindCount(countInfo); + var countInfo; + if (resultSet.length === 0) { + countInfo = Strings.FIND_NO_RESULTS; + } else if (resultSet.length === 1) { + countInfo = Strings.FIND_RESULT_COUNT_SINGLE; + } else { + countInfo = StringUtils.format(Strings.FIND_RESULT_COUNT, resultSet.length); } + findBar.showFindCount(countInfo); state.foundAny = (resultSet.length > 0); indicateHasMatches(resultSet.length); } else { // On huge documents, just look for first match & then stop - if (findBar) { - findBar.showFindCount(""); - } + findBar.showFindCount(""); state.foundAny = cursor.findNext(); indicateHasMatches(); } @@ -516,7 +503,7 @@ define(function (require, exports, module) { } /** - * Called each time the search query field changes. Updates state.query (query will be falsy if the field + * Called each time the search query field changes. Updates state.parsedQuery (parsedQuery will be falsy if the field * was blank OR contained a regexp with invalid syntax). Then calls updateResultSet(), and then jumps to * the first matching result, starting from the original cursor position. * @param {!Editor} editor The editor we're searching in. @@ -525,10 +512,10 @@ define(function (require, exports, module) { * In that case, we don't want to change the selection unnecessarily. */ function handleQueryChange(editor, state, initial) { - setQueryInfo(state, findBar && findBar.getQueryInfo()); + setQueryInfo(state, findBar.getQueryInfo()); updateResultSet(editor); - if (state.query && state.queryInfo) { + if (state.parsedQuery) { // 3rd arg: prefer to avoid scrolling if result is anywhere within view, since in this case user // is in the middle of typing, not navigating explicitly; viewport jumping would be distracting. findNext(editor, false, true, state.searchStartPos); @@ -579,7 +566,7 @@ define(function (require, exports, module) { // Create the search bar UI (closing any previous find bar in the process) findBar = new FindBar({ - navigator: true, + multifile: false, replace: replace, initialQuery: initialQuery, queryPlaceholder: Strings.CMD_FIND_FIELD_PLACEHOLDER @@ -613,7 +600,7 @@ define(function (require, exports, module) { */ function doSearch(editor, rev) { var state = getSearchState(editor._codeMirror); - if (state.query && state.queryInfo) { + if (state.parsedQuery) { findNext(editor, rev); return; } @@ -643,7 +630,7 @@ define(function (require, exports, module) { // Delegate to Replace in Files. FindInFilesUI.searchAndShowResults(state.queryInfo, editor.document.file, null, replaceText); } else { - cm.replaceSelection(typeof state.query === "string" ? replaceText : FindUtils.parseDollars(replaceText, state.lastMatch)); + cm.replaceSelection(state.queryInfo.isRegexp ? FindUtils.parseDollars(replaceText, state.lastMatch) : replaceText); updateResultSet(editor); // we updated the text, so result count & tickmarks must be refreshed @@ -657,16 +644,20 @@ define(function (require, exports, module) { function replace(editor) { // If Replace bar already open, treat the shortcut as a hotkey for the Replace button - if (findBar && findBar.getOptions().replace) { + if (findBar && findBar.getOptions().replace && findBar.isReplaceEnabled()) { doReplace(editor, false); return; } openSearchBar(editor, true); - $(findBar).on("doReplace.FindReplace", function (e, all) { - doReplace(editor, all); - }); + $(findBar) + .on("doReplace.FindReplace", function (e) { + doReplace(editor, false); + }) + .on("doReplaceAll.FindReplace", function (e) { + doReplace(editor, true); + }); } function _launchFind() { diff --git a/src/search/FindUtils.js b/src/search/FindUtils.js index 2f2431bef34..fa9872477d9 100644 --- a/src/search/FindUtils.js +++ b/src/search/FindUtils.js @@ -73,8 +73,11 @@ define(function (require, exports, module) { function _doReplaceInDocument(doc, matchInfo, replaceText, isRegexp) { // Double-check that the open document's timestamp matches the one we recorded. This // should normally never go out of sync, because if it did we wouldn't start the - // replace in the first place, but we want to double-check. This will *not* handle - // cases where the document has been edited in memory since the matchInfo was generated. + // replace in the first place (due to the fact that we immediately close the search + // results panel whenever we detect a filesystem change that affects the results), + // but we want to double-check in case we don't happen to get the change in time. + // This will *not* handle cases where the document has been edited in memory since + // the matchInfo was generated. if (doc.diskTimestamp.getTime() !== matchInfo.timestamp.getTime()) { return new $.Deferred().reject(exports.ERROR_FILE_CHANGED).promise(); } @@ -171,6 +174,8 @@ define(function (require, exports, module) { * Checks timestamps to ensure replacements are not performed in files that have changed on disk since * the original search results were generated. However, does *not* check whether edits have been performed * in in-memory documents since the search; it's up to the caller to guarantee this hasn't happened. + * (When called from the standard Find in Files UI, SearchResultsView guarantees this. If called headlessly, + * the caller needs to track changes.) * * Replacements in documents that are already open in memory at the start of the replacement are guaranteed to * happen synchronously; replacements in files on disk will return an error if the on-disk file changes between @@ -211,6 +216,7 @@ define(function (require, exports, module) { if (firstPath) { var newDoc = DocumentManager.getOpenDocumentForPath(firstPath); + // newDoc might be null if the replacement failed. if (newDoc) { DocumentManager.setCurrentDocument(newDoc); } diff --git a/src/search/SearchModel.js b/src/search/SearchModel.js index e9aa42d6687..ae32fe9d879 100644 --- a/src/search/SearchModel.js +++ b/src/search/SearchModel.js @@ -79,7 +79,7 @@ define(function (require, exports, module) { /** * The file/folder path representing the scope that this query was performed in. - * @type {string} + * @type {FileSystemEntry} */ SearchModel.prototype.scope = null; @@ -89,6 +89,12 @@ define(function (require, exports, module) { */ SearchModel.prototype.filter = null; + /** + * The total number of matches in the model. + * @type {number} + */ + SearchModel.prototype.numMatches = 0; + /** * Whether or not we hit the maximum number of results for the type of search we did. * @type {boolean} @@ -105,6 +111,7 @@ define(function (require, exports, module) { this.isReplace = false; this.replaceText = null; this.scope = null; + this.numMatches = 0; this.foundMaximum = false; }; @@ -121,8 +128,8 @@ define(function (require, exports, module) { this.queryInfo = queryInfo; this.queryExpr = null; - // TODO: only apparent difference between this one and the one in FindReplace is that this one returns - // null instead of "" for a bad query, and this always returns a regexp even for simple strings. Reconcile. + // TODO: only major difference between this one and the one in FindReplace is that + // this always returns a regexp even for simple strings. Reconcile. if (!queryInfo || !queryInfo.query) { return {empty: true}; } @@ -150,32 +157,42 @@ define(function (require, exports, module) { }; /** - * Adds the given result matches to the search results - * @param {string} fullpath - * @param {Array.} matches - * @return true if at least some matches were added, false if we've hit the limit on how many can be added - */ - SearchModel.prototype.addResultMatches = function (fullpath, matches, timestamp) { - if (this.foundMaximum) { - return false; - } + * Sets the list of matches for the given path, removing the previous match info, if any, and updating + * the total match count. Note that for the count to remain accurate, the previous match info must not have + * been mutated since it was set. + * @param {string} fullpath Full path to the file containing the matches. + * @param {!{matches: Object, timestamp: Date, collapsed: boolean=}} resultInfo Info for the matches to set: + * matches - Array of matches, in the format returned by FindInFiles._getSearchMatches() + * timestamp - The timestamp of the document at the time we searched it. + * collapsed - Optional: whether the results should be collapsed in the UI (default false). + */ + SearchModel.prototype.setResults = function (fullpath, resultInfo) { + this.removeResults(fullpath); - this.results[fullpath] = { - matches: matches, - collapsed: false, - timestamp: timestamp - }; + if (this.foundMaximum || !resultInfo.matches.length) { + return; + } - var curNumMatches = this.countFilesMatches().matches; - if (curNumMatches >= SearchModel.MAX_TOTAL_RESULTS) { + this.results[fullpath] = resultInfo; + this.numMatches += resultInfo.matches.length; + if (this.numMatches >= SearchModel.MAX_TOTAL_RESULTS) { this.foundMaximum = true; } - - return true; }; - + /** - * @return true if there are any results in this model. + * Removes the given result's matches from the search results and updates the total match count. + * @param {string} fullpath Full path to the file containing the matches. + */ + SearchModel.prototype.removeResults = function (fullpath) { + if (this.results[fullpath]) { + this.numMatches -= this.results[fullpath].matches.length; + delete this.results[fullpath]; + } + }; + + /** + * @return {boolean} true if there are any results in this model. */ SearchModel.prototype.hasResults = function () { return Object.keys(this.results).length > 0; @@ -186,13 +203,7 @@ define(function (require, exports, module) { * @return {{files: number, matches: number}} */ SearchModel.prototype.countFilesMatches = function () { - var numFiles = 0, numMatches = 0; - _.forEach(this.results, function (item) { - numFiles++; - numMatches += item.matches.length; - }); - - return {files: numFiles, matches: numMatches}; + return {files: Object.keys(this.results).length, matches: this.numMatches}; }; /** @@ -201,10 +212,7 @@ define(function (require, exports, module) { * @return {Array.} */ SearchModel.prototype.getSortedFiles = function (firstFile) { - var searchFiles = Object.keys(this.results), - self = this; - - searchFiles.sort(function (key1, key2) { + return Object.keys(this.results).sort(function (key1, key2) { if (firstFile === key1) { return -1; } else if (firstFile === key2) { @@ -212,8 +220,6 @@ define(function (require, exports, module) { } return FileUtils.comparePaths(key1, key2); }); - - return searchFiles; }; /** diff --git a/src/search/SearchResultsView.js b/src/search/SearchResultsView.js index e370a0fb93f..75de81f266f 100644 --- a/src/search/SearchResultsView.js +++ b/src/search/SearchResultsView.js @@ -23,6 +23,9 @@ /*global define, $, window, Mustache */ +/* + * Panel showing search results for a Find/Replace in Files operation. + */ define(function (require, exports, module) { "use strict"; @@ -41,13 +44,22 @@ define(function (require, exports, module) { searchPanelTemplate = require("text!htmlContent/search-panel.html"), searchResultsTemplate = require("text!htmlContent/search-results.html"), - searchPagingTemplate = require("text!htmlContent/search-summary-paging.html"), searchSummaryTemplate = require("text!htmlContent/search-summary.html"); - /** @const Constants used to define the maximum results show per page and found in a single file */ - var RESULTS_PER_PAGE = 100, - UPDATE_TIMEOUT = 400; + /** + * @const + * The maximum results to show per page. + * @type {number} + */ + var RESULTS_PER_PAGE = 100; + + /** + * @const + * Debounce time for document changes updating the search results view. + * @type {number} + */ + var UPDATE_TIMEOUT = 400; /** * @constructor @@ -74,7 +86,7 @@ define(function (require, exports, module) { /** * Array with content used in the Results Panel - * @type {Array.<{file: number, filename: string, fullPath: string, items: Array.}>} + * @type {Array.<{fileIndex: number, filename: string, fullPath: string, items: Array.}>} */ SearchResultsView.prototype._searchList = []; @@ -163,7 +175,7 @@ define(function (require, exports, module) { // Add the file to the working set on double click .on("dblclick.searchResults", ".table-container tr:not(.file-section)", function (e) { - var item = self._searchList[$(this).data("file")]; + var item = self._searchList[$(this).data("file-index")]; FileViewController.addToWorkingSetAndSelect(item.fullPath); }) @@ -178,7 +190,7 @@ define(function (require, exports, module) { $row.addClass("selected"); self._$selectedRow = $row; - var searchItem = self._searchList[$row.data("file")], + var searchItem = self._searchList[$row.data("file-index")], fullPath = searchItem.fullPath; // This is a file title row, expand/collapse on click @@ -194,7 +206,7 @@ define(function (require, exports, module) { } $titleRows.each(function () { - fullPath = self._searchList[$(this).data("file")].fullPath; + fullPath = self._searchList[$(this).data("file-index")].fullPath; searchItem = self._model.results[fullPath]; if (searchItem.collapsed !== collapsed) { @@ -214,7 +226,7 @@ define(function (require, exports, module) { // This is a file row, show the result on click } else { // Grab the required item data - var item = searchItem.items[$row.data("item")]; + var item = searchItem.items[$row.data("item-index")]; CommandManager.execute(Commands.FILE_OPEN, {fullPath: fullPath}) .done(function (doc) { @@ -241,8 +253,8 @@ define(function (require, exports, module) { }) .on("click.searchResults", ".check-one", function (e) { var $row = $(e.target).closest("tr"), - item = self._searchList[$row.data("file")], - match = self._model.results[item.fullPath].matches[$row.data("index")], + item = self._searchList[$row.data("file-index")], + match = self._model.results[item.fullPath].matches[$row.data("match-index")], $checkAll = self._panel.$panel.find(".check-all"); match.isChecked = $(this).is(":checked"); @@ -252,7 +264,7 @@ define(function (require, exports, module) { e.stopPropagation(); }) .on("click.searchResults", ".replace-checked", function (e) { - $(self).triggerHandler("doReplaceAll"); + $(self).triggerHandler("replaceAll"); }); } }; @@ -285,8 +297,8 @@ define(function (require, exports, module) { ); this._$summary.html(Mustache.render(searchSummaryTemplate, { - query: _.escape((this._model.queryInfo.query && this._model.queryInfo.query.toString()) || ""), - replaceWith: _.escape(this._model.replaceText), + query: (this._model.queryInfo.query && this._model.queryInfo.query.toString()) || "", + replaceWith: this._model.replaceText, titleLabel: this._model.isReplace ? Strings.FIND_REPLACE_TITLE_LABEL : Strings.FIND_TITLE_LABEL, scope: this._model.scope ? " " + FindUtils.labelForScope(this._model.scope) + " " : "", summary: summary, @@ -297,7 +309,7 @@ define(function (require, exports, module) { hasNext: lastIndex < count.matches, replace: this._model.isReplace, Strings: Strings - }, { paging: searchPagingTemplate })); + })); }; /** @@ -355,16 +367,16 @@ define(function (require, exports, module) { multiLine = match.start.line !== match.end.line; searchItems.push({ - file: self._searchList.length, - item: searchItems.length, - index: i, - line: match.start.line + 1, - pre: match.line.substr(0, match.start.ch), - highlight: match.line.substring(match.start.ch, multiLine ? undefined : match.end.ch), - post: multiLine ? "\u2026" : match.line.substr(match.end.ch), - start: match.start, - end: match.end, - isChecked: match.isChecked + fileIndex: self._searchList.length, + itemIndex: searchItems.length, + matchIndex: i, + line: match.start.line + 1, + pre: match.line.substr(0, match.start.ch), + highlight: match.line.substring(match.start.ch, multiLine ? undefined : match.end.ch), + post: multiLine ? "\u2026" : match.line.substr(match.end.ch), + start: match.start, + end: match.end, + isChecked: match.isChecked }); matchesCounter++; i++; @@ -381,10 +393,10 @@ define(function (require, exports, module) { ); self._searchList.push({ - file: self._searchList.length, - filename: displayFileName, - fullPath: fullPath, - items: searchItems + fileIndex: self._searchList.length, + filename: displayFileName, + fullPath: fullPath, + items: searchItems }); } }); @@ -400,7 +412,7 @@ define(function (require, exports, module) { })) // Restore the collapsed files .find(".file-section").each(function () { - var fullPath = self._searchList[$(this).data("file")].fullPath; + var fullPath = self._searchList[$(this).data("file-index")].fullPath; if (self._model.results[fullPath].collapsed) { self._model.results[fullPath].collapsed = false; @@ -421,6 +433,8 @@ define(function (require, exports, module) { * Updates the results view after a model change, preserving scroll position and selection. */ SearchResultsView.prototype._updateResults = function () { + // In general this shouldn't get called if the panel is closed, but in case some + // asynchronous process kicks this (e.g. a debounced model change), we double-check. if (this._panel.isVisible()) { var scrollTop = this._$table.scrollTop(), index = this._$selectedRow ? this._$selectedRow.index() : null, @@ -442,7 +456,7 @@ define(function (require, exports, module) { /** * @private - * Returns the last result index displayed + * Returns one past the last result index displayed for the current page. * @param {number} numMatches * @return {number} */ diff --git a/src/utils/Async.js b/src/utils/Async.js index 19d69a755cd..b90dd3a6e2c 100644 --- a/src/utils/Async.js +++ b/src/utils/Async.js @@ -422,8 +422,9 @@ define(function (require, exports, module) { } /** - * Utility for converting a method that takes an errback to one that returns a promise; useful - * for using FileSystem methods in a promise-oriented workflow. For example, instead of + * Utility for converting a method that takes (error, callback) to one that returns a promise; + * useful for using FileSystem methods (or other Node-style API methods) in a promise-oriented + * workflow. For example, instead of * * var deferred = new $.Deferred(); * file.read(function (err, contents) { diff --git a/src/utils/StringUtils.js b/src/utils/StringUtils.js index cec90be7122..2f709a4be68 100644 --- a/src/utils/StringUtils.js +++ b/src/utils/StringUtils.js @@ -198,21 +198,6 @@ define(function (require, exports, module) { return returnVal; } - /** - * Creates an HTML string for a list of files to be reported on, suitable for use in a dialog. - * @param {Array.} Array of filenames or paths to display. - */ - function makeDialogFileList(paths) { - var result = "
    "; - paths.forEach(function (path) { - result += "
  • "; - result += breakableUrl(path); - result += "
  • "; - }); - result += "
"; - return result; - } - // Define public API exports.format = format; exports.htmlEscape = htmlEscape; @@ -224,5 +209,4 @@ define(function (require, exports, module) { exports.breakableUrl = breakableUrl; exports.endsWith = endsWith; exports.prettyPrintBytes = prettyPrintBytes; - exports.makeDialogFileList = makeDialogFileList; }); diff --git a/test/spec/FileFilters-test.js b/test/spec/FileFilters-test.js index 733d0d49ed3..ec7abc1df59 100644 --- a/test/spec/FileFilters-test.js +++ b/test/spec/FileFilters-test.js @@ -486,7 +486,7 @@ define(function (require, exports, module) { }); }); - describe("Find in Files filtering", function () { + describe("Integration", function () { this.category = "integration"; @@ -499,9 +499,11 @@ define(function (require, exports, module) { CommandManager, $; - beforeFirst(function () { + // We don't directly call beforeFirst/afterLast here because it appears that we need + // separate test windows for the nested describe suites. + function setupTestWindow(spec) { // Create a new window that will be shared by ALL tests in this spec. - SpecRunnerUtils.createTestWindowAndRun(this, function (w) { + SpecRunnerUtils.createTestWindowAndRun(spec, function (w) { testWindow = w; // Load module instances from brackets.test @@ -514,9 +516,9 @@ define(function (require, exports, module) { SpecRunnerUtils.loadProjectInTestWindow(testPath); }); - }); + } - afterLast(function () { + function teardownTestWindow() { testWindow = null; FileSystem = null; FileFilters = null; @@ -525,8 +527,12 @@ define(function (require, exports, module) { CommandManager = null; $ = null; SpecRunnerUtils.closeTestWindow(); - }); + } + // These helper functions are slight variations of the ones in FindInFiles, so need to be + // kept in sync with those. It's a bit awkward to try to share them, and as long as + // it's just these few functions it's probably okay to just keep them in sync manually, + // but if this gets more complicated we should probably figure out how to break them out. function openSearchBar(scope) { // Make sure search bar from previous test has animated out fully runs(function () { @@ -535,8 +541,19 @@ define(function (require, exports, module) { }, "search bar close"); }); runs(function () { + FindInFiles._searchDone = false; FindInFilesUI._showFindBar(scope); }); + waitsFor(function () { + return $(".modal-bar").length === 1; + }, "search bar open"); + } + + function closeSearchBar() { + runs(function () { + var $searchField = $(".modal-bar #find-group input"); + SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_ESCAPE, "keydown", $searchField[0]); + }); } function executeSearch(searchString) { @@ -549,503 +566,459 @@ define(function (require, exports, module) { }, "Find in Files done"); } - it("should search all files by default", function () { - openSearchBar(); - runs(function () { - executeSearch("{1}"); - }); - runs(function () { - expect(FindInFiles.searchModel.results[testPath + "/test1.css"]).toBeTruthy(); - expect(FindInFiles.searchModel.results[testPath + "/test1.html"]).toBeTruthy(); + describe("Find in Files filtering", function () { + beforeFirst(function () { + setupTestWindow(this); }); - }); - - // This finishes async, since clickDialogButton() finishes async (dialogs close asynchronously) - function setExcludeCSSFiles() { - // Launch filter editor - FileFilters.editFilter({ name: "", patterns: [] }, -1); - - // Edit the filter & confirm changes - $(".modal.instance textarea").val("*.css"); - SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); - } - - it("should exclude files from search", function () { - openSearchBar(); - runs(function () { - setExcludeCSSFiles(); - }); - runs(function () { - executeSearch("{1}"); - }); - runs(function () { - // *.css should have been excluded this time - expect(FindInFiles.searchModel.results[testPath + "/test1.css"]).toBeFalsy(); - expect(FindInFiles.searchModel.results[testPath + "/test1.html"]).toBeTruthy(); - }); - }); - - it("should respect filter when searching folder", function () { - var dirEntry = FileSystem.getDirectoryForPath(testPath); - openSearchBar(dirEntry); - runs(function () { - setExcludeCSSFiles(); - }); - runs(function () { - executeSearch("{1}"); - }); - runs(function () { - // *.css should have been excluded this time - expect(FindInFiles.searchModel.results[testPath + "/test1.css"]).toBeFalsy(); - expect(FindInFiles.searchModel.results[testPath + "/test1.html"]).toBeTruthy(); - }); - }); - - it("should ignore filter when searching a single file", function () { - var fileEntry = FileSystem.getFileForPath(testPath + "/test1.css"); - openSearchBar(fileEntry); - runs(function () { - // Cannot explicitly set *.css filter in dialog because button is hidden - // (which is verified here), but filter persists from previous test - expect($("button.file-filter-picker").is(":visible")).toBeFalsy(); - }); - runs(function () { - executeSearch("{1}"); - }); - runs(function () { - // ignore *.css exclusion since we're explicitly searching this file - expect(FindInFiles.searchModel.results[testPath + "/test1.css"]).toBeTruthy(); + afterLast(teardownTestWindow); + + it("should search all files by default", function () { + openSearchBar(); + runs(function () { + executeSearch("{1}"); + }); + runs(function () { + expect(FindInFiles.searchModel.results[testPath + "/test1.css"]).toBeTruthy(); + expect(FindInFiles.searchModel.results[testPath + "/test1.html"]).toBeTruthy(); + }); }); - }); - - it("should show error when filter excludes all files", function () { - openSearchBar(); - runs(function () { + + // This finishes async, since clickDialogButton() finishes async (dialogs close asynchronously) + function setExcludeCSSFiles() { // Launch filter editor FileFilters.editFilter({ name: "", patterns: [] }, -1); // Edit the filter & confirm changes - $(".modal.instance textarea").val("test1.*\n*.css"); + $(".modal.instance textarea").val("*.css"); SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); + } + + it("should exclude files from search", function () { + openSearchBar(); + runs(function () { + setExcludeCSSFiles(); + }); + runs(function () { + executeSearch("{1}"); + }); + runs(function () { + // *.css should have been excluded this time + expect(FindInFiles.searchModel.results[testPath + "/test1.css"]).toBeFalsy(); + expect(FindInFiles.searchModel.results[testPath + "/test1.html"]).toBeTruthy(); + }); }); - runs(function () { - executeSearch("{1}"); + + it("should respect filter when searching folder", function () { + var dirEntry = FileSystem.getDirectoryForPath(testPath); + openSearchBar(dirEntry); + runs(function () { + setExcludeCSSFiles(); + }); + runs(function () { + executeSearch("{1}"); + }); + runs(function () { + // *.css should have been excluded this time + expect(FindInFiles.searchModel.results[testPath + "/test1.css"]).toBeFalsy(); + expect(FindInFiles.searchModel.results[testPath + "/test1.html"]).toBeTruthy(); + }); }); - runs(function () { - var $modalBar = $(".modal-bar"); - // Dialog still showing - expect($modalBar.length).toBe(1); + it("should ignore filter when searching a single file", function () { + var fileEntry = FileSystem.getFileForPath(testPath + "/test1.css"); + openSearchBar(fileEntry); + runs(function () { + // Cannot explicitly set *.css filter in dialog because button is hidden + // (which is verified here), but filter persists from previous test + expect($("button.file-filter-picker").is(":visible")).toBeFalsy(); + }); + runs(function () { + executeSearch("{1}"); + }); + runs(function () { + // ignore *.css exclusion since we're explicitly searching this file + expect(FindInFiles.searchModel.results[testPath + "/test1.css"]).toBeTruthy(); + }); + }); + + it("should show error when filter excludes all files", function () { + openSearchBar(); + runs(function () { + // Launch filter editor + FileFilters.editFilter({ name: "", patterns: [] }, -1); + + // Edit the filter & confirm changes + $(".modal.instance textarea").val("test1.*\n*.css"); + SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); + }); + runs(function () { + executeSearch("{1}"); + }); + runs(function () { + var $modalBar = $(".modal-bar"); - // Error message displayed - expect($modalBar.find("#find-group div.error").is(":visible")).toBeTruthy(); + // Dialog still showing + expect($modalBar.length).toBe(1); - // Search panel not showing - expect($("#find-in-files-results").is(":visible")).toBeFalsy(); + // Error message displayed + expect($modalBar.find("#find-group div.error").is(":visible")).toBeTruthy(); - // Close search bar - var $searchField = $modalBar.find("#find-group input"); - SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_ESCAPE, "keydown", $searchField[0]); - }); - }); - - it("should respect filter when editing code", function () { - openSearchBar(); - runs(function () { - setExcludeCSSFiles(); - }); - runs(function () { - executeSearch("{1}"); - }); - runs(function () { - var promise = testWindow.brackets.test.DocumentManager.getDocumentForPath(testPath + "/test1.css"); - waitsForDone(promise); - promise.done(function (doc) { - // Modify line that contains potential search result - expect(doc.getLine(5).indexOf("{1}")).not.toBe(-1); - doc.replaceRange("X", {line: 5, ch: 0}); + // Search panel not showing + expect($("#find-in-files-results").is(":visible")).toBeFalsy(); + + // Close search bar + var $searchField = $modalBar.find("#find-group input"); + SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_ESCAPE, "keydown", $searchField[0]); }); }); - runs(function () { - waits(800); // ensure _documentChangeHandler()'s timeout has time to run - }); - runs(function () { - expect(FindInFiles.searchModel.results[testPath + "/test1.css"]).toBeFalsy(); // *.css should still be excluded - expect(FindInFiles.searchModel.results[testPath + "/test1.html"]).toBeTruthy(); + + it("should respect filter when editing code", function () { + openSearchBar(); + runs(function () { + setExcludeCSSFiles(); + }); + runs(function () { + executeSearch("{1}"); + }); + runs(function () { + var promise = testWindow.brackets.test.DocumentManager.getDocumentForPath(testPath + "/test1.css"); + waitsForDone(promise); + promise.done(function (doc) { + // Modify line that contains potential search result + expect(doc.getLine(5).indexOf("{1}")).not.toBe(-1); + doc.replaceRange("X", {line: 5, ch: 0}); + }); + }); + runs(function () { + waits(800); // ensure _documentChangeHandler()'s timeout has time to run + }); + runs(function () { + expect(FindInFiles.searchModel.results[testPath + "/test1.css"]).toBeFalsy(); // *.css should still be excluded + expect(FindInFiles.searchModel.results[testPath + "/test1.html"]).toBeTruthy(); + }); }); }); - }); - describe("Filter picker UI", function () { - - this.category = "integration"; - - var testPath = SpecRunnerUtils.getTestPath("/spec/InlineEditorProviders-test-files"), - testWindow, - FileFilters, - FileSystem, - FindInFiles, - CommandManager, - $; - - beforeFirst(function () { - // Create a new window that will be shared by ALL tests in this spec. - SpecRunnerUtils.createTestWindowAndRun(this, function (w) { - testWindow = w; - - // Load module instances from brackets.test - FileFilters = testWindow.brackets.test.FileFilters; - FileSystem = testWindow.brackets.test.FileSystem; - FindInFiles = testWindow.brackets.test.FindInFiles; - CommandManager = testWindow.brackets.test.CommandManager; - $ = testWindow.$; - - SpecRunnerUtils.loadProjectInTestWindow(testPath); + describe("Filter picker UI", function () { + beforeFirst(function () { + setupTestWindow(this); }); - }); + afterLast(teardownTestWindow); - afterLast(function () { - testWindow = null; - FileSystem = null; - FileFilters = null; - FindInFiles = null; - CommandManager = null; - $ = null; - SpecRunnerUtils.closeTestWindow(); - }); - - function openSearchBar(scope) { - // Make sure search bar from previous test has animated out fully - runs(function () { - waitsFor(function () { - return $(".modal-bar").length === 0; - }, "search bar close"); + beforeEach(function () { + openSearchBar(); }); - runs(function () { - FindInFiles._doFindInFiles(scope); - }); - } - function closeSearchBar() { - runs(function () { - var $searchField = $(".modal-bar #find-group input"); - SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_ESCAPE, "keydown", $searchField[0]); + afterEach(function () { + closeSearchBar(); }); - } - beforeEach(function () { - openSearchBar(); - }); - - afterEach(function () { - closeSearchBar(); - }); - - function verifyButtonLabel(expectedLabel) { - var newButtonLabel = StringUtils.format(Strings.EXCLUDE_FILE_FILTER, expectedLabel); - - if (expectedLabel) { - // Verify filter picker button label is updated with the patterns of the selected filter set. - expect($("button.file-filter-picker").text()).toEqual(newButtonLabel); - } else { - expect($("button.file-filter-picker").text()).toEqual(Strings.NO_FILE_FILTER); + function verifyButtonLabel(expectedLabel) { + var newButtonLabel = StringUtils.format(Strings.EXCLUDE_FILE_FILTER, expectedLabel); + + if (expectedLabel) { + // Verify filter picker button label is updated with the patterns of the selected filter set. + expect($("button.file-filter-picker").text()).toEqual(newButtonLabel); + } else { + expect($("button.file-filter-picker").text()).toEqual(Strings.NO_FILE_FILTER); + } } - } - - // This finishes async, since clickDialogButton() finishes async (dialogs close asynchronously) - function setExcludeCSSFiles() { - // Edit the filter & confirm changes - $(".modal.instance .exclusions-name").val("CSS Files"); - $(".modal.instance .exclusions-editor").val("*.css\n*.less\n*.scss"); - SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); - } - // Trigger a mouseover event on the 'parent' and then click on the button with the given 'selector'. - function clickOnMouseOverButton(selector, parent) { - runs(function () { - parent.trigger("mouseover"); - }); - runs(function () { - expect($(selector, parent).is(":visible")).toBeTruthy(); - $(selector, parent).click(); - }); - } - - it("should show 'No files Excluded' in filter picker button by default", function () { - runs(function () { - verifyButtonLabel(); - }); - }); - - it("should show two filter commands by default", function () { - runs(function () { - FileFilters.showDropdown(); - }); - - runs(function () { - var $dropdown = $(".dropdown-menu.dropdownbutton-popup"); - expect($dropdown.is(":visible")).toBeTruthy(); - expect($dropdown.children().length).toEqual(2); - expect($($dropdown.children()[0]).text()).toEqual(Strings.NEW_FILE_FILTER); - expect($($dropdown.children()[1]).text()).toEqual(Strings.CLEAR_FILE_FILTER); - }); + // This finishes async, since clickDialogButton() finishes async (dialogs close asynchronously) + function setExcludeCSSFiles() { + // Edit the filter & confirm changes + $(".modal.instance .exclusions-name").val("CSS Files"); + $(".modal.instance .exclusions-editor").val("*.css\n*.less\n*.scss"); + SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); + } - runs(function () { - FileFilters.closeDropdown(); - }); - }); + // Trigger a mouseover event on the 'parent' and then click on the button with the given 'selector'. + function clickOnMouseOverButton(selector, parent) { + runs(function () { + parent.trigger("mouseover"); + }); + runs(function () { + expect($(selector, parent).is(":visible")).toBeTruthy(); + $(selector, parent).click(); + }); + } - it("should launch filter editor and add a new filter set when invoked from new filter command", function () { - var $dropdown; - runs(function () { - FileFilters.showDropdown(); - }); - - // Invoke new filter command by pressing down arrow key once and then enter key. - runs(function () { - $dropdown = $(".dropdown-menu.dropdownbutton-popup"); - SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_DOWN, "keydown", $dropdown[0]); - SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_RETURN, "keydown", $dropdown[0]); + it("should show 'No files Excluded' in filter picker button by default", function () { + runs(function () { + verifyButtonLabel(); + }); }); - runs(function () { - setExcludeCSSFiles(); - }); - - runs(function () { - FileFilters.showDropdown(); - }); - - runs(function () { - var filterSuffix = StringUtils.format(Strings.FILE_FILTER_CLIPPED_SUFFIX, 1); - - // Verify filter picker button is updated with the name of the new filter. - verifyButtonLabel("CSS Files"); - - $dropdown = $(".dropdown-menu.dropdownbutton-popup"); - expect($dropdown.is(":visible")).toBeTruthy(); - expect($dropdown.children().length).toEqual(4); - expect($($dropdown.children()[0]).text()).toEqual(Strings.NEW_FILE_FILTER); - expect($($dropdown.children()[1]).text()).toEqual(Strings.CLEAR_FILE_FILTER); - expect($(".recent-filter-name", $($dropdown.children()[3])).text()).toEqual("CSS Files"); - expect($(".recent-filter-patterns", $($dropdown.children()[3])).text()).toEqual(" - *.css, *.less " + filterSuffix); - }); - - runs(function () { - FileFilters.closeDropdown(); - }); - }); + it("should show two filter commands by default", function () { + runs(function () { + FileFilters.showDropdown(); + }); - it("should clear the active filter set when invoked from clear filter command", function () { - var $dropdown; - runs(function () { - FileFilters.showDropdown(); - }); + runs(function () { + var $dropdown = $(".dropdown-menu.dropdownbutton-popup"); + expect($dropdown.is(":visible")).toBeTruthy(); + expect($dropdown.children().length).toEqual(2); + expect($($dropdown.children()[0]).text()).toEqual(Strings.NEW_FILE_FILTER); + expect($($dropdown.children()[1]).text()).toEqual(Strings.CLEAR_FILE_FILTER); + }); - // Invoke new filter command by pressing down arrow key twice and then enter key. - runs(function () { - $dropdown = $(".dropdown-menu.dropdownbutton-popup"); - SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_DOWN, "keydown", $dropdown[0]); - SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_DOWN, "keydown", $dropdown[0]); - SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_RETURN, "keydown", $dropdown[0]); + runs(function () { + FileFilters.closeDropdown(); + }); }); - runs(function () { - // Verify filter picker button is updated to show no active filter. - verifyButtonLabel(); - expect($dropdown.is(":visible")).toBeFalsy(); - }); - - // Re-open dropdown list and verify that nothing changed. - runs(function () { - FileFilters.showDropdown(); - }); - - runs(function () { - var filterSuffix = StringUtils.format(Strings.FILE_FILTER_CLIPPED_SUFFIX, 1); - - $dropdown = $(".dropdown-menu.dropdownbutton-popup"); - expect($dropdown.is(":visible")).toBeTruthy(); - expect($dropdown.children().length).toEqual(4); - expect($($dropdown.children()[0]).text()).toEqual(Strings.NEW_FILE_FILTER); - expect($($dropdown.children()[1]).text()).toEqual(Strings.CLEAR_FILE_FILTER); - expect($(".recent-filter-name", $($dropdown.children()[3])).text()).toEqual("CSS Files"); - expect($(".recent-filter-patterns", $($dropdown.children()[3])).text()).toEqual(" - *.css, *.less " + filterSuffix); - }); - - runs(function () { - FileFilters.closeDropdown(); - }); - }); + it("should launch filter editor and add a new filter set when invoked from new filter command", function () { + var $dropdown; + runs(function () { + FileFilters.showDropdown(); + }); - it("should switch the active filter set to the selected one", function () { - var $dropdown; - runs(function () { - // Verify that there is no active filter (was set from the previous test). - verifyButtonLabel(); - FileFilters.showDropdown(); - }); + // Invoke new filter command by pressing down arrow key once and then enter key. + runs(function () { + $dropdown = $(".dropdown-menu.dropdownbutton-popup"); + SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_DOWN, "keydown", $dropdown[0]); + SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_RETURN, "keydown", $dropdown[0]); + }); - // Select the last filter set in the dropdown by pressing up arrow key once and then enter key. - runs(function () { - $dropdown = $(".dropdown-menu.dropdownbutton-popup"); - SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_UP, "keydown", $dropdown[0]); - SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_RETURN, "keydown", $dropdown[0]); - }); + runs(function () { + setExcludeCSSFiles(); + }); - runs(function () { - // Verify filter picker button label is updated with the name of the selected filter set. - verifyButtonLabel("CSS Files"); - expect($dropdown.is(":visible")).toBeFalsy(); - }); - }); + runs(function () { + FileFilters.showDropdown(); + }); - it("should launch filter editor and fill in the text fields with selected filter info", function () { - var $dropdown; + runs(function () { + var filterSuffix = StringUtils.format(Strings.FILE_FILTER_CLIPPED_SUFFIX, 1); - runs(function () { - FileFilters.showDropdown(); - }); - - // Click on the edit icon that shows up in the first filter set on mouseover. - runs(function () { - $dropdown = $(".dropdown-menu.dropdownbutton-popup"); - clickOnMouseOverButton(".filter-edit-icon", $($dropdown.children()[3])); - }); - - // Remove the name of the filter set and reduce the filter set to '*.css'. - runs(function () { - expect($(".modal.instance .exclusions-name").val()).toEqual("CSS Files"); - expect($(".modal.instance .exclusions-editor").val()).toEqual("*.css\n*.less\n*.scss"); - - $(".modal.instance .exclusions-name").val(""); - $(".modal.instance .exclusions-editor").val("*.css"); - SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); - }); - - runs(function () { - // Verify filter picker button label is updated with the patterns of the selected filter set. - verifyButtonLabel("*.css"); - expect($dropdown.is(":visible")).toBeFalsy(); - }); - }); + // Verify filter picker button is updated with the name of the new filter. + verifyButtonLabel("CSS Files"); - it("should remove selected filter from filter sets preferences without changing picker button label", function () { - var $dropdown, - filters = [{name: "Node Modules", patterns: ["node_module"]}, - {name: "Mark Down Files", patterns: ["*.md"]}, - {name: "CSS Files", patterns: ["*.css", "*.less"]}]; - - // Create three filter sets and make the last one active. - runs(function () { - FileFilters.editFilter(filters[0], 0); - SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); - }); + $dropdown = $(".dropdown-menu.dropdownbutton-popup"); + expect($dropdown.is(":visible")).toBeTruthy(); + expect($dropdown.children().length).toEqual(4); + expect($($dropdown.children()[0]).text()).toEqual(Strings.NEW_FILE_FILTER); + expect($($dropdown.children()[1]).text()).toEqual(Strings.CLEAR_FILE_FILTER); + expect($(".recent-filter-name", $($dropdown.children()[3])).text()).toEqual("CSS Files"); + expect($(".recent-filter-patterns", $($dropdown.children()[3])).text()).toEqual(" - *.css, *.less " + filterSuffix); + }); - runs(function () { - FileFilters.editFilter(filters[1], -1); - SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); - }); - - runs(function () { - FileFilters.editFilter(filters[2], -1); - SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); + runs(function () { + FileFilters.closeDropdown(); + }); }); - runs(function () { - verifyButtonLabel("CSS Files"); - FileFilters.showDropdown(); - }); - - runs(function () { - $dropdown = $(".dropdown-menu.dropdownbutton-popup"); - expect($dropdown.children().length).toEqual(6); - }); + it("should clear the active filter set when invoked from clear filter command", function () { + var $dropdown; + runs(function () { + FileFilters.showDropdown(); + }); - // Click on the delete icon that shows up in the first filter set on mouseover. - runs(function () { - clickOnMouseOverButton(".filter-trash-icon", $($dropdown.children()[3])); - }); - - runs(function () { - expect($dropdown.is(":visible")).toBeTruthy(); - // Verify that button label is still the same since the deleted one is not the active one. - verifyButtonLabel("CSS Files"); - - // Verify that the list has one less item (from 6 to 5). - expect($dropdown.children().length).toEqual(5); - - // Verify data-index of the two remaining filter sets. - expect($("a", $dropdown.children()[3]).data("index")).toBe(3); - expect($("a", $dropdown.children()[4]).data("index")).toBe(4); - }); - - runs(function () { - FileFilters.closeDropdown(); - }); - }); + // Invoke new filter command by pressing down arrow key twice and then enter key. + runs(function () { + $dropdown = $(".dropdown-menu.dropdownbutton-popup"); + SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_DOWN, "keydown", $dropdown[0]); + SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_DOWN, "keydown", $dropdown[0]); + SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_RETURN, "keydown", $dropdown[0]); + }); + + runs(function () { + // Verify filter picker button is updated to show no active filter. + verifyButtonLabel(); + expect($dropdown.is(":visible")).toBeFalsy(); + }); - it("should remove selected filter from filter sets preferences plus changing picker button label", function () { - var $dropdown; + // Re-open dropdown list and verify that nothing changed. + runs(function () { + FileFilters.showDropdown(); + }); - runs(function () { - verifyButtonLabel("CSS Files"); - FileFilters.showDropdown(); - }); - - runs(function () { - $dropdown = $(".dropdown-menu.dropdownbutton-popup"); - expect($dropdown.children().length).toEqual(5); - }); + runs(function () { + var filterSuffix = StringUtils.format(Strings.FILE_FILTER_CLIPPED_SUFFIX, 1); - // Click on the delete icon that shows up in the last filter set on mouseover. - runs(function () { - clickOnMouseOverButton(".filter-trash-icon", $($dropdown.children()[4])); - }); - - runs(function () { - expect($dropdown.is(":visible")).toBeTruthy(); - // Verify that button label is changed to "No Files Excluded". - verifyButtonLabel(); - - // Verify that the list has one less item. - expect($dropdown.children().length).toEqual(4); - }); - - runs(function () { - FileFilters.closeDropdown(); + $dropdown = $(".dropdown-menu.dropdownbutton-popup"); + expect($dropdown.is(":visible")).toBeTruthy(); + expect($dropdown.children().length).toEqual(4); + expect($($dropdown.children()[0]).text()).toEqual(Strings.NEW_FILE_FILTER); + expect($($dropdown.children()[1]).text()).toEqual(Strings.CLEAR_FILE_FILTER); + expect($(".recent-filter-name", $($dropdown.children()[3])).text()).toEqual("CSS Files"); + expect($(".recent-filter-patterns", $($dropdown.children()[3])).text()).toEqual(" - *.css, *.less " + filterSuffix); + }); + + runs(function () { + FileFilters.closeDropdown(); + }); }); - }); - it("should also remove the divider from the dropdown list after removing the last remaining filter set", function () { - var $dropdown; + it("should switch the active filter set to the selected one", function () { + var $dropdown; + runs(function () { + // Verify that there is no active filter (was set from the previous test). + verifyButtonLabel(); + FileFilters.showDropdown(); + }); - runs(function () { - verifyButtonLabel(); - FileFilters.showDropdown(); + // Select the last filter set in the dropdown by pressing up arrow key once and then enter key. + runs(function () { + $dropdown = $(".dropdown-menu.dropdownbutton-popup"); + SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_UP, "keydown", $dropdown[0]); + SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_RETURN, "keydown", $dropdown[0]); + }); + + runs(function () { + // Verify filter picker button label is updated with the name of the selected filter set. + verifyButtonLabel("CSS Files"); + expect($dropdown.is(":visible")).toBeFalsy(); + }); }); - - runs(function () { - $dropdown = $(".dropdown-menu.dropdownbutton-popup"); - expect($dropdown.children().length).toEqual(4); + + it("should launch filter editor and fill in the text fields with selected filter info", function () { + var $dropdown; + + runs(function () { + FileFilters.showDropdown(); + }); + + // Click on the edit icon that shows up in the first filter set on mouseover. + runs(function () { + $dropdown = $(".dropdown-menu.dropdownbutton-popup"); + clickOnMouseOverButton(".filter-edit-icon", $($dropdown.children()[3])); + }); + + // Remove the name of the filter set and reduce the filter set to '*.css'. + runs(function () { + expect($(".modal.instance .exclusions-name").val()).toEqual("CSS Files"); + expect($(".modal.instance .exclusions-editor").val()).toEqual("*.css\n*.less\n*.scss"); + + $(".modal.instance .exclusions-name").val(""); + $(".modal.instance .exclusions-editor").val("*.css"); + SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); + }); + + runs(function () { + // Verify filter picker button label is updated with the patterns of the selected filter set. + verifyButtonLabel("*.css"); + expect($dropdown.is(":visible")).toBeFalsy(); + }); }); - // Click on the delete icon that shows up in the last filter set on mouseover. - runs(function () { - clickOnMouseOverButton(".filter-trash-icon", $($dropdown.children()[3])); + it("should remove selected filter from filter sets preferences without changing picker button label", function () { + var $dropdown, + filters = [{name: "Node Modules", patterns: ["node_module"]}, + {name: "Mark Down Files", patterns: ["*.md"]}, + {name: "CSS Files", patterns: ["*.css", "*.less"]}]; + + // Create three filter sets and make the last one active. + runs(function () { + FileFilters.editFilter(filters[0], 0); + SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); + }); + + runs(function () { + FileFilters.editFilter(filters[1], -1); + SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); + }); + + runs(function () { + FileFilters.editFilter(filters[2], -1); + SpecRunnerUtils.clickDialogButton(Dialogs.DIALOG_BTN_OK, true); + }); + + runs(function () { + verifyButtonLabel("CSS Files"); + FileFilters.showDropdown(); + }); + + runs(function () { + $dropdown = $(".dropdown-menu.dropdownbutton-popup"); + expect($dropdown.children().length).toEqual(6); + }); + + // Click on the delete icon that shows up in the first filter set on mouseover. + runs(function () { + clickOnMouseOverButton(".filter-trash-icon", $($dropdown.children()[3])); + }); + + runs(function () { + expect($dropdown.is(":visible")).toBeTruthy(); + // Verify that button label is still the same since the deleted one is not the active one. + verifyButtonLabel("CSS Files"); + + // Verify that the list has one less item (from 6 to 5). + expect($dropdown.children().length).toEqual(5); + + // Verify data-index of the two remaining filter sets. + expect($("a", $dropdown.children()[3]).data("index")).toBe(3); + expect($("a", $dropdown.children()[4]).data("index")).toBe(4); + }); + + runs(function () { + FileFilters.closeDropdown(); + }); }); - - runs(function () { - expect($dropdown.is(":visible")).toBeTruthy(); - // Verify that button label still shows "No Files Excluded". - verifyButtonLabel(); - - // Verify that the list has only two filter commands. - expect($dropdown.children().length).toEqual(2); + + it("should remove selected filter from filter sets preferences plus changing picker button label", function () { + var $dropdown; + + runs(function () { + verifyButtonLabel("CSS Files"); + FileFilters.showDropdown(); + }); + + runs(function () { + $dropdown = $(".dropdown-menu.dropdownbutton-popup"); + expect($dropdown.children().length).toEqual(5); + }); + + // Click on the delete icon that shows up in the last filter set on mouseover. + runs(function () { + clickOnMouseOverButton(".filter-trash-icon", $($dropdown.children()[4])); + }); + + runs(function () { + expect($dropdown.is(":visible")).toBeTruthy(); + // Verify that button label is changed to "No Files Excluded". + verifyButtonLabel(); + + // Verify that the list has one less item. + expect($dropdown.children().length).toEqual(4); + }); + + runs(function () { + FileFilters.closeDropdown(); + }); }); - - runs(function () { - FileFilters.closeDropdown(); + + it("should also remove the divider from the dropdown list after removing the last remaining filter set", function () { + var $dropdown; + + runs(function () { + verifyButtonLabel(); + FileFilters.showDropdown(); + }); + + runs(function () { + $dropdown = $(".dropdown-menu.dropdownbutton-popup"); + expect($dropdown.children().length).toEqual(4); + }); + + // Click on the delete icon that shows up in the last filter set on mouseover. + runs(function () { + clickOnMouseOverButton(".filter-trash-icon", $($dropdown.children()[3])); + }); + + runs(function () { + expect($dropdown.is(":visible")).toBeTruthy(); + // Verify that button label still shows "No Files Excluded". + verifyButtonLabel(); + + // Verify that the list has only two filter commands. + expect($dropdown.children().length).toEqual(2); + }); + + runs(function () { + FileFilters.closeDropdown(); + }); }); }); }); diff --git a/test/spec/FindInFiles-test.js b/test/spec/FindInFiles-test.js index ce0c3437294..942b84466d5 100644 --- a/test/spec/FindInFiles-test.js +++ b/test/spec/FindInFiles-test.js @@ -49,6 +49,7 @@ define(function (require, exports, module) { var defaultSourcePath = SpecRunnerUtils.getTestPath("/spec/FindReplace-test-files"), testPath, nextFolderIndex = 1, + searchResults, CommandManager, DocumentManager, EditorManager, @@ -147,6 +148,110 @@ define(function (require, exports, module) { }, "Find in Files done"); } + function numMatches(results) { + return _.reduce(_.pluck(results, "matches"), function (sum, matches) { + return sum + matches.length; + }, 0); + } + + function doSearch(options) { + runs(function () { + FindInFiles.doSearchInScope(options.queryInfo, null, null, options.replaceText).done(function (results) { + searchResults = results; + }); + }); + waitsFor(function () { return searchResults; }, 1000, "search completed"); + runs(function () { + expect(numMatches(searchResults)).toBe(options.numMatches); + }); + } + + function doReplace(options) { + return FindInFiles.doReplace(searchResults, options.replaceText, { + forceFilesOpen: options.forceFilesOpen, + isRegexp: options.queryInfo.isRegexp + }); + } + + /** + * Helper function that calls the given asynchronous processor once on each file in the given subtree + * and returns a promise that's resolved when all files are processed. + * @param {string} rootPath The root of the subtree to search. + * @param {function(string, string): $.Promise} processor The function that processes each file. Args are: + * contents: the contents of the file + * fullPath: the full path to the file on disk + * @return {$.Promise} A promise that is resolved when all files are processed, or rejected if there was + * an error reading one of the files or one of the process steps was rejected. + */ + function visitAndProcessFiles(rootPath, processor) { + var rootEntry = FileSystem.getDirectoryForPath(rootPath), + files = []; + + function visitor(file) { + if (!file.isDirectory) { + // Skip binary files, since we don't care about them for these purposes and we can't read them + // to get their contents. + if (!LanguageManager.getLanguageForPath(file.fullPath).isBinary()) { + files.push(file); + } + } + return true; + } + return promisify(rootEntry, "visit", visitor).then(function () { + return Async.doInParallel(files, function (file) { + return promisify(file, "read").then(function (contents) { + return processor(contents, file.fullPath); + }); + }); + }); + } + + function ensureParentExists(file) { + var parentDir = FileSystem.getDirectoryForPath(file.parentPath); + return promisify(parentDir, "exists").then(function (exists) { + if (!exists) { + return promisify(parentDir, "create"); + } + return null; + }); + } + + function copyWithLineEndings(src, dest, lineEndings) { + function copyOneFileWithLineEndings(contents, srcPath) { + var destPath = dest + srcPath.slice(src.length), + destFile = FileSystem.getFileForPath(destPath), + newContents = FileUtils.translateLineEndings(contents, lineEndings); + return ensureParentExists(destFile).then(function () { + return promisify(destFile, "write", newContents); + }); + } + + return promisify(FileSystem.getDirectoryForPath(dest), "create").then(function () { + return visitAndProcessFiles(src, copyOneFileWithLineEndings); + }); + } + + // Creates a clean copy of the test project before each test. We don't delete the old + // folders as we go along (to avoid problems with deleting the project out from under the + // open test window); we just delete the whole temp folder at the end. + function openTestProjectCopy(sourcePath, lineEndings) { + testPath = SpecRunnerUtils.getTempDirectory() + "/find-in-files-test-" + (nextFolderIndex++); + runs(function () { + if (lineEndings) { + waitsForDone(copyWithLineEndings(sourcePath, testPath, lineEndings), "copy test files with line endings"); + } else { + // Note that we don't skip image files in this case, but it doesn't matter since we'll + // only compare files that have an associated file in the known goods folder. + waitsForDone(SpecRunnerUtils.copy(sourcePath, testPath), "copy test files"); + } + }); + SpecRunnerUtils.loadProjectInTestWindow(testPath); + } + + beforeEach(function () { + searchResults = null; + }); + describe("Find", function () { beforeEach(function () { openProject(defaultSourcePath); @@ -211,7 +316,7 @@ define(function (require, exports, module) { expectedMessage = StringUtils.format(Strings.FILTER_FILE_COUNT_ALL, 0, Strings.FIND_IN_FILES_NO_SCOPE); }); - // Message loads asynchronously, but dialog should evetually state: "Allows all 0 files in project" + // Message loads asynchronously, but dialog should eventually state: "Allows all 0 files in project" waitsFor(function () { actualMessage = $dlg.find(".exclusions-filecount").text(); return (actualMessage === expectedMessage); @@ -541,16 +646,16 @@ define(function (require, exports, module) { // Check for presence of file and first/last item rows within each file options.matchRanges.forEach(function (range) { - var $fileRow = $("#find-in-files-results tr.file-section[data-file='" + range.file + "']"); + var $fileRow = $("#find-in-files-results tr.file-section[data-file-index='" + range.file + "']"); expect($fileRow.length).toBe(1); expect($fileRow.find(".dialog-filename").text()).toEqual(range.filename); - var $firstMatchRow = $("#find-in-files-results tr[data-file='" + range.file + "'][data-item='" + range.first + "']"); + var $firstMatchRow = $("#find-in-files-results tr[data-file-index='" + range.file + "'][data-item-index='" + range.first + "']"); expect($firstMatchRow.length).toBe(1); expect($firstMatchRow.find(".line-number").text().match("\\b" + range.firstLine + "\\b")).toBeTruthy(); expect($firstMatchRow.find(".line-text").text().match(range.pattern)).toBeTruthy(); - var $lastMatchRow = $("#find-in-files-results tr[data-file='" + range.file + "'][data-item='" + range.last + "']"); + var $lastMatchRow = $("#find-in-files-results tr[data-file-index='" + range.file + "'][data-item-index='" + range.last + "']"); expect($lastMatchRow.length).toBe(1); expect($lastMatchRow.find(".line-number").text().match("\\b" + range.lastLine + "\\b")).toBeTruthy(); expect($lastMatchRow.find(".line-text").text().match(range.pattern)).toBeTruthy(); @@ -605,84 +710,279 @@ define(function (require, exports, module) { }); }); - describe("Replace", function () { - var searchResults; + describe("SearchModel update on change events", function () { + var oldResults, gotChange, wasQuickChange; - /** - * Helper function that calls the given asynchronous processor once on each file in the given subtree - * and returns a promise that's resolved when all files are processed. - * @param {string} rootPath The root of the subtree to search. - * @param {function(string, string): $.Promise} processor The function that processes each file. Args are: - * contents: the contents of the file - * fullPath: the full path to the file on disk - * @return {$.Promise} A promise that is resolved when all files are processed, or rejected if there was - * an error reading one of the files or one of the process steps was rejected. - */ - function visitAndProcessFiles(rootPath, processor) { - var rootEntry = FileSystem.getDirectoryForPath(rootPath), - files = []; - - function visitor(file) { - if (!file.isDirectory) { - // Skip binary files, since we don't care about them for these purposes and we can't read them - // to get their contents. - if (!LanguageManager.getLanguageForPath(file.fullPath).isBinary()) { - files.push(file); - } - } - return true; - } - return promisify(rootEntry, "visit", visitor).then(function () { - return Async.doInParallel(files, function (file) { - return promisify(file, "read").then(function (contents) { - return processor(contents, file.fullPath); - }); - }); - }); + function fullTestPath(path) { + return testPath + "/" + path; } - function ensureParentExists(file) { - var parentDir = FileSystem.getDirectoryForPath(file.parentPath); - return promisify(parentDir, "exists").then(function (exists) { - if (!exists) { - return promisify(parentDir, "create"); + function expectUnchangedExcept(paths) { + Object.keys(FindInFiles.searchModel.results).forEach(function (path) { + if (paths.indexOf(path) === -1) { + expect(FindInFiles.searchModel.results[path]).toEqual(oldResults[path]); } - return null; }); } - function copyWithLineEndings(src, dest, lineEndings) { - function copyOneFileWithLineEndings(contents, srcPath) { - var destPath = dest + srcPath.slice(src.length), - destFile = FileSystem.getFileForPath(destPath), - newContents = FileUtils.translateLineEndings(contents, lineEndings); - return ensureParentExists(destFile).then(function () { - return promisify(destFile, "write", newContents); + beforeEach(function () { + gotChange = false; + oldResults = null; + wasQuickChange = false; + $(FindInFiles.searchModel).on("change.FindInFilesTest", function (event, quickChange) { + gotChange = true; + wasQuickChange = quickChange; + }); + + openTestProjectCopy(defaultSourcePath); + doSearch({ + queryInfo: {query: "foo"}, + numMatches: 14 + }); + runs(function () { + oldResults = _.cloneDeep(FindInFiles.searchModel.results); + }); + }); + + afterEach(function () { + $(FindInFiles.searchModel).off(".FindInFilesTest"); + waitsForDone(CommandManager.execute(Commands.FILE_CLOSE_ALL, { _forceClose: true }), "close all files"); + }); + + describe("when filename changes", function () { + it("should handle a filename change", function () { + runs(function () { + FindInFiles._fileNameChangeHandler(null, fullTestPath("foo.html"), fullTestPath("newfoo.html")); + }); + waitsFor(function () { return gotChange; }, "model change event"); + runs(function () { + expectUnchangedExcept([fullTestPath("foo.html"), fullTestPath("newfoo.html")]); + expect(FindInFiles.searchModel.results[fullTestPath("foo.html")]).toBeUndefined(); + expect(FindInFiles.searchModel.results[fullTestPath("newfoo.html")]).toEqual(oldResults[fullTestPath("foo.html")]); + expect(FindInFiles.searchModel.countFilesMatches()).toEqual({files: 3, matches: 14}); + expect(wasQuickChange).toBeFalsy(); }); - } - - return promisify(FileSystem.getDirectoryForPath(dest), "create").then(function () { - return visitAndProcessFiles(src, copyOneFileWithLineEndings); }); - } + + it("should handle a folder change", function () { + runs(function () { + FindInFiles._fileNameChangeHandler(null, fullTestPath("css"), fullTestPath("newcss")); + }); + waitsFor(function () { return gotChange; }, "model change event"); + runs(function () { + expectUnchangedExcept([fullTestPath("css/foo.css"), fullTestPath("newcss/foo.css")]); + expect(FindInFiles.searchModel.results[fullTestPath("css/foo.css")]).toBeUndefined(); + expect(FindInFiles.searchModel.results[fullTestPath("newcss/foo.css")]).toEqual(oldResults[fullTestPath("css/foo.css")]); + expect(FindInFiles.searchModel.countFilesMatches()).toEqual({files: 3, matches: 14}); + expect(wasQuickChange).toBeFalsy(); + }); + }); + }); - // Creates a clean copy of the test project before each test. We don't delete the old - // folders as we go along (to avoid problems with deleting the project out from under the - // open test window); we just delete the whole temp folder at the end. - function openTestProjectCopy(sourcePath, lineEndings) { - testPath = SpecRunnerUtils.getTempDirectory() + "/find-in-files-test-" + (nextFolderIndex++); - runs(function () { - if (lineEndings) { - waitsForDone(copyWithLineEndings(sourcePath, testPath, lineEndings), "copy test files with line endings"); - } else { - // Note that we don't skip image files in this case, but it doesn't matter since we'll - // only compare files that have an associated file in the known goods folder. - waitsForDone(SpecRunnerUtils.copy(sourcePath, testPath), "copy test files"); - } + describe("when in-memory document changes", function () { + it("should update the results when a matching line is added, updating line numbers and adding the match", function () { + runs(function () { + waitsForDone(CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, { fullPath: fullTestPath("foo.html") })); + }); + runs(function () { + var doc = DocumentManager.getOpenDocumentForPath(fullTestPath("foo.html")), + i; + expect(doc).toBeTruthy(); + + // Insert another line containing "foo" immediately above the second "foo" match. + doc.replaceRange("this is a foo instance\n", {line: 5, ch: 0}); + + // This should update synchronously. + expect(gotChange).toBe(true); + + var oldFileResults = oldResults[fullTestPath("foo.html")], + newFileResults = FindInFiles.searchModel.results[fullTestPath("foo.html")]; + + // First match should be unchanged. + expect(newFileResults.matches[0]).toEqual(oldFileResults.matches[0]); + + // Next match should be the new match. We just check the offsets here, not everything in the match record. + expect(newFileResults.matches[1].start).toEqual({line: 5, ch: 10}); + expect(newFileResults.matches[1].end).toEqual({line: 5, ch: 13}); + + // Rest of the matches should have had their lines adjusted. + for (i = 2; i < newFileResults.matches.length; i++) { + var newMatch = newFileResults.matches[i], + oldMatch = oldFileResults.matches[i - 1]; + expect(newMatch.start).toEqual({line: oldMatch.start.line + 1, ch: oldMatch.start.ch}); + expect(newMatch.end).toEqual({line: oldMatch.end.line + 1, ch: oldMatch.end.ch}); + } + + // There should be one new match. + expect(FindInFiles.searchModel.countFilesMatches()).toEqual({files: 3, matches: 15}); + + // Make sure the model is adding the flag that will make the view debounce changes. + expect(wasQuickChange).toBeTruthy(); + }); }); - SpecRunnerUtils.loadProjectInTestWindow(testPath); - } + + it("should update the results when a matching line is deleted, updating line numbers and removing the match", function () { + runs(function () { + waitsForDone(CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, { fullPath: fullTestPath("foo.html") })); + }); + runs(function () { + var doc = DocumentManager.getOpenDocumentForPath(fullTestPath("foo.html")), + i; + expect(doc).toBeTruthy(); + + // Remove the second "foo" match. + doc.replaceRange("", {line: 5, ch: 0}, {line: 6, ch: 0}); + + // This should update synchronously. + expect(gotChange).toBe(true); + + var oldFileResults = oldResults[fullTestPath("foo.html")], + newFileResults = FindInFiles.searchModel.results[fullTestPath("foo.html")]; + + // First match should be unchanged. + expect(newFileResults.matches[0]).toEqual(oldFileResults.matches[0]); + + // Second match should be deleted. The rest of the matches should have their lines adjusted. + for (i = 1; i < newFileResults.matches.length; i++) { + var newMatch = newFileResults.matches[i], + oldMatch = oldFileResults.matches[i + 1]; + expect(newMatch.start).toEqual({line: oldMatch.start.line - 1, ch: oldMatch.start.ch}); + expect(newMatch.end).toEqual({line: oldMatch.end.line - 1, ch: oldMatch.end.ch}); + } + + // There should be one fewer match. + expect(FindInFiles.searchModel.countFilesMatches()).toEqual({files: 3, matches: 13}); + + // Make sure the model is adding the flag that will make the view debounce changes. + expect(wasQuickChange).toBeTruthy(); + }); + }); + + it("should replace matches in a portion of the document that was edited to include a new match", function () { + runs(function () { + waitsForDone(CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, { fullPath: fullTestPath("foo.html") })); + }); + runs(function () { + var doc = DocumentManager.getOpenDocumentForPath(fullTestPath("foo.html")), + i; + expect(doc).toBeTruthy(); + + // Replace the second and third foo matches (on two adjacent lines) with a single foo match on a single line. + doc.replaceRange("this is a new foo match\n", {line: 5, ch: 0}, {line: 7, ch: 0}); + + // This should update synchronously. + expect(gotChange).toBe(true); + + var oldFileResults = oldResults[fullTestPath("foo.html")], + newFileResults = FindInFiles.searchModel.results[fullTestPath("foo.html")]; + + // First match should be unchanged. + expect(newFileResults.matches[0]).toEqual(oldFileResults.matches[0]); + + // Second match should be changed to reflect the new position. + expect(newFileResults.matches[1].start).toEqual({line: 5, ch: 14}); + expect(newFileResults.matches[1].end).toEqual({line: 5, ch: 17}); + + // Third match should be deleted. The rest of the matches should have their lines adjusted. + for (i = 2; i < newFileResults.matches.length; i++) { + var newMatch = newFileResults.matches[i], + oldMatch = oldFileResults.matches[i + 1]; + expect(newMatch.start).toEqual({line: oldMatch.start.line - 1, ch: oldMatch.start.ch}); + expect(newMatch.end).toEqual({line: oldMatch.end.line - 1, ch: oldMatch.end.ch}); + } + + // There should be one fewer match. + expect(FindInFiles.searchModel.countFilesMatches()).toEqual({files: 3, matches: 13}); + + // Make sure the model is adding the flag that will make the view debounce changes. + expect(wasQuickChange).toBeTruthy(); + }); + }); + + it("should completely remove the document from the results list if all matches in the document are deleted", function () { + runs(function () { + waitsForDone(CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, { fullPath: fullTestPath("foo.html") })); + }); + runs(function () { + var doc = DocumentManager.getOpenDocumentForPath(fullTestPath("foo.html")), + i; + expect(doc).toBeTruthy(); + + // Replace all matches and check that the entire file was removed from the results list. + doc.replaceRange("this will not match", {line: 4, ch: 0}, {line: 18, ch: 0}); + + // This should update synchronously. + expect(gotChange).toBe(true); + expect(FindInFiles.searchModel.results[fullTestPath("foo.html")]).toBeUndefined(); + + // There should be one fewer file and the matches for that file should be gone. + expect(FindInFiles.searchModel.countFilesMatches()).toEqual({files: 2, matches: 7}); + + // Make sure the model is adding the flag that will make the view debounce changes. + expect(wasQuickChange).toBeTruthy(); + }); + }); + }); + // Unfortunately, we can't easily mock file changes, so we just do them in a copy of the project. + // This set of tests isn't as thorough as it could be, because it's difficult to perform file + // ops that will exercise all possible scenarios of change events (e.g. change events with + // both added and removed files), and conversely it's difficult to mock all the filesystem stuff + // without doing a bunch of work. So this is really just a set of basic sanity tests to make + // sure that stuff being refactored between the change handler and the model doesn't break + // basic update functionality. + describe("when on-disk file or folder changes", function () { + it("should add matches for a new file", function () { + var newFilePath; + runs(function () { + newFilePath = fullTestPath("newfoo.html"); + expect(FindInFiles.searchModel.results[newFilePath]).toBeFalsy(); + waitsForDone(promisify(FileSystem.getFileForPath(newFilePath), "write", "this is a new foo match\n"), "add new file"); + }); + waitsFor(function () { return gotChange; }, "model change event"); + runs(function () { + var newFileResults = FindInFiles.searchModel.results[newFilePath]; + expect(newFileResults).toBeTruthy(); + expect(newFileResults.matches.length).toBe(1); + expect(newFileResults.matches[0].start).toEqual({line: 0, ch: 14}); + expect(newFileResults.matches[0].end).toEqual({line: 0, ch: 17}); + + // There should be one new file and match. + expect(FindInFiles.searchModel.countFilesMatches()).toEqual({files: 4, matches: 15}); + }); + }); + + it("should remove matches for a deleted file", function () { + runs(function () { + expect(FindInFiles.searchModel.results[fullTestPath("foo.html")]).toBeTruthy(); + waitsForDone(promisify(FileSystem.getFileForPath(fullTestPath("foo.html")), "unlink"), "delete file"); + }); + waitsFor(function () { return gotChange; }, "model change event"); + runs(function () { + expect(FindInFiles.searchModel.results[fullTestPath("foo.html")]).toBeFalsy(); + + // There should be one fewer file and the matches should be removed. + expect(FindInFiles.searchModel.countFilesMatches()).toEqual({files: 2, matches: 7}); + }); + }); + + it("should remove matches for a deleted folder", function () { + runs(function () { + expect(FindInFiles.searchModel.results[fullTestPath("css/foo.css")]).toBeTruthy(); + waitsForDone(promisify(FileSystem.getFileForPath(fullTestPath("css")), "unlink"), "delete folder"); + }); + waitsFor(function () { return gotChange; }, "model change event"); + runs(function () { + expect(FindInFiles.searchModel.results[fullTestPath("css/foo.css")]).toBeFalsy(); + + // There should be one fewer file and the matches should be removed. + expect(FindInFiles.searchModel.countFilesMatches()).toEqual({files: 2, matches: 11}); + }); + }); + }); + }); + + describe("Replace", function () { function expectProjectToMatchKnownGood(kgFolder, lineEndings, filesToSkip) { runs(function () { var testRootPath = ProjectManager.getProjectRoot().fullPath, @@ -704,31 +1004,6 @@ define(function (require, exports, module) { }); } - function numMatches(results) { - return _.reduce(_.pluck(results, "matches"), function (sum, matches) { - return sum + matches.length; - }, 0); - } - - function doSearch(options) { - runs(function () { - FindInFiles.doSearchInScope(options.queryInfo, null, null, options.replaceText).done(function (results) { - searchResults = results; - }); - }); - waitsFor(function () { return searchResults; }, 1000, "search completed"); - runs(function () { - expect(numMatches(searchResults)).toBe(options.numMatches); - }); - } - - function doReplace(options) { - return FindInFiles.doReplace(searchResults, options.replaceText, { - forceFilesOpen: options.forceFilesOpen, - isRegexp: options.queryInfo.isRegexp - }); - } - // Does a standard test for files on disk: search, replace, and check that files on disk match. // Options: // knownGoodFolder: name of folder containing known goods to match to project files on disk @@ -824,10 +1099,6 @@ define(function (require, exports, module) { expectInMemoryFiles(options); } - beforeEach(function () { - searchResults = null; - }); - afterEach(function () { runs(function () { waitsForDone(CommandManager.execute(Commands.FILE_CLOSE_ALL, { _forceClose: true }), "close all files"); diff --git a/test/spec/SpecRunnerUtils.js b/test/spec/SpecRunnerUtils.js index 9dc49d32540..ba9cbc0eb0a 100644 --- a/test/spec/SpecRunnerUtils.js +++ b/test/spec/SpecRunnerUtils.js @@ -327,14 +327,13 @@ define(function (require, exports, module) { var dummyFile = _getFileSystem().getFileForPath(filename); var docToShim = new DocumentManager.Document(dummyFile, new Date(), content); - // Prevent adding doc to working set + // Prevent adding doc to working set by not dispatching "dirtyFlagChange". + // TODO: Other functionality here needs to be kept in sync with Document._handleEditorChange(). In the + // future, we should fix things so that we either don't need mock documents or that this + // is factored so it will just run in both. docToShim._handleEditorChange = function (event, editor, changeList) { this.isDirty = !editor._codeMirror.isClean(); - - // TODO: This needs to be kept in sync with Document._handleEditorChange(). In the - // future, we should fix things so that we either don't need mock documents or that this - // is factored so it will just run in both. - $(this).triggerHandler("change", [this, changeList]); + this._notifyDocumentChange(changeList); }; docToShim.notifySaved = function () { throw new Error("Cannot notifySaved() a unit-test dummy Document");
{{{filename}}}
{{line}} {{pre}}{{highlight}}{{post}}