From 4bdac09872f0df02713db16975c4d928462fb9a0 Mon Sep 17 00:00:00 2001
From: Jeremy Ruston <jeremy@jermolene.com>
Date: Sun, 30 Jul 2023 18:04:05 +0100
Subject: [PATCH] Fix transclude inefficiency (#7647)

* Refactor parse mode out of getTransclusionTarget

* Refactor missing transclusion target

* Add a test to avoid regressions on the handling of macros vs procedures

* Refactor condition logic

* Preparing to split getTransclusionTarget into two separate functions

* Split getTransclusionTarget into getTransclusionTargetIncludingParseTreeNodes

* Resolve another inefficiency

The transclusion target was sometimes being parsed twice when transcluding as text/plain

Associated test results are also made more consistent

* Simplify method naming

* Neatening up
---
 core/modules/widgets/transclude.js            | 101 ++++++++++++------
 .../data/functions/WikifiedFunctions.tid      |   2 +-
 .../Procedures-Double-Underscore.tid          |  26 +++++
 3 files changed, 93 insertions(+), 36 deletions(-)
 create mode 100644 editions/test/tiddlers/tests/data/transclude/Procedures-Double-Underscore.tid

diff --git a/core/modules/widgets/transclude.js b/core/modules/widgets/transclude.js
index 05d03a702..ac467a2c8 100755
--- a/core/modules/widgets/transclude.js
+++ b/core/modules/widgets/transclude.js
@@ -41,30 +41,43 @@ TranscludeWidget.prototype.execute = function() {
 	this.collectAttributes();
 	this.collectStringParameters();
 	this.collectSlotFillParameters();
-	// Get the target text and parse tree nodes that we are transcluding
-	var target = this.getTransclusionTarget(),
-		parseTreeNodes;
-	this.sourceText = target.text;
-	this.parserType = target.type;
-	this.parseAsInline = target.parseAsInline;
+	// Determine whether we're being used in inline or block mode
+	var parseAsInline = !this.parseTreeNode.isBlock;
+	if(this.transcludeMode === "inline") {
+		parseAsInline = true;
+	} else if(this.transcludeMode === "block") {
+		parseAsInline = false;
+	}
 	// Set 'thisTiddler'
 	this.setVariable("thisTiddler",this.transcludeTitle);
+	var parseTreeNodes, target;
 	// Process the transclusion according to the output type
 	switch(this.transcludeOutput || "text/html") {
 		case "text/html":
-			// Return the parse tree nodes
+			// Return the parse tree nodes of the target
+			target = this.parseTransclusionTarget(parseAsInline);
+			this.parseAsInline = target.parseAsInline;
 			parseTreeNodes = target.parseTreeNodes;
 			break;
 		case "text/raw":
 			// Just return the raw text
-			parseTreeNodes = [{type: "text", text: this.sourceText}];
+			target = this.getTransclusionTarget();
+			parseTreeNodes = [{type: "text", text: target.text}];
 			break;
 		default:
-			// text/plain
-			var plainText = this.wiki.renderText("text/plain",this.parserType,this.sourceText,{parentWidget: this});
-			parseTreeNodes = [{type: "text", text: plainText}];
+			// "text/plain" is the plain text result of wikifying the text
+			target = this.parseTransclusionTarget(parseAsInline);
+			var widgetNode = this.wiki.makeWidget(target.parser,{
+				parentWidget: this,
+				document: $tw.fakeDocument
+			});
+			var container = $tw.fakeDocument.createElement("div");
+			widgetNode.render(container,null);
+			parseTreeNodes = [{type: "text", text: container.textContent}];
 			break;
 	}
+	this.sourceText = target.text;
+	this.parserType = target.type;
 	// Set the legacy transclusion context variables only if we're not transcluding a variable
 	if(!this.transcludeVariable) {
 		var recursionMarker = this.makeRecursionMarker();
@@ -161,17 +174,44 @@ TranscludeWidget.prototype.collectSlotFillParameters = function() {
 };
 
 /*
-Get transcluded parse tree nodes as an object {text:,type:,parseTreeNodes:,parseAsInline:}
+Get transcluded details as an object {text:,type:}
 */
 TranscludeWidget.prototype.getTransclusionTarget = function() {
 	var self = this;
-	// Determine whether we're being used in inline or block mode
-	var parseAsInline = !this.parseTreeNode.isBlock;
-	if(this.transcludeMode === "inline") {
-		parseAsInline = true;
-	} else if(this.transcludeMode === "block") {
-		parseAsInline = false;
+	var text;
+	// Return the text and type of the target
+	if(this.hasAttribute("$variable")) {
+		if(this.transcludeVariable) {
+			// Transcluding a variable
+			var variableInfo = this.getVariableInfo(this.transcludeVariable,{params: this.getOrderedTransclusionParameters()});
+			text = variableInfo.text;
+			return {
+				text: variableInfo.text,
+				type: this.transcludeType
+			};
+		}
+	} else {
+		// Transcluding a text reference
+		var parserInfo = this.wiki.getTextReferenceParserInfo(
+						this.transcludeTitle,
+						this.transcludeField,
+						this.transcludeIndex,
+						{
+							subTiddler: this.transcludeSubTiddler,
+							defaultType: this.transcludeType
+						});
+		return {
+			text: parserInfo.text,
+			type: parserInfo.type
+		};
 	}
+};
+
+/*
+Get transcluded parse tree nodes as an object {text:,type:,parseTreeNodes:,parseAsInline:}
+*/
+TranscludeWidget.prototype.parseTransclusionTarget = function(parseAsInline) {
+	var self = this;
 	var parser;
 	// Get the parse tree
 	if(this.hasAttribute("$variable")) {
@@ -237,7 +277,7 @@ TranscludeWidget.prototype.getTransclusionTarget = function() {
 							}
 							$tw.utils.addAttributeToParseTreeNode(parser.tree[0],name,param["default"])
 						});
-					} else if(srcVariable && (srcVariable.isMacroDefinition || !srcVariable.isFunctionDefinition)) {
+					} else if(srcVariable && !srcVariable.isFunctionDefinition) {
 						// For macros and ordinary variables, wrap the parse tree in a vars widget assigning the parameters to variables named "__paramname__"
 						parser = {
 							tree: [
@@ -269,22 +309,13 @@ TranscludeWidget.prototype.getTransclusionTarget = function() {
 						});
 	}
 	// Return the parse tree
-	if(parser) {
-		return {
-			parseTreeNodes: parser.tree,
-			parseAsInline: parseAsInline,
-			text: parser.source,
-			type: parser.type
-		};
-	} else {
-		// If there's no parse tree then return the missing slot value
-		return {
-			parseTreeNodes: (this.slotFillParseTrees["ts-missing"] || []),
-			parseAsInline: parseAsInline,
-			text: null,
-			type: null
-		};
-	}
+	return {
+		parser: parser,
+		parseTreeNodes: parser ? parser.tree : (this.slotFillParseTrees["ts-missing"] || []),
+		parseAsInline: parseAsInline,
+		text: parser && parser.source,
+		type: parser && parser.type
+	};
 };
 
 /*
diff --git a/editions/test/tiddlers/tests/data/functions/WikifiedFunctions.tid b/editions/test/tiddlers/tests/data/functions/WikifiedFunctions.tid
index 733fbdaef..36b64e4a3 100644
--- a/editions/test/tiddlers/tests/data/functions/WikifiedFunctions.tid
+++ b/editions/test/tiddlers/tests/data/functions/WikifiedFunctions.tid
@@ -33,4 +33,4 @@ $param$ with a ''buffalo''
 +
 title: ExpectedResult
 
-<p>Going to lunch with a ''buffalo''</p><p>Going to breakfastwith a<strong>buffalo</strong></p><p>Going to dinner with a <strong>buffalo</strong></p>Going to lunch with a buffalo with a buffaloGoing to dinner with a buffalo
\ No newline at end of file
+<p>Going to lunch with a ''buffalo''</p><p>Going to breakfastwith a<strong>buffalo</strong></p><p>Going to dinner with a <strong>buffalo</strong></p>Going to lunch with a ''buffalo''Going to breakfastwith abuffaloGoing to dinner with a buffalo
\ No newline at end of file
diff --git a/editions/test/tiddlers/tests/data/transclude/Procedures-Double-Underscore.tid b/editions/test/tiddlers/tests/data/transclude/Procedures-Double-Underscore.tid
new file mode 100644
index 000000000..f22efb4f6
--- /dev/null
+++ b/editions/test/tiddlers/tests/data/transclude/Procedures-Double-Underscore.tid
@@ -0,0 +1,26 @@
+title: Procedures/Double/Underscore
+description: Checking that procedures don't expose parameters as variables wrapped in double underscores
+type: text/vnd.tiddlywiki-multiple
+tags: [[$:/tags/wiki-test-spec]]
+
+title: Output
+
+\whitespace trim
+\procedure mamacro(one:"red",two:"green")
+It is $one$ and $two$<<__one__>><<__two__>>.
+\end
+
+<$macrocall $name="mamacro"/>
+
+<$transclude $variable="mamacro"/>
+
+<$transclude $variable="mamacro" one="orange"/>
+
+<$transclude $variable="mamacro" 0="pink"/>
+
+<$transclude $variable="mamacro" one="purple" 1="pink"/>
+
++
+title: ExpectedResult
+
+<p>It is $one$ and $two$.</p><p>It is $one$ and $two$.</p><p>It is $one$ and $two$.</p><p>It is $one$ and $two$.</p><p>It is $one$ and $two$.</p>
\ No newline at end of file