Hi Winfried, On Thursday, 2012-06-07 14:51:28 +0200, Winfried Donkers wrote:
This time the patch is the one that covers XOR.
Looks good in general, some nitpicks (as usual ;-)
--- a/formula/inc/formula/compiler.hrc +++ b/formula/inc/formula/compiler.hrc @@ -89,7 +89,8 @@ #define SC_OPCODE_INTERSECT 54 #define SC_OPCODE_UNION 55 #define SC_OPCODE_RANGE 56 -#define SC_OPCODE_STOP_BIN_OP 57 +#define SC_OPCODE_XOR 57 +#define SC_OPCODE_STOP_BIN_OP 58
Having AND and OR as binary operators is a legacy we should not follow with new functions like XOR. It's necessary to load existing documents that use this inlining, but further work even is required to store such in ODF compliance as the inlining operators are not defined there. So just add the new opcode to the 2-and-more-parameters section.
--- a/formula/source/core/api/FormulaCompiler.cxx
+++ b/formula/source/core/api/FormulaCompiler.cxx
@@ -1128,6 +1129,7 @@ void FormulaCompiler::Factor()
|| eOp == ocMacro
|| eOp == ocAnd
|| eOp == ocOr
+ || eOp == ocXor
|| eOp == ocBad
|| ( eOp >= ocInternalBegin && eOp <= ocInternalEnd )
|| (bCompileForFAP && ((eOp == ocIf) || (eOp == ocChose)))
Then adding ocXor is unnecessary there.
@@ -1440,7 +1442,7 @@ OpCode FormulaCompiler::Expression()
return ocStop; //! generate token instead?
}
NotLine();
- while (pToken->GetOpCode() == ocAnd || pToken->GetOpCode() == ocOr)
+ while (pToken->GetOpCode() == ocAnd || pToken->GetOpCode() == ocOr || pToken->GetOpCode() ==
ocXor)
{
FormulaTokenRef p = pToken;
pToken->SetByte( 2 ); // 2 parameters!
And there.
@@ -1617,7 +1619,7 @@ FormulaToken* FormulaCompiler::CreateStringFromToken( rtl::OUStringBuffer&
rBuff
bool bSpaces = false;
FormulaToken* t = pTokenP;
OpCode eOp = t->GetOpCode();
- if( eOp >= ocAnd && eOp <= ocOr )
+ if( eOp >= ocAnd && eOp <= ocXor )
{
// AND, OR infix?
if ( bAllowArrAdvance )
And there.
@@ -1822,8 +1824,8 @@ OpCode FormulaCompiler::NextToken()
else
{
// Before an operator there must not be another operator, with the
- // exception of AND and OR.
- if ( eOp != ocAnd && eOp != ocOr &&
+ // exception of AND, OR and XOR.
+ if ( eOp != ocAnd && eOp != ocOr && eOp != ocXor &&
(SC_OPCODE_START_BIN_OP <= eOp && eOp < SC_OPCODE_STOP_BIN_OP )
&& (eLastOp == ocOpen || eLastOp == ocSep ||
(SC_OPCODE_START_BIN_OP <= eLastOp && eLastOp < SC_OPCODE_STOP_UN_OP)))
And there.
--- a/formula/source/core/api/token.cxx
+++ b/formula/source/core/api/token.cxx
@@ -90,7 +90,7 @@ bool FormulaToken::IsFunction() const
|| (SC_OPCODE_START_2_PAR <= eOp && eOp < SC_OPCODE_STOP_2_PAR) // x parameters
(cByte==0 in
// FuncAutoPilot)
|| eOp == ocMacro || eOp == ocExternal // macros, AddIns
- || eOp == ocAnd || eOp == ocOr // former binary, now x
parameters
+ || eOp == ocAnd || eOp == ocOr || eOp == ocXor // former binary, now x
parameters
|| eOp == ocNot || eOp == ocNeg // unary but function
|| (eOp >= ocInternalBegin && eOp <= ocInternalEnd) // internal
));
And there. ;-)
--- a/sc/inc/helpids.h +++ b/sc/inc/helpids.h @@ -88,7 +88,6 @@ #define HID_SC_FORM_ARGS "SC_HID_SC_FORM_ARGS" #define HID_SCPAGE_SORT_FIELDS "SC_HID_SCPAGE_SORT_FIELDS" #define HID_SCPAGE_SORT_OPTIONS "SC_HID_SCPAGE_SORT_OPTIONS" -#define HID_SCPAGE_SORTKEY_FIELDS "SC_HID_SCPAGE_SORTKEY_FIELDS"
Huh? Accidentally hit the delete line key in your editor?
--- a/sc/source/core/tool/interpr1.cxx +++ b/sc/source/core/tool/interpr1.cxx +void ScInterpreter::ScXor() [...] + short nRes = 0;
Using a short here and incrementing it for values!=0 may easily overflow
if ranges are passed. A better approach would be to use the C++ ^ xor
operator, e.g.
short nRes = 0;
nRes ^= (PopDouble() != 0.0);
--- a/sc/source/core/tool/parclass.cxx
+++ b/sc/source/core/tool/parclass.cxx
@@ -498,6 +499,7 @@ void ScParameterClassification::GenerateDocumentation()
{
case ocAnd:
case ocOr:
+ case ocXor:
aToken.SetByte(1); // (r1)AND(r2) --> AND( r1, ...)
break;
default:
This is also unnecessary if the opcode is not allowed as binary inline operator.
diff --git a/sc/source/filter/oox/formulabase.cxx b/sc/source/filter/oox/formulabase.cxx
index b80dbfa..a5276eb 100644
--- a/sc/source/filter/oox/formulabase.cxx
+++ b/sc/source/filter/oox/formulabase.cxx
@@ -669,6 +669,7 @@ static const FunctionData saFuncTableBiff5[] =
{ 0, "DATESTRING", 352, 352, 1, 1, V, { VR },
FUNCFLAG_IMPORTONLY }, // not supported in Calc, missing in OOXML spec
{ 0, "NUMBERSTRING", 353, 353, 2, 2, V, { VR },
FUNCFLAG_IMPORTONLY }, // not supported in Calc, missing in OOXML spec
{ "ROMAN", "ROMAN", 354, 354, 1, 2, V, { VR }, 0 },
+ { "XOR", "XOR", 355, 355, 1, MX, V, { RX }, 0 },
This does not work. Excel doesn't know the XOR function so there is no function identifier available. Inventing a value like 355 also does not work, the values have to be those that Excel uses. So, ...
@@ -762,7 +763,6 @@ static const FunctionData saFuncTableOdf[] =
{ "SKEWP", 0, NOID, NOID, 1, MX, V, { RX },
FUNCFLAG_MACROCALLODF },
{ "UNICHAR", 0, NOID, NOID, 1, 1, V, { VR },
FUNCFLAG_MACROCALLODF },
{ "UNICODE", 0, NOID, NOID, 1, 1, V, { VR },
FUNCFLAG_MACROCALLODF },
- { "XOR", 0, NOID, NOID, 1, MX, V, { RX },
FUNCFLAG_MACROCALLODF }
... just leave the function in that section then at least Calc can use it if the .xls is reopened. With a few changes I think we'd have a working implementation. Please try and test. Btw, instead of reusing fdo#50488 for all new implementations I'd prefer if we created a new bug once we have a function ready and make fdo#50488 depend on the new bug to easily get a list of solutions. Otherwise when committing a change with fdo#50488 in the commit summary we'd add a target to that bug that wouldn't match the real release when a new function will be available. Keep up the good work :) I'm looking forward ot the corrrected patch. Eike -- LibreOffice Calc developer. Number formatter stricken i18n transpositionizer. GnuPG key 0x293C05FD : 997A 4C60 CE41 0149 0DB3 9E96 2F1A D073 293C 05FD
Attachment:
pgpQENOTNTQm7.pgp
Description: PGP signature