tex2lyx: improve module support

The current heuristics only considered modules with styles that defined
a searched command in their preamble, and only for commands/environments
that were defined in the document's preamble. This limited the module
support drastically.

The new heuristics also checks for commands coming from packages. If the
command is not (re-)defined in the document preamble, it checks modules
that provide a style with a matching LaTeXName, checks for their
requirements and matches those with the packages loaded by the document.

If no module provides a searched style, but we found modules that load
packages that are loaded in the imported tex file, and if those packages
are not auto-loaded by LyX anyway, we also load this module.

fixes: #11259, part of #8229
(cherry picked from commit 0b54650f0e)

src/tex2lyx/Preamble.cpp

@@ -373,6 +373,12 @@ bool Preamble::isPackageUsed(string const & package) const
 }
 
 
+bool Preamble::isPackageAutoLoaded(string const & package) const
+{
+	return auto_packages.find(package) != auto_packages.end();
+}
+
+
 vector<string> Preamble::getPackageOptions(string const & package) const
 {
 	map<string, vector<string> >::const_iterator it = used_packages.find(package);
@@ -390,6 +396,10 @@ void Preamble::registerAutomaticallyLoadedPackage(std::string const & package)
 
 void Preamble::addModule(string const & module)
 {
+	for (auto const & m : used_modules) {
+		if (m == module)
+			return;
+	}		
 	used_modules.push_back(module);
 }
 
@@ -901,8 +911,10 @@ void Preamble::handle_package(Parser &p, string const & name,
 	else if (name == "amsmath" || name == "amssymb" || name == "cancel" ||
 	         name == "esint" || name == "mhchem" || name == "mathdots" ||
 	         name == "mathtools" || name == "stackrel" ||
-		 name == "stmaryrd" || name == "undertilde")
+		 name == "stmaryrd" || name == "undertilde") {
 		h_use_packages[name] = "2";
+		registerAutomaticallyLoadedPackage(name);
+	}
 
 	else if (name == "babel") {
 		h_language_package = "default";

src/tex2lyx/Preamble.h

@@ -67,6 +67,8 @@ public:
 	///
 	bool isPackageUsed(std::string const & package) const;
 	///
+	bool isPackageAutoLoaded(std::string const & package) const;
+	///
 	std::vector<std::string>
 	getPackageOptions(std::string const & package) const;
 	/// Tell that \p package will be loaded automatically by LyX.

src/tex2lyx/test/test-modules.lyx.lyx

@@ -106,32 +106,8 @@ theorem.
 The proof is recognized as a builtin style provided by the text class.
 \end_layout
 
-\begin_layout Standard
-
-\begin_inset ERT
-status collapsed
-
-\begin_layout Plain Layout
-
-\backslash
-begin{lem}
-\end_layout
-
-\end_inset
-
- this is a lemma
-\begin_inset ERT
-status collapsed
-
-\begin_layout Plain Layout
-
-\backslash
-end{lem}
-\end_layout
-
-\end_inset
-
- 
+\begin_layout Lemma
+this is a lemma
 \end_layout
 
 \begin_layout Theorem

src/tex2lyx/test/test-structure.lyx.lyx

@@ -43,6 +43,7 @@
 \options dummyoption
 \use_default_options false
 \begin_modules
+fixltx2e
 logicalmkup
 \end_modules
 \maintain_unincluded_children false

src/tex2lyx/tex2lyx.cpp

@@ -322,17 +322,18 @@ bool checkModule(string const & name, bool command)
 	// Cache to avoid slowdown by repated searches
 	static set<string> failed[2];
 
-	// Only add the module if the command was actually defined in the LyX preamble
+	// Record whether the command was actually defined in the LyX preamble
 	bool theorem = false;
+	bool preamble_def = true;
 	if (command) {
 		if (possible_textclass_commands.find('\\' + name) == possible_textclass_commands.end())
-			return false;
+			preamble_def = false;
 	} else {
 		if (possible_textclass_environments.find(name) == possible_textclass_environments.end()) {
 			if (possible_textclass_theorems.find(name) != possible_textclass_theorems.end())
 				theorem = true;
 			else
-				return false;
+				preamble_def = false;
 		}
 	}
 	if (failed[command].find(name) != failed[command].end())
@@ -342,12 +343,16 @@ bool checkModule(string const & name, bool command)
 	LayoutFile const & baseClass = LayoutFileList::get()[textclass.name()];
 
 	// Try to find a module that defines the command.
-	// Only add it if the definition can be found in the preamble of the
-	// style that corresponds to the command. This is a heuristic and
-	// different from the way how we parse the builtin commands of the
-	// text class (in that case we only compare the name), but it is
-	// needed since it is not unlikely that two different modules define a
+	// For commands with preamble definitions we prefer modules where the definition
+	// can be found in the preamble of the style that corresponds to the command. 
+	// For others we check whether the command or module requires a package that is loaded
+	// in the tex file and use a style with the respective command.
+	// This is a heuristic and different from the way how we parse the builtin
+	// commands of the text class (in that case we only compare the name), 
+	// but it is needed since it is not unlikely that two different modules define a
 	// command with the same name.
+	string found_module;
+	vector<string> potential_modules;
 	ModuleMap::iterator const end = modules.end();
 	for (ModuleMap::iterator it = modules.begin(); it != end; ++it) {
 		string const module = it->first;
@@ -359,53 +364,102 @@ bool checkModule(string const & name, bool command)
 			continue;
 		DocumentClassConstPtr c = it->second;
 		Layout const * layout = findLayoutWithoutModule(*c, name, command);
-		InsetLayout const * insetlayout = layout ? 0 :
+		InsetLayout const * insetlayout = layout ? nullptr :
 			findInsetLayoutWithoutModule(*c, name, command);
 		docstring dpre;
-		if (layout)
+		std::set<std::string> cmd_reqs;
+		bool found_style = false;
+		if (layout) {
+			found_style = true;
 			dpre = layout->preamble();
-		else if (insetlayout)
+			std::set<std::string> lreqs = layout->requires();
+			if (!lreqs.empty())
+				cmd_reqs.insert(lreqs.begin(), lreqs.end());
+		} else if (insetlayout) {
+			found_style = true;
 			dpre = insetlayout->preamble();
-		if (dpre.empty())
+			std::set<std::string> lreqs = insetlayout->requires();
+			if (!lreqs.empty())
+				cmd_reqs.insert(lreqs.begin(), lreqs.end());
+		}
+		if (dpre.empty() && preamble_def)
 			continue;
-		bool add = false;
-		if (command) {
-			FullCommand const & cmd =
-				possible_textclass_commands['\\' + name];
-			if (dpre.find(cmd.def) != docstring::npos)
-				add = true;
-		} else if (theorem) {
-			FullCommand const & thm =
-				possible_textclass_theorems[name];
-			if (dpre.find(thm.def) != docstring::npos)
-				add = true;
-		} else {
-			FullEnvironment const & env =
-				possible_textclass_environments[name];
-			if (dpre.find(env.beg) != docstring::npos &&
-			    dpre.find(env.end) != docstring::npos)
-				add = true;
+		bool const package_cmd = dpre.empty();
+		bool match_req = false;
+		if (package_cmd) {
+			std::set<std::string> mreqs = it->second->requires();
+			if (!mreqs.empty())
+				cmd_reqs.insert(mreqs.begin(), mreqs.end());
+			for (auto const & pack : cmd_reqs) {
+				// If a requirement of the module matches a used package
+				// we load the module except if we have an auto-loaded package
+				// which is only required generally by the module, and the module
+				// does not provide the [inset]layout we are looking for.
+				// This heuristics should
+				// * load modules if the provide a style we don't have in the class
+				// * load modules that provide a package support generally (such as fixltx2e)
+				// * not unnecessarily load modules just because they require a package which we
+				//   load anyway.
+				if (preamble.isPackageUsed(pack)
+				    && (found_style || !preamble.isPackageAutoLoaded(pack))) {
+				    if (found_style)
+					    match_req = true;
+				    else		
+					    potential_modules.push_back(module);
+				    break;
+				}
+			}
+		}
+		bool add = match_req;
+		if (preamble_def) {
+			if (command) {
+				FullCommand const & cmd =
+					possible_textclass_commands['\\' + name];
+				if (dpre.find(cmd.def) != docstring::npos)
+					add = true;
+			} else if (theorem) {
+				FullCommand const & thm =
+					possible_textclass_theorems[name];
+				if (dpre.find(thm.def) != docstring::npos)
+					add = true;
+			} else {
+				FullEnvironment const & env =
+					possible_textclass_environments[name];
+				if (dpre.find(env.beg) != docstring::npos &&
+				    dpre.find(env.end) != docstring::npos)
+					add = true;
+			}
 		}
 		if (add) {
-			vector<string> v;
-			LayoutModuleList mods;
-			// addModule is necessary in order to catch required modules
-			// as well (see #11156)
-			if (!addModule(module, baseClass, mods, v))
+			found_module = module;
+			break;
+		}
+	}
+	if (found_module.empty()) {
+		// take one of the second row
+		if (!potential_modules.empty())
+			found_module = potential_modules.front();  
+	}
+		
+	if (!found_module.empty()) {
+		vector<string> v;
+		LayoutModuleList mods;
+		// addModule is necessary in order to catch required modules
+		// as well (see #11156)
+		if (!addModule(found_module, baseClass, mods, v))
+			return false;
+		for (auto const & mod : mods) {
+			if (!used_modules.moduleCanBeAdded(mod, &baseClass))
 				return false;
-			for (auto const & mod : mods) {
-				if (!used_modules.moduleCanBeAdded(mod, &baseClass))
-					return false;
-				FileName layout_file = libFileSearch("layouts", mod, "module");
-				if (textclass.read(layout_file, TextClass::MODULE)) {
-					used_modules.push_back(mod);
-					// speed up further searches:
-					// the module does not need to be checked anymore.
-					ModuleMap::iterator const it = modules.find(mod);
-					if (it != modules.end())
-						modules.erase(it);
-					return true;
-				}
+			FileName layout_file = libFileSearch("layouts", mod, "module");
+			if (textclass.read(layout_file, TextClass::MODULE)) {
+				used_modules.push_back(mod);
+				// speed up further searches:
+				// the module does not need to be checked anymore.
+				ModuleMap::iterator const it = modules.find(mod);
+				if (it != modules.end())
+					modules.erase(it);
+				return true;
 			}
 		}
 	}

status.23x

@@ -73,6 +73,8 @@ What's new
 
 * TEX2LYX
 
+- Significantly improve support for modules (bug 11259, 8229).
+
 - Prevent multiplication of preamble code on (re-)import (bug 8229).