The branch, cleanup/updateMacros, has been updated. - Log -----------------------------------------------------------------
commit 91d82d50bae9e75ff81b5be9002c233974e57610 Author: Richard Kimberly Heck <[email protected]> Date: Mon Nov 9 18:29:26 2020 -0500 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. diff --git a/src/Buffer.cpp b/src/Buffer.cpp index c541be9..f991978 100644 --- a/src/Buffer.cpp +++ b/src/Buffer.cpp @@ -217,6 +217,55 @@ public: /// which maps the macro definition position to the scope and the MacroData. MacroMap macros; + 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_; + }; + MacroTable macro_table; + /// Each child Buffer is listed in this map, together with where /// it is first included in this Buffer. typedef map<Buffer const * const, DocIterator> BufferPositionMap; @@ -1971,7 +2020,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 +3652,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; } @@ -3637,7 +3686,7 @@ MacroData const * Buffer::Impl::getBufferMacro(docstring const & name, while (true) { // do we know something better (i.e. later) already? - if (it->first < bestPos ) + if (mit->first < bestPos ) break; // scope ends behind pos? @@ -3650,7 +3699,7 @@ MacroData const * Buffer::Impl::getBufferMacro(docstring const & name, = it->second.buffer->getMacro(name, false); macro_lock = false; if (data) { - bestPos = it->first; + bestPos = mit->first; bestData = data; break; } @@ -3659,7 +3708,7 @@ MacroData const * Buffer::Impl::getBufferMacro(docstring const & name, // try previous file if there is one if (it == position_to_children.begin()) break; - --it; + --mit; } // return the best macro we have found @@ -3807,10 +3856,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 +3893,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 +3968,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 +3988,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 2dbc16c..c8946d6 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 0d5e4a2..c0e3aa6 100644 --- 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. commit 1a952c3ed4621f698352d95be9bceba035b27a11 Author: Richard Kimberly Heck <[email protected]> Date: Mon Nov 9 17:29:27 2020 -0500 Renaming diff --git a/src/Buffer.cpp b/src/Buffer.cpp index b6b742b..c541be9 100644 --- a/src/Buffer.cpp +++ b/src/Buffer.cpp @@ -3690,7 +3690,7 @@ MacroData const * Buffer::getMacro(docstring const & name, } if (global) { - data = MacroTable::get().getMacro(name); + data = GlobalMacros::get().getMacro(name); if (data != nullptr) return data; } diff --git a/src/mathed/InsetMathMacro.cpp b/src/mathed/InsetMathMacro.cpp index 600562f..c051c8e 100644 --- a/src/mathed/InsetMathMacro.cpp +++ b/src/mathed/InsetMathMacro.cpp @@ -938,7 +938,7 @@ MacroData const * InsetMathMacro::macroBackup() const { if (macro()) return &d->macroBackup_; - if (MacroData const * data = MacroTable::get().getMacro(name())) + if (MacroData const * data = GlobalMacros::get().getMacro(name())) return data; return nullptr; } @@ -957,7 +957,7 @@ void InsetMathMacro::validate(LaTeXFeatures & features) const features.require(d->required_); else if (!d->macro_) { // Update requires for known global macros. - MacroData const * data = MacroTable::get().getMacro(name()); + MacroData const * data = GlobalMacros::get().getMacro(name()); if (data && !data->required().empty()) features.require(data->required()); } diff --git a/src/mathed/InsetMathNest.cpp b/src/mathed/InsetMathNest.cpp index 19aa745..ee4cf77 100644 --- a/src/mathed/InsetMathNest.cpp +++ b/src/mathed/InsetMathNest.cpp @@ -2106,7 +2106,7 @@ MathCompletionList::MathCompletionList(Cursor const & cur) // fill in global macros macros.clear(); - MacroTable::get().getMacroNames(macros, false); + GlobalMacros::get().getMacroNames(macros, false); //lyxerr << "Globals completion macros: "; for (it = macros.begin(); it != macros.end(); ++it) { //lyxerr << "\\" + *it << " "; diff --git a/src/mathed/MacroTable.cpp b/src/mathed/MacroTable.cpp index 2bc427b..e6daac6 100644 --- a/src/mathed/MacroTable.cpp +++ b/src/mathed/MacroTable.cpp @@ -215,22 +215,22 @@ int MacroData::write(odocstream & os, bool overwriteRedefinition) const // ///////////////////////////////////////////////////////////////////// -MacroTable & MacroTable::get() +GlobalMacros & GlobalMacros::get() { - static MacroTable theGlobalMacros; + static GlobalMacros theGlobalMacros; return theGlobalMacros; } -MacroData const * MacroTable::getMacro(docstring const & name) const +MacroData const * GlobalMacros::getMacro(docstring const & name) const { const_iterator it = find(name); return it == end() ? 0 : &it->second; } -MacroTable::iterator -MacroTable::insert(docstring const & name, MacroData const & data) +GlobalMacros::iterator +GlobalMacros::insert(docstring const & name, MacroData const & data) { //lyxerr << "MacroTable::insert: " << to_utf8(name) << endl; iterator it = find(name); @@ -243,8 +243,8 @@ MacroTable::insert(docstring const & name, MacroData const & data) } -MacroTable::iterator -MacroTable::insert(Buffer * buf, docstring const & definition) +GlobalMacros::iterator +GlobalMacros::insert(Buffer * buf, docstring const & definition) { //lyxerr << "MacroTable::insert, def: " << to_utf8(def) << endl; InsetMathMacroTemplate mac(buf); @@ -254,7 +254,7 @@ MacroTable::insert(Buffer * buf, docstring const & definition) } -void MacroTable::getMacroNames(std::set<docstring> & names, bool gethidden) const +void GlobalMacros::getMacroNames(std::set<docstring> & names, bool gethidden) const { for (const_iterator it = begin(); it != end(); ++it) if (gethidden || !it->second.hidden()) @@ -262,7 +262,7 @@ void MacroTable::getMacroNames(std::set<docstring> & names, bool gethidden) cons } -void MacroTable::dump() +void GlobalMacros::dump() { lyxerr << "\n------------------------------------------" << endl; for (const_iterator it = begin(); it != end(); ++it) diff --git a/src/mathed/MacroTable.h b/src/mathed/MacroTable.h index a6996da..0d5e4a2 100644 --- a/src/mathed/MacroTable.h +++ b/src/mathed/MacroTable.h @@ -168,15 +168,15 @@ class MacroSet : public std::set<MacroData const *> {}; * hack to display certain contents nicely. * **/ -class MacroTable : public std::map<docstring, MacroData> +class GlobalMacros : public std::map<docstring, MacroData> { public: /// We are a singleton - MacroTable() = default; - MacroTable(MacroTable const &) = delete; - void operator=(MacroTable const &) = delete; + GlobalMacros() = default; + GlobalMacros(GlobalMacros const &) = delete; + void operator=(GlobalMacros const &) = delete; /// the global list: our unique instance - static MacroTable & get(); + static GlobalMacros & get(); /// Parse full "\\def..." or "\\newcommand..." or ... iterator insert(Buffer * buf, docstring const & definition); /// Insert pre-digested macro definition diff --git a/src/mathed/MathFactory.cpp b/src/mathed/MathFactory.cpp index f956973..49baa5d 100644 --- a/src/mathed/MathFactory.cpp +++ b/src/mathed/MathFactory.cpp @@ -202,7 +202,7 @@ void initSymbols() required = ""; } else htmlname = xmlname = ""; - MacroTable::iterator it = MacroTable::get().insert( + GlobalMacros::iterator it = GlobalMacros::get().insert( 0, from_utf8(macro)); if (!extra.empty() || !htmlname.empty() || !xmlname.empty() || !required.empty()) { MathWordList::iterator wit = theMathWordList.find(it->first); commit 30415c80f7934580ce2e1fdf0b5f84b5faeba9da Author: Richard Kimberly Heck <[email protected]> Date: Mon Nov 9 17:27:57 2020 -0500 Some comments, and some conditional code to dump the macro table. diff --git a/src/Buffer.cpp b/src/Buffer.cpp index 91602da..b6b742b 100644 --- a/src/Buffer.cpp +++ b/src/Buffer.cpp @@ -204,17 +204,21 @@ public: 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; }; typedef map<DocIterator, ScopeMacro> PositionScopeMacroMap; - typedef map<docstring, PositionScopeMacroMap> NamePositionScopeMacroMap; + 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. - NamePositionScopeMacroMap macros; + MacroMap macros; /// Each child Buffer is listed in this map, together with where - /// it is included in this Buffer. + /// it is first included in this Buffer. typedef map<Buffer const * const, DocIterator> BufferPositionMap; BufferPositionMap children_positions; /// We also have a map from the positions where Buffers are included @@ -3599,9 +3603,9 @@ MacroData const * Buffer::Impl::getBufferMacro(docstring const & name, MacroData const * bestData = nullptr; // find macro definitions for name - NamePositionScopeMacroMap::const_iterator nameIt = macros.find(name); + MacroMap::const_iterator nameIt = macros.find(name); if (nameIt != macros.end()) { - // find last definition in front of pos or at pos itself + // find last definition in front of pos PositionScopeMacroMap::const_iterator it = greatest_below(nameIt->second, pos); if (it != nameIt->second.end()) { @@ -3825,9 +3829,22 @@ void Buffer::updateMacros() const { if (d->macro_lock) return; - +#if 0 + LYXERR0("Starting macro dump"); + for (auto const & m : d->macros) { + LYXERR0(m.first); + for (auto const & m1 : m.second) { + LYXERR0(m1.first); + LYXERR0("Scope: " << m1.second.scope); + LYXERR0("Def: " << m1.second.macro.definition()); + LYXERR0(""); + } + } + LYXERR0("Ending macro dump"); + LYXERR0(""); LYXERR(Debug::MACROS, "updateMacro of " << d->filename.onlyFileName()); - +#endif + // start with empty table d->macros.clear(); d->children_positions.clear(); ----------------------------------------------------------------------- Summary of changes: src/Buffer.cpp | 117 +++++++++++++++++++++++++++++++--------- src/Buffer.h | 4 +- src/mathed/InsetMathMacro.cpp | 4 +- src/mathed/InsetMathNest.cpp | 2 +- src/mathed/MacroTable.cpp | 18 +++--- src/mathed/MacroTable.h | 12 ++-- src/mathed/MathFactory.cpp | 2 +- 7 files changed, 111 insertions(+), 48 deletions(-) hooks/post-receive -- Repository for new features -- lyx-cvs mailing list [email protected] http://lists.lyx.org/mailman/listinfo/lyx-cvs
