Addressing the reviewer's comments

- adjusting the tests]
- minor tweaks

app/assets/javascripts/editor/editor_markdown_ext.js

@@ -95,98 +95,3 @@ export class EditorMarkdownExtension extends EditorLiteExtension {
     this.setSelection(newSelection);
   }
 }
-// export default {
-//   getSelectedText(selection = this.getSelection()) {
-//     const { startLineNumber, endLineNumber, startColumn, endColumn } = selection;
-//     const valArray = this.getValue().split('\n');
-//     let text = '';
-//     if (startLineNumber === endLineNumber) {
-//       text = valArray[startLineNumber - 1].slice(startColumn - 1, endColumn - 1);
-//     } else {
-//       const startLineText = valArray[startLineNumber - 1].slice(startColumn - 1);
-//       const endLineText = valArray[endLineNumber - 1].slice(0, endColumn - 1);
-//
-//       for (let i = startLineNumber, k = endLineNumber - 1; i < k; i += 1) {
-//         text += `${valArray[i]}`;
-//         if (i !== k - 1) text += `\n`;
-//       }
-//       text = text
-//         ? [startLineText, text, endLineText].join('\n')
-//         : [startLineText, endLineText].join('\n');
-//     }
-//     return text;
-//   },
-//
-//   replaceSelectedText(text, select = undefined) {
-//     const forceMoveMarkers = !select;
-//     this.executeEdits('', [{ range: this.getSelection(), text, forceMoveMarkers }]);
-//   },
-//
-//   moveCursor(dx = 0, dy = 0) {
-//     const pos = this.getPosition();
-//     pos.column += dx;
-//     pos.lineNumber += dy;
-//     this.setPosition(pos);
-//   },
-//
-//   /**
-//    * Adjust existing selection to select text within the original selection.
-//    * - If `selectedText` is not supplied, we fetch selected text with
-//    *
-//    * ALGORITHM:
-//    *
-//    * MULTI-LINE SELECTION
-//    * 1. Find line that contains `toSelect` text.
-//    * 2. Using the index of this line and the position of `toSelect` text in it,
-//    * construct:
-//    *   * newStartLineNumber
-//    *   * newStartColumn
-//    *
-//    * SINGLE-LINE SELECTION
-//    * 1. Use `startLineNumber` from the current selection as `newStartLineNumber`
-//    * 2. Find the position of `toSelect` text in it to get `newStartColumn`
-//    *
-//    * 3. `newEndLineNumber` — Since this method is supposed to be used with
-//    * markdown decorators that are pretty short, the `newEndLineNumber` is
-//    * suggested to be assumed the same as the startLine.
-//    * 4. `newEndColumn` — pretty obvious
-//    * 5. Adjust the start and end positions of the current selection
-//    * 6. Re-set selection on the instance
-//    *
-//    * @param {string} toSelect - New text to select within current selection.
-//    * @param {string} selectedText - Currently selected text. It's just a
-//    * shortcut: If it's not supplied, we fetch selected text from the instance
-//    */
-//   selectWithinSelection(toSelect, selectedText) {
-//     const currentSelection = this.getSelection();
-//     if (currentSelection.isEmpty() || !toSelect) {
-//       return;
-//     }
-//     const text = selectedText || this.getSelectedText(currentSelection);
-//     let lineShift;
-//     let newStartLineNumber;
-//     let newStartColumn;
-//
-//     const textLines = text.split('\n');
-//
-//     if (textLines.length > 1) {
-//       // Multi-line selection
-//       lineShift = textLines.findIndex(line => line.indexOf(toSelect) !== -1);
-//       newStartLineNumber = currentSelection.startLineNumber + lineShift;
-//       newStartColumn = textLines[lineShift].indexOf(toSelect) + 1;
-//     } else {
-//       // Single-line selection
-//       newStartLineNumber = currentSelection.startLineNumber;
-//       newStartColumn = currentSelection.startColumn + text.indexOf(toSelect);
-//     }
-//
-//     const newEndLineNumber = newStartLineNumber;
-//     const newEndColumn = newStartColumn + toSelect.length;
-//
-//     const newSelection = currentSelection
-//       .setStartPosition(newStartLineNumber, newStartColumn)
-//       .setEndPosition(newEndLineNumber, newEndColumn);
-//
-//     this.setSelection(newSelection);
-//   },
-// };

spec/frontend/editor/editor_lite_extension_base_spec.js

@@ -2,25 +2,43 @@ import { ERROR_INSTANCE_REQUIRED_FOR_EXTENSION } from '~/editor/constants';
 import { EditorLiteExtension } from '~/editor/editor_lite_extension_base';
 
 describe('The basis for an Editor Lite extension', () => {
-  const instance = {};
   let ext;
+  const defaultOptions = { foo: 'bar' };
 
-  it('accepts configuration options for an instance', () => {
-    expect(instance.foo).toBeUndefined();
-    ext = new EditorLiteExtension({ instance, foo: 'bar' });
-    expect(ext.foo).toBeUndefined();
-    expect(instance.foo).toBe('bar');
-  });
+  it.each`
+    description                                                     | instance     | options
+    ${'accepts configuration options and instance'}                 | ${{}}        | ${defaultOptions}
+    ${'leaves instance intact if no options are passed'}            | ${{}}        | ${undefined}
+    ${'does not fail if both instance and the options are omitted'} | ${undefined} | ${undefined}
+    ${'throws if only options are passed'}                          | ${undefined} | ${defaultOptions}
+  `('$description', ({ instance, options } = {}) => {
+    const originalInstance = { ...instance };
 
-  it('throws if only options are passed', () => {
+    if (instance) {
+      if (options) {
+        Object.entries(options).forEach(prop => {
+          expect(instance[prop]).toBeUndefined();
+        });
+        // Both instance and options are passed
+        ext = new EditorLiteExtension({ instance, ...options });
+        Object.entries(options).forEach(([prop, value]) => {
+          expect(ext[prop]).toBeUndefined();
+          expect(instance[prop]).toBe(value);
+        });
+      } else {
+        ext = new EditorLiteExtension({ instance });
+        expect(instance).toEqual(originalInstance);
+      }
+    } else if (options) {
+      // Options are passed without instance
       expect(() => {
-      ext = new EditorLiteExtension({ foo: 'bar' });
+        ext = new EditorLiteExtension({ ...options });
       }).toThrow(ERROR_INSTANCE_REQUIRED_FOR_EXTENSION);
-  });
-
-  it('does not fail if both instance and the options are omitted', () => {
+    } else {
+      // Neither options nor instance are passed
       expect(() => {
         ext = new EditorLiteExtension();
       }).not.toThrow();
+    }
   });
 });

spec/frontend/editor/editor_lite_spec.js

@@ -2,6 +2,7 @@
 import { editor as monacoEditor, languages as monacoLanguages, Uri } from 'monaco-editor';
 import waitForPromises from 'helpers/wait_for_promises';
 import Editor from '~/editor/editor_lite';
+import { EditorLiteExtension } from '~/editor/editor_lite_extension_base';
 import { DEFAULT_THEME, themes } from '~/ide/lib/themes';
 import { EDITOR_LITE_INSTANCE_ERROR_NO_EL, URI_PREFIX } from '~/editor/constants';
 
@@ -260,6 +261,25 @@ describe('Base editor', () => {
         return this?.nonExistentProp || betaRes;
       }
     }
+    class WithStaticMethod {
+      constructor({ instance: inst, ...options } = {}) {
+        Object.assign(inst, options);
+      }
+      static computeBoo(a) {
+        return a + 1;
+      }
+      boo() {
+        return WithStaticMethod.computeBoo(this.base);
+      }
+    }
+    class WithStaticMethodExtended extends EditorLiteExtension {
+      static computeBoo(a) {
+        return a + 1;
+      }
+      boo() {
+        return WithStaticMethodExtended.computeBoo(this.base);
+      }
+    }
     const AlphaExt = new AlphaClass();
     const BetaExt = new BetaClass();
     const FooObjExt = {
@@ -319,15 +339,46 @@ describe('Base editor', () => {
         });
       });
 
+      it('does not extend instance with private data of an extension', () => {
+        const ext = new WithStaticMethod({ instance });
+        ext.staticMethod = () => {
+          return 'foo';
+        };
+        ext.staticProp = 'bar';
+
+        expect(instance.boo).toBeUndefined();
+        expect(instance.staticMethod).toBeUndefined();
+        expect(instance.staticProp).toBeUndefined();
+
+        instance.use(ext);
+
+        expect(instance.boo).toBeDefined();
+        expect(instance.staticMethod).toBeUndefined();
+        expect(instance.staticProp).toBeUndefined();
+      });
+
+      it.each([WithStaticMethod, WithStaticMethodExtended])(
+        'properly resolves data for an extension with private data',
+        ExtClass => {
+          const base = 1;
+          expect(instance.base).toBeUndefined();
+          expect(instance.boo).toBeUndefined();
+
+          const ext = new ExtClass({ instance, base });
+
+          instance.use(ext);
+          expect(instance.base).toBe(1);
+          expect(instance.boo()).toBe(2);
+        },
+      );
+
       it('uses the last definition of a method in case of an overlap', () => {
         const FooObjExt2 = { foo: 'foo2' };
         instance.use([FooObjExt, BarObjExt, FooObjExt2]);
-        expect(instance).toEqual(
-          expect.objectContaining({
+        expect(instance).toMatchObject({
           foo: 'foo2',
           ...BarObjExt,
-          }),
-        );
+        });
       });
 
       it('correctly resolves references withing extensions', () => {