Re: [REVIEW 3-6, 3-6-0] [PUSHED 3-6] resolved fdo#52205 do not force all text cells in CSV import
On 07/19/2012 12:31 PM, Eike Rathke wrote: Hi, Please review and cherry-pick to 3-6 and 3-6-0 http://cgit.freedesktop.org/libreoffice/core/commit/?id=b7dbc768a71ccfb567e3b2979e57d0d1318977cf that fixes https://bugs.freedesktop.org/show_bug.cgi?id=52205 I reviewed and tested several csv files and one related pivot table use case, and it did fix the bug. The old behavior for ScSetStringParam::mbSetTextCellFormat was somewhat confusing and there was a reason for that. I now slightly changed the behavior of that flag and you commit kept that new behavior. But I couldn't reproduce the original issue that the old behavior was intended to fix, so I think it's safe to keep the new behavior. Pushed to the -3-6 branch with my sign-off. We need 2 more sign-offs to push this to the -3-6-0 branch (I assume?). Kohei -- Kohei Yoshida, LibreOffice hacker, Calc ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [REVIEW 3-6, 3-6-0] [PUSHED 3-6] resolved fdo#52205 do not force all text cells in CSV import
Hey, 2012/7/20 Kohei Yoshida kohei.yosh...@gmail.com: On 07/19/2012 12:31 PM, Eike Rathke wrote: Hi, Please review and cherry-pick to 3-6 and 3-6-0 http://cgit.freedesktop.org/libreoffice/core/commit/?id=b7dbc768a71ccfb567e3b2979e57d0d1318977cf that fixes https://bugs.freedesktop.org/show_bug.cgi?id=52205 I reviewed and tested several csv files and one related pivot table use case, and it did fix the bug. The old behavior for ScSetStringParam::mbSetTextCellFormat was somewhat confusing and there was a reason for that. I now slightly changed the behavior of that flag and you commit kept that new behavior. But I couldn't reproduce the original issue that the old behavior was intended to fix, so I think it's safe to keep the new behavior. Pushed to the -3-6 branch with my sign-off. We need 2 more sign-offs to push this to the -3-6-0 branch (I assume?). Looks good to me. One more needed for 3-6-0. ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
ScSetStringParam::mbSetTextCellFormat (was: [REVIEW 3-6, 3-6-0] [PUSHED 3-6] resolved fdo#52205 do not force all text cells in CSV import)
Hi Kohei, On Friday, 2012-07-20 09:25:54 -0400, Kohei Yoshida wrote: The old behavior for ScSetStringParam::mbSetTextCellFormat was somewhat confusing and there was a reason for that. I now slightly changed the behavior of that flag and you commit kept that new behavior. I inspected places where mbSetTextCellFormat is used and in sc/source/filter/rtf/eeimpars.cxx line 334 in ScEEImport::WriteToDocument() the if(bSimple) case looks similar suspicious, that's used for RTF and HTML import. But there's no extra handling for a forced text case as it is in CSV import, so this may be not as easy to fix. I didn't investigate yet deeper, just imported a simple number from HTML, and that indeed gets imported as text now. Eike -- LibreOffice Calc developer. Number formatter stricken i18n transpositionizer. GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD pgpe2vS7nB2Qn.pgp Description: PGP signature ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
