Lars Gullik BjÃnnes wrote:
> | I guess it's a matter of preferences... (I prefer to think them as two
> | simple variables grouped toghether in a struct for clarity.)
>
> So this is two independant variables grouped together for clarity
> because thay are not really independant?
Sort of. They are conceptually connected ((x,y) absolute pixel coordinates),
but not necesarily used toghether. They are set toghether at draw time, so
maybe an (x,y) ctor is not absurd after all.
> or if they truly are independant I'd rather go for the "every variable
> for itself" approach.
Angus or Lars, tell me what to do to make you happy (in this respect at
least :^) and I'll do it.
Alfredo
PS: to make *me* happy, you should look at the two chunks below and tell if
replacing the first one with the second one will miss something important.
It's not easy to tell by testing because the old one is currently not
working in many aspects.
old:
void LyXText::cursorNext()
{
int topy = bv()->top_y();
ParagraphList::iterator cpit = cursorPar();
RowList::iterator crit = cpit->getRow(cursor.pos());
if (isLastRow(cpit, *crit)) {
int y = cursor.y() - crit->baseline() + crit->height();
if (y > topy + bv()->workHeight())
bv()->updateScrollbar();
return;
}
int y = topy + bv()->workHeight();
if (inset_owner && !topy) {
y += - bv()->text->cursor.y()
+ bv()->top_y()
- bv()->cursor().innerInset()->insetInInsetY();
}
ParagraphList::iterator dummypit;
Row const & row = *getRowNearY(y, dummypit);
y = dummypit->y + row.y_offset();
setCursorFromCoordinates(bv()->x_target(), y);
// + bv->workHeight());
finishUndo();
int new_y;
if (crit == bv()->text->cursorRow()) {
// we have a row which is taller than the workarea. The
// simplest solution is to move to the next row instead.
cursorDown(true);
return;
// This is what we used to do, so we wouldn't skip right past
// tall rows, but it's not working right now.
#if 0
new_y = bv->top_y() + bv->workHeight();
#endif
}
if (inset_owner) {
new_y = bv()->text->cursor.y()
+ bv()->cursor().innerInset()->insetInInsetY()
+ y - crit->baseline();
} else {
new_y = cursor.y() - crit->baseline();
}
nextRow(cpit, crit);
LyXCursor cur;
setCursor(cur, parOffset(cpit), crit->pos(), false);
if (cur.y() < bv()->top_y() + bv()->workHeight())
cursorDown(true);
bv()->updateScrollbar();
}
new:
void LyXText::cursorNext()
{
ParagraphList::iterator cpit = cursorPar();
RowList::iterator crit = cpit->getRow(cursor.pos());
int y = bv()->top_y() + bv()->workHeight() - pos_.y;
setCursorFromCoordinates(bv()->x_target() - pos_.x, y);
if (crit == cursorRow()) {
// we have a row which is taller than the workarea. The
// simplest solution is to move to the next row instead.
cursorDown(true);
}
bv()->updateScrollbar();
finishUndo();
}
