Bazaar

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

html-cmd-help
Merge into bzr.dev

Proposed by Andrew Bennetts on 2010-09-22

Status:	Work in progress
Proposed branch:	lp://staging/~spiv/bzr/html-cmd-help
Merge into:	lp://staging/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://staging/~spiv/bzr/html-cmd-help
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Martin Pool		2010-09-22	Approve on 2010-09-22
Review via email: mp+36244@code.staging.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.

Revision history for this message

Martin Pool (mbp) on 2010-09-22:

review: Approve

lp://staging/~spiv/bzr/html-cmd-help updated on 2010-09-22

5439. By Andrew Bennetts on 2010-09-22: Add NEWS entry.

Revision history for this message

Andrew Bennetts (spiv) wrote on 2010-09-22:

sent to pqm by email

lp://staging/~spiv/bzr/html-cmd-help updated on 2010-09-22

5440. By Andrew Bennetts on 2010-09-22: Do not use SphinxHelpFormatter if get_help_text was asked for plain output.

Revision history for this message

Vincent Ladeuil (vila) wrote on 2010-09-22:

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 on 2010-09-22:

[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 on 2010-09-22:

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 on 2010-09-22:

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

Revision history for this message

Andrew Bennetts (spiv) wrote on 2010-09-24:

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 on 2010-09-24:

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.