Issue 6945 - Add script to make filter list diffs

abp/filters/diff_script.py

Index: abp/filters/diff_script.py
===================================================================
new file mode 100644
--- /dev/null
+++ b/abp/filters/diff_script.py
@@ -0,0 +1,62 @@
+# This file is part of Adblock Plus <https://adblockplus.org/>,
+# Copyright (C) 2006-present eyeo GmbH
+#
+# Adblock Plus is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3 as
+# published by the Free Software Foundation.
+#
+# Adblock Plus is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Command line script for rendering Adblock Plus filter list diffs."""
+
+import argparse
+import io
+import sys
+
+from collections import namedtuple
+
+from .sources import NotFound
+from .renderer import render_diff
+__all__ = ['main']
+
+
+def parse_args():
+    parser = argparse.ArgumentParser(description='Render a filter list diff.')
+    parser.add_argument(
+        'base', help='the older filter list that needs to be updated',
+        default='-', nargs='?')

[Vasily Kuznetsov, 2018/09/19 18:40:04]

It seems that base and latest should be mandatory arguments (without defaults).

[rhowell, 2018/09/21 08:37:09]

Done.

+    parser.add_argument(
+        'latest', help='the most recent version of the filter list',
+        default='-', nargs='?')
+    parser.add_argument(
+        'outfile', help='output file for filter list diff',
+        default='-', nargs='?')
+    return parser.parse_args()
+
+
+def main(args_in=None):
+    """Entry point for the diff rendering script (diffrender)."""
+    if args_in:
+        Parse = namedtuple('Test', 'base, latest, outfile')

[Vasily Kuznetsov, 2018/09/19 18:40:04]

Are we going to use this code path? If yes, I would recommend to expect a list
of arguments and then pass it to `parser.parse_args()` (it takes a parameter
that defaults to `sys.args`) rather than emulating parsed arguments.

[rhowell, 2018/09/21 08:37:08]

Good point. This was helpful for me while testing, but I don't know any case
where it's useful in the real world. I'll remove it.

+        args = Parse(args_in['base'], args_in['latest'], args_in['outfile'])
+    else:
+        args = parse_args()
+
+    with io.open(args.base, 'r') as base, io.open(args.latest, 'r') as latest:
+        try:
+            lines = render_diff(base, latest)

[Sebastian Noack, 2018/09/18 19:13:18]

Iterating over a file-object yields each line including the line ending
character(s). While for most lines (all except metadata) the lines are stripped
in parse_line(), this is an implementation detail of the parser. So we might
want to pass lines without line endings here.

[Vasily Kuznetsov, 2018/09/19 18:40:04]

Actually metadata is also stripped of the line endings -- I was worried that it
wouldn't be after the latest changes, but the regex matching seems to take care
of it.

The parser is intended to work this way (see example in README and the code in
render_script.py), so I would say that the following pattern is idiomatic:

    with open(file) as f:
        for parsed_line in parse_filterlist(f):
            # do something with the parsed line

This is a common enough use case that we rather would like this to work instead
of requiring the caller to always strip the lines by themselves.

[Sebastian Noack, 2018/09/20 16:59:12]

Ah, right, that is because by default . doesn't match newline characters.
Alright.

[rhowell, 2018/09/21 08:37:09]

Acknowledged.

+            if args.outfile == '-':
+                for line in lines:
+                    sys.stdout.write(line + '\n')

[Sebastian Noack, 2018/09/18 19:13:18]

Does that work on Python 2 if there are any non-ascii characters?

[Sebastian Noack, 2018/09/20 16:59:12]

For reference, the problem is that in Python 2 if no shell (or no unicode-aware
shell) is attached to stdout, sys.stdout.encoding will be None (which means
ascii), while Python 3 defaults to UTF-8 in that case.

I think the simplest way, with the least duplication and special cases, giving
consitent behavior on Python 2 and 3 would be this:

  if args.outfile == '-':
      outfile = io.open(sys.stdout.fileno(), 'w',
                        closefd=False,
                        encoding=sys.stdout.encoding or 'utf-8')
  else:
      outfile = io.open(args.outfile, 'w', encoding='utf-8')

  with outfile:
      for line in lines:
          print(line, file=outfile)

[Vasily Kuznetsov, 2018/09/21 07:48:42]

This is more or less the approach that we took in TrustedNews tools for a
similar problem (there we wanted the file to be open in binary mode regardless
of whether it's a real file or stdout). I also think this is a good way to go.

[rhowell, 2018/09/21 08:37:08]

This approach seems to be working well. Thanks, I was having some trouble with
encoding.

[Sebastian Noack, 2018/09/26 11:55:47]

I just noticed, we might have the same issue in render_script. This should
probably be addressed in a separate change.

[Vasily Kuznetsov, 2018/09/26 16:50:40]

Yeah, makes sense. Perhaps we could move this piece of code out and import into
both scripts.

+            else:
+                with io.open(args.outfile, 'w', encoding='utf-8') as out_fp:
+                    for line in lines:
+                        out_fp.write(line + '\n')
+        except NotFound as exc:
+            sys.exit(exc)