> On Aug. 2, 2013, 8:35 p.m., Inge Wallin wrote: > > I have looked through this rather big patch, and I must say that in many > > places where these writers are used it results in easier to read code. I > > also like that it's possible to mix the old manual and this new automatic > > style of writing ODF. So in general I'm in favour of this patch and the > > ideas behind it. > > > > But I think that if we want to have this in libs/odf we need more > > documentation. I miss a general overview of the classes and how they are > > supposed to be used together. I also miss other types of API docs even if > > it's difficult to say exactly where it should be put since we are talking > > about automatically generated code here. Finally, the code should follow > > the coding standards of Calligra. See comments below. > > > > I also have a question: Will this be able to handle extensions to ODF, > > like the special tags that we write in some of our filters to create better > > compatibility with MS Office documents? > > > > In conclusion I think it's very promising but not ready for merge yet.
I've added a document that explains how the headers work. This code can handle extension to the RNG. Just add them to the file ./devtools/scripts/OpenDocument-v1.2-cs01-schema-calligra.rng It might even be possible to use it with the RNG files of OOXML. > On Aug. 2, 2013, 8:35 p.m., Inge Wallin wrote: > > devtools/rng2cpp/rng2cpp.cpp, line 8 > > <http://git.reviewboard.kde.org/r/111773/diff/1/?file=174558#file174558line8> > > > > I think some small documentation of FULL and SUB wouldn't hurt. I try > > to look at the code below (void test()...) and I don't understand the > > purpose. Junk that I removed now. > On Aug. 2, 2013, 8:35 p.m., Inge Wallin wrote: > > filters/libmso/shapes.cpp, line 132 > > <http://git.reviewboard.kde.org/r/111773/diff/1/?file=174560#file174560line132> > > > > This may be a personal thing but wouldn't it be easier if this was > > written: > > > > draw_enhanced_geometry eg = rect.add_draw_enhanced_geometry(); > > > > ? That's a good option too, until we have std::move from c++11 it's more expensive though. > On Aug. 2, 2013, 8:35 p.m., Inge Wallin wrote: > > filters/sheets/excel/import/excelimporttoods.cc, line 1309 > > <http://git.reviewboard.kde.org/r/111773/diff/1/?file=174563#file174563line1309> > > > > This seems to be a good place to ask this question, but it's not > > specific to these lines: > > > > Is it possible to add convenience functions to write a bool (for > > instance) so we don't have to use this ugly way: > > > > <boolexpr> ? "true" : "false" > > > > ? Yes, that's possible once we have support for the value types. Currently that is not there yet, although parts towards it are in already. - Jos ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111773/#review37001 ----------------------------------------------------------- On July 28, 2013, 9:47 p.m., Jos van den Oever wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111773/ > ----------------------------------------------------------- > > (Updated July 28, 2013, 9:47 p.m.) > > > Review request for Calligra. > > > Description > ------- > > This patch is also available in the branch libs-writeodf-vandenoever-2. > > Two years ago I wrote an initial version of this patch and a detailed > discussion on the mailing list [1] followed. The main objections to the patch > have been dealt with (see below). > Most of this new version was written at Akademy in Bilbao. > > Very short summary of the patch: > This patch should help everybody, young and old, with coding C++ for writing > ODF and make errors easier to catch. > The OpenDocument Format specification is published with a Relax NG file that > specifies the XML format. This file can be used to check if ODF files are > valid. It can also be used to generate a C++ API headers. This is what this > patch does. > > Example: > Instead of writing: > == > xmlWriter->startElement("text:p"); > xmlWriter->addAttribute("text:style-name", "italic"); > xmlWriter->startElement("text:p"); > xmlWriter->addAttribute("text:style-name", "bold"); > xmlWriter->addTextNode("Hello World!"); > xmlWriter->endElement(); > xmlWriter->endElement(); > == > you can write: > == > text_p p(xmlWriter); > p.set_text_style_name("italic"); > text_span span(p.add_text_span()); > span.set_text_style_name("italic"); > span.addTextNode("Hello World!"); > == > > Some advantages: > - autocompletion when coding: faster coding > - tag and attribute names are not strings but class and function names: less > errors > - nesting is checked by the compiler > - you write to elements (span, p), not xmlwriter: easier to read > - required attributes are part of the element constructor > > Implementation considerations: > - Calligra is large, so the generated code mixes well with the use of > KoXmlWriter and porting can be done in small steps. > - class and function names are similar to the xml tags with ':' and '-' > replaced by '_'. > - stack based: no heap allocations > - only header files: all code will inline and have low impact on runtime > - modular: one header file per namespace to reduce compile overhead > - code generator is Qt code, part of Calligra and runs as a build step > > Not in this patch (and places where you can help in the future): > - generate enumerations based on Relax NG > - check data type for attributes (you can still write "hello" to an integer > attribute) > - complete port of Calligra to the generated code > - improved speed by using static QString instead of const char* > > Provided solutions to previously raised issues: > - modular headers to reduce compile overhead > - function "end()" to optionally close an element before it goes out of scope > - use structure of Relax NG file to reduce header sizes by inheritance from > common groups > - provide most KoXmlWriter functionality safely through the element instances > - closing elements is now automatic at a slight runtime overhead > > > [1] http://lists.kde.org/?t=130768700500002 > > > Diffs > ----- > > devtools/CMakeLists.txt 15008fb > devtools/rng2cpp/CMakeLists.txt PRE-CREATION > devtools/rng2cpp/rng2cpp.cpp PRE-CREATION > filters/libmso/CMakeLists.txt 6bc145f > filters/libmso/shapes.cpp 073e061 > filters/libmso/shapes2.cpp 0f0b906 > filters/sheets/excel/import/CMakeLists.txt 2466218 > filters/sheets/excel/import/excelimporttoods.cc de788d4 > filters/stage/powerpoint/PptToOdp.h 8d85c1f > filters/stage/powerpoint/PptToOdp.cpp 9258564 > libs/kotext/KoInlineNote.cpp 6faa9a9 > libs/odf/CMakeLists.txt a2e3695 > libs/odf/writeodf/CMakeLists.txt PRE-CREATION > libs/odf/writeodf/helpers.h PRE-CREATION > libs/odf/writeodf/odfwriter.h PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/111773/diff/ > > > Testing > ------- > > Opened several ppt and xls files. > Checked ppt conversion for two files and checked that XML was equivalent. > > > Thanks, > > Jos van den Oever > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
