[Libreoffice] [REVIEW][3-5] Fix pivot table's date and number grouping, take 2

2012-01-24 Thread Kohei Yoshida
Hi there,

I've found a better way to fix it.  Attached is tailored specifically
for the 3-5 branch.

I'll port this fix to master separately since master has my earlier
commit that would make this patch not apply without adjusting.

Again, this fixes

https://bugs.freedesktop.org/show_bug.cgi?id=45067

Since we've already branched for 3.5.0, I'll need three sign-off's to
get this piece into 3.5.0 final.  Reviews and sign-off will be
appreciated.

Kohei

P.S. This NUMERIC flag was used exactly for this purpose prior to the
rework of the data pilot data cache code done by IBM/Sun back in the old
days (around the OOo 3.3 release period).  During the rework this check
was unfortunately removed, which probably introduced the original issue
I "fixed" for fdo#42169 (though apparently my fix was not entirely
correct).

-- 
Kohei Yoshida, LibreOffice hacker, Calc
>From 6bdd9f6d5c4bfd9066384539454fad87eeebafc8 Mon Sep 17 00:00:00 2001
From: Kohei Yoshida 
Date: Tue, 24 Jan 2012 16:32:10 -0500
Subject: [PATCH] fdo#45067: Differentiate numeric and non-numeric field member values.

---
 sc/source/core/data/dpoutput.cxx |   14 +++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/sc/source/core/data/dpoutput.cxx b/sc/source/core/data/dpoutput.cxx
index 990e212..32179e3 100644
--- a/sc/source/core/data/dpoutput.cxx
+++ b/sc/source/core/data/dpoutput.cxx
@@ -795,10 +795,18 @@ void ScDPOutput::HeaderCell( SCCOL nCol, SCROW nRow, SCTAB nTab,
 
 if ( nFlags & sheet::MemberResultFlags::HASMEMBER )
 {
-// Avoid unwanted automatic format detection.
+bool bNumeric = (nFlags & sheet::MemberResultFlags::NUMERIC) != 0;
 ScSetStringParam aParam;
-aParam.mbDetectNumberFormat = false;
-aParam.mbSetTextCellFormat = true;
+if (bNumeric)
+{
+aParam.mbDetectNumberFormat = true;
+aParam.mbSetTextCellFormat = false;
+}
+else
+{
+aParam.mbDetectNumberFormat = false;
+aParam.mbSetTextCellFormat = true;
+}
 pDoc->SetString(nCol, nRow, nTab, rData.Caption, &aParam);
 }
 
-- 
1.7.3.4

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [REVIEW][3-5] Fix pivot table's date and number grouping

2012-01-24 Thread Kohei Yoshida
On Tue, 2012-01-24 at 12:24 +0100, Eike Rathke wrote:
> Hi,
> 
> On Tuesday, 2012-01-24 11:42:57 +0100, Markus Mohrhard wrote:
> 
> >  Did you check with libreoffice 3-5? I think that eike's work for date
> > patterns is missing in 3-5
> 
> Yes, that's only on master.
> 
> > so that this patch does reopen the bug we
> > discussed on IRC. At least your unit test fails and I think we should
> > check that before adding the patch to 3-5.
> 
> Relying on date patterns sounds very fragile, they vary between locales
> and may change at any time.

Sure, but that's how it used to work.  It's fragile, yes, but to fix it
properly we'd have to make more invasive changes, and definitely not for
3.5.

-- 
Kohei Yoshida, LibreOffice hacker, Calc

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [REVIEW][3-5] Fix pivot table's date and number grouping

2012-01-24 Thread Eike Rathke
Hi,

On Tuesday, 2012-01-24 11:42:57 +0100, Markus Mohrhard wrote:

>  Did you check with libreoffice 3-5? I think that eike's work for date
> patterns is missing in 3-5

Yes, that's only on master.

> so that this patch does reopen the bug we
> discussed on IRC. At least your unit test fails and I think we should
> check that before adding the patch to 3-5.

Relying on date patterns sounds very fragile, they vary between locales
and may change at any time.

  Eike

-- 
LibreOffice Calc developer. Number formatter stricken i18n transpositionizer.
GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3  9E96 2F1A D073 293C 05FD


pgp80WFH7OTZ0.pgp
Description: PGP signature
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [Libreoffice] [REVIEW][3-5] Fix pivot table's date and number grouping

2012-01-24 Thread Markus Mohrhard
Hello Kohei,

2012/1/24 Kohei Yoshida :
> Hi there,
>
> I would like
>
> http://cgit.freedesktop.org/libreoffice/core/commit/?id=2c659e692a17cc10d364a2304bead9403fc6bdde
>
> to be backported to the 3-5 branch.  It fixes
>
> https://bugs.freedesktop.org/show_bug.cgi?id=45067
>
> Review and sign-off appreciated.
>

 Did you check with libreoffice 3-5? I think that eike's work for date
patterns is missing in 3-5 so that this patch does reopen the bug we
discussed on IRC. At least your unit test fails and I think we should
check that before adding the patch to 3-5.

Regards,
Markus
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


[Libreoffice] [REVIEW][3-5] Fix pivot table's date and number grouping

2012-01-23 Thread Kohei Yoshida
Hi there,

I would like

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2c659e692a17cc10d364a2304bead9403fc6bdde

to be backported to the 3-5 branch.  It fixes

https://bugs.freedesktop.org/show_bug.cgi?id=45067

Review and sign-off appreciated.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice