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

Find in subset of all files #2084

Merged
merged 5 commits into from
Nov 14, 2012
Merged

Find in subset of all files #2084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1a4145b
Initial impl of 'Find in Subtree' context menu item: 'Find in Files' …
peterflynn Oct 25, 2012
ec62c30
Change scoped 'Find in Files' so scope can be a single file, not just a
peterflynn Nov 8, 2012
8deee96
Merge remote-tracking branch 'origin/master' into pflynn/find-in-file…
peterflynn Nov 8, 2012
78e1034
Code review: change Find in Files label to only say "in" once, on the…
peterflynn Nov 13, 2012
a91c7bb
Code review: support "Find in..." from working set context menu also.
peterflynn Nov 14, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/command/Commands.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ define(function (require, exports, module) {
exports.EDIT_SELECT_LINE = "edit.selectLine";
exports.EDIT_FIND = "edit.find";
exports.EDIT_FIND_IN_FILES = "edit.findInFiles";
exports.EDIT_FIND_IN_SUBTREE = "edit.findInSubtree";
exports.EDIT_FIND_NEXT = "edit.findNext";
exports.EDIT_FIND_PREVIOUS = "edit.findPrevious";
exports.EDIT_REPLACE = "edit.replace";
Expand Down
4 changes: 4 additions & 0 deletions src/command/Menus.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,13 +993,17 @@ define(function (require, exports, module) {
project_cmenu.addMenuItem(Commands.FILE_NEW);
project_cmenu.addMenuItem(Commands.FILE_NEW_FOLDER);
project_cmenu.addMenuItem(Commands.FILE_RENAME, "F2");
project_cmenu.addMenuDivider();
project_cmenu.addMenuItem(Commands.EDIT_FIND_IN_SUBTREE);

var working_set_cmenu = registerContextMenu(ContextMenuIds.WORKING_SET_MENU);
Copy link
Member

Choose a reason for hiding this comment

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

Should the command be added to the working set menu? It should work there.

working_set_cmenu.addMenuItem(Commands.FILE_CLOSE);
working_set_cmenu.addMenuItem(Commands.FILE_SAVE);
working_set_cmenu.addMenuItem(Commands.FILE_RENAME);
working_set_cmenu.addMenuItem(Commands.NAVIGATE_SHOW_IN_FILE_TREE);
working_set_cmenu.addMenuDivider();
working_set_cmenu.addMenuItem(Commands.EDIT_FIND_IN_SUBTREE);
working_set_cmenu.addMenuDivider();
working_set_cmenu.addMenuItem(Commands.SORT_WORKINGSET_BY_ADDED);
working_set_cmenu.addMenuItem(Commands.SORT_WORKINGSET_BY_NAME);
working_set_cmenu.addMenuItem(Commands.SORT_WORKINGSET_BY_TYPE);
Expand Down
7 changes: 5 additions & 2 deletions src/nls/root/strings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,16 @@ define({
"NO_UPDATE_TITLE" : "You're up to date!",
"NO_UPDATE_MESSAGE" : "You are running the latest version of {APP_NAME}.",

"FIND_IN_FILES_TITLE" : "for \"{4}\" - {0} {1} in {2} {3}",
"FIND_IN_FILES_TITLE" : "for \"{4}\" {5} - {0} {1} in {2} {3}",
"FIND_IN_FILES_SCOPED" : "in <span class='dialog-filename'>{0}</span>",
"FIND_IN_FILES_NO_SCOPE" : "in project",
"FIND_IN_FILES_FILE" : "file",
"FIND_IN_FILES_FILES" : "files",
"FIND_IN_FILES_MATCH" : "match",
"FIND_IN_FILES_MATCHES" : "matches",
"FIND_IN_FILES_MORE_THAN" : "More than ",
"FIND_IN_FILES_MAX" : " (showing the first {0} matches)",
"FIND_IN_FILES_FILE_PATH" : "File: <b>{0}</b>",
"FIND_IN_FILES_FILE_PATH" : "File: <span class='dialog-filename'>{0}</span>",
"FIND_IN_FILES_LINE" : "line:&nbsp;{0}",

"ERROR_FETCHING_UPDATE_INFO_TITLE" : "Error getting update info",
Expand Down Expand Up @@ -178,6 +180,7 @@ define({
"CMD_SELECT_LINE" : "Select Line",
"CMD_FIND" : "Find",
"CMD_FIND_IN_FILES" : "Find in Files",
"CMD_FIND_IN_SUBTREE" : "Find in...",
"CMD_FIND_NEXT" : "Find Next",
"CMD_FIND_PREVIOUS" : "Find Previous",
"CMD_REPLACE" : "Replace",
Expand Down
2 changes: 2 additions & 0 deletions src/project/ProjectManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,8 @@ define(function (require, exports, module) {
* If absPath lies within the project, returns a project-relative path. Else returns absPath
* unmodified.
* Does not support paths containing ".."
* @param {!string} absPath
* @return {!string}
*/
function makeProjectRelativeIfPossible(absPath) {
if (isWithinProject(absPath)) {
Expand Down
129 changes: 93 additions & 36 deletions src/search/FindInFiles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ define(function (require, exports, module) {
Commands = require("command/Commands"),
Strings = require("strings"),
StringUtils = require("utils/StringUtils"),
ProjectManager = require("project/ProjectManager"),
DocumentManager = require("document/DocumentManager"),
EditorManager = require("editor/EditorManager"),
FileIndexManager = require("project/FileIndexManager"),
Expand Down Expand Up @@ -86,6 +87,21 @@ define(function (require, exports, module) {
return new RegExp(query, "gi");
}

/**
* Returns label text to indicate the search scope. Already HTML-escaped.
* @param {?Entry} scope
*/
function _labelForScope(scope) {
var projName = ProjectManager.getProjectRoot().name;
if (scope) {
var displayPath = StringUtils.htmlEscape(ProjectManager.makeProjectRelativeIfPossible(scope.fullPath));
return StringUtils.format(Strings.FIND_IN_FILES_SCOPED, displayPath);
} else {
return Strings.FIND_IN_FILES_NO_SCOPE;
}
}


// This dialog class was mostly copied from QuickOpen. We should have a common dialog
// class that everyone can use.

Expand Down Expand Up @@ -126,12 +142,14 @@ define(function (require, exports, module) {
/**
* Shows the search dialog
* @param {?string} initialString Default text to prepopulate the search field with
* @param {?Entry} scope Search scope, or null to search whole proj
* @returns {$.Promise} that is resolved with the string to search for
*/
FindInFilesDialog.prototype.showDialog = function (initialString) {
var dialogHTML = Strings.CMD_FIND_IN_FILES +
": <input type='text' id='findInFilesInput' style='width: 10em'> <span style='color: #888'>(" +
Strings.SEARCH_REGEXP_INFO + ")</span>";
FindInFilesDialog.prototype.showDialog = function (initialString, scope) {
// Note the prefix label is a simple "Find:" - the "in ..." part comes after the text field
var dialogHTML = Strings.CMD_FIND +
": <input type='text' id='findInFilesInput' style='width: 10em'> <span id='findInFilesScope'></span> &nbsp;" +
"<span style='color: #888'>(" + Strings.SEARCH_REGEXP_INFO + ")</span>";
Copy link
Member

Choose a reason for hiding this comment

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

I have a suggestion for simplifying this dialog a bit. As implemented here, when searching all files, the dialog looks like:

Find in Files: [__________] in all of brackets (use /re/ syntax for regexp search)

When doing a scoped search, the dialog looks like:

Find in Files: [__________] in brackets/src (use /re/ syntax for regexp search)

As an alternative, how about adding the scope to the prompt label instead. Global searches would look like:

Find in project: [___________] (use /re/ syntax for regexp search)

Scoped searches would look like:

Find in src/brackets: [_________] (use /re/ syntax for regexp search)

Bolding the scope folder/file will help draw attention to the fact that this is a scoped search.

Copy link
Member Author

Choose a reason for hiding this comment

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

My concern with putting the folder name on the left is it would make the horizontal position of the text field vary quite a bit, perhaps unpredictably so.

Your other proposed changes would still work if we left the label on the right, though...

My only other hesitation is that we've avoided the term "project" up until recently (pretty sure that's why I didn't use it when I originally wrote this). But we started using it fairly prominently in the UI last sprint, so I think that change would be ok now.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I was hesitant to use "project" at first, but we are using it in the UI, and it does sound better (and is more accurate) than "all files". The problem with using the project name here is we will eventually want to search the working set, too, and that may contain files that aren't in the main project directory.

My concern with putting the folder name on the left is it would make the horizontal position of the text field vary quite a bit, perhaps unpredictably so.

Good point. It wouldn't be a problem for shallow paths, but could be a problem with long document names and/or paths. How about using just "Find:" on the left and adding the path (or "project") on the right.

this.result = new $.Deferred();
this._createDialogDiv(dialogHTML);
var $searchField = $("input#findInFilesInput");
Expand All @@ -140,6 +158,8 @@ define(function (require, exports, module) {
$searchField.attr("value", initialString || "");
$searchField.get(0).select();

$("#findInFilesScope").html(_labelForScope(scope));

$searchField.bind("keydown", function (event) {
if (event.keyCode === KeyEvent.DOM_VK_RETURN || event.keyCode === KeyEvent.DOM_VK_ESCAPE) { // Enter/Return key or Esc key
event.stopPropagation();
Expand Down Expand Up @@ -209,7 +229,7 @@ define(function (require, exports, module) {
return matches;
}

function _showSearchResults(searchResults, query) {
function _showSearchResults(searchResults, query, scope) {
var $searchResultsDiv = $("#search-results");

if (searchResults && searchResults.length) {
Expand All @@ -229,17 +249,19 @@ define(function (require, exports, module) {
}
numMatchesStr += String(numMatches);

// This text contains some formatting, so all the strings are assumed to be already escaped
var summary = StringUtils.format(
Strings.FIND_IN_FILES_TITLE,
numMatchesStr,
(numMatches > 1) ? Strings.FIND_IN_FILES_MATCHES : Strings.FIND_IN_FILES_MATCH,
searchResults.length,
(searchResults.length > 1 ? Strings.FIND_IN_FILES_FILES : Strings.FIND_IN_FILES_FILE),
query
query,
scope ? _labelForScope(scope) : ""
);

$("#search-result-summary")
.text(summary +
.html(summary +
(numMatches > FIND_IN_FILES_MAX ? StringUtils.format(Strings.FIND_IN_FILES_MAX, FIND_IN_FILES_MAX) : ""))
.prepend("&nbsp;"); // putting a normal space before the "-" is not enough

Expand All @@ -251,19 +273,16 @@ define(function (require, exports, module) {
return $("<td/>").html(content);
};

var esc = function (str) {
str = str.replace(/</g, "&lt;");
str = str.replace(/>/g, "&gt;");
return str;
};
// shorthand function name
var esc = StringUtils.htmlEscape;

var highlightMatch = function (line, start, end) {
return esc(line.substr(0, start)) + "<span class='highlight'>" + esc(line.substring(start, end)) + "</span>" + esc(line.substr(end));
};

// Add row for file name
$("<tr class='file-section' />")
.append("<td colspan='3'>" + StringUtils.format(Strings.FIND_IN_FILES_FILE_PATH, StringUtils.breakableUrl(item.fullPath)) + "</td>")
.append("<td colspan='3'>" + StringUtils.format(Strings.FIND_IN_FILES_FILE_PATH, StringUtils.breakableUrl(esc(item.fullPath))) + "</td>")
.click(function () {
// Clicking file section header collapses/expands result rows for that file
var $fileHeader = $(this);
Expand Down Expand Up @@ -313,11 +332,30 @@ define(function (require, exports, module) {
EditorManager.resizeEditor();
}

/**
* @param {!FileInfo} fileInfo File in question
* @param {?Entry} scope Search scope, or null if whole project
* @return {boolean}
*/
function inScope(fileInfo, scope) {
if (scope) {
if (scope.isDirectory) {
// Dirs always have trailing slash, so we don't have to worry about being
// a substring of another dir name
return fileInfo.fullPath.indexOf(scope.fullPath) === 0;
} else {
return fileInfo.fullPath === scope.fullPath;
}
}
return true;
}

/**
* Displays a non-modal embedded dialog above the code mirror editor that allows the user to do
* a find operation across all files in the project.
* @param {?Entry} scope Project file/subfolder to search within; else searches whole project.
*/
function doFindInFiles() {
function doFindInFiles(scope) {

var dialog = new FindInFilesDialog();

Expand All @@ -328,7 +366,7 @@ define(function (require, exports, module) {
searchResults = [];
maxHitsFoundInFile = false;

dialog.showDialog(initialString)
dialog.showDialog(initialString, scope)
.done(function (query) {
if (query) {
var queryExpr = _getQueryRegExp(query);
Expand All @@ -341,28 +379,33 @@ define(function (require, exports, module) {
Async.doInParallel(fileListResult, function (fileInfo) {
var result = new $.Deferred();

DocumentManager.getDocumentForPath(fileInfo.fullPath)
.done(function (doc) {
var matches = _getSearchMatches(doc.getText(), queryExpr);

if (matches && matches.length) {
searchResults.push({
fullPath: fileInfo.fullPath,
matches: matches
});
}
result.resolve();
})
.fail(function (error) {
// Error reading this file. This is most likely because the file isn't a text file.
// Resolve here so we move on to the next file.
result.resolve();
});

if (!inScope(fileInfo, scope)) {
result.resolve();
} else {
// Search one file
DocumentManager.getDocumentForPath(fileInfo.fullPath)
.done(function (doc) {
var matches = _getSearchMatches(doc.getText(), queryExpr);

if (matches && matches.length) {
searchResults.push({
fullPath: fileInfo.fullPath,
matches: matches
});
}
result.resolve();
})
.fail(function (error) {
// Error reading this file. This is most likely because the file isn't a text file.
// Resolve here so we move on to the next file.
result.resolve();
});
}
return result.promise();
})
.done(function () {
_showSearchResults(searchResults, query);
// Done searching all files: show results
_showSearchResults(searchResults, query, scope);
StatusBar.hideBusyIndicator();
})
.fail(function () {
Expand All @@ -374,11 +417,23 @@ define(function (require, exports, module) {
});
}

/** Search within the file/subtree defined by the sidebar selection */
function doFindInSubtree() {
// Prefer project tree selection, else use working set selection
var selectedEntry = ProjectManager.getSelectedItem();
if (!selectedEntry) {
var doc = DocumentManager.getCurrentDocument();
selectedEntry = (doc && doc.file);
}

doFindInFiles(selectedEntry);
}


// Initialize items dependent on HTML DOM
AppInit.htmlReady(function () {
var $searchResults = $("#search-results"),
$searchContent = $("#search-results .table-container");

});

function _fileNameChangeHandler(event, oldName, newName) {
Expand All @@ -392,5 +447,7 @@ define(function (require, exports, module) {
}

$(DocumentManager).on("fileNameChange", _fileNameChangeHandler);
CommandManager.register(Strings.CMD_FIND_IN_FILES, Commands.EDIT_FIND_IN_FILES, doFindInFiles);

CommandManager.register(Strings.CMD_FIND_IN_FILES, Commands.EDIT_FIND_IN_FILES, doFindInFiles);
CommandManager.register(Strings.CMD_FIND_IN_SUBTREE, Commands.EDIT_FIND_IN_SUBTREE, doFindInSubtree);
});