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

Issue 29880577: Issue 6877 - Only parse headers in the first line of the filter list (Closed)

Created:
Sept. 14, 2018, 4:37 p.m. by Vasily Kuznetsov
Modified:
Sept. 19, 2018, 1:30 p.m.
Reviewers:
Sebastian Noack
Visibility:
Public.

Description

Issue 6877 - Only parse headers in the first line of the filter list This is an alternative version of https://codereview.adblockplus.org/29880555/ Based on this review: https://codereview.adblockplus.org/29879650/

Patch Set 1 : Initial #

Total comments: 15

Patch Set 2 : Correct behavior, add comments, improve naming, add tests #

Total comments: 14

Patch Set 3 : Fix header parsing, improve argument naming and documentation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+111 lines, -55 lines) Patch
M abp/filters/parser.py View 1 2 3 chunks +58 lines, -48 lines 0 comments Download
M abp/filters/rpy.py View 1 chunk +4 lines, -2 lines 0 comments Download
M tests/test_parser.py View 1 2 1 chunk +39 lines, -4 lines 0 comments Download
M tests/test_rpy.py View 1 2 2 chunks +10 lines, -1 line 0 comments Download

Messages

Total messages: 8
Vasily Kuznetsov
Here's an implementation of the idea from https://issues.adblockplus.org/ticket/6877#comment:3 The logic is the same as in ...
Sept. 14, 2018, 4:52 p.m. (2018-09-14 16:52:50 UTC) #1
Sebastian Noack
https://codereview.adblockplus.org/29880577/diff/29880583/abp/filters/parser.py File abp/filters/parser.py (left): https://codereview.adblockplus.org/29880577/diff/29880583/abp/filters/parser.py#oldcode279 abp/filters/parser.py:279: content = line_text.strip() Adblock Plus doesn't strip the line ...
Sept. 15, 2018, 4:08 p.m. (2018-09-15 16:08:33 UTC) #2
Vasily Kuznetsov
Hi Sebastian, Thanks for the comments, I will address them. See also some follow up ...
Sept. 17, 2018, 10:40 a.m. (2018-09-17 10:40:27 UTC) #3
Sebastian Noack
On 2018/09/17 10:40:27, Vasily Kuznetsov wrote: > What do you think about the whole approach? ...
Sept. 17, 2018, 6:11 p.m. (2018-09-17 18:11:52 UTC) #4
Vasily Kuznetsov
Hi Sebastian, I've addressed / replied to your comments and added a couple of tests ...
Sept. 18, 2018, 12:41 p.m. (2018-09-18 12:41:15 UTC) #5
Sebastian Noack
https://codereview.adblockplus.org/29880577/diff/29884555/abp/filters/parser.py File abp/filters/parser.py (left): https://codereview.adblockplus.org/29880577/diff/29884555/abp/filters/parser.py#oldcode145 abp/filters/parser.py:145: HEADER_REGEXP = re.compile(r'\[(Adblock(?:\s*Plus\s*[\d\.]+?)?)\]', flags=re.I) Does this regexp is missing ...
Sept. 18, 2018, 3:19 p.m. (2018-09-18 15:19:08 UTC) #6
Vasily Kuznetsov
Hi Sebastian, Thanks for all the comments and for the discussion about header parsing in ...
Sept. 18, 2018, 6:11 p.m. (2018-09-18 18:11:44 UTC) #7
Sebastian Noack
Sept. 18, 2018, 8:53 p.m. (2018-09-18 20:53:36 UTC) #8
LGTM

Powered by Google App Engine
This is Rietveld