The attached patch does as advertised, replacing the shared pointer with a global cache of sorts of the TextClass's used by Buffers---or, more strictly, constructed by BufferParams::makeTextClass(). The action is in TextClass.{h,cpp}.

I've left the typedef in TextClassPtr.h. At the moment, it's kind of silly. But I've left it mostly because it helps to identify where the TextClass's stored in the TextClassBundle are used, and maybe it'd be worth having some sort of strong typedef like the one for BaseClassIndex here.

I need to check whether the textClass_ member of InsetCollapsable is needed now. I think not.

Comments welcome.

Richard

Index: src/insets/InsetCommandParams.cpp
===================================================================
--- src/insets/InsetCommandParams.cpp	(revision 23198)
+++ src/insets/InsetCommandParams.cpp	(working copy)
@@ -30,10 +30,10 @@
 #include "Lexer.h"
 
 #include "support/debug.h"
+#include "support/docstream.h"
 #include "support/ExceptionMessage.h"
 #include "support/gettext.h"
 #include "support/lstrings.h"
-#include "support/docstream.h"
 
 #include <boost/assert.hpp>
 
@@ -42,55 +42,85 @@
 
 namespace lyx {
 
-ParamInfo::ParamData::ParamData(std::string const & s, bool b) :
-	name_(s), optional_(b)
+ParamInfo::ParamData::ParamData(std::string const & s, ParamType t) :
+	name_(s), type_(t)
 {}
 
 
+bool ParamInfo::ParamData::isOptional() const
+{
+	return type_ == ParamInfo::LATEX_OPTIONAL ||
+	       type_ == ParamInfo::LATEX_KV_OPTIONAL;
+}
+
+
+bool ParamInfo::ParamData::isKeyValArg() const
+{
+	return type_ == ParamInfo::LATEX_KV_REQUIRED ||
+	       type_ == ParamInfo::LATEX_KV_OPTIONAL;
+}
+
+#if 0
+//presently unused but perhaps useful
+bool ParamInfo::ParamData::isArgument() const
+{ 
+	return isOptional() || isRequired(); 
+}
+
+
+bool ParamInfo::ParamData::isRequired() const
+{
+	return type_ == ParamInfo::LATEX_REQUIRED ||
+	       type_ == ParamInfo::LATEX_KV_REQUIRED;
+}
+#endif
+
 bool ParamInfo::ParamData::operator==(ParamInfo::ParamData const & rhs) const
 {
-	return name() == rhs.name() && isOptional() == rhs.isOptional();
+	return name() == rhs.name() && type() == rhs.type();
 }
 
 
+// note that this returns FALSE if name corresponds to a `parameter'
+// of type LATEX_KV_*, because these do not really represent parameters
+// but just argument places.
 bool ParamInfo::hasParam(std::string const & name) const
 {
 	const_iterator it = begin();
-	for (; it != end(); ++it) {
+	const_iterator last = end();
+	for (; it != last; ++it) {
 		if (it->name() == name)
-			return true;
+			return !it->isKeyValArg();
 	}
 	return false;
 }
 
 
-void ParamInfo::add(std::string const & name, bool opt)
+void ParamInfo::add(std::string const & name, ParamType type)
 { 
-	info_.push_back(ParamData(name, opt)); 
+	info_.push_back(ParamData(name, type)); 
 }
 
 
 bool ParamInfo::operator==(ParamInfo const & rhs) const
 {
-	// the idea here is to check each ParamData for equality
-	const_iterator itL  = begin();
-	const_iterator itR  = rhs.begin();
-	const_iterator endL = end();
-	const_iterator endR = rhs.end();
-	while (true) {
-		// if they both end together, return true
-		if (itL == endL && itR == endR)
-				return true;
-		// but if one ends before the other, return false
-		if (itL == endL || itR == endR)
-			return false;
-		//check this one for equality
-		if (*itL != *itR)
-			return false;
-		// equal, so check the next one
-		++itL;
-		++itR;
+	if (size() != rhs.size())
+		return false;
+	return equal(begin(), end(), rhs.begin());
+}
+
+
+ParamInfo::ParamData const & 
+	ParamInfo::operator[](std::string const & name) const
+{
+	BOOST_ASSERT(hasParam(name));
+	const_iterator it = begin();
+	const_iterator last = end();
+	for (; it != last; ++it) {
+		if (it->name() == name)
+			return *it;
 	}
+	return *it; // silence warning
 }
 
 
@@ -318,6 +348,66 @@
 }
 
 
+docstring InsetCommandParams::makeKeyValArgument() const
+{
+	odocstringstream os;
+	bool didone = false;
+	ParamInfo::const_iterator it  = info_.begin();
+	ParamInfo::const_iterator end = info_.end();
+	for (; it != end; ++it) {
+		if (!it->isKey())
+			continue;
+		string const & name = it->name();
+		docstring const & data = (*this)[name];
+		if (data.empty())
+			continue;
+		if (didone)
+			os << ",";
+		else 
+			didone = true;
+		os << from_utf8(name) << "=" << data;
+	}
+	return os.str();
+}
+
+
+// returns true if a non-empty optional parameter follows ci
+bool InsetCommandParams::writeEmptyOptional(ParamInfo::const_iterator ci) const
+{
+	if (!ci->isOptional())
+		BOOST_ASSERT(false);
+	++ci; // we want to start with the next one
+	ParamInfo::const_iterator end = info_.end();
+	for (; ci != end; ++ci) {
+		switch (ci->type()) {
+		case ParamInfo::LATEX_KEY:
+		case ParamInfo::LYX_INTERNAL:
+			break;
+
+		case ParamInfo::LATEX_REQUIRED:
+		case ParamInfo::LATEX_KV_REQUIRED:
+			return false;
+
+		case ParamInfo::LATEX_OPTIONAL: {
+			std::string const & name = ci->name();
+			docstring const & data = (*this)[name];
+			if (!data.empty())
+				return true;
+			break;
+		}
+
+		case ParamInfo::LATEX_KV_OPTIONAL: {
+			docstring data = makeKeyValArgument();
+			if (!data.empty())
+				return true;
+			break;
+		}
+		} //end switch
+	}
+	return false;
+}
+
+
 docstring const InsetCommandParams::getCommand() const
 {
 	docstring s = '\\' + from_ascii(cmdName_);
@@ -326,33 +416,45 @@
 	ParamInfo::const_iterator end = info_.end();
 	for (; it != end; ++it) {
 		std::string const & name = it->name();
-		docstring const & data = (*this)[name];
-		if (!it->isOptional()) {
+		switch (it->type()) {
+		case ParamInfo::LATEX_KEY:
+		case ParamInfo::LYX_INTERNAL:
+			break;
+
+		case ParamInfo::LATEX_REQUIRED: {
+			docstring const & data = (*this)[name];
 			s += '{' + data + '}';
 			noparam = false;
-			continue;
+			break;
 		}
-		if (!data.empty()) {
-			s += '[' + data + ']';
+		case ParamInfo::LATEX_KV_REQUIRED: {
+			s += "{" + makeKeyValArgument() + "}";
 			noparam = false;
-			continue;
+			break;
 		}
-		// This param is therefore optional but empty.
-		// But we need to write it anyway if nonempty
-		// optional parameters follow before the next
-		// required parameter.
-		ParamInfo::const_iterator it2 = it;
-		for (++it2; it2 != end; ++it2) {
-			if (!it2->isOptional())
-				break;
-			std::string const & name2 = it2->name();
-			docstring const & data2 = (*this)[name2];
-			if (!data2.empty()) {
-				s += "[]";
+		case ParamInfo::LATEX_OPTIONAL: {
+			docstring const & data = (*this)[name];
+			if (!data.empty()) {
+				s += '[' + data + ']';
 				noparam = false;
-				break;
+			} else if (writeEmptyOptional(it)) {
+					s += "[]";
+					noparam = false;
 			}
+			break;
+		} 
+		case ParamInfo::LATEX_KV_OPTIONAL: {
+			docstring data = makeKeyValArgument();
+			if (!data.empty()) {
+				s += '[' + data + ']';
+				noparam = false;
+			} else if (writeEmptyOptional(it)) {
+					s += "[]";
+					noparam = false;
+				}
+			break;
 		}
+		} //end switch
 	}
 	if (noparam)
 		// Make sure that following stuff does not change the
@@ -362,18 +464,11 @@
 }
 
 
-namespace {
-	//predicate for what follows
-	bool paramIsNonOptional(ParamInfo::ParamData pi)
-	{
-		return !pi.isOptional();
-	}
-}
-
 docstring const InsetCommandParams::getFirstNonOptParam() const
 {
 	ParamInfo::const_iterator it = 
-		find_if(info_.begin(), info_.end(), paramIsNonOptional);
+		find_if(info_.begin(), info_.end(), 
+			not1(mem_fun_ref(&ParamInfo::ParamData::isOptional)));
 	if (it == info_.end())
 		BOOST_ASSERT(false);
 	return (*this)[it->name()];
@@ -383,8 +478,7 @@
 docstring const & InsetCommandParams::operator[](string const & name) const
 {
 	static const docstring dummy; //so we don't return a ref to temporary
-	if (!info_.hasParam(name))
-		BOOST_ASSERT(false);
+	BOOST_ASSERT(info_.hasParam(name));
 	ParamMap::const_iterator data = params_.find(name);
 	if (data == params_.end() || data->second.empty())
 		return dummy;
@@ -394,8 +488,8 @@
 
 docstring & InsetCommandParams::operator[](string const & name)
 {
-	if (!info_.hasParam(name))
-		BOOST_ASSERT(false);
+	BOOST_ASSERT(info_.hasParam(name));
+	ParamInfo::ParamData const & pd = info_[name];
 	return params_[name];
 }
 
Index: src/insets/InsetCommandParams.h
===================================================================
--- src/insets/InsetCommandParams.h	(revision 23198)
+++ src/insets/InsetCommandParams.h	(working copy)
@@ -29,17 +29,47 @@
 
 class ParamInfo {
 public:
+	/// Types of parameters
+	/// WARNING: LATEX_KV_* `parameters' aren't really parameters at all
+	/// but merely markers for where the keyval-type parameters should
+	/// appear in the LaTeX output. ParamInfo::hasParam(name) therefore 
+	/// returns FALSE if the corresponding `parameter' is of type
+	/// LATEX_KV_*.
+	/// It is assumed here that there is exactly one argument that accepts
+	/// the key=value pairs.
+	enum ParamType {
+		LATEX_OPTIONAL,    /// normal optional argument
+		LATEX_REQUIRED,    /// normal required argument
+		LATEX_KV_OPTIONAL, /// optional argument that uses keyval
+		LATEX_KV_REQUIRED, /// required argument that uses keyval
+		LATEX_KEY,         /// a key to be used with keyval argument
+		LYX_INTERNAL       /// a parameter used internally by LyX
+	};
 	///
 	class ParamData {
 	// No parameter may be named "preview", because that is a required
 	// flag for all commands.
 	public:
 		///
-		ParamData(std::string const &, bool);
+		ParamData(std::string const &, ParamType);
 		///
 		std::string name() const { return name_; }
 		///
-		bool isOptional() const { return optional_; }
+		ParamType type() const { return type_; }
+		/// whether this is a key for use with keyval
+		bool isKey() const
+			{ return type_ == LATEX_KEY; }
+		/// whether this is an optional LaTeX argument
+		inline bool isOptional() const;
+		/// whether this is a keyval argument
+		inline bool isKeyValArg() const;
+#if 0
+		//presently unused but perhaps useful
+		/// whether this is a required LaTeX argument
+		inline bool isRequired() const;
+		/// whether this is a LaTeX argument
+		inline bool isArgument() const;
+#endif
 		///
 		bool operator==(ParamData const &) const;
 		/// 
@@ -49,11 +79,11 @@
 		///
 		std::string name_;
 		///
-		bool optional_;
+		ParamType type_;
 	};
 
 	/// adds a new parameter
-	void add(std::string const & name, bool optional);
+	void add(std::string const & name, ParamType type);
 	///
 	bool empty() const { return info_.empty(); }
 	///
@@ -61,12 +91,14 @@
 	///
 	typedef std::vector<ParamData>::const_iterator const_iterator;
 	///
-	const_iterator begin() const { return info_.begin(); }
+	const_iterator const begin() const { return info_.begin(); }
 	///
-	const_iterator end() const { return info_.end(); }
+	const_iterator const end() const { return info_.end(); }
 	///
 	bool hasParam(std::string const & name) const;
 	///
+	ParamData const & operator[](std::string const & name) const;
+	///
 	bool operator==(ParamInfo const &) const;
 private:
 	///
@@ -103,8 +135,12 @@
 	/// ways that make removal hard.
 	docstring const getFirstNonOptParam() const;
 	/// get parameter \p name
+	/// WARNING: You cannot access LATEX_KV_* arguments in this way.
+	/// LyX will assert if you attempt to do so.
 	docstring const & operator[](std::string const & name) const;
 	/// set parameter \p name
+	/// WARNING: You cannot access LATEX_KV_* arguments in this way.
+	/// LyX will assert if you attempt to do so.
 	docstring & operator[](std::string const & name);
 	///
 	bool preview() const { return preview_; }
@@ -128,6 +164,10 @@
 	static bool isCompatibleCommand(InsetCode code, std::string const & s);
 	///
 	std::string getDefaultCmd(InsetCode);
+	///
+	docstring makeKeyValArgument() const;
+	/// checks whether we need to write an empty optional parameter
+	bool writeEmptyOptional(ParamInfo::const_iterator ci) const;
 	/// Description of all command properties
 	ParamInfo info_;
 	/// what kind of inset we're the parameters for
@@ -135,6 +175,9 @@
 	/// The name of this command as it appears in .lyx and .tex files
 	std::string cmdName_;
 	///
+	// if we need to allow more than one value for a parameter, this
+	// could be made a multimap. it may be that the only thing that
+	// would then need changing is operator[].
 	typedef std::map<std::string, docstring> ParamMap;
 	/// The parameters, by name.
 	ParamMap params_;
Index: src/insets/InsetBibitem.cpp
===================================================================
--- src/insets/InsetBibitem.cpp	(revision 23198)
+++ src/insets/InsetBibitem.cpp	(working copy)
@@ -53,8 +53,8 @@
 {
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("label", true);
-		param_info_.add("key", false);
+		param_info_.add("label", ParamInfo::LATEX_OPTIONAL);
+		param_info_.add("key", ParamInfo::LATEX_REQUIRED);
 	}
 	return param_info_;
 }
Index: src/insets/InsetBibtex.cpp
===================================================================
--- src/insets/InsetBibtex.cpp	(revision 23198)
+++ src/insets/InsetBibtex.cpp	(working copy)
@@ -57,10 +57,10 @@
 {
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("options", true);
-		param_info_.add("btprint", true);
-		param_info_.add("bibfiles", false);
-		param_info_.add("embed", false);
+		param_info_.add("btprint", ParamInfo::LATEX_OPTIONAL);
+		param_info_.add("bibfiles", ParamInfo::LATEX_REQUIRED);
+		param_info_.add("embed", ParamInfo::LYX_INTERNAL);
+		param_info_.add("options", ParamInfo::LYX_INTERNAL);
 	}
 	return param_info_;
 }
Index: src/insets/InsetCitation.cpp
===================================================================
--- src/insets/InsetCitation.cpp	(revision 23198)
+++ src/insets/InsetCitation.cpp	(working copy)
@@ -390,9 +390,9 @@
 	// we have to allow both here. InsetCitation takes care that
 	// LaTeX output is nevertheless correct.
 	if (param_info_.empty()) {
-		param_info_.add("after", true);
-		param_info_.add("before", true);
-		param_info_.add("key", false);
+		param_info_.add("after", ParamInfo::LATEX_OPTIONAL);
+		param_info_.add("before", ParamInfo::LATEX_OPTIONAL);
+		param_info_.add("key", ParamInfo::LATEX_REQUIRED);
 	}
 	return param_info_;
 }
Index: src/insets/InsetFloatList.cpp
===================================================================
--- src/insets/InsetFloatList.cpp	(revision 23198)
+++ src/insets/InsetFloatList.cpp	(working copy)
@@ -52,7 +52,7 @@
 {
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("type", false);
+		param_info_.add("type", ParamInfo::LATEX_REQUIRED);
 	}
 	return param_info_;
 }
Index: src/insets/InsetHyperlink.cpp
===================================================================
--- src/insets/InsetHyperlink.cpp	(revision 23198)
+++ src/insets/InsetHyperlink.cpp	(working copy)
@@ -37,9 +37,9 @@
 {
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("name", true);
-		param_info_.add("target", false);
-		param_info_.add("type", false);
+		param_info_.add("name", ParamInfo::LATEX_OPTIONAL);
+		param_info_.add("target", ParamInfo::LATEX_REQUIRED);
+		param_info_.add("type", ParamInfo::LATEX_REQUIRED);
 	}
 	return param_info_;
 }
Index: src/insets/InsetInclude.cpp
===================================================================
--- src/insets/InsetInclude.cpp	(revision 23198)
+++ src/insets/InsetInclude.cpp	(working copy)
@@ -173,9 +173,9 @@
 	// In the other cases, this second parameter should just be empty.
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("filename", false);
-		param_info_.add("embed", false);
-		param_info_.add("lstparams", true);
+		param_info_.add("filename", ParamInfo::LATEX_REQUIRED);
+		param_info_.add("lstparams", ParamInfo::LATEX_OPTIONAL);
+		param_info_.add("embed", ParamInfo::LYX_INTERNAL);
 	}
 	return param_info_;
 }
Index: src/insets/InsetIndex.cpp
===================================================================
--- src/insets/InsetIndex.cpp	(revision 23198)
+++ src/insets/InsetIndex.cpp	(working copy)
@@ -84,7 +84,7 @@
 {
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("name", false);
+		param_info_.add("name", ParamInfo::LATEX_REQUIRED);
 	}
 	return param_info_;
 }
Index: src/insets/InsetLabel.cpp
===================================================================
--- src/insets/InsetLabel.cpp	(revision 23198)
+++ src/insets/InsetLabel.cpp	(working copy)
@@ -37,7 +37,7 @@
 {
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("name", false);
+		param_info_.add("name", ParamInfo::LATEX_REQUIRED);
 	}
 	return param_info_;
 }
Index: src/insets/InsetNomencl.cpp
===================================================================
--- src/insets/InsetNomencl.cpp	(revision 23198)
+++ src/insets/InsetNomencl.cpp	(working copy)
@@ -39,9 +39,9 @@
 {
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("prefix", true);
-		param_info_.add("symbol", false);
-		param_info_.add("description", false);
+		param_info_.add("prefix", ParamInfo::LATEX_OPTIONAL);
+		param_info_.add("symbol", ParamInfo::LATEX_REQUIRED);
+		param_info_.add("description", ParamInfo::LATEX_REQUIRED);
 	}
 	return param_info_;
 }
@@ -92,7 +92,7 @@
 {
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("labelwidth", true);
+		param_info_.add("labelwidth", ParamInfo::LATEX_REQUIRED);
 	}
 	return param_info_;
 }
Index: src/insets/InsetRef.cpp
===================================================================
--- src/insets/InsetRef.cpp	(revision 23198)
+++ src/insets/InsetRef.cpp	(working copy)
@@ -56,8 +56,8 @@
 {
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("name", true);
-		param_info_.add("reference", false);
+		param_info_.add("name", ParamInfo::LATEX_OPTIONAL);
+		param_info_.add("reference", ParamInfo::LATEX_REQUIRED);
 	}
 	return param_info_;
 }
Index: src/insets/InsetTOC.cpp
===================================================================
--- src/insets/InsetTOC.cpp	(revision 23198)
+++ src/insets/InsetTOC.cpp	(working copy)
@@ -36,7 +36,7 @@
 {
 	static ParamInfo param_info_;
 	if (param_info_.empty()) {
-		param_info_.add("type", false);
+		param_info_.add("type", ParamInfo::LATEX_REQUIRED);
 	}
 	return param_info_;
 }

Reply via email to