Re: [Docutils-develop] Adding rst2odt/odtwriter to the queue for the next release

[David G. <go...@py...>]

On Wed, Oct 1, 2008 at 20:02, Dave Kuhlman <dku...@re...> wrote:
> David Goodger <goodger <at> python.org> writes:
>>
>> On Mon, Sep 29, 2008 at 13:52, Dave Kuhlman wrote:
>> > I'd like rst2odt/odtwriter to be considered for the the next
>> > release of Docutils.
>>
>> We do have documentation for this, at
>> http://docutils.sourceforge.net/docs/dev/policies.html#check-ins
>
> I'll try to follow that guidance on check-ins.
>
>> > Currently, when I run rst2odt.py on demo.txt, it successfully runs;
>> > it produces an output file that looks good to me; and it shows me
>> > that rst2odt does not support citations and sidebars.  Are these
>> > required for inclusion?
>>
>> No, it's not required that a writer be finished, just that it be
>> reasonably complete:
>>
>> """
>> Reasonably complete means that the code must handle all input. Here
>> "handle" means that no input can cause the code to fail (cause an
>> exception, or silently and incorrectly produce nothing). "Reasonably
>> complete" does not mean "finished" (no work left to be done). For
>> example, a writer must handle every standard element from the Docutils
>> document model; for unimplemented elements, it must at the very least
>> warn that "Output for element X is not yet implemented in writer Y".
>> """
>>
>> Of course, completeness ("finished") is a desirable goal. Once a
>> writer is in the core, that ought to happen.
>>
>> > I suppose that I am asking how we decide whether to include it.  I
>> > suspect that there needs to be some demand for it and that it needs
>> > to pass certain tests.  I believe that David (G.) said that the
>> > ability to process demo.txt without failing was a requirement,
>> > so it has passed that test.
>> >
>> > So, is there anything I can do to move this forward or at least
>> > have it considered?
>>
>> The writer should have at least a functional test to catch
>> regressions. The ODT writer may need a test fixture or special
>> processing to generate a useful result, since two zip files differing
>> is not very useful -- the test should say *how* they differ. See
>> http://docutils.sourceforge.net/docs/dev/testing.html
>
> I'm looking at the files in docutils/test/test_writers.  I'll try
> to mimic one of those.

Those are actually a bunch of unique tests, not applicable to all
writers. The ODT writer may need some of those (e.g. to test for the
contents of the zip files, to test settings, etc.). But the main tests
a writer needs are functional tests, which are described here:
http://docutils.sourceforge.net/docs/dev/testing.html#functional-tests
They run the entire Docutils system (in various configurations, each
defined in a file in tests/functional/tests/) on input files (in
tests/functional/input/) and compare the resulting output to a
pre-defined expected output files (in tests/functional/expected).

> The content we'd need to check is in content.xml inside that zipped
> .odt file.  That XML is all on one line.  So, I'll look into
> unzipping and then pretty printing it (perhaps with minidom).  That
> would give the test something reasonable to compare against.

Yes. And the functional test framework (tests/test_funcitonal.py) will
need to be modified to provide useful results for ODT's compound zip
files.

> About non-standard code, I've tried to follow PEP 8 ("Style Guide
> for Python Code") as much as I can.  I've also looked at code in
> the Docutils implementation, and the code in odtwriter seems
> consistent with that.  Can you think of anything in particular that
> I should look for?  A hint would help.  It's hard to see the flaws
> in your own baby.

Everybody has a slightly different style. It may seem nit-picky, but
adhering to the standard makes code easier for everyone to read. Some
areas that I notice right away:

* Vertical whitespace: one blank line between functions, two between
classes. Don't have multiple statements on one line (e.g.
ListLevel.get_sibling etc.).

* Backslashes: use parentheses instead.

* Use "raise Exception('message')" instead of the older form "raise
Exception, 'message'".

* Line length: keep lines under 80 columns (the line wrapping throws
off the "code shape" sense).

>> * The conditional classes seem kludgey to me; why not break them out
>> into another module, and import that conditionally?
>
> One of those conditionally defined classes was there because
> directives were not supported in Docutils prior to 0.4, I believe.

They were supported in a different way. The new API is class-based.
The old one was function-based.

> I don't believe that we need to worry about that now.  So, I
> removed the "if" statement.  I'll also look at the others.

1. Yes, there's no need to check the API for directives any more.
Better to track trunk.

2. There shouldn't be any directive code in a writer though.
Directives are for the reST parser only. A syntax highlighting
directive is a common request, and there are several implementations
out there. Let's work on generalizing the directive for all of
Docutils (so it works with all writers), and move it into the parser
code.

-- 
David Goodger <http://python.net/~goodger>