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

From f106663644174b924f0782ef99aac992bbd8f933 Mon Sep 17 00:00:00 2001
From: Yuriy Skalko <yuriy.ska...@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

-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel

Reply via email to