Le 13/12/2015 11:41, Jean-Marc Lasgouttes a écrit :
Le 13/12/2015 04:37, Guillaume Munch a écrit :
Dear list

This has been annoying me for a while. OK to push?

It would deserve a commit log with explanations of what this fixes. The
patch does many things it is difficult for me to say whether this is
correct.

You're right, see now second patch attached. It's a fairly simple
change, only adding some information to Georg's displayColAlign(), in
addition to some mechanical refactoring and disabling of horizontal
alignment buttons which were wrongly enabled. See the new commit log.

Moreover I saw that Georg has a related patch at
<http://www.lyx.org/trac/ticket/1861> (!!) so maybe he has an opinion
about this. Georg?

Anyway this could also be something that we keep for 2.2.x.

I am now done with any "big" patch for 2.2., so this leaves
enough time for discussion: more than a week for LFUN_TABLE_MODIFY in
another thread (it is subject to the feature freeze) and three weeks for
these UI bugfixes. For these, I know this looks big, but the actual
changes are small, the rest is mainly refactoring and cleaning (as you
prompted me to do).


FWIW, I also tend to be annoyed by bad on screen alignment, and the
change of ordering of enums seems to be a good catch. However, it would
be a good idea to get rid of the test on >= hullAlign and replace it
with a helper function. This will make the code more robust IMO.

This was a very good suggestion because the situation was even less clean than I imagined. I did that in the first patch attached. Please
have a look.

I also attach a third patch to apply a similar solution to fix the
spacing of the columns in AMS environments. They are all independent from one another.

>From 075989c35201ecb05fdb086ff66dee379f83019b Mon Sep 17 00:00:00 2001
From: Guillaume Munch <g...@lyx.org>
Date: Mon, 14 Dec 2015 01:54:27 +0000
Subject: [PATCH 1/3] Sanitize InsetMathHull and add a check for mutability in
 LFUN_MATH_MUTATE

The transformation is purely mechanical (apart for the addition of
isMutable()). Remove in particular all comparisons < and >= involving HullType.

Add a guard to make sure that mutate() only operates on types it has been
designed for. Then I figured I could use this new knowledge to give feedback
when math-mutate is not implemented via getStatus(). (To test this, insert a
regexp in Advanced Search & Replace and try to change it into a standard
equation via the contextual menu.)
---
 src/mathed/InsetMath.h       |   3 +-
 src/mathed/InsetMathHull.cpp | 346 +++++++++++++++++++++++++++----------------
 src/mathed/InsetMathHull.h   |   2 +
 3 files changed, 223 insertions(+), 128 deletions(-)

diff --git a/src/mathed/InsetMath.h b/src/mathed/InsetMath.h
index bd72863..deb6915 100644
--- a/src/mathed/InsetMath.h
+++ b/src/mathed/InsetMath.h
@@ -34,7 +34,8 @@ enum HullType {
 	hullFlAlign,
 	hullMultline,
 	hullGather,
-	hullRegexp
+	hullRegexp,
+	hullUnknown
 };
 
 HullType hullType(docstring const & name);
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index 097a344..68318a2 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -79,16 +79,18 @@ namespace {
 	int getCols(HullType type)
 	{
 		switch (type) {
-			case hullEqnArray:
-				return 3;
-			case hullAlign:
-			case hullFlAlign:
-			case hullAlignAt:
-			case hullXAlignAt:
-			case hullXXAlignAt:
-				return 2;
-			default:
-				return 1;
+		case hullEqnArray:
+			return 3;
+
+		case hullAlign:
+		case hullFlAlign:
+		case hullAlignAt:
+		case hullXAlignAt:
+		case hullXXAlignAt:
+			return 2;
+
+		default:
+			return 1;
 		}
 	}
 
@@ -128,28 +130,29 @@ HullType hullType(docstring const & s)
 	if (s == "flalign")   return hullFlAlign;
 	if (s == "regexp")    return hullRegexp;
 	lyxerr << "unknown hull type '" << to_utf8(s) << "'" << endl;
-	return HullType(-1);
+	return hullUnknown;
 }
 
 
 docstring hullName(HullType type)
 {
 	switch (type) {
-		case hullNone:       return from_ascii("none");
-		case hullSimple:     return from_ascii("simple");
-		case hullEquation:   return from_ascii("equation");
-		case hullEqnArray:   return from_ascii("eqnarray");
-		case hullAlign:      return from_ascii("align");
-		case hullAlignAt:    return from_ascii("alignat");
-		case hullXAlignAt:   return from_ascii("xalignat");
-		case hullXXAlignAt:  return from_ascii("xxalignat");
-		case hullMultline:   return from_ascii("multline");
-		case hullGather:     return from_ascii("gather");
-		case hullFlAlign:    return from_ascii("flalign");
-		case hullRegexp:     return from_ascii("regexp");
-		default:
-			lyxerr << "unknown hull type '" << type << "'" << endl;
-			return from_ascii("none");
+	case hullNone:       return from_ascii("none");
+	case hullSimple:     return from_ascii("simple");
+	case hullEquation:   return from_ascii("equation");
+	case hullEqnArray:   return from_ascii("eqnarray");
+	case hullAlign:      return from_ascii("align");
+	case hullAlignAt:    return from_ascii("alignat");
+	case hullXAlignAt:   return from_ascii("xalignat");
+	case hullXXAlignAt:  return from_ascii("xxalignat");
+	case hullMultline:   return from_ascii("multline");
+	case hullGather:     return from_ascii("gather");
+	case hullFlAlign:    return from_ascii("flalign");
+	case hullRegexp:     return from_ascii("regexp");
+	case hullUnknown:
+	default:
+		lyxerr << "unknown hull type '" << type << "'" << endl;
+		return from_ascii("unknown");
 	}
 }
 
@@ -326,10 +329,15 @@ Inset * InsetMathHull::editXY(Cursor & cur, int x, int y)
 
 InsetMath::mode_type InsetMathHull::currentMode() const
 {
-	if (type_ == hullNone)
+	switch (type_) {
+	case hullNone:
+	case hullUnknown:
 		return UNDECIDED_MODE;
+
 	// definitely math mode ...
-	return MATH_MODE;
+	default:
+		return MATH_MODE;
+	}
 }
 
 
@@ -351,15 +359,24 @@ bool InsetMathHull::idxLast(Cursor & cur) const
 
 char InsetMathHull::defaultColAlign(col_type col)
 {
-	if (type_ == hullEqnArray)
+	switch (type_) {
+	case hullEqnArray:
 		return "rcl"[col];
-	if (type_ == hullMultline)
+
+	case hullMultline:
+	case hullGather:
 		return 'c';
-	if (type_ == hullGather)
-		return 'c';
-	if (type_ >= hullAlign)
+
+	case hullAlign:
+	case hullFlAlign:
+	case hullAlignAt:
+	case hullXAlignAt:
+	case hullXXAlignAt:
 		return "rl"[col & 1];
-	return 'c';
+
+	default:
+		return 'c';
+	}
 }
 
 
@@ -378,45 +395,48 @@ char InsetMathHull::displayColAlign(idx_type idx) const
 
 int InsetMathHull::defaultColSpace(col_type col)
 {
-	if (type_ == hullAlign || type_ == hullAlignAt)
+	switch (type_) {
+	case hullAlign:
+	case hullAlignAt:
+		//FIXME: this should be wider
 		return 0;
-	if (type_ == hullXAlignAt)
+
+	case hullXAlignAt:
 		return (col & 1) ? 20 : 0;
-	if (type_ == hullXXAlignAt || type_ == hullFlAlign)
+
+	case hullXXAlignAt:
+	case hullFlAlign:
 		return (col & 1) ? 40 : 0;
-	return 0;
+
+	default:
+		return 0;
+	}
 }
 
 
 docstring InsetMathHull::standardFont() const
 {
-	docstring font_name;
 	switch (type_) {
 	case hullRegexp:
-		font_name = from_ascii("texttt");
-		break;
+		return from_ascii("texttt");
 	case hullNone:
-		font_name = from_ascii("lyxnochange");
-		break;
+		return from_ascii("lyxnochange");
 	default:
-		font_name = from_ascii("mathnormal");
+		return from_ascii("mathnormal");
 	}
-	return font_name;
 }
 
 
 ColorCode InsetMathHull::standardColor() const
 {
-	ColorCode color;
 	switch (type_) {
 	case hullRegexp:
 	case hullNone:
-		color = Color_foreground;
-		break;
+		return Color_foreground;
+
 	default:
-		color = Color_math;
+		return Color_math;
 	}
-	return color;
 }
 
 
@@ -840,20 +860,17 @@ bool InsetMathHull::numbered(row_type row) const
 bool InsetMathHull::ams() const
 {
 	switch (type_) {
-		case hullAlign:
-		case hullFlAlign:
-		case hullMultline:
-		case hullGather:
-		case hullAlignAt:
-		case hullXAlignAt:
-		case hullXXAlignAt:
-			return true;
-		case hullNone:
-		case hullSimple:
-		case hullEquation:
-		case hullEqnArray:
-		case hullRegexp:
-			break;
+	case hullAlign:
+	case hullFlAlign:
+	case hullMultline:
+	case hullGather:
+	case hullAlignAt:
+	case hullXAlignAt:
+	case hullXXAlignAt:
+		return true;
+
+	default:
+		break;
 	}
 	for (size_t row = 0; row < numbered_.size(); ++row)
 		if (numbered_[row] == NOTAG)
@@ -864,25 +881,34 @@ bool InsetMathHull::ams() const
 
 Inset::DisplayType InsetMathHull::display() const
 {
-	if (type_ == hullSimple || type_ == hullNone || type_ == hullRegexp)
+	switch (type_) {
+	case hullSimple:
+	case hullNone:
+	case hullRegexp:
+	case hullUnknown:
 		return Inline;
-	return AlignCenter;
+
+	default:
+		return AlignCenter;
+	}
 }
 
 bool InsetMathHull::numberedType() const
 {
-	if (type_ == hullNone)
-		return false;
-	if (type_ == hullSimple)
-		return false;
-	if (type_ == hullXXAlignAt)
-		return false;
-	if (type_ == hullRegexp)
-		return false;
-	for (row_type row = 0; row < nrows(); ++row)
-		if (numbered(row))
-			return true;
-	return false;
+	switch (type_) {
+	case hullNone:
+	case hullSimple:
+	case hullXXAlignAt:
+	case hullRegexp:
+	case hullUnknown:
+		return false;
+
+	default:
+		for (row_type row = 0; row < nrows(); ++row)
+			if (numbered(row))
+				return true;
+		return false;
+	}
 }
 
 
@@ -1241,10 +1267,41 @@ void InsetMathHull::setType(HullType type)
 }
 
 
+bool InsetMathHull::isMutable(HullType type)
+{
+	switch (type) {
+	case hullNone:
+	case hullSimple:
+	case hullEquation:
+	case hullEqnArray:
+	case hullAlign:
+	case hullFlAlign:
+	case hullAlignAt:
+	case hullXAlignAt:
+	case hullXXAlignAt:
+	case hullMultline:
+	case hullGather:
+		return true;
+	default:
+		return false;
+	}
+}
+
+
 void InsetMathHull::mutate(HullType newtype)
 {
 	//lyxerr << "mutating from '" << type_ << "' to '" << newtype << "'" << endl;
 
+  	if (newtype == type_)
+		return;
+
+	// This guards the algorithm below it, which is designed with certain types
+	// in mind.
+	if (!isMutable(newtype) || !isMutable(type_))
+		lyxerr << "mutation from '" << to_utf8(hullName(type_))
+		       << "' to '" << to_utf8(hullName(newtype))
+		       << "' not implemented" << endl;
+
 	// we try to move along the chain
 	// none <-> simple <-> equation <-> eqnarray -> *align* -> multline, gather -+
 	//                                     ^                                     |
@@ -1253,22 +1310,14 @@ void InsetMathHull::mutate(HullType newtype)
 	// directly supported because it handles labels and numbering for
 	// "down mutation".
 
-	if (newtype == type_) {
-		// done
-	}
-
-	else if (newtype < hullNone) {
-		// unknown type
-		dump();
-	}
-
-	else if (type_ == hullNone) {
+	switch (type_) {
+	case hullNone:
 		setType(hullSimple);
 		numbered(0, false);
 		mutate(newtype);
-	}
+		break;
 
-	else if (type_ == hullSimple) {
+	case hullSimple:
 		if (newtype == hullNone) {
 			setType(hullNone);
 			numbered(0, false);
@@ -1277,95 +1326,138 @@ void InsetMathHull::mutate(HullType newtype)
 			numbered(0, label_[0] ? true : false);
 			mutate(newtype);
 		}
-	}
+		break;
 
-	else if (type_ == hullEquation) {
-		if (newtype < type_) {
+	case hullEquation:
+		switch (newtype) {
+		case hullNone:
+		case hullSimple:
 			setType(hullSimple);
 			numbered(0, false);
 			mutate(newtype);
-		} else if (newtype == hullEqnArray) {
+			break;
+		case hullEqnArray:
 			// split it "nicely" on the first relop
 			splitTo3Cols();
 			setType(hullEqnArray);
-		} else if (newtype == hullMultline || newtype == hullGather) {
+			break;
+		case hullMultline:
+		case hullGather:
 			setType(newtype);
-		} else {
+			break;
+		default:
+			// *align*
 			// split it "nicely"
 			splitTo2Cols();
 			setType(hullAlign);
 			mutate(newtype);
+			break;
 		}
-	}
+		break;
 
-	else if (type_ == hullEqnArray) {
-		if (newtype < type_) {
+	case hullEqnArray:
+		switch (newtype) {
+		case hullNone:
+		case hullSimple:
+		case hullEquation:
 			glueall(newtype);
 			mutate(newtype);
-		} else { // align & Co.
+			break;
+		default:
+			// align & Co.
 			changeCols(2);
 			setType(hullAlign);
 			mutate(newtype);
+			break;
 		}
-	}
+		break;
 
-	else if (type_ ==  hullAlign || type_ == hullAlignAt ||
-		 type_ == hullXAlignAt || type_ == hullFlAlign) {
-		if (newtype < hullAlign) {
+	case hullAlign:
+	case hullAlignAt:
+	case hullXAlignAt:
+	case hullFlAlign:
+		switch (newtype) {
+		case hullNone:
+		case hullSimple:
+		case hullEquation:
+		case hullEqnArray:
 			changeCols(3);
 			setType(hullEqnArray);
 			mutate(newtype);
-		} else if (newtype == hullGather || newtype == hullMultline) {
+			break;
+		case hullGather:
+		case hullMultline:
 			changeCols(1);
 			setType(newtype);
-		} else if (newtype ==   hullXXAlignAt) {
+			break;
+		case hullXXAlignAt:
 			for (row_type row = 0; row < nrows(); ++row)
 				numbered(row, false);
 			setType(newtype);
-		} else {
+			break;
+		default:
 			setType(newtype);
+			break;
 		}
-	}
+		break;
 
-	else if (type_ == hullXXAlignAt) {
+	case hullXXAlignAt:
 		for (row_type row = 0; row < nrows(); ++row)
 			numbered(row, false);
-		if (newtype < hullAlign) {
+		switch (newtype) {
+		case hullNone:
+		case hullSimple:
+		case hullEquation:
+		case hullEqnArray:
 			changeCols(3);
 			setType(hullEqnArray);
 			mutate(newtype);
-		} else if (newtype == hullGather || newtype == hullMultline) {
+			break;
+		case hullGather:
+		case hullMultline:
 			changeCols(1);
 			setType(newtype);
-		} else {
+			break;
+		default:
 			setType(newtype);
+			break;
 		}
-	}
+		break;
 
-	else if (type_ == hullMultline || type_ == hullGather) {
-		if (newtype == hullGather || newtype == hullMultline)
+	case hullMultline:
+	case hullGather:
+		switch (newtype) {
+		case hullGather:
+		case hullMultline:
 			setType(newtype);
-		else if (newtype == hullAlign || newtype == hullFlAlign  ||
-			 newtype == hullAlignAt || newtype == hullXAlignAt) {
+			break;
+		case hullAlign:
+		case hullFlAlign:
+		case hullAlignAt:
+		case hullXAlignAt:
 			splitTo2Cols();
 			setType(newtype);
-		} else if (newtype ==   hullXXAlignAt) {
+			break;
+		case hullXXAlignAt:
 			splitTo2Cols();
 			for (row_type row = 0; row < nrows(); ++row)
 				numbered(row, false);
 			setType(newtype);
-		} else {
+			break;
+		default:
+			// first we mutate to EqnArray
 			splitTo3Cols();
 			setType(hullEqnArray);
 			mutate(newtype);
+			break;
 		}
-	}
+		break;
 
-	else {
-		lyxerr << "mutation from '" << to_utf8(hullName(type_))
-		       << "' to '" << to_utf8(hullName(newtype))
-		       << "' not implemented" << endl;
-	}
+	default:
+		// we passed the guard so we should not be here
+		LASSERT("Mutation not implemented, but should have been.", return);
+		break;
+	}// switch
 }
 
 
@@ -1733,9 +1825,9 @@ bool InsetMathHull::getStatus(Cursor & cur, FuncRequest const & cmd,
 	case LFUN_MATH_MUTATE: {
 		HullType const ht = hullType(cmd.argument());
 		status.setOnOff(type_ == ht);
-		status.setEnabled(true);
+		status.setEnabled(isMutable(ht) && isMutable(type_));
 
-		if (ht != hullSimple) {
+		if (ht != hullSimple && status.enabled()) {
 			Cursor tmpcur = cur;
 			while (!tmpcur.empty()) {
 				InsetCode code = tmpcur.inset().lyxCode();
diff --git a/src/mathed/InsetMathHull.h b/src/mathed/InsetMathHull.h
index 9598ad4..462cac2 100644
--- a/src/mathed/InsetMathHull.h
+++ b/src/mathed/InsetMathHull.h
@@ -105,6 +105,8 @@ public:
 
 	/// get type
 	HullType getType() const;
+	/// is mutation implemented for this type?
+	static bool isMutable(HullType type);
 	/// change type
 	void mutate(HullType newtype);
 
-- 
2.1.4

>From 0bad1f24efd67bbdf32df5ec276ee326398592d9 Mon Sep 17 00:00:00 2001
From: Guillaume Munch <g...@lyx.org>
Date: Sun, 13 Dec 2015 03:32:32 +0000
Subject: [PATCH 2/3] Display the correct horizontal alignment in AMS
 environments

A longstanding problem... (related: #1861)

The columns in AMS math environments have a fixed alignment (colAlign() in
InsetMathGrid.cpp). We set this alignment for display (Georg's
displayColAlign()) in InsetMathHull and InsetMathSplit. This is done according
to tests and documentation for the various environments.

There is also some mechanical code factoring via colAlign(). (Note that I am not
sure that defaultColAlign() in either classes matters anymore for AMS
environments.)

Finally, I disable setting the horizontal alignment in InsetMathSplit, which has
no impact on the LaTeX output, and has no longer any impact on the screen. (As
for vertical alignment I discovered that it was in fact customisable for
\aligned & friends! I hope that the more faithful interface will let other
users discover that too.)
---
 src/mathed/InsetMathGrid.cpp  | 26 ++++++++++++++++++++++++++
 src/mathed/InsetMathGrid.h    |  5 +++--
 src/mathed/InsetMathHull.cpp  | 37 ++++++++++++++++---------------------
 src/mathed/InsetMathSplit.cpp | 36 ++++++++++++++++++++++++++++++------
 src/mathed/InsetMathSplit.h   |  2 ++
 5 files changed, 77 insertions(+), 29 deletions(-)

diff --git a/src/mathed/InsetMathGrid.cpp b/src/mathed/InsetMathGrid.cpp
index 8ea3940..fcab5f8 100644
--- a/src/mathed/InsetMathGrid.cpp
+++ b/src/mathed/InsetMathGrid.cpp
@@ -1837,4 +1837,30 @@ bool InsetMathGrid::getStatus(Cursor & cur, FuncRequest const & cmd,
 }
 
 
+// static
+char InsetMathGrid::colAlign(HullType type, col_type col)
+{
+	switch (type) {
+	case hullMultline:
+	case hullGather:
+		return 'c';
+
+	case hullEqnArray:
+		return "rcl"[col % 3];
+
+	case hullAlign:
+	case hullAlignAt:
+	case hullXAlignAt:
+	case hullXXAlignAt:
+	case hullFlAlign:
+		return "rl"[col & 1];
+
+	default:
+		break;
+	}
+	return 'c';
+}
+
+
+
 } // namespace lyx
diff --git a/src/mathed/InsetMathGrid.h b/src/mathed/InsetMathGrid.h
index bd3066d..709f492 100644
--- a/src/mathed/InsetMathGrid.h
+++ b/src/mathed/InsetMathGrid.h
@@ -258,10 +258,11 @@ protected:
 	virtual docstring eocString(col_type col, col_type lastcol) const;
 	/// splits cells and shifts right part to the next cell
 	void splitCell(Cursor & cur);
-	/// Column aligmment for display of cell \p idx.
+	/// Column alignment for display of cell \p idx.
 	/// Must not be written to file!
 	virtual char displayColAlign(idx_type idx) const;
-
+	/// The value of a fixed col align for a certain hull type 
+	static char colAlign(HullType type, col_type col);
 
 	/// row info.
 	/// rowinfo_[nrows()] is a dummy row used only for hlines.
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index 68318a2..f09ec27 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -359,35 +359,31 @@ bool InsetMathHull::idxLast(Cursor & cur) const
 
 char InsetMathHull::defaultColAlign(col_type col)
 {
-	switch (type_) {
-	case hullEqnArray:
-		return "rcl"[col];
-
-	case hullMultline:
-	case hullGather:
-		return 'c';
-
-	case hullAlign:
-	case hullFlAlign:
-	case hullAlignAt:
-	case hullXAlignAt:
-	case hullXXAlignAt:
-		return "rl"[col & 1];
-
-	default:
-		return 'c';
-	}
+	return colAlign(type_, col);
 }
 
 
 char InsetMathHull::displayColAlign(idx_type idx) const
 {
-	if (type_ == hullMultline) {
+	switch (type_) {
+	case hullMultline: {
 		row_type const r = row(idx);
 		if (r == 0)
 			return 'l';
 		if (r == nrows() - 1)
 			return 'r';
+		return 'c';
+	}
+	case hullEqnArray:
+	case hullGather:
+	case hullAlign:
+	case hullAlignAt:
+	case hullXAlignAt:
+	case hullXXAlignAt:
+	case hullFlAlign:
+		return colAlign(type_, col(idx));
+	default:
+		break;
 	}
 	return InsetMathGrid::displayColAlign(idx);
 }
@@ -1962,10 +1958,9 @@ bool InsetMathHull::getStatus(Cursor & cur, FuncRequest const & cmd,
 	}
 
 	default:
-		return InsetMathGrid::getStatus(cur, cmd, status);
+		break;
 	}
 
-	// This cannot really happen, but inserted to shut-up gcc
 	return InsetMathGrid::getStatus(cur, cmd, status);
 }
 
diff --git a/src/mathed/InsetMathSplit.cpp b/src/mathed/InsetMathSplit.cpp
index 5c425fb..a2f919f 100644
--- a/src/mathed/InsetMathSplit.cpp
+++ b/src/mathed/InsetMathSplit.cpp
@@ -50,18 +50,38 @@ Inset * InsetMathSplit::clone() const
 
 char InsetMathSplit::defaultColAlign(col_type col)
 {
-	if (name_ == "split")
-		return 'l';
 	if (name_ == "gathered")
 		return 'c';
-	if (name_ == "aligned" || name_ == "align")
-		return (col & 1) ? 'l' : 'r';
-	if (name_ == "alignedat")
-		return (col & 1) ? 'l' : 'r';
+	if (name_ == "lgathered")
+		return 'l';
+	if (name_ == "rgathered")
+		return 'r';
+	if (name_ == "split"
+	    || name_ == "aligned"
+	    || name_ == "align"
+	    || name_ == "alignedat")
+		return colAlign(hullAlign, col);
 	return 'l';
 }
 
 
+char InsetMathSplit::displayColAlign(idx_type idx) const
+{
+	if (name_ == "gathered")
+		return 'c';
+	if (name_ == "lgathered")
+		return 'l';
+	if (name_ == "rgathered")
+		return 'r';
+	if (name_ == "split"
+	    || name_ == "aligned"
+	    || name_ == "align"
+	    || name_ == "alignedat")
+		return colAlign(hullAlign, col(idx));
+	return InsetMathGrid::displayColAlign(idx);
+}
+
+
 void InsetMathSplit::draw(PainterInfo & pi, int x, int y) const
 {
 	InsetMathGrid::draw(pi, x, y);
@@ -86,6 +106,10 @@ bool InsetMathSplit::getStatus(Cursor & cur, FuncRequest const & cmd,
 			flag.setEnabled(false);
 			return true;
 		}
+		if (s == "align-left" || s == "align-center" || s == "align-right") {
+			flag.setEnabled(false);
+			return true;
+		}
 		break;
 	}
 	default:
diff --git a/src/mathed/InsetMathSplit.h b/src/mathed/InsetMathSplit.h
index b0ff437..1b2aa23 100644
--- a/src/mathed/InsetMathSplit.h
+++ b/src/mathed/InsetMathSplit.h
@@ -43,6 +43,8 @@ public:
 	///
 	char defaultColAlign(col_type);
 	///
+	char displayColAlign(idx_type idx) const;
+	///
 	InsetCode lyxCode() const { return MATH_SPLIT_CODE; }
 
 private:
-- 
2.1.4

>From fdf24ee0ea9ed3ee53347f17f475cc5852d7fa74 Mon Sep 17 00:00:00 2001
From: Guillaume Munch <g...@lyx.org>
Date: Mon, 14 Dec 2015 03:40:57 +0000
Subject: [PATCH 3/3] Fix the display of column spacing in AMS environments

AMS align environment should have some spacing between odd and even columns.

Add a new virtual method displayColSpace() to InsetMathGrid, InsetMathHull and
InsetMathSplit. I left defaultColSpace() unchanged for fear that it would affect
the output on some other backend than LaTeX, though as I see it, it has to be
broken anyway.
---
 src/mathed/InsetMathGrid.cpp  | 38 ++++++++++++++++++++++++++++++++++----
 src/mathed/InsetMathGrid.h    | 10 +++++++++-
 src/mathed/InsetMathHull.cpp  |  7 ++++++-
 src/mathed/InsetMathHull.h    |  2 ++
 src/mathed/InsetMathSplit.cpp | 11 +++++++++++
 src/mathed/InsetMathSplit.h   |  2 ++
 6 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/src/mathed/InsetMathGrid.cpp b/src/mathed/InsetMathGrid.cpp
index fcab5f8..bbf0a26 100644
--- a/src/mathed/InsetMathGrid.cpp
+++ b/src/mathed/InsetMathGrid.cpp
@@ -486,7 +486,7 @@ void InsetMathGrid::metrics(MetricsInfo & mi, Dimension & dim) const
 		colinfo_[col].offset_ =
 			colinfo_[col - 1].offset_ +
 			colinfo_[col - 1].width_ +
-			colinfo_[col - 1].skip_ +
+			displayColSpace(col - 1) +
 			colsep() +
 			colinfo_[col].lines_ * vlinesep();
 	}
@@ -508,7 +508,7 @@ void InsetMathGrid::metrics(MetricsInfo & mi, Dimension & dim) const
 			int const nextoffset =
 				colinfo_[first].offset_ +
 				wid +
-				colinfo_[last].skip_ +
+				displayColSpace(last) +
 				colsep() +
 				colinfo_[last+1].lines_ * vlinesep();
 			int const dx = nextoffset - colinfo_[last+1].offset_;
@@ -741,7 +741,7 @@ void InsetMathGrid::metricsT(TextMetricsInfo const & mi, Dimension & dim) const
 		colinfo_[col].offset_ =
 			colinfo_[col - 1].offset_ +
 			colinfo_[col - 1].width_ +
-			colinfo_[col - 1].skip_ +
+			displayColSpace(col - 1) +
 			1 ; //colsep() +
 			//colinfo_[col].lines_ * vlinesep();
 	}
@@ -953,7 +953,7 @@ int InsetMathGrid::cellWidth(idx_type idx) const
 		col_type c2 = c1 + ncellcols(idx);
 		return colinfo_[c2].offset_
 			- colinfo_[c1].offset_
-			- colinfo_[c2].skip_
+			- displayColSpace(c2)
 			- colsep()
 			- colinfo_[c2].lines_ * vlinesep();
 	}
@@ -1378,6 +1378,11 @@ char InsetMathGrid::displayColAlign(idx_type idx) const
 }
 
 
+int InsetMathGrid::displayColSpace(col_type col) const
+{
+	return colinfo_[col].skip_;
+}
+
 void InsetMathGrid::doDispatch(Cursor & cur, FuncRequest & cmd)
 {
 	//lyxerr << "*** InsetMathGrid: request: " << cmd << endl;
@@ -1862,5 +1867,30 @@ char InsetMathGrid::colAlign(HullType type, col_type col)
 }
 
 
+//static
+int InsetMathGrid::colSpace(HullType type, col_type col)
+{
+	int alignInterSpace;
+	switch (type) {
+	// FIXME: some values should vary depending on the available length. See #1861
+	case hullAlign:
+		alignInterSpace = 20;
+		break;
+	case hullAlignAt:
+		alignInterSpace = 0;
+		break;
+	case hullXAlignAt:
+		alignInterSpace = 40;
+		break;
+	case hullXXAlignAt:
+	case hullFlAlign:
+		alignInterSpace = 60;
+		break;
+	default:
+		return 0;
+	}
+	return (col % 2) ? alignInterSpace : 0;
+}
+
 
 } // namespace lyx
diff --git a/src/mathed/InsetMathGrid.h b/src/mathed/InsetMathGrid.h
index 709f492..d4df80d 100644
--- a/src/mathed/InsetMathGrid.h
+++ b/src/mathed/InsetMathGrid.h
@@ -261,8 +261,16 @@ protected:
 	/// Column alignment for display of cell \p idx.
 	/// Must not be written to file!
 	virtual char displayColAlign(idx_type idx) const;
-	/// The value of a fixed col align for a certain hull type 
+	/// Column spacing for display of column \p col.
+	/// Must not be written to file!
+	virtual int displayColSpace(col_type col) const;
+
+	// The following two functions are used in InsetMathHull and
+	// InsetMathSplit.
+	/// The value of a fixed col align for a certain hull type
 	static char colAlign(HullType type, col_type col);
+	/// The value of a fixed col spacing for a certain hull type
+	static int colSpace(HullType type, col_type col);
 
 	/// row info.
 	/// rowinfo_[nrows()] is a dummy row used only for hlines.
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index f09ec27..c4e80ce 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -394,7 +394,6 @@ int InsetMathHull::defaultColSpace(col_type col)
 	switch (type_) {
 	case hullAlign:
 	case hullAlignAt:
-		//FIXME: this should be wider
 		return 0;
 
 	case hullXAlignAt:
@@ -410,6 +409,12 @@ int InsetMathHull::defaultColSpace(col_type col)
 }
 
 
+int InsetMathHull::displayColSpace(col_type col) const
+{
+	return colSpace(type_, col);
+}
+
+
 docstring InsetMathHull::standardFont() const
 {
 	switch (type_) {
diff --git a/src/mathed/InsetMathHull.h b/src/mathed/InsetMathHull.h
index 462cac2..233d444 100644
--- a/src/mathed/InsetMathHull.h
+++ b/src/mathed/InsetMathHull.h
@@ -115,6 +115,8 @@ public:
 	///
 	char defaultColAlign(col_type col);
 	///
+	int displayColSpace(col_type col) const;
+	///
 	char displayColAlign(idx_type idx) const;
 	///
 	bool idxFirst(Cursor &) const;
diff --git a/src/mathed/InsetMathSplit.cpp b/src/mathed/InsetMathSplit.cpp
index a2f919f..5e01561 100644
--- a/src/mathed/InsetMathSplit.cpp
+++ b/src/mathed/InsetMathSplit.cpp
@@ -82,6 +82,17 @@ char InsetMathSplit::displayColAlign(idx_type idx) const
 }
 
 
+int InsetMathSplit::displayColSpace(col_type col) const
+{
+	if (name_ == "split" || name_ == "aligned" || name_ == "align")
+		return colSpace(hullAlign, col);
+	if (name_ == "alignedat")
+		return colSpace(hullAlignAt, col);
+	return 0;
+}
+
+
+
 void InsetMathSplit::draw(PainterInfo & pi, int x, int y) const
 {
 	InsetMathGrid::draw(pi, x, y);
diff --git a/src/mathed/InsetMathSplit.h b/src/mathed/InsetMathSplit.h
index 1b2aa23..d9bc52f 100644
--- a/src/mathed/InsetMathSplit.h
+++ b/src/mathed/InsetMathSplit.h
@@ -43,6 +43,8 @@ public:
 	///
 	char defaultColAlign(col_type);
 	///
+	int displayColSpace(col_type col) const;
+	///
 	char displayColAlign(idx_type idx) const;
 	///
 	InsetCode lyxCode() const { return MATH_SPLIT_CODE; }
-- 
2.1.4

Reply via email to