Dom & plam, there was no logical errors. Just a question of missing
comments to explain the thinking at the time.
FWIW I think _findFirstAfter started life as a void return type
function, the two only callers ignoring the case where the POB value was
0 anyway (since they both want to go back in the list).
I then later added the return value to reuse the code in ::add, and that
made the behaviour of the two other callers illogical.
The comments should make it clear, I hope.
Jesper
Index: fl_Squiggles.cpp
===================================================================
RCS file: /cvsroot/abi/src/text/fmt/xp/fl_Squiggles.cpp,v
retrieving revision 1.10
diff -u -p -u -5 -p -r1.10 fl_Squiggles.cpp
--- fl_Squiggles.cpp 21 Mar 2002 09:46:05 -0000 1.10
+++ fl_Squiggles.cpp 16 Jul 2002 06:14:02 -0000
@@ -108,10 +108,17 @@ fl_Squiggles::_purge(void)
Find first squiggle after the given offset
\param iOffset Offset
\result iIndex Index of POB, or index of last POB+1
\return True if found, otherwise false
+ \note Callers may use the iIndex result even if the search fails:
+ this allows them to look at the last POB on the line which may
+ span the point they are looking for (remember this function
+ finds the squiggle past a given offset, not the one spanning it
+ - there may well be a squiggle spanning a point without there
+ being further squiggles behind it).
+
\fixme This function should be rewritten using binary search
*/
bool
fl_Squiggles::_findFirstAfter(UT_sint32 iOffset, UT_sint32& iIndex) const
{
@@ -144,10 +151,15 @@ UT_sint32
fl_Squiggles::_find(UT_sint32 iOffset) const
{
UT_sint32 iIndex;
_findFirstAfter(iOffset, iIndex);
+ // Note that the return value is not checked: either there is no
+ // POBs at all, in which case we'll catch it in the statement
+ // below (no previous POB). Otherwise we want to look at the last
+ // POB on the line since it may span past iOffset.
+
// If no previous POB, return
if (0 == iIndex) return -1;
// Load up with previous POB
fl_PartOfBlock* pPOB = getNth(--iIndex);
@@ -664,12 +676,19 @@ fl_Squiggles::findRange(UT_sint32 iStart
fl_PartOfBlock* pPOB;
UT_sint32 s, e;
// Look for the first POB.start that is higher than the end offset
_findFirstAfter(iEnd, e);
- // Return with empty set if the first POB's offset is higher than
- // the region end.
+ // Note that the return value is not checked: either there is no
+ // POBs at all, in which case we'll catch it in the statement
+ // below (first POB's offset past end point). Otherwise we want to
+ // look at the last POB on the line since it may span past
+ // iOffset.
+
+ // Return with empty set if the offset of the first POB on the
+ // line is higher than the region end (i.e. there is no previous
+ // POB that could span the region end).
if (0 == e)
{
UT_ASSERT(getNth(0)->getOffset() > iEnd);
return false;
}
added some comments
CVS: ----------------------------------------------------------------------
CVS: Enter Log. Lines beginning with `CVS:' are removed automatically
CVS:
CVS: Committing in .
CVS:
CVS: Modified Files:
CVS: fl_Squiggles.cpp
CVS: ----------------------------------------------------------------------
