[LyX features/cleanup/updateMacros] Refactor the macro tables in Buffer.

Richard Kimberly Heck rikiheck at lyx.org
Mon Nov 9 23:06:26 UTC 2020


The branch, cleanup/updateMacros, has been updated.
  discards  91d82d50bae9e75ff81b5be9002c233974e57610 (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (91d82d50bae9e75ff81b5be9002c233974e57610)
            \
             N -- N -- N (7ef7662c68ef8bc8b7943d48898e6e576c490637)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

- Log -----------------------------------------------------------------

commit 7ef7662c68ef8bc8b7943d48898e6e576c490637
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..995e3a4 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<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 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.

-----------------------------------------------------------------------

Summary of changes:
 src/Buffer.cpp |   24 +++---------------------
 1 files changed, 3 insertions(+), 21 deletions(-)


hooks/post-receive
-- 
Repository for new features


More information about the lyx-cvs mailing list