John Levon wrote:
> Yes, but returning 0 is far far more meaningful.
I disagree: you don't know what this 0 means.
> 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.
But setCursor(lastPos()) puts the cursor _before_ the last char on the row
(not used very much really).
OTOH, the more used setCursor(lastPos() + 1) is always valid in my case and
can be invalid in yours. Moreover, your convention loses information: 1
char == 0 chars for you. How do you put the cursor after the end of the
row? You have to resort to another method to get the size.
Actually, on a paragraph with n chars there are n+1 cursor positions. As we
want to use the convention 0,...,n then maybe we can have two 'last
position' methods, one for using chars, the other for using cursor
positions, if you find the -1 disturbing (one being the other + 1).
>> 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
It's not a magical value, it's the position before the start of the
container. (and btw is what the std library does). It allows doing a loop
like
for (post_type pos = 0; pos <= row->lastPos(); ++pos)
getchar(pos);
How would you do that with your way?
> 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
I don't see the special semantics here, the value "-1" is perfectly
consistent as I see it. Your 0 on the other hand *is* special semantics.
> if (row->par()->empty())
> setCursor(cur.par(), 0);
> else
> setCursor(cur.par(), row->lastPos() + 1);
>
> it is a LOT more obvious.
Come on, so you special case the convention and then special case the code
that uses it. Pretty consistent. ;)
>> 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
Really you don't get an assert when you write inside a table?
> 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...
I'm saying you that with the patch you get 0 asserts (so they *are* related
to this in some way). This is a proof (well the strongness of proofs is
pretty much devaluated nowadays... at least in politics ;) that the other
convention is more natural, and it was assumed when the code was
programmed.
> Admittedly, I didn't audit this very well, and there are some obvious
> goof ups :
>
> setCursor(cursor.par(), cursor.row()->lastPos() + 1);
This is exactly what I'm talking about. Please don't special case all the
code because of choosing the wrong convention.
Regards, Alfredo
