From e4d7c381bc394de168486a9f809babcde3c67244 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 26 Jan 2023 09:21:54 -0800 Subject: [PATCH] chore: revert changes to procedure blocks (#6794) * chore: revert procedure blocks * chore: fix tests --- blocks/procedures.js | 1768 ++++++++++--------------- tests/mocha/blocks/procedures_test.js | 866 +----------- 2 files changed, 735 insertions(+), 1899 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index a2bf4606a..c7a5d7fc3 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -16,9 +16,9 @@ goog.module('Blockly.libraryBlocks.procedures'); const AbstractEvent = goog.requireType('Blockly.Events.Abstract'); const ContextMenu = goog.require('Blockly.ContextMenu'); const Events = goog.require('Blockly.Events'); -const Extensions = goog.require('Blockly.Extensions'); const Procedures = goog.require('Blockly.Procedures'); const Variables = goog.require('Blockly.Variables'); +const Xml = goog.require('Blockly.Xml'); const xmlUtils = goog.require('Blockly.utils.xml'); const {Align} = goog.require('Blockly.Input'); /* eslint-disable-next-line no-unused-vars */ @@ -27,22 +27,19 @@ const {Block} = goog.requireType('Blockly.Block'); // TODO (6248): Properly import the BlockDefinition type. /* eslint-disable-next-line no-unused-vars */ const BlockDefinition = Object; -const {isProcedureBlock} = goog.require('Blockly.procedures.IProcedureBlock'); -const {ObservableProcedureModel} = goog.require('Blockly.procedures.ObservableProcedureModel'); -const {ObservableParameterModel} = goog.require('Blockly.procedures.ObservableParameterModel'); const {config} = goog.require('Blockly.config'); /* eslint-disable-next-line no-unused-vars */ +const {FieldCheckbox} = goog.require('Blockly.FieldCheckbox'); const {FieldLabel} = goog.require('Blockly.FieldLabel'); +const {FieldTextInput} = goog.require('Blockly.FieldTextInput'); const {Msg} = goog.require('Blockly.Msg'); const {Mutator} = goog.require('Blockly.Mutator'); const {Names} = goog.require('Blockly.Names'); -const serialization = goog.require('Blockly.serialization'); -const {triggerProceduresUpdate} = goog.require('Blockly.procedures.updateProcedures'); /* eslint-disable-next-line no-unused-vars */ const {VariableModel} = goog.requireType('Blockly.VariableModel'); /* eslint-disable-next-line no-unused-vars */ const {Workspace} = goog.requireType('Blockly.Workspace'); -const {createBlockDefinitionsFromJsonArray, defineBlocks} = goog.require('Blockly.common'); +const {defineBlocks} = goog.require('Blockly.common'); /** @suppress {extraRequire} */ goog.require('Blockly.Comment'); /** @suppress {extraRequire} */ @@ -53,389 +50,14 @@ goog.require('Blockly.Warning'); * A dictionary of the block definitions provided by this module. * @type {!Object} */ -const blocks = createBlockDefinitionsFromJsonArray([ - { - 'type': 'procedures_defnoreturn', - 'message0': '%{BKY_PROCEDURES_DEFNORETURN_TITLE} %1 %2 %3', - 'message1': '%{BKY_PROCEDURES_DEFNORETURN_DO} %1', - 'args0': [ - { - 'type': 'field_input', - 'name': 'NAME', - 'text': '', - 'spellcheck': false, - }, - { - 'type': 'field_label', - 'name': 'PARAMS', - 'text': '', - }, - { - 'type': 'input_dummy', - 'name': 'TOP', - }, - ], - 'args1': [ - { - 'type': 'input_statement', - 'name': 'STACK', - }, - ], - 'style': 'procedure_blocks', - 'helpUrl': '%{BKY_PROCEDURES_DEFNORETURN_HELPURL}', - 'tooltip': '%{BKY_PROCEDURES_DEFNORETURN_TOOLTIP}', - 'extensions': [ - 'procedure_def_get_def_mixin', - 'procedure_def_var_mixin', - 'procedure_def_update_shape_mixin', - 'procedure_def_context_menu_mixin', - 'procedure_def_onchange_mixin', - 'procedure_def_validator_helper', - 'procedure_defnoreturn_get_caller_block_mixin', - 'procedure_defnoreturn_set_comment_helper', - 'procedure_def_set_no_return_helper', - ], - 'mutator': 'procedure_def_mutator', - }, - { - 'type': 'procedures_callnoreturn', - 'message0': '%1 %2', - 'args0': [ - {'type': 'field_label', 'name': 'NAME', 'text': '%{BKY_UNNAMED_KEY}'}, - { - 'type': 'input_dummy', - 'name': 'TOPROW', - }, - ], - 'nextStatement': null, - 'previousStatement': null, - 'style': 'procedure_blocks', - 'helpUrl': '%{BKY_PROCEDURES_CALLNORETURN_HELPURL}', - 'extensions': [ - 'procedure_caller_get_def_mixin', - 'procedure_caller_update_shape_mixin', - 'procedure_caller_context_menu_mixin', - 'procedure_caller_onchange_mixin', - 'procedure_callernoreturn_get_def_block_mixin', - ], - 'mutator': 'procedure_caller_mutator', - }, - { - 'type': 'procedures_defreturn', - 'message0': '%{BKY_PROCEDURES_DEFRETURN_TITLE} %1 %2 %3', - 'message1': '%{BKY_PROCEDURES_DEFRETURN_DO} %1', - 'message2': '%{BKY_PROCEDURES_DEFRETURN_RETURN} %1', - 'args0': [ - { - 'type': 'field_input', - 'name': 'NAME', - 'text': '', - 'spellcheck': false, - }, - { - 'type': 'field_label', - 'name': 'PARAMS', - 'text': '', - }, - { - 'type': 'input_dummy', - 'name': 'TOP', - }, - ], - 'args1': [ - { - 'type': 'input_statement', - 'name': 'STACK', - }, - ], - 'args2': [ - { - 'type': 'input_value', - 'align': 'right', - 'name': 'RETURN', - }, - ], - 'style': 'procedure_blocks', - 'helpUrl': '%{BKY_PROCEDURES_DEFRETURN_HELPURL}', - 'tooltip': '%{BKY_PROCEDURES_DEFRETURN_TOOLTIP}', - 'extensions': [ - 'procedure_def_get_def_mixin', - 'procedure_def_var_mixin', - 'procedure_def_update_shape_mixin', - 'procedure_def_context_menu_mixin', - 'procedure_def_onchange_mixin', - 'procedure_def_validator_helper', - 'procedure_defreturn_get_caller_block_mixin', - 'procedure_defreturn_set_comment_helper', - 'procedure_def_set_return_helper', - ], - 'mutator': 'procedure_def_mutator', - }, - { - 'type': 'procedures_callreturn', - 'message0': '%1 %2', - 'args0': [ - {'type': 'field_label', 'name': 'NAME', 'text': '%{BKY_UNNAMED_KEY}'}, - { - 'type': 'input_dummy', - 'name': 'TOPROW', - }, - ], - 'output': null, - 'style': 'procedure_blocks', - 'helpUrl': '%{BKY_PROCEDURES_CALLRETURN_HELPURL}', - 'extensions': [ - 'procedure_caller_get_def_mixin', - 'procedure_caller_update_shape_mixin', - 'procedure_caller_context_menu_mixin', - 'procedure_caller_onchange_mixin', - 'procedure_callerreturn_get_def_block_mixin', - ], - 'mutator': 'procedure_caller_mutator', - }, - { - 'type': 'procedures_mutatorcontainer', - 'message0': '%{BKY_PROCEDURES_MUTATORCONTAINER_TITLE} %1 %2', - 'message1': '%{BKY_PROCEDURES_ALLOW_STATEMENTS} %1 %2', - 'args0': [ - { - 'type': 'input_dummy', - }, - { - 'type': 'input_statement', - 'name': 'STACK', - }, - ], - 'args1': [ - { - 'type': 'field_checkbox', - 'checked': true, - 'name': 'STATEMENTS', - }, - { - 'type': 'input_dummy', - 'name': 'STATEMENT_INPUT', - }, - ], - 'style': 'procedure_blocks', - 'tooltip': '%{BKY_PROCEDURES_MUTATORCONTAINER_TOOLTIP}', - 'enableContextMenu': false, - }, - { - 'type': 'procedures_mutatorarg', - 'message0': '%{BKY_PROCEDURES_MUTATORARG_TITLE} %1', - 'args0': [ - { - 'type': 'field_input', - 'name': 'NAME', - }, - ], - 'nextStatement': null, - 'previousStatement': null, - 'style': 'procedure_blocks', - 'tooltip': '%{BKY_PROCEDURES_MUTATORARG_TOOLTIP}', - 'enableContextMenu': false, - 'extensions': [ - 'procedure_mutatorarg_validate_mixin', - 'procedure_mutatorarg_add_validator_helper', - ], - }, - { - 'type': 'procedures_ifreturn', - 'message0': '%{BKY_CONTROLS_IF_MSG_IF} %1', - 'message1': '%{BKY_PROCEDURES_DEFRETURN_RETURN} %1', - 'args0': [ - { - 'type': 'input_value', - 'name': 'CONDITION', - 'check': 'Boolean', - }, - ], - 'args1': [ - { - 'type': 'input_value', - 'name': 'VALUE', - }, - ], - 'inputsInline': true, - 'nextStatement': null, - 'previousStatement': null, - 'style': 'procedure_blocks', - 'tooltip': '%{BKY_PROCEDURES_IFRETURN_TOOLTIP}', - 'helpUrl': '%{BKY_PROCEDURES_IFRETURN_HELPURL}', - 'extensions': [ - 'procedure_ifreturn_onchange_mixin', - 'procedure_ifreturn_function_types_mixin', - ], - 'mutator': 'procedure_ifreturn_mutator', - }, -]); +const blocks = {}; exports.blocks = blocks; - -/** @this {Block} */ -const procedureDefGetDefMixin = function() { - const mixin = { - model_: null, - - /** - * Returns the data model for this procedure block. - * @return {!IProcedureModel} The data model for this procedure - * block. - */ - getProcedureModel() { - return this.model_; - }, - - /** - * True if this is a procedure definition block, false otherwise (i.e. - * it is a caller). - * @return {boolean} True because this is a procedure definition block. - */ - isProcedureDef() { - return true; - }, - - /** - * Return all variables referenced by this block. - * @return {!Array} List of variable names. - * @this {Block} - */ - getVars: function() { - return this.getProcedureModel().getParameters().map( - (p) => p.getVariableModel().name); - }, - - /** - * Return all variables referenced by this block. - * @return {!Array} List of variable models. - * @this {Block} - */ - getVarModels: function() { - return this.getProcedureModel().getParameters().map( - (p) => p.getVariableModel()); - }, - - /** - * Disposes of the data model for this procedure block when the block is - * disposed. - */ - destroy: function() { - if (this.isInsertionMarker()) return; - this.workspace.getProcedureMap().delete(this.getProcedureModel().getId()); - }, - }; - - mixin.model_ = new ObservableProcedureModel( - this.workspace, - Procedures.findLegalName(this.getFieldValue('NAME'), this)); - this.workspace.getProcedureMap().add(mixin.getProcedureModel()); - - this.mixin(mixin, true); -}; -// Using register instead of registerMixin to avoid triggering warnings about -// overriding built-ins. -Extensions.register('procedure_def_get_def_mixin', procedureDefGetDefMixin); - -/** @this {Block} */ -const procedureDefVarMixin = function() { - const mixin = { - /** - * Notification that a variable is renaming. - * If the ID matches one of this block's variables, rename it. - * @param {string} oldId ID of variable to rename. - * @param {string} newId ID of new variable. May be the same as oldId, but - * with an updated name. Guaranteed to be the same type as the old - * variable. - * @override - * @this {Block} - */ - renameVarById: function(oldId, newId) { - const oldVar = this.workspace.getVariableById(oldId); - const model = this.getProcedureModel(); - const index = model.getParameters().findIndex( - (p) => p.getVariableModel() === oldVar); - if (index === -1) return; // Not found. - const newVar = this.workspace.getVariableById(newId); - const oldParam = model.getParameter(index); - model.deleteParameter(index); - model.insertParameter( - new ObservableParameterModel( - this.workspace, newVar.name, oldParam.getId()), - index); - }, - - /** - * Notification that a variable is renaming but keeping the same ID. If the - * variable is in use on this block, rerender to show the new name. - * @param {!VariableModel} variable The variable being renamed. - * @package - * @override - * @this {Block} - */ - updateVarName: function(variable) { - const containsVar = this.getProcedureModel().getParameters().some( - (p) => p.getVariableModel() === variable); - if (containsVar) { - triggerProceduresUpdate(this.workspace); - } - }, - }; - - this.mixin(mixin, true); -}; -// Using register instead of registerMixin to avoid triggering warnings about -// overriding built-ins. -Extensions.register('procedure_def_var_mixin', procedureDefVarMixin); - -const procedureDefUpdateShapeMixin = { - /** - * Updates the block to reflect the state of the procedure model. - */ - doProcedureUpdate: function() { - this.setFieldValue(this.getProcedureModel().getName(), 'NAME'); - this.setEnabled(this.getProcedureModel().getEnabled()); - this.updateParameters_(); - this.updateMutator_(); - }, - - /** - * Updates the parameters field to reflect the parameters in the procedure - * model. - */ - updateParameters_: function() { - const params = - this.getProcedureModel().getParameters().map((p) => p.getName()); - const paramString = params.length ? - `${Msg['PROCEDURES_BEFORE_PARAMS']} ${params.join(', ')}` : - ''; - - // The field is deterministic based on other events, no need to fire. - Events.disable(); - try { - this.setFieldValue(paramString, 'PARAMS'); - } finally { - Events.enable(); - } - }, - - /** - * Updates the parameter blocks in the mutator (if it is open) to reflect - * the state of the procedure model. - */ - updateMutator_: function() { - if (!this.mutator?.isVisible()) return; - - const mutatorWorkspace = this.mutator.getWorkspace(); - for (const p of this.getProcedureModel().getParameters()) { - const block = mutatorWorkspace.getBlockById(p.getId()); - if (!block) continue; // Should not happen. - if (block.getFieldValue('NAME') !== p.getName()) { - block.setFieldValue(p.getName(), 'NAME'); - } - } - }, - +/** + * Common properties for the procedure_defnoreturn and + * procedure_defreturn blocks. + */ +const PROCEDURE_DEF_COMMON = { /** * Add or remove the statement block from this function definition. * @param {boolean} hasStatements True if a statement block is needed. @@ -451,38 +73,32 @@ const procedureDefUpdateShapeMixin = { if (this.getInput('RETURN')) { this.moveInputBefore('STACK', 'RETURN'); } - // Restore the stack, if one was saved. - Mutator.reconnect(this.statementConnection_, this, 'STACK'); - this.statementConnection_ = null; } else { - // Save the stack, then disconnect it. - const stackConnection = this.getInput('STACK').connection; - this.statementConnection_ = stackConnection.targetConnection; - if (this.statementConnection_) { - const stackBlock = stackConnection.targetBlock(); - stackBlock.unplug(); - stackBlock.bumpNeighbours(); - } this.removeInput('STACK', true); } this.hasStatements_ = hasStatements; }, -}; -Extensions.registerMixin( - 'procedure_def_update_shape_mixin', procedureDefUpdateShapeMixin); - -/** @this {Block} */ -const procedureDefValidatorHelper = function() { - const nameField = this.getField('NAME'); - nameField.setValue(Procedures.findLegalName('', this)); - nameField.setValidator(Procedures.rename); -}; -Extensions.register( - 'procedure_def_validator_helper', procedureDefValidatorHelper); - -const procedureDefMutator = { - hasStatements_: true, - + /** + * Update the display of parameters for this procedure definition block. + * @private + * @this {Block} + */ + updateParams_: function() { + // Merge the arguments into a human-readable list. + let paramString = ''; + if (this.arguments_.length) { + paramString = + Msg['PROCEDURES_BEFORE_PARAMS'] + ' ' + this.arguments_.join(', '); + } + // The params field is deterministic based on the mutation, + // no need to fire a change event. + Events.disable(); + try { + this.setFieldValue(paramString, 'PARAMS'); + } finally { + Events.enable(); + } + }, /** * Create XML to represent the argument inputs. * Backwards compatible serialization implementation. @@ -496,15 +112,13 @@ const procedureDefMutator = { if (opt_paramIds) { container.setAttribute('name', this.getFieldValue('NAME')); } - - const params = this.getProcedureModel().getParameters(); - for (let i = 0; i < params.length; i++) { + for (let i = 0; i < this.argumentVarModels_.length; i++) { const parameter = xmlUtils.createElement('arg'); - const varModel = params[i].getVariableModel(); - parameter.setAttribute('name', varModel.name); - parameter.setAttribute('varid', varModel.getId()); - if (opt_paramIds) { - parameter.setAttribute('paramId', params[i].getId()); + const argModel = this.argumentVarModels_[i]; + parameter.setAttribute('name', argModel.name); + parameter.setAttribute('varid', argModel.getId()); + if (opt_paramIds && this.paramIds_) { + parameter.setAttribute('paramId', this.paramIds_[i]); } container.appendChild(parameter); } @@ -515,7 +129,6 @@ const procedureDefMutator = { } return container; }, - /** * Parse XML to restore the argument inputs. * Backwards compatible serialization implementation. @@ -523,21 +136,30 @@ const procedureDefMutator = { * @this {Block} */ domToMutation: function(xmlElement) { - for (let i = 0; i < xmlElement.childNodes.length; i++) { - const node = xmlElement.childNodes[i]; - if (node.nodeName.toLowerCase() !== 'arg') continue; - const varId = node.getAttribute('varid'); - this.getProcedureModel().insertParameter( - new ObservableParameterModel( - this.workspace, node.getAttribute('name'), undefined, varId), - i); + this.arguments_ = []; + this.argumentVarModels_ = []; + for (let i = 0, childNode; (childNode = xmlElement.childNodes[i]); i++) { + if (childNode.nodeName.toLowerCase() === 'arg') { + const varName = childNode.getAttribute('name'); + const varId = + childNode.getAttribute('varid') || childNode.getAttribute('varId'); + this.arguments_.push(varName); + const variable = Variables.getOrCreateVariablePackage( + this.workspace, varId, varName, ''); + if (variable !== null) { + this.argumentVarModels_.push(variable); + } else { + console.log( + `Failed to create a variable named "${varName}", ignoring.`); + } + } } - this.setStatements_(xmlElement.getAttribute('statements') !== 'false'); - - // Call mutate callers for backwards compatibility. + this.updateParams_(); Procedures.mutateCallers(this); - }, + // Show or hide the statement input. + this.setStatements_(xmlElement.getAttribute('statements') !== 'false'); + }, /** * Returns the state of this block as a JSON serializable object. * @return {?{params: (!Array<{name: string, id: string}>|undefined), @@ -545,77 +167,47 @@ const procedureDefMutator = { * parameters and statements. */ saveExtraState: function() { + if (!this.argumentVarModels_.length && this.hasStatements_) { + return null; + } const state = Object.create(null); - state['procedureId'] = this.getProcedureModel().getId(); - - const params = this.getProcedureModel().getParameters(); - if (!params.length && this.hasStatements_) return state; - - if (params.length) { - state['params'] = params.map((p) => { - return { - 'name': p.getName(), - 'id': p.getVariableModel().getId(), - // Ideally this would be id, and the other would be varId, - // but backwards compatibility :/ - 'paramId': p.getId(), - }; - }); + if (this.argumentVarModels_.length) { + state['params'] = []; + for (let i = 0; i < this.argumentVarModels_.length; i++) { + state['params'].push({ + // We don't need to serialize the name, but just in case we decide + // to separate params from variables. + 'name': this.argumentVarModels_[i].name, + 'id': this.argumentVarModels_[i].getId(), + }); + } } if (!this.hasStatements_) { state['hasStatements'] = false; } return state; }, - /** * Applies the given state to this block. * @param {*} state The state to apply to this block, eg the parameters and * statements. */ loadExtraState: function(state) { - const map = this.workspace.getProcedureMap(); - const procedureId = state['procedureId']; - if (procedureId && procedureId != this.model_.getId() && - map.has(procedureId) && - (this.isInsertionMarker() || - this.noBlockHasClaimedModel_(procedureId))) { - if (map.has(this.model_.getId())) { - map.delete(this.model_.getId()); - } - this.model_ = map.get(procedureId); - } - + this.arguments_ = []; + this.argumentVarModels_ = []; if (state['params']) { for (let i = 0; i < state['params'].length; i++) { - const {name, id, paramId} = state['params'][i]; - this.getProcedureModel().insertParameter( - new ObservableParameterModel(this.workspace, name, paramId, id), i); + const param = state['params'][i]; + const variable = Variables.getOrCreateVariablePackage( + this.workspace, param['id'], param['name'], ''); + this.arguments_.push(variable.name); + this.argumentVarModels_.push(variable); } } - - this.doProcedureUpdate(); - this.setStatements_(state['hasStatements'] === false ? false : true); - - // Call mutate callers for backwards compatibility. + this.updateParams_(); Procedures.mutateCallers(this); + this.setStatements_(state['hasStatements'] === false ? false : true); }, - - /** - * Returns true if there is no definition block currently associated with the - * given procedure ID. False otherwise. - * @param {string} procedureId The ID of the procedure to check for a claiming - * block. - * @return {boolean} True if there is no definition block currently associated - * with the given procedure ID. False otherwise. - */ - noBlockHasClaimedModel_(procedureId) { - const model = this.workspace.getProcedureMap().get(procedureId); - return this.workspace.getAllBlocks(false).every( - (b) => !isProcedureBlock(b) || !b.isProcedureDef() || - b.getProcedureModel() !== model); - }, - /** * Populate the mutator's dialog with this block's components. * @param {!Workspace} workspace Mutator's workspace. @@ -623,28 +215,41 @@ const procedureDefMutator = { * @this {Block} */ decompose: function(workspace) { - const containerBlockDef = { - 'type': 'procedures_mutatorcontainer', - 'inputs': { - 'STACK': {}, - }, - }; + /* + * Creates the following XML: + * + * + * + * arg1_name + * etc... + * + * + * + */ - let connDef = containerBlockDef['inputs']['STACK']; - for (const param of this.getProcedureModel().getParameters()) { - connDef['block'] = { - 'type': 'procedures_mutatorarg', - 'id': param.getId(), - 'fields': { - 'NAME': param.getName(), - }, - 'next': {}, - }; - connDef = connDef['block']['next']; + const containerBlockNode = xmlUtils.createElement('block'); + containerBlockNode.setAttribute('type', 'procedures_mutatorcontainer'); + const statementNode = xmlUtils.createElement('statement'); + statementNode.setAttribute('name', 'STACK'); + containerBlockNode.appendChild(statementNode); + + let node = statementNode; + for (let i = 0; i < this.arguments_.length; i++) { + const argBlockNode = xmlUtils.createElement('block'); + argBlockNode.setAttribute('type', 'procedures_mutatorarg'); + const fieldNode = xmlUtils.createElement('field'); + fieldNode.setAttribute('name', 'NAME'); + const argumentName = xmlUtils.createTextNode(this.arguments_[i]); + fieldNode.appendChild(argumentName); + argBlockNode.appendChild(fieldNode); + const nextNode = xmlUtils.createElement('next'); + argBlockNode.appendChild(nextNode); + + node.appendChild(argBlockNode); + node = nextNode; } - const containerBlock = serialization.blocks.append( - containerBlockDef, workspace, {recordUndo: false}); + const containerBlock = Xml.domToBlock(containerBlockNode, workspace); if (this.type === 'procedures_defreturn') { containerBlock.setFieldValue(this.hasStatements_, 'STATEMENTS'); @@ -652,52 +257,150 @@ const procedureDefMutator = { containerBlock.removeInput('STATEMENT_INPUT'); } - // Update callers (called for backwards compatibility). + // Initialize procedure's callers with blank IDs. Procedures.mutateCallers(this); - return containerBlock; }, - /** * Reconfigure this block based on the mutator dialog's components. * @param {!Block} containerBlock Root block in mutator. * @this {Block} */ compose: function(containerBlock) { - const model = this.getProcedureModel(); - const count = model.getParameters().length; - model.startBulkUpdate(); - for (let i = count - 1; i >= 0; i--) { - model.deleteParameter(i); - } - - let i = 0; + // Parameter list. + this.arguments_ = []; + this.paramIds_ = []; + this.argumentVarModels_ = []; let paramBlock = containerBlock.getInputTargetBlock('STACK'); while (paramBlock && !paramBlock.isInsertionMarker()) { - model.insertParameter( - new ObservableParameterModel( - this.workspace, paramBlock.getFieldValue('NAME'), paramBlock.id), - i); + const varName = paramBlock.getFieldValue('NAME'); + this.arguments_.push(varName); + const variable = this.workspace.getVariable(varName, ''); + this.argumentVarModels_.push(variable); + + this.paramIds_.push(paramBlock.id); paramBlock = paramBlock.nextConnection && paramBlock.nextConnection.targetBlock(); - i++; } - model.endBulkUpdate(); - - const hasStatements = containerBlock.getFieldValue('STATEMENTS'); - if (hasStatements !== null) { - this.setStatements_(hasStatements === 'TRUE'); - } - - // Call mutate callers for backwards compatibility. + this.updateParams_(); Procedures.mutateCallers(this); - }, -}; -Extensions.registerMutator( - 'procedure_def_mutator', procedureDefMutator, undefined, - ['procedures_mutatorarg']); -const procedureDefContextMenuMixin = { + // Show/hide the statement input. + let hasStatements = containerBlock.getFieldValue('STATEMENTS'); + if (hasStatements !== null) { + hasStatements = hasStatements === 'TRUE'; + if (this.hasStatements_ !== hasStatements) { + if (hasStatements) { + this.setStatements_(true); + // Restore the stack, if one was saved. + Mutator.reconnect(this.statementConnection_, this, 'STACK'); + this.statementConnection_ = null; + } else { + // Save the stack, then disconnect it. + const stackConnection = this.getInput('STACK').connection; + this.statementConnection_ = stackConnection.targetConnection; + if (this.statementConnection_) { + const stackBlock = stackConnection.targetBlock(); + stackBlock.unplug(); + stackBlock.bumpNeighbours(); + } + this.setStatements_(false); + } + } + } + }, + /** + * Return all variables referenced by this block. + * @return {!Array} List of variable names. + * @this {Block} + */ + getVars: function() { + return this.arguments_; + }, + /** + * Return all variables referenced by this block. + * @return {!Array} List of variable models. + * @this {Block} + */ + getVarModels: function() { + return this.argumentVarModels_; + }, + /** + * Notification that a variable is renaming. + * If the ID matches one of this block's variables, rename it. + * @param {string} oldId ID of variable to rename. + * @param {string} newId ID of new variable. May be the same as oldId, but + * with an updated name. Guaranteed to be the same type as the old + * variable. + * @override + * @this {Block} + */ + renameVarById: function(oldId, newId) { + const oldVariable = this.workspace.getVariableById(oldId); + if (oldVariable.type !== '') { + // Procedure arguments always have the empty type. + return; + } + const oldName = oldVariable.name; + const newVar = this.workspace.getVariableById(newId); + + let change = false; + for (let i = 0; i < this.argumentVarModels_.length; i++) { + if (this.argumentVarModels_[i].getId() === oldId) { + this.arguments_[i] = newVar.name; + this.argumentVarModels_[i] = newVar; + change = true; + } + } + if (change) { + this.displayRenamedVar_(oldName, newVar.name); + Procedures.mutateCallers(this); + } + }, + /** + * Notification that a variable is renaming but keeping the same ID. If the + * variable is in use on this block, rerender to show the new name. + * @param {!VariableModel} variable The variable being renamed. + * @package + * @override + * @this {Block} + */ + updateVarName: function(variable) { + const newName = variable.name; + let change = false; + let oldName; + for (let i = 0; i < this.argumentVarModels_.length; i++) { + if (this.argumentVarModels_[i].getId() === variable.getId()) { + oldName = this.arguments_[i]; + this.arguments_[i] = newName; + change = true; + } + } + if (change) { + this.displayRenamedVar_(oldName, newName); + Procedures.mutateCallers(this); + } + }, + /** + * Update the display to reflect a newly renamed argument. + * @param {string} oldName The old display name of the argument. + * @param {string} newName The new display name of the argument. + * @private + * @this {Block} + */ + displayRenamedVar_: function(oldName, newName) { + this.updateParams_(); + // Update the mutator's variables if the mutator is open. + if (this.mutator && this.mutator.isVisible()) { + const blocks = this.mutator.workspace_.getAllBlocks(false); + for (let i = 0, block; (block = blocks[i]); i++) { + if (block.type === 'procedures_mutatorarg' && + Names.equals(oldName, block.getFieldValue('NAME'))) { + block.setFieldValue(newName, 'NAME'); + } + } + } + }, /** * Add custom menu options to this block's context menu. * @param {!Array} options List of menu options to add to. @@ -713,10 +416,9 @@ const procedureDefContextMenuMixin = { option.text = Msg['PROCEDURES_CREATE_DO'].replace('%1', name); const xmlMutation = xmlUtils.createElement('mutation'); xmlMutation.setAttribute('name', name); - const params = this.getProcedureModel().getParameters(); - for (const param of params) { + for (let i = 0; i < this.arguments_.length; i++) { const xmlArg = xmlUtils.createElement('arg'); - xmlArg.setAttribute('name', param.getName()); + xmlArg.setAttribute('name', this.arguments_[i]); xmlMutation.appendChild(xmlArg); } const xmlBlock = xmlUtils.createElement('block'); @@ -726,64 +428,53 @@ const procedureDefContextMenuMixin = { options.push(option); // Add options to create getters for each parameter. - if (this.isCollapsed()) return; + if (!this.isCollapsed()) { + for (let i = 0; i < this.argumentVarModels_.length; i++) { + const argOption = {enabled: true}; + const argVar = this.argumentVarModels_[i]; + argOption.text = + Msg['VARIABLES_SET_CREATE_GET'].replace('%1', argVar.name); - for (const param of params) { - const argOption = {enabled: true}; - const argVar = param.getVariableModel(); - argOption.text = - Msg['VARIABLES_SET_CREATE_GET'].replace('%1', argVar.name); - - const argXmlField = Variables.generateVariableFieldDom(argVar); - const argXmlBlock = xmlUtils.createElement('block'); - argXmlBlock.setAttribute('type', 'variables_get'); - argXmlBlock.appendChild(argXmlField); - argOption.callback = ContextMenu.callbackFactory(this, argXmlBlock); - options.push(argOption); + const argXmlField = Variables.generateVariableFieldDom(argVar); + const argXmlBlock = xmlUtils.createElement('block'); + argXmlBlock.setAttribute('type', 'variables_get'); + argXmlBlock.appendChild(argXmlField); + argOption.callback = ContextMenu.callbackFactory(this, argXmlBlock); + options.push(argOption); + } } }, }; -Extensions.registerMixin( - 'procedure_def_context_menu_mixin', procedureDefContextMenuMixin); -const procedureDefOnChangeMixin = { - onchange: function(e) { - if (e.type === Events.BLOCK_CHANGE && e.blockId === this.id && - e.element === 'disabled') { - this.getProcedureModel().setEnabled(!e.newValue); +blocks['procedures_defnoreturn'] = { + ...PROCEDURE_DEF_COMMON, + /** + * Block for defining a procedure with no return value. + * @this {Block} + */ + init: function() { + const initName = Procedures.findLegalName('', this); + const nameField = new FieldTextInput(initName, Procedures.rename); + nameField.setSpellcheck(false); + this.appendDummyInput() + .appendField(Msg['PROCEDURES_DEFNORETURN_TITLE']) + .appendField(nameField, 'NAME') + .appendField('', 'PARAMS'); + this.setMutator(new Mutator(['procedures_mutatorarg'], this)); + if ((this.workspace.options.comments || + (this.workspace.options.parentWorkspace && + this.workspace.options.parentWorkspace.options.comments)) && + Msg['PROCEDURES_DEFNORETURN_COMMENT']) { + this.setCommentText(Msg['PROCEDURES_DEFNORETURN_COMMENT']); } + this.setStyle('procedure_blocks'); + this.setTooltip(Msg['PROCEDURES_DEFNORETURN_TOOLTIP']); + this.setHelpUrl(Msg['PROCEDURES_DEFNORETURN_HELPURL']); + this.arguments_ = []; + this.argumentVarModels_ = []; + this.setStatements_(true); + this.statementConnection_ = null; }, -}; -Extensions.registerMixin( - 'procedure_def_onchange_mixin', procedureDefOnChangeMixin); - -/** @this {Block} */ -const procedureDefNoReturnSetCommentHelper = function() { - if ((this.workspace.options.comments || - (this.workspace.options.parentWorkspace && - this.workspace.options.parentWorkspace.options.comments)) && - Msg['PROCEDURES_DEFNORETURN_COMMENT']) { - this.setCommentText(Msg['PROCEDURES_DEFNORETURN_COMMENT']); - } -}; -Extensions.register( - 'procedure_defnoreturn_set_comment_helper', - procedureDefNoReturnSetCommentHelper); - -/** @this {Block} */ -const procedureDefReturnSetCommentHelper = function() { - if ((this.workspace.options.comments || - (this.workspace.options.parentWorkspace && - this.workspace.options.parentWorkspace.options.comments)) && - Msg['PROCEDURES_DEFRETURN_COMMENT']) { - this.setCommentText(Msg['PROCEDURES_DEFRETURN_COMMENT']); - } -}; -Extensions.register( - 'procedure_defreturn_set_comment_helper', - procedureDefReturnSetCommentHelper); - -const procedureDefNoReturnGetCallerBlockMixin = { /** * Return the signature of this procedure definition. * @return {!Array} Tuple containing three elements: @@ -793,16 +484,43 @@ const procedureDefNoReturnGetCallerBlockMixin = { * @this {Block} */ getProcedureDef: function() { - return [this.getFieldValue('NAME'), this.getVars(), false]; + return [this.getFieldValue('NAME'), this.arguments_, false]; }, - callType_: 'procedures_callnoreturn', }; -Extensions.registerMixin( - 'procedure_defnoreturn_get_caller_block_mixin', - procedureDefNoReturnGetCallerBlockMixin); -const procedureDefReturnGetCallerBlockMixin = { +blocks['procedures_defreturn'] = { + ...PROCEDURE_DEF_COMMON, + /** + * Block for defining a procedure with a return value. + * @this {Block} + */ + init: function() { + const initName = Procedures.findLegalName('', this); + const nameField = new FieldTextInput(initName, Procedures.rename); + nameField.setSpellcheck(false); + this.appendDummyInput() + .appendField(Msg['PROCEDURES_DEFRETURN_TITLE']) + .appendField(nameField, 'NAME') + .appendField('', 'PARAMS'); + this.appendValueInput('RETURN') + .setAlign(Align.RIGHT) + .appendField(Msg['PROCEDURES_DEFRETURN_RETURN']); + this.setMutator(new Mutator(['procedures_mutatorarg'], this)); + if ((this.workspace.options.comments || + (this.workspace.options.parentWorkspace && + this.workspace.options.parentWorkspace.options.comments)) && + Msg['PROCEDURES_DEFRETURN_COMMENT']) { + this.setCommentText(Msg['PROCEDURES_DEFRETURN_COMMENT']); + } + this.setStyle('procedure_blocks'); + this.setTooltip(Msg['PROCEDURES_DEFRETURN_TOOLTIP']); + this.setHelpUrl(Msg['PROCEDURES_DEFRETURN_HELPURL']); + this.arguments_ = []; + this.argumentVarModels_ = []; + this.setStatements_(true); + this.statementConnection_ = null; + }, /** * Return the signature of this procedure definition. * @return {!Array} Tuple containing three elements: @@ -812,30 +530,66 @@ const procedureDefReturnGetCallerBlockMixin = { * @this {Block} */ getProcedureDef: function() { - return [this.getFieldValue('NAME'), this.getVars(), true]; + return [this.getFieldValue('NAME'), this.arguments_, true]; }, - callType_: 'procedures_callreturn', }; -Extensions.registerMixin( - 'procedure_defreturn_get_caller_block_mixin', - procedureDefReturnGetCallerBlockMixin); -/** @this {Block} */ -const procedureDefSetNoReturnHelper = function() { - this.getProcedureModel().setReturnTypes(null); +blocks['procedures_mutatorcontainer'] = { + /** + * Mutator block for procedure container. + * @this {Block} + */ + init: function() { + this.appendDummyInput().appendField( + Msg['PROCEDURES_MUTATORCONTAINER_TITLE']); + this.appendStatementInput('STACK'); + this.appendDummyInput('STATEMENT_INPUT') + .appendField(Msg['PROCEDURES_ALLOW_STATEMENTS']) + .appendField(new FieldCheckbox('TRUE'), 'STATEMENTS'); + this.setStyle('procedure_blocks'); + this.setTooltip(Msg['PROCEDURES_MUTATORCONTAINER_TOOLTIP']); + this.contextMenu = false; + }, }; -Extensions.register( - 'procedure_def_set_no_return_helper', procedureDefSetNoReturnHelper); -/** @this {Block} */ -const procedureDefSetReturnHelper = function() { - this.getProcedureModel().setReturnTypes([]); -}; -Extensions.register( - 'procedure_def_set_return_helper', procedureDefSetReturnHelper); +blocks['procedures_mutatorarg'] = { + /** + * Mutator block for procedure argument. + * @this {Block} + */ + init: function() { + const field = new FieldTextInput(Procedures.DEFAULT_ARG, this.validator_); + // Hack: override showEditor to do just a little bit more work. + // We don't have a good place to hook into the start of a text edit. + field.oldShowEditorFn_ = field.showEditor_; + /** + * @this {FieldTextInput} + */ + const newShowEditorFn = function() { + this.createdVariables_ = []; + this.oldShowEditorFn_(); + }; + field.showEditor_ = newShowEditorFn; + + this.appendDummyInput() + .appendField(Msg['PROCEDURES_MUTATORARG_TITLE']) + .appendField(field, 'NAME'); + this.setPreviousStatement(true); + this.setNextStatement(true); + this.setStyle('procedure_blocks'); + this.setTooltip(Msg['PROCEDURES_MUTATORARG_TOOLTIP']); + this.contextMenu = false; + + // Create the default variable when we drag the block in from the flyout. + // Have to do this after installing the field on the block. + field.onFinishEditing_ = this.deleteIntermediateVars_; + // Create an empty list so onFinishEditing_ has something to look at, even + // though the editor was never opened. + field.createdVariables_ = []; + field.onFinishEditing_('x'); + }, -const validateProcedureParamMixin = { /** * Obtain a valid name for the procedure argument. Create a variable if * necessary. @@ -887,7 +641,6 @@ const validateProcedureParamMixin = { this.createdVariables_.push(model); } } - return varName; }, @@ -895,8 +648,9 @@ const validateProcedureParamMixin = { * Called when focusing away from the text field. * Deletes all variables that were created as the user typed their intended * variable name. - * @this {FieldTextInput} * @param {string} newText The new variable name. + * @private + * @this {FieldTextInput} */ deleteIntermediateVars_: function(newText) { const outerWs = Mutator.findParentWs(this.getSourceBlock().workspace); @@ -911,346 +665,21 @@ const validateProcedureParamMixin = { } }, }; -Extensions.registerMixin( - 'procedure_mutatorarg_validate_mixin', validateProcedureParamMixin); - -/** @this {Block} */ -const addValidatorToParamFieldHelper = function() { - const nameField = this.getField('NAME'); - - nameField.setValidator(this.validator_); - - // TODO: We can probably just handle this in onFinishedEditing_. - // Hack: override showEditor to do just a little bit more work. - // We don't have a good place to hook into the start of a text edit. - nameField.oldShowEditorFn_ = nameField.showEditor_; - /** @this {FieldTextInput} */ - const newShowEditorFn = function() { - this.createdVariables_ = []; - this.oldShowEditorFn_(); - }; - nameField.showEditor_ = newShowEditorFn; - - nameField.onFinishEditing_ = this.deleteIntermediateVars_; - // Create an empty list so onFinishEditing_ has something to look at, even - // though the editor was never opened. - nameField.createdVariables_ = []; - nameField.onFinishEditing_('x'); -}; -Extensions.register( - 'procedure_mutatorarg_add_validator_helper', - addValidatorToParamFieldHelper); - -/** @this {Block} */ -const procedureCallerGetDefMixin = function() { - const mixin = { - model_: null, - - prevParams_: [], - - argsMap_: new Map(), - - /** - * @return {IProcedureModel} The procedure model associated with this - * block. - */ - getProcedureModel() { - return this.model_; - }, - - /** - * @param {string} name The name of the procedure model to find. - * @param {Array} params The param names of the procedure model - * to find. - * @return {IProcedureModel} The procedure model that was found. - */ - findProcedureModel_(name, params = []) { - const workspace = this.getTargetWorkspace_(); - const model = workspace.getProcedureMap().getProcedures().find( - (proc) => proc.getName() === name); - if (!model) return null; - - const returnTypes = model.getReturnTypes(); - const hasMatchingReturn = this.hasReturn_ ? returnTypes : !returnTypes; - if (!hasMatchingReturn) return null; - - const hasMatchingParams = - model.getParameters().every((p, i) => p.getName() === params[i]); - if (!hasMatchingParams) return null; - - return model; - }, - - /** - * @return {Workspace} The main workspace (i.e. not the flyout workspace) - * associated with this block. - */ - getTargetWorkspace_() { - return this.workspace.isFlyout ? this.workspace.targetWorkspace : - this.workspace; - }, - - /** - * Returns the name of the procedure this block calls. - * @return {string} Procedure name. - * @this {Block} - */ - getProcedureCall: function() { - // The NAME field is guaranteed to exist, null will never be returned. - return /** @type {string} */ (this.getFieldValue('NAME')); - }, - - /** - * True if this is a procedure definition block, false otherwise (i.e. - * it is a caller). - * @return {boolean} False because this is not a procedure definition block. - */ - isProcedureDef() { - return false; - }, - - /** - * Return all variables referenced by this block. - * @return {!Array} List of variable names. - * @this {Block} - */ - getVars: function() { - return this.getProcedureModel().getParameters().map( - (p) => p.getVariableModel().name); - }, - - /** - * Return all variables referenced by this block. - * @return {!Array} List of variable models. - * @this {Block} - */ - getVarModels: function() { - return this.getProcedureModel().getParameters().map( - (p) => p.getVariableModel()); - }, - }; - - this.mixin(mixin, true); -}; -// Using register instead of registerMixin to avoid triggering warnings about -// overriding built-ins. -Extensions.register( - 'procedure_caller_get_def_mixin', procedureCallerGetDefMixin); - -const procedureCallerMutator = { - previousEnabledState_: true, - - paramsFromSerializedState_: [], +/** + * Common properties for the procedure_callnoreturn and + * procedure_callreturn blocks. + */ +const PROCEDURE_CALL_COMMON = { /** - * Create XML to represent the (non-editable) name and arguments. - * Backwards compatible serialization implementation. - * @return {!Element} XML storage element. + * Returns the name of the procedure this block calls. + * @return {string} Procedure name. * @this {Block} */ - mutationToDom: function() { - const container = xmlUtils.createElement('mutation'); - const model = this.getProcedureModel(); - if (!model) return container; - - container.setAttribute('name', model.getName()); - for (const param of model.getParameters()) { - const arg = xmlUtils.createElement('arg'); - arg.setAttribute('name', param.getName()); - container.appendChild(arg); - } - return container; + getProcedureCall: function() { + // The NAME field is guaranteed to exist, null will never be returned. + return /** @type {string} */ (this.getFieldValue('NAME')); }, - - /** - * Parse XML to restore the (non-editable) name and parameters. - * Backwards compatible serialization implementation. - * @param {!Element} xmlElement XML storage element. - * @this {Block} - */ - domToMutation: function(xmlElement) { - const name = xmlElement.getAttribute('name'); - const params = []; - for (const n of xmlElement.childNodes) { - if (n.nodeName.toLowerCase() === 'arg') { - params.push(n.getAttribute('name')); - } - } - this.deserialize_(name, params); - }, - - /** - * Returns the state of this block as a JSON serializable object. - * @return {{name: string, params:(!Array|undefined)}} The state of - * this block, ie the params and procedure name. - */ - saveExtraState: function() { - const state = Object.create(null); - const model = this.getProcedureModel(); - if (!model) return state; - state['name'] = model.getName(); - if (model.getParameters().length) { - state['params'] = model.getParameters().map((p) => p.getName()); - } - return state; - }, - - /** - * Applies the given state to this block. - * @param {*} state The state to apply to this block, ie the params and - * procedure name. - */ - loadExtraState: function(state) { - this.deserialize_(state['name'], state['params'] || []); - }, - - /** - * Applies the given name and params from the serialized state to the block. - * @param {string} name The name to apply to the block. - * @param {!Array} params The parameters to apply to the block. - */ - deserialize_: function(name, params) { - this.setFieldValue(name, 'NAME'); - if (!this.model_) this.model_ = this.findProcedureModel_(name, params); - if (this.getProcedureModel()) { - this.initBlockWithProcedureModel_(); - } else { - // Create inputs based on the mutation so that children can be connected. - this.createArgInputs_(params); - } - this.paramsFromSerializedState_ = params; - }, -}; -Extensions.registerMutator('procedure_caller_mutator', procedureCallerMutator); - -const procedureCallerUpdateShapeMixin = { - /** - * Renders the block for the first time based on the procedure model. - */ - initBlockWithProcedureModel_() { - this.prevParams_ = [...this.getProcedureModel().getParameters()]; - this.doProcedureUpdate(); - }, - - /** - * Updates the shape of this block to reflect the state of the data model. - */ - doProcedureUpdate: function() { - if (!this.getProcedureModel()) return; - const id = this.getProcedureModel().getId(); - if (!this.getTargetWorkspace_().getProcedureMap().has(id)) { - this.dispose(); - return; - } - this.updateName_(); - this.updateEnabled_(); - this.updateParameters_(); - }, - - /** - * Updates the name field of this block to match the state of the data model. - */ - updateName_: function() { - const name = this.getProcedureModel().getName(); - this.setFieldValue(name, 'NAME'); - const baseMsg = this.outputConnection ? - Msg['PROCEDURES_CALLRETURN_TOOLTIP'] : - Msg['PROCEDURES_CALLNORETURN_TOOLTIP']; - this.setTooltip(baseMsg.replace('%1', name)); - }, - - /** - * Updates the enabled state of this block to match the state of the data - * model. - */ - updateEnabled_: function() { - if (!this.getProcedureModel().getEnabled()) { - this.previousEnabledState_ = this.isEnabled(); - this.setEnabled(false); - } else { - this.setEnabled(this.previousEnabledState_); - } - }, - - /** - * Updates the parameter fields/inputs of this block to match the state of the - * data model. - */ - updateParameters_: function() { - this.updateArgsMap_(); - this.deleteAllArgInputs_(); - this.addParametersLabel__(); - this.createArgInputs_(); - this.reattachBlocks_(); - this.prevParams_ = [...this.getProcedureModel().getParameters()]; - }, - - /** - * Saves a map of parameter IDs to target blocks attached to the inputs - * of this caller block. - */ - updateArgsMap_: function() { - for (const [i, p] of this.prevParams_.entries()) { - const target = this.getInputTargetBlock(`ARG${i}`); - if (target) this.argsMap_.set(p.getId(), target); - } - }, - - /** - * Deletes all the parameter inputs on this block. - */ - deleteAllArgInputs_: function() { - let i = 0; - while (this.getInput(`ARG${i}`)) { - this.removeInput(`ARG${i}`); - i++; - } - }, - - /** - * Adds or removes the parameter label to match the state of the data model. - */ - addParametersLabel__: function() { - const topRow = this.getInput('TOPROW'); - if (this.getProcedureModel().getParameters().length) { - if (!this.getField('WITH')) { - topRow.appendField(Msg['PROCEDURES_CALL_BEFORE_PARAMS'], 'WITH'); - topRow.init(); - } - } else if (this.getField('WITH')) { - topRow.removeField('WITH'); - } - }, - - /** - * Creates all of the parameter inputs to match the state of the data model. - * @param {Array} params The params to add to the block, or null to - * use the params defined in the procedure model. - */ - createArgInputs_: function(params = null) { - if (!params) { - params = this.getProcedureModel().getParameters().map((p) => p.getName()); - } - for (const [i, p] of params.entries()) { - this.appendValueInput(`ARG${i}`) - .appendField(new FieldLabel(p), `ARGNAME${i}`) - .setAlign(Align.RIGHT); - } - }, - - /** - * Reattaches blocks to this blocks' inputs based on the data saved in the - * argsMap_. - */ - reattachBlocks_: function() { - const params = this.getProcedureModel().getParameters(); - for (const [i, p] of params.entries()) { - if (!this.argsMap_.has(p.getId())) continue; - this.getInput(`ARG${i}`).connection.connect( - this.argsMap_.get(p.getId()).outputConnection); - } - }, - /** * Notification that a procedure is renaming. * If the name matches this block's procedure, rename it. @@ -1267,11 +696,225 @@ const procedureCallerUpdateShapeMixin = { this.setTooltip(baseMsg.replace('%1', newName)); } }, -}; -Extensions.registerMixin( - 'procedure_caller_update_shape_mixin', procedureCallerUpdateShapeMixin); + /** + * Notification that the procedure's parameters have changed. + * @param {!Array} paramNames New param names, e.g. ['x', 'y', 'z']. + * @param {!Array} paramIds IDs of params (consistent for each + * parameter through the life of a mutator, regardless of param renaming), + * e.g. ['piua', 'f8b_', 'oi.o']. + * @private + * @this {Block} + */ + setProcedureParameters_: function(paramNames, paramIds) { + // Data structures: + // this.arguments = ['x', 'y'] + // Existing param names. + // this.quarkConnections_ {piua: null, f8b_: Connection} + // Look-up of paramIds to connections plugged into the call block. + // this.quarkIds_ = ['piua', 'f8b_'] + // Existing param IDs. + // Note that quarkConnections_ may include IDs that no longer exist, but + // which might reappear if a param is reattached in the mutator. + const defBlock = + Procedures.getDefinition(this.getProcedureCall(), this.workspace); + const mutatorOpen = + defBlock && defBlock.mutator && defBlock.mutator.isVisible(); + if (!mutatorOpen) { + this.quarkConnections_ = {}; + this.quarkIds_ = null; + } else { + // fix #6091 - this call could cause an error when outside if-else + // expanding block while mutating prevents another error (ancient fix) + this.setCollapsed(false); + } + // Test arguments (arrays of strings) for changes. '\n' is not a valid + // argument name character, so it is a valid delimiter here. + if (paramNames.join('\n') === this.arguments_.join('\n')) { + // No change. + this.quarkIds_ = paramIds; + return; + } + if (paramIds.length !== paramNames.length) { + throw RangeError('paramNames and paramIds must be the same length.'); + } + if (!this.quarkIds_) { + // Initialize tracking for this block. + this.quarkConnections_ = {}; + this.quarkIds_ = []; + } + // Switch off rendering while the block is rebuilt. + const savedRendered = this.rendered; + this.rendered = false; + // Update the quarkConnections_ with existing connections. + for (let i = 0; i < this.arguments_.length; i++) { + const input = this.getInput('ARG' + i); + if (input) { + const connection = input.connection.targetConnection; + this.quarkConnections_[this.quarkIds_[i]] = connection; + if (mutatorOpen && connection && + paramIds.indexOf(this.quarkIds_[i]) === -1) { + // This connection should no longer be attached to this block. + connection.disconnect(); + connection.getSourceBlock().bumpNeighbours(); + } + } + } + // Rebuild the block's arguments. + this.arguments_ = [].concat(paramNames); + // And rebuild the argument model list. + this.argumentVarModels_ = []; + for (let i = 0; i < this.arguments_.length; i++) { + const variable = Variables.getOrCreateVariablePackage( + this.workspace, null, this.arguments_[i], ''); + this.argumentVarModels_.push(variable); + } -const procedureCallerOnChangeMixin = { + this.updateShape_(); + this.quarkIds_ = paramIds; + // Reconnect any child blocks. + if (this.quarkIds_) { + for (let i = 0; i < this.arguments_.length; i++) { + const quarkId = this.quarkIds_[i]; + if (quarkId in this.quarkConnections_) { + const connection = this.quarkConnections_[quarkId]; + if (!Mutator.reconnect(connection, this, 'ARG' + i)) { + // Block no longer exists or has been attached elsewhere. + delete this.quarkConnections_[quarkId]; + } + } + } + } + // Restore rendering and show the changes. + this.rendered = savedRendered; + if (this.rendered) { + this.render(); + } + }, + /** + * Modify this block to have the correct number of arguments. + * @private + * @this {Block} + */ + updateShape_: function() { + for (let i = 0; i < this.arguments_.length; i++) { + const argField = this.getField('ARGNAME' + i); + if (argField) { + // Ensure argument name is up to date. + // The argument name field is deterministic based on the mutation, + // no need to fire a change event. + Events.disable(); + try { + argField.setValue(this.arguments_[i]); + } finally { + Events.enable(); + } + } else { + // Add new input. + const newField = new FieldLabel(this.arguments_[i]); + const input = this.appendValueInput('ARG' + i) + .setAlign(Align.RIGHT) + .appendField(newField, 'ARGNAME' + i); + input.init(); + } + } + // Remove deleted inputs. + for (let i = this.arguments_.length; this.getInput('ARG' + i); i++) { + this.removeInput('ARG' + i); + } + // Add 'with:' if there are parameters, remove otherwise. + const topRow = this.getInput('TOPROW'); + if (topRow) { + if (this.arguments_.length) { + if (!this.getField('WITH')) { + topRow.appendField(Msg['PROCEDURES_CALL_BEFORE_PARAMS'], 'WITH'); + topRow.init(); + } + } else { + if (this.getField('WITH')) { + topRow.removeField('WITH'); + } + } + } + }, + /** + * Create XML to represent the (non-editable) name and arguments. + * Backwards compatible serialization implementation. + * @return {!Element} XML storage element. + * @this {Block} + */ + mutationToDom: function() { + const container = xmlUtils.createElement('mutation'); + container.setAttribute('name', this.getProcedureCall()); + for (let i = 0; i < this.arguments_.length; i++) { + const parameter = xmlUtils.createElement('arg'); + parameter.setAttribute('name', this.arguments_[i]); + container.appendChild(parameter); + } + return container; + }, + /** + * Parse XML to restore the (non-editable) name and parameters. + * Backwards compatible serialization implementation. + * @param {!Element} xmlElement XML storage element. + * @this {Block} + */ + domToMutation: function(xmlElement) { + const name = xmlElement.getAttribute('name'); + this.renameProcedure(this.getProcedureCall(), name); + const args = []; + const paramIds = []; + for (let i = 0, childNode; (childNode = xmlElement.childNodes[i]); i++) { + if (childNode.nodeName.toLowerCase() === 'arg') { + args.push(childNode.getAttribute('name')); + paramIds.push(childNode.getAttribute('paramId')); + } + } + this.setProcedureParameters_(args, paramIds); + }, + /** + * Returns the state of this block as a JSON serializable object. + * @return {{name: string, params:(!Array|undefined)}} The state of + * this block, ie the params and procedure name. + */ + saveExtraState: function() { + const state = Object.create(null); + state['name'] = this.getProcedureCall(); + if (this.arguments_.length) { + state['params'] = this.arguments_; + } + return state; + }, + /** + * Applies the given state to this block. + * @param {*} state The state to apply to this block, ie the params and + * procedure name. + */ + loadExtraState: function(state) { + this.renameProcedure(this.getProcedureCall(), state['name']); + const params = state['params']; + if (params) { + const ids = []; + ids.length = params.length; + ids.fill(null); + this.setProcedureParameters_(params, ids); + } + }, + /** + * Return all variables referenced by this block. + * @return {!Array} List of variable names. + * @this {Block} + */ + getVars: function() { + return this.arguments_; + }, + /** + * Return all variables referenced by this block. + * @return {!Array} List of variable models. + * @this {Block} + */ + getVarModels: function() { + return this.argumentVarModels_; + }, /** * Procedure calls cannot exist without the corresponding procedure * definition. Enforce this link whenever an event is fired. @@ -1288,72 +931,90 @@ const procedureCallerOnChangeMixin = { return; } if (event.type === Events.BLOCK_CREATE && - (event.blockId === this.id || event.ids.indexOf(this.id) !== -1)) { + event.ids.indexOf(this.id) !== -1) { // Look for the case where a procedure call was created (usually through // paste) and there is no matching definition. In this case, create // an empty definition block with the correct signature. const name = this.getProcedureCall(); let def = Procedures.getDefinition(name, this.workspace); - if (!this.defMatches_(def)) def = null; + if (def && + (def.type !== this.defType_ || + JSON.stringify(def.getVars()) !== JSON.stringify(this.arguments_))) { + // The signatures don't match. + def = null; + } if (!def) { - // We have no def nor procedure model. Events.setGroup(event.group); - this.model_ = this.createDef_( - this.getFieldValue('NAME'), this.paramsFromSerializedState_); + /** + * Create matching definition block. + * + * + * + * + * + * test + * + * + */ + const xml = xmlUtils.createElement('xml'); + const block = xmlUtils.createElement('block'); + block.setAttribute('type', this.defType_); + const xy = this.getRelativeToSurfaceXY(); + const x = xy.x + config.snapRadius * (this.RTL ? -1 : 1); + const y = xy.y + config.snapRadius * 2; + block.setAttribute('x', x); + block.setAttribute('y', y); + const mutation = this.mutationToDom(); + block.appendChild(mutation); + const field = xmlUtils.createElement('field'); + field.setAttribute('name', 'NAME'); + const callName = this.getProcedureCall(); + const newName = Procedures.findLegalName(callName, this); + if (callName !== newName) { + this.renameProcedure(callName, newName); + } + field.appendChild(xmlUtils.createTextNode(callName)); + block.appendChild(field); + xml.appendChild(block); + Xml.domToWorkspace(xml, this.workspace); Events.setGroup(false); } - if (!this.getProcedureModel()) { - // We have a def, but no reference to its model. - this.model_ = this.findProcedureModel_( - this.getFieldValue('NAME'), this.paramsFromSerializedState_); + } else if (event.type === Events.BLOCK_DELETE) { + // Look for the case where a procedure definition has been deleted, + // leaving this block (a procedure call) orphaned. In this case, delete + // the orphan. + const name = this.getProcedureCall(); + const def = Procedures.getDefinition(name, this.workspace); + if (!def) { + Events.setGroup(event.group); + this.dispose(true); + Events.setGroup(false); + } + } else if (event.type === Events.CHANGE && event.element === 'disabled') { + const name = this.getProcedureCall(); + const def = Procedures.getDefinition(name, this.workspace); + if (def && def.id === event.blockId) { + // in most cases the old group should be '' + const oldGroup = Events.getGroup(); + if (oldGroup) { + // This should only be possible programmatically and may indicate a + // problem with event grouping. If you see this message please + // investigate. If the use ends up being valid we may need to reorder + // events in the undo stack. + console.log( + 'Saw an existing group while responding to a definition change'); + } + Events.setGroup(event.group); + if (event.newValue) { + this.previousEnabledState_ = this.isEnabled(); + this.setEnabled(false); + } else { + this.setEnabled(this.previousEnabledState_); + } + Events.setGroup(oldGroup); } - this.initBlockWithProcedureModel_(); } }, - - /** - * Returns true if the given def block matches the definition of this caller - * block. - * @param {Block} defBlock The definition block to check against. - * @return {boolean} Whether the def block matches or not. - */ - defMatches_(defBlock) { - return defBlock && defBlock.type === this.defType_ && - JSON.stringify(defBlock.getVars()) === - JSON.stringify(this.paramsFromSerializedState_); - }, - - /** - * Creates a procedure definition block with the given name and params, - * and returns the procedure model associated with it. - * @param {string} name The name of the procedure to create. - * @param {Array} params The names of the parameters to create. - * @return {IProcedureModel} The procedure model associated with the new - * procedure definition block. - */ - createDef_(name, params = []) { - const xy = this.getRelativeToSurfaceXY(); - const newName = Procedures.findLegalName(name, this); - this.renameProcedure(name, newName); - - const blockDef = { - 'type': this.defType_, - 'x': xy.x + config.snapRadius * (this.RTL ? -1 : 1), - 'y': xy.y + config.snapRadius * 2, - 'extraState': { - 'params': params.map((p) => ({'name': p})), - }, - 'fields': {'NAME': newName}, - }; - return serialization.blocks - .append(blockDef, this.getTargetWorkspace_(), {recordUndo: true}) - .getProcedureModel(); - }, -}; -Extensions.registerMixin( - 'procedure_caller_onchange_mixin', procedureCallerOnChangeMixin); - -const procedureCallerContextMenuMixin = { /** * Add menu option to find the definition block for this call. * @param {!Array} options List of menu options to add to. @@ -1380,28 +1041,71 @@ const procedureCallerContextMenuMixin = { options.push(option); }, }; -Extensions.registerMixin( - 'procedure_caller_context_menu_mixin', procedureCallerContextMenuMixin); -const procedureCallerNoReturnGetDefBlockMixin = { - hasReturn_: false, +blocks['procedures_callnoreturn'] = { + ...PROCEDURE_CALL_COMMON, + /** + * Block for calling a procedure with no return value. + * @this {Block} + */ + init: function() { + this.appendDummyInput('TOPROW').appendField('', 'NAME'); + this.setPreviousStatement(true); + this.setNextStatement(true); + this.setStyle('procedure_blocks'); + // Tooltip is set in renameProcedure. + this.setHelpUrl(Msg['PROCEDURES_CALLNORETURN_HELPURL']); + this.arguments_ = []; + this.argumentVarModels_ = []; + this.quarkConnections_ = {}; + this.quarkIds_ = null; + this.previousEnabledState_ = true; + }, + defType_: 'procedures_defnoreturn', }; -Extensions.registerMixin( - 'procedure_callernoreturn_get_def_block_mixin', - procedureCallerNoReturnGetDefBlockMixin); -const procedureCallerReturnGetDefBlockMixin = { - hasReturn_: true, +blocks['procedures_callreturn'] = { + ...PROCEDURE_CALL_COMMON, + /** + * Block for calling a procedure with a return value. + * @this {Block} + */ + init: function() { + this.appendDummyInput('TOPROW').appendField('', 'NAME'); + this.setOutput(true); + this.setStyle('procedure_blocks'); + // Tooltip is set in domToMutation. + this.setHelpUrl(Msg['PROCEDURES_CALLRETURN_HELPURL']); + this.arguments_ = []; + this.argumentVarModels_ = []; + this.quarkConnections_ = {}; + this.quarkIds_ = null; + this.previousEnabledState_ = true; + }, + defType_: 'procedures_defreturn', }; -Extensions.registerMixin( - 'procedure_callerreturn_get_def_block_mixin', - procedureCallerReturnGetDefBlockMixin); - -const procedureIfReturnMutator = { - hasReturnValue_: true, +blocks['procedures_ifreturn'] = { + /** + * Block for conditionally returning a value from a procedure. + * @this {Block} + */ + init: function() { + this.appendValueInput('CONDITION') + .setCheck('Boolean') + .appendField(Msg['CONTROLS_IF_MSG_IF']); + this.appendValueInput('VALUE').appendField( + Msg['PROCEDURES_DEFRETURN_RETURN']); + this.setInputsInline(true); + this.setPreviousStatement(true); + this.setNextStatement(true); + this.setStyle('procedure_blocks'); + this.setTooltip(Msg['PROCEDURES_IFRETURN_TOOLTIP']); + this.setHelpUrl(Msg['PROCEDURES_IFRETURN_HELPURL']); + this.hasReturnValue_ = true; + }, /** * Create XML to represent whether this block has a return value. * @return {!Element} XML storage element. @@ -1412,7 +1116,6 @@ const procedureIfReturnMutator = { container.setAttribute('value', Number(this.hasReturnValue_)); return container; }, - /** * Parse XML to restore whether this block has a return value. * @param {!Element} xmlElement XML storage element. @@ -1432,12 +1135,7 @@ const procedureIfReturnMutator = { // loadExtraState) because the state of this block is already encoded in the // block's position in the workspace. // XML hooks are kept for backwards compatibility. -}; -Extensions.registerMutator( - 'procedure_ifreturn_mutator', procedureIfReturnMutator); - -const procedureIfReturnOnChangeMixin = { /** * Called whenever anything on the workspace changes. * Add warning if this flow block is not nested inside a loop. @@ -1485,12 +1183,6 @@ const procedureIfReturnOnChangeMixin = { Events.setGroup(group); } }, -}; - -Extensions.registerMixin( - 'procedure_ifreturn_onchange_mixin', procedureIfReturnOnChangeMixin); - -const procedureIfReturnFunctionTypesMixin = { /** * List of block types that are functions and thus do not need warnings. * To add a new function type add this to your code: @@ -1499,9 +1191,5 @@ const procedureIfReturnFunctionTypesMixin = { FUNCTION_TYPES: ['procedures_defnoreturn', 'procedures_defreturn'], }; -Extensions.registerMixin( - 'procedure_ifreturn_function_types_mixin', - procedureIfReturnFunctionTypesMixin); - // Register provided blocks. defineBlocks(blocks); diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index 24a908a45..42c486042 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -7,7 +7,6 @@ goog.declareModuleId('Blockly.test.procedures'); import * as Blockly from '../../../build/src/core/blockly.js'; -import {ObservableParameterModel} from '../../../build/src/core/procedures.js'; import {assertCallBlockStructure, assertDefBlockStructure, createProcDefBlock, createProcCallBlock} from '../test_helpers/procedures.js'; import {runSerializationTestSuite} from '../test_helpers/serialization.js'; import {createGenUidStubWithReturns, sharedTestSetup, sharedTestTeardown, workspaceTeardown} from '../test_helpers/setup_teardown.js'; @@ -28,712 +27,6 @@ suite('Procedures', function() { sharedTestTeardown.call(this); }); - suite('updating data models', function() { - test( - 'renaming a procedure def block updates the procedure model', - function() { - const defBlock = createProcDefBlock(this.workspace); - - defBlock.setFieldValue('new name', 'NAME'); - - chai.assert.equal( - defBlock.getProcedureModel().getName(), - 'new name', - 'Expected the procedure model name to be updated'); - }); - - test( - 'disabling a procedure def block updates the procedure model', - function() { - const defBlock = createProcDefBlock(this.workspace); - - defBlock.setEnabled(false); - this.clock.runAll(); - - chai.assert.isFalse( - defBlock.getProcedureModel().getEnabled(), - 'Expected the procedure model to be disabled'); - }); - - test( - 'adding a parameter to a procedure def updates the procedure model', - function() { - // Create a stack of container, parameter. - const defBlock = createProcDefBlock(this.workspace); - defBlock.mutator.setVisible(true); - const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = mutatorWorkspace.getTopBlocks()[0]; - const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); - paramBlock.setFieldValue('param name', 'NAME'); - containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - this.clock.runAll(); - - chai.assert.equal( - defBlock.getProcedureModel().getParameter(0).getName(), - 'param name', - 'Expected the procedure model to have a matching parameter'); - }); - - test('adding a parameter adds a variable to the variable map', function() { - // Create a stack of container, parameter. - const defBlock = createProcDefBlock(this.workspace); - defBlock.mutator.setVisible(true); - const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = mutatorWorkspace.getTopBlocks()[0]; - const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); - paramBlock.setFieldValue('param name', 'NAME'); - containerBlock.getInput('STACK').connection - .connect(paramBlock.previousConnection); - this.clock.runAll(); - - chai.assert.isTrue( - this.workspace.getVariableMap().getVariablesOfType('') - .some((variable) => variable.name === 'param name'), - 'Expected the variable map to have a matching variable'); - }); - - - test( - 'moving a parameter in the procedure def updates the procedure model', - function() { - // Create a stack of container, param1, param2. - const defBlock = createProcDefBlock(this.workspace); - defBlock.mutator.setVisible(true); - const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = mutatorWorkspace.getTopBlocks()[0]; - const paramBlock1 = mutatorWorkspace.newBlock('procedures_mutatorarg'); - paramBlock1.setFieldValue('param name1', 'NAME'); - const paramBlock2 = mutatorWorkspace.newBlock('procedures_mutatorarg'); - paramBlock2.setFieldValue('param name2', 'NAME'); - containerBlock.getInput('STACK').connection - .connect(paramBlock1.previousConnection); - paramBlock1.nextConnection.connect(paramBlock2.previousConnection); - this.clock.runAll(); - const id1 = defBlock.getProcedureModel().getParameter(0).getId(); - const id2 = defBlock.getProcedureModel().getParameter(1).getId(); - - // Reconfigure the stack to be container, param2, param1. - paramBlock2.previousConnection.disconnect(); - paramBlock1.previousConnection.disconnect(); - containerBlock.getInput('STACK').connection - .connect(paramBlock2.previousConnection); - paramBlock2.nextConnection.connect(paramBlock1.previousConnection); - this.clock.runAll(); - - chai.assert.equal( - defBlock.getProcedureModel().getParameter(0).getName(), - 'param name2', - 'Expected the first parameter of the procedure to be param 2'); - chai.assert.equal( - defBlock.getProcedureModel().getParameter(0).getId(), - id2, - 'Expected the first parameter of the procedure to be param 2'); - chai.assert.equal( - defBlock.getProcedureModel().getParameter(1).getName(), - 'param name1', - 'Expected the second parameter of the procedure to be param 1'); - chai.assert.equal( - defBlock.getProcedureModel().getParameter(1).getId(), - id1, - 'Expected the second parameter of the procedure to be param 1'); - }); - - test('decomposing and recomposing maintains parameter IDs', function() { - // Create a stack of container, param. - const defBlock = createProcDefBlock(this.workspace); - defBlock.mutator.setVisible(true); - const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = mutatorWorkspace.getTopBlocks()[0]; - const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); - paramBlock.setFieldValue('param name', 'NAME'); - containerBlock.getInput('STACK').connection - .connect(paramBlock.previousConnection); - this.clock.runAll(); - const paramBlockId = defBlock.getProcedureModel().getParameter(0).getId(); - - Blockly.Events.disable(); - mutatorWorkspace.clear(); - Blockly.Events.enable(); - const container = defBlock.decompose(mutatorWorkspace); - defBlock.compose(container); - - chai.assert.equal( - defBlock.getProcedureModel().getParameter(0).getId(), - paramBlockId, - 'Expected the parameter ID to be maintained'); - }); - - test( - 'deleting a parameter from a procedure def updates the procedure model', - function() { - // Create a stack of container, parameter. - const defBlock = createProcDefBlock(this.workspace); - defBlock.mutator.setVisible(true); - const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = mutatorWorkspace.getTopBlocks()[0]; - const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); - paramBlock.setFieldValue('param name', 'NAME'); - containerBlock.getInput('STACK').connection - .connect(paramBlock.previousConnection); - this.clock.runAll(); - - containerBlock.getInput('STACK').connection.disconnect(); - this.clock.runAll(); - - chai.assert.isEmpty( - defBlock.getProcedureModel().getParameters(), - 'Expected the procedure model to have no parameters'); - }); - - test('renaming a procedure parameter updates the parameter model', function() { - // Create a stack of container, parameter. - const defBlock = createProcDefBlock(this.workspace); - defBlock.mutator.setVisible(true); - const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = mutatorWorkspace.getTopBlocks()[0]; - const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); - paramBlock.setFieldValue('param name', 'NAME'); - containerBlock.getInput('STACK').connection - .connect(paramBlock.previousConnection); - this.clock.runAll(); - - paramBlock.setFieldValue('new param name', 'NAME'); - this.clock.runAll(); - - chai.assert.equal( - defBlock.getProcedureModel().getParameter(0).getName(), - 'new param name', - 'Expected the procedure model to have a matching parameter'); - }); - - test('deleting a procedure deletes the procedure model', function() { - const defBlock = createProcDefBlock(this.workspace); - const model = defBlock.getProcedureModel(); - defBlock.dispose(); - - chai.assert.isUndefined( - this.workspace.getProcedureMap().get(model.getId()), - 'Expected the model to be removed from the procedure map'); - }); - }); - - suite('responding to data model updates', function() { - suite('def blocks', function() { - test('renaming the procedure data model updates blocks', function() { - const defBlock = createProcDefBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - - procModel.setName('new name'); - - chai.assert.equal( - defBlock.getFieldValue('NAME'), - 'new name', - 'Expected the procedure block to be renamed'); - }); - - test('disabling a procedure data model disables blocks', function() { - const defBlock = createProcDefBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - - procModel.setEnabled(false); - - chai.assert.isFalse( - defBlock.isEnabled(), - 'Expected the procedure block to be disabled'); - }); - - test('adding a parameter to a data model updates blocks', function() { - const defBlock = createProcDefBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - - procModel.insertParameter( - new ObservableParameterModel(this.workspace, 'param1', 'id'), 0); - - chai.assert.isNotNull( - defBlock.getField('PARAMS'), - 'Expected the params field to exist'); - chai.assert.isTrue( - defBlock.getFieldValue('PARAMS').includes('param1'), - 'Expected the params field to contain the name of the new param'); - }); - - test('moving a parameter in the data model updates blocks', function() { - const defBlock = createProcDefBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - const param1 = - new ObservableParameterModel(this.workspace, 'param1', 'id1'); - const param2 = - new ObservableParameterModel(this.workspace, 'param2', 'id2'); - procModel.insertParameter(param1, 0); - procModel.insertParameter(param2, 1); - - procModel.deleteParameter(1); - procModel.insertParameter(param2, 0); - - chai.assert.isNotNull( - defBlock.getField('PARAMS'), - 'Expected the params field to exist'); - chai.assert.isTrue( - defBlock.getFieldValue('PARAMS').includes('param2, param1'), - 'Expected the params field order to match the parameter order'); - }); - - test( - 'deleting a parameter from the data model updates blocks', - function() { - const defBlock = createProcDefBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - const param1 = - new ObservableParameterModel(this.workspace, 'param1', 'id1'); - const param2 = - new ObservableParameterModel(this.workspace, 'param2', 'id2'); - procModel.insertParameter(param1, 0); - procModel.insertParameter(param2, 1); - - procModel.deleteParameter(0); - - chai.assert.isNotNull( - defBlock.getField('PARAMS'), - 'Expected the params field to exist'); - chai.assert.isTrue( - defBlock.getFieldValue('PARAMS').includes('param2'), - 'Expected the params field order to contain one parameter'); - chai.assert.isFalse( - defBlock.getFieldValue('PARAMS').includes('param1'), - 'Expected the params field to not contain the deleted parameter'); - }); - - test( - 'renaming a procedure parameter in the data model updates blocks', - function() { - const defBlock = createProcDefBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - const param1 = - new ObservableParameterModel(this.workspace, 'param1', 'id1'); - procModel.insertParameter(param1, 0); - - param1.setName('new name'); - - chai.assert.isNotNull( - defBlock.getField('PARAMS'), - 'Expected the params field to exist'); - chai.assert.isTrue( - defBlock.getFieldValue('PARAMS').includes('new name'), - 'Expected the params field to contain the new param name'); - }); - }); - - suite('caller blocks', function() { - test('renaming the procedure data model updates blocks', function() { - const defBlock = createProcDefBlock(this.workspace); - const callBlock = createProcCallBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - - procModel.setName('new name'); - - chai.assert.equal( - callBlock.getFieldValue('NAME'), - 'new name', - 'Expected the procedure block to be renamed'); - }); - - test('disabling a procedure data model disables blocks', function() { - const defBlock = createProcDefBlock(this.workspace); - const callBlock = createProcCallBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - - procModel.setEnabled(false); - - chai.assert.isFalse( - callBlock.isEnabled(), - 'Expected the procedure block to be disabled'); - }); - - test('adding a parameter to a data model updates blocks', function() { - const defBlock = createProcDefBlock(this.workspace); - const callBlock = createProcCallBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - - procModel.insertParameter( - new ObservableParameterModel(this.workspace, 'param1', 'id'), 0); - - chai.assert.isNotNull( - callBlock.getInput('ARG0'), - 'Expected the param input to exist'); - chai.assert.equal( - callBlock.getFieldValue('ARGNAME0'), - 'param1', - 'Expected the params field to match the name of the new param'); - }); - - test('moving a parameter in the data model updates blocks', function() { - const defBlock = createProcDefBlock(this.workspace); - const callBlock = createProcCallBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - const param1 = - new ObservableParameterModel(this.workspace, 'param1', 'id1'); - const param2 = - new ObservableParameterModel(this.workspace, 'param2', 'id2'); - procModel.insertParameter(param1, 0); - procModel.insertParameter(param2, 1); - - procModel.deleteParameter(1); - procModel.insertParameter(param2, 0); - - chai.assert.isNotNull( - callBlock.getInput('ARG0'), - 'Expected the first param input to exist'); - chai.assert.isNotNull( - callBlock.getInput('ARG1'), - 'Expected the second param input to exist'); - chai.assert.equal( - callBlock.getFieldValue('ARGNAME0'), - 'param2', - 'Expected the first params field to match the name of the param'); - chai.assert.equal( - callBlock.getFieldValue('ARGNAME1'), - 'param1', - 'Expected the second params field to match the name of the param'); - }); - - test( - 'moving a parameter in the data model moves input blocks', - function() { - const defBlock = createProcDefBlock(this.workspace); - const callBlock = createProcCallBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - const param1 = - new ObservableParameterModel(this.workspace, 'param1', 'id1'); - const param2 = - new ObservableParameterModel(this.workspace, 'param2', 'id2'); - procModel.insertParameter(param1, 0); - procModel.insertParameter(param2, 1); - const rowBlock1 = this.workspace.newBlock('row_block'); - const rowBlock2 = this.workspace.newBlock('row_block'); - callBlock.getInput('ARG0').connection - .connect(rowBlock1.outputConnection); - callBlock.getInput('ARG1').connection - .connect(rowBlock2.outputConnection); - - procModel.deleteParameter(1); - procModel.insertParameter(param2, 0); - - chai.assert.isNotNull( - callBlock.getInput('ARG0'), - 'Expected the first param input to exist'); - chai.assert.equal( - callBlock.getInputTargetBlock('ARG0'), - rowBlock2, - 'Expected the second row block to be attached to the first input'); - chai.assert.isNotNull( - callBlock.getInput('ARG1'), - 'Expected the second param input to exist'); - chai.assert.equal( - callBlock.getInputTargetBlock('ARG1'), - rowBlock1, - 'Expected the first row block to be attached to the second input'); - }); - - test( - 'deleting a parameter from the data model updates blocks', - function() { - const defBlock = createProcDefBlock(this.workspace); - const callBlock = createProcCallBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - const param1 = - new ObservableParameterModel(this.workspace, 'param1', 'id1'); - const param2 = - new ObservableParameterModel(this.workspace, 'param2', 'id2'); - procModel.insertParameter(param1, 0); - procModel.insertParameter(param2, 1); - - procModel.deleteParameter(0); - - chai.assert.isNotNull( - callBlock.getInput('ARG0'), - 'Expected the first param input to exist'); - chai.assert.isNull( - callBlock.getInput('ARG1'), - 'Expected the second param input to not exist'); - chai.assert.equal( - callBlock.getFieldValue('ARGNAME0'), - 'param2', - 'Expected the first params field to match the name of the param'); - }); - - test( - 'renaming a procedure parameter in the data model updates blocks', - function() { - const defBlock = createProcDefBlock(this.workspace); - const callBlock = createProcCallBlock(this.workspace); - const procModel = defBlock.getProcedureModel(); - const param1 = - new ObservableParameterModel(this.workspace, 'param1', 'id1'); - procModel.insertParameter(param1, 0); - - param1.setName('new name'); - - chai.assert.isNotNull( - callBlock.getInput('ARG0'), - 'Expected the param input to exist'); - chai.assert.equal( - callBlock.getFieldValue('ARGNAME0'), - 'new name', - 'Expected the params field to match the new name of the param'); - }); - }); - }); - - suite('deserializing data models', function() { - suite('return types', function() { - test('procedure defs without returns have null return types', function() { - const json = { - 'blocks': { - 'languageVersion': 0, - 'blocks': [ - { - 'type': 'procedures_defnoreturn', - 'fields': { - 'NAME': 'test name', - }, - }, - ], - }, - }; - Blockly.serialization.workspaces.load(json, this.workspace); - const procedureModel = - this.workspace.getProcedureMap().getProcedures()[0]; - - chai.assert.isNull( - procedureModel.getReturnTypes(), - 'Expected the return types to be null'); - }); - - test('procedure defs with returns have array return types', function() { - const json = { - 'blocks': { - 'languageVersion': 0, - 'blocks': [ - { - 'type': 'procedures_defreturn', - 'fields': { - 'NAME': 'test name', - }, - }, - ], - }, - }; - Blockly.serialization.workspaces.load(json, this.workspace); - const procedureModel = - this.workspace.getProcedureMap().getProcedures()[0]; - - chai.assert.isArray( - procedureModel.getReturnTypes(), - 'Expected the return types to be an array'); - }); - }); - - suite('json', function() { - test('procedure names get deserialized', function() { - const json = { - 'blocks': { - 'languageVersion': 0, - 'blocks': [ - { - 'type': 'procedures_defnoreturn', - 'fields': { - 'NAME': 'test name', - }, - }, - ], - }, - }; - Blockly.serialization.workspaces.load(json, this.workspace); - const procedureModel = - this.workspace.getProcedureMap().getProcedures()[0]; - - chai.assert.equal( - procedureModel.name, - 'test name', - 'Expected the name of the procedure model to equal the name ' + - 'being deserialized.'); - }); - - test('procedure parameter names get deserialized', function() { - const json = { - 'blocks': { - 'languageVersion': 0, - 'blocks': [ - { - 'type': 'procedures_defnoreturn', - 'fields': { - 'NAME': 'test name', - }, - 'extraState': { - 'params': [ - { - 'id': 'test id 1', - 'name': 'test name 1', - }, - { - 'id': 'test id 2', - 'name': 'test name 2', - }, - ], - }, - }, - ], - }, - }; - Blockly.serialization.workspaces.load(json, this.workspace); - const procedureModel = - this.workspace.getProcedureMap().getProcedures()[0]; - - chai.assert.equal( - procedureModel.getParameter(0).getName(), - 'test name 1', - 'Expected the name of the first parameter to equal the name ' + - 'being deserialized.'); - chai.assert.equal( - procedureModel.getParameter(1).getName(), - 'test name 2', - 'Expected the name of the second parameter to equal the name ' + - 'being deserialized.'); - }); - - test('procedure variables get matching IDs', function() { - const json = { - 'blocks': { - 'languageVersion': 0, - 'blocks': [ - { - 'type': 'procedures_defnoreturn', - 'extraState': { - 'params': [ - { - 'name': 'test param name', - 'id': 'test param id', - }, - ], - }, - 'fields': { - 'NAME': 'test proc name', - }, - }, - ], - }, - 'variables': [ - { - 'name': 'test param name', - 'id': 'test param id', - }, - ], - }; - Blockly.serialization.workspaces.load(json, this.workspace); - const procedureModel = - this.workspace.getProcedureMap().getProcedures()[0]; - - chai.assert.equal( - procedureModel.getParameter(0).getVariableModel().getId(), - 'test param id', - 'Expected the variable id to match the serialized param id'); - }); - }); - - suite('xml', function() { - test('procedure names get deserialized', function() { - const xml = Blockly.Xml.textToDom( - `` + - ` test name` + - ``); - Blockly.Xml.domToBlock(xml, this.workspace); - const procedureModel = - this.workspace.getProcedureMap().getProcedures()[0]; - - chai.assert.equal( - procedureModel.name, - 'test name', - 'Expected the name of the procedure model to equal the name ' + - 'being deserialized.'); - }); - - test('procedure parameter names get deserialized', function() { - const xml = Blockly.Xml.textToDom( - `` + - ` ` + - ` ` + - ` ` + - ` ` + - ` test name` + - ``); - Blockly.Xml.domToBlock(xml, this.workspace); - const procedureModel = - this.workspace.getProcedureMap().getProcedures()[0]; - - chai.assert.equal( - procedureModel.getParameter(0).getName(), - 'test name 1', - 'Expected the name of the first parameter to equal the name ' + - 'being deserialized.'); - chai.assert.equal( - procedureModel.getParameter(1).getName(), - 'test name 2', - 'Expected the name of the second parameter to equal the name ' + - 'being deserialized.'); - }); - - test('procedure variables get matching IDs', function() { - const json = { - 'blocks': { - 'languageVersion': 0, - 'blocks': [ - { - 'type': 'procedures_defnoreturn', - 'extraState': { - 'params': [ - { - 'name': 'test param name', - 'id': 'test param id', - }, - ], - }, - 'fields': { - 'NAME': 'test proc name', - }, - }, - ], - }, - 'variables': [ - { - 'name': 'test param name', - 'id': 'test param id', - }, - ], - }; - const xml = Blockly.Xml.textToDom( - `` + - ` ` + - ` test param name` + - ` ` + - ` ` + - ` ` + - ` ` + - ` ` + - ` test name` + - ` ` + - ``); - Blockly.Xml.domToWorkspace(xml, this.workspace); - const procedureModel = - this.workspace.getProcedureMap().getProcedures()[0]; - - chai.assert.equal( - procedureModel.getParameter(0).getVariableModel().getId(), - 'test param id', - 'Expected the variable id to match the serialized param id'); - }); - }); - }); - suite('renaming procedures', function() { test('callers are updated to have the new name', function() { const defBlock = createProcDefBlock(this.workspace); @@ -1459,6 +752,7 @@ suite('Procedures', function() { const callBlock1 = createProcCallBlock(this.workspace); const callBlock2 = createProcCallBlock(this.workspace); + this.clock.runAll(); defBlock.dispose(); this.clock.runAll(); @@ -1467,35 +761,6 @@ suite('Procedures', function() { chai.assert.isTrue( callBlock2.disposed, 'Expected the second caller to be disposed'); }); - - test('undoing and redoing a procedure delete will still associate ' + - 'procedure and caller with the same model', - function() { - const defBlock = createProcDefBlock(this.workspace); - createProcCallBlock(this.workspace); - // TODO: Apparently we need to call checkAndDelete to handle event - // grouping, this seems like possibly a bug. - const oldModel = defBlock.getProcedureModel(); - defBlock.checkAndDelete(); - this.clock.runAll(); - - this.workspace.undo(); - this.clock.runAll(); - - const newDefBlock = - this.workspace.getBlocksByType('procedures_defnoreturn')[0]; - const newCallBlock = - this.workspace.getBlocksByType('procedures_callnoreturn')[0]; - - chai.assert.equal( - newDefBlock.getProcedureModel(), - newCallBlock.getProcedureModel(), - 'Expected both new blocks to be associated with the same model'); - chai.assert.equal( - oldModel.getId(), - newDefBlock.getProcedureModel().getId(), - 'Expected the new model to have the same ID as the old model'); - }); }); suite('caller blocks creating new def blocks', function() { @@ -1568,7 +833,7 @@ suite('Procedures', function() { callBlock, ['y'], [this.TEST_VAR_ID], 'do something2'); }); - test( + test.skip( 'callers whose defs are deserialized later do not create defs', function() { Blockly.Xml.domToWorkspace(Blockly.Xml.textToDom(` @@ -1591,12 +856,10 @@ suite('Procedures', function() { this.workspace.getBlocksByType('procedures_defreturn')[0]; const callBlock = this.workspace.getBlocksByType('procedures_callreturn')[0]; + // TODO: Currently the callers are creating variables with different + // IDs than those serialized to XML, so these assertions fail. assertDefBlockStructure(defBlock, true, ['x'], ['arg']); assertCallBlockStructure(callBlock, ['x'], ['arg'], 'do something'); - chai.assert.equal( - defBlock.getProcedureModel(), - callBlock.getProcedureModel(), - 'Expected the blocks to have the same procedure model'); }); }); @@ -1679,7 +942,7 @@ suite('Procedures', function() { callBlock, ['y'], [this.TEST_VAR_ID], 'do something2'); }); - test( + test.skip( 'callers whose defs are deserialized later do not create defs', function() { Blockly.serialization.workspaces.load({ @@ -1714,12 +977,10 @@ suite('Procedures', function() { this.workspace.getBlocksByType('procedures_defreturn')[0]; const callBlock = this.workspace.getBlocksByType('procedures_callreturn')[0]; + // TODO: Currently the callers are creating variables with different + // IDs than those serialized to JSON, so these assertions fail. assertDefBlockStructure(defBlock, true, ['x'], ['arg']); assertCallBlockStructure(callBlock, ['x'], ['arg'], 'do something'); - chai.assert.equal( - defBlock.getProcedureModel(), - callBlock.getProcedureModel(), - 'Expected the blocks to have the same procedure model'); }); }); }); @@ -1985,107 +1246,6 @@ suite('Procedures', function() { }); }); - suite('full workspace serialization test cases', function() { - test('definitions with parameters are properly rendered', function() { - Blockly.serialization.workspaces.load({ - "blocks": { - "languageVersion": 0, - "blocks": [ - { - "type": "procedures_defnoreturn", - "extraState": { - "procedureId": "procId", - "params": [ - { - "name": "x", - "id": "varId", - "paramId": "paramId", - }, - ], - }, - "fields": { - "NAME": "do something", - }, - }, - ], - }, - "procedures": [ - { - "id": "procId", - "name": "do something", - "returnTypes": null, - "parameters": [ - { - "id": "paramId", - "name": "x", - }, - ], - }, - ], - "variables": [ - { - "name": "x", - "id": "varId", - }, - ], - }, this.workspace); - assertDefBlockStructure( - this.workspace.getTopBlocks(false)[0], false, ['x'], ['varId']); - }); - test( - 'multiple definitions pointing to the same model end up with ' + - 'different models', - function() { - Blockly.serialization.workspaces.load({ - "blocks": { - "languageVersion": 0, - "blocks": [ - { - "type": "procedures_defnoreturn", - "extraState": { - "procedureId": "procId", - }, - "fields": { - "NAME": "do something", - }, - }, - { - "type": "procedures_defnoreturn", - "y": 10, - "extraState": { - "procedureId": "procId", - }, - "fields": { - "NAME": "do something", - }, - }, - ], - }, - "procedures": [ - { - "id": "procId", - "name": "do something", - "returnTypes": null, - }, - ], - }, this.workspace); - const def1 = this.workspace.getTopBlocks(true)[0]; - const def2 = this.workspace.getTopBlocks(true)[1]; - chai.assert.equal( - def1.getProcedureModel().getName(), - 'do something', - 'Expected the first procedure definition to have the name in XML'); - chai.assert.equal( - def2.getProcedureModel().getName(), - 'do something2', - 'Expected the second procedure definition to be renamed'); - chai.assert.notEqual( - def1.getProcedureModel(), - def2.getProcedureModel(), - 'Expected the procedures to have different models'); - }); - }); - const testSuites = [ {title: 'procedures_defreturn', hasReturn: true, defType: 'procedures_defreturn', callType: 'procedures_callreturn'}, @@ -2735,9 +1895,6 @@ suite('Procedures', function() { 'fields': { 'NAME': 'unnamed', }, - 'extraState': { - 'procedureId': '1', - }, }, assertBlockStructure: (block) => { @@ -2758,9 +1915,6 @@ suite('Procedures', function() { 'fields': { 'NAME': 'do something', }, - 'extraState': { - 'procedureId': '1', - }, }, assertBlockStructure: (block) => { @@ -2794,17 +1948,14 @@ suite('Procedures', function() { 'NAME': 'do something', }, 'extraState': { - 'procedureId': '1', 'params': [ { 'name': 'x', 'id': 'arg1', - 'paramId': '1', }, { 'name': 'y', 'id': 'arg2', - 'paramId': '1', }, ], }, @@ -2838,12 +1989,10 @@ suite('Procedures', function() { 'NAME': 'do something', }, 'extraState': { - 'procedureId': '1', 'params': [ { 'name': 'preCreatedVar', 'id': 'preCreatedVarId', - 'paramId': '1', }, ], }, @@ -2872,7 +2021,6 @@ suite('Procedures', function() { 'NAME': 'do something', }, 'extraState': { - 'procedureId': '1', 'hasStatements': false, }, },