[LyX features/cleanup/updateMacros] Refactor the macro tables in Buffer.
Richard Kimberly Heck
rikiheck at lyx.org
Mon Nov 9 23:01:04 UTC 2020
The branch, cleanup/updateMacros, has been updated.
- Log -----------------------------------------------------------------
commit 91d82d50bae9e75ff81b5be9002c233974e57610
Author: Richard Kimberly Heck <rikiheck at lyx.org>
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 <rikiheck at lyx.org>
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 <rikiheck at lyx.org>
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
More information about the lyx-cvs
mailing list