From 657492e191af4fc39f6cda6a995f8dffebfe6a2d Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 25 Oct 2021 01:21:19 -0400 Subject: [PATCH] Changeset: Turn `newOp()` into a real class --- CHANGELOG.md | 1 + src/node/utils/padDiff.js | 4 +- src/static/js/Changeset.js | 143 +++++++++++++++++---------- src/static/js/ace2_inner.js | 2 +- src/static/js/contentcollector.js | 2 +- src/static/js/linestylefilter.js | 2 +- src/tests/frontend/specs/easysync.js | 6 +- 7 files changed, 99 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e11c6b691..03aab8890 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ * `opAttributeValue()` * `appendATextToAssembler()`: Deprecated in favor of the new `opsFromAText()` generator function. + * `newOp()`: Deprecated in favor of the new `Op` class. # 1.8.15 diff --git a/src/node/utils/padDiff.js b/src/node/utils/padDiff.js index 05cba308f..d0c61633f 100644 --- a/src/node/utils/padDiff.js +++ b/src/node/utils/padDiff.js @@ -270,7 +270,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) { let curChar = 0; let curLineOpIter = null; let curLineOpIterLine; - let curLineNextOp = Changeset.newOp('+'); + let curLineNextOp = new Changeset.Op('+'); const unpacked = Changeset.unpack(cs); const csIter = Changeset.opIterator(unpacked.ops); @@ -302,7 +302,7 @@ PadDiff.prototype._createDeletionChangeset = function (cs, startAText, apool) { } if (!curLineNextOp.chars) { - curLineNextOp = curLineOpIter.hasNext() ? curLineOpIter.next() : Changeset.newOp(); + curLineNextOp = curLineOpIter.hasNext() ? curLineOpIter.next() : new Changeset.Op(); } const charsToUse = Math.min(numChars, curLineNextOp.chars); diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index fd34f39e4..0b3b01a7c 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -83,31 +83,74 @@ exports.numToString = (num) => num.toString(36).toLowerCase(); /** * An operation to apply to a shared document. - * - * @typedef {object} Op - * @property {('+'|'-'|'='|'')} opcode - The operation's operator: - * - '=': Keep the next `chars` characters (containing `lines` newlines) from the base - * document. - * - '-': Remove the next `chars` characters (containing `lines` newlines) from the base - * document. - * - '+': Insert `chars` characters (containing `lines` newlines) at the current position in - * the document. The inserted characters come from the changeset's character bank. - * - '' (empty string): Invalid operator used in some contexts to signifiy the lack of an - * operation. - * @property {number} chars - The number of characters to keep, insert, or delete. - * @property {number} lines - The number of characters among the `chars` characters that are - * newlines. If non-zero, the last character must be a newline. - * @property {string} attribs - Identifiers of attributes to apply to the text, represented as a - * repeated (zero or more) sequence of asterisk followed by a non-negative base-36 (lower-case) - * integer. For example, '*2*1o' indicates that attributes 2 and 60 apply to the text affected - * by the operation. The identifiers come from the document's attribute pool. This is the empty - * string for remove ('-') operations. For keep ('=') operations, the attributes are merged with - * the base text's existing attributes: - * - A keep op attribute with a non-empty value replaces an existing base text attribute that - * has the same key. - * - A keep op attribute with an empty value is interpreted as an instruction to remove an - * existing base text attribute that has the same key, if one exists. */ +class Op { + /** + * @param {(''|'='|'+'|'-')} [opcode=''] - Initial value of the `opcode` property. + */ + constructor(opcode = '') { + /** + * The operation's operator: + * - '=': Keep the next `chars` characters (containing `lines` newlines) from the base + * document. + * - '-': Remove the next `chars` characters (containing `lines` newlines) from the base + * document. + * - '+': Insert `chars` characters (containing `lines` newlines) at the current position in + * the document. The inserted characters come from the changeset's character bank. + * - '' (empty string): Invalid operator used in some contexts to signifiy the lack of an + * operation. + * + * @type {(''|'='|'+'|'-')} + * @public + */ + this.opcode = opcode; + + /** + * The number of characters to keep, insert, or delete. + * + * @type {number} + * @public + */ + this.chars = 0; + + /** + * The number of characters among the `chars` characters that are newlines. If non-zero, the + * last character must be a newline. + * + * @type {number} + * @public + */ + this.lines = 0; + + /** + * Identifiers of attributes to apply to the text, represented as a repeated (zero or more) + * sequence of asterisk followed by a non-negative base-36 (lower-case) integer. For example, + * '*2*1o' indicates that attributes 2 and 60 apply to the text affected by the operation. The + * identifiers come from the document's attribute pool. + * + * For keep ('=') operations, the attributes are merged with the base text's existing + * attributes: + * - A keep op attribute with a non-empty value replaces an existing base text attribute that + * has the same key. + * - A keep op attribute with an empty value is interpreted as an instruction to remove an + * existing base text attribute that has the same key, if one exists. + * + * This is the empty string for remove ('-') operations. + * + * @type {string} + * @public + */ + this.attribs = ''; + } + + toString() { + if (!this.opcode) throw new TypeError('null op'); + if (typeof this.attribs !== 'string') throw new TypeError('attribs must be a string'); + const l = this.lines ? `|${exports.numToString(this.lines)}` : ''; + return this.attribs + l + this.opcode + exports.numToString(this.chars); + } +} +exports.Op = Op; /** * Describes changes to apply to a document. Does not include the attribute pool or the original @@ -166,8 +209,7 @@ exports.opIterator = (opsStr) => { }; let regexResult = nextRegexMatch(); - const next = (optOp) => { - const op = optOp || exports.newOp(); + const next = (op = new Op()) => { if (regexResult[0]) { op.attribs = regexResult[1]; op.lines = exports.parseNum(regexResult[2] || '0'); @@ -203,15 +245,14 @@ const clearOp = (op) => { /** * Creates a new Op object * + * @deprecated Use the `Op` class instead. * @param {('+'|'-'|'='|'')} [optOpcode=''] - The operation's operator. * @returns {Op} */ -exports.newOp = (optOpcode) => ({ - opcode: (optOpcode || ''), - chars: 0, - lines: 0, - attribs: '', -}); +exports.newOp = (optOpcode) => { + padutils.warnWithStack('Changeset.newOp() is deprecated; use the Changeset.Op class instead'); + return new Op(optOpcode); +}; /** * Copies op1 to op2 @@ -220,7 +261,7 @@ exports.newOp = (optOpcode) => ({ * @param {Op} [op2] - dest Op. If not given, a new Op is used. * @returns {Op} `op2` */ -const copyOp = (op1, op2 = exports.newOp()) => Object.assign(op2, op1); +const copyOp = (op1, op2 = new Op()) => Object.assign(op2, op1); /** * Serializes a sequence of Ops. @@ -257,7 +298,7 @@ const copyOp = (op1, op2 = exports.newOp()) => Object.assign(op2, op1); * @returns {Generator} */ const opsFromText = function* (opcode, text, attribs = '', pool = null) { - const op = exports.newOp(opcode); + const op = new Op(opcode); op.attribs = typeof attribs === 'string' ? attribs : new AttributeMap(pool).update(attribs || [], opcode === '+').toString(); const lastNewlinePos = text.lastIndexOf('\n'); @@ -447,7 +488,7 @@ exports.smartOpAssembler = () => { */ exports.mergingOpAssembler = () => { const assem = exports.opAssembler(); - const bufOp = exports.newOp(); + const bufOp = new Op(); // If we get, for example, insertions [xxx\n,yyy], those don't merge, // but if we get [xxx\n,yyy,zzz\n], that merges to [xxx\nyyyzzz\n]. @@ -523,12 +564,8 @@ exports.opAssembler = () => { * @param {Op} op - Operation to add. Ownership remains with the caller. */ const append = (op) => { - if (!op.opcode) throw new TypeError('null op'); - if (typeof op.attribs !== 'string') throw new TypeError('attribs must be a string'); - serialized += op.attribs; - if (op.lines) serialized += `|${exports.numToString(op.lines)}`; - serialized += op.opcode; - serialized += exports.numToString(op.chars); + assert(op instanceof Op, 'argument must be an instance of Op'); + serialized += op.toString(); }; const toString = () => serialized; @@ -972,8 +1009,8 @@ const applyZip = (in1, in2, func) => { const iter1 = exports.opIterator(in1); const iter2 = exports.opIterator(in2); const assem = exports.smartOpAssembler(); - const op1 = exports.newOp(); - const op2 = exports.newOp(); + const op1 = new Op(); + const op2 = new Op(); while (op1.opcode || iter1.hasNext() || op2.opcode || iter2.hasNext()) { if ((!op1.opcode) && iter1.hasNext()) iter1.next(op1); if ((!op2.opcode) && iter2.hasNext()) iter2.next(op2); @@ -1148,7 +1185,7 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => { * @returns {Op} The result of applying `csOp` to `attOp`. */ const slicerZipperFunc = (attOp, csOp, pool) => { - const opOut = exports.newOp(); + const opOut = new Op(); if (!attOp.opcode) { copyOp(csOp, opOut); csOp.opcode = ''; @@ -1231,7 +1268,7 @@ exports.mutateAttributionLines = (cs, lines, pool) => { const line = mut.removeLines(1); lineIter = exports.opIterator(line); } - if (!lineIter || !lineIter.hasNext()) return exports.newOp(); + if (!lineIter || !lineIter.hasNext()) return new Op(); return lineIter.next(); }; let lineAssem = null; @@ -1248,8 +1285,8 @@ exports.mutateAttributionLines = (cs, lines, pool) => { lineAssem = null; }; - let csOp = exports.newOp(); - let attOp = exports.newOp(); + let csOp = new Op(); + let attOp = new Op(); while (csOp.opcode || csIter.hasNext() || attOp.opcode || isNextMutOp()) { if (!csOp.opcode && csIter.hasNext()) csOp = csIter.next(); if ((!csOp.opcode) && (!attOp.opcode) && (!lineAssem) && (!(lineIter && lineIter.hasNext()))) { @@ -1826,7 +1863,7 @@ exports.attribsAttributeValue = (attribs, key, pool) => { */ exports.builder = (oldLen) => { const assem = exports.smartOpAssembler(); - const o = exports.newOp(); + const o = new Op(); const charBank = exports.stringAssembler(); const self = { @@ -1930,8 +1967,8 @@ exports.makeAttribsString = (opcode, attribs, pool) => { exports.subattribution = (astr, start, optEnd) => { const iter = exports.opIterator(astr); const assem = exports.smartOpAssembler(); - let attOp = exports.newOp(); - const csOp = exports.newOp(); + let attOp = new Op(); + const csOp = new Op(); const doCsOp = () => { if (!csOp.chars) return; @@ -1994,7 +2031,7 @@ exports.inverse = (cs, lines, alines, pool) => { let curChar = 0; let curLineOpIter = null; let curLineOpIterLine; - let curLineNextOp = exports.newOp('+'); + let curLineNextOp = new Op('+'); const unpacked = exports.unpack(cs); const csIter = exports.opIterator(unpacked.ops); @@ -2025,7 +2062,7 @@ exports.inverse = (cs, lines, alines, pool) => { curLineOpIter = exports.opIterator(alinesGet(curLine)); } if (!curLineNextOp.chars) { - curLineNextOp = curLineOpIter.hasNext() ? curLineOpIter.next() : exports.newOp(); + curLineNextOp = curLineOpIter.hasNext() ? curLineOpIter.next() : new Op(); } const charsToUse = Math.min(numChars, curLineNextOp.chars); func(charsToUse, curLineNextOp.attribs, charsToUse === curLineNextOp.chars && @@ -2135,7 +2172,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => { const hasInsertFirst = exports.attributeTester(['insertorder', 'first'], pool); const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2) => { - const opOut = exports.newOp(); + const opOut = new Op(); if (op1.opcode === '+' || op2.opcode === '+') { let whichToDo; if (op2.opcode !== '+') { diff --git a/src/static/js/ace2_inner.js b/src/static/js/ace2_inner.js index 3597959ac..484205c06 100644 --- a/src/static/js/ace2_inner.js +++ b/src/static/js/ace2_inner.js @@ -523,7 +523,7 @@ function Ace2Inner(editorInfo, cssManagers) { const upToLastLine = rep.lines.offsetOfIndex(numLines - 1); const lastLineLength = rep.lines.atIndex(numLines - 1).text.length; const assem = Changeset.smartOpAssembler(); - const o = Changeset.newOp('-'); + const o = new Changeset.Op('-'); o.chars = upToLastLine; o.lines = numLines - 1; assem.append(o); diff --git a/src/static/js/contentcollector.js b/src/static/js/contentcollector.js index 2e6005bd8..7dd70e512 100644 --- a/src/static/js/contentcollector.js +++ b/src/static/js/contentcollector.js @@ -83,7 +83,7 @@ const makeContentCollector = (collectStyles, abrowser, apool, className2Author) const textArray = []; const attribsArray = []; let attribsBuilder = null; - const op = Changeset.newOp('+'); + const op = new Changeset.Op('+'); const self = { length: () => textArray.length, atColumnZero: () => textArray[textArray.length - 1] === '', diff --git a/src/static/js/linestylefilter.js b/src/static/js/linestylefilter.js index f6bcbb54e..19751999c 100644 --- a/src/static/js/linestylefilter.js +++ b/src/static/js/linestylefilter.js @@ -102,7 +102,7 @@ linestylefilter.getLineStyleFilter = (lineLength, aline, textAndClassFunc, apool let nextOp, nextOpClasses; const goNextOp = () => { - nextOp = attributionIter.hasNext() ? attributionIter.next() : Changeset.newOp(); + nextOp = attributionIter.hasNext() ? attributionIter.next() : new Changeset.Op(); nextOpClasses = (nextOp.opcode && attribsToClasses(nextOp.attribs)); }; goNextOp(); diff --git a/src/tests/frontend/specs/easysync.js b/src/tests/frontend/specs/easysync.js index 3ffd2a211..fc56c62a1 100644 --- a/src/tests/frontend/specs/easysync.js +++ b/src/tests/frontend/specs/easysync.js @@ -57,7 +57,7 @@ describe('easysync', function () { const mutationsToChangeset = (oldLen, arrayOfArrays) => { const assem = Changeset.smartOpAssembler(); - const op = Changeset.newOp(); + const op = new Changeset.Op(); const bank = Changeset.stringAssembler(); let oldPos = 0; let newLen = 0; @@ -507,7 +507,7 @@ describe('easysync', function () { const opAssem = Changeset.smartOpAssembler(); const oldLen = origText.length; - const nextOp = Changeset.newOp(); + const nextOp = new Changeset.Op(); const appendMultilineOp = (opcode, txt) => { nextOp.opcode = opcode; @@ -651,7 +651,7 @@ describe('easysync', function () { const testSplitJoinAttributionLines = (randomSeed) => { const stringToOps = (str) => { const assem = Changeset.mergingOpAssembler(); - const o = Changeset.newOp('+'); + const o = new Changeset.Op('+'); o.chars = 1; for (let i = 0; i < str.length; i++) { const c = str.charAt(i);