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
[email protected]
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 <[email protected]> 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
