diff --git a/blocks/procedures.js b/blocks/procedures.js index 97141471a..523003f7e 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -36,6 +36,7 @@ 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 */ @@ -112,8 +113,8 @@ const blocks = createBlockDefinitionsFromJsonArray([ 'extensions': [ 'procedure_caller_get_def_mixin', 'procedure_caller_update_shape_mixin', - 'procedure_caller_onchange_mixin', 'procedure_caller_context_menu_mixin', + 'procedure_caller_onchange_mixin', 'procedure_callernoreturn_get_def_block_mixin', ], 'mutator': 'procedure_caller_mutator', @@ -185,8 +186,8 @@ const blocks = createBlockDefinitionsFromJsonArray([ 'extensions': [ 'procedure_caller_get_def_mixin', 'procedure_caller_update_shape_mixin', - 'procedure_caller_onchange_mixin', 'procedure_caller_context_menu_mixin', + 'procedure_caller_onchange_mixin', 'procedure_callerreturn_get_def_block_mixin', ], 'mutator': 'procedure_caller_mutator', @@ -291,7 +292,8 @@ const procedureDefGetDefMixin = function() { * @this {Block} */ getVars: function() { - return this.arguments_; + return this.getProcedureModel().getParameters().map( + (p) => p.getVariableModel().name); }, /** @@ -300,7 +302,8 @@ const procedureDefGetDefMixin = function() { * @this {Block} */ getVarModels: function() { - return this.argumentVarModels_; + return this.getProcedureModel().getParameters().map( + (p) => p.getVariableModel()); }, /** @@ -338,26 +341,18 @@ const procedureDefVarMixin = function() { * @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 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); - - 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); - } + const oldParam = model.getParameter(index); + model.deleteParameter(index); + model.insertParameter( + new ObservableParameterModel( + this.workspace, newVar.name, oldParam.getId()), + index); }, /** @@ -369,19 +364,10 @@ const procedureDefVarMixin = function() { * @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); + const containsVar = this.getProcedureModel().getParameters().some( + (p) => p.getVariableModel() === variable); + if (containsVar) { + triggerProceduresUpdate(this.workspace); } }, }; @@ -400,6 +386,7 @@ const procedureDefUpdateShapeMixin = { this.setFieldValue(this.getProcedureModel().getName(), 'NAME'); this.setEnabled(this.getProcedureModel().getEnabled()); this.updateParameters_(); + this.updateMutator_(); }, /** @@ -422,6 +409,23 @@ const procedureDefUpdateShapeMixin = { } }, + /** + * 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'); + } + } + }, + /** * Add or remove the statement block from this function definition. * @param {boolean} hasStatements True if a statement block is needed. @@ -453,49 +457,6 @@ const procedureDefUpdateShapeMixin = { } this.hasStatements_ = hasStatements; }, - - /** - * 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(); - } - }, - - /** - * 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'); - } - } - } - }, }; Extensions.registerMixin( 'procedure_def_update_shape_mixin', procedureDefUpdateShapeMixin); @@ -510,10 +471,6 @@ Extensions.register( 'procedure_def_validator_helper', procedureDefValidatorHelper); const procedureDefMutator = { - arguments_: [], - - argumentVarModels_: [], - hasStatements_: true, /** @@ -536,8 +493,8 @@ const procedureDefMutator = { const varModel = params[i].getVariableModel(); parameter.setAttribute('name', varModel.name); parameter.setAttribute('varid', varModel.getId()); - if (opt_paramIds && this.paramIds_) { - parameter.setAttribute('paramId', this.paramIds_[i]); + if (opt_paramIds) { + parameter.setAttribute('paramId', params[i].getId()); } container.appendChild(parameter); } @@ -565,31 +522,10 @@ const procedureDefMutator = { this.workspace, node.getAttribute('name'), undefined, varId), i); } - - // TODO: Remove this data update code. - 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.updateParams_(); - Procedures.mutateCallers(this); - - // Show or hide the statement input. this.setStatements_(xmlElement.getAttribute('statements') !== 'false'); + + // Call mutate callers for backwards compatibility. + Procedures.mutateCallers(this); }, /** @@ -646,21 +582,10 @@ const procedureDefMutator = { } } - // TODO: Remove this data update code. - this.arguments_ = []; - this.argumentVarModels_ = []; - if (state['params']) { - for (let i = 0; i < state['params'].length; 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.updateParams_(); - Procedures.mutateCallers(this); this.setStatements_(state['hasStatements'] === false ? false : true); + + // Call mutate callers for backwards compatibility. + Procedures.mutateCallers(this); }, /** @@ -711,35 +636,15 @@ const procedureDefMutator = { * @this {Block} */ compose: function(containerBlock) { - // Parameter list. - this.arguments_ = []; - this.paramIds_ = []; - this.argumentVarModels_ = []; - - // TODO: Remove old data handling logic? - let paramBlock = containerBlock.getInputTargetBlock('STACK'); - while (paramBlock && !paramBlock.isInsertionMarker()) { - const varName = paramBlock.getFieldValue('NAME'); - this.arguments_.push(varName); - const variable = Variables.getOrCreateVariablePackage( - this.workspace, null, varName, ''); - this.argumentVarModels_.push(variable); - - this.paramIds_.push(paramBlock.id); - paramBlock = - paramBlock.nextConnection && paramBlock.nextConnection.targetBlock(); - } - this.updateParams_(); - Procedures.mutateCallers(this); - 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; - paramBlock = containerBlock.getInputTargetBlock('STACK'); + let paramBlock = containerBlock.getInputTargetBlock('STACK'); while (paramBlock && !paramBlock.isInsertionMarker()) { model.insertParameter( new ObservableParameterModel( @@ -749,11 +654,15 @@ const procedureDefMutator = { 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. + Procedures.mutateCallers(this); }, }; Extensions.registerMutator( @@ -776,9 +685,10 @@ const procedureDefContextMenuMixin = { option.text = Msg['PROCEDURES_CREATE_DO'].replace('%1', name); const xmlMutation = xmlUtils.createElement('mutation'); xmlMutation.setAttribute('name', name); - for (let i = 0; i < this.arguments_.length; i++) { + const params = this.getProcedureModel().getParameters(); + for (const param of params) { const xmlArg = xmlUtils.createElement('arg'); - xmlArg.setAttribute('name', this.arguments_[i]); + xmlArg.setAttribute('name', param.getName()); xmlMutation.appendChild(xmlArg); } const xmlBlock = xmlUtils.createElement('block'); @@ -788,20 +698,20 @@ const procedureDefContextMenuMixin = { options.push(option); // Add options to create getters for each parameter. - 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); + if (this.isCollapsed()) return; - 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); - } + 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); } }, }; @@ -855,7 +765,7 @@ const procedureDefNoReturnGetCallerBlockMixin = { * @this {Block} */ getProcedureDef: function() { - return [this.getFieldValue('NAME'), this.arguments_, false]; + return [this.getFieldValue('NAME'), this.getVars(), false]; }, callType_: 'procedures_callnoreturn', @@ -874,7 +784,7 @@ const procedureDefReturnGetCallerBlockMixin = { * @this {Block} */ getProcedureDef: function() { - return [this.getFieldValue('NAME'), this.arguments_, true]; + return [this.getFieldValue('NAME'), this.getVars(), true]; }, callType_: 'procedures_callreturn', @@ -1031,6 +941,10 @@ const procedureCallerGetDefMixin = function() { (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; @@ -1049,6 +963,7 @@ const procedureCallerGetDefMixin = function() { createDef_(name, params = []) { const xy = this.getRelativeToSurfaceXY(); const newName = Procedures.findLegalName(name, this); + this.renameProcedure(name, newName); const blockDef = { 'type': this.defType_, @@ -1060,7 +975,7 @@ const procedureCallerGetDefMixin = function() { 'fields': {'NAME': newName}, }; return serialization.blocks.append(blockDef, this.getTargetWorkspace_()) - .getProcedureModel(); + .getProcedureModel(); }, /** @@ -1111,14 +1026,6 @@ Extensions.register( 'procedure_caller_get_def_mixin', procedureCallerGetDefMixin); const procedureCallerMutator = { - arguments_: [], - - argumentVarModels_: [], - - quarkConnections_: {}, - - quarkIds_: null, - previousEnabledState_: true, paramsFromSerializedState_: [], @@ -1131,11 +1038,14 @@ const procedureCallerMutator = { */ 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); + 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; }, @@ -1164,9 +1074,11 @@ const procedureCallerMutator = { */ saveExtraState: function() { const state = Object.create(null); - state['name'] = this.getProcedureCall(); - if (this.arguments_.length) { - state['params'] = this.arguments_; + 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; }, @@ -1191,14 +1103,10 @@ const procedureCallerMutator = { if (this.getProcedureModel()) { this.initBlockWithProcedureModel_(); } else { - // We need to create/find our procedure def in our change listener. - this.paramsFromSerializedState_ = params; // Create inputs based on the mutation so that children can be connected. this.createArgInputs_(params); } - - // Temporarily maintained for logic that relies on arguments_ - this.arguments_ = params; + this.paramsFromSerializedState_ = params; }, }; Extensions.registerMutator('procedure_caller_mutator', procedureCallerMutator); @@ -1225,10 +1133,6 @@ const procedureCallerUpdateShapeMixin = { this.updateName_(); this.updateEnabled_(); this.updateParameters_(); - - // Temporarily maintained for code that relies on arguments_ - this.arguments_ = - this.getProcedureModel().getParameters().map((p) => p.getName()); }, /** @@ -1351,148 +1255,6 @@ const procedureCallerUpdateShapeMixin = { this.setTooltip(baseMsg.replace('%1', newName)); } }, - - /** - * 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); - } - - 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'); - } - } - } - }, }; Extensions.registerMixin( 'procedure_caller_update_shape_mixin', procedureCallerUpdateShapeMixin); @@ -1535,23 +1297,13 @@ const procedureCallerOnChangeMixin = { this.getFieldValue('NAME'), this.paramsFromSerializedState_); } this.initBlockWithProcedureModel_(); - } else if (event.type === Events.BLOCK_DELETE && event.blockId != this.id) { - // 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); - } } }, defMatches_(defBlock) { return defBlock && defBlock.type === this.defType_ && - JSON.stringify(defBlock.getVars()) === JSON.stringify(this.arguments_); + JSON.stringify(defBlock.getVars()) === + JSON.stringify(this.paramsFromSerializedState_); }, }; Extensions.registerMixin( @@ -1588,6 +1340,7 @@ Extensions.registerMixin( 'procedure_caller_context_menu_mixin', procedureCallerContextMenuMixin); const procedureCallerNoReturnGetDefBlockMixin = { + hasReturn_: false, defType_: 'procedures_defnoreturn', }; Extensions.registerMixin( @@ -1595,6 +1348,7 @@ Extensions.registerMixin( procedureCallerNoReturnGetDefBlockMixin); const procedureCallerReturnGetDefBlockMixin = { + hasReturn_: true, defType_: 'procedures_defreturn', }; Extensions.registerMixin( diff --git a/core/procedures/observable_procedure_model.ts b/core/procedures/observable_procedure_model.ts index d7b4f1ad9..588f771e3 100644 --- a/core/procedures/observable_procedure_model.ts +++ b/core/procedures/observable_procedure_model.ts @@ -22,6 +22,7 @@ export class ObservableProcedureModel implements IProcedureModel { private returnTypes: string[]|null = null; private enabled = true; private shouldFireEvents = false; + private shouldTriggerUpdates = true; constructor( private readonly workspace: Workspace, name: string, id?: string) { @@ -34,7 +35,7 @@ export class ObservableProcedureModel implements IProcedureModel { if (name === this.name) return this; const prevName = this.name; this.name = name; - triggerProceduresUpdate(this.workspace); + if (this.shouldTriggerUpdates) triggerProceduresUpdate(this.workspace); if (this.shouldFireEvents) { eventUtils.fire(new (eventUtils.get(eventUtils.PROCEDURE_RENAME))( this.workspace, this, prevName)); @@ -63,7 +64,7 @@ export class ObservableProcedureModel implements IProcedureModel { } } - triggerProceduresUpdate(this.workspace); + if (this.shouldTriggerUpdates) triggerProceduresUpdate(this.workspace); if (this.shouldFireEvents) { eventUtils.fire( new (eventUtils.get(eventUtils.PROCEDURE_PARAMETER_CREATE))( @@ -78,7 +79,7 @@ export class ObservableProcedureModel implements IProcedureModel { const oldParam = this.parameters[index]; this.parameters.splice(index, 1); - triggerProceduresUpdate(this.workspace); + if (this.shouldTriggerUpdates) triggerProceduresUpdate(this.workspace); if (isObservable(oldParam)) { oldParam.stopPublishing(); } @@ -110,7 +111,7 @@ export class ObservableProcedureModel implements IProcedureModel { if (!!types === !!this.returnTypes) return this; const oldReturnTypes = this.returnTypes; this.returnTypes = types; - triggerProceduresUpdate(this.workspace); + if (this.shouldTriggerUpdates) triggerProceduresUpdate(this.workspace); if (this.shouldFireEvents) { eventUtils.fire(new (eventUtils.get(eventUtils.PROCEDURE_CHANGE_RETURN))( this.workspace, this, oldReturnTypes)); @@ -125,7 +126,7 @@ export class ObservableProcedureModel implements IProcedureModel { setEnabled(enabled: boolean): this { if (enabled === this.enabled) return this; this.enabled = enabled; - triggerProceduresUpdate(this.workspace); + if (this.shouldTriggerUpdates) triggerProceduresUpdate(this.workspace); if (this.shouldFireEvents) { eventUtils.fire(new (eventUtils.get(eventUtils.PROCEDURE_ENABLE))( this.workspace, this)); @@ -133,6 +134,27 @@ export class ObservableProcedureModel implements IProcedureModel { return this; } + /** + * Disables triggering updates to procedure blocks until the endBulkUpdate + * is called. + * + * @internal + */ + startBulkUpdate() { + this.shouldTriggerUpdates = false; + } + + /** + * Triggers an update to procedure blocks. Should be used with + * startBulkUpdate. + * + * @internal + */ + endBulkUpdate() { + this.shouldTriggerUpdates = true; + triggerProceduresUpdate(this.workspace); + } + /** Returns the unique language-neutral ID for the procedure. */ getId(): string { return this.id; diff --git a/core/procedures/update_procedures.ts b/core/procedures/update_procedures.ts index b355a9c56..fae2ed4c4 100644 --- a/core/procedures/update_procedures.ts +++ b/core/procedures/update_procedures.ts @@ -4,9 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ +import * as goog from '../../closure/goog/goog.js'; import {isProcedureBlock} from '../interfaces/i_procedure_block.js'; import {Workspace} from '../workspace.js'; +goog.declareModuleId('Blockly.procedures.updateProcedures'); + /** * Calls the `doProcedureUpdate` method on all blocks which implement it. diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index b2ccdfa4a..ae8a5ba2c 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -992,6 +992,33 @@ suite('Procedures', function() { 'Expected the params field to contain the new name of the param'); }); + test('defs are updated for parameter renames when two params exist', + 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 paramBlock1 = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock1.setFieldValue('param1', 'NAME'); + const paramBlock2 = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock2.setFieldValue('param2', 'NAME'); + containerBlock.getInput('STACK').connection + .connect(paramBlock1.previousConnection); + paramBlock1.nextConnection.connect(paramBlock2.previousConnection); + this.clock.runAll(); + + paramBlock1.setFieldValue('new name', 'NAME'); + this.clock.runAll(); + + 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 name of the param'); + }); + test('callers are updated for parameter renames', function() { // Create a stack of container, parameter. const defBlock = createProcDefBlock(this.workspace); @@ -1048,7 +1075,8 @@ suite('Procedures', function() { const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); - containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); + containerBlock.getInput('STACK').connection + .connect(paramBlock.previousConnection); this.clock.runAll(); defBlock.mutator.setVisible(false); @@ -1063,6 +1091,29 @@ suite('Procedures', function() { 'Expected the params field to contain the new name of the param'); }); + test( + 'renaming a variable associated with a parameter updates mutator parameters', + 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('param1', 'NAME'); + containerBlock.getInput('STACK').connection + .connect(paramBlock.previousConnection); + this.clock.runAll(); + + const variable = this.workspace.getVariable('param1', ''); + this.workspace.renameVariableById(variable.getId(), 'new name'); + + chai.assert.equal( + paramBlock.getFieldValue('NAME'), + 'new name', + 'Expected the params field to contain the new name of the param'); + }); + test( 'renaming a variable associated with a parameter updates procedure callers', function() { @@ -1074,7 +1125,8 @@ suite('Procedures', function() { const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); - containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); + containerBlock.getInput('STACK').connection + .connect(paramBlock.previousConnection); this.clock.runAll(); defBlock.mutator.setVisible(false); @@ -1090,6 +1142,83 @@ suite('Procedures', function() { 'Expected the params field to match the name of the new param'); }); + test( + 'coalescing a variable associated with a parameter updates procedure defs', + 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('param1', 'NAME'); + containerBlock.getInput('STACK').connection + .connect(paramBlock.previousConnection); + this.clock.runAll(); + defBlock.mutator.setVisible(false); + + const variable = this.workspace.getVariable('param1', ''); + this.workspace.renameVariableById(variable.getId(), 'preCreatedVar'); + + chai.assert.isNotNull( + defBlock.getField('PARAMS'), + 'Expected the params field to exist'); + chai.assert.isTrue( + defBlock.getFieldValue('PARAMS').includes('preCreatedVar'), + 'Expected the params field to contain the new name of the param'); + }); + + test( + 'coalescing a variable associated with a parameter updates mutator parameters', + 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('param1', 'NAME'); + containerBlock.getInput('STACK').connection + .connect(paramBlock.previousConnection); + this.clock.runAll(); + + const variable = this.workspace.getVariable('param1', ''); + this.workspace.renameVariableById(variable.getId(), 'preCreatedVar'); + + chai.assert.equal( + paramBlock.getFieldValue('NAME'), + 'preCreatedVar', + 'Expected the params field to contain the new name of the param'); + }); + + test( + 'coalescing a variable associated with a parameter updates procedure callers', + function() { + // Create a stack of container, parameter. + const defBlock = createProcDefBlock(this.workspace); + const callBlock = createProcCallBlock(this.workspace); + defBlock.mutator.setVisible(true); + const mutatorWorkspace = defBlock.mutator.getWorkspace(); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; + const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock.setFieldValue('param1', 'NAME'); + containerBlock.getInput('STACK').connection + .connect(paramBlock.previousConnection); + this.clock.runAll(); + defBlock.mutator.setVisible(false); + + const variable = this.workspace.getVariable('param1', ''); + this.workspace.renameVariableById(variable.getId(), 'preCreatedVar'); + + chai.assert.isNotNull( + callBlock.getInput('ARG0'), + 'Expected the param input to exist'); + chai.assert.equal( + callBlock.getFieldValue('ARGNAME0'), + 'preCreatedVar', + 'Expected the params field to match the name of the new param'); + }); + test.skip( 'renaming a variable such that you get a parameter ' + 'conflict does... something!', @@ -1377,10 +1506,10 @@ suite('Procedures', function() { suite('xml', function() { test('callers without defs create new defs', function() { - const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( - '' + - '' + - '' + const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(` + + + ` ), this.workspace); this.clock.runAll(); assertDefBlockStructure( @@ -1388,7 +1517,7 @@ suite('Procedures', function() { assertCallBlockStructure(callBlock, [], [], 'do something'); }); - test('callers without mutations create unamed defs', function() { + test('callers without mutations create unnamed defs', function() { const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( '' ), this.workspace); @@ -1595,6 +1724,57 @@ suite('Procedures', function() { }); }); + suite('definition block context menu', function() { + test('the context menu includes an option for creating the caller', function() { + const def = Blockly.serialization.blocks.append({ + 'type': 'procedures_defnoreturn', + 'fields': { + 'NAME': 'test name', + }, + }, this.workspace); + + const options = []; + def.customContextMenu(options); + + chai.assert.isTrue( + options[0].text.includes('test name'), + 'Expected the context menu to have an option to create the caller'); + }); + + test('the context menu includes an option for each parameter', function() { + const def = Blockly.serialization.blocks.append({ + 'type': 'procedures_defnoreturn', + 'fields': { + 'NAME': 'test name', + }, + 'extraState': { + 'params': [ + { + 'name': 'testParam1', + 'id': 'varId1', + 'paramId': 'paramId1', + }, + { + 'name': 'testParam2', + 'id': 'varId2', + 'paramId': 'paramId2', + }, + ], + }, + }, this.workspace); + + const options = []; + def.customContextMenu(options); + + chai.assert.isTrue( + options[1].text.includes('testParam1'), + 'Expected the context menu to have an option to create the first param'); + chai.assert.isTrue( + options[2].text.includes('testParam2'), + 'Expected the context menu to have an option to create the second param'); + }); + }); + suite('allProcedures', function() { test('Only Procedures', function() { const noReturnBlock = this.workspace.newBlock('procedures_defnoreturn'); @@ -1850,10 +2030,18 @@ suite('Procedures', function() { }); suite('rename', function() { setup(function() { - this.defBlock = this.workspace.newBlock(testSuite.defType); - this.defBlock.setFieldValue('proc name', 'NAME'); - this.callBlock = this.workspace.newBlock(testSuite.callType); - this.callBlock.setFieldValue('proc name', 'NAME'); + this.defBlock = Blockly.serialization.blocks.append({ + 'type': testSuite.defType, + 'fields': { + 'NAME': 'proc name', + }, + }, this.workspace); + this.callBlock = Blockly.serialization.blocks.append({ + 'type': testSuite.callType, + 'fields': { + 'NAME': 'proc name', + }, + }, this.workspace); sinon.stub(this.defBlock.getField('NAME'), 'resizeEditor_'); }); test('Simple, Programmatic', function() { @@ -1963,10 +2151,18 @@ suite('Procedures', function() { }); suite('getCallers', function() { setup(function() { - this.defBlock = this.workspace.newBlock(testSuite.defType); - this.defBlock.setFieldValue('proc name', 'NAME'); - this.callBlock = this.workspace.newBlock(testSuite.callType); - this.callBlock.setFieldValue('proc name', 'NAME'); + this.defBlock = Blockly.serialization.blocks.append({ + 'type': testSuite.defType, + 'fields': { + 'NAME': 'proc name', + }, + }, this.workspace); + this.callBlock = Blockly.serialization.blocks.append({ + 'type': testSuite.callType, + 'fields': { + 'NAME': 'proc name', + }, + }, this.workspace); }); test('Simple', function() { const callers = @@ -2036,10 +2232,18 @@ suite('Procedures', function() { }); suite('getDefinition', function() { setup(function() { - this.defBlock = this.workspace.newBlock(testSuite.defType); - this.defBlock.setFieldValue('proc name', 'NAME'); - this.callBlock = this.workspace.newBlock(testSuite.callType); - this.callBlock.setFieldValue('proc name', 'NAME'); + this.defBlock = Blockly.serialization.blocks.append({ + 'type': testSuite.defType, + 'fields': { + 'NAME': 'proc name', + }, + }, this.workspace); + this.callBlock = Blockly.serialization.blocks.append({ + 'type': testSuite.callType, + 'fields': { + 'NAME': 'proc name', + }, + }, this.workspace); }); test('Simple', function() { const def = @@ -2150,18 +2354,18 @@ suite('Procedures', function() { } function assertArgs(argArray) { chai.assert.equal( - this.defBlock.arguments_.length, + this.defBlock.getVars().length, argArray.length, 'Expected the def to have the right number of arguments'); for (let i = 0; i < argArray.length; i++) { - chai.assert.equal(this.defBlock.arguments_[i], argArray[i]); + chai.assert.equal(this.defBlock.getVars()[i], argArray[i]); } chai.assert.equal( - this.callBlock.arguments_.length, + this.callBlock.getVars().length, argArray.length, 'Expected the call to have the right number of arguments'); for (let i = 0; i < argArray.length; i++) { - chai.assert.equal(this.callBlock.arguments_[i], argArray[i]); + chai.assert.equal(this.callBlock.getVars()[i], argArray[i]); } } test('Simple Add Arg', function() { diff --git a/tests/mocha/test_helpers/serialization.js b/tests/mocha/test_helpers/serialization.js index ebc6deb5d..94fc511d5 100644 --- a/tests/mocha/test_helpers/serialization.js +++ b/tests/mocha/test_helpers/serialization.js @@ -61,7 +61,7 @@ export const runSerializationTestSuite = (testCases) => { let block; if (testCase.json) { block = Blockly.serialization.blocks.append( - testCase.json, this.workspace); + testCase.json, this.workspace, {recordUndo: true}); } else { block = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( testCase.xml), this.workspace); @@ -80,12 +80,14 @@ export const runSerializationTestSuite = (testCases) => { if (testCase.json) { const block = Blockly.serialization.blocks.append( testCase.json, this.workspace); + this.clock.runAll(); const generatedJson = Blockly.serialization.blocks.save(block); const expectedJson = testCase.expectedJson || testCase.json; chai.assert.deepEqual(generatedJson, expectedJson); } else { const block = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( testCase.xml), this.workspace); + this.clock.runAll(); const generatedXml = Blockly.Xml.domToPrettyText( Blockly.Xml.blockToDom(block));