> I had a closer look at this. It does what it's supposed to do. As for style, 
> please keep line
> length under 80 characters as described in the Python style guide [1]. Could 
> you rewrite it
> taking that into account?

diff3_1_2vsorig.txt or http://pastebin.com/raw.php?i=t2tC3TqA

This one fixes several things:
1) the length of the lines (72 for the comments and 79 for the rest).
(Should it count spaces?);
2) removes spaces from the end of the lines. (Does it really matter?);
3) replaces print >> sys.stderr with sys.stderr.write().

I'm not sure that I did it right for the docs and comments.
What style is the best?
1.
+    This is a suite that can be added to a distribution, i.e. packages
+    from it complement the principal "distribution" suite.
+    A supplementary suite usually provides newer package versions or
+    extra functionality (e.g. a multimedia suite)."""

2.
+    This is a suite that can be added to a distribution, i.e. packages
+    from it complement the principal "distribution" suite. A
+    supplementary suite usually provides newer package versions or
+    extra functionality (e.g. a multimedia suite)."""

(I've changed the second line.)

According to the typographic tradition, an article shouldn't stay
without a noun at the end of the line.
But maybe it's OK for the code.

Is there a guide for this?

Maybe we should get rid of backslashes. What do you think?
"Long lines can be broken over multiple lines by wrapping expressions
in parentheses.
These should be used in preference to using a backslash for line
continuation." [a]

I put this code in my .emacs file:
(require 'whitespace)
(setq whitespace-style '(face empty tabs lines-tail trailing))
(setq whitespace-line-column 79)
(global-whitespace-mode t)

It will hightlight everything longer then 79 chars.

There is also a test, but it doesn't cover everything.
debderiver_patch3_1_2_tester.py or http://pastebin.com/raw.php?i=fUhZY3t6

[a] http://www.python.org/dev/peps/pep-0008/
--- debderiver
+++ debderiver_patch3_1_2.py
@@ -34,7 +34,7 @@
 
 class Suite(object):
     """Suite.
-    
+
     A suite is a repository of packages."""
 
     def __init__(self):
@@ -49,7 +49,7 @@
 
 class Distribution(Suite):
     """Distribution.
-    
+
     This is the suite that can stand on its own, i.e. provides a working
     operating system. There's only one of these per operating system
     version."""
@@ -70,11 +70,11 @@
 
 class Supplementary_Suite(Suite):
     """Supplementary suite.
-    
-    This is a suite that can be added to a distribution, i.e. packages from 
-    it complement the principal "distribution" suite.  A supplementary suite 
-    usually provides newer package versions or extra functionality (e.g. a 
-    multimedia suite)."""
+
+    This is a suite that can be added to a distribution, i.e. packages
+    from it complement the principal "distribution" suite.
+    A supplementary suite usually provides newer package versions or
+    extra functionality (e.g. a multimedia suite)."""
 
     def __init__(self, pocket, distribution):
         Suite.__init__(self)
@@ -85,6 +85,7 @@
         self.upstream = None
         distribution.add_suppl_suite(self)
 
+
 class OperatingSystem(object):
     """Operating System."""
 
@@ -102,7 +103,8 @@
 class Reprepro(object):
     """Configure and operate reprepro.
 
-    Write reprepro's configuration files and execute reprepro commands."""
+    Write reprepro's configuration files and execute
+    reprepro commands."""
 
     def __init__(self, op_sys):
         self._DEFAULT_COMPONENT = 'main'
@@ -139,20 +141,22 @@
 
         dist['Components'] = self._DEFAULT_COMPONENT
         if suite.fake_component_prefix:
-            dist['Components'] = '/'.join((suite.fake_component_prefix, 
+            dist['Components'] = '/'.join((suite.fake_component_prefix,
                     dist['Components']))
-        dist['DebIndices'] = ' '.join(('Packages', self._DEFAULT_INDICES))
-        dist['DscIndices'] = ' '.join(('Sources', self._DEFAULT_INDICES))
+        dist['DebIndices'] = ' '.join(('Packages',
+                                       self._DEFAULT_INDICES))
+        dist['DscIndices'] = ' '.join(('Sources',
+                                       self._DEFAULT_INDICES))
         if not suite.exclude_installer:
             dist['UDebComponents'] = dist['Components']
             dist['UDebIndices'] = dist['DebIndices']
         self._distributions.append(dist)
 
-    def _add_filter_list(self, codename, filter_list=None, 
+    def _add_filter_list(self, codename, filter_list=None,
             dist_codename=None):
         if not filter_list and not dist_codename:
-            print >> sys.stderr, ('Either filter_list or dist_codename '
-                    'must be given.')
+            sys.stderr.write('Either filter_list or dist_codename '
+                             'must be given\n')
             sys.exit(1)
         if filter_list:
             self._filter_lists[codename] = filter_list
@@ -161,13 +165,14 @@
 
     def _add_update(self, suite):
         upd = {}
-        upd['FilterSrcList'] = ' '.join(('install', os.path.join('filters', 
-                suite.codename)))
+        upd['FilterSrcList'] = ' '.join(('install',
+                                         os.path.join('filters',
+                                                      suite.codename)))
         upd['Method'] = suite.upstream.method
         upd['Name'] = suite.upstream.name
         upd['Suite'] = suite.upstream.suite
         if suite.fake_component_prefix:
-            upd['Suite'] = '/'.join((upd['Suite'], 
+            upd['Suite'] = '/'.join((upd['Suite'],
                 suite.fake_component_prefix))
         upd['VerifyRelease'] = suite.upstream.key
 
@@ -205,7 +210,7 @@
                 ext = f.split('.')[-1].lower()
                 if ext in ('deb', 'dsc', 'udeb'):
                     self._do(' '.join(('include' + ext, suite, p)))
-                    
+
     def _include_thirdparty(self):
         """Recursively include third-party packages."""
 
@@ -218,7 +223,8 @@
                         self._include(s, dir_path, filenames)
 
     def _link_filter_file(self, sup_suite_codename, dist_codename):
-        src_path = os.path.join(self._conf_path, 'filters', dist_codename)
+        src_path = os.path.join(self._conf_path, 'filters',
+                                dist_codename)
         if os.path.isfile(src_path):
             dest_path = os.path.join(self._conf_path, 'filters', \
                     sup_suite_codename)
@@ -227,40 +233,45 @@
     def _purge_purge_lists(self):
         """Purge packages that are in the purge lists.
 
-        Reprepro only blocks packages that are marked with "purge" in the
-        FilterLists.  To get rid of those packages if they were added to a
-        FilterList later, they need to be removed explicitly. This doesn't
-        delete packages that have the operating system's name in their 
version."""
+        Reprepro only blocks packages that are marked with "purge" in
+        the FilterLists.  To get rid of those packages if they were
+        added to a FilterList later, they need to be removed explicitly.
+        This doesn't delete packages that have the operating system's
+        name in their version."""
 
         for dist in self._op_sys.distributions:
             for src_pkg in dist.purge_list:
                 # Check for operating system name in version.
                 command = ['-T', 'dsc', 'listfilter', dist.codename,
-                        '($Source (==' + src_pkg + '), $Version (% *' + \
+                        '($Source (==' + src_pkg + '), $Version (% *' +
                         self._op_sys.name.lower() + '*))']
                 if not self._do2(command):
                     # Package version is not ours, it can be deleted.
-                    self._do(' '.join(('removesrc', dist.codename, src_pkg)))
+                    self._do(' '.join(('removesrc', dist.codename,
+                                       src_pkg)))
 
     def _write_configuration(self):
         if os.path.isdir(self._conf_path):
             shutil.rmtree(self._conf_path)
-        for path in (self._base_path, self._conf_path, self._filters_path,
-                self._include_path):
+        for path in (self._base_path, self._conf_path,
+                     self._filters_path, self._include_path):
             if not os.path.isdir(path):
                 try:
                     os.mkdir(path)
                 except:
-                    print >> sys.stderr, 'Unable to create directory %s.' % \
-                            path
+                    sys.stderr.write('Unable to create directory '
+                                     '"%s"\n' % path)
 
-        # Reprepro wants some fields after others (e.g. Codename first). 
-        # Sorting them all is the easiest way to do that. We take scalars 
-        #first, lists second, each alphabetically in so far as reprepro allows.
-        FIELD_ORDER_DISTS = ('Codename', 'Description', 'FakeComponentPrefix', 
-                'Label', 'Origin', 'SignWith', 'Suite', 'Update', 'Version', 
-                'Architectures', 'Components', 'DebIndices', 'DscIndices', 
-                'UDebComponents', 'UDebIndices')
+        # Reprepro wants some fields after others (e.g. Codename first).
+        # Sorting them all is the easiest way to do that.
+        # We take scalars first, lists second, each alphabetically in so
+        # far as reprepro allows.
+        FIELD_ORDER_DISTS = ('Codename', 'Description',
+                             'FakeComponentPrefix', 'Label', 'Origin',
+                             'SignWith', 'Suite', 'Update', 'Version',
+                             'Architectures', 'Components',
+                             'DebIndices', 'DscIndices',
+                             'UDebComponents', 'UDebIndices')
         dist_path = os.path.join(self._conf_path, 'distributions')
         f_dist = open(dist_path, 'w')
         for dist in self._distributions:
@@ -279,10 +290,10 @@
         f_upd.close()
 
         # First write real filter files, then create links.
-        for fl in [f for f in self._filter_lists.items() if 
+        for fl in [f for f in self._filter_lists.items() if
                 not isinstance(f[1], basestring)]:
             self._write_filter_file(fl[0], fl[1])
-        for fl in [f for f in self._filter_lists.items() if 
+        for fl in [f for f in self._filter_lists.items() if
                 isinstance(f[1], basestring)]:
             self._link_filter_file(fl[0], fl[1])
 
@@ -294,18 +305,18 @@
             try:
                 os.mkdir(incoming_dir_path)
             except:
-                print >> sys.stderr, 'Unable to create directory %s.' % \
-                        incoming_dir_path
+                sys.stderr.write('Unable to create directory "%s"\n' %
+                                 incoming_dir_path)
         if not os.path.isdir(temp_dir_path):
             try:
                 os.mkdir(temp_dir_path)
             except:
-                print >> sys.stderr, 'Unable to create directory %s.' % \
-                        temp_dir_path
+                sys.stderr.write('Unable to create directory "%s"\n' %
+                                 temp_dir_path)
         f_inc = open(incoming_path, 'w')
         f_inc.write('Name: all\n')
-        f_inc.write('Allow: ' + 
-                ' '.join([' '.join([d['Codename'], d['Suite']]) for d in 
+        f_inc.write('Allow: ' +
+                ' '.join([' '.join([d['Codename'], d['Suite']]) for d in
                     self._distributions]) + '\n')
         f_inc.write('IncomingDir: ' + incoming_dir_path + '\n')
         f_inc.write('TempDir: ' + temp_dir_path + '\n')
@@ -318,14 +329,16 @@
                 try:
                     os.mkdir(p)
                 except:
-                    print >> sys.stderr, 'Unable to create directory %s.' % p
+                    sys.stderr.write('Unable to create directory '
+                                     '"%s"\n' % p)
 
     def _write_filter_file(self, codename, purge_list):
         filter_path = os.path.join(self._conf_path, 'filters', codename)
         f_filter = open(filter_path, 'w')
         f_filter.write('\n'.join([src_pkg + ' purge' for src_pkg in \
                 purge_list]))
-        # Reprepro complains about 'overlong lines' if there's no newline
+        # Reprepro complains about 'overlong lines'
+        # if there's no newline
         # at the end of the file.
         f_filter.write('\n')
         f_filter.close()
@@ -338,7 +351,8 @@
             for s in d.suppl_suites:
                 self._add_distribution(self._op_sys, s)
                 self._add_update(s)
-                self._add_filter_list(s.codename, dist_codename=d.codename)
+                self._add_filter_list(s.codename,
+                                      dist_codename=d.codename)
         self._write_configuration()
 
     def update(self):
@@ -359,77 +373,81 @@
     try:
         main_conf = yaml.load(open(conf_file_path, 'r'))
     except:
-        print >> sys.stderr, 'Unable to load %s.' % conf_file_path
+        sys.stderr.write('Unable to load "%s"\n' % conf_file_path)
         sys.exit(1)
 
     # Validate operating system level.
-    for key in ('os', 'base_dir', 'description', 'signing_key', 
'distributions'):
+    for key in ('os', 'base_dir', 'description', 'signing_key',
+                'distributions'):
         if not main_conf.has_key(key):
-            print >> sys.stderr, 'Missing key: "%s".' % key
+            sys.stderr.write('Missing key: %s\n' % key)
             sys.exit(1)
 
     # Validate upstreams.
     ups = main_conf['upstreams']
     if not ups:
-        print >> sys.stderr, 'No upstreams defined.'
+        sys.stderr.write('No upstreams defined\n')
         sys.exit(1)
     for up in ups:
         if not isinstance(ups[up], dict):
-            print >> sys.stderr, 'Upstream %s, no properties defined.' % up
+            sys.stderr.write('Upstream %s: no properties defined\n' %
+                             up)
             sys.exit(1)
         for key in ('method', 'suite'):
             if not ups[up].has_key(key):
-                print >> sys.stderr, 'Upstream %s, missing key: "%s"' % \
-                        (up, key)
+                sys.stderr.write('Upstream %s: missing key: %s\n' %
+                                 (up, key))
                 sys.exit(1)
 
     # Validate distributions.
     dists = main_conf['distributions']
     if not dists:
-        print >> sys.stderr, 'No distributions defined.'
+        sys.stderr.write('No distributions defined\n')
         sys.exit(1)
     for dist in dists:
-        if not isinstance(dists[dist], dict): 
-            print >> sys.stderr, 'Distribution %s, no properties defined.' % \
-                    dist
+        if not isinstance(dists[dist], dict):
+            sys.stderr.write('Distribution %s: no properties '
+                             'defined\n' % dist)
             sys.exit(1)
         for key in ('alias', 'architectures', 'upstream', 'version'):
             if not dists[dist].has_key(key):
-                print >> sys.stderr, 'Distribution %s, missing key: "%s".' % \
-                        (dist, key)
+                sys.stderr.write('Distribution %s: missing key: %s\n' %
+                                 (dist, key))
                 sys.exit(1)
         if not dists[dist]['architectures']:
-            print >> sys.stderr, 'Distribution %s, no architectures defined.' \
-                    % dist
+            sys.stderr.write('Distribution %s: no architectures '
+                             'defined\n' % dist)
             sys.exit(1)
         if dists[dist].has_key('supplementary_suites'):
             suites = dists[dist]['supplementary_suites']
             if not isinstance(suites, dict):
-                print >> sys.stderr, ('Distribution %s, no supplementary '
-                        'suites defined.' % dist)
+                sys.stderr.write('Distribution %s: no supplementary '
+                                 'suites defined\n' % dist)
                 sys.exit(1)
             for suite in suites:
                 if not suites[suite].has_key('upstream'):
-                    print >> sys.stderr, 'Supplementary suite %s-%s, missing \
-                            key: "%s".' % (dist, suite, 'upstream')
+                    sys.stderr.write('Supplementary suite %s-%s: '
+                                     'missing key: %s\n' %
+                                     (dist, suite, 'upstream'))
 
     filter_dir_path = os.path.join(conf_dir_path, 'filters')
     for filter_file_name in os.listdir(filter_dir_path):
-        filter_file_path = os.path.join(filter_dir_path, filter_file_name)
+        filter_file_path = os.path.join(filter_dir_path,
+                                        filter_file_name)
         try:
             filter_list = yaml.load(open(filter_file_path, 'r'))
         except:
-            print >> sys.stderr, 'Unable to load %s.' % filter_file_path
+            sys.stderr.write('Unable to load "%s"\n' % filter_file_path)
             sys.exit(1)
         if not isinstance(filter_list, list):
-            print >> sys.stderr, \
-                    'Filter file %s must contain a list of source packages.' % 
\
-                    filter_file_path
+            sys.stderr.write('Filter file "%s" must contain a list '
+                             'of source packages\n' % filter_file_path)
             sys.exit(1)
         for item in filter_list:
             if not isinstance(item, basestring):
-                print >> sys.stderr, 'Filter file %s: %s is not valid source \
-                        package name.' % (filter_file_path, item)
+                sys.stderr.write('Filter file "%s": "%s" is not a '
+                                 'valid source package name\n' %
+                                 (filter_file_path, item))
 
     # Pour into objects.
     opsys = OperatingSystem(main_conf['os'], main_conf['description'], \
@@ -441,11 +459,13 @@
                 dists[dist]['version'], dists[dist]['architectures'])
         upstream_name = dists[dist]['upstream']
         upstream = main_conf['upstreams'][upstream_name]
-        u = Upstream(upstream_name, upstream['suite'], upstream['method'])
+        u = Upstream(upstream_name, upstream['suite'],
+                     upstream['method'])
         if upstream.has_key('verify_release'):
             u.key = str(upstream['verify_release'])
         d.upstream = u
-        filter_file_path = os.path.join(filter_dir_path, d.codename + '.yaml')
+        filter_file_path = os.path.join(filter_dir_path,
+                                        d.codename + '.yaml')
         if os.path.isfile(filter_file_path):
             d.purge_list.extend(yaml.load(open(filter_file_path, 'r')))
         opsys.add_distribution(d)
@@ -459,9 +479,9 @@
                             'fake_component_prefix']
                 # This is a configuration option instead of an automatic
                 # detection because detection means checking if a
-                # debian-installer directory (or Release file) exists, which
-                # means handling the upstream URI and protocol. That's more
-                # than I want to do now.
+                # debian-installer directory (or Release file) exists,
+                # which means handling the upstream URI and protocol.
+                # That's more than I want to do now.
                 if suites[suite].has_key('exclude_installer'):
                     if suites[suite]['exclude_installer']:
                         s.exclude_installer = True
import sys
import os

sys.stderr.write('Either filter_list or dist_codename '
                 'must be given\n')

up = 42
sys.stderr.write('Upstream %s: no properties defined\n' %
                 up)

key = 42
sys.stderr.write('Upstream %s: missing key: %s\n' %
                 (up, key))

dist = 42
sys.stderr.write('Distribution %s: no properties '
                 'defined\n' % dist)

sys.stderr.write('Distribution %s: missing key: %s\n' %
                 (dist, key))

sys.stderr.write('Distribution %s: no architectures '
                 'defined\n' % dist)

sys.stderr.write('Distribution %s: no supplementary '
                 'suites defined\n' % dist)

suite = 42
sys.stderr.write('Supplementary suite %s-%s: '
                 'missing key: %s\n' %
                 (dist, suite, 'upstream'))

filter_dir_path = '42'
filter_file_name = '42'
filter_file_path = os.path.join(filter_dir_path,
                                filter_file_name)

sys.stderr.write('Unable to load "%s"\n' % filter_file_path)

sys.stderr.write('Filter file "%s" must contain a list '
                 'of source packages\n' % filter_file_path)

item = 42
sys.stderr.write('Filter file "%s": "%s" is not a '
                 'valid source package name\n' %
                 (filter_file_path, item))

def Upstream(*args):
    print args

upstream_name = 42
upstream = {'suite': 42, 'method': 42}
u = Upstream(upstream_name, upstream['suite'],
             upstream['method'])

class D: pass
d = D()
d.codename = '42'
filter_file_path = os.path.join(filter_dir_path,
                                d.codename + '.yaml')
print filter_file_path
_______________________________________________
gNewSense-dev mailing list
gNewSense-dev@nongnu.org
https://lists.nongnu.org/mailman/listinfo/gnewsense-dev

Reply via email to