When inserting math inset, put cursor selection in the correct cell

The original use case for this bug is entering an overset inset when
there is a selection. The expected result was to have the selection
pasted in main text, but the result was to have it in the cell.

Insets already have idxFirst() that is able to set cursor to the
"entry" cell of an inset. This patch introduces firstIdx(), which is
the index of this cell and uses it in idxFirst() (idem for
lastIdx/idxLast).

As a consequence, several instances of idxFirst/idxLast can be removed.

Now for the real fix: the two places where the cell in which selection
is inserted seem to be:
* Cursor::macroModeClose
* Cursor::handleNest

These two methods are changed to insert material in the entry cell
instead of cell 0.

idxFirst/Last methods are added to InsetMathRoot and InsetMathStackrel
so that the natural entry point is the nucleus of those insets.

Finallly, a typo is fixed in InsetMathNest::edit() where enter_front
computation was incorrect.

src/Cursor.cpp

@@ -1646,6 +1646,12 @@ void Cursor::handleNest(MathAtom const & a, int c)
 }
 
 
+void Cursor::handleNest(MathAtom const & a)
+{
+	handleNest(a, a.nucleus()->asNestInset()->firstIdx());
+}
+
+
 int Cursor::targetX() const
 {
 	if (x_target() != -1)
@@ -1703,6 +1709,7 @@ bool Cursor::macroModeClose(bool cancel)
 	// try to put argument into macro, if we just inserted a macro
 	bool macroArg = false;
 	InsetMathMacro * atomAsMacro = atom.nucleus()->asMacro();
+	InsetMathNest * atomAsNest = atom.nucleus()->asNestInset();
 	if (atomAsMacro) {
 		// macros here are still unfolded (in init mode in fact). So
 		// we have to resolve the macro here manually and check its arity
@@ -1717,8 +1724,8 @@ bool Cursor::macroModeClose(bool cancel)
 	}
 
 	// insert remembered selection into first argument of a non-macro
-	else if (atom.nucleus()->nargs() > 0)
+	else if (atomAsNest && atomAsNest->nargs() > 0)
-		atom.nucleus()->cell(0).append(selection);
+		atomAsNest->cell(atomAsNest->firstIdx()).append(selection);
 
 	MathWordList const & words = mathedWordList();
 	MathWordList::const_iterator it = words.find(name);

src/Cursor.h

@@ -514,9 +514,12 @@ public:
 	docstring macroName();
 
 
+	/// replace selected stuff with at, placing the former
+	// selection in entry cell of atom
+	void handleNest(MathAtom const & at);
 	/// replace selected stuff with at, placing the former
 	// selection in given cell of atom
-	void handleNest(MathAtom const & at, int cell = 0);
+	void handleNest(MathAtom const & at, int cell);
 
 	/// make sure cursor position is valid
 	/// FIXME: It does a subset of fixIfBroken. Maybe merge them?

src/mathed/InsetMathGrid.cpp

@@ -1007,47 +1007,47 @@ bool InsetMathGrid::idxForward(Cursor & cur) const
 }
 
 
-bool InsetMathGrid::idxFirst(Cursor & cur) const
+idx_type InsetMathGrid::firstIdx() const
 {
+	size_type idx = 0;
 	switch (v_align_) {
 		case 't':
-			cur.idx() = 0;
+			//idx = 0;
 			break;
 		case 'b':
-			cur.idx() = (nrows() - 1) * ncols();
+			idx = (nrows() - 1) * ncols();
 			break;
 		default:
-			cur.idx() = ((nrows() - 1) / 2) * ncols();
+			idx = ((nrows() - 1) / 2) * ncols();
 	}
 	// If we are in a multicolumn cell, move to the "real" cell
-	while (cellinfo_[cur.idx()].multi_ == CELL_PART_OF_MULTICOLUMN) {
+	while (cellinfo_[idx].multi_ == CELL_PART_OF_MULTICOLUMN) {
-		LASSERT(cur.idx() > 0, return false);
+		LASSERT(idx > 0, return 0);
-		--cur.idx();
+		--idx;
 	}
-	cur.pos() = 0;
+	return idx;
-	return true;
 }
 
 
-bool InsetMathGrid::idxLast(Cursor & cur) const
+idx_type InsetMathGrid::lastIdx() const
 {
+	size_type idx = 0;
 	switch (v_align_) {
 		case 't':
-			cur.idx() = ncols() - 1;
+			idx = ncols() - 1;
 			break;
 		case 'b':
-			cur.idx() = nargs() - 1;
+			idx = nargs() - 1;
 			break;
 		default:
-			cur.idx() = ((nrows() - 1) / 2 + 1) * ncols() - 1;
+			idx = ((nrows() - 1) / 2 + 1) * ncols() - 1;
 	}
 	// If we are in a multicolumn cell, move to the "real" cell
-	while (cellinfo_[cur.idx()].multi_ == CELL_PART_OF_MULTICOLUMN) {
+	while (cellinfo_[idx].multi_ == CELL_PART_OF_MULTICOLUMN) {
-		LASSERT(cur.idx() > 0, return false);
+		LASSERT(idx > 0, return false);
-		--cur.idx();
+		--idx;
 	}
-	cur.pos() = cur.lastpos();
+	return idx;
-	return true;
 }
 
 

src/mathed/InsetMathGrid.h

@@ -162,10 +162,10 @@ public:
 	bool idxBackward(Cursor &) const;
 	///
 	bool idxForward(Cursor &) const;
-	///
+	//
-	bool idxFirst(Cursor &) const;
+	idx_type firstIdx() const;
-	///
+	//
-	bool idxLast(Cursor &) const;
+	idx_type lastIdx() const;
 	///
 	bool idxDelete(idx_type & idx);
 	/// pulls cell after pressing erase

src/mathed/InsetMathHull.cpp

@@ -419,22 +419,6 @@ InsetMath::mode_type InsetMathHull::currentMode() const
 }
 
 
-bool InsetMathHull::idxFirst(Cursor & cur) const
-{
-	cur.idx() = 0;
-	cur.pos() = 0;
-	return true;
-}
-
-
-bool InsetMathHull::idxLast(Cursor & cur) const
-{
-	cur.idx() = nargs() - 1;
-	cur.pos() = cur.lastpos();
-	return true;
-}
-
-
 // FIXME: InsetMathGrid should be changed to let the real column alignment be
 // given by a virtual method like displayColAlign, because the values produced
 // by defaultColAlign can be invalidated by lfuns such as add-column. For the
@@ -2253,17 +2237,14 @@ void InsetMathHull::handleFont2(Cursor & cur, docstring const & arg)
 	font.fromString(to_utf8(arg), b);
 	if (font.fontInfo().color() != Color_inherit) {
 		MathAtom at = MathAtom(new InsetMathColor(buffer_, true, font.fontInfo().color()));
-		cur.handleNest(at, 0);
+		cur.handleNest(at);
 	}
 }
 
 
 void InsetMathHull::edit(Cursor & cur, bool front, EntryDirection entry_from)
 {
-	cur.push(*this);
+	InsetMathNest::edit(cur, front, entry_from);
-	bool enter_front = (entry_from == Inset::ENTRY_DIRECTION_LEFT ||
-		(entry_from == Inset::ENTRY_DIRECTION_IGNORE && front));
-	enter_front ? idxFirst(cur) : idxLast(cur);
 	// The inset formula dimension is not necessarily the same as the
 	// one of the instant preview image, so we have to indicate to the
 	// BufferView that a metrics update is needed.

src/mathed/InsetMathHull.h

@@ -120,10 +120,6 @@ public:
 	char defaultColAlign(col_type col);
 	///
 	char displayColAlign(idx_type idx) const;
-	///
-	bool idxFirst(Cursor &) const;
-	///
-	bool idxLast(Cursor &) const;
 
 	///
 	void write(WriteStream & os) const;

src/mathed/InsetMathNest.cpp

@@ -235,7 +235,7 @@ bool InsetMathNest::idxFirst(Cursor & cur) const
 	LASSERT(&cur.inset() == this, return false);
 	if (nargs() == 0)
 		return false;
-	cur.idx() = 0;
+	cur.idx() = firstIdx();
 	cur.pos() = 0;
 	return true;
 }
@@ -246,7 +246,7 @@ bool InsetMathNest::idxLast(Cursor & cur) const
 	LASSERT(&cur.inset() == this, return false);
 	if (nargs() == 0)
 		return false;
-	cur.idx() = cur.lastidx();
+	cur.idx() = lastIdx();
 	cur.pos() = cur.lastpos();
 	return true;
 }
@@ -1491,10 +1491,9 @@ bool InsetMathNest::getStatus(Cursor & cur, FuncRequest const & cmd,
 void InsetMathNest::edit(Cursor & cur, bool front, EntryDirection entry_from)
 {
 	cur.push(*this);
-	bool enter_front = (entry_from == Inset::ENTRY_DIRECTION_RIGHT ||
+	bool enter_front = (entry_from == Inset::ENTRY_DIRECTION_LEFT ||
 		(entry_from == Inset::ENTRY_DIRECTION_IGNORE && front));
-	cur.idx() = enter_front ? 0 : cur.lastidx();
+	enter_front ? idxFirst(cur) : idxLast(cur);
-	cur.pos() = enter_front ? 0 : cur.lastpos();
 	cur.resetAnchor();
 	//lyxerr << "InsetMathNest::edit, cur:\n" << cur << endl;
 }
@@ -1734,7 +1733,7 @@ bool InsetMathNest::interpretChar(Cursor & cur, char_type const c)
 			MathAtom const atom = cur.prevAtom();
 			if (atom->asNestInset() && atom->isActive()) {
 				cur.posBackward();
-				cur.pushBackward(*cur.nextInset());
+				cur.nextInset()->edit(cur, true);
 			}
 		}
 		if (c == '{')

src/mathed/InsetMathNest.h

@@ -62,6 +62,11 @@ public:
 	/// move to previous cell
 	bool idxPrev(Cursor &) const;
 
+	// The index of the cell entered while moving forward
+	virtual idx_type firstIdx() const { return 0; }
+	// The index of the cell entered while moving backward
+	virtual idx_type lastIdx() const { return nargs() - 1; }
+
 	/// target pos when we enter the inset while moving forward
 	bool idxFirst(Cursor &) const;
 	/// target pos when we enter the inset while moving backwards

src/mathed/InsetMathOverset.cpp

@@ -57,22 +57,6 @@ void InsetMathOverset::draw(PainterInfo & pi, int x, int y) const
 }
 
 
-bool InsetMathOverset::idxFirst(Cursor & cur) const
-{
-	cur.idx() = 1;
-	cur.pos() = 0;
-	return true;
-}
-
-
-bool InsetMathOverset::idxLast(Cursor & cur) const
-{
-	cur.idx() = 1;
-	cur.pos() = cur.lastpos();
-	return true;
-}
-
-
 void InsetMathOverset::write(WriteStream & os) const
 {
 	MathEnsurer ensurer(os);

src/mathed/InsetMathOverset.h

@@ -28,9 +28,9 @@ public:
 	///
 	void draw(PainterInfo & pi, int x, int y) const;
 	///
-	bool idxFirst(Cursor &) const;
+	idx_type firstIdx() const { return 1; }
 	///
-	bool idxLast(Cursor &) const;
+	idx_type lastIdx() const { return 1; }
 	///
 	void write(WriteStream & os) const;
 	///

src/mathed/InsetMathRoot.h

@@ -30,6 +30,10 @@ public:
 	void metrics(MetricsInfo & mi, Dimension & dim) const;
 	///
 	void draw(PainterInfo & pi, int x, int y) const;
+	///
+	idx_type firstIdx() const { return 1; }
+	///
+	idx_type lastIdx() const { return 1; }
 
 	///
 	void write(WriteStream & os) const;

src/mathed/InsetMathScript.cpp

@@ -72,22 +72,6 @@ InsetMathScript * InsetMathScript::asScriptInset()
 }
 
 
-bool InsetMathScript::idxFirst(Cursor & cur) const
-{
-	cur.idx() = 0;
-	cur.pos() = 0;
-	return true;
-}
-
-
-bool InsetMathScript::idxLast(Cursor & cur) const
-{
-	cur.idx() = 0;
-	cur.pos() = nuc().size();
-	return true;
-}
-
-
 MathData const & InsetMathScript::down() const
 {
 	if (nargs() == 3)

src/mathed/InsetMathScript.h

@@ -49,10 +49,8 @@ public:
 	bool idxForward(Cursor & cur) const;
 	/// move cursor up or down
 	bool idxUpDown(Cursor & cur, bool up) const;
-	/// Target pos when we enter the inset while moving forward
+	/// The index of the cell entered while moving backward
-	bool idxFirst(Cursor & cur) const;
+	size_type lastIdx() const { return 0; }
-	/// Target pos when we enter the inset while moving backwards
-	bool idxLast(Cursor & cur) const;
 
 	/// write LaTeX and Lyx code
 	void write(WriteStream & os) const;

src/mathed/InsetMathSideset.cpp

@@ -62,22 +62,6 @@ Inset * InsetMathSideset::clone() const
 }
 
 
-bool InsetMathSideset::idxFirst(Cursor & cur) const
-{
-	cur.idx() = 0;
-	cur.pos() = 0;
-	return true;
-}
-
-
-bool InsetMathSideset::idxLast(Cursor & cur) const
-{
-	cur.idx() = 0;
-	cur.pos() = nuc().size();
-	return true;
-}
-
-
 int InsetMathSideset::dybt(BufferView const & bv, int asc, int des, bool top) const
 {
 	bool isCharBox = nuc().empty() ? false : isAlphaSymbol(nuc().back());

src/mathed/InsetMathSideset.h

@@ -46,10 +46,8 @@ public:
 	bool idxForward(Cursor & cur) const;
 	/// move cursor up or down
 	bool idxUpDown(Cursor & cur, bool up) const;
-	/// Target pos when we enter the inset while moving forward
+	/// The index of the cell entered while moving backward
-	bool idxFirst(Cursor & cur) const;
+	size_type lastIdx() const { return 0; }
-	/// Target pos when we enter the inset while moving backwards
-	bool idxLast(Cursor & cur) const;
 
 	/// write LaTeX and Lyx code
 	void write(WriteStream & os) const;

src/mathed/InsetMathStackrel.h

@@ -30,6 +30,10 @@ public:
 	///
 	void draw(PainterInfo & pi, int x, int y) const;
 	///
+	idx_type firstIdx() const { return 1; }
+	///
+	idx_type lastIdx() const { return 1; }
+	///
 	void write(WriteStream & os) const;
 	///
 	void normalize(NormalStream &) const;

src/mathed/InsetMathUnderset.cpp

@@ -58,22 +58,6 @@ void InsetMathUnderset::draw(PainterInfo & pi, int x, int y) const
 }
 
 
-bool InsetMathUnderset::idxFirst(Cursor & cur) const
-{
-	cur.idx() = 1;
-	cur.pos() = 0;
-	return true;
-}
-
-
-bool InsetMathUnderset::idxLast(Cursor & cur) const
-{
-	cur.idx() = 1;
-	cur.pos() = cur.lastpos();
-	return true;
-}
-
-
 bool InsetMathUnderset::idxUpDown(Cursor & cur, bool up) const
 {
 	idx_type target = up; // up ? 1 : 0, since upper cell has idx 1

src/mathed/InsetMathUnderset.h

@@ -28,9 +28,9 @@ public:
 	///
 	void draw(PainterInfo & pi, int x, int y) const;
 	///
-	bool idxFirst(Cursor & cur) const;
+	idx_type firstIdx() const { return 1; }
 	///
-	bool idxLast(Cursor & cur) const;
+	idx_type lastIdx() const { return 1; }
 	///
 	bool idxUpDown(Cursor & cur, bool up) const;
 	///