Subramanya Sastry has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/54100


Change subject: Code cleanup: Moved AttributeExpander specific helpers out of 
Util.
......................................................................

Code cleanup: Moved AttributeExpander specific helpers out of Util.

* As I was looking at some include* specific code changes, I found
  AttributeExpander specific helpers in Util -- there is no reason
  for them to be there since they are extremely specific to attr.
  expansion and have no value to other classes.

* Added a few FIXMEs to dead/duplicate utility functions.  Worth
  cleaning up at some point.

Change-Id: I23ab23a81d93c87efe51bd5c38b028d0ce015af2
---
M js/lib/ext.core.AttributeExpander.js
M js/lib/mediawiki.Util.js
2 files changed, 94 insertions(+), 92 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Parsoid 
refs/changes/00/54100/1

diff --git a/js/lib/ext.core.AttributeExpander.js 
b/js/lib/ext.core.AttributeExpander.js
index 2b005d2..cfb310f 100644
--- a/js/lib/ext.core.AttributeExpander.js
+++ b/js/lib/ext.core.AttributeExpander.js
@@ -12,6 +12,88 @@
                                                                        
.AttributeTransformManager,
        defines = require('./mediawiki.parser.defines.js');
 
+/* ----------------------------------------------------------
+ * This helper method does two different things:
+ *
+ * 1. Strips all meta tags
+ *    FIXME: should I be selective and only strip mw:Object/ * tags?
+ * 2. In wrap-template mode, it identifies the meta-object type
+ *    and returns it.
+ * ---------------------------------------------------------- */
+function stripMetaTags( tokens, wrapTemplates ) {
+       var isPushed, buf = [],
+               wikitext = [],
+               metaObjTypes = [],
+               inTemplate = false;
+
+       for (var i = 0, l = tokens.length; i < l; i++) {
+               var token = tokens[i];
+               isPushed = false;
+               if ([TagTk, SelfclosingTagTk].indexOf(token.constructor) !== 
-1) {
+                       // Strip all meta tags.
+                       // SSS FIXME: should I be selective and only strip 
mw:Object/* tags?
+                       if (wrapTemplates) {
+                               // If we are in wrap-template mode, extract 
info from the meta-tag
+                               var t = token.getAttribute("typeof");
+                               var typeMatch = t && 
t.match(/(mw:Object(?:\/.*)?$)/);
+                               if (typeMatch) {
+                                       inTemplate = 
!(typeMatch[1].match(/\/End$/));
+                                       if (inTemplate) {
+                                               metaObjTypes.push(typeMatch[1]);
+                                               
wikitext.push(token.dataAttribs.src);
+                                       }
+                               } else {
+                                       isPushed = true;
+                                       buf.push(token);
+                               }
+                       }
+
+                       if (!isPushed && token.name !== "meta") {
+                               // Dont strip token if it is not a meta-tag
+                               buf.push(token);
+                       }
+               } else {
+                       // Assumes that non-template tokens are always text.
+                       // In turn, based on assumption that HTML attribute 
values
+                       // cannot contain any HTML (SSS FIXME: Isn't this true?)
+                       if (!inTemplate) {
+                               wikitext.push(token);
+                       }
+                       buf.push(token);
+               }
+       }
+
+       return {
+               // SSS FIXME: Assumes that either the attr. has only 1 expansion
+               // OR all expansions are of the same type.
+               // Consider the attr composed of pieces: s1, s2, s3
+               // - s1 can be generated by a template
+               // - s2 can be plain text
+               // - s3 can be generated by an extension.
+               // While that might be considered utter madness, there is 
nothing in
+               // the spec right now that prevents this.  In any case, not sure
+               // we do require all expandable types to be tracked.
+               metaObjType: metaObjTypes[0],
+               wikitext: Util.tokensToString(wikitext),
+               value: buf
+       };
+}
+
+function makeTplAffectedMeta ( contentType, key, val ) {
+       // SSS FIXME: Assumes that all expanded attrs. have the same expandable 
type
+       // - attr1 can be expanded by a template
+       // - attr2 can be expanded by an extension
+       // While that might be considered madness, there is nothing in the spec 
right
+       // not that prevents this.  In any case, not sure we do require all
+       // expandable types to be tracked.
+
+       // <meta about="#mwt1" property="mw:objectAttr#href" data-parsoid="...">
+       // about will be filled out in the end
+       return new SelfclosingTagTk( 'meta',
+               [new KV( "property", "mw:" + contentType + "#" + key )],
+               { src: val.wikitext });
+}
+
 /**
  * @class
  *
@@ -102,11 +184,11 @@
                        var contentType = "objectAttrKey"; // default
                        if (a.k.constructor === Array) {
                                if ( newK.constructor === String && newK.match( 
/mw\:maybeContent/ ) ) {
-                                       updatedK = Util.stripMetaTags( 
'mw:keyAffected', this.options.wrapTemplates );
+                                       updatedK = stripMetaTags( 
'mw:keyAffected', this.options.wrapTemplates );
                                        newAttrs.push( new KV( 
'mw:keyAffected', newA.v ) );
                                        newK = updatedK.value;
                                } else {
-                                       updatedK = Util.stripMetaTags(newK, 
this.options.wrapTemplates);
+                                       updatedK = stripMetaTags(newK, 
this.options.wrapTemplates);
                                        newK = updatedK.value;
                                        if (newA.v === '') {
                                                // Some templates can return 
content that should be
@@ -141,7 +223,7 @@
                                        metaObjType = updatedK.metaObjType;
                                        if (metaObjType) {
                                                producerObjType = metaObjType;
-                                               metaTokens.push( 
Util.makeTplAffectedMeta(contentType, newK, updatedK) );
+                                               metaTokens.push( 
makeTplAffectedMeta(contentType, newK, updatedK) );
                                        }
                                }
                        } else if (newK !== a.k) {
@@ -173,22 +255,22 @@
                                        // attr has been generated by a 
template/extension, keep around both the
                                        // original as well as the stripped 
versions of the template-generated
                                        // attr, and in the link handler, we 
will pick the right version.
-                                       updatedV = Util.stripMetaTags( newA.v, 
this.options.wrapTemplates );
+                                       updatedV = stripMetaTags( newA.v, 
this.options.wrapTemplates );
                                        metaObjType = updatedV.metaObjType;
                                        if (metaObjType) {
                                                kv = new KV('mw:valAffected', [
                                                        metaObjType,
-                                                       
Util.makeTplAffectedMeta("objectAttrVal", newK, updatedV)
+                                                       
makeTplAffectedMeta("objectAttrVal", newK, updatedV)
                                                ]);
                                                newAttrs.push( kv );
                                        }
                                } else if (!newK.match(/^mw:/)) {
-                                       updatedV = Util.stripMetaTags( newA.v, 
this.options.wrapTemplates );
+                                       updatedV = stripMetaTags( newA.v, 
this.options.wrapTemplates );
                                        newA.v = updatedV.value;
                                        metaObjType = updatedV.metaObjType;
                                        if (metaObjType) {
                                                producerObjType = metaObjType;
-                                               metaTokens.push( 
Util.makeTplAffectedMeta("objectAttrVal", newK, updatedV) );
+                                               metaTokens.push( 
makeTplAffectedMeta("objectAttrVal", newK, updatedV) );
                                        }
                                }
                        } else if (newA.v !== a.v) {
diff --git a/js/lib/mediawiki.Util.js b/js/lib/mediawiki.Util.js
index ec2dcd3..d4da504 100644
--- a/js/lib/mediawiki.Util.js
+++ b/js/lib/mediawiki.Util.js
@@ -1,8 +1,8 @@
-"use strict";
-
 /*
  * This file contains general utilities for token transforms
  */
+
+"use strict";
 
 var domino = require( './domino' ),
        path = require('path'),
@@ -855,13 +855,14 @@
                        }
                }
                console.error(out.join(arguments[1]));
-       },
+       }
 };
 
 /**
  * Utility function for stripping useless paragraphs from the beginning of a 
list item,
  * because they get appended by VisualEditor sometimes.
  */
+// FIXME: Dead function?  SelectiveSerializer doesn't seem to use this
 Util.stripFirstParagraph = function ( node ) {
        var thisnode, hasAttrs, dataParsoid, attrs, exemptAttrs = 0, haveGone = 
false;
        for ( var i = 0; i < node.childNodes.length; i++ ) {
@@ -886,6 +887,7 @@
        }
 };
 
+// FIXME: There is also a DOMUtils.getJSONAttribute. Consolidate
 Util.getJSONAttribute = function ( node, attr, fallback ) {
     fallback = fallback || null;
     var atext;
@@ -899,88 +901,6 @@
     } else {
         return fallback;
     }
-};
-
-/* ----------------------------------------------------------
- * This method does two different things:
- *
- * 1. Strips all meta tags
- *    (FIXME: should I be selective and only strip mw:Object/* tags?)
- * 2. In wrap-template mode, it identifies the meta-object type
- *    and returns it.
- * ---------------------------------------------------------- */
-Util.stripMetaTags = function ( tokens, wrapTemplates ) {
-       var isPushed, buf = [],
-               wikitext = [],
-               metaObjTypes = [],
-               inTemplate = false;
-
-       for (var i = 0, l = tokens.length; i < l; i++) {
-               var token = tokens[i];
-               isPushed = false;
-               if ([TagTk, SelfclosingTagTk].indexOf(token.constructor) !== 
-1) {
-                       // Strip all meta tags.
-                       // SSS FIXME: should I be selective and only strip 
mw:Object/* tags?
-                       if (wrapTemplates) {
-                               // If we are in wrap-template mode, extract 
info from the meta-tag
-                               var t = token.getAttribute("typeof");
-                               var typeMatch = t && 
t.match(/(mw:Object(?:\/.*)?$)/);
-                               if (typeMatch) {
-                                       inTemplate = 
!(typeMatch[1].match(/\/End$/));
-                                       if (inTemplate) {
-                                               metaObjTypes.push(typeMatch[1]);
-                                               
wikitext.push(token.dataAttribs.src);
-                                       }
-                               } else {
-                                       isPushed = true;
-                                       buf.push(token);
-                               }
-                       }
-
-                       if (!isPushed && token.name !== "meta") {
-                               // Dont strip token if it is not a meta-tag
-                               buf.push(token);
-                       }
-               } else {
-                       // Assumes that non-template tokens are always text.
-                       // In turn, based on assumption that HTML attribute 
values
-                       // cannot contain any HTML (SSS FIXME: Isn't this true?)
-                       if (!inTemplate) {
-                               wikitext.push(token);
-                       }
-                       buf.push(token);
-               }
-       }
-
-       return {
-               // SSS FIXME: Assumes that either the attr. has only 1 expansion
-               // OR all expansions are of the same type.
-               // Consider the attr composed of pieces: s1, s2, s3
-               // - s1 can be generated by a template
-               // - s2 can be plain text
-               // - s3 can be generated by an extension.
-               // While that might be considered utter madness, there is 
nothing in
-               // the spec right now that prevents this.  In any case, not sure
-               // we do require all expandable types to be tracked.
-               metaObjType: metaObjTypes[0],
-               wikitext: Util.tokensToString(wikitext),
-               value: buf
-       };
-};
-
-Util.makeTplAffectedMeta = function ( contentType, key, val ) {
-       // SSS FIXME: Assumes that all expanded attrs. have the same expandable 
type
-       // - attr1 can be expanded by a template
-       // - attr2 can be expanded by an extension
-       // While that might be considered madness, there is nothing in the spec 
right
-       // not that prevents this.  In any case, not sure we do require all
-       // expandable types to be tracked.
-
-       // <meta about="#mwt1" property="mw:objectAttr#href" data-parsoid="...">
-       // about will be filled out in the end
-       return new SelfclosingTagTk( 'meta',
-               [new KV( "property", "mw:" + contentType + "#" + key )],
-               { src: val.wikitext });
 };
 
 // Separate closure for normalize functions that

-- 
To view, visit https://gerrit.wikimedia.org/r/54100
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I23ab23a81d93c87efe51bd5c38b028d0ce015af2
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Parsoid
Gerrit-Branch: master
Gerrit-Owner: Subramanya Sastry <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to