Merge lp:~allenap/launchpad/sub-filters-via-api-bug-672619 into lp:launchpad

[Gavin Panella]

[r=deryck][ui=none][bug=672619] Expose bug subscription filters via the web service API.

This exposes bug subscription filters to the API.

There are a few parts to making this happen:

- Providing URL information for bug subscription filters:

  lib/lp/bugs/browser/configure.zcml

- Providing navigation for bug subscription filters:

  lib/lp/registry/browser/configure.zcml
  lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py
    for TestBugSubscriptionFilterNavigation
  lib/lp/registry/browser/structuralsubscription.py
    for StructuralSubscriptionNavigation

- Exporting IStructuralSubscription.bug_filters and .newBugFilter().

  lib/canonical/launchpad/interfaces/_schema_circular_imports.py
  lib/lp/registry/browser/tests/test_structuralsubscription.py
  lib/lp/registry/interfaces/structuralsubscription.py

- Exporting IBugSubscriptionFilter.

  lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py
    for TestBugSubscriptionFilterAPI
    for TestBugSubscriptionFilterAPIModifications
  lib/lp/bugs/interfaces/bugsubscriptionfilter.py
  lib/lp/bugs/interfaces/webservice.py

- Fix a weird issue where navigation was attempted on a non-flushed
  object. Haven't investigated this, but a flush prevents issues.

  lib/lp/registry/model/structuralsubscription.py

[Gavin Panella (allenap)]

I also fixed some lint in utilities/format-imports.

[Deryck Hodge (deryck)]

Looks fine to me, per our discussions on IRC.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2010-12-01 19:12:00 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2010-12-21 23:23:17 +0000
@@ -420,6 +420,8 @@
 # IStructuralSubscription
 patch_collection_property(
     IStructuralSubscription, 'bug_filters', IBugSubscriptionFilter)
+patch_entry_return_type(
+    IStructuralSubscription, "newBugFilter", IBugSubscriptionFilter)
 patch_reference_property(
     IStructuralSubscription, 'target', IStructuralSubscriptionTarget)
 
 
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml	2010-10-28 09:11:36 +0000
+++ lib/lp/bugs/browser/configure.zcml	2010-12-21 23:23:17 +0000
@@ -1176,4 +1176,13 @@
                 BugWatchSetNavigation"/>
     </facet>
 
+    <!-- Bug Subscription Filters -->
+    <facet facet="bugs">
+      <browser:url
+          for="lp.bugs.interfaces.bugsubscriptionfilter.IBugSubscriptionFilter"
+          path_expression="string:+filter/${id}"
+          attribute_to_parent="structural_subscription"
+          rootsite="bugs" />
+    </facet>
+
 </configure>
 
=== added file 'lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py'
--- lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py	2010-12-21 23:23:17 +0000
@@ -0,0 +1,210 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for bug subscription filter browser code."""
+
+__metaclass__ = type
+
+from functools import partial
+from urlparse import urlparse
+
+from lazr.restfulclient.errors import BadRequest
+from storm.exceptions import LostObjectError
+from testtools.matchers import StartsWith
+import transaction
+
+from canonical.launchpad.webapp.publisher import canonical_url
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+from canonical.testing.layers import (
+    AppServerLayer,
+    LaunchpadFunctionalLayer,
+    )
+from lp.bugs.interfaces.bugtask import (
+    BugTaskImportance,
+    BugTaskStatus,
+    )
+from lp.registry.browser.structuralsubscription import (
+    StructuralSubscriptionNavigation,
+    )
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    ws_object,
+    )
+
+
+class TestBugSubscriptionFilterBase:
+
+    def setUp(self):
+        super(TestBugSubscriptionFilterBase, self).setUp()
+        self.owner = self.factory.makePerson(name=u"foo")
+        self.structure = self.factory.makeProduct(
+            owner=self.owner, name=u"bar")
+        with person_logged_in(self.owner):
+            self.subscription = self.structure.addBugSubscription(
+                self.owner, self.owner)
+            self.subscription_filter = self.subscription.newBugFilter()
+
+
+class TestBugSubscriptionFilterNavigation(
+    TestBugSubscriptionFilterBase, TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def test_canonical_url(self):
+        url = urlparse(canonical_url(self.subscription_filter))
+        self.assertThat(url.hostname, StartsWith("bugs."))
+        self.assertEqual(
+            "/bar/+subscription/foo/+filter/%d" % (
+                self.subscription_filter.id),
+            url.path)
+
+    def test_navigation(self):
+        request = LaunchpadTestRequest()
+        request.setTraversalStack([unicode(self.subscription_filter.id)])
+        navigation = StructuralSubscriptionNavigation(
+            self.subscription, request)
+        view = navigation.publishTraverse(request, '+filter')
+        self.assertIsNot(None, view)
+
+
+class TestBugSubscriptionFilterAPI(
+    TestBugSubscriptionFilterBase, TestCaseWithFactory):
+
+    layer = AppServerLayer
+
+    def test_visible_attributes(self):
+        # Bug subscription filters are not private objects. All attributes are
+        # visible to everyone.
+        transaction.commit()
+        # Create a service for a new person.
+        service = self.factory.makeLaunchpadService()
+        get_ws_object = partial(ws_object, service)
+        ws_subscription = get_ws_object(self.subscription)
+        ws_subscription_filter = get_ws_object(self.subscription_filter)
+        self.assertEqual(
+            ws_subscription.self_link,
+            ws_subscription_filter.structural_subscription_link)
+        self.assertEqual(
+            self.subscription_filter.find_all_tags,
+            ws_subscription_filter.find_all_tags)
+        self.assertEqual(
+            self.subscription_filter.description,
+            ws_subscription_filter.description)
+        self.assertEqual(
+            list(self.subscription_filter.statuses),
+            ws_subscription_filter.statuses)
+        self.assertEqual(
+            list(self.subscription_filter.importances),
+            ws_subscription_filter.importances)
+        self.assertEqual(
+            list(self.subscription_filter.tags),
+            ws_subscription_filter.tags)
+
+    def test_structural_subscription_cannot_be_modified(self):
+        # Bug filters cannot be moved from one structural subscription to
+        # another. In other words, the structural_subscription field is
+        # read-only.
+        user = self.factory.makePerson(name=u"baz")
+        with person_logged_in(self.owner):
+            user_subscription = self.structure.addBugSubscription(user, user)
+        transaction.commit()
+        # Create a service for the structure owner.
+        service = self.factory.makeLaunchpadService(self.owner)
+        get_ws_object = partial(ws_object, service)
+        ws_user_subscription = get_ws_object(user_subscription)
+        ws_subscription_filter = get_ws_object(self.subscription_filter)
+        ws_subscription_filter.structural_subscription = ws_user_subscription
+        error = self.assertRaises(BadRequest, ws_subscription_filter.lp_save)
+        self.assertEqual(400, error.response.status)
+        self.assertEqual(
+            self.subscription,
+            self.subscription_filter.structural_subscription)
+
+
+class TestBugSubscriptionFilterAPIModifications(
+    TestBugSubscriptionFilterBase, TestCaseWithFactory):
+
+    layer = AppServerLayer
+
+    def setUp(self):
+        super(TestBugSubscriptionFilterAPIModifications, self).setUp()
+        transaction.commit()
+        self.service = self.factory.makeLaunchpadService(self.owner)
+        self.ws_subscription_filter = ws_object(
+            self.service, self.subscription_filter)
+
+    def test_modify_tags_fields(self):
+        # Two tags-related fields - find_all_tags and tags - can be
+        # modified. The other two tags-related fields - include_any_tags and
+        # exclude_any_tags - are not exported because the tags field provides
+        # a more intuitive way to update them (from the perspective of an API
+        # consumer).
+        self.assertFalse(self.subscription_filter.find_all_tags)
+        self.assertFalse(self.subscription_filter.include_any_tags)
+        self.assertFalse(self.subscription_filter.exclude_any_tags)
+        self.assertEqual(set(), self.subscription_filter.tags)
+
+        # Modify, save, and start a new transaction.
+        self.ws_subscription_filter.find_all_tags = True
+        self.ws_subscription_filter.tags = ["foo", "-bar", "*", "-*"]
+        self.ws_subscription_filter.lp_save()
+        transaction.begin()
+
+        # Updated state.
+        self.assertTrue(self.subscription_filter.find_all_tags)
+        self.assertTrue(self.subscription_filter.include_any_tags)
+        self.assertTrue(self.subscription_filter.exclude_any_tags)
+        self.assertEqual(
+            set(["*", "-*", "foo", "-bar"]),
+            self.subscription_filter.tags)
+
+    def test_modify_description(self):
+        # The description can be modified.
+        self.assertEqual(
+            None, self.subscription_filter.description)
+
+        # Modify, save, and start a new transaction.
+        self.ws_subscription_filter.description = u"It's late."
+        self.ws_subscription_filter.lp_save()
+        transaction.begin()
+
+        # Updated state.
+        self.assertEqual(
+            u"It's late.", self.subscription_filter.description)
+
+    def test_modify_statuses(self):
+        # The statuses field can be modified.
+        self.assertEqual(set(), self.subscription_filter.statuses)
+
+        # Modify, save, and start a new transaction.
+        self.ws_subscription_filter.statuses = ["New", "Triaged"]
+        self.ws_subscription_filter.lp_save()
+        transaction.begin()
+
+        # Updated state.
+        self.assertEqual(
+            set([BugTaskStatus.NEW, BugTaskStatus.TRIAGED]),
+            self.subscription_filter.statuses)
+
+    def test_modify_importances(self):
+        # The importances field can be modified.
+        self.assertEqual(set(), self.subscription_filter.importances)
+
+        # Modify, save, and start a new transaction.
+        self.ws_subscription_filter.importances = ["Low", "High"]
+        self.ws_subscription_filter.lp_save()
+        transaction.begin()
+
+        # Updated state.
+        self.assertEqual(
+            set([BugTaskImportance.LOW, BugTaskImportance.HIGH]),
+            self.subscription_filter.importances)
+
+    def test_delete(self):
+        # Subscription filters can be deleted.
+        self.ws_subscription_filter.lp_delete()
+        transaction.begin()
+        self.assertRaises(
+            LostObjectError, getattr, self.subscription_filter,
+            "find_all_tags")
 
=== modified file 'lib/lp/bugs/interfaces/bugsubscriptionfilter.py'
--- lib/lp/bugs/interfaces/bugsubscriptionfilter.py	2010-10-04 13:24:54 +0000
+++ lib/lp/bugs/interfaces/bugsubscriptionfilter.py	2010-12-21 23:23:17 +0000
@@ -9,12 +9,18 @@
     ]
 
 
+from lazr.restful.declarations import (
+    export_as_webservice_entry,
+    export_destructor_operation,
+    exported,
+    )
 from lazr.restful.fields import Reference
 from zope.interface import Interface
 from zope.schema import (
     Bool,
     Choice,
     FrozenSet,
+    Int,
     Text,
     )
 
@@ -32,14 +38,18 @@
 class IBugSubscriptionFilterAttributes(Interface):
     """Attributes of `IBugSubscriptionFilter`."""
 
-    structural_subscription = Reference(
-        IStructuralSubscription,
-        title=_("Structural subscription"),
-        required=True, readonly=True)
-
-    find_all_tags = Bool(
-        title=_("All given tags must be found, or any."),
-        required=True, default=False)
+    id = Int(required=True, readonly=True)
+
+    structural_subscription = exported(
+        Reference(
+            IStructuralSubscription,
+            title=_("Structural subscription"),
+            required=True, readonly=True))
+
+    find_all_tags = exported(
+        Bool(
+            title=_("All given tags must be found, or any."),
+            required=True, default=False))
     include_any_tags = Bool(
         title=_("Include any tags."),
         required=True, default=False)
@@ -47,31 +57,36 @@
         title=_("Exclude all tags."),
         required=True, default=False)
 
-    description = Text(
-        title=_("Description of this filter."),
-        required=False)
-
-    statuses = FrozenSet(
-        title=_("The statuses to filter on."),
-        required=True, default=frozenset(),
-        value_type=Choice(
-            title=_('Status'), vocabulary=BugTaskStatus))
-
-    importances = FrozenSet(
-        title=_("The importances to filter on."),
-        required=True, default=frozenset(),
-        value_type=Choice(
-            title=_('Importance'), vocabulary=BugTaskImportance))
-
-    tags = FrozenSet(
-        title=_("The tags to filter on."),
-        required=True, default=frozenset(),
-        value_type=SearchTag())
+    description = exported(
+        Text(
+            title=_("Description of this filter."),
+            required=False))
+
+    statuses = exported(
+        FrozenSet(
+            title=_("The statuses to filter on."),
+            required=True, default=frozenset(),
+            value_type=Choice(
+                title=_('Status'), vocabulary=BugTaskStatus)))
+
+    importances = exported(
+        FrozenSet(
+            title=_("The importances to filter on."),
+            required=True, default=frozenset(),
+            value_type=Choice(
+                title=_('Importance'), vocabulary=BugTaskImportance)))
+
+    tags = exported(
+        FrozenSet(
+            title=_("The tags to filter on."),
+            required=True, default=frozenset(),
+            value_type=SearchTag()))
 
 
 class IBugSubscriptionFilterMethods(Interface):
     """Methods of `IBugSubscriptionFilter`."""
 
+    @export_destructor_operation()
     def delete():
         """Delete this bug subscription filter."""
 
@@ -79,3 +94,4 @@
 class IBugSubscriptionFilter(
     IBugSubscriptionFilterAttributes, IBugSubscriptionFilterMethods):
     """A bug subscription filter."""
+    export_as_webservice_entry()
 
=== modified file 'lib/lp/bugs/interfaces/webservice.py'
--- lib/lp/bugs/interfaces/webservice.py	2010-12-01 22:18:07 +0000
+++ lib/lp/bugs/interfaces/webservice.py	2010-12-21 23:23:17 +0000
@@ -50,7 +50,6 @@
 from lp.bugs.interfaces.bugattachment import IBugAttachment
 from lp.bugs.interfaces.bugbranch import IBugBranch
 from lp.bugs.interfaces.buglink import IBugLinkTarget
-from lp.bugs.interfaces.malone import IMaloneApplication
 from lp.bugs.interfaces.bugnomination import (
     BugNominationStatusError,
     IBugNomination,
@@ -58,6 +57,7 @@
     NominationSeriesObsoleteError,
     )
 from lp.bugs.interfaces.bugsubscription import IBugSubscription
+from lp.bugs.interfaces.bugsubscriptionfilter import IBugSubscriptionFilter
 from lp.bugs.interfaces.bugtarget import (
     IBugTarget,
     IHasBugs,
@@ -82,6 +82,11 @@
     ICve,
     ICveSet,
     )
+from lp.bugs.interfaces.malone import IMaloneApplication
+
+
+
 
+
 # XXX: JonathanLange 2010-11-09 bug=673083: Legacy work-around for circular
 # import bugs.  Break this up into a per-package thing.
 from canonical.launchpad.interfaces import _schema_circular_imports
 
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2010-12-15 22:05:43 +0000
+++ lib/lp/registry/browser/configure.zcml	2010-12-21 23:23:17 +0000
@@ -2225,6 +2225,9 @@
         for="lp.registry.interfaces.structuralsubscription.IStructuralSubscription"
         path_expression="string:+subscription/${subscriber/name}"
         attribute_to_parent="target"/>
+    <browser:navigation
+        module="lp.registry.browser.structuralsubscription"
+        classes="StructuralSubscriptionNavigation"/>
 
     <browser:url
         for="lp.registry.interfaces.personproduct.IPersonProduct"
 
=== modified file 'lib/lp/registry/browser/structuralsubscription.py'
--- lib/lp/registry/browser/structuralsubscription.py	2010-11-29 12:25:43 +0000
+++ lib/lp/registry/browser/structuralsubscription.py	2010-12-21 23:23:17 +0000
@@ -23,13 +23,14 @@
     SimpleVocabulary,
     )
 
-from canonical.launchpad.webapp import (
+from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.menu import Link
+from canonical.launchpad.webapp.publisher import (
     canonical_url,
     LaunchpadView,
+    Navigation,
     stepthrough,
     )
-from canonical.launchpad.webapp.authorization import check_permission
-from canonical.launchpad.webapp.menu import Link
 from canonical.widgets import LabeledMultiCheckBoxWidget
 from lp.app.browser.launchpadform import (
     action,
@@ -44,14 +45,29 @@
 from lp.registry.interfaces.milestone import IProjectGroupMilestone
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.structuralsubscription import (
+    IStructuralSubscription,
     IStructuralSubscriptionForm,
     IStructuralSubscriptionTarget,
     )
 from lp.services.propertycache import cachedproperty
 
 
+class StructuralSubscriptionNavigation(Navigation):
+
+    usedfor = IStructuralSubscription
+
+    @stepthrough("+filter")
+    def bug_filter(self, filter_id):
+        bug_filter_id = int(filter_id)
+        for bug_filter in self.context.bug_filters:
+            if bug_filter.id == bug_filter_id:
+                return bug_filter
+        return None
+
+
 class StructuralSubscriptionView(LaunchpadFormView,
                                  AdvancedSubscriptionMixin):
+
     """View class for structural subscriptions."""
 
     schema = IStructuralSubscriptionForm
 
=== modified file 'lib/lp/registry/browser/tests/test_structuralsubscription.py'
--- lib/lp/registry/browser/tests/test_structuralsubscription.py	2010-11-30 11:38:12 +0000
+++ lib/lp/registry/browser/tests/test_structuralsubscription.py	2010-12-21 23:23:17 +0000
@@ -3,7 +3,10 @@
 
 """Tests for structural subscription traversal."""
 
+from urlparse import urlparse
+
 from lazr.restful.testing.webservice import FakeRequest
+import transaction
 from zope.publisher.interfaces import NotFound
 
 from canonical.launchpad.ftests import (
@@ -14,6 +17,7 @@
 from canonical.launchpad.webapp.publisher import canonical_url
 from canonical.launchpad.webapp.servers import StepsToGo
 from canonical.testing.layers import (
+    AppServerLayer,
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
     )
@@ -27,13 +31,15 @@
 from lp.registry.browser.productseries import ProductSeriesNavigation
 from lp.registry.browser.project import ProjectNavigation
 from lp.registry.browser.structuralsubscription import (
-    StructuralSubscriptionView)
+    StructuralSubscriptionView,
+    )
 from lp.registry.enum import BugNotificationLevel
 from lp.testing import (
     feature_flags,
     person_logged_in,
     set_feature_flag,
     TestCaseWithFactory,
+    ws_object,
     )
 from lp.testing.views import create_initialized_view
 
@@ -356,3 +362,49 @@
         self.assertEqual(
             "To all bugs in %s" % self.target.displayname,
             self.view.target_label)
+
+
+class TestStructuralSubscriptionAPI(TestCaseWithFactory):
+
+    layer = AppServerLayer
+
+    def setUp(self):
+        super(TestStructuralSubscriptionAPI, self).setUp()
+        self.owner = self.factory.makePerson(name=u"foo")
+        self.structure = self.factory.makeProduct(
+            owner=self.owner, name=u"bar")
+        with person_logged_in(self.owner):
+            self.subscription = self.structure.addBugSubscription(
+                self.owner, self.owner)
+        transaction.commit()
+        self.service = self.factory.makeLaunchpadService(self.owner)
+        self.ws_subscription = ws_object(self.service, self.subscription)
+
+    def test_newBugFilter(self):
+        # New bug subscription filters can be created with newBugFilter().
+        ws_subscription_filter = self.ws_subscription.newBugFilter()
+        self.assertEqual(
+            "bug_subscription_filter",
+            urlparse(ws_subscription_filter.resource_type_link).fragment)
+        self.assertEqual(
+            ws_subscription_filter.structural_subscription.self_link,
+            self.ws_subscription.self_link)
+
+    def test_bug_filters(self):
+        # The bug_filters property is a collection of IBugSubscriptionFilter
+        # instances previously created by newBugFilter().
+        bug_filter_links = lambda: set(
+            bug_filter.self_link for bug_filter in (
+                self.ws_subscription.bug_filters))
+        self.assertEqual(set(), bug_filter_links())
+        # A new filter appears in the bug_filters collection.
+        ws_subscription_filter1 = self.ws_subscription.newBugFilter()
+        self.assertEqual(
+            set([ws_subscription_filter1.self_link]),
+            bug_filter_links())
+        # A second new filter also appears in the bug_filters collection.
+        ws_subscription_filter2 = self.ws_subscription.newBugFilter()
+        self.assertEqual(
+            set([ws_subscription_filter1.self_link,
+                 ws_subscription_filter2.self_link]),
+            bug_filter_links())
 
=== modified file 'lib/lp/registry/interfaces/structuralsubscription.py'
--- lib/lp/registry/interfaces/structuralsubscription.py	2010-11-09 11:00:55 +0000
+++ lib/lp/registry/interfaces/structuralsubscription.py	2010-12-21 23:23:17 +0000
@@ -129,11 +129,12 @@
         required=True, readonly=True,
         title=_("The structure to which this subscription belongs.")))
 
-    bug_filters = CollectionField(
+    bug_filters = exported(CollectionField(
         title=_('List of bug filters that narrow this subscription.'),
         readonly=True, required=False,
-        value_type=Reference(schema=Interface))
+        value_type=Reference(schema=Interface)))
 
+    @export_factory_operation(Interface, [])
     def newBugFilter():
         """Returns a new `BugSubscriptionFilter` for this subscription."""
 
 
=== modified file 'lib/lp/registry/model/structuralsubscription.py'
--- lib/lp/registry/model/structuralsubscription.py	2010-11-26 17:08:03 +0000
+++ lib/lp/registry/model/structuralsubscription.py	2010-12-21 23:23:17 +0000
@@ -160,6 +160,8 @@
         """See `IStructuralSubscription`."""
         bug_filter = BugSubscriptionFilter()
         bug_filter.structural_subscription = self
+        # This flush is needed for the web service API.
+        IStore(StructuralSubscription).flush()
         return bug_filter
 
 
 
=== modified file 'lib/lp/registry/stories/webservice/xx-structuralsubscription.txt'
--- lib/lp/registry/stories/webservice/xx-structuralsubscription.txt	2010-08-05 20:49:08 +0000
+++ lib/lp/registry/stories/webservice/xx-structuralsubscription.txt	2010-12-21 23:23:17 +0000
@@ -46,6 +46,7 @@
     start: 0
     total_size: 1
     ---
+    bug_filters_collection_link: u'.../fooix/+subscription/eric/bug_filters'
     date_created: u'...'
     date_last_updated: u'...'
     resource_type_link: u'http://.../#structural_subscription'
@@ -60,6 +61,7 @@
     >>> pprint_entry(eric_webservice.named_get(
     ...     '/fooix', 'getSubscription',
     ...     person=webservice.getAbsoluteUrl('/~eric')).jsonBody())
+    bug_filters_collection_link: u'.../fooix/+subscription/eric/bug_filters'
     date_created: u'...'
     date_last_updated: u'...'
     resource_type_link: u'http://.../#structural_subscription'
@@ -117,6 +119,7 @@
     start: 0
     total_size: 1
     ---
+    bug_filters_collection_link: u'.../fooix/+subscription/pythons/bug_filters'
     date_created: u'...'
     date_last_updated: u'...'
     resource_type_link: u'http://.../#structural_subscription'
 
=== modified file 'utilities/format-imports'
--- utilities/format-imports	2010-08-27 20:17:23 +0000
+++ utilities/format-imports	2010-12-21 23:23:17 +0000
@@ -24,7 +24,7 @@
 that start with "import" or "from" or are indented with at least one space or
 are blank lines. Comment lines are also included if they are followed by an
 import statement. An inital __future__ import and a module docstring are
-explicitly skipped.  
+explicitly skipped.
 
 The import section is rewritten as three subsections, each separated by a
 blank line. Any of the sections may be empty.
@@ -123,7 +123,7 @@
     from lp.app.verylongnames.orverlydeep.modulestructure.leavenoroom \
         import object
 }}}
-""" 
+"""
 
 __metaclass__ = type
 
@@ -145,7 +145,8 @@
     "(?P<objects>[*]|[a-zA-Z0-9_, ]+)"
     "(?P<comment>#.*)?$", re.M)
 from_import_multi_regex = re.compile(
-    "^from +(?P<module>.+) +import *[(](?P<objects>[a-zA-Z0-9_, \n]+)[)]$", re.M)
+    "^from +(?P<module>.+) +import *[(](?P<objects>[a-zA-Z0-9_, \n]+)[)]$",
+    re.M)
 comment_regex = re.compile(
     "(?P<comment>(^#.+\n)+)(^import|^from) +(?P<module>[a-zA-Z0-9_.]+)", re.M)
 split_regex = re.compile(",\s*")
@@ -153,14 +154,16 @@
 # Module docstrings are multiline (""") strings that are not indented and are
 # followed at some point by an import .
 module_docstring_regex = re.compile(
-    '(?P<docstring>^["]{3}[^"]+["]{3}\n).*^(import |from .+ import)', re.M | re.S)
+    '(?P<docstring>^["]{3}[^"]+["]{3}\n).*^(import |from .+ import)',
+    re.M | re.S)
 # The imports section starts with an import state that is not a __future__
 # import and consists of import lines, indented lines, empty lines and
 # comments which are followed by an import line. Sometimes we even find
 # lines that contain a single ")"... :-(
 imports_section_regex = re.compile(
     "(^#.+\n)*^(import|(from ((?!__future__)\S+) import)).*\n"
-    "(^import .+\n|^from .+\n|^[\t ]+.+\n|(^#.+\n)+((^import|^from) .+\n)|^\n|^[)]\n)*",
+    "(^import .+\n|^from .+\n|^[\t ]+.+\n|(^#.+\n)+((^import|^from) "
+    ".+\n)|^\n|^[)]\n)*",
     re.M)
 
 
@@ -227,17 +230,17 @@
     imported as a sorted list of strings."""
     imports = {}
     # Search for escaped newlines and remove them.
-    searchpos =  0
+    searchpos = 0
     while True:
         match = escaped_nl_regex.search(import_section, searchpos)
         if match is None:
             break
         start = match.start()
         end = match.end()
-        import_section = import_section[:start]+import_section[end:]
+        import_section = import_section[:start] + import_section[end:]
         searchpos = start
     # Search for simple one-line import statements.
-    searchpos =  0
+    searchpos = 0
     while True:
         match = import_regex.search(import_section, searchpos)
         if match is None:
@@ -299,7 +302,7 @@
             standard_section[module] = statement
         else:
             thirdparty_section[module] = statement
-    
+
     all_import_lines = []
     # Sort within each section and generate statement strings.
     sections = (
@@ -321,7 +324,7 @@
         if len(import_lines) > 0:
             all_import_lines.append('\n'.join(import_lines))
     # Sections are separated by two blank lines.
-    return '\n\n'.join(all_import_lines)        
+    return '\n\n'.join(all_import_lines)
 
 
 def reformat_importsection(filename):
@@ -334,17 +337,17 @@
     imports_section = pyfile[import_start:import_end]
     imports = parse_import_statements(imports_section)
 
-    if pyfile[import_end:import_end+1] != '#':
+    if pyfile[import_end:import_end + 1] != '#':
         # Two newlines before anything but comments.
         number_of_newlines = 3
     else:
         number_of_newlines = 2
 
-    new_imports = format_imports(imports)+"\n"*number_of_newlines
+    new_imports = format_imports(imports) + ("\n" * number_of_newlines)
     if new_imports == imports_section:
-      # No change, no need to write a new file.
-      return False
-    
+        # No change, no need to write a new file.
+        return False
+
     new_file = open(filename, "w")
     new_file.write(pyfile[:import_start])
     new_file.write(new_imports)
@@ -372,7 +375,7 @@
     if len(sys.argv) == 1 or sys.argv[1] in ("-h", "-?", "--help"):
         sys.stderr.write(dedent("""\
         usage: format-imports <file or directory> ...
-        
+
         Type "format-imports --docstring | less" to see the documentation.
         """))
         sys.exit(1)