| Summary: | Annoying feature: empty cell treated as zero within range/array | ||
|---|---|---|---|
| Product: | LibreOffice | Reporter: | Teraslilja <m-Matti-a.Lehtonen> |
| Component: | Calc | Assignee: | Teraslilja <m-Matti-a.Lehtonen> |
| Status: | RESOLVED FIXED | ||
| Severity: | major | CC: | raal, robinson.libreoffice, xiscofauli |
| Priority: | medium | Keywords: | patch |
| Version: | Inherited From OOo | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | target:6.0.0 | ||
| Crash report or crash signature: | Regression By: | ||
| Attachments: |
Contains basic function that stacks nonempty content of cells of given range
Demo of 'shadow range' method |
||
Hello,
you can identify cell content with this code.
Is this code what you need?
function isempty()
Dim Doc As Object
Dim Sheet As Object
Dim Cell As Object
Doc = ThisComponent
Sheet = Doc.Sheets(0)
Cell = Sheet.getCellByPosition(1,1) 'Cell "B2" (0-based!)
With com.sun.star.table.CellContentType
Select Case Cell.Type
Case .EMPTY
MsgBox "Content: Empty"
Case .VALUE
MsgBox "Content: Value"
Case .TEXT
MsgBox "Content: Text"
Case .FORMULA
MsgBox "Content: Formula"
End Select
End With
end function
Well ... No.
The function needs a *range of cells* as parameter, and apparently it is given as Array, even if function declares otherwise.
Function StackNonEmpty( range As com.sun.star.table.CellRange, delimiter As String )
Dim result As String
MsgBox "IsArray = " & IsArray( range )
result = ""
...
Shows a message box with text: "IsArray = True"
The use of a range in a spreadsheet function does produce an array of values rather than an array of cells, at least within Basic.
The solution/workaround to get the cell status into a Basic macro is to use the ISBLANK function to create a shadow range in the worksheet. This range records whether or not the corresponding cell in the original study range is empty or not (column D in attachment). The Function then gets passed two arrays and can work as desired.
Here is sample (similar, working code in attachment):
Function StackNonEmpty2( range As Variant, IsEmpty As Variant, delimiter As String )
' NEED to check parameters and exit gracefully
' NEED to check for workable dimension: range larger than IsEmpty => bug
Dim result, tag As String
Dim row, col As Integer
result = ""
For row = LBound( range, 1 ) To UBound( range, 1 )
For col = LBound( range, 2 ) To UBound( range, 2 )
If NOT CBool( IsEmpty( row, col ) ) then
Select Case VarType(range(row, col)):
Case 5: tag = "Double"
Case 8: tag = "String"
Case Else: tag = "other type"
end select
If result <> "" Then
result = result & delimiter
End If
result = result & range( row, col ) & ": " & tag
End If
Next
Next
StackNonEmpty2 = result
End Function
There is almost certainly another solution that passes ADDRESS information to the function. The function would then interrogate the spreadsheet directly as in raal's example above. This second method would allow the Basic code to gather more information about the Cells' type and status.
Created attachment 113327 [details]
Demo of 'shadow range' method
Attachment demonstrates solution by passing shadow range that has ISBLANK() values for original target range to get required info into Basic macro.
Nice idea, but I think that this time the solution won't work with cases, where the range is rather large, both in rows AND columns (in my example there were just a single column for simplicity). In my opinion, the only correct fix is to pass value of type of "Empty" or "Null" for empty cells in the array. (In reply to Teraslilja from comment #5) > Nice idea, but I think that this time the solution won't work [...] This bug has an assignee and it sounds like there's been actual useful discussion in the comments, so I'm tentatively changing status to NEW. That being said, it hasn't had any activity for over a *year*, so unless there's going to be ongoing discussion, I suggest we close it. We can always open a new bug if desired. (In reply to Robinson Tryon (qubit) from comment #6) > > This bug has an assignee and it sounds like there's been actual useful > discussion in the comments, so I'm tentatively changing status to NEW. > > That being said, it hasn't had any activity for over a *year*, so unless > there's going to be ongoing discussion, I suggest we close it. We can always > open a new bug if desired. After waiting an year, I decided to fix this myself. It did take couple days to identify the place, where the parameter of function is filled. And another day to write a patch. Warning: If user defined macros assumes that only string/double types are passed, they shall fail or change behaviour. But anyway, I believe that this is STILL only a partial fix, because I spotted two different places and there are more, e.g. case svExternalSingleRef or (potentially) when bUseVBAObjects is true at ScInterpreter::ScMacro(). Typically the current code behaves so that it checks, if value of cell is a string, then a string is passed. Otherwise a double is passed, even thought the content of cell can be e.g. empty, boolean etc. The partial patch: diff --git a/sc/source/core/tool/interpr4.cxx b/sc/source/core/tool/interpr4.cxx index 055d6d9..c07dbac 100644 --- a/sc/source/core/tool/interpr4.cxx +++ b/sc/source/core/tool/interpr4.cxx @@ -3280,14 +3280,30 @@ void ScInterpreter::ScMacro() { nIdx[ 1 ] = static_cast<sal_Int32>(nMatCol+1); SbxVariable* p = refArray->Get32( nIdx ); - if (pMat->IsString(nMatCol, nMatRow)) + if (pMat->IsEmpty(nMatCol, nMatRow)) + { + p->PutEmpty(); + } + if (pMat->IsEmptyPath(nMatCol, nMatRow)) + { + p->PutEmpty(); + } + else if (pMat->IsBoolean(nMatCol, nMatRow)) + { + p->PutBool( pMat->Get(nMatCol, nMatRow).GetBoolean() ); + } + else if (pMat->IsString(nMatCol, nMatRow)) { p->PutString( pMat->GetString(nMatCol, nMatRow).getString() ); } - else + else if (pMat->IsValue(nMatCol, nMatRow)) { p->PutDouble( pMat->GetDouble(nMatCol, nMatRow)); } + else + { + p->PutEmpty(); + } } } pPar->PutObject( refArray ); @@ -3449,11 +3465,11 @@ bool ScInterpreter::SetSbxVariable( SbxVariable* pVar, const ScAddress& rPos ) } break; default : - pVar->PutDouble( 0.0 ); + pVar->PutEmpty(); } } else - pVar->PutDouble( 0.0 ); + pVar->PutEmpty(); return bOk; } And a patch for case svExternalSingleRef:
diff --git a/sc/source/core/tool/interpr4.cxx b/sc/source/core/tool/interpr4.cxx
index 055d6d9..5da022e 100644
--- a/sc/source/core/tool/interpr4.cxx
+++ b/sc/source/core/tool/interpr4.cxx
@@ -3189,14 +3189,24 @@ void ScInterpreter::ScMacro()
bOk = false;
else
{
- if ( pToken->GetType() == svString )
- pPar->PutString( pToken->GetString().getString() );
- else if ( pToken->GetType() == svDouble )
- pPar->PutDouble( pToken->GetDouble() );
- else
+ switch( pToken->GetType() )
{
- SetError( errIllegalArgument );
- bOk = false;
+ case svEmptyCell:
+ pPar->PutEmpty();
+ break;
+
+ case svString:
+ pPar->PutString( pToken->GetString().getString() );
+ break;
+
+ case svDouble:
+ pPar->PutDouble( pToken->GetDouble() );
+ break;
+
+ default:
+ SetError( errIllegalArgument );
+ bOk = false;
+ break;
}
}
}
As I suspected, there are similar code (if not string, then double) used in other parts of calc, like e.g.
./sc/source/core/tool/interpr5.cxx: if (!pMat1->IsString(i,j) && !pMat2->IsString(i,j))
./sc/source/core/tool/interpr5.cxx- {
./sc/source/core/tool/interpr5.cxx- fVal = pMat1->GetDouble(i,j);
./sc/source/core/tool/interpr5.cxx- fSum += fVal * fVal;
./sc/source/core/tool/interpr5.cxx- fVal = pMat2->GetDouble(i,j);
./sc/source/core/tool/rangeseq.cxx: if ( pMatrix->IsString( nCol, nRow ) )
...
./sc/source/core/tool/rangeseq.cxx- else
./sc/source/core/tool/rangeseq.cxx- pColAry[nCol] = pMatrix->GetDouble( nCol, nRow );
./sc/source/core/tool/interpr3.cxx: if (!pMat1->IsString(i,j))
./sc/source/core/tool/interpr3.cxx- {
./sc/source/core/tool/interpr3.cxx- fVal = pMat1->GetDouble(i,j);
...
./sc/source/core/tool/interpr3.cxx- }
etc
Perhaps a place for another bug report ...
Setting Assignee back to default. Please change it back if you're still working on this issue Hello Teraslilja, Thanks for the patch. Could you please submit the patch to gerrit as described here: https://wiki.documentfoundation.org/Development/gerrit/SubmitPatch? (In reply to Xisco Faulí from comment #11) > Hello Teraslilja, > Thanks for the patch. > Could you please submit the patch to gerrit as described here: > https://wiki.documentfoundation.org/Development/gerrit/SubmitPatch? The patch(es) above won't work any more. The object passing a cell range to basic macro has been changed. So I have written a new patch that I will submit later this week. diff --git a/sc/source/core/tool/interpr4.cxx b/sc/source/core/tool/interpr4.cxx index b2dc0adee55e..518a1475a5f2 100644 --- a/sc/source/core/tool/interpr4.cxx +++ b/sc/source/core/tool/interpr4.cxx @@ -3544,11 +3545,12 @@ bool ScInterpreter::SetSbxVariable( SbxVariable* pVar, const ScAddress& rPos ) } break; default : - pVar->PutDouble( 0.0 ); + pVar->PutEmpty(); } } else - pVar->PutDouble( 0.0 ); + pVar->PutEmpty(); + return bOk; } Thanks. Please, do not forget to submit it, otherwise, your work will be in vain Matti Lehtonen committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=3ed87ba36e59f774917eac49389770da404cbc06 tdf#89216 forward empty cells as empty to BASIC instead of 0.0 It will be available in 6.0.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback. Not sure if this might have some side effects not yet aware of. In Basic, arithmetic calculations take an empty SbxVar as 0 and string operations as empty string, as they should, so in that context we're good. A polite ping to Matti Lehtonen & Eike Rathke: is this bug fixed? if so, could you please close it as RESOLVED FIXED ? Thanks Let's call this fixed. |
Created attachment 113216 [details] Contains basic function that stacks nonempty content of cells of given range It seems impossible for basic function to identify empty cell from zero at given array range. Expecting: VarType of Variant of empty cell is either Empty or Null. Feature(bug?): VarType of Variant of empty cell is actually returned as Double. Function StackNonEmpty( range As Variant, delimiter As String ) Dim result As String result = "" If NOT IsMissing( range ) Then If NOT IsArray( range ) Then result = range Else Dim oCell As Variant Dim bInclude, bString as Boolean Dim sCell As String Dim sType As String Dim row, col As Integer For row = LBound( range, 1 ) To UBound( range, 1 ) For col = LBound( range, 2 ) To UBound( range, 2 ) oCell = range( row, col ) Select Case VarType( oCell ) Case 0 bInclude = False bString = False sType="Empty" Case 1 bInclude = False bString = False sType="Null" Case 2 bInclude = True bString = False sType="Integer" Case 3 bInclude = True bString = False sType="Long" Case 4 bInclude = True bString = False sType="Single" Case 5 bInclude = True bString = False sType="Double" Case 7 bInclude = True bString = False sType="Date" Case 8 bInclude = True bString = True sType="String" Case 9 bInclude = False bString = False sType="Object" Case 11 bInclude = True bString = False sType="Boolean" Case 12 bInclude = False bString = False sType="Variant" Case Else bInclude = False bString = False sType="Unexpected" End Select If bInclude Then If bString Then sCell = range( row, col ) If Len( sCell ) > 0 Then If result <> "" Then result = result & delimiter End If result = result & sCell & ": " & sType End If Else If result <> "" Then result = result & delimiter End If result = result & oCell & ": " & sType End If End If Next Next End If End If StackNonEmpty = result End Function