|
»
»
Review Request: Patch to load correct version of textual content, load the texts themselves correctly and also some code cleanup.
|
View:
New views
5 Messages
—
Rating Filter:
Alert me
|
 |
Review Request: Patch to load correct version of textual content, load the texts themselves correctly and also some code cleanup.
by Vesa Pikki
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1919/ ----------------------------------------------------------- Review request for KOffice. Summary ------- This patch fixes font name loading, text bytes and chars loading. Powerpoint files contain version history of changes. Currently the oldest version of DocumentContainer is loaded. This patch mofies DocumentContainer loading to use latest version available. This in turn ensures that the latest texts and text styles are loaded. This patch renames some related datatypes to match those in powerpoint's definition to make it easier to follow the specification. Some powerpoint data structures were also modified to use QSharedData & QSharedDataPointer. Diffs ----- trunk/koffice/filters/kpresenter/powerpoint/import/powerpointimport.cc 1038383 trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.h 1038383 trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp 1038383 trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.h 1038383 trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp 1038383 trunk/koffice/filters/kpresenter/powerpoint/libppt/presentation.h 1038383 trunk/koffice/filters/kpresenter/powerpoint/libppt/presentation.cpp 1038383 Diff: http://reviewboard.kde.org/r/1919/diff Testing ------- Thanks, Vesa _______________________________________________ koffice-devel mailing list koffice-devel@... https://mail.kde.org/mailman/listinfo/koffice-devel |
|
|
Re: Review Request: Patch to load correct version of textual content, load the texts themselves correctly and also some code cleanup.
by Bugzilla from zander@kde.org
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1919/#review2738 ----------------------------------------------------------- trunk/koffice/filters/kpresenter/powerpoint/import/powerpointimport.cc <http://reviewboard.kde.org/r/1919/#comment2058> Instead of using replace, I think you should use QString::remove() trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp <http://reviewboard.kde.org/r/1919/#comment2059> This constructor should not be needed trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp <http://reviewboard.kde.org/r/1919/#comment2060> This constructor should not be needed trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp <http://reviewboard.kde.org/r/1919/#comment2061> This constructor should not be needed trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp <http://reviewboard.kde.org/r/1919/#comment2063> A QString does not have to be initialized. This line does nothing. trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp <http://reviewboard.kde.org/r/1919/#comment2062> This constructor should not be needed trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp <http://reviewboard.kde.org/r/1919/#comment2066> This constructor should not be needed trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp <http://reviewboard.kde.org/r/1919/#comment2064> A QString does not have to be initialized. This just slows things down :) trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp <http://reviewboard.kde.org/r/1919/#comment2065> This constructor should not be needed trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp <http://reviewboard.kde.org/r/1919/#comment2067> use name.clear() instead, which is much faster. trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp <http://reviewboard.kde.org/r/1919/#comment2068> This constructor should not be needed trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp <http://reviewboard.kde.org/r/1919/#comment2069> This constructor should not be needed trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp <http://reviewboard.kde.org/r/1919/#comment2070> This constructor should not be needed trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp <http://reviewboard.kde.org/r/1919/#comment2071> This constructor should not be needed trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp <http://reviewboard.kde.org/r/1919/#comment2072> unsigned char * ? Why the 'unsigned' ? trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp <http://reviewboard.kde.org/r/1919/#comment2073> that unsigned too is weird here... - Thomas On 2009-10-21 09:36:15, Vesa Pikki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1919/ > ----------------------------------------------------------- > > (Updated 2009-10-21 09:36:15) > > > Review request for KOffice. > > > Summary > ------- > > This patch fixes font name loading, text bytes and chars loading. > Powerpoint files contain version history of changes. Currently the oldest version of DocumentContainer is loaded. This patch mofies DocumentContainer loading to use latest version available. This in turn ensures that the latest texts and text styles are loaded. > > This patch renames some related datatypes to match those in powerpoint's definition to make it easier to follow the specification. > Some powerpoint data structures were also modified to use QSharedData & QSharedDataPointer. > > > Diffs > ----- > > trunk/koffice/filters/kpresenter/powerpoint/import/powerpointimport.cc 1038383 > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.h 1038383 > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp 1038383 > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.h 1038383 > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp 1038383 > trunk/koffice/filters/kpresenter/powerpoint/libppt/presentation.h 1038383 > trunk/koffice/filters/kpresenter/powerpoint/libppt/presentation.cpp 1038383 > > Diff: http://reviewboard.kde.org/r/1919/diff > > > Testing > ------- > > > Thanks, > > Vesa > > _______________________________________________ koffice-devel mailing list koffice-devel@... https://mail.kde.org/mailman/listinfo/koffice-devel |
|
|
Re: Review Request: Patch to load correct version of textual content, load the texts themselves correctly and also some code cleanup.
by Vesa Pikki
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1919/ ----------------------------------------------------------- (Updated 2009-10-22 08:53:17.277355) Review request for KOffice. Changes ------- Removed unnecessary constructors and string initializations. Also added some more comments to the code and applied astyle as is now in trunk as well. Summary ------- This patch fixes font name loading, text bytes and chars loading. Powerpoint files contain version history of changes. Currently the oldest version of DocumentContainer is loaded. This patch mofies DocumentContainer loading to use latest version available. This in turn ensures that the latest texts and text styles are loaded. This patch renames some related datatypes to match those in powerpoint's definition to make it easier to follow the specification. Some powerpoint data structures were also modified to use QSharedData & QSharedDataPointer. Diffs (updated) ----- trunk/koffice/filters/kpresenter/powerpoint/import/powerpointimport.cc 1038581 trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.h 1038581 trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp 1038581 trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.h 1038581 trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp 1038581 trunk/koffice/filters/kpresenter/powerpoint/libppt/presentation.h 1038581 trunk/koffice/filters/kpresenter/powerpoint/libppt/presentation.cpp 1038581 Diff: http://reviewboard.kde.org/r/1919/diff Testing ------- Thanks, Vesa _______________________________________________ koffice-devel mailing list koffice-devel@... https://mail.kde.org/mailman/listinfo/koffice-devel |
|
|
Re: Review Request: Patch to load correct version of textual content, load the texts themselves correctly and also some code cleanup.
by Vesa Pikki
::
Rate this Message:
> On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/import/powerpointimport.cc, line 1383 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13072#file13072line1383> > > > > Instead of using replace, I think you should use QString::remove() fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp, line 246 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13074#file13074line246> > > > > This constructor should not be needed fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp, line 317 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13074#file13074line317> > > > > This constructor should not be needed fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp, line 654 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13074#file13074line654> > > > > This constructor should not be needed fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp, line 696 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13074#file13074line696> > > > > A QString does not have to be initialized. This line does nothing. fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp, line 705 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13074#file13074line705> > > > > This constructor should not be needed fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp, line 1160 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13076#file13076line1160> > > > > This constructor should not be needed fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp, line 1171 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13076#file13076line1171> > > > > A QString does not have to be initialized. This just slows things down :) fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp, line 1180 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13076#file13076line1180> > > > > This constructor should not be needed fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp, line 1270 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13076#file13076line1270> > > > > use name.clear() instead, which is much faster. fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp, line 2893 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13076#file13076line2893> > > > > This constructor should not be needed fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp, line 3319 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13076#file13076line3319> > > > > This constructor should not be needed fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp, line 3759 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13076#file13076line3759> > > > > This constructor should not be needed fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp, line 3782 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13076#file13076line3782> > > > > This constructor should not be needed fixed > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp, line 5125 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13076#file13076line5125> > > > > unsigned char * ? Why the 'unsigned' ? The class is inherited from Record class that has a virtual method setData with also unsigned char*. I just followed the original method's way of using unsigned. > On 2009-10-21 11:08:49, Thomas Zander wrote: > > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp, line 5206 > > <http://reviewboard.kde.org/r/1919/diff/1/?file=13076#file13076line5206> > > > > that unsigned too is weird here... see earlier reply. - Vesa ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1919/#review2738 ----------------------------------------------------------- On 2009-10-22 08:53:17, Vesa Pikki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1919/ > ----------------------------------------------------------- > > (Updated 2009-10-22 08:53:17) > > > Review request for KOffice. > > > Summary > ------- > > This patch fixes font name loading, text bytes and chars loading. > Powerpoint files contain version history of changes. Currently the oldest version of DocumentContainer is loaded. This patch mofies DocumentContainer loading to use latest version available. This in turn ensures that the latest texts and text styles are loaded. > > This patch renames some related datatypes to match those in powerpoint's definition to make it easier to follow the specification. > Some powerpoint data structures were also modified to use QSharedData & QSharedDataPointer. > > > Diffs > ----- > > trunk/koffice/filters/kpresenter/powerpoint/import/powerpointimport.cc 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.h 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.h 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/presentation.h 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/presentation.cpp 1038581 > > Diff: http://reviewboard.kde.org/r/1919/diff > > > Testing > ------- > > > Thanks, > > Vesa > > _______________________________________________ koffice-devel mailing list koffice-devel@... https://mail.kde.org/mailman/listinfo/koffice-devel |
|
|
Re: Review Request: Patch to load correct version of textual content, load the texts themselves correctly and also some code cleanup.
by jos.van.den.oever
::
Rate this Message:
----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1919/#review2756 ----------------------------------------------------------- Ship it! all comments handled nicely lets get it in before the RC - vandenoever On 2009-10-22 08:53:17, Vesa Pikki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/1919/ > ----------------------------------------------------------- > > (Updated 2009-10-22 08:53:17) > > > Review request for KOffice. > > > Summary > ------- > > This patch fixes font name loading, text bytes and chars loading. > Powerpoint files contain version history of changes. Currently the oldest version of DocumentContainer is loaded. This patch mofies DocumentContainer loading to use latest version available. This in turn ensures that the latest texts and text styles are loaded. > > This patch renames some related datatypes to match those in powerpoint's definition to make it easier to follow the specification. > Some powerpoint data structures were also modified to use QSharedData & QSharedDataPointer. > > > Diffs > ----- > > trunk/koffice/filters/kpresenter/powerpoint/import/powerpointimport.cc 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.h 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/objects.cpp 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.h 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/powerpoint.cpp 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/presentation.h 1038581 > trunk/koffice/filters/kpresenter/powerpoint/libppt/presentation.cpp 1038581 > > Diff: http://reviewboard.kde.org/r/1919/diff > > > Testing > ------- > > > Thanks, > > Vesa > > _______________________________________________ koffice-devel mailing list koffice-devel@... https://mail.kde.org/mailman/listinfo/koffice-devel |
| Free embeddable forum powered by Nabble | Forum Help |
