Issue 4217 - Fix Prefix Regression Due To Mercurial Version Behavior Change

cms/sources.py

 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2016 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
@@ skipping 209 matching lines @@
 
     def __exit__(self, type, value, traceback):
         self.close()
         return False
 
     def close(self):
         self._archive.close()
 
     def has_file(self, filename):
         try:
-            self._archive.getinfo('root/%s' % filename)

[Sebastian Noack, 2016/07/01 11:41:11]

In the case here, it would be preferable to simply use the + operator:

  'root/' + filename

From our coding style guide:

  Use the + operator when concatenating exactly two strings,
  use the format() method for more complex string formatting

+            self._archive.getinfo('root/' + filename)
         except KeyError:
             return False
         return True
 
     def read_file(self, filename, binary=False):
-        data = self._archive.read('root/%s' % filename)

[Sebastian Noack, 2016/07/01 11:41:11]

Same here.

+        data = self._archive.read('root/' + filename)
         if not binary:
             data = data.decode('utf-8')
         return (data, '%s!%s' % (self._name, filename))
 
     def list_files(self, subdir):
-        prefix = 'root/%s/' % subdir

[Sebastian Noack, 2016/07/01 11:41:11]

Please use format() rather than % in new code.

[Sebastian Noack, 2016/07/01 11:46:46]

It's not unrelated here, bacuase you are changing/replacing this code anyway.
And new code should always follow our latest coding practices.

[Sebastian Noack, 2016/07/01 11:58:57]

What is about this comment?

+        prefix = 'root/{}/'.format(subdir)
         for filename in self._archive.namelist():
             if filename.startswith(prefix):
                 yield filename[len(prefix):]
 
     if os.name == 'posix':
         def get_cache_dir(self):
             return '/var/cache/' + self._name
 
 
 class FileSource(Source):
@@ skipping 39 matching lines @@
                 path = os.path.join(dir, filename)
                 if os.path.isfile(path):
                     result.append(relpath + filename)
                 elif os.path.isdir(path):
                     do_list(path, relpath + filename + '/')
         do_list(self.get_path(subdir), '')
         return result
 
     def get_cache_dir(self):
         return os.path.join(self._dir, 'cache')