corinthia-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Kelly <pmke...@apache.org>
Subject Re: ODF Lenses
Date Mon, 06 Jul 2015 10:42:52 GMT
Hi Ian, looks decent so far. Just a few comments:

ODFLenses.h is missing from the CMakeLists.txt file. This won’t affect compilation at all,
but it does mean that it will now show up in the file list in IDEs like Xcode and Visual Studio.

I got a number of “control reaches end of non-void function” warnings, mostly in functions
marked TBD, like ODFPut and ODFCreate. I’d recommend setting a “not implemented” error
here using DFErrorFormat, and then returning 0.

There were also some warnings about incorrect use of format strings in ODFStyles.c. There’s
some places here where color macros like CYAN and RESET are used where in one instance they’re
part of the format string itself, and in others they’re passed as string parameters, like
the following:

    printf(CYAN "Create CSS Properties for %s\n", styleName, RESET);

This gives me a warning that not all the format parameters are used.

ODFContainerGet and ODFConainerPut:

These functions are only really relevant for "container" elements that can have a variable
set of children - that is, where the children can be added, removed, and rearranged. Where
this is not the case, it is possible (better? i'm not sure) to do this without the container
logic.

For example, <office:body> has exactly one child - either a <office:text>, <office:spreadsheet>,
or <office:presentation>. This determines the type of document. The code that's currently
there calls ODFContainerGet with the ODFTextLens function table as a parameter, and that in
turn calls ODFTextGet, which calls ODFContainerGet with an ODFTextLevelLens. Finally, this
results in ODFTextLevelGet being called. A more direct approach would be to simply call ODFTextGet
from ODFBodyGet, since we can deduce that's what will be called in all cases.

A case where you *would* need to use the generic container functions is for ODFTextLevelLens
(i.e. a containing element which can have multiple block-level elements such as paragraphs
and tables), and similarly for any element that can contain inline elements, like <text:h>,
<text:p>, and <text:span>.

On the topic of the latter, this should be handled recursively. Currently, ODFParagraphContentGet
only handlines child nodes that are text nodes. But if you have a line with multiple, different
formatting in it, e.g. bold and italic, then each of these will be a <text:span>. These
can be arbitrarily nested, unlike in Word, but like in HTML.

—
Dr Peter M. Kelly
pmkelly@apache.org

PGP key: http://www.kellypmk.net/pgp-key <http://www.kellypmk.net/pgp-key>
(fingerprint 5435 6718 59F0 DD1F BFA0 5E46 2523 BAA1 44AE 2966)

> On 4 Jul 2015, at 11:40 am, Ian C <ian@amham.net> wrote:
> 
> FYI
> 
> I just pushed a new branch ODFLenses so the world can see what I am doing.
> Currently it only works in one direction. ODF -> HTML.
> 
> Working on the return path now.
> 
> -- 
> Cheers,
> 
> Ian C


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message