On Sat, Mar 22, 2003 at 06:35:15PM +0100, Alfredo Braunstein wrote:
> > It is also valid for empty paragraphs. Its semantics mean "if you look
> > in getChar(lastPos()), you'll find something, unless it's an empty
>
> That's the same with both ways.
Yes, but returning 0 is far far more meaningful.
> > paragraph". Now, before, we did that by returning -1 in the empty case,
> > and we had all sorts of bits of code getting it dead wrong and accessing
> > places it shouldn't.
>
> Accessing it when the par is empty is wrong in all cases. If there is code
> like you say, then it's wrong even now. What are you talking about?
No, it's valid for things like setCursor and the font settings.
setCursor(lastPos()) is OK, setCursor(-1) ain't. This probably doesn't
make much different to most lastPos() using code, but you cannot deny
that lastPos() == 0 is a *valid* result, and lastPos() == -1 is not.
Fundamentally, I don't find an interface returning a magical value to be
beneficial. This how the bugs got caused that I fixed the first time
round. Someone new to the code is NOT going to see that :
pos_type last = row->lastPos();
setCursor(cur.par(), last + 1);
is actually relying on some "special" semantics. Where as when they see
:
if (row->par()->empty())
setCursor(cur.par(), 0);
else
setCursor(cur.par(), row->lastPos() + 1);
it is a LOT more obvious.
> No, I wanted to piss you off ;). Ok, sorry for that.
heh :)
> > testing of this when I made the change as part of the change tracking
> > stuff (which basically required it in order for sanity to prevail)
>
> Activate the strong asserts (that you have obviously torn off), then
> file->new. Insert a table, and try to write something inside. crash.
Nope ! I am not seeing this even with the strong assert. Now, I'm seeing
lots of asserts, but nowhere near as simple as that, and *every* one
I've done minimal investigation of so far has had *nothing* to do with
the change of lastPos(). Unlike when I made the change in the first
place...
Admittedly, I didn't audit this very well, and there are some obvious
goof ups :
setCursor(cursor.par(), cursor.row()->lastPos() + 1);
I'll look into fixing these.
regards,
john
