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

Proposed by Andrew Bennetts
Status: Work in progress
Proposed branch: lp:~spiv/bzr/html-cmd-help
Merge into: lp:bzr
Diff against target: 117 lines (+51/-14)
5 files modified
NEWS (+3/-0)
bzrlib/commands.py (+6/-14)
bzrlib/option.py (+32/-0)
doc/en/_static/bzr-docs.css (+5/-0)
doc/en/_templates/layout.html (+5/-0)
To merge this branch: bzr merge lp:~spiv/bzr/html-cmd-help
Reviewer Review Type Date Requested Status
Martin Pool Approve
Review via email: mp+36244@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) :
review: Approve
lp:~spiv/bzr/html-cmd-help updated
5439. By Andrew Bennetts

Add NEWS entry.

Revision history for this message
Andrew Bennetts (spiv) wrote :

sent to pqm by email

lp:~spiv/bzr/html-cmd-help updated
5440. By Andrew Bennetts

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

Revision history for this message
Vincent Ladeuil (vila) 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.

Revision history for this message
Andrew Bennetts (spiv) wrote :

[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.

Revision history for this message
Martin Pool (mbp) wrote :

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

Revision history for this message
John A Meinel (jameinel) wrote :

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

Revision history for this message
Andrew Bennetts (spiv) wrote :

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...

Revision history for this message
Vincent Ladeuil (vila) wrote :

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

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

5439. By Andrew Bennetts

Add NEWS entry.

5438. By Andrew Bennetts

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

5437. By Andrew Bennetts

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-09-21 10:48:23 +0000
3+++ NEWS 2010-09-22 04:13:46 +0000
4@@ -25,6 +25,9 @@
5 Documentation
6 *************
7
8+* Fix HTML rendering of command-line options with dots such as ``--1.14``
9+ in the User Reference. (Andrew Bennetts)
10+
11 API Changes
12 ***********
13
14
15=== modified file 'bzrlib/commands.py'
16--- bzrlib/commands.py 2010-08-13 07:56:06 +0000
17+++ bzrlib/commands.py 2010-09-22 04:13:46 +0000
18@@ -513,21 +513,13 @@
19 # XXX: optparse implicitly rewraps the help, and not always perfectly,
20 # so we get <https://bugs.launchpad.net/bzr/+bug/249908>. -- mbp
21 # 20090319
22- options = option.get_optparser(self.options()).format_option_help()
23- # XXX: According to the spec, ReST option lists actually don't support
24- # options like --1.9 so that causes syntax errors (in Sphinx at least).
25- # As that pattern always appears in the commands that break, we trap
26- # on that and then format that block of 'format' options as a literal
27- # block.
28- if not plain and options.find(' --1.9 ') != -1:
29- options = options.replace(' format:\n', ' format::\n\n', 1)
30- if options.startswith('Options:'):
31- result += ':' + options
32- elif options.startswith('options:'):
33- # Python 2.4 version of optparse
34- result += ':Options:' + options[len('options:'):]
35+ if plain:
36+ formatter = None
37 else:
38- result += options
39+ formatter = option.SphinxHelpFormatter()
40+ options = option.get_optparser(self.options()).format_option_help(
41+ formatter)
42+ result += options
43 result += '\n'
44
45 if verbose:
46
47=== modified file 'bzrlib/option.py'
48--- bzrlib/option.py 2010-05-23 18:57:38 +0000
49+++ bzrlib/option.py 2010-09-22 04:13:46 +0000
50@@ -26,6 +26,7 @@
51 errors,
52 revisionspec,
53 )
54+import textwrap
55 """)
56
57 from bzrlib import (
58@@ -579,3 +580,34 @@
59 diff_writer_registry = _mod_registry.Registry()
60 diff_writer_registry.register('plain', lambda x: x, 'Plaintext diff output.')
61 diff_writer_registry.default_key = 'plain'
62+
63+
64+class SphinxHelpFormatter(optparse.IndentedHelpFormatter):
65+ """The default rest format for command line options fails to parse options
66+ with '.' in their name, so define a custom formatter to use the 'describe'
67+ directive in sphinx to replace it.
68+ """
69+
70+ def format_heading(self, heading):
71+ return "%*s:%s:\n" % (self.current_indent, "", heading)
72+
73+ def format_option(self, option):
74+ result = []
75+ result.append(
76+ '%*s.. cssclass:: bzr-option\n' % (self.current_indent, ''))
77+ opts = self.option_strings[option]
78+ opts = '%*s.. describe:: %s\n' % (self.current_indent, '', opts)
79+ result.append(opts)
80+ result.append('\n')
81+ self.indent()
82+ if option.help:
83+ help_text = self.expand_default(option)
84+ help_lines = textwrap.wrap(help_text, self.help_width)
85+ result.extend(['%*s%s\n' % (self.current_indent, '', line)
86+ for line in help_lines])
87+ self.dedent()
88+ result.append('\n')
89+ return ''.join(result)
90+
91+
92+
93
94=== added file 'doc/en/_static/bzr-docs.css'
95--- doc/en/_static/bzr-docs.css 1970-01-01 00:00:00 +0000
96+++ doc/en/_static/bzr-docs.css 2010-09-22 04:13:46 +0000
97@@ -0,0 +1,5 @@
98+dl.bzr-option dt tt.descname {
99+ font-family: monospace;
100+ font-weight: normal;
101+ font-size: 1em;
102+}
103
104=== modified file 'doc/en/_templates/layout.html'
105--- doc/en/_templates/layout.html 2010-03-02 10:21:39 +0000
106+++ doc/en/_templates/layout.html 2010-09-22 04:13:46 +0000
107@@ -1,5 +1,10 @@
108 {% extends "!layout.html" %}
109
110+{% block extrahead %}
111+{{ super() }}
112+<link rel="stylesheet" href="{{ pathto("_static/bzr-docs.css", 1) }}" />
113+{% endblock %}
114+
115 {% block rootrellink %}
116 <li><a href="http://bazaar.canonical.com/">
117 <img src="{{ pathto("_static/bzr icon 16.png", 1) }}" /> Home</a>&nbsp;|&nbsp;</li>