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

Don't add empty filter sets to the file filter dropdown list. #7975

Merged
bchintx merged 1 commit into from
May 29, 2014
Merged

Don't add empty filter sets to the file filter dropdown list. #7975

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 27 additions & 17 deletions src/search/FileFilters.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,26 @@ define(function (require, exports, module) {
/** @type {DropdownButton} */
var _picker = null;

/**
* Get the condensed form of the filter set by joining the first two in the set with
* a comma separator and appending a short message with the number of filters being clipped.
* @param {Array.<string>} filter
* @return {string} Condensed form of filter set if `filter` is a valid array.
* Otherwise, return an empty string.
*/
function _getCondensedForm(filter) {
if (!_.isArray(filter)) {
return "";
}
Copy link
Copy Markdown
Contributor

@bchintx bchintx May 29, 2014

Choose a reason for hiding this comment

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

@RaymondLim
Instead of just dropping patterns that aren't defined in the Preferences file as an array, could we allow them by just returning the single pattern?

For example, instead of returning an empty string here, could we just return filter; in the line above? That would allow a user to specify a single pattern (ie. without the array notation) in the Preferences file like this:
"fileFilters": [ { "name": "JS files", "patterns": "*.js" } ]

Copy link
Copy Markdown
Contributor Author

@RaymondLim RaymondLim May 29, 2014

Choose a reason for hiding this comment

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

@bchintx I think we should enforce the consistency of our preferences format and data type. Otherwise, we may end up with complicated code that handles multiple data types. For example, how about multiple filter strings without the []? I know that multiple strings will break the JSON file, but it's possible that someone may mistakenly create a filter set with no enclosing brackets. So instead of handling one extra case, I'd rather enforce the correct data format and data type.

Copy link
Copy Markdown
Contributor

@bchintx bchintx May 29, 2014

Choose a reason for hiding this comment

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

@RaymondLim Ok, sounds fine.


// Format filter in condensed form
if (filter.length > 2) {
return filter.slice(0, 2).join(", ") + " " +
StringUtils.format(Strings.FILE_FILTER_CLIPPED_SUFFIX, filter.length - 2);
}
return filter.join(", ");
}

/**
* Populate the list of dropdown menu with two filter commands and
* the list of saved filter sets.
Expand All @@ -64,6 +84,12 @@ define(function (require, exports, module) {

if (filterSets.length) {
dropdownItems.push("---");

// Remove all the empty exclusion sets before concatenating to the dropdownItems.
filterSets = filterSets.filter(function (filter) {
return (_getCondensedForm(filter.patterns) !== "");
});

// FIRST_FILTER_INDEX needs to stay in sync with the number of static items (plus separator)
// ie. the number of items populated so far before we concatenate with the actual filter sets.
dropdownItems = dropdownItems.concat(filterSets);
Expand All @@ -74,7 +100,7 @@ define(function (require, exports, module) {
/**
* Find the index of a filter set in the list of saved filter sets.
* @param {Array.<{name: string, patterns: Array.<string>}>} filterSets
* @param {{name: string, patterns: Array.<string>}} filter
* @return {{name: string, patterns: Array.<string>}} filter
*/
function _getFilterIndex(filterSets, filter) {
var index = -1,
Expand Down Expand Up @@ -116,22 +142,6 @@ define(function (require, exports, module) {
return activeFilter;
}

/**
* Get the condensed form of the filter set by joining the first two in the set with
* a comma separator and appending a short message with the number of filters being clipped.
* @param {Array.<string>} filter
* @param {string} condensed form of filter set
*/
function _getCondensedForm(filter) {
// Format filter in condensed form
if (filter.length > 2) {
return filter.slice(0, 2).join(", ") + " " +
StringUtils.format(Strings.FILE_FILTER_CLIPPED_SUFFIX, filter.length - 2);
} else {
return filter.join(", ");
}
}

/**
* Update the picker button label with the name/patterns of the selected filter or
* No Files Excluded if no filter is selected.
Expand Down
⚡