Le 31/03/2018 à 19:49, Scott Kostyshak a écrit :
To see an issue that has long annoyed me, do the following:

1. In math mode, type "=".
2. Select the "=" you just typed.
3. Choose "Overset" from the math toolbar (alternatively, just type "\overset").

The "=" is put in the "above" box, because that is the first LaTeX
argument. From the user perspective, I think that putting "=" in the
primary box is usually expected. For example, I think that this would be
more consistent with when the user selects something and chooses
"subscript". The thing selected is in the main box (it is not placed
into the subscript).

This patch tries to fix this. This touches several commonly used methods, so there is a real risk to introduce a regression. Let's see what happens.

If the result is satisfactory, I will have to finish the patch (more refactoring needs to be done). Note that some insets other than Overset are touched: Script, Sideset, Underset. I did not really test them.

Comments welcome.

JMarc


From 988ffb35975aeaad173d9dea665aaccff620b300 Mon Sep 17 00:00:00 2001
From: Jean-Marc Lasgouttes <lasgout...@lyx.org>
Date: Wed, 4 Apr 2018 18:24:14 +0200
Subject: [PATCH] (WIP) 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 useit 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.

Finallly, fix a typo in InsetMathNest::edit() where enter_front
computation was wrong.
---
 src/Cursor.cpp                   | 11 +++++++++--
 src/Cursor.h                     |  5 ++++-
 src/mathed/InsetMathHull.cpp     |  2 +-
 src/mathed/InsetMathNest.cpp     | 12 ++++++------
 src/mathed/InsetMathNest.h       |  5 +++++
 src/mathed/InsetMathOverset.cpp  | 16 ----------------
 src/mathed/InsetMathOverset.h    |  4 ++--
 src/mathed/InsetMathScript.cpp   | 16 ----------------
 src/mathed/InsetMathScript.h     |  6 ++----
 src/mathed/InsetMathSideset.cpp  | 16 ----------------
 src/mathed/InsetMathSideset.h    |  6 ++----
 src/mathed/InsetMathUnderset.cpp | 16 ----------------
 src/mathed/InsetMathUnderset.h   |  4 ++--
 13 files changed, 33 insertions(+), 86 deletions(-)

diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index eb8c501..f3fccfa 100644
--- a/src/Cursor.cpp
+++ b/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)
-		atom.nucleus()->cell(0).append(selection);
+	else if (atomAsNest && atomAsNest->nargs() > 0)
+		atomAsNest->cell(atomAsNest->firstIdx()).append(selection);
 
 	MathWordList const & words = mathedWordList();
 	MathWordList::const_iterator it = words.find(name);
diff --git a/src/Cursor.h b/src/Cursor.h
index 5ca98cb..08309fa 100644
--- a/src/Cursor.h
+++ b/src/Cursor.h
@@ -515,8 +515,11 @@ public:
 
 
 	/// 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?
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index 8f40d2c..c132997 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -2253,7 +2253,7 @@ 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);
 	}
 }
 
diff --git a/src/mathed/InsetMathNest.cpp b/src/mathed/InsetMathNest.cpp
index 2ce31a4..df3cac9 100644
--- a/src/mathed/InsetMathNest.cpp
+++ b/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,10 @@ 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();
-	cur.pos() = enter_front ? 0 : cur.lastpos();
+	enter_front ? idxFirst(cur) : idxLast(cur);
+	LYXERR0("enter_fromt=" << enter_front << ", idx=" << cur.idx());
 	cur.resetAnchor();
 	//lyxerr << "InsetMathNest::edit, cur:\n" << cur << endl;
 }
@@ -1734,7 +1734,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 == '{')
diff --git a/src/mathed/InsetMathNest.h b/src/mathed/InsetMathNest.h
index 3d4977d..57e7ab3 100644
--- a/src/mathed/InsetMathNest.h
+++ b/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
diff --git a/src/mathed/InsetMathOverset.cpp b/src/mathed/InsetMathOverset.cpp
index cea8840..1295685 100644
--- a/src/mathed/InsetMathOverset.cpp
+++ b/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);
diff --git a/src/mathed/InsetMathOverset.h b/src/mathed/InsetMathOverset.h
index ac055e1..1202bbc 100644
--- a/src/mathed/InsetMathOverset.h
+++ b/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;
 	///
diff --git a/src/mathed/InsetMathScript.cpp b/src/mathed/InsetMathScript.cpp
index 57f3b87..8c6416a 100644
--- a/src/mathed/InsetMathScript.cpp
+++ b/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)
diff --git a/src/mathed/InsetMathScript.h b/src/mathed/InsetMathScript.h
index 0b96d03..c1589b1 100644
--- a/src/mathed/InsetMathScript.h
+++ b/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
-	bool idxFirst(Cursor & cur) const;
-	/// Target pos when we enter the inset while moving backwards
-	bool idxLast(Cursor & cur) const;
+	/// The index of the cell entered while moving backward
+	size_type lastIdx() const { return 0; }
 
 	/// write LaTeX and Lyx code
 	void write(WriteStream & os) const;
diff --git a/src/mathed/InsetMathSideset.cpp b/src/mathed/InsetMathSideset.cpp
index cdc5637..ccbab52 100644
--- a/src/mathed/InsetMathSideset.cpp
+++ b/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());
diff --git a/src/mathed/InsetMathSideset.h b/src/mathed/InsetMathSideset.h
index b88548b..b99e630 100644
--- a/src/mathed/InsetMathSideset.h
+++ b/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
-	bool idxFirst(Cursor & cur) const;
-	/// Target pos when we enter the inset while moving backwards
-	bool idxLast(Cursor & cur) const;
+	/// The index of the cell entered while moving backward
+	size_type lastIdx() const { return 0; }
 
 	/// write LaTeX and Lyx code
 	void write(WriteStream & os) const;
diff --git a/src/mathed/InsetMathUnderset.cpp b/src/mathed/InsetMathUnderset.cpp
index b8c5d14..4df7f59 100644
--- a/src/mathed/InsetMathUnderset.cpp
+++ b/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
diff --git a/src/mathed/InsetMathUnderset.h b/src/mathed/InsetMathUnderset.h
index 239a576..ff671ad 100644
--- a/src/mathed/InsetMathUnderset.h
+++ b/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;
 	///
-- 
2.7.4

Reply via email to