From eae814f5a066b0ea24361bf2e13ea9620a37ec6a Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Mon, 25 Oct 2021 01:18:17 -0400 Subject: [PATCH 1/2] pad_utils: New `warnWithStack()` function --- src/static/js/pad_utils.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/static/js/pad_utils.js b/src/static/js/pad_utils.js index 6fb46fde0e0..9bea959da88 100644 --- a/src/static/js/pad_utils.js +++ b/src/static/js/pad_utils.js @@ -89,6 +89,25 @@ const urlRegex = (() => { })(); const padutils = { + /** + * Prints a warning message followed by a stack trace (to make it easier to figure out what code + * is using the deprecated function). + * + * Most browsers include UI widget to examine the stack at the time of the warning, but this + * includes the stack in the log message for a couple of reasons: + * - This makes it possible to see the stack if the code runs in Node.js. + * - Users are more likely to paste the stack in bug reports they might file. + * + * @param {...*} args - Passed to `console.warn`, with a stack trace appended. + */ + warnWithStack: (...args) => { + const err = new Error(); + if (Error.captureStackTrace) Error.captureStackTrace(err, padutils.warnWithStack); + err.name = ''; + if (err.stack) args.push(err.stack); + console.warn(...args); + }, + escapeHtml: (x) => Security.escapeHTML(String(x)), uniqueId: () => { const pad = require('./pad').pad; // Sidestep circular dependency From a65498e849c9d49b33a008c44af0ba3f8d2ba62c Mon Sep 17 00:00:00 2001 From: Richard Hansen Date: Wed, 13 Oct 2021 00:19:09 -0400 Subject: [PATCH 2/2] Changeset: Move `SmartOpAssembler.appendOpWithText()` to a standalone function --- CHANGELOG.md | 3 ++ src/static/js/Changeset.js | 65 ++++++++++++++++++++++++++------------ 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cf6d1ded35..98ccb583d06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,9 @@ * 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. + * `smartOpAssembler()`: The returned object's `appendOpWithText()` method is + deprecated without a replacement available to plugins (if you need one, + let us know and we can make the private `opsFromText()` function public). * Several functions that should have never been public are no longer exported: `applyZip()`, `assert()`, `clearOp()`, `cloneOp()`, `copyOp()`, `error()`, `followAttributes()`, `opString()`, `stringOp()`, diff --git a/src/static/js/Changeset.js b/src/static/js/Changeset.js index 21de89c43da..a335f7ccb88 100644 --- a/src/static/js/Changeset.js +++ b/src/static/js/Changeset.js @@ -23,6 +23,7 @@ */ const AttributePool = require('./AttributePool'); +const {padutils} = require('./pad_utils'); /** * A `[key, value]` pair of strings describing a text attribute. @@ -230,6 +231,36 @@ const copyOp = (op1, op2 = exports.newOp()) => Object.assign(op2, op1); * @property {Function} toString - */ +/** + * Generates operations from the given text and attributes. + * + * @param {('-'|'+'|'=')} opcode - The operator to use. + * @param {string} text - The text to remove/add/keep. + * @param {(string|Attribute[])} [attribs] - The attributes to apply to the operations. See + * `makeAttribsString`. + * @param {?AttributePool} [pool] - See `makeAttribsString`. + * @yields {Op} One or two ops (depending on the presense of newlines) that cover the given text. + * @returns {Generator} + */ +const opsFromText = function* (opcode, text, attribs = '', pool = null) { + const op = exports.newOp(opcode); + op.attribs = exports.makeAttribsString(opcode, attribs, pool); + const lastNewlinePos = text.lastIndexOf('\n'); + if (lastNewlinePos < 0) { + op.chars = text.length; + op.lines = 0; + yield op; + } else { + op.chars = lastNewlinePos + 1; + op.lines = text.match(/\n/g).length; + yield op; + const op2 = copyOp(op); + op2.chars = text.length - (lastNewlinePos + 1); + op2.lines = 0; + yield op2; + } +}; + /** * Creates an object that allows you to append operations (type Op) and also compresses them if * possible. Like MergingOpAssembler, but able to produce conforming exportss from slightly looser @@ -353,6 +384,7 @@ exports.smartOpAssembler = () => { /** * Generates operations from the given text and attributes. * + * @deprecated Use `opsFromText` instead. * @param {('-'|'+'|'=')} opcode - The operator to use. * @param {string} text - The text to remove/add/keep. * @param {(string|Attribute[])} attribs - The attributes to apply to the operations. See @@ -360,21 +392,9 @@ exports.smartOpAssembler = () => { * @param {?AttributePool} pool - See `makeAttribsString`. */ const appendOpWithText = (opcode, text, attribs, pool) => { - const op = exports.newOp(opcode); - op.attribs = exports.makeAttribsString(opcode, attribs, pool); - const lastNewlinePos = text.lastIndexOf('\n'); - if (lastNewlinePos < 0) { - op.chars = text.length; - op.lines = 0; - append(op); - } else { - op.chars = lastNewlinePos + 1; - op.lines = text.match(/\n/g).length; - append(op); - op.chars = text.length - (lastNewlinePos + 1); - op.lines = 0; - append(op); - } + padutils.warnWithStack('Changeset.smartOpAssembler().appendOpWithText() is deprecated; ' + + 'use opsFromText() instead.'); + for (const op of opsFromText(opcode, text, attribs, pool)) append(op); }; const toString = () => { @@ -1450,9 +1470,12 @@ exports.makeSplice = (oldFullText, spliceStart, numRemoved, newText, optNewTextA const newLen = oldLen + newText.length - oldText.length; const assem = exports.smartOpAssembler(); - assem.appendOpWithText('=', oldFullText.substring(0, spliceStart)); - assem.appendOpWithText('-', oldText); - assem.appendOpWithText('+', newText, optNewTextAPairs, pool); + const ops = (function* () { + yield* opsFromText('=', oldFullText.substring(0, spliceStart)); + yield* opsFromText('-', oldText); + yield* opsFromText('+', newText, optNewTextAPairs, pool); + })(); + for (const op of ops) assem.append(op); assem.endDocument(); return exports.pack(oldLen, newLen, assem.toString(), newText); }; @@ -1580,7 +1603,7 @@ exports.moveOpsToNewPool = (cs, oldPool, newPool) => { */ exports.makeAttribution = (text) => { const assem = exports.smartOpAssembler(); - assem.appendOpWithText('+', text); + for (const op of opsFromText('+', text)) assem.append(op); return assem.toString(); }; @@ -1839,7 +1862,7 @@ exports.builder = (oldLen) => { * @returns {Builder} this */ keepText: (text, attribs, pool) => { - assem.appendOpWithText('=', text, attribs, pool); + for (const op of opsFromText('=', text, attribs, pool)) assem.append(op); return self; }, @@ -1852,7 +1875,7 @@ exports.builder = (oldLen) => { * @returns {Builder} this */ insert: (text, attribs, pool) => { - assem.appendOpWithText('+', text, attribs, pool); + for (const op of opsFromText('+', text, attribs, pool)) assem.append(op); charBank.append(text); return self; },