Re: [Libreoffice] Change in addition for date handling (intentional change??)

Hi Andrew,

I don't know any about the content, but I have searched, when the 
changes where made:

The part
    // #45465 Date needs with "+" a special handling: forces date type
              if( GetType() == SbxDATE || rOp.GetType() == SbxDATE )
                 aL.eType = SbxDATE;
moved from after the switch inside the switch with commit
1421b098ae19a10acb4836fa3752188cf7c52eb2
by Noel Power 06-Dec-2010

and the "#if 0" is a WaE fix by Tor Lillqvist 06-Oct-2011
8529da08517b41bd9317714e3216bb6d487b24ee

So I think, Noel should know.

Kind regards
Regina

Andrew Douglas Pitonyak schrieb:
> I thought I found a bug, I decided to take a crack at fixing the bug,
> and then I became confused because the bug appears to be intentional.
> Let me explain:
>
> Consider some comments related to Basic.
>
> In the last release of OOo, the statement "Now + 2" returns a date /
> time that is two days later than now. In LibreOffice, the same statement
> appears to return the double representation of the same. Now, off hand,
> this appears fine, until one tries to do something like this (which
> works in OOo, but fails in LO)
>
> DateValue(Now + 2)
>
> This is no big deal if I am writing new code, because I can compensate:
> DateValue(CDate(Now + 2)), but t kills legacy code.
>
> I started poking through the code and I found
> core/basic/source/sbx/sbxvalue.cxx
>
> In the version of the LO code that I have, around line 1312, I see the
> following:
>
> case SbxPLUS:
> aL.nDouble += aR.nDouble; break;
> #if 0
> // See 'break' on preceding line... this
> // is unreachable code. Do not delete this
> // #if 0 block unless you know for sure
> // the 'break' above is intentional.
>
> // #45465 Date needs with "+" a special handling: forces date type
> if( GetType() == SbxDATE || rOp.GetType() == SbxDATE )
> aL.eType = SbxDATE;
> #endif
>
> In OOo, the code ends at the break, but following the switch statement,
> there is the following check:
>
> // #45465 Date braucht bei + eine Spezial-Behandlung
> if( eOp == SbxPLUS && (GetType() == SbxDATE || rOp.GetType() == SbxDATE ) )
> aL.eType = SbxDATE;
>
> So, it looks like someone did a code clean-up and the special handling
> code was moved into the switch statement and then it was commented out.
>
> I was so proud that I figured out where this occurred, but, I am a bit
> stymied as to what happened here. It seems that I have almost nothing to
> fix because someone did this intentionally, or was this left as is
> because whomever made the change did not understand why the code was
> there....
>
> Sorry, just a bit confused as to what should be done. My opinion is that
> a bug was introduced and that the fix is clear, move the break and let
> the code run...
>
> case SbxPLUS:
> aL.nDouble += aR.nDouble;
> // #45465 Date needs with "+" a special handling: forces date type
> if( GetType() == SbxDATE || rOp.GetType() == SbxDATE )
> aL.eType = SbxDATE;
> break;
>
> So, what am I missing?