ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

sc: bind the # operator to the reference it follows

Browse Source 
The # postfix operator was parsed at the same level as the percent
sign, which is looser than the prefix operators. Because of this
=@A1# compiled to the spill range of an implicit intersection and
=-A1# to the spill range of a negated value. Neither of those means
anything, and OOXML writes the # around the reference alone in both
cases.

The # is now parsed right after the factor it follows, tighter than any
operator that can come before it. A whole range reference goes inside,
because the lexer reads A1:A3 as a single reference. The percent sign
stays above the reference operators, so =A1%# leaves the # with nothing
to bind to and doesn't parse, and the export drops such a # (OOXML has
no spelling for it either).

A blank between two operands is only the intersection operator if both
sides yield a reference. The # does, so the checks that keep the blank
alive and that keep the operator once the operands are known had to be
told about it.

The interpreter passes the spill range on as a matrix that carries the
range it was read from. The intersection, range and union operators
only took references and refused that, so PopReferenceOperand now
hands them the carried range as a double reference.

Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
Change-Id: I1a587bf291ff639b326a366b8efc332106eb5fd3
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/10156
Tested-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
(cherry picked from commit c589c34aaec00d60fc47f1933762b105f9d22de3)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210293
Tested-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
This commit is contained in:
Tomaž Vajngerl
2026-08-31 17:35:25 +02:00
committed by Xisco Fauli
parent cf45ff8f40
commit 1ff770e5bf
10 changed files with 270 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
formula/qa/unit/OOXMLRewriterTest.cxx
+17
View File
@@ -80,6 +80,8 @@ private:
return u"_xlfn.ANCHORARRAY"_ustr;
case ocOffset:
return u"OFFSET"_ustr;
case ocPercentSign:
return u"%"_ustr;
case ocMissing:
return u"missing"_ustr;
case ocBad:
@@ -240,6 +242,21 @@ CPPUNIT_TEST_FIXTURE(OOXMLRewriterTest, testAnAtWithNothingToEncloseIsDropped)
CPPUNIT_ASSERT_EQUAL(u"missing"_ustr, describe(*pMissing));
}
CPPUNIT_TEST_FIXTURE(OOXMLRewriterTest, testASpillWithNoFactorBeforeItIsDropped)
{
// A percent sign ends the factor, so the # that follows has nothing to enclose. The pair
// would span no tokens at all, so it goes and the # with it.
FormulaTokenArray aTokens;
addOperand(aTokens);
aTokens.AddOpCode(ocPercentSign);
aTokens.AddOpCode(ocSpill);
const std::unique_ptr<FormulaTokenArray> pRewritten = rewrite(aTokens);
CPPUNIT_ASSERT(pRewritten);
CPPUNIT_ASSERT_EQUAL(u"operand %"_ustr, describe(*pRewritten));
}
CPPUNIT_TEST_FIXTURE(OOXMLRewriterTest, testTextThatDidNotParseTakesNoWrapper)
{
// Parentheses cannot go around text that never parsed, so a wrapper waiting on it is
formula/source/core/api/FormulaCompiler.cxx
+28 -2
View File
@@ -339,6 +339,9 @@ bool isPotentialRangeLeftOp( OpCode eOp )
switch (eOp)
{
case ocClose:
// The # comes after its operand, so it is the last opcode of a sub-expression that
// yields a reference.
case ocSpill:
return true;
default:
return false;
@@ -351,6 +354,9 @@ bool isRangeResultFunction( OpCode eOp )
{
case ocIndirect:
case ocOffset:
// In OOXML the # is written before its operand, so there it is the first opcode of a
// sub-expression that yields a reference.
case ocSpill:
return true;
default:
return false;
@@ -366,6 +372,9 @@ bool isRangeResultOpCode( OpCode eOp )
case ocIntersect:
case ocIndirect:
case ocOffset:
// The # postfix operator gives the spill range of the master cell before it, which
// is a reference just like the operators above.
case ocSpill:
return true;
default:
return false;
@@ -2321,15 +2330,32 @@ void FormulaCompiler::Factor()
}
}
void FormulaCompiler::SpillOperator()
{
while (mpToken->GetOpCode() == ocSpill)
{ // this operator _follows_ the reference it takes the spill range of, and binds
// to that reference alone
if (mbComputeII)
{
FormulaToken** pArg = mpCode - 1;
HandleIIOpCode(mpToken.get(), &pArg, 1);
}
PutCode(mpToken);
NextToken();
}
}
void FormulaCompiler::RangeLine()
{
Factor();
SpillOperator();
while (mpToken->GetOpCode() == ocRange)
{
FormulaToken** pCode1 = mpCode - 1;
FormulaTokenRef p = mpToken;
NextToken();
Factor();
SpillOperator();
FormulaToken** pCode2 = mpCode - 1;
if (!MergeRangeReference( pCode1, pCode2))
PutCode(p);
@@ -2544,8 +2570,8 @@ void FormulaCompiler::UnaryLine()
void FormulaCompiler::PostOpLine()
{
UnaryLine();
while (mpToken->GetOpCode() == ocPercentSign || mpToken->GetOpCode() == ocSpill)
{ // these operators _follow_ their operand
while (mpToken->GetOpCode() == ocPercentSign)
{ // this operator _follows_ its operand
if (mbComputeII)
{
FormulaToken** pArg = mpCode - 1;
formula/source/core/api/OOXMLRewriter.cxx
+22 -17
View File
@@ -111,6 +111,8 @@ private:
sal_uInt16 mnContentStart;
// Start of the union-level expression we are reading.
sal_uInt16 mnExpressionStart;
// Start of the current factor: an operand plus its parentheses and arguments.
sal_uInt16 mnFactorStart;
// Start of an open union list, NO_POSITION if there is none.
sal_uInt16 mnListStart;
// The scope is a grouping parenthesis, not a call's argument list.
@@ -232,6 +234,8 @@ void RewriteCollector::movePastToken(Scope& rScope, sal_uInt16 nPosition)
rScope.mnContentStart = sal_uInt16(nPosition + 1);
if (rScope.mnExpressionStart == nPosition)
rScope.mnExpressionStart = sal_uInt16(nPosition + 1);
if (rScope.mnFactorStart == nPosition)
rScope.mnFactorStart = sal_uInt16(nPosition + 1);
}
// Mark pending operands as filled. A mark is never cleared, so walking from the back can
@@ -295,7 +299,7 @@ void RewriteCollector::closeUnionList(sal_uInt16 nEnd, bool bMayTakeScopeParenth
void RewriteCollector::collect()
{
const sal_uInt16 nLength = mrTokens.GetLen();
maScopes.push_back({ 0, 0, NO_POSITION, false });
maScopes.push_back({ 0, 0, 0, NO_POSITION, false });
// The next token is where an operand starts, so a plus or minus there is the unary form.
bool bOperandPosition = true;
// The current factor is raw text that never parsed - we cannot put wrapper parentheses
@@ -315,29 +319,23 @@ void RewriteCollector::collect()
if (eOp == ocSingleValue)
{
Scope& rScope = maScopes.back();
// Any start sitting on the @ moves on to the operand after it.
// Any start sitting on the @ moves on to the operand after it, and so does the
// factor it begins.
movePastToken(rScope, nPosition);
rScope.mnFactorStart = sal_uInt16(nPosition + 1);
maPendingSingles.push_back({ maScopes.size(), sal_uInt16(nPosition + 1), false });
continue;
}
// The # is written as a call around the whole expression under it, so the union list
// closes first. Text that never parsed takes no parentheses, so drop a # after one.
// The # is written as a call around the factor before it and binds tighter than any
// operator that can come before that factor. Raw text that never parsed takes no
// parentheses, so drop a # after one, and drop a # with no factor before it at all.
if (eOp == ocSpill)
{
if (!bBadFactor)
{
closeUnionList(nPosition, false);
const sal_uInt16 nBegin = maScopes.back().mnExpressionStart;
mrWrappers.push_back({ nBegin, nPosition, ocAnchorArray, true });
// The # binds tighter than an @ before it, so an @ that starts inside the
// wrapper takes all of it as its operand. An @ on one part of a union list
// is such a case.
for (PendingSingleValue& rSingle : maPendingSingles)
{
if (rSingle.mnBegin > nBegin)
rSingle.mnBegin = nBegin;
}
mrWrappers.push_back(
{ maScopes.back().mnFactorStart, nPosition, ocAnchorArray, true });
markPendingOperand();
}
bOperandPosition = false;
@@ -361,7 +359,8 @@ void RewriteCollector::collect()
markPendingOperand();
// A grouping parenthesis is where an operand starts, a call's parenthesis comes
// after the function name.
maScopes.push_back({ sal_uInt16(nPosition + 1), sal_uInt16(nPosition + 1), NO_POSITION,
maScopes.push_back({ sal_uInt16(nPosition + 1), sal_uInt16(nPosition + 1),
sal_uInt16(nPosition + 1), NO_POSITION,
eOp == ocOpen && bOperandPosition });
bOperandPosition = true;
bBadFactor = false;
@@ -403,7 +402,7 @@ void RewriteCollector::collect()
}
Scope& rScope = maScopes.back();
// Whitespace before a scope or an expression is not part of it.
// Whitespace before a scope, expression or factor is not part of it.
if (bWhitespace)
movePastToken(rScope, nPosition);
if (!bPrefix && !bWhitespace && !lclWritesNoText(eOp))
@@ -419,6 +418,12 @@ void RewriteCollector::collect()
// one does not, it belongs to the operand before it.
rScope.mnExpressionStart = sal_uInt16(nPosition + 1);
}
if (bPrefix || bSeparator || bEndsOperand)
{
// Every operator and separator ends the factor before it, so the next factor
// starts after. A function name does not.
rScope.mnFactorStart = sal_uInt16(nPosition + 1);
}
if (bSeparator)
bBadFactor = false;
if (!bWhitespace)