On Fri, Jan 20, 2006 at 04:11:24PM +0100, Jean-Marc Lasgouttes wrote:
> >>>>> "Angus" == Angus Leeming <[EMAIL PROTECTED]> writes:
> 
> Angus> I have some questions/points for you...
> 
> + // Unsafe if .find() can return .end()
>   CoordCache::InnerParPosCache const & cache =
>     theCoords.getParPos().find(cursor.bottom().text())->second;
> 
> bv::funcs::status does the same, so it has to be safe in this setting :)

We could even ASSERT.

>   // Get an iterator on the first paragraph in the cache
>   DocIterator it(inset);
>   it.push_back(CursorSlice(inset));
> + // Do you need to check whether it.pit() == 0 after assignment?
>   it.pit() = cache.begin()->first;
> 
> pit == 0 is perfectly valid, I think.

Indeed. This is the first paragraph in a ParagraphList.

>   // Get an iterator after the last paragraph in the cache
>   DocIterator et(inset);
>   et.push_back(CursorSlice(inset));
> + // What happens if cache.empty()? Is it guaranteed not to be if
> + // .find() above does not return .end() ?
>   et.pit() = boost::prior(cache.end())->first;
> 
> In general I believe this is safe.

ASSERT as well.

>   // Is it possible for et.pit() to be zero and
>   // et.lastpit() to be non-zero?
> 
> I don't think so.

[Why should this be a problem?]
 
>   if (et.pit() >= et.lastpit())
> +   // Given that doc_iterator_end(inset) is identical to
> +   // DocIterator(inset), why not be consistent?
>     et = doc_iterator_end(inset);
> 
> Indeed.
> 
> All in all, I think an approach similar to this one is perfectly safe
> wrt bruteFind. Of course, it may be that fixing bruteFind3 is the right
> solution to Martin's problem. 

I think the approach is correct for the 'break out of math' problem.
And given the fact that it uses the coord cache for distance
calculations, it hardly can't work for anything not in the coord 
cache. So it should be correct for all current an potential use cases.

And is fixes a regression as in 'unacceptable-performance-is-a-regression'

> I guess what we need is some lecture from Andre' about these bruteFind
> functions. 

I can't remember anything. Basically ad-hoc catch-all functions for
cases when 'clever' tricks failed. In math, using brute find for up/down
yields pretty bad results as the layout there is much less rectangular
than in plain text. So there have to be 'clever' rules like 'try to go
first to an exponent, then ty to go to a cell above in the current grid,
and if everything fails, just go to somewhere reasonably close'.

Reply via email to