[MERGE] bzr help locationspec
Marius Gedminas
marius at pov.lt
Sun Jan 21 20:28:32 GMT 2007
On Sun, Jan 21, 2007 at 03:40:22PM +0100, Goffredo Baroncelli wrote:
> Enclose a new patch, which address most of the comment of Aaron and others.
> First of all, I would say that the Registry class is not able to handle the
> transport as is now implemented because often more transports are registered
> under the same prefix. So I still used the old registry_lazy_transport
> function, with an additional parameters named "shorthelp", which is the help
> passed to the transports topics. So
>
> 1) The transport topics is build on the basis of the "shorthelp" passed to the
> registry_lazy_transport function. The help messages are the same of the
> initial Marious patch.
> 2) The transport topics show only the transport/decorator ( which I
> called "modifier" ) which have a shorthelp registered. So I deleted the
> blacklist of the transport which we don't want to be displayed.
> 4) Now an user can get more info about a transport withe the follow command:
>
> $ bzr help <protocol prefix>
>
> Because Aaron highlighted that most of the comments in the transport classes
> are outdated, the info are the one of the help_txt class field when present (
> or the shorthelp otherwise).
>
>
> Example of use
>
> $ ./bzr help transports
>
> Supported transport protocols:
> ------------------------------
> http:// Read-only access of branches exported on the web.
> aftp:// Access using active FTP.
> https+pycurl:// Read-only access of branches exported on the web using
> SSL.
> https+urllib:// Read-only access of branches exported on the web using
> SSL.
> http+urllib:// Read-only access of branches exported on the web.
> http+pycurl:// Read-only access of branches exported on the web.
Do typical users of bzr care whether it's using pycurl or urllib? I
know I don't.
> bzr+http:// Fast access using the Bazaar smart server over HTTP.
> bzr+ssh:// Fast access using the Bazaar smart server over SSH.
> sftp:// Access using SFTP (most SSH servers provide SFTP).
> https:// Read-only access of branches exported on the web using
> SSL.
> ftp:// Access using passive FTP.
> bzr:// Fast access using the Bazaar smart server.
The order seems arbitrary. It would be nicer to use either logical
ordering (ftp next to aftp, https next to http) or alphabetic ordering.
> Supported modifier:
> -------------------
> readonly+ This modifier converts any transport to be readonly.
>
> You can get more info with 'bzr help <protocol prefix>'
>
> $ ./bzr help bzr://
> This protocol permit to connect to a smart server over plain tcp.
>
> $ ./bzr help https://
> Read-only access of branches exported on the web using SSL
>
> Commenst are welcome
>
> 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
> # Bazaar revision bundle v0.8
> #
> # message:
> # The help for the single protocol are created on the basis of the help_txt class field
> #
> # committer: ghigo <ghigo at venice>
> # date: Sun 2007-01-21 15:35:15.631999969 +0100
>
> === modified file bzrlib/help_topics.py
> --- bzrlib/help_topics.py
> +++ bzrlib/help_topics.py
> @@ -75,6 +75,8 @@
>
> out = []
> for topic in topics:
> + if topic.endswith("://") or topic.endswith("+"): # skip the protocol
> + continue
> summary = topic_registry.get_summary(topic)
> out.append("%-*s %s\n" % (lmax, topic, summary))
> return ''.join(out)
> @@ -151,6 +153,40 @@
> Note: --version must be supplied before any command.
> """
>
> +def _help_on_transport(name):
> +
> + def add_string(proto, help, maxl):
> + out = []
> + while len(help):
> + out.append("%-20s%s\n"%(proto, help[:maxl-20]))
> + proto = ""
> + help = help[maxl-20:]
> + return out
How about using the textwrap module from the stdlib here? Your way
might wrap the help text in the middle of a word. The textwrap solution
would be something like
def add_string(proto, help, maxl, prefix_width=20):
help_lines = textwrap.wrap(help, maxl - prefix_width)
line_with_indent = '\n' + ' ' * prefix_width
help_text = line_with_indent.join(help_lines)
return "%-20s%s\n" % (proto, help_text)
Also, it is a bit hard to test this function separately if it is hidden
inside the scope of another function.
> +
> + protl = []
> + decl = []
> + protl.append("\nSupported transport protocols:\n------------------------------\n")
> + decl.append("\nSupported modifier:\n-------------------\n")
Maybe "Supported modifiers" would sound better? OTOH there is only one
that is shown at the moment, so maybe singular is OK.
> + done = []
> + for proto, (klass, shorthelp) in bzrlib.transport._protocol_help.iteritems( ):
The space between the parentheses looks strange to me. Also, does this
line fit in a 80-column-wide screen?
> + if proto in done:
> + continue
> + done.append(proto)
> + if proto.endswith("://"):
> + protl.extend(add_string(proto, shorthelp, 79))
> + else:
> + decl.extend(add_string(proto, shorthelp, 79))
> +
> +
> + out = ''.join(protl)
> + out = out + ''.join(decl)
Or simply
out = ''.join(protl + decl)
> + out = out + """
> +You can get more info with 'bzr help <protocol prefix>'
> +
> +"""
> +
> + return out
>
> topic_registry.register("revisionspec", _help_on_revisionspec,
> "Explain how to use --revision")
> @@ -162,3 +198,31 @@
> topic_registry.register('formats', get_format_topic, 'Directory formats')
> topic_registry.register('global-options', _global_options,
> 'Options that can be used with any command')
> +topic_registry.register('transports', _help_on_transport,
> + 'Transports available')
> +
> +import bzrlib.transport
> +def _help_on_single_transport(proto):
> + klass,shorthelp = bzrlib.transport._protocol_help[proto]
Could you move one of the two spaces after the '=' to right after the
','?
> + try:
> + if klass.__module__ == "bzrlib.transport":
> + # this is a lazy load transport, because no real ones
> + # are directlry in bzrlib.transport
s/directlry/directly/
> + mod = __import__(klass.module, globals(), locals(), [klass.classname])
Does this line fit in 80 columns?
> + klass = getattr(mod, klass.classname)
> + else:
> + klass = klass.__module__
It feels weird then a variable named "klass" suddenly refers to a
module, but only half the time.
> + if hasattr(klass, "help_txt"):
> + doc = klass.help_txt
> + else:
> + doc = shorthelp+"\n"
This could be shortened to
doc = getattr(klass, "help_txt", shorthelp + "\n")
> + except:
Bare excepts are usually a bad idea. What if I press Ctrl+C at just the
wrong moment, and my KeyboardInterrupt gets silently swallowed?
> + doc = "This transport is currently unavailable and its help wont be displayed.\n"
"Won't" needs an apostrophe.
This line definitely doesn't fit in 80 columns.
> + return doc
> +
> +for proto, (klass, shorthelp) in bzrlib.transport._protocol_help.iteritems( ):
The extra space between ( ) again.
> + topic_registry.register(proto,
> + _help_on_single_transport,
> + shorthelp)
>
> === modified file bzrlib/transport/__init__.py // last-changed:ghigo at venice-200
> ... 70121140422-eyi5wb1r7y8jvhua
> --- bzrlib/transport/__init__.py
> +++ bzrlib/transport/__init__.py
> @@ -66,8 +66,10 @@
> # want.
> _protocol_handlers = {
> }
> +_protocol_help = {
> +}
>
> -def register_transport(prefix, klass, override=DEPRECATED_PARAMETER):
> +def register_transport(prefix, klass, override=DEPRECATED_PARAMETER, shorthelp=None):
I think it's time to wrap this line.
> """Register a transport that can be used to open URLs
>
> Normally you should use register_lazy_transport, which defers loading the
> @@ -80,9 +82,10 @@
> if deprecated_passed(override):
> warnings.warn("register_transport(override) is deprecated")
> _protocol_handlers.setdefault(prefix, []).insert(0, klass)
> -
> -
> -def register_lazy_transport(scheme, module, classname):
> + if shorthelp and prefix:
> + _protocol_help[prefix] = (klass, shorthelp)
> +
> +def register_lazy_transport(scheme, module, classname, shorthelp=None):
> """Register lazy-loaded transport class.
>
> When opening a URL with the given scheme, load the module and then
> @@ -102,7 +105,8 @@
> klass = getattr(mod, classname)
> return klass(base)
> _loader.module = module
> - register_transport(scheme, _loader)
> + _loader.classname = classname
> + register_transport(scheme, _loader, shorthelp=shorthelp)
>
>
> def _get_protocol_handlers():
> @@ -1165,37 +1169,53 @@
> # None is the default transport, for things with no url scheme
> register_lazy_transport(None, 'bzrlib.transport.local', 'LocalTransport')
> register_lazy_transport('file://', 'bzrlib.transport.local', 'LocalTransport')
> -register_lazy_transport('sftp://', 'bzrlib.transport.sftp', 'SFTPTransport')
> +register_lazy_transport('sftp://', 'bzrlib.transport.sftp', 'SFTPTransport',
> + shorthelp="Access using SFTP (most SSH servers provide SFTP).")
> register_lazy_transport('http+urllib://', 'bzrlib.transport.http._urllib',
> - 'HttpTransport_urllib')
> + 'HttpTransport_urllib',
> + shorthelp="Read-only access of branches exported on the web.")
> register_lazy_transport('https+urllib://', 'bzrlib.transport.http._urllib',
> - 'HttpTransport_urllib')
> + 'HttpTransport_urllib',
> + shorthelp="Read-only access of branches exported on the web using SSL.")
> register_lazy_transport('http+pycurl://', 'bzrlib.transport.http._pycurl',
> - 'PyCurlTransport')
> + 'PyCurlTransport',
> + shorthelp="Read-only access of branches exported on the web.")
> register_lazy_transport('https+pycurl://', 'bzrlib.transport.http._pycurl',
> - 'PyCurlTransport')
> + 'PyCurlTransport',
> + shorthelp="Read-only access of branches exported on the web using SSL.")
> register_lazy_transport('http://', 'bzrlib.transport.http._urllib',
> - 'HttpTransport_urllib')
> + 'HttpTransport_urllib',
> + shorthelp="Read-only access of branches exported on the web.")
> register_lazy_transport('https://', 'bzrlib.transport.http._urllib',
> - 'HttpTransport_urllib')
> -register_lazy_transport('http://', 'bzrlib.transport.http._pycurl', 'PyCurlTransport')
> -register_lazy_transport('https://', 'bzrlib.transport.http._pycurl', 'PyCurlTransport')
> -register_lazy_transport('ftp://', 'bzrlib.transport.ftp', 'FtpTransport')
> -register_lazy_transport('aftp://', 'bzrlib.transport.ftp', 'FtpTransport')
> + 'HttpTransport_urllib',
> + shorthelp="Read-only access of branches exported on the web using SSL.")
> +register_lazy_transport('http://', 'bzrlib.transport.http._pycurl', 'PyCurlTransport',
> + shorthelp="Read-only access of branches exported on the web.")
> +register_lazy_transport('https://', 'bzrlib.transport.http._pycurl', 'PyCurlTransport',
> + shorthelp="Read-only access of branches exported on the web using SSL.")
> +register_lazy_transport('ftp://', 'bzrlib.transport.ftp', 'FtpTransport',
> + shorthelp="Access using passive FTP.")
> +register_lazy_transport('aftp://', 'bzrlib.transport.ftp', 'FtpTransport',
> + shorthelp="Access using active FTP.")
> register_lazy_transport('memory://', 'bzrlib.transport.memory', 'MemoryTransport')
> register_lazy_transport('chroot+', 'bzrlib.transport.chroot',
> 'ChrootTransportDecorator')
> -register_lazy_transport('readonly+', 'bzrlib.transport.readonly', 'ReadonlyTransportDecorator')
> +register_lazy_transport('readonly+', 'bzrlib.transport.readonly',
> + 'ReadonlyTransportDecorator',
> + shorthelp="This modifier converts any transport to be readonly.")
> register_lazy_transport('fakenfs+', 'bzrlib.transport.fakenfs', 'FakeNFSTransportDecorator')
> register_lazy_transport('vfat+',
> 'bzrlib.transport.fakevfat',
> 'FakeVFATTransportDecorator')
> register_lazy_transport('bzr://',
> 'bzrlib.transport.smart',
> - 'SmartTCPTransport')
> + 'SmartTCPTransport',
> + shorthelp="Fast access using the Bazaar smart server.")
> register_lazy_transport('bzr+http://',
> 'bzrlib.transport.smart',
> - 'SmartHTTPTransport')
> + 'SmartHTTPTransport',
> + shorthelp="Fast access using the Bazaar smart server over HTTP.")
> register_lazy_transport('bzr+ssh://',
> 'bzrlib.transport.smart',
> - 'SmartSSHTransport')
> + 'SmartSSHTransport',
> + shorthelp="Fast access using the Bazaar smart server over SSH.")
>
> === modified file bzrlib/transport/smart.py
> --- bzrlib/transport/smart.py
> +++ bzrlib/transport/smart.py
> @@ -1726,6 +1726,11 @@
> SmartTCPClientMedium).
> """
>
> + help_txt = """\
> +This protocol permit to connect to a smart server over plain tcp.
s/permit/permits/.
I have a feeling that "allows" would be a better word here. But I'm not
a native English speaker, so I might be wrong.
TCP should be capitalized.
Maybe it would be a good idea to mention that you should run 'bzr serve'
on the remote machine before you can use this transport. That's
assuming I remembered it correctly.
> +
> +"""
> +
> def __init__(self, url):
> _scheme, _username, _password, _host, _port, _path = \
> transport.split_url(url)
>
I like your patch much better than mine. Certain bits look a bit
unclean, though -- accessing private variables defined in other modules,
for example, or that bit about klass.__module__ == "bzrlib.transport".
Perhaps bzrlib.transport should provide public API for accessing
protocol help strings, and loading lazy transports.
Marius Gedminas
--
Look! Before our very eyes, the future is becoming the past.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070121/bc51847e/attachment.pgp
More information about the bazaar
mailing list