Changeset: Move SmartOpAssembler.appendOpWithText() to a standalone function

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()`,

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<Op>}
+ */
+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,28 +384,17 @@ 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
    *     `makeAttribsString`.
    * @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;
     },

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