[prev in list] [next in list] [prev in thread] [next in thread]
List: koffice-devel
Subject: Re: Review Request: Hyperlink support for powerpoint filter.
From: "Amit Aggarwal" <amitcs06 () gmail ! com>
Date: 2010-01-27 17:13:01
Message-ID: 20100127171301.17369.65539 () localhost
[Download RAW message or body]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2737/#review3914
-----------------------------------------------------------
Please find my some comments
* @param text TextContainer to search hyperlinks from
Extra Please delete it.
HyperlinkRange findNextHyperlinkStart(const PPT::TextContainer& text,
Comment param is not proper
> writeTextSpan(KoXmlWriter& xmlWriter,
const QString &text)
this function is doing only simplified. Can we use QString::simplifed and remove \
this function ?
container->interactiveInfoAtom.action == 3
For this magic numbers can we use enum as per the MS-PPT specs
utf16ToString function
Can we use macro or inline function as it is very small ?
if (text.endsWith("\r")) {
text = text.left(text.length() - 1);
}
Instead of this I think QString::Simplified function will help ?
if (!getListStyleName(0, 0).isEmpty()) {
out.xml.addAttribute("text:style-name", getListStyleName(0, 0));
}
Wrong indentation
nameStr.remove('\r');
nameStr.remove('\v');
Not required. getText instead of returing QString directly we can assign QString into \
variable and use simplified function.
ex:- QString text = QString::fromAscii(textChars, textChars.size());
text = text.simplified();
return text;
- Amit
On 2010-01-27 11:43:04, Vesa Pikki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2737/
> -----------------------------------------------------------
>
> (Updated 2010-01-27 11:43:04)
>
>
> Review request for KOffice.
>
>
> Summary
> -------
>
> Here is a patch for adding a rudimentary support for hyperlinks within powerpoint \
> files in powerpoint filter.
>
> Diffs
> -----
>
> trunk/koffice/filters/kpresenter/powerpoint/mso/simpleParser.h 1080994
> trunk/koffice/filters/kpresenter/powerpoint/mso/simpleParser.cpp 1080994
> trunk/koffice/filters/kpresenter/powerpoint/PptToOdp.cpp 1080994
> trunk/koffice/filters/kpresenter/powerpoint/PptToOdp.h 1080994
>
> Diff: http://reviewboard.kde.org/r/2737/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Vesa
>
>
_______________________________________________
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