From 638062e982769553fead0db6cb838a5c1caf7b9c Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 8 Apr 2019 15:20:14 -0700 Subject: [PATCH 01/14] pass `blockInfo` to extension methods Sometimes a single extension method needs to service several different instances of the same opcode. This can happen with variables or custom procedures, for example. This change allows the extension method to inspect the `blockInfo` for instance data, including arbitrary extension-specific properties if necessary. --- src/extension-support/extension-manager.js | 27 +++++++++++----------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js index bf666a28c..e70929b63 100644 --- a/src/extension-support/extension-manager.js +++ b/src/extension-support/extension-manager.js @@ -391,23 +391,22 @@ class ExtensionManager { } break; default: - if (!blockInfo.opcode) { - throw new Error('Missing opcode for block'); - } + if (blockInfo.opcode) { + const funcName = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; - blockInfo.func = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; - - // Avoid promise overhead if possible - if (dispatch._isRemoteService(serviceName)) { - blockInfo.func = dispatch.call.bind(dispatch, serviceName, blockInfo.func); - } else { - const serviceObject = dispatch.services[serviceName]; - const func = serviceObject[blockInfo.func]; - if (func) { - blockInfo.func = func.bind(serviceObject); + // Avoid promise latency unless necessary + if (dispatch._isRemoteService(serviceName)) { + blockInfo.func = (args, util) => dispatch.call(serviceName, funcName, args, util, blockInfo); } else { - throw new Error(`Could not find extension block function called ${blockInfo.func}`); + const serviceObject = dispatch.services[serviceName]; + if (!serviceObject[funcName]) { + // The function might show up later as a dynamic property of the service object + log.warn(`Could not find extension block function called ${funcName}`); + } + blockInfo.func = (args, util) => serviceObject[funcName](args, util, blockInfo); } + } else { + throw new Error('Missing opcode for block'); } break; } From 3b395a10d191c648e5736eb19b8aebe9b6021075 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 8 Apr 2019 17:59:15 -0700 Subject: [PATCH 02/14] add test for extension methods receiving `blockInfo` --- test/integration/internal-extension.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/test/integration/internal-extension.js b/test/integration/internal-extension.js index 9ad399402..970db809a 100644 --- a/test/integration/internal-extension.js +++ b/test/integration/internal-extension.js @@ -1,6 +1,8 @@ const test = require('tap').test; const Worker = require('tiny-worker'); +const BlockType = require('../../src/extension-support/block-type'); + const dispatch = require('../../src/dispatch/central-dispatch'); const VirtualMachine = require('../../src/virtual-machine'); @@ -33,8 +35,9 @@ class TestInternalExtension { }; } - go () { + go (args, util, blockInfo) { this.status.goCalled = true; + return blockInfo; } _buildAMenu () { @@ -62,9 +65,22 @@ test('internal extension', t => { t.type(func, 'function'); t.notOk(extension.status.goCalled); - func(); + const goBlockInfo = func(); t.ok(extension.status.goCalled); + // The 'go' block returns its own blockInfo. Make sure it matches the expected info. + // Note that the extension parser fills in missing fields so there are more fields here than in `getInfo`. + const expectedBlockInfo = { + arguments: {}, + blockAllThreads: false, + blockType: BlockType.COMMAND, + func: goBlockInfo.func, // Cheat since we don't have a good way to ensure we generate the same function + opcode: 'go', + terminal: false, + text: 'go' + }; + t.deepEqual(goBlockInfo, expectedBlockInfo); + // There should be 2 menus - one is an array, one is the function to call. t.equal(vm.runtime._blockInfo[0].menus.length, 2); // First menu has 3 items. From 833d33355c07897cd9b22e96a76e281b71be36f3 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Fri, 19 Apr 2019 12:28:19 -0700 Subject: [PATCH 03/14] retrieve blockInfo from args when isDynamic is set --- src/engine/mutation-adapter.js | 4 ++ src/extension-support/extension-manager.js | 46 ++++++++++++++-------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/engine/mutation-adapter.js b/src/engine/mutation-adapter.js index d1984d30e..8cfac7634 100644 --- a/src/engine/mutation-adapter.js +++ b/src/engine/mutation-adapter.js @@ -13,6 +13,10 @@ const mutatorTagToObject = function (dom) { for (const prop in dom.attribs) { if (prop === 'xmlns') continue; obj[prop] = decodeHtml(dom.attribs[prop]); + if (prop === 'blockinfo') { + obj.blockInfo = JSON.parse(obj.blockinfo); + delete obj.blockinfo; + } } for (let i = 0; i < dom.children.length; i++) { obj.children.push( diff --git a/src/extension-support/extension-manager.js b/src/extension-support/extension-manager.js index e70929b63..a2f89aae0 100644 --- a/src/extension-support/extension-manager.js +++ b/src/extension-support/extension-manager.js @@ -390,26 +390,40 @@ class ExtensionManager { log.warn(`Ignoring opcode "${blockInfo.opcode}" for button with text: ${blockInfo.text}`); } break; - default: - if (blockInfo.opcode) { - const funcName = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; - - // Avoid promise latency unless necessary - if (dispatch._isRemoteService(serviceName)) { - blockInfo.func = (args, util) => dispatch.call(serviceName, funcName, args, util, blockInfo); - } else { - const serviceObject = dispatch.services[serviceName]; - if (!serviceObject[funcName]) { - // The function might show up later as a dynamic property of the service object - log.warn(`Could not find extension block function called ${funcName}`); - } - blockInfo.func = (args, util) => serviceObject[funcName](args, util, blockInfo); - } - } else { + default: { + if (!blockInfo.opcode) { throw new Error('Missing opcode for block'); } + + const funcName = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; + + const getBlockInfo = blockInfo.isDynamic ? + args => args && args.mutation && args.mutation.blockInfo : + () => blockInfo; + const callBlockFunc = (() => { + if (dispatch._isRemoteService(serviceName)) { + return (args, util, realBlockInfo) => + dispatch.call(serviceName, funcName, args, util, realBlockInfo); + } + + // avoid promise latency if we can call direct + const serviceObject = dispatch.services[serviceName]; + if (!serviceObject[funcName]) { + // The function might show up later as a dynamic property of the service object + log.warn(`Could not find extension block function called ${funcName}`); + } + return (args, util, realBlockInfo) => + serviceObject[funcName](args, util, realBlockInfo); + })(); + + blockInfo.func = (args, util) => { + const realBlockInfo = getBlockInfo(args); + // TODO: filter args using the keys of realBlockInfo.arguments? maybe only if sandboxed? + return callBlockFunc(args, util, realBlockInfo); + }; break; } + } return blockInfo; } From bd1aaecdf372589530a6423646e23bd434de4014 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Thu, 18 Apr 2019 09:45:22 -0700 Subject: [PATCH 04/14] add category info to extension add & update events --- src/engine/runtime.js | 12 +++--------- src/virtual-machine.js | 8 ++++---- test/unit/extension_conversion.js | 3 ++- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index b5739579c..310d9e77b 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -787,11 +787,9 @@ class Runtime extends EventEmitter { this._fillExtensionCategory(categoryInfo, extensionInfo); - const fieldTypeDefinitionsForScratch = []; for (const fieldTypeName in categoryInfo.customFieldTypes) { if (extensionInfo.customFieldTypes.hasOwnProperty(fieldTypeName)) { const fieldTypeInfo = categoryInfo.customFieldTypes[fieldTypeName]; - fieldTypeDefinitionsForScratch.push(fieldTypeInfo.scratchBlocksDefinition); // Emit events for custom field types from extension this.emit(Runtime.EXTENSION_FIELD_ADDED, { @@ -801,9 +799,7 @@ class Runtime extends EventEmitter { } } - const allBlocks = fieldTypeDefinitionsForScratch.concat(categoryInfo.blocks).concat(categoryInfo.menus); - - this.emit(Runtime.EXTENSION_ADDED, allBlocks); + this.emit(Runtime.EXTENSION_ADDED, categoryInfo); } /** @@ -812,18 +808,16 @@ class Runtime extends EventEmitter { * @private */ _refreshExtensionPrimitives (extensionInfo) { - let extensionBlocks = []; for (const categoryInfo of this._blockInfo) { if (extensionInfo.id === categoryInfo.id) { categoryInfo.name = maybeFormatMessage(extensionInfo.name); categoryInfo.blocks = []; categoryInfo.menus = []; this._fillExtensionCategory(categoryInfo, extensionInfo); - extensionBlocks = extensionBlocks.concat(categoryInfo.blocks, categoryInfo.menus); + + this.emit(Runtime.BLOCKSINFO_UPDATE, categoryInfo); } } - - this.emit(Runtime.BLOCKSINFO_UPDATE, extensionBlocks); } /** diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 3320238f4..f383c0b0f 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -109,14 +109,14 @@ class VirtualMachine extends EventEmitter { this.runtime.on(Runtime.BLOCK_DRAG_END, (blocks, topBlockId) => { this.emit(Runtime.BLOCK_DRAG_END, blocks, topBlockId); }); - this.runtime.on(Runtime.EXTENSION_ADDED, blocksInfo => { - this.emit(Runtime.EXTENSION_ADDED, blocksInfo); + this.runtime.on(Runtime.EXTENSION_ADDED, categoryInfo => { + this.emit(Runtime.EXTENSION_ADDED, categoryInfo); }); this.runtime.on(Runtime.EXTENSION_FIELD_ADDED, (fieldName, fieldImplementation) => { this.emit(Runtime.EXTENSION_FIELD_ADDED, fieldName, fieldImplementation); }); - this.runtime.on(Runtime.BLOCKSINFO_UPDATE, blocksInfo => { - this.emit(Runtime.BLOCKSINFO_UPDATE, blocksInfo); + this.runtime.on(Runtime.BLOCKSINFO_UPDATE, categoryInfo => { + this.emit(Runtime.BLOCKSINFO_UPDATE, categoryInfo); }); this.runtime.on(Runtime.BLOCKS_NEED_UPDATE, () => { this.emitWorkspaceUpdate(); diff --git a/test/unit/extension_conversion.js b/test/unit/extension_conversion.js index 3a59c0a01..001130a32 100644 --- a/test/unit/extension_conversion.js +++ b/test/unit/extension_conversion.js @@ -160,7 +160,8 @@ const testLoop = function (t, loop) { test('registerExtensionPrimitives', t => { const runtime = new Runtime(); - runtime.on(Runtime.EXTENSION_ADDED, blocksInfo => { + runtime.on(Runtime.EXTENSION_ADDED, categoryInfo => { + const blocksInfo = categoryInfo.blocks; t.equal(blocksInfo.length, testExtensionInfo.blocks.length); blocksInfo.forEach(blockInfo => { From 02474477921dba00f2d86cb4b01cef012ab1a5d7 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 22 Apr 2019 09:53:05 -0700 Subject: [PATCH 05/14] embed extension blockInfo into block XML --- src/engine/runtime.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 310d9e77b..44b015fd0 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -1127,7 +1127,9 @@ class Runtime extends EventEmitter { ++outLineNum; } - const blockXML = `${context.inputList.join('')}`; + const mutation = blockInfo.isDynamic ? `` : ''; + const inputs = context.inputList.join(''); + const blockXML = `${mutation}${inputs}`; return { info: context.blockInfo, From 107e49245fe2835b6a2e3dd4e92bdb2515413b38 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 22 Apr 2019 12:32:49 -0700 Subject: [PATCH 06/14] adjust getBlocksXML to return categories separately before: getBlocksXML returns one big XML string after: getBlocksXML returns an array of {id,xml}, one entry per category --- src/engine/runtime.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 44b015fd0..a7f97d85d 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -1243,11 +1243,12 @@ class Runtime extends EventEmitter { } /** - * @returns {string} scratch-blocks XML description for all dynamic blocks, wrapped in elements. + * @returns {Array.} scratch-blocks XML for each category of extension blocks, in category order. + * @property {string} id - the category / extension ID + * @property {string} xml - the XML text for this category, starting with `` and ending with `` */ getBlocksXML () { - const xmlParts = []; - for (const categoryInfo of this._blockInfo) { + return this._blockInfo.map(categoryInfo => { const {name, color1, color2} = categoryInfo; const paletteBlocks = categoryInfo.blocks.filter(block => !block.info.hideFromPalette); const colorXML = `colour="${color1}" secondaryColour="${color2}"`; @@ -1268,12 +1269,12 @@ class Runtime extends EventEmitter { statusButtonXML = 'showStatusButton="true"'; } - xmlParts.push(``); - xmlParts.push.apply(xmlParts, paletteBlocks.map(block => block.xml)); - xmlParts.push(''); - } - return xmlParts.join('\n'); + return { + id: categoryInfo.id, + xml: `${ + paletteBlocks.map(block => block.xml).join('')}` + }; + }); } /** From a27ea76d25bd1b09b92cfc261ed3bb6f83d477a7 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Mon, 22 Apr 2019 17:47:31 -0700 Subject: [PATCH 07/14] add "scratch_extension" only if a block has an icon --- src/engine/runtime.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index a7f97d85d..8b066b0a5 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -1003,8 +1003,7 @@ class Runtime extends EventEmitter { category: categoryInfo.name, colour: categoryInfo.color1, colourSecondary: categoryInfo.color2, - colourTertiary: categoryInfo.color3, - extensions: ['scratch_extension'] + colourTertiary: categoryInfo.color3 }; const context = { // TODO: store this somewhere so that we can map args appropriately after translation. @@ -1024,6 +1023,7 @@ class Runtime extends EventEmitter { const iconURI = blockInfo.blockIconURI || categoryInfo.blockIconURI; if (iconURI) { + blockJSON.extensions = ['scratch_extension']; blockJSON.message0 = '%1 %2'; const iconJSON = { type: 'field_image', From 297047a6b929f1c7ced03a7ea2a9464094758ccc Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Tue, 23 Apr 2019 11:44:45 -0400 Subject: [PATCH 08/14] Fix serialization of blockInfo mutation property to XML --- src/engine/blocks.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/engine/blocks.js b/src/engine/blocks.js index db0d5a5bc..165d5096c 100644 --- a/src/engine/blocks.js +++ b/src/engine/blocks.js @@ -1108,8 +1108,14 @@ class Blocks { let mutationString = `<${mutation.tagName}`; for (const prop in mutation) { if (prop === 'children' || prop === 'tagName') continue; - const mutationValue = (typeof mutation[prop] === 'string') ? + let mutationValue = (typeof mutation[prop] === 'string') ? xmlEscape(mutation[prop]) : mutation[prop]; + + // Handle dynamic extension blocks + if (prop === 'blockInfo') { + mutationValue = xmlEscape(JSON.stringify(mutation[prop])); + } + mutationString += ` ${prop}="${mutationValue}"`; } mutationString += '>'; From 91f0d59be078f8c120b5429352f284ce87bc8c6b Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 23 Apr 2019 12:17:48 -0700 Subject: [PATCH 09/14] fix extension block color application --- docs/extensions.md | 4 ++++ src/engine/runtime.js | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/extensions.md b/docs/extensions.md index 6cb632e1c..f2a8779ac 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -152,6 +152,10 @@ class SomeBlocks { // Will be used as the extension's namespace. id: 'someBlocks', + // Core extensions only: override the default extension block colors. + color1: '#FF8C1A', + color2: '#DB6E00', + // Optional: the human-readable name of this extension as string. // This and any other string to be displayed in the Scratch UI may either be // a string or a call to `formatMessage`; a plain string will not be diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 8b066b0a5..14ded0443 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -40,6 +40,8 @@ const defaultBlockPackages = { scratch3_procedures: require('../blocks/scratch3_procedures') }; +const defaultExtensionColors = ['#0FBD8C', '#0DA57A', '#0B8E69']; + /** * Information used for converting Scratch argument types into scratch-blocks data. * @type {object.} @@ -775,14 +777,21 @@ class Runtime extends EventEmitter { showStatusButton: extensionInfo.showStatusButton, blockIconURI: extensionInfo.blockIconURI, menuIconURI: extensionInfo.menuIconURI, - color1: extensionInfo.colour || '#0FBD8C', - color2: extensionInfo.colourSecondary || '#0DA57A', - color3: extensionInfo.colourTertiary || '#0B8E69', customFieldTypes: {}, blocks: [], menus: [] }; + if (extensionInfo.color1) { + categoryInfo.color1 = extensionInfo.color1; + categoryInfo.color2 = extensionInfo.color2; + categoryInfo.color3 = extensionInfo.color3; + } else { + categoryInfo.color1 = defaultExtensionColors[0]; + categoryInfo.color2 = defaultExtensionColors[1]; + categoryInfo.color3 = defaultExtensionColors[2]; + } + this._blockInfo.push(categoryInfo); this._fillExtensionCategory(categoryInfo, extensionInfo); From 92a73fef5536b8e504804205a09571b839cc006a Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Wed, 24 Apr 2019 11:28:02 -0400 Subject: [PATCH 10/14] Add a runtime event to track when the toolbox extension blocks need updating. --- src/engine/runtime.js | 22 +++++++++++++++++++++- src/virtual-machine.js | 3 +++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 14ded0443..26b7451cb 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -509,6 +509,14 @@ class Runtime extends EventEmitter { return 'PROJECT_CHANGED'; } + /** + * Event name for report that a change was made that can be saved + * @const {string} + */ + static get TOOLBOX_EXTENSIONS_NEED_UPDATE () { + return 'TOOLBOX_EXTENSIONS_NEED_UPDATE'; + } + /** * Event name for targets update report. * @const {string} @@ -1963,10 +1971,15 @@ class Runtime extends EventEmitter { * @param {!Target} editingTarget New editing target. */ setEditingTarget (editingTarget) { + const oldEditingTarget = this._editingTarget; this._editingTarget = editingTarget; // Script glows must be cleared. this._scriptGlowsPreviousFrame = []; this._updateGlows(); + + if (oldEditingTarget !== this._editingTarget) { + this.requestToolboxExtensionsUpdate(); + } } /** @@ -2384,12 +2397,19 @@ class Runtime extends EventEmitter { } /** - * Emit an event that indicate that the blocks on the workspace need updating. + * Emit an event that indicates that the blocks on the workspace need updating. */ requestBlocksUpdate () { this.emit(Runtime.BLOCKS_NEED_UPDATE); } + /** + * Emit an event that indicates that the toolbox extension blocks need updating. + */ + requestToolboxExtensionsUpdate () { + this.emit(Runtime.TOOLBOX_EXTENSIONS_NEED_UPDATE); + } + /** * Set up timers to repeatedly step in a browser. */ diff --git a/src/virtual-machine.js b/src/virtual-machine.js index f383c0b0f..b1f1d6b53 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -121,6 +121,9 @@ class VirtualMachine extends EventEmitter { this.runtime.on(Runtime.BLOCKS_NEED_UPDATE, () => { this.emitWorkspaceUpdate(); }); + this.runtime.on(Runtime.TOOLBOX_EXTENSIONS_NEED_UPDATE, () => { + this.extensionManager.refreshBlocks(); + }); this.runtime.on(Runtime.PERIPHERAL_LIST_UPDATE, info => { this.emit(Runtime.PERIPHERAL_LIST_UPDATE, info); }); From 39f15d3699f82283854c4aa62d22a8b652afeaa6 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Thu, 25 Apr 2019 11:36:31 -0400 Subject: [PATCH 11/14] Update comment for new extension update event --- src/engine/runtime.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 26b7451cb..1a1dd23f7 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -510,7 +510,7 @@ class Runtime extends EventEmitter { } /** - * Event name for report that a change was made that can be saved + * Event name for report that a change was made to an extension in the toolbox. * @const {string} */ static get TOOLBOX_EXTENSIONS_NEED_UPDATE () { From ab715901a6b84496d5c4c74bf12dbbadd7ac4a2d Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 11 Jun 2019 15:31:34 -0700 Subject: [PATCH 12/14] add tests for new e16n support features --- test/unit/engine_blocks.js | 21 ++++++++++++++- test/unit/engine_mutation-adapter.js | 26 +++++++++++++++++++ test/unit/extension_conversion.js | 39 +++++++++++++++++++++++++--- 3 files changed, 82 insertions(+), 4 deletions(-) create mode 100644 test/unit/engine_mutation-adapter.js diff --git a/test/unit/engine_blocks.js b/test/unit/engine_blocks.js index 8ab483e29..7fdaafc73 100644 --- a/test/unit/engine_blocks.js +++ b/test/unit/engine_blocks.js @@ -25,7 +25,7 @@ test('spec', t => { t.type(b.getNextBlock, 'function'); t.type(b.getBranch, 'function'); t.type(b.getOpcode, 'function'); - + t.type(b.mutationToXML, 'function'); t.end(); }); @@ -239,6 +239,25 @@ test('getOpcode', t => { t.end(); }); +test('mutationToXML', t => { + const b = new Blocks(new Runtime()); + const testStringRaw = '"arbitrary" & \'complicated\' test string'; + const testStringEscaped = '\\"arbitrary\\" & 'complicated' test string'; + const mutation = { + tagName: 'mutation', + children: [], + blockInfo: { + text: testStringRaw + } + }; + const xml = b.mutationToXML(mutation); + t.equals( + xml, + `` + ); + t.end(); +}); + // Block events tests test('create', t => { const b = new Blocks(new Runtime()); diff --git a/test/unit/engine_mutation-adapter.js b/test/unit/engine_mutation-adapter.js new file mode 100644 index 000000000..30a627740 --- /dev/null +++ b/test/unit/engine_mutation-adapter.js @@ -0,0 +1,26 @@ +const test = require('tap').test; + +const mutationAdapter = require('../../src/engine/mutation-adapter'); + +test('spec', t => { + t.type(mutationAdapter, 'function'); + t.end(); +}); + +test('convert DOM to Scratch object', t => { + const testStringRaw = '"arbitrary" & \'complicated\' test string'; + const testStringEscaped = '\\"arbitrary\\" & 'complicated' test string'; + const xml = ``; + const expectedMutation = { + tagName: 'mutation', + children: [], + blockInfo: { + text: testStringRaw + } + }; + + // TODO: do we want to test passing a DOM node to `mutationAdapter`? Node.js doesn't have built-in DOM support... + const mutationFromString = mutationAdapter(xml); + t.deepEqual(mutationFromString, expectedMutation); + t.end(); +}); diff --git a/test/unit/extension_conversion.js b/test/unit/extension_conversion.js index 001130a32..58f4ad59d 100644 --- a/test/unit/extension_conversion.js +++ b/test/unit/extension_conversion.js @@ -11,6 +11,9 @@ const ScratchBlocksConstants = require('../../src/engine/scratch-blocks-constant const testExtensionInfo = { id: 'test', name: 'fake test extension', + color1: '#111111', + color2: '#222222', + color3: '#333333', blocks: [ { func: 'MAKE_A_VARIABLE', @@ -20,7 +23,8 @@ const testExtensionInfo = { { opcode: 'reporter', blockType: BlockType.REPORTER, - text: 'simple text' + text: 'simple text', + blockIconURI: 'invalid icon URI' // trigger the 'scratch_extension' path }, '---', // separator between groups of blocks in an extension { @@ -63,6 +67,14 @@ const testExtensionInfo = { ] }; +const testCategoryInfo = function (t, block) { + t.equal(block.json.category, 'fake test extension'); + t.equal(block.json.colour, '#111111'); + t.equal(block.json.colourSecondary, '#222222'); + t.equal(block.json.colourTertiary, '#333333'); + t.equal(block.json.inputsInline, true); +}; + const testButton = function (t, button) { t.same(button.json, null); // should be null or undefined t.equal(button.xml, ''); @@ -70,13 +82,28 @@ const testButton = function (t, button) { const testReporter = function (t, reporter) { t.equal(reporter.json.type, 'test_reporter'); + testCategoryInfo(t, reporter); + t.equal(reporter.json.checkboxInFlyout, true); t.equal(reporter.json.outputShape, ScratchBlocksConstants.OUTPUT_SHAPE_ROUND); t.equal(reporter.json.output, 'String'); t.notOk(reporter.json.hasOwnProperty('previousStatement')); t.notOk(reporter.json.hasOwnProperty('nextStatement')); - t.equal(reporter.json.message0, 'simple text'); + t.same(reporter.json.extensions, ['scratch_extension']); + t.equal(reporter.json.message0, '%1 %2simple text'); // "%1 %2" from the block icon t.notOk(reporter.json.hasOwnProperty('message1')); - t.notOk(reporter.json.hasOwnProperty('args0')); + t.same(reporter.json.args0, [ + // %1 in message0: the block icon + { + type: 'field_image', + src: 'invalid icon URI', + width: 40, + height: 40 + }, + // %2 in message0: separator between icon and text (only added when there's also an icon) + { + type: 'field_vertical_separator' + } + ]); t.notOk(reporter.json.hasOwnProperty('args1')); t.equal(reporter.xml, ''); }; @@ -88,9 +115,11 @@ const testSeparator = function (t, separator) { const testCommand = function (t, command) { t.equal(command.json.type, 'test_command'); + testCategoryInfo(t, command); t.equal(command.json.outputShape, ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE); t.assert(command.json.hasOwnProperty('previousStatement')); t.assert(command.json.hasOwnProperty('nextStatement')); + t.notOk(command.json.extensions && command.json.extensions.length); // OK if it's absent or empty t.equal(command.json.message0, 'text with %1'); t.notOk(command.json.hasOwnProperty('message1')); t.strictSame(command.json.args0[0], { @@ -105,9 +134,11 @@ const testCommand = function (t, command) { const testConditional = function (t, conditional) { t.equal(conditional.json.type, 'test_ifElse'); + testCategoryInfo(t, conditional); t.equal(conditional.json.outputShape, ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE); t.ok(conditional.json.hasOwnProperty('previousStatement')); t.ok(conditional.json.hasOwnProperty('nextStatement')); + t.notOk(conditional.json.extensions && conditional.json.extensions.length); // OK if it's absent or empty t.equal(conditional.json.message0, 'test if %1 is spiffy and if so then'); t.equal(conditional.json.message1, '%1'); // placeholder for substack #1 t.equal(conditional.json.message2, 'or elsewise'); @@ -133,9 +164,11 @@ const testConditional = function (t, conditional) { const testLoop = function (t, loop) { t.equal(loop.json.type, 'test_loop'); + testCategoryInfo(t, loop); t.equal(loop.json.outputShape, ScratchBlocksConstants.OUTPUT_SHAPE_SQUARE); t.ok(loop.json.hasOwnProperty('previousStatement')); t.notOk(loop.json.hasOwnProperty('nextStatement')); // isTerminal is set on this block + t.notOk(loop.json.extensions && loop.json.extensions.length); // OK if it's absent or empty t.equal(loop.json.message0, 'loopty %1 loops'); t.equal(loop.json.message1, '%1'); // placeholder for substack t.equal(loop.json.message2, '%1'); // placeholder for loop arrow From edd6aafed01d49be35d7298ba2561376991f5b8e Mon Sep 17 00:00:00 2001 From: Chris Willis-Ford Date: Tue, 18 Jun 2019 14:32:00 -0700 Subject: [PATCH 13/14] call out capitalization change since it's important but easy to miss Co-Authored-By: Karishma Chadha --- src/engine/mutation-adapter.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/engine/mutation-adapter.js b/src/engine/mutation-adapter.js index 8cfac7634..6c3bd84de 100644 --- a/src/engine/mutation-adapter.js +++ b/src/engine/mutation-adapter.js @@ -13,6 +13,8 @@ const mutatorTagToObject = function (dom) { for (const prop in dom.attribs) { if (prop === 'xmlns') continue; obj[prop] = decodeHtml(dom.attribs[prop]); + // Note: the capitalization of block info in the following lines is important. + // The lowercase is read in from xml which normalizes case. The VM uses camel case everywhere else. if (prop === 'blockinfo') { obj.blockInfo = JSON.parse(obj.blockinfo); delete obj.blockinfo; From fd776025e5fe2cfd2b2d5f7fa004ea94985b1447 Mon Sep 17 00:00:00 2001 From: Christopher Willis-Ford Date: Tue, 18 Jun 2019 15:04:37 -0700 Subject: [PATCH 14/14] refactor _refreshExtensionPrimitives for clarity The previous form made it harder to see that at most one extension category is handled per call. --- src/engine/runtime.js | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/engine/runtime.js b/src/engine/runtime.js index 1a1dd23f7..ac7c7bb26 100644 --- a/src/engine/runtime.js +++ b/src/engine/runtime.js @@ -825,15 +825,14 @@ class Runtime extends EventEmitter { * @private */ _refreshExtensionPrimitives (extensionInfo) { - for (const categoryInfo of this._blockInfo) { - if (extensionInfo.id === categoryInfo.id) { - categoryInfo.name = maybeFormatMessage(extensionInfo.name); - categoryInfo.blocks = []; - categoryInfo.menus = []; - this._fillExtensionCategory(categoryInfo, extensionInfo); + const categoryInfo = this._blockInfo.find(info => info.id === extensionInfo.id); + if (categoryInfo) { + categoryInfo.name = maybeFormatMessage(extensionInfo.name); + categoryInfo.blocks = []; + categoryInfo.menus = []; + this._fillExtensionCategory(categoryInfo, extensionInfo); - this.emit(Runtime.BLOCKSINFO_UPDATE, categoryInfo); - } + this.emit(Runtime.BLOCKSINFO_UPDATE, categoryInfo); } }