Changers

Yuriy Skalko yuriy.skalko at gmail.com
Wed Nov 11 12:08:13 UTC 2020


>>  Now I've found that Changer is just no-op variant of RefChanger. Their separation into different headers confused me. 
> 
> 
> Isn't it just some type of abstract base class?
> 
> JMarc

Yes, it is base class, but it cannot be abstract because it is used in 
client code for no-change cases.

I've tried to simplify and make Changers implementation more consistent 
and located in one place. Please check the attached patch.
	

Yuriy

-------------- next part --------------
From f106663644174b924f0782ef99aac992bbd8f933 Mon Sep 17 00:00:00 2001
From: Yuriy Skalko <yuriy.skalko at gmail.com>
Date: Wed, 11 Nov 2020 12:10:41 +0200
Subject: [PATCH 1/2] Simplify Changers

---
 src/FontInfo.cpp                |  1 -
 src/MetricsInfo.cpp             | 20 ++++-----
 src/TextMetrics.cpp             |  2 +-
 src/insets/InsetCollapsible.cpp |  4 +-
 src/insets/InsetText.cpp        |  2 +-
 src/mathed/InsetMathChar.cpp    |  4 +-
 src/mathed/InsetMathDiagram.cpp |  4 +-
 src/mathed/InsetMathFontOld.cpp |  4 +-
 src/mathed/InsetMathFrac.cpp    |  4 +-
 src/mathed/InsetMathHull.cpp    |  6 +--
 src/mathed/InsetMathMacro.cpp   |  2 +-
 src/mathed/MathStream.cpp       |  1 -
 src/mathed/MathSupport.cpp      |  5 ++-
 src/support/Changer.h           | 62 ++++++++++++++++++++++++++--
 src/support/Makefile.am         |  1 -
 src/support/RefChanger.h        | 73 ---------------------------------
 16 files changed, 86 insertions(+), 109 deletions(-)
 delete mode 100644 src/support/RefChanger.h

diff --git a/src/FontInfo.cpp b/src/FontInfo.cpp
index 967771373a..8a333810ee 100644
--- a/src/FontInfo.cpp
+++ b/src/FontInfo.cpp
@@ -25,7 +25,6 @@
 #include "support/docstring.h"
 #include "support/gettext.h"
 #include "support/lstrings.h"
-#include "support/RefChanger.h"
 
 #include <algorithm>
 #include <ostream>
diff --git a/src/MetricsInfo.cpp b/src/MetricsInfo.cpp
index 5aeb9a2980..c62f77f045 100644
--- a/src/MetricsInfo.cpp
+++ b/src/MetricsInfo.cpp
@@ -21,8 +21,6 @@
 #include "frontends/FontMetrics.h"
 #include "frontends/Painter.h"
 
-#include "support/RefChanger.h"
-
 using namespace std;
 
 
@@ -83,16 +81,16 @@ Changer MetricsBase::changeEnsureMath(Inset::mode_type mode)
 {
 	switch (mode) {
 	case Inset::UNDECIDED_MODE:
-		return Changer();
+		return make_change();
 	case Inset::TEXT_MODE:
-		return isMathFont(fontname) ? changeFontSet("textnormal") : Changer();
+		return isMathFont(fontname) ? changeFontSet("textnormal") : make_change();
 	case Inset::MATH_MODE:
 		// FIXME:
 		//   \textit{\ensuremath{\text{a}}}
 		// should appear in italics
-		return isTextFont(fontname) ? changeFontSet("mathnormal"): Changer();
+		return isTextFont(fontname) ? changeFontSet("mathnormal"): make_change();
 	}
-	return Changer();
+	return make_change();
 }
 
 
@@ -196,10 +194,10 @@ Changer MetricsBase::changeScript()
 		return font.changeStyle(SCRIPTSCRIPT_STYLE);
 	case INHERIT_STYLE:
 	case IGNORE_STYLE:
-		return Changer();
+		return make_change();
 	}
 	//remove Warning
-	return Changer();
+	return make_change();
 }
 
 
@@ -215,10 +213,10 @@ Changer MetricsBase::changeFrac()
 		return font.changeStyle(SCRIPTSCRIPT_STYLE);
 	case INHERIT_STYLE:
 	case IGNORE_STYLE:
-		return Changer();
+		return make_change();
 	}
 	//remove Warning
-	return Changer();
+	return make_change();
 }
 
 
@@ -227,7 +225,7 @@ Changer MetricsBase::changeArray(bool small)
 	if (small)
 		return font.changeStyle(SCRIPT_STYLE);
 	return (font.style() == DISPLAY_STYLE) ? font.changeStyle(TEXT_STYLE)
-		: Changer();
+		: make_change();
 }
 
 
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index 5cd7a603a0..36f34acd2b 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -43,7 +43,7 @@
 
 #include "support/debug.h"
 #include "support/lassert.h"
-#include "support/RefChanger.h"
+#include "support/Changer.h"
 
 #include <stdlib.h>
 #include <cmath>
diff --git a/src/insets/InsetCollapsible.cpp b/src/insets/InsetCollapsible.cpp
index c118545002..632eea80a6 100644
--- a/src/insets/InsetCollapsible.cpp
+++ b/src/insets/InsetCollapsible.cpp
@@ -39,7 +39,7 @@
 #include "support/gettext.h"
 #include "support/lassert.h"
 #include "support/lstrings.h"
-#include "support/RefChanger.h"
+#include "support/Changer.h"
 #include "support/TempFile.h"
 
 using namespace std;
@@ -315,7 +315,7 @@ void InsetCollapsible::draw(PainterInfo & pi, int x, int y) const
 		// that's enough.
 		Changer cdummy = (pi.change.type == Change::INSERTED)
 			? make_change(pi.change, Change())
-			: Changer();
+			: make_change();
 		InsetText::draw(pi, textx, texty);
 		break;
 	}
diff --git a/src/insets/InsetText.cpp b/src/insets/InsetText.cpp
index 3f97d4f728..95ca9b3374 100644
--- a/src/insets/InsetText.cpp
+++ b/src/insets/InsetText.cpp
@@ -61,7 +61,7 @@
 #include "support/gettext.h"
 #include "support/lassert.h"
 #include "support/lstrings.h"
-#include "support/RefChanger.h"
+#include "support/Changer.h"
 
 #include <algorithm>
 #include <stack>
diff --git a/src/mathed/InsetMathChar.cpp b/src/mathed/InsetMathChar.cpp
index aa1ddf8c5b..4239a8119d 100644
--- a/src/mathed/InsetMathChar.cpp
+++ b/src/mathed/InsetMathChar.cpp
@@ -121,7 +121,7 @@ void InsetMathChar::metrics(MetricsInfo & mi, Dimension & dim) const
 	} else if (!isASCII(char_) && Encodings::unicodeCharInfo(char_).isUnicodeSymbol()) {
 		Changer dummy1 = mi.base.changeFontSet("mathnormal");
 		Changer dummy2 = Encodings::isMathAlpha(char_)
-				? Changer()
+				? make_change()
 				: mi.base.font.changeShape(UP_SHAPE);
 		dim = theFontMetrics(mi.base.font).dimension(char_);
 		kerning_ = -mathed_char_kerning(mi.base.font, char_);
@@ -166,7 +166,7 @@ void InsetMathChar::draw(PainterInfo & pi, int x, int y) const
 		} else if (!isASCII(char_) && Encodings::unicodeCharInfo(char_).isUnicodeSymbol()) {
 			Changer dummy1 = pi.base.changeFontSet("mathnormal");
 			Changer dummy2 = Encodings::isMathAlpha(char_)
-					? Changer()
+					? make_change()
 					: pi.base.font.changeShape(UP_SHAPE);
 			pi.draw(x, y, char_);
 			return;
diff --git a/src/mathed/InsetMathDiagram.cpp b/src/mathed/InsetMathDiagram.cpp
index 4a7ea3c3b8..59aa4d2692 100644
--- a/src/mathed/InsetMathDiagram.cpp
+++ b/src/mathed/InsetMathDiagram.cpp
@@ -51,7 +51,7 @@ void InsetMathDiagram::metrics(MetricsInfo & mi, Dimension & dim) const
 	Changer dummy2 = mi.base.changeEnsureMath();
 	FontInfo & f = mi.base.font;
 	Changer dummy = (f.style() == DISPLAY_STYLE) ? f.changeStyle(TEXT_STYLE)
-		: Changer();
+		: make_change();
 	InsetMathGrid::metrics(mi, dim);
 }
 
@@ -61,7 +61,7 @@ void InsetMathDiagram::draw(PainterInfo & pi, int x, int y) const
 	Changer dummy2 = pi.base.changeEnsureMath();
 	FontInfo & f = pi.base.font;
 	Changer dummy = (f.style() == DISPLAY_STYLE) ? f.changeStyle(TEXT_STYLE)
-		: Changer();
+		: make_change();
 	InsetMathGrid::draw(pi, x, y);
 }
 
diff --git a/src/mathed/InsetMathFontOld.cpp b/src/mathed/InsetMathFontOld.cpp
index cde388840c..828064f663 100644
--- a/src/mathed/InsetMathFontOld.cpp
+++ b/src/mathed/InsetMathFontOld.cpp
@@ -60,7 +60,7 @@ void InsetMathFontOld::metrics(MetricsInfo & mi, Dimension & dim) const
 	bool really_change_font = fontname != "textcal";
 
 	Changer dummy = really_change_font ? mi.base.changeFontSet(fontname)
-		: Changer();
+		: make_change();
 	cell(0).metrics(mi, dim);
 }
 
@@ -77,7 +77,7 @@ void InsetMathFontOld::draw(PainterInfo & pi, int x, int y) const
 	bool really_change_font = fontname != "textcal";
 
 	Changer dummy = really_change_font ? pi.base.changeFontSet(fontname)
-		: Changer();
+		: make_change();
 	cell(0).draw(pi, x, y);
 }
 
diff --git a/src/mathed/InsetMathFrac.cpp b/src/mathed/InsetMathFrac.cpp
index 03125e99ca..8d2bbc2c20 100644
--- a/src/mathed/InsetMathFrac.cpp
+++ b/src/mathed/InsetMathFrac.cpp
@@ -215,7 +215,7 @@ void InsetMathFrac::metrics(MetricsInfo & mi, Dimension & dim) const
 			dim.des = dim2.des;
 		}
 		Changer dummy = (kind_ == UNITFRAC) ? mi.base.font.changeShape(UP_SHAPE)
-			: Changer();
+			: make_change();
 		Changer dummy2 = mi.base.changeScript();
 		if (latexkeys const * slash = slash_symbol()) {
 			Dimension dimslash;
@@ -297,7 +297,7 @@ void InsetMathFrac::draw(PainterInfo & pi, int x, int y) const
 			xx += cell(2).dimension(*pi.base.bv).wid + 4;
 		}
 		Changer dummy = (kind_ == UNITFRAC) ? pi.base.font.changeShape(UP_SHAPE)
-			: Changer();
+			: make_change();
 		// nice fraction
 		Changer dummy2 = pi.base.changeScript();
 		cell(0).draw(pi, xx + 1, y - dy);
diff --git a/src/mathed/InsetMathHull.cpp b/src/mathed/InsetMathHull.cpp
index a026977021..6c17bd7fe8 100644
--- a/src/mathed/InsetMathHull.cpp
+++ b/src/mathed/InsetMathHull.cpp
@@ -65,7 +65,7 @@
 #include "support/filetools.h"
 #include "support/lassert.h"
 #include "support/lstrings.h"
-#include "support/RefChanger.h"
+#include "support/Changer.h"
 
 #include <sstream>
 
@@ -617,7 +617,7 @@ void InsetMathHull::draw(PainterInfo & pi, int x, int y) const
 		// Do not draw change tracking cue if taken care of by RowPainter
 		// already.
 		Changer dummy = !canPaintChange(*bv) ? make_change(pi.change, Change())
-			: Changer();
+			: make_change();
 		if (previewTooSmall(dim)) {
 			// we have an extra frame
 			preview_->draw(pi, x + ERROR_FRAME_WIDTH, y);
@@ -635,7 +635,7 @@ void InsetMathHull::draw(PainterInfo & pi, int x, int y) const
 				? Color_selectiontext : standardColor();
 	bool const really_change_color = pi.base.font.color() == Color_none;
 	Changer dummy0 = really_change_color ? pi.base.font.changeColor(color)
-		: Changer();
+		: make_change();
 	if (numberedType()) {
 		BufferParams::MathNumber const math_number = buffer().params().getMathNumber();
 		for (row_type row = 0; row < nrows(); ++row) {
diff --git a/src/mathed/InsetMathMacro.cpp b/src/mathed/InsetMathMacro.cpp
index 57294c0753..464b9cb60f 100644
--- a/src/mathed/InsetMathMacro.cpp
+++ b/src/mathed/InsetMathMacro.cpp
@@ -39,7 +39,7 @@
 #include "support/gettext.h"
 #include "support/lassert.h"
 #include "support/lstrings.h"
-#include "support/RefChanger.h"
+#include "support/Changer.h"
 #include "support/textutils.h"
 
 #include <ostream>
diff --git a/src/mathed/MathStream.cpp b/src/mathed/MathStream.cpp
index 3d759bc261..9b708f9292 100644
--- a/src/mathed/MathStream.cpp
+++ b/src/mathed/MathStream.cpp
@@ -19,7 +19,6 @@
 #include "TexRow.h"
 
 #include "support/docstring.h"
-#include "support/RefChanger.h"
 #include "support/textutils.h"
 
 #include <algorithm>
diff --git a/src/mathed/MathSupport.cpp b/src/mathed/MathSupport.cpp
index 4f276856e5..0b6a38020f 100644
--- a/src/mathed/MathSupport.cpp
+++ b/src/mathed/MathSupport.cpp
@@ -27,6 +27,7 @@
 #include "frontends/FontMetrics.h"
 #include "frontends/Painter.h"
 
+#include "support/Changer.h"
 #include "support/debug.h"
 #include "support/docstream.h"
 #include "support/lassert.h"
@@ -696,7 +697,7 @@ int mathedSymbolDim(MetricsBase & mb, Dimension & dim, latexkeys const * sym)
 				 mb.fontname != "mathfrak" &&
 				 mb.fontname != "mathcal" &&
 				 mb.fontname != "mathscr");
-	Changer dummy = change_font ? mb.changeFontSet(font) : Changer();
+	Changer dummy = change_font ? mb.changeFontSet(font) : make_change();
 	mathed_string_dim(mb.font, mathedSymbol(mb, sym), dim);
 	return mathed_char_kerning(mb.font, mathedSymbol(mb, sym).back());
 }
@@ -720,7 +721,7 @@ void mathedSymbolDraw(PainterInfo & pi, int x, int y, latexkeys const * sym)
 				 pi.base.fontname != "mathfrak" &&
 				 pi.base.fontname != "mathcal" &&
 				 pi.base.fontname != "mathscr");
-	Changer dummy = change_font ? pi.base.changeFontSet(font) : Changer();
+	Changer dummy = change_font ? pi.base.changeFontSet(font) : make_change();
 	pi.draw(x, y, mathedSymbol(pi.base, sym));
 }
 
diff --git a/src/support/Changer.h b/src/support/Changer.h
index 212bcf1d85..16b48acb26 100644
--- a/src/support/Changer.h
+++ b/src/support/Changer.h
@@ -17,16 +17,70 @@
 
 namespace lyx {
 
-// Forward declaration for support/RefChanger.h
 struct Revertible {
-	virtual ~Revertible() {}
-	virtual void revert() {}
-	virtual void keep() {}
+	virtual ~Revertible() = default;
 };
 
 using Changer = unique_ptr<Revertible>;
 
 
+/// A RefChanger records the current value of \param ref, allowing to
+/// temporarily assign new values to it.  The original value is restored
+/// automatically when the object is destroyed, unless it is disabled.
+///
+/// RefChanger is movable, and doing so prolongs the duration of the temporary
+/// assignment. This allows classes to supply their own changer methods.
+///
+/// Naturally, be careful not to extend the life of a RefChanger beyond that of
+/// the reference it modifies. The RefChanger can be disabled by calling
+/// ->keep() or ->revert(). Once disabled, the reference is never accessed
+/// again.
+template<typename X>
+class RevertibleRef : public Revertible {
+public:
+	RevertibleRef(X & ref) : ref(ref), old(ref), enabled(true) {}
+	//
+	~RevertibleRef() override { revert(); }
+	//
+	void revert() { if (enabled) { enabled = false; ref = old; } }
+	//
+	void keep() { enabled = false; }
+	//
+	X & ref;
+	X const old;
+private:
+	bool enabled;
+};
+
+
+template <typename X>
+using RefChanger = unique_ptr<RevertibleRef<X>>;
+
+
+/// Saves the value of \param ref in a movable object
+template <typename X>
+inline RefChanger<X> make_save(X & ref)
+{
+	return make_unique<RevertibleRef<X>>(ref);
+}
+
+inline Changer make_change()
+{
+	return Changer();
+}
+
+/// Temporarily assign value val to reference ref.
+/// To apply the change conditionally, one can write:
+///     Changer dummy = (cond) ? make_change(a, b) : make_change();
+template <typename X>
+inline Changer make_change(X & ref, X const val)
+{
+	auto rc = make_save(ref);
+	ref = val;
+	return rc;
+}
+
 } // namespace lyx
 
+
 #endif //LYX_CHANGER_H
diff --git a/src/support/Makefile.am b/src/support/Makefile.am
index 785b97f395..addeab86b8 100644
--- a/src/support/Makefile.am
+++ b/src/support/Makefile.am
@@ -98,7 +98,6 @@ liblyxsupport_a_SOURCES = \
 	qstring_helpers.cpp \
 	qstring_helpers.h \
 	regex.h \
-	RefChanger.h \
 	signals.h \
 	socktools.cpp \
 	socktools.h \
diff --git a/src/support/RefChanger.h b/src/support/RefChanger.h
deleted file mode 100644
index ae35cc3891..0000000000
--- a/src/support/RefChanger.h
+++ /dev/null
@@ -1,73 +0,0 @@
-// -*- C++ -*-
-/**
- * \file RefChanger.h
- * This file is part of LyX, the document processor.
- * Licence details can be found in the file COPYING.
- *
- * \author Guillaume Munch
- *
- * Full author contact details are available in file CREDITS.
- */
-
-#ifndef LYX_REFCHANGER_H
-#define LYX_REFCHANGER_H
-
-#include "support/Changer.h"
-
-
-namespace lyx {
-
-/// A RefChanger records the current value of \param ref, allowing to
-/// temporarily assign new values to it.  The original value is restored
-/// automatically when the object is destroyed, unless it is disabled.
-///
-/// RefChanger is movable, and doing so prolongs the duration of the temporary
-/// assignment. This allows classes to supply their own changer methods.
-///
-/// Naturally, be careful not to extend the life of a RefChanger beyond that of
-/// the reference it modifies. The RefChanger can be disabled by calling
-/// ->keep() or ->revert(). Once disabled, the reference is never accessed
-/// again.
-template<typename X>
-class RevertibleRef : public Revertible {
-public:
-	RevertibleRef(X & ref) : ref(ref), old(ref), enabled(true) {}
-	//
-	~RevertibleRef() { revert(); }
-	//
-	void revert() override { if (enabled) { enabled = false; ref = old; } }
-	//
-	void keep() override { enabled = false; }
-	//
-	X & ref;
-	X const old;
-private:
-	bool enabled;
-};
-
-
-template <typename X> using RefChanger = unique_ptr<RevertibleRef<X>>;
-
-
-/// Saves the value of \param ref in a movable object
-template <typename X> RefChanger<X> make_save(X & ref)
-{
-	return make_unique<RevertibleRef<X>>(ref);
-}
-
-/// Temporarily assign value val to reference ref.
-/// To apply the change conditionnally, one can write:
-///     Changer dummy = (cond) ? make_change(a, b) : Changer();
-template <typename X>
-RefChanger<X> make_change(X & ref, X const val)
-{
-	auto rc = make_save(ref);
-	ref = val;
-	return rc;
-}
-
-
-} // namespace lyx
-
-
-#endif //LYX_REFCHANGER_H
-- 
2.28.0.windows.1



More information about the lyx-devel mailing list