Refactor the macro tables in Buffer.

The use of maps of maps of structs is beyond confusing. Replace that
with a class that hides at least some of the complexity. There is
probably more that could be done along the same lines.

(Note: This fixes a thinko but is otherwise the same as the previous
commit. So it rewrites history.)

diff --git a/src/Buffer.cpp b/src/Buffer.cpp
index c541be9ca06dc10db54b376c094f91b20a4a7498..de08db8b5d3bae41a51738960ccaafa40a88e5b0 100644 (file)
--- a/src/Buffer.cpp
+++ b/src/Buffer.cpp
@@ -199,23 +199,54 @@ public:
        ///
        mutable TocBackend toc_backend;
 
-       /// macro tables
-       struct ScopeMacro {
-               ScopeMacro() {}
-               ScopeMacro(DocIterator const & s, MacroData const & m)
-                       : scope(s), macro(m) {}
-               // The SCOPE is really just the last position at which the macro
-               // is in force. This will be the end of the file (if there are no
-               // children) unless it is in (say) a note, or an inactive branch,
-               // in which case it will be the end of that construct.
-               DocIterator scope;
-               MacroData macro;
+       class MacroTable {
+       public:
+               struct MacroDefinition {
+                       MacroDefinition() {}
+                       MacroDefinition(DocIterator const & s, MacroData const & m)
+                               : scope(s), macro(m) {}
+                       // The SCOPE is really just the last position at which the macro
+                       // is in force. This will be the end of the file (if there are no
+                       // children) unless it is in (say) a note, or an inactive branch,
+                       // in which case it will be the end of that construct.
+                       DocIterator scope;
+                       MacroData macro;
+               };
+               typedef map<DocIterator, MacroDefinition> MacroDefList;
+               typedef map<docstring, MacroDefList> MacroMap;
+
+               MacroDefList const & getMacroDefinitions(docstring const & name) const
+               {
+                       MacroMap::const_iterator it = macro_map_.find(name);
+                       if (it == macro_map_.end()) {
+                               static MacroDefList dummy;
+                               return dummy;
+                       }
+                       return it->second;
+               }
+
+               set<docstring> getMacroNames() const
+               {
+                       set<docstring> names;
+                       for (auto const & m : macro_map_)
+                               names.insert(m.first);
+                       return names;
+               }
+
+               void clear() { macro_map_.clear(); }
+               
+               void addMacroDefinition(docstring const & name, 
+                                                               DocIterator const & pos,
+                                                               DocIterator const & scope,
+                                                               MacroData const & data) 
+               {
+                       MacroDefList & m = macro_map_[name];
+                       m[pos] = {scope, data};
+               }
+       private:
+               map<docstring, MacroDefList> macro_map_;
        };
-       typedef map<DocIterator, ScopeMacro> PositionScopeMacroMap;
-       typedef map<docstring, PositionScopeMacroMap> MacroMap;
-       /// map from the macro name to the position map,
-       /// which maps the macro definition position to the scope and the MacroData.
-       MacroMap macros;
+       MacroTable macro_table;
 
        /// Each child Buffer is listed in this map, together with where
        /// it is first included in this Buffer.
@@ -1971,7 +2002,7 @@ Buffer::ExportStatus Buffer::writeLaTeXSource(otexstream & os,
 
                // get parent macros (if this buffer has a parent) which will be
                // written at the document begin further down.
-               MacroSet parentMacros;
+               MacroDataSet parentMacros;
                listParentMacros(parentMacros, features);
 
                // Write the preamble
@@ -3603,25 +3634,25 @@ MacroData const * Buffer::Impl::getBufferMacro(docstring const & name,
        MacroData const * bestData = nullptr;
 
        // find macro definitions for name
-       MacroMap::const_iterator nameIt = macros.find(name);
-       if (nameIt != macros.end()) {
+       MacroTable::MacroDefList mdl = macro_table.getMacroDefinitions(name);
+       if (!mdl.empty()) {
                // find last definition in front of pos
-               PositionScopeMacroMap::const_iterator it
-                       = greatest_below(nameIt->second, pos);
-               if (it != nameIt->second.end()) {
+               MacroTable::MacroDefList::const_iterator it = greatest_below(mdl, pos);
+               if (it != mdl.end()) {
                        while (true) {
                                // scope ends behind pos?
                                if (pos < it->second.scope) {
                                        // Looks good, remember this. If there
                                        // is no external macro behind this,
+                                       // (i.e., one in a child file), then
                                        // we found the right one already.
                                        bestPos = it->first;
                                        bestData = &it->second.macro;
                                        break;
                                }
-
+       
                                // try previous macro if there is one
-                               if (it == nameIt->second.begin())
+                               if (it == mdl.begin())
                                        break;
                                --it;
                        }
@@ -3807,10 +3838,8 @@ void Buffer::Impl::updateMacros(DocIterator & it, DocIterator & scope)
                                        break;
        
                                // register macro
-                               // FIXME (Abdel), I don't understand why we pass 'it' here
-                               // instead of 'macroTemplate' defined above... is this correct?
-                               macros[macroTemplate.name()][it] =
-                                       Impl::ScopeMacro(scope, MacroData(owner_, it));
+                               macro_table.addMacroDefinition(macroTemplate.name(), 
+                                               it, scope, MacroData(owner_, it));
                                break;
                        }
                        default:
@@ -3846,7 +3875,7 @@ void Buffer::updateMacros() const
 #endif
        
        // start with empty table
-       d->macros.clear();
+       d->macro_table.clear();
        d->children_positions.clear();
        d->position_to_children.clear();
 
@@ -3921,9 +3950,8 @@ void Buffer::listMacroNames(MacroNameSet & macros) const
 
        d->macro_lock = true;
 
-       // loop over macro names
-       for (auto const & nameit : d->macros)
-               macros.insert(nameit.first);
+       set<docstring> names = d->macro_table.getMacroNames();
+       macros.insert(names.begin(), names.end());
 
        // loop over children
        for (auto const & p : d->children_positions) {
@@ -3942,7 +3970,7 @@ void Buffer::listMacroNames(MacroNameSet & macros) const
 }
 
 
-void Buffer::listParentMacros(MacroSet & macros, LaTeXFeatures & features) const
+void Buffer::listParentMacros(MacroDataSet & macros, LaTeXFeatures & features) const
 {
        Buffer const * const pbuf = d->parent();
        if (!pbuf)

diff --git a/src/Buffer.h b/src/Buffer.h
index 2dbc16ca4f150ffbd664f80d314bda3ce9fa85e9..c8946d625cdd5e2c98b8080415c4783c109d209a 100644 (file)
--- a/src/Buffer.h
+++ b/src/Buffer.h
@@ -45,7 +45,7 @@ class LaTeXFeatures;
 class Language;
 class MacroData;
 class MacroNameSet;
-class MacroSet;
+class MacroDataSet;
 class OutputParams;
 class otexstream;
 class ParagraphList;
@@ -605,7 +605,7 @@ public:
        /// List macro names of this buffer, the parent and the children
        void listMacroNames(MacroNameSet & macros) const;
        /// Collect macros of the parent and its children in front of this buffer.
-       void listParentMacros(MacroSet & macros, LaTeXFeatures & features) const;
+       void listParentMacros(MacroDataSet & macros, LaTeXFeatures & features) const;
 
        /// Return macro defined before pos (or in the master buffer)
        MacroData const * getMacro(docstring const & name, DocIterator const & pos, bool global = true) const;

diff --git a/src/mathed/MacroTable.h b/src/mathed/MacroTable.h
index 0d5e4a241b1937025e520af6b960b860b6787e36..c0e3aa618b58ec9fb077ea0a5513f3fac3f1f29c 100644 (file)
--- a/src/mathed/MacroTable.h
+++ b/src/mathed/MacroTable.h
@@ -158,7 +158,7 @@ private:
 ///
 class MacroNameSet : public std::set<docstring> {};
 ///
-class MacroSet : public std::set<MacroData const *> {};
+class MacroDataSet : public std::set<MacroData const *> {};
 
 
 /// A lookup table of macro definitions.