----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111773/#review37053 -----------------------------------------------------------
Ship it! Soooo much nicer! Good work! This time I only had some nits to pick and some questions. But I don't see a need for another round after the issues are fixed. devtools/rng2cpp/rng2cpp.cpp <http://git.reviewboard.kde.org/r/111773/#comment27382> I'm not sure but wouldn't xor (^) be a better operator than +? devtools/rng2cpp/rng2cpp.cpp <http://git.reviewboard.kde.org/r/111773/#comment27383> Still would like some empty lines between classes. devtools/rng2cpp/rng2cpp.cpp <http://git.reviewboard.kde.org/r/111773/#comment27389> I think all these qDebugs should be disabled (commented out) before the merge. filters/stage/powerpoint/PptToOdp.cpp <http://git.reviewboard.kde.org/r/111773/#comment27391> I think these includes should be near the other library includes below and here, near the local includes. - Inge Wallin On Aug. 3, 2013, 2:44 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 Aug. 3, 2013, 2:44 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 425ac33 > libs/kotext/KoInlineNote.cpp 6faa9a9 > libs/odf/CMakeLists.txt 8549ace > libs/odf/writeodf/CMakeLists.txt PRE-CREATION > libs/odf/writeodf/README.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
