Merge lp:~spiv/bzr/html-cmd-help into lp:bzr

[Andrew Bennetts]

Fix rendering of HTML of command-line options such as "--1.14".

I noticed that the HTML rendering of command help was once again failing due to options with dots in them, like "--1.14". Neither plain ReST nor Sphinx's option markup allows for dots in option names. Previously we were doing a rather fragile hack by looking for a fixed string in the output, and if so doing a crude string munging to make all branch format options get rendered as preformatted text.

This change instead subclasses optparse's IndentedHelpFormatter to make it emit more appropriate Sphinx markup in the first place. Because that markup cannot support our options with dots in the names, I resort to using ".. describe:" (i.e. a description list in HTML terms, <dl> etc). I also use ".. cssclass:" so that I can add some simple CSS to tweak the rendering to more closely resemble the existing output (which when it worked was nicer than a raw description list).

Hopefully this will prove less fragile, although I'm not sure if that will work out to be true in practice.

Perhaps instead we should fix bug 330494, to remove those option names? It seems a bit perverse to change our UI to match an oversight in our documentation tools, though.

[Martin Pool (mbp)]

review: Approve

[lp:~spiv/bzr/html-cmd-help]

5439. By Andrew Bennetts on 2010-09-22

Add NEWS entry.

[Andrew Bennetts (spiv)]

sent to pqm by email

[lp:~spiv/bzr/html-cmd-help]

5440. By Andrew Bennetts on 2010-09-22

Do not use SphinxHelpFormatter if get_help_text was asked for plain output.

[Vincent Ladeuil (vila)]

Doing HTML specific stuff is the doc is the wrong path.

It won't work with texinfo.

I don't know how to address it (yet), but please consider the above.

[Andrew Bennetts (spiv)]

[The branch failed PQM due to a test in test_help that tests for the exact
output for get_help_text(plain=False)]

Vincent Ladeuil wrote:
> Doing HTML specific stuff is the doc is the wrong path.
>
> It won't work with texinfo.
>
> I don't know how to address it (yet), but please consider the above.

Well, what we're doing atm doesn't work with ReST at all.

Isn't the texinfo generated by sphinx too? If so I think this is still a
reasonable approach. Sphinx presumably knows how to turn 'describe' elements
into appropriate texinfo, and ditto for 'cssclass' (probably by ignoring them).

I think the 'cssclass' sphinx directive is actually just 'class' in plain ReST,
renamed because sphinx uses 'class' for documenting classes in code. (And I
think it may be renamed again to 'rst-class' in newer versions of sphinx?) So in
concept (if not spelling) it's fairly standard ReST, and I'd hope our texinfo
tool chain is able to cope with it without too much effort. If not we can
always write another optparse formatter that generates something
texinfo-friendly.

The only other cheap route I see is to give up and make the whole options
description be preformatted text, which will give equally dissatisfying results
in all outputs.

[Martin Pool (mbp)]

There's another cheap route which is to make people say 'upgrade
--format=1.7' etc, which is not so bad as a ui. A bit unfortunate if
it breaks existing muscle memory though.

--
Martin

[John A Meinel (jameinel)]

We could document it as "--format=1.7" but still allow "--1.7"...

[Andrew Bennetts (spiv)]

I'm not sure I like having the docs be out of sync at all with the allowed behaviour. That's a bit... smelly. Anyway, I'll pop this branch into Work In Progress until I resolve the test failure or we decide to do something else...

[Vincent Ladeuil (vila)]

I don't know when I'll be able to resume working on texinfo, so don't block on that as long as you feel you're still in the sphinx/doctest area and not into an html specific one (I trust you on that), I'll see what is needed there then.

Unmerged revisions

5440. By Andrew Bennetts on 2010-09-22

Do not use SphinxHelpFormatter if get_help_text was asked for plain output.

5439. By Andrew Bennetts on 2010-09-22

Add NEWS entry.

5438. By Andrew Bennetts on 2010-09-22

Tweak the HTML to approximately match Sphinx's default rendering of command-line options.

5437. By Andrew Bennetts on 2010-09-22

Replace crude munging of optparse's indented help output -> ReST with slightly less crude subclassing of IndentedHelpFormatter.

=== modified file 'NEWS'
--- NEWS	2010-09-21 10:48:23 +0000
+++ NEWS	2010-09-22 04:13:46 +0000
@@ -25,6 +25,9 @@
 Documentation
 *************
 
+* Fix HTML rendering of command-line options with dots such as ``--1.14``
+  in the User Reference.  (Andrew Bennetts)
+
 API Changes
 ***********
 

=== modified file 'bzrlib/commands.py'
--- bzrlib/commands.py	2010-08-13 07:56:06 +0000
+++ bzrlib/commands.py	2010-09-22 04:13:46 +0000
@@ -513,21 +513,13 @@
         # XXX: optparse implicitly rewraps the help, and not always perfectly,
         # so we get <https://bugs.launchpad.net/bzr/+bug/249908>.  -- mbp
         # 20090319
-        options = option.get_optparser(self.options()).format_option_help()
-        # XXX: According to the spec, ReST option lists actually don't support 
-        # options like --1.9 so that causes syntax errors (in Sphinx at least).
-        # As that pattern always appears in the commands that break, we trap
-        # on that and then format that block of 'format' options as a literal
-        # block.
-        if not plain and options.find('  --1.9  ') != -1:
-            options = options.replace(' format:\n', ' format::\n\n', 1)
-        if options.startswith('Options:'):
-            result += ':' + options
-        elif options.startswith('options:'):
-            # Python 2.4 version of optparse
-            result += ':Options:' + options[len('options:'):]
+        if plain:
+            formatter = None
         else:
-            result += options
+            formatter = option.SphinxHelpFormatter()
+        options = option.get_optparser(self.options()).format_option_help(
+            formatter)
+        result += options
         result += '\n'
 
         if verbose:

=== modified file 'bzrlib/option.py'
--- bzrlib/option.py	2010-05-23 18:57:38 +0000
+++ bzrlib/option.py	2010-09-22 04:13:46 +0000
@@ -26,6 +26,7 @@
     errors,
     revisionspec,
     )
+import textwrap
 """)
 
 from bzrlib import (
@@ -579,3 +580,34 @@
 diff_writer_registry = _mod_registry.Registry()
 diff_writer_registry.register('plain', lambda x: x, 'Plaintext diff output.')
 diff_writer_registry.default_key = 'plain'
+
+
+class SphinxHelpFormatter(optparse.IndentedHelpFormatter):
+    """The default rest format for command line options fails to parse options
+    with '.' in their name, so define a custom formatter to use the 'describe'
+    directive in sphinx to replace it.
+    """
+
+    def format_heading(self, heading):
+        return "%*s:%s:\n" % (self.current_indent, "", heading)
+
+    def format_option(self, option):
+        result = []
+        result.append(
+            '%*s.. cssclass:: bzr-option\n' % (self.current_indent, ''))
+        opts = self.option_strings[option]
+        opts = '%*s.. describe:: %s\n' % (self.current_indent, '', opts)
+        result.append(opts)
+        result.append('\n')
+        self.indent()
+        if option.help:
+            help_text = self.expand_default(option)
+            help_lines = textwrap.wrap(help_text, self.help_width)
+            result.extend(['%*s%s\n' % (self.current_indent, '', line)
+                           for line in help_lines])
+        self.dedent()
+        result.append('\n')
+        return ''.join(result)
+
+
+

=== added file 'doc/en/_static/bzr-docs.css'
--- doc/en/_static/bzr-docs.css	1970-01-01 00:00:00 +0000
+++ doc/en/_static/bzr-docs.css	2010-09-22 04:13:46 +0000
@@ -0,0 +1,5 @@
+dl.bzr-option dt tt.descname {
+    font-family: monospace;
+    font-weight: normal;
+    font-size: 1em;
+}

=== modified file 'doc/en/_templates/layout.html'
--- doc/en/_templates/layout.html	2010-03-02 10:21:39 +0000
+++ doc/en/_templates/layout.html	2010-09-22 04:13:46 +0000
@@ -1,5 +1,10 @@
 {% extends "!layout.html" %}
 
+{% block extrahead %}
+{{ super() }}
+<link rel="stylesheet" href="{{ pathto("_static/bzr-docs.css", 1) }}" />
+{% endblock %}
+
 {% block rootrellink %}
 <li><a href="http://bazaar.canonical.com/">
     <img src="{{ pathto("_static/bzr icon 16.png", 1) }}" /> Home</a>&nbsp;|&nbsp;</li>