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

File exclusions usability improvements (bug #7149) #7400

Merged
merged 2 commits into from
Apr 5, 2014
Merged

File exclusions usability improvements (bug #7149) #7400

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04b8718
Fix several issues raised in #7149:
peterflynn Apr 3, 2014
6118786
Code review: restyle filter editor file count and make UI element widths
peterflynn Apr 4, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/nls/root/strings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ define({
"FIND_IN_FILES_TITLE_PART3" : "— {0} {1} {2} in {3} {4}",
"FIND_IN_FILES_SCOPED" : "in <span class='dialog-filename'>{0}</span>",
"FIND_IN_FILES_NO_SCOPE" : "in project",
"FIND_IN_FILES_ZERO_FILES" : "Filter excludes all files {0}",
"FIND_IN_FILES_FILE" : "file",
"FIND_IN_FILES_FILES" : "files",
"FIND_IN_FILES_MATCH" : "match",
Expand All @@ -175,9 +176,12 @@ define({
"NO_FILE_FILTER" : "Exclude files\u2026",
"EDIT_FILE_FILTER" : "Edit\u2026",
"FILE_FILTER_DIALOG" : "Edit Filter",
"FILE_FILTER_INSTRUCTIONS" : "Exclude files and folders matching any of the following strings / substrings or <a href='{0}' title='{0}'>globs</a>. Enter each string on a new line.",
"FILE_FILTER_INSTRUCTIONS" : "Exclude files and folders matching any of the following strings / substrings or <a href='{0}' title='{0}'>wildcards</a>. Enter each string on a new line.",
Copy link
Member Author

Choose a reason for hiding this comment

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

We changed from "globs" to "wildcards" in last sprint's update notification & the wiki documentation, but didn't update the UI to match. This syncs it up with the preferred terminology.

"FILE_FILTER_LIST_PREFIX" : "except",
"FILE_FILTER_CLIPPED_SUFFIX" : "and {0} more",
"FILTER_COUNTING_FILES" : "Counting files\u2026",
"FILTER_FILE_COUNT" : "Allows {0} of {1} files {2}",
"FILTER_FILE_COUNT_ALL" : "Allows all {0} files {1}",

// Quick Edit
"ERROR_QUICK_EDIT_PROVIDER_NOT_FOUND" : "No Quick Edit provider found for current cursor position",
Expand Down
184 changes: 107 additions & 77 deletions src/search/FileFilters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ define(function (require, exports, module) {
"use strict";

var _ = require("thirdparty/lodash"),
ProjectManager = require("project/ProjectManager"),
DefaultDialogs = require("widgets/DefaultDialogs"),
Dialogs = require("widgets/Dialogs"),
DropdownButton = require("widgets/DropdownButton").DropdownButton,
Expand Down Expand Up @@ -59,44 +60,6 @@ define(function (require, exports, module) {
}


/**
* Opens a dialog box to edit the given filter. When editing is finished, the value of getLastFilter() changes to
* reflect the edits. If the dialog was canceled, the preference is left unchanged.
* @param {!Array.<string>} filter
* @return {!$.Promise} Dialog box promise
*/
function editFilter(filter) {
var lastFocus = window.document.activeElement;

var html = StringUtils.format(Strings.FILE_FILTER_INSTRUCTIONS, brackets.config.glob_help_url) +
"<textarea class='exclusions-editor'></textarea>";
var buttons = [
{ className : Dialogs.DIALOG_BTN_CLASS_NORMAL, id: Dialogs.DIALOG_BTN_CANCEL, text: Strings.CANCEL },
{ className : Dialogs.DIALOG_BTN_CLASS_PRIMARY, id: Dialogs.DIALOG_BTN_OK, text: Strings.OK }
];
var dialog = Dialogs.showModalDialog(DefaultDialogs.DIALOG_ID_INFO, Strings.FILE_FILTER_DIALOG, html, buttons);

dialog.getElement().find(".exclusions-editor").val(filter.join("\n")).focus();

dialog.done(function (buttonId) {
if (buttonId === Dialogs.DIALOG_BTN_OK) {
var newFilter = dialog.getElement().find(".exclusions-editor").val().split("\n");

// Remove blank lines
newFilter = newFilter.filter(function (glob) {
return glob.trim().length;
});

// Update saved filter preference
setLastFilter(newFilter);
}
lastFocus.focus(); // restore focus to old pos
});

return dialog.getPromise();
}


/**
* Converts a user-specified filter object (as chosen in picker or retrieved from getFilters()) to a 'compiled' form
* that can be used with filterPath()/filterFileList().
Expand Down Expand Up @@ -153,6 +116,109 @@ define(function (require, exports, module) {
}


/**
* Returns false if the given path matches any of the exclusion globs in the given filter. Returns true
* if the path does not match any of the globs. If filtering many paths at once, use filterFileList()
* for much better performance.
*
* @param {!string} compiledFilter 'Compiled' filter object as returned by compile()
* @param {!string} fullPath
* @return {boolean}
*/
function filterPath(compiledFilter, fullPath) {
if (!compiledFilter) {
return true;
}

var re = new RegExp(compiledFilter);
return !fullPath.match(re);
}

/**
* Returns a copy of 'files' filtered to just those that don't match any of the exclusion globs in the filter.
*
* @param {!string} compiledFilter 'Compiled' filter object as returned by compile()
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: if {!string} means that it can never be null, why do we if (!compiledFilter) in the first line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, will fix the docs

* @param {!Array.<File>} files
* @return {!Array.<File>}
*/
function filterFileList(compiledFilter, files) {
if (!compiledFilter) {
return files;
}

var re = new RegExp(compiledFilter);
return files.filter(function (f) {
return !f.fullPath.match(re);
});
}
Copy link
Member Author

Choose a reason for hiding this comment

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

filterPath() and filterFileList() were simply moved higher up in the file, with zero changes to the actual code (verified by diffing the text)



/**
* Opens a dialog box to edit the given filter. When editing is finished, the value of getLastFilter() changes to
* reflect the edits. If the dialog was canceled, the preference is left unchanged.
* @param {!Array.<string>} filter
* @param {?{label:string, promise:$.Promise}} context Info on which files the filter will be applied to. If specified,
* editing UI will indicate how many files are excluded by the filter. Label should be of the form "in ..."
* @return {!$.Promise} Dialog box promise
*/
function editFilter(filter, context) {
var lastFocus = window.document.activeElement;

var html = StringUtils.format(Strings.FILE_FILTER_INSTRUCTIONS, brackets.config.glob_help_url) +
"<textarea class='exclusions-editor'></textarea><div class='exclusions-filecount'>" + Strings.FILTER_COUNTING_FILES + "</div>";
var buttons = [
{ className : Dialogs.DIALOG_BTN_CLASS_NORMAL, id: Dialogs.DIALOG_BTN_CANCEL, text: Strings.CANCEL },
{ className : Dialogs.DIALOG_BTN_CLASS_PRIMARY, id: Dialogs.DIALOG_BTN_OK, text: Strings.OK }
];
var dialog = Dialogs.showModalDialog(DefaultDialogs.DIALOG_ID_INFO, Strings.FILE_FILTER_DIALOG, html, buttons);

var $editField = dialog.getElement().find(".exclusions-editor");
$editField.val(filter.join("\n")).focus();

function getValue() {
var newFilter = $editField.val().split("\n");

// Remove blank lines
return newFilter.filter(function (glob) {
return glob.trim().length;
});
}

dialog.done(function (buttonId) {
if (buttonId === Dialogs.DIALOG_BTN_OK) {
// Update saved filter preference
setLastFilter(getValue());
}
lastFocus.focus(); // restore focus to old pos
});


// Code to update the file count readout at bottom of dialog (if context provided)
var $fileCount = dialog.getElement().find(".exclusions-filecount");

function updateFileCount() {
context.promise.done(function (files) {
var filter = getValue();
if (filter.length) {
var filtered = filterFileList(compile(getValue()), files);
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of calling getValue() filter could be used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will fix

$fileCount.html(StringUtils.format(Strings.FILTER_FILE_COUNT, filtered.length, files.length, context.label));
} else {
$fileCount.html(StringUtils.format(Strings.FILTER_FILE_COUNT_ALL, files.length, context.label));
}
});
}

if (context) {
$editField.on("input", _.debounce(updateFileCount, 400));
updateFileCount();
} else {
$fileCount.hide();
}

return dialog.getPromise();
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Since this block moved too, it's a little hard to spot which parts were changed. In case it's useful, here's a diff of just the the text within the editFilter() block:
editfilter diff



/**
* Marks the filter picker's currently selected item as most-recently used, and returns the corresponding
* 'compiled' filter object ready for use with filterPath().
Expand All @@ -170,9 +236,10 @@ define(function (require, exports, module) {
* client should call commitDropdown() when the UI containing the filter picker is confirmed (which updates the MRU
* order) and then use the returned filter object as needed.
*
* @param {?{label:string, promise:$.Promise}} context Info on files filter will apply to - see editFilter()
* @return {!jQueryObject} Picker UI. To retrieve the selected value, use commitPicker().
*/
function createFilterPicker() {
function createFilterPicker(contextPromise) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rename contextPromise to something that fits better?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, that was supposed to be named context like the other places in the code. Will fix.

var $picker = $("<div class='filter-picker'><span class='filter-label'></span><button class='btn no-focus'></button></div>"),
$button = $picker.find("button");

Expand Down Expand Up @@ -208,7 +275,7 @@ define(function (require, exports, module) {
updatePicker();

$button.click(function () {
editFilter(getLastFilter())
editFilter(getLastFilter(), contextPromise)
.done(function (buttonId) {
if (buttonId === Dialogs.DIALOG_BTN_OK) {
updatePicker();
Expand All @@ -220,43 +287,6 @@ define(function (require, exports, module) {
}


/**
* Returns false if the given path matches any of the exclusion globs in the given filter. Returns true
* if the path does not match any of the globs. If filtering many paths at once, use filterFileList()
* for much better performance.
*
* @param {!string} compiledFilter 'Compiled' filter object as returned by compile()
* @param {!string} fullPath
* @return {boolean}
*/
function filterPath(compiledFilter, fullPath) {
if (!compiledFilter) {
return true;
}

var re = new RegExp(compiledFilter);
return !fullPath.match(re);
}

/**
* Returns a copy of 'files' filtered to just those that don't match any of the exclusion globs in the filter.
*
* @param {!string} compiledFilter 'Compiled' filter object as returned by compile()
* @param {!Array.<File>} files
* @return {!Array.<File>}
*/
function filterFileList(compiledFilter, files) {
if (!compiledFilter) {
return files;
}

var re = new RegExp(compiledFilter);
return files.filter(function (f) {
return !f.fullPath.match(re);
});
}


exports.createFilterPicker = createFilterPicker;
exports.commitPicker = commitPicker;
exports.getLastFilter = getLastFilter;
Expand Down
Loading