Issue 2664 - Remove ignoreHeaders argument from parseFilter(s) and move the logic to the UI

Issue 2664 - Remove ignoreHeaders argument from parseFilter(s) and move the logic to the UI

[Wladimir Palant, 2015-06-08 18:15:46]

http://codereview.adblockplus.org/4515985834901504/diff/5757334940811264/opti...
File options.js (right):

http://codereview.adblockplus.org/4515985834901504/diff/5757334940811264/opti...
options.js:548: }
Why not:


  var errors = result.errors.filter(function(e)
  {
    return e.type != "unexpected-filter-list-header"
  });

http://codereview.adblockplus.org/4515985834901504/diff/5757334940811264/quni...
File qunit/tests/filterValidation.js (right):

http://codereview.adblockplus.org/4515985834901504/diff/5757334940811264/quni...
qunit/tests/filterValidation.js:19:
ok(/\b4\b/.test(parseFilters("!comment\r\n||example.com^\n\n##/").errors[0]),
"error contains corresponding line number");
Maybe test for line number field here instead of parsing the text message? Now
we can do that.

[Sebastian Noack, 2015-06-08 18:36:46]

http://codereview.adblockplus.org/4515985834901504/diff/5757334940811264/opti...
File options.js (right):

http://codereview.adblockplus.org/4515985834901504/diff/5757334940811264/opti...
options.js:548: }
On 2015/06/08 18:15:47, Wladimir Palant wrote:
> Why not:
> 
> 
>   var errors = result.errors.filter(function(e)
>   {
>     return e.type != "unexpected-filter-list-header"
>   });

Done.

http://codereview.adblockplus.org/4515985834901504/diff/5757334940811264/quni...
File qunit/tests/filterValidation.js (right):

http://codereview.adblockplus.org/4515985834901504/diff/5757334940811264/quni...
qunit/tests/filterValidation.js:19:
ok(/\b4\b/.test(parseFilters("!comment\r\n||example.com^\n\n##/").errors[0]),
"error contains corresponding line number");
On 2015/06/08 18:15:47, Wladimir Palant wrote:
> Maybe test for line number field here instead of parsing the text message? Now
> we can do that.

I considered that. But then decided to keep it as it is to  also test the code
path adding the line number to the error message. However, I wouldn't mind
changing it if you disagree.

[Wladimir Palant, 2015-06-08 19:14:16]

LGTM

http://codereview.adblockplus.org/4515985834901504/diff/5757334940811264/quni...
File qunit/tests/filterValidation.js (right):

http://codereview.adblockplus.org/4515985834901504/diff/5757334940811264/quni...
qunit/tests/filterValidation.js:19:
ok(/\b4\b/.test(parseFilters("!comment\r\n||example.com^\n\n##/").errors[0]),
"error contains corresponding line number");
On 2015/06/08 18:36:46, Sebastian Noack wrote:
> I considered that. But then decided to keep it as it is to  also test the code
> path adding the line number to the error message. However, I wouldn't mind
> changing it if you disagree.

Well, I'm not going to insist - I merely dislike parsing localizable strings.