From 4a65c2c8ff6036222a2b2c48e5161c82c199e8c9 Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Sun, 21 Mar 2021 14:13:50 -0400 Subject: [PATCH] Changeset: Unexport unnecessarily exported functions These functions aren't used outside of this file. --- CHANGELOG.md | 4 + src/static/js/Changeset.js | 121 ++++++++++++++------------- src/tests/frontend/specs/easysync.js | 14 ++-- 3 files changed, 75 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ce91f39d..0cf6d1ded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,10 @@ * Changes to the `src/static/js/Changeset.js` library: * `opIterator()`: The unused start index parameter has been removed, as has the unused `lastIndex()` method on the returned object. + * Several functions that should have never been public are no longer + exported: `applyZip()`, `assert()`, `clearOp()`, `cloneOp()`, `copyOp()`, + `error()`, `followAttributes()`, `opString()`, `stringOp()`, + `textLinesMutator()`, `toBaseTen()`, `toSplices()`. ### Notable enhancements diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index f43a236cf..fa1962340 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -35,7 +35,7 @@ const AttributePool = require('./AttributePool'); * * @param {string} msg - Just some message */ -exports.error = (msg) => { +const error = (msg) => { const e = new Error(msg); e.easysync = true; throw e; @@ -49,8 +49,8 @@ exports.error = (msg) => { * @param {string} msg - error message to include in the exception * @type {(b: boolean, msg: string) => asserts b} */ -exports.assert = (b, msg) => { - if (!b) exports.error(`Failed assertion: ${msg}`); +const assert = (b, msg) => { + if (!b) error(`Failed assertion: ${msg}`); }; /** @@ -147,7 +147,7 @@ exports.opIterator = (opsStr) => { const nextRegexMatch = () => { const result = regex.exec(opsStr); if (result[0] === '?') { - exports.error('Hit error opcode in op stream'); + error('Hit error opcode in op stream'); } return result; @@ -163,7 +163,7 @@ exports.opIterator = (opsStr) => { op.chars = exports.parseNum(regexResult[4]); regexResult = nextRegexMatch(); } else { - exports.clearOp(op); + clearOp(op); } return op; }; @@ -181,7 +181,7 @@ exports.opIterator = (opsStr) => { * * @param {Op} op - object to clear */ -exports.clearOp = (op) => { +const clearOp = (op) => { op.opcode = ''; op.chars = 0; op.lines = 0; @@ -207,7 +207,7 @@ exports.newOp = (optOpcode) => ({ * @param {Op} op1 - src Op * @param {Op} op2 - dest Op */ -exports.copyOp = (op1, op2) => { +const copyOp = (op1, op2) => { op2.opcode = op1.opcode; op2.chars = op1.chars; op2.lines = op1.lines; @@ -280,13 +280,13 @@ exports.checkRep = (cs) => { break; case '-': oldPos += o.chars; - exports.assert(oldPos <= oldLen, `${oldPos} > ${oldLen} in ${cs}`); + assert(oldPos <= oldLen, `${oldPos} > ${oldLen} in ${cs}`); break; case '+': { calcNewLen += o.chars; numInserted += o.chars; - exports.assert(calcNewLen <= newLen, `${calcNewLen} > ${newLen} in ${cs}`); + assert(calcNewLen <= newLen, `${calcNewLen} > ${newLen} in ${cs}`); break; } } @@ -301,7 +301,7 @@ exports.checkRep = (cs) => { assem.endDocument(); const normalized = exports.pack(oldLen, calcNewLen, assem.toString(), charBank); - exports.assert(normalized === cs, 'Invalid changeset (checkRep failed)'); + assert(normalized === cs, 'Invalid changeset (checkRep failed)'); return cs; }; @@ -458,7 +458,7 @@ exports.mergingOpAssembler = () => { } } else { flush(); - exports.copyOp(op, bufOp); + copyOp(op, bufOp); } } }; @@ -474,7 +474,7 @@ exports.mergingOpAssembler = () => { const clear = () => { assem.clear(); - exports.clearOp(bufOp); + clearOp(bufOp); }; return { append, @@ -536,7 +536,7 @@ exports.stringIterator = (str) => { const getnewLines = () => newLines; const assertRemaining = (n) => { - exports.assert(n <= remaining(), `!(${n} <= ${remaining()})`); + assert(n <= remaining(), `!(${n} <= ${remaining()})`); }; const take = (n) => { @@ -633,7 +633,7 @@ exports.stringAssembler = () => { * @param {(string[]|StringArrayLike)} lines - Lines to mutate (in place). * @returns {TextLinesMutator} */ -exports.textLinesMutator = (lines) => { +const textLinesMutator = (lines) => { /** * curSplice holds values that will be passed as arguments to lines.splice() to insert, delete, or * change lines: @@ -990,7 +990,7 @@ exports.textLinesMutator = (lines) => { * other out), `opOut.opcode` MUST be set to the empty string. * @returns {string} the integrated changeset */ -exports.applyZip = (in1, in2, func) => { +const applyZip = (in1, in2, func) => { const iter1 = exports.opIterator(in1); const iter2 = exports.opIterator(in2); const assem = exports.smartOpAssembler(); @@ -1020,7 +1020,7 @@ exports.unpack = (cs) => { const headerRegex = /Z:([0-9a-z]+)([><])([0-9a-z]+)|/; const headerMatch = headerRegex.exec(cs); if ((!headerMatch) || (!headerMatch[0])) { - exports.error(`Not a exports: ${cs}`); + error(`Not a exports: ${cs}`); } const oldLen = exports.parseNum(headerMatch[1]); const changeSign = (headerMatch[2] === '>') ? 1 : -1; @@ -1064,8 +1064,7 @@ exports.pack = (oldLen, newLen, opsStr, bank) => { */ exports.applyToText = (cs, str) => { const unpacked = exports.unpack(cs); - exports.assert( - str.length === unpacked.oldLen, `mismatched apply: ${str.length} / ${unpacked.oldLen}`); + assert(str.length === unpacked.oldLen, `mismatched apply: ${str.length} / ${unpacked.oldLen}`); const csIter = exports.opIterator(unpacked.ops); const bankIter = exports.stringIterator(unpacked.charBank); const strIter = exports.stringIterator(str); @@ -1113,7 +1112,7 @@ exports.mutateTextLines = (cs, lines) => { const unpacked = exports.unpack(cs); const csIter = exports.opIterator(unpacked.ops); const bankIter = exports.stringIterator(unpacked.charBank); - const mut = exports.textLinesMutator(lines); + const mut = textLinesMutator(lines); while (csIter.hasNext()) { const op = csIter.next(); switch (op.opcode) { @@ -1205,12 +1204,12 @@ exports.composeAttributes = (att1, att2, resultIsMutation, pool) => { * @param {Op} opOut - Mutated to hold the result of applying `csOp` to `attOp`. * @param {AttributePool} pool - Can be null if definitely not needed. */ -exports._slicerZipperFunc = (attOp, csOp, opOut, pool) => { +const slicerZipperFunc = (attOp, csOp, opOut, pool) => { if (attOp.opcode === '-') { - exports.copyOp(attOp, opOut); + copyOp(attOp, opOut); attOp.opcode = ''; } else if (!attOp.opcode) { - exports.copyOp(csOp, opOut); + copyOp(csOp, opOut); csOp.opcode = ''; } else { switch (csOp.opcode) { @@ -1247,7 +1246,7 @@ exports._slicerZipperFunc = (attOp, csOp, opOut, pool) => { case '+': { // insert - exports.copyOp(csOp, opOut); + copyOp(csOp, opOut); csOp.opcode = ''; break; } @@ -1281,7 +1280,7 @@ exports._slicerZipperFunc = (attOp, csOp, opOut, pool) => { } case '': { - exports.copyOp(attOp, opOut); + copyOp(attOp, opOut); attOp.opcode = ''; break; } @@ -1300,8 +1299,8 @@ exports._slicerZipperFunc = (attOp, csOp, opOut, pool) => { exports.applyToAttribution = (cs, astr, pool) => { const unpacked = exports.unpack(cs); - return exports.applyZip(astr, unpacked.ops, - (op1, op2, opOut) => exports._slicerZipperFunc(op1, op2, opOut, pool)); + return applyZip(astr, unpacked.ops, + (op1, op2, opOut) => slicerZipperFunc(op1, op2, opOut, pool)); }; exports.mutateAttributionLines = (cs, lines, pool) => { @@ -1310,7 +1309,7 @@ exports.mutateAttributionLines = (cs, lines, pool) => { const csBank = unpacked.charBank; let csBankIndex = 0; // treat the attribution lines as text lines, mutating a line at a time - const mut = exports.textLinesMutator(lines); + const mut = textLinesMutator(lines); /** @type {?OpIter} */ let lineIter = null; @@ -1336,7 +1335,7 @@ exports.mutateAttributionLines = (cs, lines, pool) => { } lineAssem.append(op); if (op.lines > 0) { - exports.assert(op.lines === 1, `Can't have op.lines of ${op.lines} in attribution lines`); + assert(op.lines === 1, `Can't have op.lines of ${op.lines} in attribution lines`); // ship it to the mut mut.insert(lineAssem.toString(), 1); lineAssem = null; @@ -1360,13 +1359,13 @@ exports.mutateAttributionLines = (cs, lines, pool) => { } else if (csOp.opcode === '+') { if (csOp.lines > 1) { const firstLineLen = csBank.indexOf('\n', csBankIndex) + 1 - csBankIndex; - exports.copyOp(csOp, opOut); + copyOp(csOp, opOut); csOp.chars -= firstLineLen; csOp.lines--; opOut.lines = 1; opOut.chars = firstLineLen; } else { - exports.copyOp(csOp, opOut); + copyOp(csOp, opOut); csOp.opcode = ''; } outputMutOp(opOut); @@ -1376,7 +1375,7 @@ exports.mutateAttributionLines = (cs, lines, pool) => { if ((!attOp.opcode) && isNextMutOp()) { nextMutOp(attOp); } - exports._slicerZipperFunc(attOp, csOp, opOut, pool); + slicerZipperFunc(attOp, csOp, opOut, pool); if (opOut.opcode) { outputMutOp(opOut); opOut.opcode = ''; @@ -1384,7 +1383,7 @@ exports.mutateAttributionLines = (cs, lines, pool) => { } } - exports.assert(!lineAssem, `line assembler not finished:${cs}`); + assert(!lineAssem, `line assembler not finished:${cs}`); mut.close(); }; @@ -1427,7 +1426,7 @@ exports.splitAttributionLines = (attrOps, text) => { let numLines = op.lines; while (numLines > 1) { const newlineEnd = text.indexOf('\n', pos) + 1; - exports.assert(newlineEnd > 0, 'newlineEnd <= 0 in splitAttributionLines'); + assert(newlineEnd > 0, 'newlineEnd <= 0 in splitAttributionLines'); op.chars = newlineEnd - pos; op.lines = 1; appendOp(op); @@ -1465,19 +1464,19 @@ exports.compose = (cs1, cs2, pool) => { const unpacked2 = exports.unpack(cs2); const len1 = unpacked1.oldLen; const len2 = unpacked1.newLen; - exports.assert(len2 === unpacked2.oldLen, 'mismatched composition of two changesets'); + assert(len2 === unpacked2.oldLen, 'mismatched composition of two changesets'); const len3 = unpacked2.newLen; const bankIter1 = exports.stringIterator(unpacked1.charBank); const bankIter2 = exports.stringIterator(unpacked2.charBank); const bankAssem = exports.stringAssembler(); - const newOps = exports.applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => { + const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => { const op1code = op1.opcode; const op2code = op2.opcode; if (op1code === '+' && op2code === '-') { bankIter1.skip(Math.min(op1.chars, op2.chars)); } - exports._slicerZipperFunc(op1, op2, opOut, pool); + slicerZipperFunc(op1, op2, opOut, pool); if (opOut.opcode === '+') { if (op2code === '+') { bankAssem.append(bankIter2.take(opOut.chars)); @@ -1560,7 +1559,7 @@ exports.makeSplice = (oldFullText, spliceStart, numRemoved, newText, optNewTextA * @param {string} cs - Changeset * @returns {[number, number, string][]} */ -exports.toSplices = (cs) => { +const toSplices = (cs) => { const unpacked = exports.unpack(cs); /** @type {[number, number, string][]} */ const splices = []; @@ -1601,7 +1600,7 @@ exports.toSplices = (cs) => { exports.characterRangeFollow = (cs, startChar, endChar, insertionsAfter) => { let newStartChar = startChar; let newEndChar = endChar; - const splices = exports.toSplices(cs); + const splices = toSplices(cs); let lengthChangeSoFar = 0; for (let i = 0; i < splices.length; i++) { const splice = splices[i]; @@ -1796,7 +1795,9 @@ exports.cloneAText = (atext) => { text: atext.text, attribs: atext.attribs, }; - } else { exports.error('atext is null'); } + } else { + error('atext is null'); + } }; /** @@ -2040,7 +2041,7 @@ exports.subattribution = (astr, start, optEnd) => { csOp.lines++; } - exports._slicerZipperFunc(attOp, csOp, opOut, null); + slicerZipperFunc(attOp, csOp, opOut, null); if (opOut.opcode) { assem.append(opOut); opOut.opcode = ''; @@ -2240,7 +2241,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => { const unpacked2 = exports.unpack(cs2); const len1 = unpacked1.oldLen; const len2 = unpacked2.oldLen; - exports.assert(len1 === len2, 'mismatched follow - cannot transform cs1 on top of cs2'); + assert(len1 === len2, 'mismatched follow - cannot transform cs1 on top of cs2'); const chars1 = exports.stringIterator(unpacked1.charBank); const chars2 = exports.stringIterator(unpacked2.charBank); @@ -2250,7 +2251,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => { const hasInsertFirst = exports.attributeTester(['insertorder', 'first'], pool); - const newOps = exports.applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => { + const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => { if (op1.opcode === '+' || op2.opcode === '+') { let whichToDo; if (op2.opcode !== '+') { @@ -2289,7 +2290,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => { } else { // whichToDo == 2 chars2.skip(op2.chars); - exports.copyOp(op2, opOut); + copyOp(op2, opOut); op2.opcode = ''; } } else if (op1.opcode === '-') { @@ -2308,7 +2309,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => { op2.opcode = ''; } } else if (op2.opcode === '-') { - exports.copyOp(op2, opOut); + copyOp(op2, opOut); if (!op1.opcode) { op2.opcode = ''; } else if (op2.chars <= op1.chars) { @@ -2328,17 +2329,17 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => { op1.opcode = ''; } } else if (!op1.opcode) { - exports.copyOp(op2, opOut); + copyOp(op2, opOut); op2.opcode = ''; } else if (!op2.opcode) { // @NOTE: Critical bugfix for EPL issue #1625. We do not copy op1 here // in order to prevent attributes from leaking into result changesets. - // exports.copyOp(op1, opOut); + // copyOp(op1, opOut); op1.opcode = ''; } else { // both keeps opOut.opcode = '='; - opOut.attribs = exports.followAttributes(op1.attribs, op2.attribs, pool); + opOut.attribs = followAttributes(op1.attribs, op2.attribs, pool); if (op1.chars <= op2.chars) { opOut.chars = op1.chars; opOut.lines = op1.lines; @@ -2374,7 +2375,7 @@ exports.follow = (cs1, cs2, reverseInsertOrder, pool) => { return exports.pack(oldLen, newLen, newOps, unpacked2.charBank); }; -exports.followAttributes = (att1, att2, pool) => { +const followAttributes = (att1, att2, pool) => { // The merge of two sets of attribute changes to the same text // takes the lexically-earlier value if there are two values // for the same key. Otherwise, all key/value changes from @@ -2416,19 +2417,19 @@ exports.composeWithDeletions = (cs1, cs2, pool) => { const unpacked2 = exports.unpack(cs2); const len1 = unpacked1.oldLen; const len2 = unpacked1.newLen; - exports.assert(len2 === unpacked2.oldLen, 'mismatched composition of two changesets'); + assert(len2 === unpacked2.oldLen, 'mismatched composition of two changesets'); const len3 = unpacked2.newLen; const bankIter1 = exports.stringIterator(unpacked1.charBank); const bankIter2 = exports.stringIterator(unpacked2.charBank); const bankAssem = exports.stringAssembler(); - const newOps = exports.applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => { + const newOps = applyZip(unpacked1.ops, unpacked2.ops, (op1, op2, opOut) => { const op1code = op1.opcode; const op2code = op2.opcode; if (op1code === '+' && op2code === '-') { bankIter1.skip(Math.min(op1.chars, op2.chars)); } - exports._slicerZipperFuncWithDeletions(op1, op2, opOut, pool); + slicerZipperFuncWithDeletions(op1, op2, opOut, pool); if (opOut.opcode === '+') { if (op2code === '+') { bankAssem.append(bankIter2.take(opOut.chars)); @@ -2441,19 +2442,19 @@ exports.composeWithDeletions = (cs1, cs2, pool) => { return exports.pack(len1, len3, newOps, bankAssem.toString()); }; -// This function is 95% like _slicerZipperFunc, we just changed two lines to +// This function is 95% like slicerZipperFunc, we just changed two lines to // ensure it merges the attribs of deletions properly. // This is necassary for correct paddiff. But to ensure these changes doesn't // affect anything else, we've created a seperate function only used for paddiffs -exports._slicerZipperFuncWithDeletions = (attOp, csOp, opOut, pool) => { +const slicerZipperFuncWithDeletions = (attOp, csOp, opOut, pool) => { // attOp is the op from the sequence that is being operated on, either an // attribution string or the earlier of two exportss being composed. // pool can be null if definitely not needed. if (attOp.opcode === '-') { - exports.copyOp(attOp, opOut); + copyOp(attOp, opOut); attOp.opcode = ''; } else if (!attOp.opcode) { - exports.copyOp(csOp, opOut); + copyOp(csOp, opOut); csOp.opcode = ''; } else { switch (csOp.opcode) { @@ -2490,7 +2491,7 @@ exports._slicerZipperFuncWithDeletions = (attOp, csOp, opOut, pool) => { case '+': { // insert - exports.copyOp(csOp, opOut); + copyOp(csOp, opOut); csOp.opcode = ''; break; } @@ -2524,10 +2525,16 @@ exports._slicerZipperFuncWithDeletions = (attOp, csOp, opOut, pool) => { } case '': { - exports.copyOp(attOp, opOut); + copyOp(attOp, opOut); attOp.opcode = ''; break; } } } }; + +exports.exportedForTestingOnly = { + followAttributes, + textLinesMutator, + toSplices, +}; diff --git a/src/tests/frontend/specs/easysync.js b/src/tests/frontend/specs/easysync.js index 5c4c47ae4..121218407 100644 --- a/src/tests/frontend/specs/easysync.js +++ b/src/tests/frontend/specs/easysync.js @@ -92,7 +92,7 @@ describe('easysync', function () { const runMutationTest = (testId, origLines, muts, correct) => { it(`runMutationTest#${testId}`, async function () { let lines = origLines.slice(); - const mu = Changeset.textLinesMutator(lines); + const mu = Changeset.exportedForTestingOnly.textLinesMutator(lines); applyMutations(mu, muts); mu.close(); expect(lines).to.eql(correct); @@ -211,7 +211,7 @@ describe('easysync', function () { const lines = ['1\n', '2\n', '3\n', '4\n']; let mu; - mu = Changeset.textLinesMutator(lines); + mu = Changeset.exportedForTestingOnly.textLinesMutator(lines); expect(mu.hasMore()).to.be(true); mu.skip(8, 4); expect(mu.hasMore()).to.be(false); @@ -219,7 +219,7 @@ describe('easysync', function () { expect(mu.hasMore()).to.be(false); // still 1,2,3,4 - mu = Changeset.textLinesMutator(lines); + mu = Changeset.exportedForTestingOnly.textLinesMutator(lines); expect(mu.hasMore()).to.be(true); mu.remove(2, 1); expect(mu.hasMore()).to.be(true); @@ -235,7 +235,7 @@ describe('easysync', function () { expect(mu.hasMore()).to.be(false); // 2,3,4,5 now - mu = Changeset.textLinesMutator(lines); + mu = Changeset.exportedForTestingOnly.textLinesMutator(lines); expect(mu.hasMore()).to.be(true); mu.remove(6, 3); expect(mu.hasMore()).to.be(true); @@ -610,8 +610,8 @@ describe('easysync', function () { const testFollow = (a, b, afb, bfa, merge) => { it(`testFollow manual #${++n}`, async function () { - expect(Changeset.followAttributes(a, b, p)).to.equal(afb); - expect(Changeset.followAttributes(b, a, p)).to.equal(bfa); + expect(Changeset.exportedForTestingOnly.followAttributes(a, b, p)).to.equal(afb); + expect(Changeset.exportedForTestingOnly.followAttributes(b, a, p)).to.equal(bfa); expect(Changeset.composeAttributes(a, afb, true, p)).to.equal(merge); expect(Changeset.composeAttributes(b, bfa, true, p)).to.equal(merge); }); @@ -701,7 +701,7 @@ describe('easysync', function () { [5, 8, '123456789'], [9, 17, 'abcdefghijk'], ]; - expect(Changeset.toSplices(cs)).to.eql(correctSplices); + expect(Changeset.exportedForTestingOnly.toSplices(cs)).to.eql(correctSplices); }); const testCharacterRangeFollow = (testId, cs, oldRange, insertionsAfter, correctNewRange) => {