Review Request: Improved html-odf Words export filter

C. Boemann cbr at boemann.dk
Tue Jan 24 10:19:24 GMT 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103778/#review10040
-----------------------------------------------------------


Lot's of style issues as you expected.

I fear the xsl is out of my league


filters/words/html-odf/ManifestParser.h
<http://git.reviewboard.kde.org/r/103778/#comment8245>

    spaces like this marked with red should be fixed all over- not going toment on them all



filters/words/html-odf/ManifestParser.h
<http://git.reviewboard.kde.org/r/103778/#comment8246>

    no space after ( or before )
    
    not going to comment on all issues



filters/words/html-odf/ManifestParser.h
<http://git.reviewboard.kde.org/r/103778/#comment8248>

    member vars really should be private with getters, asprotected breaks encapsulation, but it's not a really big issue for me



filters/words/html-odf/ManifestParser.h
<http://git.reviewboard.kde.org/r/103778/#comment8247>

    please only one space between
    
    not going to comment on all issues



filters/words/html-odf/ManifestParser.cpp
<http://git.reviewboard.kde.org/r/103778/#comment8249>

    maybe if possible
     = QString();



filters/words/html-odf/ManifestParser.cpp
<http://git.reviewboard.kde.org/r/103778/#comment8251>

    { on next line for methods



filters/words/html-odf/ManifestParser.cpp
<http://git.reviewboard.kde.org/r/103778/#comment8250>

    { on same line as if, for, while etc
    
    not going to comment on every issue



filters/words/html-odf/htmlodfexport.cpp
<http://git.reviewboard.kde.org/r/103778/#comment8252>

    Maybe we should get this done then..



filters/words/html-odf/htmlodfexport.cpp
<http://git.reviewboard.kde.org/r/103778/#comment8253>

    just a riminder of placing those { correctly


- C. Boemann


On Jan. 24, 2012, 10 a.m., Stuart Dickson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103778/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2012, 10 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> Improvements to the html-odf export filter.
> 
> Added support for: exporting images, tables with merged cells.
> Modifications to KoFilter derived class to incorporate Conversion class' convert function and provide improved error checking.
> Changes to output styles to improve legibility.
> Disabled export filter dialog as it does not have an impact on output, but can be modified in future. 
> Removing QChar::ObjectReplacementMarkers from appearing in output.
> 
> The focus has been on the content and not on styles and there is still improvement in cleaning up the generated output.
> 
> 
> Diffs
> -----
> 
>   filters/words/html-odf/CMakeLists.txt d1bee3f 
>   filters/words/html-odf/ManifestParser.h PRE-CREATION 
>   filters/words/html-odf/ManifestParser.cpp PRE-CREATION 
>   filters/words/html-odf/conversion/converter.xsl 1f01ed6 
>   filters/words/html-odf/convert.h 8cbdd6c 
>   filters/words/html-odf/convert.cpp 226fbd5 
>   filters/words/html-odf/htmlodfexport.h bff8dec 
>   filters/words/html-odf/htmlodfexport.cpp 5f0606e 
> 
> Diff: http://git.reviewboard.kde.org/r/103778/diff/diff
> 
> 
> Testing
> -------
> 
> Testing against a couple of sample documents including paragraphs, titles, simple tables and ones with additional merged cells, inline and paragraph anchored images.
> 
> 
> Thanks,
> 
> Stuart Dickson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120124/e45ecaf4/attachment.htm>


More information about the calligra-devel mailing list