diff --git a/blocks/procedures.js b/blocks/procedures.js index d9111057d..bd1d0179c 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -483,71 +483,6 @@ Blockly.Blocks['procedures_mutatorcontainer'] = { this.setTooltip(Blockly.Msg['PROCEDURES_MUTATORCONTAINER_TOOLTIP']); this.contextMenu = false; }, - - // TODO: Move this to a validator on the arg blocks, that way it can be - // tested. - /** - * This will create & delete variables and in dialogs workspace to ensure - * that when a new block is dragged out it will have a unique parameter name. - * @param {!Blockly.Events.Abstract} event Change event. - * @this {Blockly.Block} - */ - onchange: function(event) { - if (!this.workspace || this.workspace.isFlyout || - (event.type != Blockly.Events.BLOCK_DELETE && event.type != Blockly.Events.BLOCK_CREATE)) { - return; - } - var blocks = this.workspace.getAllBlocks(false); - var allVariables = this.workspace.getAllVariables(); - if (event.type == Blockly.Events.BLOCK_DELETE) { - var variableNamesToKeep = []; - for (var i = 0; i < blocks.length; i += 1) { - if (blocks[i].getFieldValue('NAME')) { - variableNamesToKeep.push(blocks[i].getFieldValue('NAME')); - } - } - for (var k = 0; k < allVariables.length; k += 1) { - if (variableNamesToKeep.indexOf(allVariables[k].name) == -1) { - this.workspace.deleteVariableById(allVariables[k].getId()); - } - } - return; - } - - if (event.type != Blockly.Events.BLOCK_CREATE) { - return; - } - - var block = this.workspace.getBlockById(event.blockId); - // This is to handle the one none variable block - // Happens when all the blocks are regenerated - if (!block.getField('NAME')) { - return; - } - var varName = block.getFieldValue('NAME'); - var variable = this.workspace.getVariable(varName); - - if (!variable) { - // This means the parameter name is not in use and we can create the variable. - variable = this.workspace.createVariable(varName); - } - // If the blocks are connected we don't have to check duplicate variables - // This only happens if the dialog box is open - if (block.previousConnection.isConnected() || block.nextConnection.isConnected()) { - return; - } - - for (var j = 0; j < blocks.length; j += 1) { - // filter block that was created - if (block.id != blocks[j].id && blocks[j].getFieldValue('NAME') == variable.name) { - // generate new name and set name field - varName = Blockly.Variables.generateUniqueName(this.workspace); - variable = this.workspace.createVariable(varName); - block.setFieldValue(variable.name, 'NAME'); - return; - } - } - } }; Blockly.Blocks['procedures_mutatorarg'] = { @@ -556,7 +491,8 @@ Blockly.Blocks['procedures_mutatorarg'] = { * @this {Blockly.Block} */ init: function() { - var field = new Blockly.FieldTextInput('x', this.validator_); + var field = new Blockly.FieldTextInput( + Blockly.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_; @@ -603,7 +539,9 @@ Blockly.Blocks['procedures_mutatorarg'] = { } // Prevents duplicate parameter names in functions - var blocks = sourceBlock.workspace.getAllBlocks(false); + var workspace = sourceBlock.workspace.targetWorkspace || + sourceBlock.workspace; + var blocks = workspace.getAllBlocks(false); for (var i = 0; i < blocks.length; i++) { if (blocks[i].id == this.getSourceBlock().id) { continue; @@ -613,6 +551,12 @@ Blockly.Blocks['procedures_mutatorarg'] = { } } + // Don't create variables for arg blocks that + // only exist in the mutator's flyout. + if (sourceBlock.isInFlyout) { + return varName; + } + var model = outerWs.getVariable(varName, ''); if (model && model.name != varName) { // Rename the variable (case change) @@ -626,6 +570,7 @@ Blockly.Blocks['procedures_mutatorarg'] = { } return varName; }, + /** * Called when focusing away from the text field. * Deletes all variables that were created as the user typed their intended diff --git a/core/mutator.js b/core/mutator.js index 113c1ab2c..f00cd0247 100644 --- a/core/mutator.js +++ b/core/mutator.js @@ -72,6 +72,15 @@ Blockly.Mutator.prototype.setBlock = function(block) { this.block_ = block; }; +/** + * Returns the workspace inside this mutator icon's bubble. + * @return {Blockly.WorkspaceSvg} The workspace inside this mutator icon's + * bubble. + */ +Blockly.Mutator.prototype.getWorkspace = function() { + return this.workspace_; +}; + /** * Draw the mutator icon. * @param {!Element} group The icon group. diff --git a/core/procedures.js b/core/procedures.js index 5205f09f5..b0bb55651 100644 --- a/core/procedures.js +++ b/core/procedures.js @@ -46,6 +46,12 @@ goog.require('Blockly.Xml'); */ Blockly.Procedures.NAME_TYPE = Blockly.PROCEDURE_CATEGORY_NAME; +/** + * The default argument for a procedures_mutatorarg block. + * @type {string} + */ +Blockly.Procedures.DEFAULT_ARG = 'x'; + /** * Procedure block type. * @typedef {{ @@ -271,6 +277,77 @@ Blockly.Procedures.flyoutCategory = function(workspace) { return xmlList; }; +/** + * Updates the procedure mutator's flyout so that the arg block is not a + * duplicate of another arg. + * @param {!Blockly.Workspace} workspace The procedure mutator's workspace. This + * workspace's flyout is what is being updated. + * @private + */ +Blockly.Procedures.updateMutatorFlyout_ = function(workspace) { + var usedNames = []; + var blocks = workspace.getBlocksByType('procedures_mutatorarg', false); + for (var i = 0, block; (block = blocks[i]); i++) { + usedNames.push(block.getFieldValue('NAME')); + } + + var xml = Blockly.utils.xml.createElement('xml'); + var argBlock = Blockly.utils.xml.createElement('block'); + argBlock.setAttribute('type', 'procedures_mutatorarg'); + var nameField = Blockly.utils.xml.createElement('field'); + nameField.setAttribute('name', 'NAME'); + var argValue = Blockly.Variables.generateUniqueNameFromOptions( + Blockly.Procedures.DEFAULT_ARG, usedNames); + var fieldContent = Blockly.utils.xml.createTextNode(argValue); + + nameField.appendChild(fieldContent); + argBlock.appendChild(nameField); + xml.appendChild(argBlock); + + workspace.updateToolbox(xml); +}; + +/** + * Listens for when a procedure mutator is opened. Then it triggers a flyout + * update and adds a mutator change listener to the mutator workspace. + * @param {!Blockly.Events.Abstract} e The event that triggered this listener. + * @package + */ +Blockly.Procedures.mutatorOpenListener = function(e) { + if (e.type != Blockly.Events.UI || e.element != 'mutatorOpen' || + !e.newValue) { + return; + } + var workspaceId = /** @type {string} */ (e.workspaceId); + var block = Blockly.Workspace.getById(workspaceId) + .getBlockById(e.blockId); + var type = block.type; + if (type != 'procedures_defnoreturn' && type != 'procedures_defreturn') { + return; + } + var workspace = block.mutator.getWorkspace(); + Blockly.Procedures.updateMutatorFlyout_(workspace); + workspace.addChangeListener(Blockly.Procedures.mutatorChangeListener_); +}; + +/** + * Listens for changes in a procedure mutator and triggers flyout updates when + * necessary. + * @param {!Blockly.Events.Abstract} e The event that triggered this listener. + * @private + */ +Blockly.Procedures.mutatorChangeListener_ = function(e) { + if (e.type != Blockly.Events.BLOCK_CREATE && + e.type != Blockly.Events.BLOCK_DELETE && + e.type != Blockly.Events.BLOCK_CHANGE) { + return; + } + var workspaceId = /** @type {string} */ (e.workspaceId); + var workspace = /** @type {!Blockly.WorkspaceSvg} */ + (Blockly.Workspace.getById(workspaceId)); + Blockly.Procedures.updateMutatorFlyout_(workspace); +}; + /** * Find all the callers of a named procedure. * @param {string} name Name of procedure. diff --git a/core/variable_map.js b/core/variable_map.js index cb2d8d7d9..0353fcde8 100644 --- a/core/variable_map.js +++ b/core/variable_map.js @@ -384,6 +384,21 @@ Blockly.VariableMap.prototype.getAllVariables = function() { return all_variables; }; +/** + * Returns all of the variable names of all types. + * @return {!Array} All of the variable names of all types. + */ +Blockly.VariableMap.prototype.getAllVariableNames = function() { + var allNames = []; + for (var key in this.variableMap_) { + var variables = this.variableMap_[key]; + for (var i = 0, variable; (variable = variables[i]); i++) { + allNames.push(variable.name); + } + } + return allNames; +}; + /** * Find all the uses of a named variable. * @param {string} id ID of the variable to find. diff --git a/core/variables.js b/core/variables.js index d6ec0c8af..5e260990d 100644 --- a/core/variables.js +++ b/core/variables.js @@ -206,6 +206,8 @@ Blockly.Variables.flyoutCategoryBlocks = function(workspace) { return xmlList; }; +Blockly.Variables.VAR_LETTER_OPTIONS = 'ijkmnopqrstuvwxyzabcdefgh'; // No 'l'. + /** * Return a new variable name that is not yet being used. This will try to * generate single letter variable names in the range 'i' to 'z' to start with. @@ -215,44 +217,51 @@ Blockly.Variables.flyoutCategoryBlocks = function(workspace) { * @return {string} New variable name. */ Blockly.Variables.generateUniqueName = function(workspace) { - var variableList = workspace.getAllVariables(); - var newName = ''; - if (variableList.length) { - var nameSuffix = 1; - var letters = 'ijkmnopqrstuvwxyzabcdefgh'; // No 'l'. - var letterIndex = 0; - var potName = letters.charAt(letterIndex); - while (!newName) { - var inUse = false; - for (var i = 0; i < variableList.length; i++) { - if (variableList[i].name.toLowerCase() == potName) { - // This potential name is already used. - inUse = true; - break; - } - } - if (inUse) { - // Try the next potential name. - letterIndex++; - if (letterIndex == letters.length) { - // Reached the end of the character sequence so back to 'i'. - // a new suffix. - letterIndex = 0; - nameSuffix++; - } - potName = letters.charAt(letterIndex); - if (nameSuffix > 1) { - potName += nameSuffix; - } - } else { - // We can use the current potential name. - newName = potName; + return Blockly.Variables.generateUniqueNameFromOptions( + Blockly.Variables.VAR_LETTER_OPTIONS.charAt(0), + workspace.getAllVariableNames() + ); +}; + +/** + * Returns a unique name that is not present in the usedNames array. This + * will try to generate single letter names in the range a -> z (skip l). It + * will start with the character passed to startChar. + * @param {string} startChar The character to start the search at. + * @param {!Array} usedNames A list of all of the used names. + * @return {string} A unique name that is not present in the usedNames array. + */ +Blockly.Variables.generateUniqueNameFromOptions = function(startChar, usedNames) { + if (!usedNames.length) { + return startChar; + } + + var letters = Blockly.Variables.VAR_LETTER_OPTIONS; + var suffix = ''; + var letterIndex = letters.indexOf(startChar); + var potName = startChar; + + // eslint-disable-next-line no-constant-condition + while (true) { + var inUse = false; + for (var i = 0; i < usedNames.length; i++) { + if (usedNames[i].toLowerCase() == potName) { + inUse = true; + break; } } - } else { - newName = 'i'; + if (!inUse) { + return potName; + } + + letterIndex++; + if (letterIndex == letters.length) { + // Reached the end of the character sequence so back to 'i'. + letterIndex = 0; + suffix = Number(suffix) + 1; + } + potName = letters.charAt(letterIndex) + suffix; } - return newName; }; /** diff --git a/core/workspace.js b/core/workspace.js index a4af7a75d..f1d794ab8 100644 --- a/core/workspace.js +++ b/core/workspace.js @@ -492,6 +492,14 @@ Blockly.Workspace.prototype.getAllVariables = function() { return this.variableMap_.getAllVariables(); }; +/** + * Returns all variable names of all types. + * @return {!Array} List of all variable names of all types. + */ +Blockly.Workspace.prototype.getAllVariableNames = function() { + return this.variableMap_.getAllVariableNames(); +}; + /* End functions that are just pass-throughs to the variable map. */ /** diff --git a/core/workspace_svg.js b/core/workspace_svg.js index 900e817f1..f24d89ba4 100644 --- a/core/workspace_svg.js +++ b/core/workspace_svg.js @@ -127,6 +127,7 @@ Blockly.WorkspaceSvg = function(options, if (Blockly.Procedures && Blockly.Procedures.flyoutCategory) { this.registerToolboxCategoryCallback(Blockly.PROCEDURE_CATEGORY_NAME, Blockly.Procedures.flyoutCategory); + this.addChangeListener(Blockly.Procedures.mutatorOpenListener); } /** diff --git a/tests/mocha/procedures_test.js b/tests/mocha/procedures_test.js index a9453dc59..f38113666 100644 --- a/tests/mocha/procedures_test.js +++ b/tests/mocha/procedures_test.js @@ -398,24 +398,6 @@ suite('Procedures', function() { clearVariables.call(this); }, 'name'); }); - // TODO: Reenable the following two once arg name validation has been - // moved to the arg blocks. - test.skip('Add Identical Arg', function() { - this.callForAllTypes(function() { - var args = ['x', 'x']; - createMutator.call(this, args); - assertArgs.call(this, ['x', 'i']); - clearVariables.call(this); - }); - }); - test.skip('Add Identical (except case) Arg', function() { - this.callForAllTypes(function() { - var args = ['x', 'X']; - createMutator.call(this, args); - assertArgs.call(this, ['x', 'i']); - clearVariables.call(this); - }); - }); test('Multiple Args', function() { this.callForAllTypes(function() { var args = ['arg1', 'arg2', 'arg3'];