Subramanya Sastry has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/329352 )
Change subject: Cleanup in TemplateHandler
......................................................................
Cleanup in TemplateHandler
* Removed stale code and refactored control flow to make the
logic a bit more clearer. There was (and still is) a fair bit
of cruft here.
Change-Id: I0a7e3a593eca93a0e4fd9fbac6cfde803ebd524c
---
M lib/wt2html/tt/TemplateHandler.js
M tests/parserTests.txt
2 files changed, 89 insertions(+), 94 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/parsoid
refs/changes/52/329352/1
diff --git a/lib/wt2html/tt/TemplateHandler.js
b/lib/wt2html/tt/TemplateHandler.js
index e11ad4f..e42c8df 100644
--- a/lib/wt2html/tt/TemplateHandler.js
+++ b/lib/wt2html/tt/TemplateHandler.js
@@ -63,51 +63,50 @@
* processes the template.
*/
TemplateHandler.prototype.onTemplate = function(token, frame, cb) {
+ var atm;
var env = this.env;
-
- // magic word variables can be mistaken for templates
- try {
- var magicWord = this.checkForMagicWordVariable(token);
- if (magicWord) {
- cb({ tokens: Array.isArray(magicWord) ? magicWord :
[magicWord] });
- return;
- }
- } catch (e) {
- env.log("error", "Exception checking magic word for token: ",
token);
- }
-
+ var text = token.dataAttribs.src;
var state = {
token: token,
wrapperType: 'mw:Transclusion',
wrappedObjectId: env.newObjectId(),
+ recordArgDict: this.options.wrapTemplates,
srcCB: this._startTokenPipeline,
- };
-
- if (this.options.wrapTemplates) {
- state.recordArgDict = true;
// Uncomment to use DOM-based template expansion
// TODO gwicke: Determine when to use this!
// - Collect stats per template and classify templates into
- // balanced/unbalanced ones based on it
+ // balanced/unbalanced ones based on it
// - Always force nesting for new templates inserted by the VE
- // state.srcCB = this._startDocumentPipeline;
-
+ //
// Default to 'safe' token-based template encapsulation for now.
+ //
+ // srcCB: this._startDocumentPipeline
+ };
+
+ var tgt = this.resolveTemplateTarget(state, token.attribs[0].k);
+ if (tgt && (tgt.isPF || tgt.isMagicWord)) {
+ // magic word variables can be mistaken for templates
+ try {
+ var magicWord = this.checkForMagicWordVariable(token);
+ if (magicWord) {
+ cb({ tokens: Array.isArray(magicWord) ?
magicWord : [magicWord] });
+ return;
+ }
+ } catch (e) {
+ env.log("error", "Exception ", e, " checking magic word
for token: ", token);
+ }
}
- var text = token.dataAttribs.src;
- var tgt = this.resolveTemplateTarget(state, token.attribs[0].k);
- var accumReceiveToksFromSibling;
- var accumReceiveToksFromChild;
-
- // console.warn("\ttgt", tgt);
if (this.options.wrapTemplates && tgt === null) {
// Target contains tags, convert template braces and pipes back
into text
// Re-join attribute tokens with '=' and '|'
this.convertAttribsToString(state, token.attribs, cb);
return;
}
+
+ var accumReceiveToksFromSibling;
+ var accumReceiveToksFromChild;
if (env.conf.parsoid.usePHPPreProcessor) {
if (this.options.wrapTemplates) {
@@ -193,15 +192,17 @@
// TokenAccumulator and can return the expansion
directly
accumReceiveToksFromSibling = cb;
}
+
+ accumReceiveToksFromSibling({tokens: [], async: true});
+
// expand argument keys, with callback set to next processing
step
// XXX: would likely be faster to do this in a tight loop here
- var atm = new AttributeTransformManager(
+ atm = new AttributeTransformManager(
this.manager,
{wrapTemplates: false, inTemplate:
true},
this._expandTemplate.bind(this, state,
frame, tgt,
accumReceiveToksFromSibling)
);
- accumReceiveToksFromSibling({tokens: [], async: true});
atm.process(token.attribs);
}
};
@@ -242,6 +243,7 @@
// 2. String with entities: {{DEFAULTSORT:"foo"bar}}
// 3. Templated key: {{DEFAULTSORT:{{foo}}bar}}
+ var env = this.manager.env;
var property, key, propAndKey, keyToks;
var magicWord = tplToken.attribs[0].k;
@@ -281,11 +283,11 @@
}
property = property.trim();
- var name = this.manager.env.conf.wiki.magicWords[property];
+ var name = env.conf.wiki.magicWords[property];
// try without the colon
if (!name) {
- name = this.manager.env.conf.wiki.magicWords[property.slice(0,
-1)];
+ name = env.conf.wiki.magicWords[property.slice(0, -1)];
}
// special case for {{!}} magic word
@@ -301,7 +303,7 @@
var state = {
token: tplToken,
wrapperType: "mw:Transclusion",
- wrappedObjectId: this.manager.env.newObjectId(),
+ wrappedObjectId: env.newObjectId(),
};
this.resolveTemplateTarget(state, "!");
@@ -362,7 +364,7 @@
};
TemplateHandler.prototype.resolveTemplateTarget = function(state, targetToks) {
- function resolvabilityInfo(tokens) {
+ function toString(tokens) {
var maybeTarget = Util.tokensToString(tokens, true);
if (Array.isArray(maybeTarget)) {
var allString = true;
@@ -382,104 +384,97 @@
&& ntt.name !==
'templatearg') {
// We are okay with empty
(comment-only) lines,
// {{..}} and {{{..}}} in
template targets.
- return { isStr: false };
+ return null;
}
} else if (ntt.constructor === CommentTk) {
// Ignore comments as well
allString = false;
} else if (ntt.constructor === TagTk ||
ntt.constructor === EndTagTk) {
- return { isStr: false };
+ return null;
}
}
- return { isStr: true, isSimpleTgt: allString,
newTarget: newTarget };
+ return newTarget;
} else {
- return { isStr: true, isSimpleTgt: true };
+ return maybeTarget;
}
}
var env = this.manager.env;
+ var wiki = env.conf.wiki;
+ var isTemplate = true;
+ var target = toString(targetToks);
- // Convert the target to a string while stripping all non-text tokens
- var target = Util.tokensToString(targetToks).trim();
+ if (!target) {
+ // Retry with a looser attempt to convert tokens to a string.
+ // This lenience only applies to parser functions.
+ isTemplate = false;
+ target = Util.tokensToString(targetToks);
+ }
- // strip subst for now.
- target = target.replace(/^(safe)?subst:/, '');
-
- // Check if we have a parser function.
- //
- // Unalias to canonical form and look in config.functionHooks
+ // Without a trim(), we get bug T147742 because
+ // the prefix === target check below fails!
+ target = target.trim().replace(/^(safe)?subst:/, '');
var pieces = target.split(':');
var prefix = pieces[0].trim();
var lowerPrefix = prefix.toLowerCase();
- var magicWordAlias = env.conf.wiki.magicWords[prefix] ||
env.conf.wiki.magicWords[lowerPrefix];
+ var isSpecialMagicWord = (target === '!');
+
+ // Check if we have a parser function.
+ // Unalias to canonical form and look in config.functionHooks
+ var magicWordAlias = wiki.magicWords[prefix] ||
wiki.magicWords[lowerPrefix];
var translatedPrefix = magicWordAlias || lowerPrefix || '';
// The check for pieces.length > 1 is require to distinguish between
// {{lc:FOO}} and {{lc|FOO}}. The latter is a template transclusion
// even though the target (=lc) matches a registered parser-function
name.
if ((magicWordAlias && this.parserFunctions &&
this.parserFunctions['pf_' + magicWordAlias]) ||
- (pieces.length > 1 && (translatedPrefix[0] === '#' ||
env.conf.wiki.functionHooks.has(translatedPrefix)))) {
+ (pieces.length > 1 && (translatedPrefix[0] === '#' ||
wiki.functionHooks.has(translatedPrefix)))) {
state.parserFunctionName = translatedPrefix;
return {
isPF: true,
+ magicWord: target,
prefix: prefix,
target: 'pf_' + translatedPrefix,
pfArg: target.substr(prefix.length + 1),
};
}
- // We are dealing with a real template, not a parser function.
- // Apply more stringent standards for template targets.
- var tgtInfo = resolvabilityInfo(targetToks);
- if (tgtInfo.isStr && !tgtInfo.isSimpleTgt) {
- // resolvabilityInfo found a new target based on the target
tokens. This
- // happens when the target contains special characters,
specially quotes.
- // For an example, look at T96090.
-
- // Without a trim(), we get bug T147742 because
- // the prefix === target check below fails!
- target = tgtInfo.newTarget.trim();
- pieces = target.split(':');
- prefix = pieces[0].trim();
- lowerPrefix = prefix.toLowerCase();
- }
-
- if (tgtInfo.isStr) {
- // FIXME: resolveTitle adds namespace prefix when it resolves
- // fragments and relative titles. Maybe we should add a flag to
- // have it do in all cases.
- if (prefix === target && !/^[#\/\.]/.test(target)) {
- var namespaceId =
env.conf.wiki.canonicalNamespaces.template;
- target = env.conf.wiki.namespaceNames[namespaceId] +
':' + target;
- }
-
- // Resolve a possibly relative link and
- // normalize the target before template processing.
- var title;
- try {
- title = env.resolveTitle(target);
- } catch (e) {
- // Invalid template target!
- return null;
- }
-
- // Entities in transclusions aren't decoded in the PHP parser
- // So, treat the title as a url-decoded string!
- title = env.makeTitleFromURLDecodedStr(title, undefined, true);
- if (!title) {
- // Invalid template target!
- return null;
- }
-
- // data-mw.target.href should be a url
- state.resolvedTemplateTarget = env.makeLink(title);
-
- return { isPF: false, target: title.getPrefixedDBKey() };
- } else {
+ if (!isTemplate) {
return null;
}
+ // FIXME: resolveTitle adds namespace prefix when it resolves
+ // fragments and relative titles. Maybe we should add a flag to
+ // have it do in all cases.
+ if (prefix === target && !/^[#\/\.]/.test(target)) {
+ var namespaceId = wiki.canonicalNamespaces.template;
+ target = wiki.namespaceNames[namespaceId] + ':' + target;
+ }
+
+ // Resolve a possibly relative link and
+ // normalize the target before template processing.
+ var title;
+ try {
+ title = env.resolveTitle(target);
+ } catch (e) {
+ // Invalid template target!
+ return null;
+ }
+
+ // Entities in transclusions aren't decoded in the PHP parser
+ // So, treat the title as a url-decoded string!
+ title = env.makeTitleFromURLDecodedStr(title, undefined, true);
+ if (!title) {
+ // Invalid template target!
+ return null;
+ }
+
+ // data-mw.target.href should be a url
+ state.resolvedTemplateTarget = env.makeLink(title);
+
+ return { isPF: false, isMagicWord: isSpecialMagicWord, target:
title.getPrefixedDBKey() };
+
};
TemplateHandler.prototype.convertAttribsToString = function(state, attribs,
cb) {
diff --git a/tests/parserTests.txt b/tests/parserTests.txt
index 1205f53..c5dd865 100644
--- a/tests/parserTests.txt
+++ b/tests/parserTests.txt
@@ -10325,7 +10325,7 @@
!! wikitext
{{DISPLAYTITLE:''{{PAGENAME}}''}}
!! html/parsoid
-<meta property="mw:PageProp/displaytitle" content="Main Page" about="#mwt2"
typeof="mw:ExpandedAttrs"
data-parsoid='{"src":"{{DISPLAYTITLE:''{{PAGENAME}}''}}"}'
data-mw='{"attribs":[[{"txt":"content"},{"html":"<i
data-parsoid='{\"dsr\":[15,31,2,2]}'><span about=\"#mwt1\"
typeof=\"mw:Transclusion\"
data-parsoid='{\"pi\":[[]],\"dsr\":[17,29,null,null]}'
data-mw='{\"parts\":[{\"template\":{\"target\":{\"wt\":\"PAGENAME\",\"function\":\"pagename\"},\"params\":{},\"i\":0}}]}'>Main
Page</span></i>"}]]}'/>
+<meta property="mw:PageProp/displaytitle" content="Main Page" about="#mwt3"
typeof="mw:ExpandedAttrs"
data-parsoid='{"src":"{{DISPLAYTITLE:''{{PAGENAME}}''}}"}'
data-mw='{"attribs":[[{"txt":"content"},{"html":"<i
data-parsoid='{\"dsr\":[15,31,2,2]}'><span about=\"#mwt2\"
typeof=\"mw:Transclusion\"
data-parsoid='{\"pi\":[[]],\"dsr\":[17,29,null,null]}'
data-mw='{\"parts\":[{\"template\":{\"target\":{\"wt\":\"PAGENAME\",\"function\":\"pagename\"},\"params\":{},\"i\":0}}]}'>Main
Page</span></i>"}]]}'/>
!! end
!! test
--
To view, visit https://gerrit.wikimedia.org/r/329352
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0a7e3a593eca93a0e4fd9fbac6cfde803ebd524c
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/parsoid
Gerrit-Branch: master
Gerrit-Owner: Subramanya Sastry <ssas...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
