From: Richard Kimberly Heck Date: Mon, 9 Nov 2020 23:29:26 +0000 (-0500) Subject: Refactor the macro tables in Buffer. X-Git-Url: https://git.lyx.org/gitweb/?a=commitdiff_plain;h=cbbeed61dc5b4e69dd476a9f1d50a86017421b94;p=features.git 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 c541be9ca0..de08db8b5d 100644 --- 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 MacroDefList; + typedef map 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 getMacroNames() const + { + set 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 macro_map_; }; - typedef map PositionScopeMacroMap; - typedef map 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 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 2dbc16ca4f..c8946d625c 100644 --- 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 0d5e4a241b..c0e3aa618b 100644 --- a/src/mathed/MacroTable.h +++ b/src/mathed/MacroTable.h @@ -158,7 +158,7 @@ private: /// class MacroNameSet : public std::set {}; /// -class MacroSet : public std::set {}; +class MacroDataSet : public std::set {}; /// A lookup table of macro definitions.