Question regarding Cursor::normalize()

[Stephan Witt]

While looking at #7209 I stumbled over Cursor::normalize().
Is it really a good idea to use the cursor inset as math whithout
any check to write it to debug output?

Stephan

Possible patch:

Index: src/Cursor.cpp
===================================================================
--- src/Cursor.cpp (Revision 37124)
+++ src/Cursor.cpp (Arbeitskopie)
@@ -1683,12 +1683,15 @@

if (pos() > lastpos()) {
lyxerr << "this should not really happen - 2: "
- << pos() << ' ' << lastpos() << " in idx: " << idx()
- << " in atom: '";
+ << pos() << ' ' << lastpos() << " in idx: " << idx();
odocstringstream os;
WriteStream wi(os, false, true, WriteStream::wsDefault);
- inset().asInsetMath()->write(wi);
- lyxerr << to_utf8(os.str()) << endl;
+ InsetMath const * m = inset().asInsetMath();
+ if (m) {
+ m->write(wi);
+ lyxerr << " in atom: '" << to_utf8(os.str()) << "'";
+ }
+ lyxerr << endl;
pos() = lastpos();
}
}

[Vincent van Ravesteijn]

Post by Stephan Witt
While looking at #7209 I stumbled over Cursor::normalize().
Is it really a good idea to use the cursor inset as math whithout
any check to write it to debug output?

No. It's not, but it's a relic of the path. Besides, it really
shouldn't happen ;).

Anyway, look at the following comments in Cursor.h:

///////////////////////////////////////////////////////////////////
//
// The part below is the non-integrated rest of the original math
// cursor. This should be either generalized for texted or moved
// back to the math insets.
//
///////////////////////////////////////////////////////////////////

and

/// make sure cursor position is valid
/// FIXME: It does a subset of fixIfBroken. Maybe merge them?
void normalize();

I have no clue right now, when we call normalize() instead of
fixIfBroken(), what exactly the difference is and whether it would be
difficult to merge them.

Fact is, we should apparently not call normalize() from a non-math
inset, so there is something wrong there if it happens.

Second, we should not have a broken cursor.

Vincent

[Stephan Witt]

Post by Vincent van Ravesteijn

Post by Stephan Witt
While looking at #7209 I stumbled over Cursor::normalize().
Is it really a good idea to use the cursor inset as math whithout
any check to write it to debug output?

No. It's not, but it's a relic of the path. Besides, it really
shouldn't happen ;).

Yes, this I better could have done, thanks.

Post by Vincent van Ravesteijn
///////////////////////////////////////////////////////////////////
//
// The part below is the non-integrated rest of the original math
// cursor. This should be either generalized for texted or moved
// back to the math insets.
//
///////////////////////////////////////////////////////////////////
and
/// make sure cursor position is valid
/// FIXME: It does a subset of fixIfBroken. Maybe merge them?
void normalize();
I have no clue right now, when we call normalize() instead of
fixIfBroken(), what exactly the difference is and whether it would be
difficult to merge them.

normalize() is called only from the LFUN_CUT case of InsetMathNest::doDispatch().
It corrects only the pos of the top cursor slice (if needed), AFAICS.

Post by Vincent van Ravesteijn
Fact is, we should apparently not call normalize() from a non-math
inset, so there is something wrong there if it happens.

Is an ASSERT better than a crash than?

Post by Vincent van Ravesteijn
Second, we should not have a broken cursor.

You mean one should call Cursor::fixIfBroken() if a broken cursor is possible?
It gets broken after a replace operation of last word in paragraph by a shorter one.

Stephan

[Stephan Witt]

Post by Stephan Witt

Post by Vincent van Ravesteijn
Second, we should not have a broken cursor.

You mean one should call Cursor::fixIfBroken() if a broken cursor is possible?
It gets broken after a replace operation of last word in paragraph by a shorter one.

fixIfBroken() is called then already of course.

Stephan

[Vincent van Ravesteijn]

Post by Stephan Witt
normalize() is called only from the LFUN_CUT case of InsetMathNest::doDispatch().
It corrects only the pos of the top cursor slice (if needed), AFAICS.

Post by Vincent van Ravesteijn
Fact is, we should apparently not call normalize() from a non-math
inset, so there is something wrong there if it happens.

Is an ASSERT better than a crash than?

Yes, of course.

However, normalize() is only called from one place. There it is called
after a call to cutSelection(). Cursor::cutSelection either :

- does nothing if there is no selection,
- calls "cur.pos() = endpos" if the cursor is in Text, or
- calls "cur.pos() = cur.lastpos()" or "cur.pos() = 0" if the cursor is in Math.

So, cur.pos() seems to be never > cur.lastpos().

If we are in math, we do not delete cells, so cur.idx() cannot be >
cur.lastidx().

So.. we can just remove Cursor::normalize() altogether.

Vincent

[Stephan Witt]

Post by Vincent van Ravesteijn

Post by Stephan Witt
normalize() is called only from the LFUN_CUT case of InsetMathNest::doDispatch().
It corrects only the pos of the top cursor slice (if needed), AFAICS.

Post by Vincent van Ravesteijn
Fact is, we should apparently not call normalize() from a non-math
inset, so there is something wrong there if it happens.

Is an ASSERT better than a crash than?

Yes, of course.
However, normalize() is only called from one place. There it is called
- does nothing if there is no selection,
- calls "cur.pos() = endpos" if the cursor is in Text, or
- calls "cur.pos() = cur.lastpos()" or "cur.pos() = 0" if the cursor is in Math.
So, cur.pos() seems to be never > cur.lastpos().
If we are in math, we do not delete cells, so cur.idx() cannot be >
cur.lastidx().
So.. we can just remove Cursor::normalize() altogether.

And Cursor::touch() too - never called and empty body.

Stephan