[MERGE] bzr help locationspec

[Goffredo Baroncelli]

Hi Aaron,

I agree with the most of your comments, but these comments are related to the 
*actual* documentation of the python class inside the source and not to my 
patch.
The real fix is to correct these info, but that is not the goal of my patch.
If we merge this patch, the people will update the internal documentation.

See below for more comments:


On Thursday 18 January 2007 01:22, you (Aaron Bentley) wrote:
> Goffredo Baroncelli wrote:
> 
> > ghigo at venice:~/bazaar/repo/bzr-help-topics-transport$ ./bzr help transport
> 
> I also think "transport" is a very poor topic name.  My favourite would
> be "locations", I'd accept "urls", but "protocol" and "transport" aren't
> good topic names, because they're not very discoverable.

I intend "location" as place ( == repositories/branches ), and not as "method 
of transmission". URL is both: place and method. 
Instead in my opinion protocol or transport are the best; but I am open to 
other opinions.

> 
> > * Transport: <default>, file://
> >     This is the transport agent for local filesystem access.
> 
> I don't think "transport agent" is useful terminology.  The word
> "transport" is both a verb and a noun, and we use it as a noun.
> 
> Any description of the locations Bazaar accepts should include directory
> paths as well as file:// urls.
> 
> "<default>" is not useful information.

Ok, it can be removed.

> 
> > * Transport: aftp://, ftp://
> >     This is the transport agent for ftp:// access.
> 
> This is too vague.  Either aftp should have its own section, or you
> should describe here how it differs from ftp.
The fix is to updated the info in te source


> 
> > * Transport: bzr+http://
> >     Just a way to connect between a bzr+http:// url and http://.
> >
> >     This connection operates slightly differently than the 
SmartSSHTransport.
> >     It uses a plain http:// transport underneath, which defines what 
remote
> >     .bzr/smart URL we are connected to. From there, all paths that are 
sent
> > are
> >     sent as relative paths, this way, the remote side can properly
> >     de-reference them, since it is likely doing rewrite rules to translate 
an
> >     HTTP path into a local path.
> 
> This is not true; the patch to do this was never merged:
> 
http://bundlebuggy.aaronbentley.com/request/%3C20070104062000.GE5903@steerpike.home.puzzling.org%3E
> 
> Also, the description shouldn't include internal details such as
> SmartSSHTransport.
The fix is to updated the info in te source
> 
> > * Transport: bzr+ssh://
> >     Connection to smart server over SSH.
> >
> >     This is essentially just a factory to get 'RemoteTransport(url,
> >         SmartSSHClientMedium).
> 
> Again, way too much info.  The first line is enough.

This is a interesting idea. We can ahve two level of description: a generic 
topic "transport" which lists every transport with a short description, and  
something like "bzr help ssh://" which shows more information about the 
ssh:// protocol.

> 
> > * Transport: bzr://
> >     Connection to smart server over plain tcp.
> >
> >     This is essentially just a factory to get 'RemoteTransport(url,
> >         SmartTCPClientMedium).
> 
> 
> Too much info again.
> 
> > * Transport: http+pycurl://, http://, https+pycurl://, https://
> >     http client transport using pycurl
> >
> >     PyCurl is a Python binding to the C "curl" multiprotocol client.
> >
> >     This transport can be significantly faster than the builtin
> >     Python client.  Advantages include: DNS caching.
> 
> Is that still an advantage?  I thought the urllib transport had closed
> the gap, and there were virtually no advantages to pycurl.
The fix is to updated the info in te source

> 
> > * Transport: http+urllib://, http://, https+urllib://, https://
> >     Python urllib transport for http and https.
> 
> This is wrong.  http is not urllib; it varies, depending on whether you
> have pycurl installed.
The fix is to updated the info in te source

> 
> > * Transport: sftp://
> >     Transport implementation for SFTP access.
> 
> It might be useful to note that it works with most SSH servers.
> Especially given that multiple protocols have been called 'SFTP'.
The fix is to updated the info in te source

> 
> > Supported decorators:
> 
> Perhaps "prefixes" would be a better term.

We should find a better definition. prefixes ( as decorators ) is a syntax 
definition: instead we need a better "semantic" definition...

> 
> > --------------------------
> > * Decorator: chroot+
> >     A decorator that can convert any transport to be chrooted.
> >
> >     This is requested via the 'chrooted+' prefix to get_transport().
> >
> >
> > * Decorator: readonly+
> >     A decorator that can convert any transport to be readonly.
> >
> >     This is requested via the 'readonly+' prefix to get_transport().
> 
> 
> 
> > +# insert here the transports which haven't to be displayed
> 
> "haven't to" => "should not"
ok

> 
> > +    transport_dict={}
> > +    out=[]
> > +    for proto, factory_list in _protocol_handlers.iteritems():
> > +        if proto == None:
> > +            proto = "<default>"
> 
> I think None should be skipped.
ok

> 
> > +        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__
> 
> This is a very ugly approach.  We already have registries, which are a
> much cleaner approach, so I think it would make sense to change
> _supported_protocols into a Registry subclass.

This is a different problem, which has to be fix with a different patch.

> 
> Aaron
> 
> 
Goffredo

-- 
gpg key@ keyserver.linux.it: Goffredo Baroncelli (ghigo) <kreijack at inwind.it>
Key fingerprint = CE3C 7E01 6782 30A3 5B87  87C0 BB86 505C 6B2A CFF9