Re: [Libreoffice] [Review] set all borders correctly in SvxRTFParser::ReadBorderAttr instead of only one
Hi Kendy, On Fri, 2011-05-27 at 07:43 +0200, Jan Holesovsky wrote: here is a short patch that sets all specified borders during parsing instead of only one border. The problem was that nBorderTyp was set for every border with the correct value but SetBorderLine was only called once after the do while loop. So now every time nBorderType will be overriden, I call SetBorderLine. If this patch is ok I think we should add it to the 3-4 branch. Cedric already approved the patch, Well, I haven't actually pushed the patch (had to check as I wasn't sure). I was about to do it but I found some remaining problems with some border properties not being copied properly (width for example). but I am wondering - before, the nBorderTyp was set only when bTableDef was true; after your patch, it is set regardless of the bTableDef value. Is that correct, or should that be in a block? If it is correct, can you please also change the indentation of the nBorderTyp = XYZ; part so that it does not look as if it is supposed to be part of the if ( bTableDef )? I'll to have a look at that... I can't remember the reason of this, but it looks weird to me. IMHO setting it in all cases should be better... but I need to dive again into RTF specs and that parser code (that I partly rewrote some time ago) -- Cédric Bosdonnat LibreOffice hacker http://documentfoundation.org OOo Eclipse Integration developer http://cedric.bosdonnat.free.fr ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [Review] set all borders correctly in SvxRTFParser::ReadBorderAttr instead of only one
Hello Kendy, Cedric, yes there might still be some problems, I only tried to fix that during pasting to calc all borders are drawn. I'm not familiar with the RTF spec and the old code(before Cedric refactored it) was really ugly and even used some gotos. This quick patch was more or less a result of https://bugs.freedesktop.org/show_bug.cgi?id=37429 as I noticed that borders were totally screwed up. If I can help in any way just mail me and I have a look at it. Regards, Markus 2011/5/27 Cedric Bosdonnat cedric.bosdonnat@free.fr Hi Kendy, On Fri, 2011-05-27 at 07:43 +0200, Jan Holesovsky wrote: here is a short patch that sets all specified borders during parsing instead of only one border. The problem was that nBorderTyp was set for every border with the correct value but SetBorderLine was only called once after the do while loop. So now every time nBorderType will be overriden, I call SetBorderLine. If this patch is ok I think we should add it to the 3-4 branch. Cedric already approved the patch, Well, I haven't actually pushed the patch (had to check as I wasn't sure). I was about to do it but I found some remaining problems with some border properties not being copied properly (width for example). but I am wondering - before, the nBorderTyp was set only when bTableDef was true; after your patch, it is set regardless of the bTableDef value. Is that correct, or should that be in a block? If it is correct, can you please also change the indentation of the nBorderTyp = XYZ; part so that it does not look as if it is supposed to be part of the if ( bTableDef )? I'll to have a look at that... I can't remember the reason of this, but it looks weird to me. IMHO setting it in all cases should be better... but I need to dive again into RTF specs and that parser code (that I partly rewrote some time ago) -- Cédric Bosdonnat LibreOffice hacker http://documentfoundation.org OOo Eclipse Integration developer http://cedric.bosdonnat.free.fr ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [Review] set all borders correctly in SvxRTFParser::ReadBorderAttr instead of only one
Hi Markus, On 2011-05-24 at 21:35 +0200, Markus Mohrhard wrote: here is a short patch that sets all specified borders during parsing instead of only one border. The problem was that nBorderTyp was set for every border with the correct value but SetBorderLine was only called once after the do while loop. So now every time nBorderType will be overriden, I call SetBorderLine. If this patch is ok I think we should add it to the 3-4 branch. Cedric already approved the patch, but I am wondering - before, the nBorderTyp was set only when bTableDef was true; after your patch, it is set regardless of the bTableDef value. Is that correct, or should that be in a block? If it is correct, can you please also change the indentation of the nBorderTyp = XYZ; part so that it does not look as if it is supposed to be part of the if ( bTableDef )? Thank you, Kendy ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Libreoffice] [Review] set all borders correctly in SvxRTFParser::ReadBorderAttr instead of only one
Hello, here is a short patch that sets all specified borders during parsing instead of only one border. The problem was that nBorderTyp was set for every border with the correct value but SetBorderLine was only called once after the do while loop. So now every time nBorderType will be overriden, I call SetBorderLine. If this patch is ok I think we should add it to the 3-4 branch. Regards, Markus 0001-set-all-border-lines-not-only-one.patch Description: Binary data ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
