corinthia-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jan i <j...@apache.org>
Subject Re: ODF Lenses
Date Mon, 06 Jul 2015 11:42:45 GMT
On 6 July 2015 at 12:42, Peter Kelly <pmkelly@apache.org> wrote:

> 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.
>
+1, I always get suspicious when I get that warning.

Should we move the code to trunk ? I suggest to do it.

rgds
jan i.


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