corinthia-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ian C <...@amham.net>
Subject Re: ODF Lenses
Date Mon, 06 Jul 2015 13:04:58 GMT
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 <jani@apache.org> wrote:
> 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
>>
>>



-- 
Cheers,

Ian C

Mime
View raw message