commit 0f15dcc69895c190811d98ad5f0b9bce887f53f6
Author: Jean-Marc Lasgouttes <[email protected]>
Date:   Mon Nov 28 13:13:36 2016 +0100

    Rewrite handling of macro nesting in math rows
    
    Macro nesting is now recorded into the macro inset itself. This allows
    the ArgumentProxy inset to determine whether it is editable or not by
    looking at its macro.
    
    Remove code in the metrics and draw methods of ArgumentProxy: this
    code is AFAICS not active anymore, since arguments are linearized into
    math rows.
    
    Use Changer idiom to change locally the values of 
MecticsInfo::base.macro_nesting.
---
 src/Buffer.cpp           |    2 +-
 src/mathed/InsetMath.cpp |    4 +-
 src/mathed/MathData.cpp  |    7 ++-
 src/mathed/MathData.h    |    2 +-
 src/mathed/MathMacro.cpp |  105 +++++++++++++++------------------------------
 src/mathed/MathMacro.h   |    5 ++-
 src/mathed/MathRow.cpp   |   38 ++++++++++-------
 src/mathed/MathRow.h     |    4 +-
 8 files changed, 71 insertions(+), 96 deletions(-)

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index 4facf09..3652e7a 100644
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -3556,7 +3556,7 @@ void Buffer::updateMacroInstances(UpdateType utype) const
                MacroContext mc = MacroContext(this, it);
                for (DocIterator::idx_type i = 0; i < n; ++i) {
                        MathData & data = minset->cell(i);
-                       data.updateMacros(0, mc, utype);
+                       data.updateMacros(0, mc, utype, 0);
                }
        }
 }
diff --git a/src/mathed/InsetMath.cpp b/src/mathed/InsetMath.cpp
index cbf33cd..e22adb6 100644
--- a/src/mathed/InsetMath.cpp
+++ b/src/mathed/InsetMath.cpp
@@ -58,9 +58,9 @@ MathClass InsetMath::mathClass() const
 }
 
 
-bool InsetMath::addToMathRow(MathRow & mrow, MetricsInfo & mi) const
+bool InsetMath::addToMathRow(MathRow & mrow, MetricsInfo & ) const
 {
-       MathRow::Element e(MathRow::INSET, mi);
+       MathRow::Element e(MathRow::INSET);
        e.inset = this;
        e.mclass = mathClass();
        mrow.push_back(e);
diff --git a/src/mathed/MathData.cpp b/src/mathed/MathData.cpp
index 2a36e55..9acefe6 100644
--- a/src/mathed/MathData.cpp
+++ b/src/mathed/MathData.cpp
@@ -222,7 +222,8 @@ bool MathData::addToMathRow(MathRow & mrow, MetricsInfo & 
mi) const
        BufferView * bv = mi.base.bv;
        MathData * ar = const_cast<MathData*>(this);
        ar->updateMacros(&bv->cursor(), mi.macrocontext,
-                        InternalUpdate);
+                        InternalUpdate, mi.base.macro_nesting);
+
 
        // FIXME: for completion, try to insert the relevant data in the
        // mathrow (like is done for text rows). We could add a pair of
@@ -341,7 +342,7 @@ void MathData::updateBuffer(ParIterator const & it, 
UpdateType utype)
 
 
 void MathData::updateMacros(Cursor * cur, MacroContext const & mc,
-               UpdateType utype)
+               UpdateType utype, int nesting)
 {
        // If we are editing a macro, we cannot update it immediately,
        // otherwise wrong undo steps will be recorded (bug 6208).
@@ -430,7 +431,7 @@ void MathData::updateMacros(Cursor * cur, MacroContext 
const & mc,
                if (inset->asScriptInset())
                        inset = inset->asScriptInset()->nuc()[0].nucleus();
                LASSERT(inset->asMacro(), continue);
-               inset->asMacro()->updateRepresentation(cur, mc, utype);
+               inset->asMacro()->updateRepresentation(cur, mc, utype, nesting 
+ 1);
        }
 }
 
diff --git a/src/mathed/MathData.h b/src/mathed/MathData.h
index 9eae466..058d86e 100644
--- a/src/mathed/MathData.h
+++ b/src/mathed/MathData.h
@@ -171,7 +171,7 @@ public:
 
        /// attach/detach arguments to macros, updating the cur to
        /// stay visually at the same position (cur==0 is allowed)
-       void updateMacros(Cursor * cur, MacroContext const & mc, UpdateType);
+       void updateMacros(Cursor * cur, MacroContext const & mc, UpdateType, 
int nesting);
        ///
        void updateBuffer(ParIterator const &, UpdateType);
 
diff --git a/src/mathed/MathMacro.cpp b/src/mathed/MathMacro.cpp
index f6791bb..a2a597f 100644
--- a/src/mathed/MathMacro.cpp
+++ b/src/mathed/MathMacro.cpp
@@ -38,6 +38,7 @@
 #include "support/gettext.h"
 #include "support/lassert.h"
 #include "support/lstrings.h"
+#include "support/RefChanger.h"
 #include "support/textutils.h"
 
 #include <ostream>
@@ -71,11 +72,11 @@ public:
        bool addToMathRow(MathRow & mrow, MetricsInfo & mi) const
        {
                // macro arguments are in macros
-               LATTEST(mi.base.macro_nesting > 0);
-               if (mi.base.macro_nesting == 1)
-                       mi.base.macro_nesting = 0;
+               LATTEST(mathMacro_->nesting() > 0);
+               /// The macro nesting can change display of insets. Change it 
locally.
+               Changer chg = make_change(mi.base.macro_nesting, 
mathMacro_->nesting());
 
-               MathRow::Element e_beg(MathRow::BEG_ARG, mi);
+               MathRow::Element e_beg(MathRow::BEG_ARG);
                e_beg.macro = mathMacro_;
                e_beg.ar = &mathMacro_->cell(idx_);
                mrow.push_back(e_beg);
@@ -92,17 +93,14 @@ public:
 
                // if there was no contents, and the contents is editable,
                // then we insert a box instead.
-               if (!has_contents && mi.base.macro_nesting == 0) {
-                       MathRow::Element e(MathRow::BOX, mi);
+               if (!has_contents && mathMacro_->nesting() == 1) {
+                       MathRow::Element e(MathRow::BOX);
                        e.color = Color_mathline;
                        mrow.push_back(e);
                        has_contents = true;
                }
 
-               if (mi.base.macro_nesting == 0)
-                       mi.base.macro_nesting = 1;
-
-               MathRow::Element e_end(MathRow::END_ARG, mi);
+               MathRow::Element e_end(MathRow::END_ARG);
                e_end.macro = mathMacro_;
                e_end.ar = &mathMacro_->cell(idx_);
 
@@ -111,22 +109,9 @@ public:
                return has_contents;
        }
        ///
-       void metrics(MetricsInfo & mi, Dimension & dim) const {
-               // macro arguments are in macros
-               LATTEST(mi.base.macro_nesting > 0);
-               if (mi.base.macro_nesting == 1)
-                       mi.base.macro_nesting = 0;
-
-               mathMacro_->macro()->unlock();
-               mathMacro_->cell(idx_).metrics(mi, dim);
-
-               if (!mathMacro_->editMetrics(mi.base.bv)
-                   && mathMacro_->cell(idx_).empty())
-                       def_.metrics(mi, dim);
-
-               mathMacro_->macro()->lock();
-               if (mi.base.macro_nesting == 0)
-                       mi.base.macro_nesting = 1;
+       void metrics(MetricsInfo &, Dimension &) const {
+               // This should never be invoked, since ArgumentProxy insets are 
linearized
+               LATTEST(false);
        }
        // write(), normalize(), infoize() and infoize2() are not needed since
        // MathMacro uses the definition and not the expanded cells.
@@ -143,31 +128,9 @@ public:
        ///
        void octave(OctaveStream & os) const { os << mathMacro_->cell(idx_); }
        ///
-       void draw(PainterInfo & pi, int x, int y) const {
-               LATTEST(pi.base.macro_nesting > 0);
-               if (pi.base.macro_nesting == 1)
-                       pi.base.macro_nesting = 0;
-
-               if (mathMacro_->editMetrics(pi.base.bv)) {
-                       // The only way a ArgumentProxy can appear is in a cell 
of the
-                       // MathMacro. Moreover the cells are only drawn in the 
DISPLAY_FOLDED
-                       // mode and then, if the macro is edited the monochrome
-                       // mode is entered by the MathMacro before calling the 
cells' draw
-                       // method. Then eventually this code is reached and the 
proxy leaves
-                       // monochrome mode temporarely. Hence, if it is not in 
monochrome
-                       // here (and the assert triggers in 
pain.leaveMonochromeMode())
-                       // it's a bug.
-                       pi.pain.leaveMonochromeMode();
-                       mathMacro_->cell(idx_).draw(pi, x, y);
-                       pi.pain.enterMonochromeMode(Color_mathbg, 
Color_mathmacroblend);
-               } else if (mathMacro_->cell(idx_).empty()) {
-                       mathMacro_->cell(idx_).setXY(*pi.base.bv, x, y);
-                       def_.draw(pi, x, y);
-               } else
-                       mathMacro_->cell(idx_).draw(pi, x, y);
-
-               if (pi.base.macro_nesting == 0)
-                       pi.base.macro_nesting = 1;
+       void draw(PainterInfo &, int, int) const {
+               // This should never be invoked, since ArgumentProxy insets are 
linearized
+               LATTEST(false);
        }
        ///
        size_t idx() const { return idx_; }
@@ -244,6 +207,8 @@ public:
        bool isUpdating_;
        /// maximal number of arguments the macro is greedy for
        size_t appetite_;
+       /// Level of nesting in macros (including this one)
+       int nesting_;
 };
 
 
@@ -329,16 +294,17 @@ bool MathMacro::addToMathRow(MathRow & mrow, MetricsInfo 
& mi) const
        // This is the same as what is done in metrics().
        d->editing_[mi.base.bv] = editMode(mi.base.bv);
 
+       /// The macro nesting can change display of insets. Change it locally.
+       Changer chg = make_change(mi.base.macro_nesting, d->nesting_);
+
        if (displayMode() != MathMacro::DISPLAY_NORMAL
            || d->editing_[mi.base.bv])
                return InsetMath::addToMathRow(mrow, mi);
 
-       MathRow::Element e_beg(MathRow::BEG_MACRO, mi);
+       MathRow::Element e_beg(MathRow::BEG_MACRO);
        e_beg.macro = this;
        mrow.push_back(e_beg);
 
-       ++mi.base.macro_nesting;
-
        d->macro_->lock();
        bool has_contents = d->expanded_.addToMathRow(mrow, mi);
        d->macro_->unlock();
@@ -346,15 +312,13 @@ bool MathMacro::addToMathRow(MathRow & mrow, MetricsInfo 
& mi) const
        // if there was no contents and the array is editable, then we
        // insert a grey box instead.
        if (!has_contents && mi.base.macro_nesting == 1) {
-               MathRow::Element e(MathRow::BOX, mi);
+               MathRow::Element e(MathRow::BOX);
                e.color = Color_mathmacroblend;
                mrow.push_back(e);
                has_contents = true;
        }
 
-       --mi.base.macro_nesting;
-
-       MathRow::Element e_end(MathRow::END_MACRO, mi);
+       MathRow::Element e_end(MathRow::END_MACRO);
        e_end.macro = this;
        mrow.push_back(e_end);
 
@@ -407,6 +371,12 @@ docstring MathMacro::macroName() const
 }
 
 
+int MathMacro::nesting() const
+{
+       return d->nesting_;
+}
+
+
 void MathMacro::cursorPos(BufferView const & bv,
                CursorSlice const & sl, bool boundary, int & x, int & y) const
 {
@@ -454,8 +424,8 @@ bool MathMacro::editMetrics(BufferView const * bv) const
 
 void MathMacro::metrics(MetricsInfo & mi, Dimension & dim) const
 {
-       // the macro contents is not editable (except the arguments)
-       ++mi.base.macro_nesting;
+       /// The macro nesting can change display of insets. Change it locally.
+       Changer chg = make_change(mi.base.macro_nesting, d->nesting_);
 
        // set edit mode for which we will have calculated metrics. But only
        d->editing_[mi.base.bv] = editMode(mi.base.bv);
@@ -551,9 +521,6 @@ void MathMacro::metrics(MetricsInfo & mi, Dimension & dim) 
const
                        dim.des += 2;
                }
        }
-
-       // restore macro nesting
-       --mi.base.macro_nesting;
 }
 
 
@@ -600,7 +567,7 @@ private:
 
 
 void MathMacro::updateRepresentation(Cursor * cur, MacroContext const & mc,
-               UpdateType utype)
+       UpdateType utype, int nesting)
 {
        // block recursive calls (bug 8999)
        if (d->isUpdating_)
@@ -612,6 +579,9 @@ void MathMacro::updateRepresentation(Cursor * cur, 
MacroContext const & mc,
        if (d->macro_ == 0)
                return;
 
+       // remember nesting level of this macro
+       d->nesting_ = nesting;
+
        // update requires
        d->requires_ = d->macro_->requires();
 
@@ -643,7 +613,7 @@ void MathMacro::updateRepresentation(Cursor * cur, 
MacroContext const & mc,
        // protected by UpdateLocker.
        if (d->macro_->expand(values, d->expanded_)) {
                if (utype == OutputUpdate && !d->expanded_.empty())
-                       d->expanded_.updateMacros(cur, mc, utype);
+                       d->expanded_.updateMacros(cur, mc, utype, nesting);
        }
        // get definition for list edit mode
        docstring const & display = d->macro_->display();
@@ -722,9 +692,6 @@ void MathMacro::draw(PainterInfo & pi, int x, int y) const
                                  dim.height() - 2, Color_mathmacroframe);
                drawMarkers2(pi, expx, expy);
        } else {
-               // the macro contents is not editable (except the arguments)
-               ++pi.base.macro_nesting;
-
                bool drawBox = lyxrc.macro_edit_style == 
LyXRC::MACRO_EDIT_INLINE_BOX;
                Changer dummy = (currentMode() == TEXT_MODE)
                        ? pi.base.font.changeShape(UP_SHAPE)
@@ -759,8 +726,6 @@ void MathMacro::draw(PainterInfo & pi, int x, int y) const
                } else
                        d->expanded_.draw(pi, expx, expy);
 
-               --pi.base.macro_nesting;
-
                if (!drawBox)
                        drawMarkers(pi, x, y);
 
diff --git a/src/mathed/MathMacro.h b/src/mathed/MathMacro.h
index 4eb93ba..157e6e6 100644
--- a/src/mathed/MathMacro.h
+++ b/src/mathed/MathMacro.h
@@ -126,6 +126,8 @@ public:
        MacroData const * macro() const;
        ///
        docstring macroName() const;
+       /// Level of nesting in macros (including this one)
+       int nesting() const;
        ///
        bool validName() const;
        ///
@@ -152,7 +154,8 @@ protected:
        /// update macro definition
        void updateMacro(MacroContext const & mc);
        /// check if macro definition changed, argument changed etc. and adapt
-       void updateRepresentation(Cursor * cur, MacroContext const & mc, 
UpdateType);
+       void updateRepresentation(Cursor * cur, MacroContext const & mc,
+                                 UpdateType, int nesting);
        /// empty macro, put arguments into args, possibly strip 
arity-attachedArgsNum_ empty ones.
        /// Includes the optional arguments.
        void detachArguments(std::vector<MathData> & args, bool strip);
diff --git a/src/mathed/MathRow.cpp b/src/mathed/MathRow.cpp
index 3dbfda6..b8132fa 100644
--- a/src/mathed/MathRow.cpp
+++ b/src/mathed/MathRow.cpp
@@ -36,17 +36,16 @@ using namespace std;
 namespace lyx {
 
 
-MathRow::Element::Element(Type t, MetricsInfo &mi)
-       : type(t), macro_nesting(mi.base.macro_nesting),
-         inset(0), mclass(MC_ORD), before(0), after(0), compl_unique_to(0),
-         macro(0), color(Color_red)
+MathRow::Element::Element(Type t)
+       : type(t), inset(0), mclass(MC_ORD), before(0), after(0),
+         compl_unique_to(0), macro(0), color(Color_red)
 {}
 
 
 MathRow::MathRow(MetricsInfo & mi, MathData const * ar)
 {
        // First there is a dummy element of type "open"
-       push_back(Element(BEGIN, mi));
+       push_back(Element(BEGIN));
        back().mclass = MC_OPEN;
 
        // Then insert the MathData argument
@@ -55,13 +54,13 @@ MathRow::MathRow(MetricsInfo & mi, MathData const * ar)
        // empty arrays are visible when they are editable
        // we reserve the necessary space anyway (even if nothing gets drawn)
        if (!has_contents) {
-               Element e(BOX, mi);
-               e.color = Color_mathline;
+               Element e(BOX);
+               e.color = mi.base.macro_nesting == 0 ? Color_mathline : 
Color_none;
                push_back(e);
        }
 
        // Finally there is a dummy element of type "close"
-       push_back(Element(END, mi));
+       push_back(Element(END));
        back().mclass = MC_CLOSE;
 
        /* Do spacing only in math mode. This test is a bit clumsy,
@@ -127,10 +126,13 @@ void MathRow::metrics(MetricsInfo & mi, Dimension & dim) 
const
        // arguments, it is necessary to keep track of them.
        map<MathMacro const *, Dimension> dim_macros;
        map<MathData const *, Dimension> dim_arrays;
+       // this vector remembers the stack of macro nesting values
+       vector<int> macro_nesting;
+       macro_nesting.push_back(mi.base.macro_nesting);
        CoordCache & coords = mi.base.bv->coordCache();
        for (Element const & e : elements_) {
+               mi.base.macro_nesting = macro_nesting.back();
                Dimension d;
-               mi.base.macro_nesting = e.macro_nesting;
                switch (e.type) {
                case BEGIN:
                case END:
@@ -141,12 +143,14 @@ void MathRow::metrics(MetricsInfo & mi, Dimension & dim) 
const
                        coords.insets().add(e.inset, d);
                        break;
                case BEG_MACRO:
+                       macro_nesting.push_back(e.macro->nesting());
                        e.macro->macro()->lock();
                        // Add a macro to current list
                        dim_macros[e.macro] = Dimension();
                        break;
                case END_MACRO:
                        LATTEST(dim_macros.find(e.macro) != dim_macros.end());
+                       macro_nesting.pop_back();
                        e.macro->macro()->unlock();
                        // Cache the dimension of the macro and remove it from
                        // tracking map.
@@ -155,14 +159,18 @@ void MathRow::metrics(MetricsInfo & mi, Dimension & dim) 
const
                        break;
                        // This is basically like macros
                case BEG_ARG:
-                       if (e.macro)
+                       if (e.macro) {
+                               macro_nesting.push_back(e.macro->nesting());
                                e.macro->macro()->unlock();
+                       }
                        dim_arrays[e.ar] = Dimension();
                        break;
                case END_ARG:
                        LATTEST(dim_arrays.find(e.ar) != dim_arrays.end());
-                       if (e.macro)
+                       if (e.macro) {
+                               macro_nesting.pop_back();
                                e.macro->macro()->lock();
+                       }
                        coords.arrays().add(e.ar, dim_arrays[e.ar]);
                        dim_arrays.erase(e.ar);
                        break;
@@ -195,7 +203,6 @@ void MathRow::draw(PainterInfo & pi, int x, int const y) 
const
 {
        CoordCache & coords = pi.base.bv->coordCache();
        for (Element const & e : elements_) {
-               pi.base.macro_nesting = e.macro_nesting;
                switch (e.type) {
                case INSET: {
                        // This is hackish: the math inset does not know that 
space
@@ -229,9 +236,9 @@ void MathRow::draw(PainterInfo & pi, int x, int const y) 
const
                case BOX: {
                        Dimension const d = 
theFontMetrics(pi.base.font).dimension('I');
                        // the box is not visible in non-editable context 
(except for grey macro boxes).
-                       if (e.macro_nesting == 0 || e.color == 
Color_mathmacroblend)
+                       if (e.color != Color_none)
                                pi.pain.rectangle(x + e.before, y - d.ascent(),
-                                                                 d.width(), 
d.height(), e.color);
+                                                 d.width(), d.height(), 
e.color);
                        x += d.wid;
                        break;
                }
@@ -290,7 +297,8 @@ ostream & operator<<(ostream & os, MathRow::Element const & 
e)
                   << "-" << e.after << ">";
                break;
        case MathRow::BEG_MACRO:
-               os << "\\" << to_utf8(e.macro->name()) << "[";
+               os << "\\" << to_utf8(e.macro->name())
+                  << "^" << e.macro->nesting() << "[";
                break;
        case MathRow::END_MACRO:
                os << "]";
diff --git a/src/mathed/MathRow.h b/src/mathed/MathRow.h
index 3d31e1c..95786b2 100644
--- a/src/mathed/MathRow.h
+++ b/src/mathed/MathRow.h
@@ -60,12 +60,10 @@ public:
        struct Element
        {
                ///
-               Element(Type t, MetricsInfo & mi);
+               Element(Type t);
 
                /// Classifies the contents of the object
                Type type;
-               /// count wether the current mathdata is nested in macro(s)
-               int macro_nesting;
 
                /// When type is INSET
                /// the math inset

Reply via email to