Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix transclude inefficiency #7647

Merged
merged 9 commits into from
Jul 30, 2023
101 changes: 66 additions & 35 deletions core/modules/widgets/transclude.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this.parseAsInline is only initialized if this case has been executed. Is that intentional? IMO undefined means "we do not know". The variable parseAsInline is defined as true or false. Just a gut feeling.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code relies on this.parseAsInline defaulting to false

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();
Expand Down Expand Up @@ -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")) {
Expand Down Expand Up @@ -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: [
Expand Down Expand Up @@ -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
};
};

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
<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
Original file line number Diff line number Diff line change
@@ -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>