Thanks for the feedback. Re warnings, I don't see any in the ODF code, I get a couple in minizip.
Am I missing some compiler/make switches I wonder. My compiler is gcc version 4.9.2 20150212 (Red Hat 4.9.2-6) (GCC) If I'd seen the warnings I would have addressed them. On Mon, Jul 6, 2015 at 7:42 PM, jan i <j...@apache.org> wrote: > On 6 July 2015 at 12:42, Peter Kelly <pmke...@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 >> pmke...@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 <i...@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 >> >> -- Cheers, Ian C
