Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(584)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 days, 1 hour ago by Vasily Kuznetsov
Modified:
5 hours, 1 minute ago
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 ...
5 days, 1 hour ago (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 ...
4 days, 2 hours ago (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 ...
2 days, 7 hours ago (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? ...
2 days ago (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 ...
1 day, 5 hours ago (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 ...
1 day, 3 hours ago (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 ...
1 day ago (2018-09-18 18:11:44 UTC) #7
Sebastian Noack
21 hours, 38 minutes ago (2018-09-18 20:53:36 UTC) #8
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5