[MERGE] bzr help locationspec

[Vincent Ladeuil]

>>>>> "aaron" == Aaron Bentley <aaron.bentley at utoronto.ca> writes:

<snip/>

    >> * Transport: http+urllib://, http://, https+urllib://, https://
    >> Python urllib transport for http and https.

    aaron> This is wrong.  http is not urllib; it varies,
    aaron> depending on whether you have pycurl installed.

Exactly. 

By the way, if more developers were willing to change the default
to use urllib for some time (weeks, months ?) we may be able to
make urllib the default.

I noticed more or more people using bzr without bzr installed,
which means urllib *is* tested in real life though.

<snip/>

    >> +        for f in factory_list:
    >> +            try:
    >> +                if f.__module__ == "bzrlib.transport":
    >> +                    # this is a lazy load transport, because no real ones
    >> +                    # are directlry in bzrlib.transport
    >> +                    mod = __import__(f.module, globals(), locals(), [f.classname])
    >> +                    klass = getattr(mod, f.classname)
    >> +                else:
    >> +                    klass = f.__module__
    >> +
    >> +                if hasattr(klass, "help_txt"):
    >> +                    doc = klass.help_txt
    >> +                else:
    >> +                    doc = klass.__doc__

    aaron> This is a very ugly approach.  We already have
    aaron> registries, which are a much cleaner approach, so I
    aaron> think it would make sense to change
    aaron> _supported_protocols into a Registry subclass.

<cough> I agree with Aaron here, but in Goffredo's defense, I
should say that he just copied the code verbatim from
transport/_init__.py (but is it really a good defense ;)

So either a Registry should be implemented and used in both cases
or, at the minimum, a FIXME should be added at both places so
that the Registry is implemented at a later time keeping in mind
that they have different needs:

- in transport/__init__.py, we do a lazy loading so that the
  package is not loaded until needed,

- for help, we will need to load the package to access the doc
  string.

    > === modified file bzrlib/tests/blackbox/test_help.py
    > --- bzrlib/tests/blackbox/test_help.py
    > +++ bzrlib/tests/blackbox/test_help.py
    > @@ -51,6 +51,25 @@
    >          self.assertContainsRe(out, 'ancestor:')
    >          self.assertContainsRe(out, 'branch:')
 
    > +    def test_help_transport(self):
    > +        """Smoke test for 'bzr help transport'"""
    > +        out, err = self.run_bzr('help', 'transport')
    > +        self.assertContainsRe(out, 'http://' )
    > +        self.assertContainsRe(out, 'https\\+pycurl://,' )
    > +        self.assertContainsRe(out, 'http\\+pycurl://,' )
    > +        self.assertContainsRe(out, 'https://' )
    > +        self.assertContainsRe(out, 'sftp://' )
    > +        self.assertContainsRe(out, '<default>' )
    > +        self.assertContainsRe(out, 'file://' )
    > +        self.assertContainsRe(out, 'bzr://' )
    > +        self.assertContainsRe(out, 'aftp://' )
    > +        self.assertContainsRe(out, 'ftp://' )
    > +        self.assertContainsRe(out, 'http://' )
    > +        self.assertContainsRe(out, 'http\\+urllib://,' )
    > +        self.assertContainsRe(out, 'https://' )
    > +        self.assertContainsRe(out, 'https\\+urllib://' )
    > +        self.assertContainsRe(out, 'bzr\\+ssh://' )
    > +
    >      def test_help_commands(self):
    >          dash_help  = self.runbzr('--help commands')[0]
    >          commands   = self.runbzr('help commands')[0]

Have you tested that without pycurl installed ?


    > === modified file bzrlib/transport/__init__.py // last-changed:ghigo at venice-200
    > ... 70117174414-zio1dyj8y2dweeg9
    > --- bzrlib/transport/__init__.py
    > +++ bzrlib/transport/__init__.py
    > @@ -102,6 +102,7 @@
    >          klass = getattr(mod, classname)
    >          return klass(base)
    >      _loader.module = module
    > +    _loader.classname = classname
    >      register_transport(scheme, _loader)

I had to make the exact same modification for a coming patch, so
+1 on this one ;)


   Vincent
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 185 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070118/f78bef0c/attachment.pgp