Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/filterValidation.js

Issue 4515985834901504: Issue 2664 - Remove ignoreHeaders argument from parseFilter(s) and move the logic to the UI (Closed)
Patch Set: Exclude filter list header errors using .forEach() Created June 8, 2015, 6:36 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « background.js ('k') | options.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/filterValidation.js
===================================================================
--- a/lib/filterValidation.js
+++ b/lib/filterValidation.js
@@ -19,6 +19,63 @@
let {Filter, InvalidFilter, ElemHideBase} = require("filterClasses");
+/**
+ * An error returned by
+ * {@link module:filterValidation.parseFilter parseFilter()} or
+ * {@link module:filterValidation.parseFilters parseFilters()}
+ * indicating that a given filter cannot be parsed,
+ * contains an invalid CSS selector or is a filter list header.
+ *
+ * @constructor
+ */
+function FilterParsingError(type, details)
+{
+ /**
+ * Indicates why the filter is rejected. Possible choices:
+ * "invalid-filter", "invalid-css-selector", "unexpected-filter-list-header"
+ *
+ * @type {string}
+ */
+ this.type = type;
+
+ if (details)
+ {
+ if ("reason" in details)
+ this.reason = details.reason;
+ if ("selector" in details)
+ this.selector = details.selector;
+ }
+}
+FilterParsingError.prototype = {
+ /**
+ * The line number the error occurred on if
+ * {@link module:filterValidation.parseFilters parseFilters()}
+ * were used. Or null if the error was returned by
+ * {@link module:filterValidation.parseFilter parseFilter()}.
+ *
+ * @type {?number}
+ */
+ lineno: null,
+
+ /**
+ * Returns a detailed translated error message.
+ *
+ * @return {string}
+ */
+ toString: function()
+ {
+ let message = this.reason || ext.i18n.getMessage(
+ this.type.replace(/-/g, "_"),
+ "selector" in this ? "'" + this.selector + "'" : null
+ );
+
+ if (this.lineno)
+ message = ext.i18n.getMessage("line", this.lineno.toLocaleString()) + ": " + message;
+
+ return message;
+ }
+};
+
function isValidCSSSelector(selector)
{
let style = document.createElement("style");
@@ -40,10 +97,10 @@
/**
* @typedef ParsedFilter
- * @property {?Filter} [filter] The parsed filter if it is valid. Or null if
- * the given string is empty or a filter list header.
- * @property {string} [error] An error message indicated that the filter cannot
- * be parsed or contains an invalid CSS selector.
+ * @property {?Filter} [filter] The parsed filter if it is valid.
+ * Or null if the given string is empty.
+ * @property {FilterParsingError} [error] See {@link module:filterValidation~FilterParsingError FilterParsingError}
+ *
*/
let parseFilter =
@@ -51,32 +108,25 @@
* Parses and validates a filter given by the user.
*
* @param {string} text
- * @param {Boolean} [ignoreHeaders=false] If true, {filter: null} is
- returned instead an error
- for filter list headers.
* @return {ParsedFilter}
*/
-exports.parseFilter = function(text, ignoreHeaders)
+exports.parseFilter = function(text)
{
let filter = null;
text = Filter.normalize(text);
if (text)
{
- if (text[0] != "[")
- {
- filter = Filter.fromText(text);
+ if (text[0] == "[")
+ return {error: new FilterParsingError("unexpected-filter-list-header")};
- if (filter instanceof InvalidFilter)
- return {error: filter.reason};
+ filter = Filter.fromText(text);
- if (filter instanceof ElemHideBase && !isValidCSSSelector(filter.selector))
- return {error: ext.i18n.getMessage("invalid_css_selector", "'" + filter.selector + "'")};
- }
- else if (!ignoreHeaders)
- {
- return {error: ext.i18n.getMessage("unexpected_filter_list_header")};
- }
+ if (filter instanceof InvalidFilter)
+ return {error: new FilterParsingError("invalid-filter", {reason: filter.reason})};
+
+ if (filter instanceof ElemHideBase && !isValidCSSSelector(filter.selector))
+ return {error: new FilterParsingError("invalid-css-selector", {selector: filter.selector})};
}
return {filter: filter};
@@ -84,35 +134,35 @@
/**
* @typedef ParsedFilters
- * @property {Filter[]} [filters] The parsed filters if all of them are valid.
- * @property {string} [error] An error message indicated that any filter cannot
- * be parsed or contains an invalid CSS selector.
+ * @property {Filter[]} filters The parsed result without invalid filters.
+ * @property {FilterParsingError[]} errors See {@link module:filterValidation~FilterParsingError FilterParsingError}
*/
/**
* Parses and validates a newline-separated list of filters given by the user.
*
* @param {string} text
- * @param {Boolean} [ignoreHeaders=false] If true, filter list headers
- * will be stripped instead of
- * returning an error.
* @return {ParsedFilters}
*/
-exports.parseFilters = function(text, ignoreHeaders)
+exports.parseFilters = function(text)
{
let lines = text.split("\n");
let filters = [];
+ let errors = [];
for (let i = 0; i < lines.length; i++)
{
- let {filter, error} = parseFilter(lines[i], ignoreHeaders);
-
- if (error)
- return {error: ext.i18n.getMessage("line", (i + 1).toString()) + ": " + error};
+ let {filter, error} = parseFilter(lines[i]);
if (filter)
filters.push(filter);
+
+ if (error)
+ {
+ error.lineno = i + 1;
+ errors.push(error);
+ }
}
- return {filters: filters};
+ return {filters: filters, errors: errors};
};
« no previous file with comments | « background.js ('k') | options.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld