[prev in list] [next in list] [prev in thread] [next in thread]
List: koffice-devel
Subject: Re: Review Request: ToC generation step
From: "Thomas Zander" <zander () kde ! org>
Date: 2010-01-27 18:10:42
Message-ID: 20100127181042.18606.59552 () localhost
[Download RAW message or body]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2734/#review3918
-----------------------------------------------------------
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3291>
Only includes that are in the same directory should have the double quotes; \
koshape is in flake, so use <>
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3292>
Why is this in kotext and not in kword?
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3293>
That user visible string looks very odd. Please don't camelCase.
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3295>
its a list, make the variable name plural.
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3296>
Hardcoding the style looks wrong.
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3297>
Watch your coding style, there should be a space between the if and the open \
brace.
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3305>
I don't understand why you have a list, why don't you just insert at the right \
position directly?
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3298>
Using outline level is not right, items that go into the ToC don't have to have a \
number. Also using 'intProperty' looks wrong; use hasProperty().
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3299>
use single quotes around a single character. (the tab).
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3300>
Hardcoding the style looks wrong.
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3302>
hardcoding this looks wrong.
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3303>
use a section number in the debug
trunk/koffice/libs/kotext/KoTextEditor.cpp
<http://reviewboard.kde.org/r/2734/#comment3304>
For speed reasons you should use a const QString &
and please don't use single character variable names.
trunk/koffice/libs/kotext/styles/KoParagraphStyle.cpp
<http://reviewboard.kde.org/r/2734/#comment3306>
why did you change this?
trunk/koffice/plugins/textshape/TextTool.cpp
<http://reviewboard.kde.org/r/2734/#comment3307>
into -> in
- Thomas
On 2010-01-27 10:41:29, Jean-Nicolas Artaud wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2734/
> -----------------------------------------------------------
>
> (Updated 2010-01-27 10:41:29)
>
>
> Review request for KOffice.
>
>
> Summary
> -------
>
> Here Is the ToC generation step.
>
> http://img687.imageshack.us/img687/5207/tocgeneration.png
>
> I think somethings have to be improved:
> * The style for instance are not really adapted, but if you open existing contents \
> styles, it works really good (see the screenshots).
> * pages number is not actually updated because a new signal is necessary : \
> layoutFinished
>
> Diffs
> -----
>
> trunk/koffice/kword/part/KWDocument.cpp 1079518
> trunk/koffice/kword/part/kword.rc 1079518
> trunk/koffice/libs/kotext/KoTextEditor.h 1080908
> trunk/koffice/libs/kotext/KoTextEditor.cpp 1080908
> trunk/koffice/libs/kotext/styles/KoParagraphStyle.h 1080908
> trunk/koffice/libs/kotext/styles/KoParagraphStyle.cpp 1080908
> trunk/koffice/plugins/textshape/TextTool.h 1079518
> trunk/koffice/plugins/textshape/TextTool.cpp 1079518
>
> Diff: http://reviewboard.kde.org/r/2734/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Jean-Nicolas
>
>
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic