BearND has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/403725 )
Change subject: WIP: Remove parentheses using state machine instead of regex
......................................................................
WIP: Remove parentheses using state machine instead of regex
Do not merge!
Bug: T184557
Change-Id: Ia135df4eb6fbb26064eb95764d1529f294ea4dbe
---
M lib/transformations/summarize.js
M test/lib/transformations/summarize.js
2 files changed, 245 insertions(+), 25 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/services/mobileapps
refs/changes/25/403725/1
diff --git a/lib/transformations/summarize.js b/lib/transformations/summarize.js
index 06c777b..1072f0f 100644
--- a/lib/transformations/summarize.js
+++ b/lib/transformations/summarize.js
@@ -34,31 +34,229 @@
}
}
+// Non-ASCII examples of parentheses are taken from
+// https://www.mediawiki.org/wiki/Topic:Tn0k9fjn0g8e9fpe.
+// They were all the same in the Asian languages.
+const TOKEN = {
+ Open: { ASCII: '(', NON_ASCII: '（' }, // %28, %uFF08
+ Close: { ASCII: ')', NON_ASCII: '）' }, // %29, %uFF09
+ Space: { NORMAL: ' ' }
+};
+
+class Range {
+ constructor(startNode, startNodeIndex, startCharacterIndex) {
+ this._startNode = startNode;
+ this._startNodeIndex = startNodeIndex;
+ this._startCharacterIndex = startCharacterIndex;
+ }
+
+ endRange(endNode, endNodeIndex, endCharacterIndex) {
+ this._endNode = endNode;
+ this._endNodeIndex = endNodeIndex;
+ this._endCharacterIndex = endCharacterIndex;
+ }
+
+ getStartNode() {
+ return this._startNode;
+ }
+
+ getStartNodeIndex() {
+ return this._startNodeIndex;
+ }
+
+ getStartCharacterIndex() {
+ return this._startCharacterIndex;
+ }
+
+ getEndNode() {
+ return this._endNode;
+ }
+
+ getEndNodeIndex() {
+ return this._endNodeIndex;
+ }
+
+ getEndIndex() {
+ return this._endCharacterIndex;
+ }
+
+ startsBefore(nextRange) {
+ return this._startNodeIndex < nextRange._startNodeIndex
+ || (
+ this._startNodeIndex === nextRange._startNodeIndex
+ && this._startCharacterIndex < nextRange._startCharacterIndex
+ );
+ }
+
+ endsBefore(nextRange) {
+ return this._endNodeIndex < nextRange._endNodeIndex
+ || (
+ this._endNodeIndex === nextRange._endNodeIndex
+ && this._endCharacterIndex < nextRange._endCharacterIndex
+ );
+ }
+
+ setSpace() {
+ this._hasSpace = true;
+ }
+
+ hasSpaces() {
+ return this._hasSpace;
+ }
+
+ debug() {
+ if (this._startNode === this._endNode) {
+ const text = this._startNode.textContent;
+ return `${text.slice(this._startCharacterIndex,
this._endCharacterIndex + 1)}`;
+ } else {
+ const startText =
this._startNode.textContent.slice(this._startCharacterIndex);
+ const endText = this._endNode.textContent.slice(0,
this._endCharacterIndex + 1);
+ return `'${startText}' to '${endText}'`;
+ }
+ }
+}
+
+function markParentheses(node, state) {
+ const text = node.textContent;
+ for (let i = 0; i < text.length; i++) {
+ const char = text[i];
+ if (char === TOKEN.Open.ASCII || char === TOKEN.Open.NON_ASCII) {
+ const range = new Range(node, node._nid, i);
+ state.ranges.push(range);
+ state.stack.push(range);
+ } else if (char === TOKEN.Close.ASCII || char ===
TOKEN.Close.NON_ASCII) {
+ const range = state.stack[state.stack.length - 1];
+ if (range) {
+ range.endRange(node, node._nid, i);
+ state.stack.pop();
+ }
+ } else if (char === TOKEN.Space.NORMAL) {
+ // TODO: do for all in stack???
+ const range = state.stack[state.stack.length - 1];
+ if (range && !range.hasSpaces()) {
+ range.setSpace();
+ }
+ }
+ }
+}
+
+// '(d e)'
+function discardIneligibleRanges(ranges) {
+ for (let i = 0; i < ranges.length; i++) {
+ const currentRange = ranges[i];
+ if (!currentRange.hasSpaces()) {
+ ranges.splice(i, 1); // remove current range
+ }
+ }
+}
+
+// 'a(b'
+function discardUnclosedRanges(ranges) {
+ for (let i = 0; i < ranges.length; i++) {
+ const currentRange = ranges[i];
+ if (!currentRange.getEndNode()) {
+ ranges.splice(i, 1); // remove current range
+ }
+ }
+}
+
+// 'a(b (d e) f)g'
+function discardNestedRanges(ranges) {
+ for (let i = 0; i < ranges.length - 1; i++) {
+ const currentRange = ranges[i];
+ const nextRange = ranges[i + 1];
+ if (currentRange.startsBefore(nextRange)
+ && nextRange.endsBefore(currentRange)
+ && currentRange.hasSpaces()) {
+ // next range is nested inside current eligible range
+ ranges.splice(i + 1, 1); // remove next range
+ }
+ }
+}
+
+function removeParenthesesInTextNode(node, state) {
+ let range = state.ranges[0];
+ let offset = 0;
+ while (range) {
+ if (state.deleting && node !== range.getEndNode()) {
+ node.remove();
+ }
+
+ if (node === range.getStartNode() && node === range.getEndNode()) {
+ // start and end in this text node
+ const originalText = node.textContent;
+ const startText = originalText.slice(0,
range.getStartCharacterIndex() - offset);
+ const endText = originalText.slice(range.getEndIndex() + 1 -
offset);
+ node.textContent = `${startText} ${endText}`;
+ offset = range.getEndIndex() - range.getStartCharacterIndex();
+ state.ranges.shift();
+ range = state.ranges[0];
+ } else if (node === range.getStartNode()) {
+ // just start
+ state.deleting = true;
+ const originalText = node.textContent;
+ const startText = originalText.slice(0,
range.getStartCharacterIndex() - offset);
+ node.textContent = startText;
+ break;
+ } else if (node === range.getEndNode()) {
+ // just end
+ state.deleting = false;
+ const originalText = node.textContent;
+ const endText = originalText.slice(range.getEndIndex() + 1 -
offset);
+ node.textContent = endText;
+ offset = range.getEndIndex();
+ state.ranges.shift();
+ range = state.ranges[0];
+ } else {
+ break;
+ }
+ }
+}
+
/**
* Visits one DOM node. Do the stuff that needs to be done when a single DOM
node is handled.
* In this case, remove DOM nodes and some attributes we don't want to keep.
* @param {!Node} node the node to visit
+ * @param {!Object} state some state to pass around to the visitor
*/
-function visitor(node) {
+function visitorPass1(node, state) {
if (node.nodeType === NodeType.ELEMENT_NODE) {
rmDisallowedElements(node);
if (node.tagName !== 'IMG') {
rmUnwantedAttributes(node);
}
- } else if (node.nodeType !== NodeType.TEXT_NODE) {
+ } else if (node.nodeType === NodeType.TEXT_NODE) {
+ markParentheses(node, state);
+ } else {
node.remove();
+ }
+}
+
+/**
+ * Visits one DOM node. Do the stuff that needs to be done when a single DOM
node is handled.
+ * In this case, remove DOM nodes and some attributes we don't want to keep.
+ * @param {!Node} node the node to visit
+ * @param {!Object} state some state to pass around to the visitor
+ */
+function visitorPass2(node, state) {
+ if (node.nodeType === NodeType.ELEMENT_NODE && state.deleting) {
+ node.remove();
+ } else if (node.nodeType === NodeType.TEXT_NODE) {
+ removeParenthesesInTextNode(node, state);
}
}
/**
* Traverses DOM tree iteratively (depth first).
* @param {!Element} rootElement the root of the DOM tree which needs to be
traversed
+ * @param {!Function} visitor called whenever a node gets encountered
+ * @param {!Object} state some state to pass around to the visitor
*/
-function traverseDF(rootElement) {
+function traverseDF(rootElement, visitor, state) {
let nodesToVisit = [ rootElement ];
while (nodesToVisit.length > 0) {
const currentNode = nodesToVisit.shift();
- visitor(currentNode);
+ visitor(currentNode, state);
nodesToVisit = [
...(currentNode.childNodes || []), // depth first
...nodesToVisit,
@@ -71,25 +269,32 @@
* @param {!Document} document the DOM document
*/
function removeUnwantedNodes(document) {
- traverseDF(document.body);
-}
-
-/**
- * Recursively discard any parentheticals that themselves are inside
parentheticals
- * @param {string} html
- * @return {string} html summary with nested parentheticals removed.
- */
-function removeNestedParentheticals(html) {
- // Remove any nested parentheticals
- const regex = /(\([^()]+)(\([^()]+\))/g;
- const newHtml = html.replace(regex, '$1 ');
-
- if (newHtml.match(regex)) {
- return removeNestedParentheticals(newHtml);
- } else {
- return newHtml;
+ const state = { ranges: [], stack: [] };
+ traverseDF(document.body, visitorPass1, state);
+ if (state.ranges.length > 0) { // found parentheses in previous pass
+ discardIneligibleRanges(state.ranges);
+ discardUnclosedRanges(state.ranges);
+ discardNestedRanges(state.ranges);
+ traverseDF(document.body, visitorPass2, state);
}
}
+
+// /**
+// * Recursively discard any parentheticals that themselves are inside
parentheticals
+// * @param {string} html
+// * @return {string} html summary with nested parentheticals removed.
+// */
+// function removeNestedParentheticals(html) {
+// // Remove any nested parentheticals
+// const regex = /(\([^()]+)(\([^()]+\))/g;
+// const newHtml = html.replace(regex, '$1 ');
+//
+// if (newHtml.match(regex)) {
+// return removeNestedParentheticals(newHtml);
+// } else {
+// return newHtml;
+// }
+// }
/**
* Given a chunk of HTML, create a summary of the content
@@ -108,15 +313,15 @@
removeUnwantedNodes(doc);
html = doc.body.innerHTML;
- html = removeNestedParentheticals(html);
+ // html = removeNestedParentheticals(html);
// 1. Replace any parentheticals which have at least one space inside
- html = html.replace(/\([^\)]+ [^\)]+\)/g, ' '); // eslint-disable-line
no-useless-escape
+ // html = html.replace(/\([^\)]+ [^\)]+\)/g, ' '); // eslint-disable-line
no-useless-escape
// 2. Remove any empty parentheticals due to transformations
- html = html.replace(/\(\)/g, ' ');
+ // html = html.replace(/\(\)/g, ' ');
// 3. Remove content inside any other non-latin parentheticals. The
behaviour is
// the same as 1 but for languages that are not latin based
- html = html.replace(/[（（（].+ .+[）））]/g, ' ');
+ // html = html.replace(/[（（（].+ .+[）））]/g, ' ');
// 4. remove all double spaces created by the above
html = html.replace(/ +/g, ' ');
diff --git a/test/lib/transformations/summarize.js
b/test/lib/transformations/summarize.js
index 5658cb9..3eb0f95 100644
--- a/test/lib/transformations/summarize.js
+++ b/test/lib/transformations/summarize.js
@@ -8,6 +8,21 @@
describe('summarize', () => {
it('matches the spec', () => {
const testCases = [
+ // Should remove nested parens
+ [
+ '1a(b (d e) f)g',
+ '1a g'
+ ],
+ // Should remove sequential parens
+ [
+ '2a(b c)d(e f)g',
+ '2a d g'
+ ],
+ // Should remove sequential parens except open one at the end
+ [
+ '3a(b c)d(e f)g(',
+ '3a d g('
+ ],
// Should remove unwanted elements
[
'<style>f</style><object>o</object>o',
--
To view, visit https://gerrit.wikimedia.org/r/403725
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia135df4eb6fbb26064eb95764d1529f294ea4dbe
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/services/mobileapps
Gerrit-Branch: master
Gerrit-Owner: BearND <bsitzm...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
