diff --git a/core/field_variable.js b/core/field_variable.js index abaf0a717..f93caa150 100644 --- a/core/field_variable.js +++ b/core/field_variable.js @@ -143,12 +143,12 @@ Blockly.FieldVariable.prototype.classValidator = function(text) { text = Blockly.Variables.promptName( Blockly.Msg.RENAME_VARIABLE_TITLE.replace('%1', oldVar), oldVar); if (text) { - Blockly.Variables.renameVariable(oldVar, text, workspace); + workspace.renameVariable(oldVar, text); } return null; } else if (text == Blockly.Msg.DELETE_VARIABLE.replace('%1', this.getText())) { - Blockly.Variables.delete(this.getText(), this.sourceBlock_.workspace); + workspace.deleteVariable(this.getText()); return null; } return undefined; diff --git a/core/flyout.js b/core/flyout.js index 3a523251c..726c925cb 100644 --- a/core/flyout.js +++ b/core/flyout.js @@ -89,6 +89,8 @@ Blockly.Flyout = function(workspaceOptions) { /** * List of visible buttons. + * @type {!Array.} + * @private */ this.buttons_ = []; diff --git a/core/flyout_button.js b/core/flyout_button.js index 5ba78971e..4b7559d68 100644 --- a/core/flyout_button.js +++ b/core/flyout_button.js @@ -150,6 +150,7 @@ Blockly.FlyoutButton.prototype.dispose = function() { this.svgGroup_ = null; } this.workspace_ = null; + this.targetWorkspace_ = null; }; /** diff --git a/core/variables.js b/core/variables.js index 935dbe666..00c38ad21 100644 --- a/core/variables.js +++ b/core/variables.js @@ -91,24 +91,6 @@ Blockly.Variables.allVariables = function(root) { return root.variableList; }; -/** - * Find all instances of the specified variable and rename them. - * @param {string} oldName Variable to rename. - * @param {string} newName New variable name. - * @param {!Blockly.Workspace} workspace Workspace rename variables in. - */ -Blockly.Variables.renameVariable = function(oldName, newName, workspace) { - Blockly.Events.setGroup(true); - var blocks = workspace.getAllBlocks(); - // Iterate through every block. - for (var i = 0; i < blocks.length; i++) { - blocks[i].renameVar(oldName, newName); - } - Blockly.Events.setGroup(false); - - workspace.renameVariable(oldName, newName); -}; - /** * Construct the blocks required by the flyout for the variable category. * @param {!Blockly.Workspace} workspace The workspace contianing variables. @@ -241,74 +223,6 @@ Blockly.Variables.generateUniqueName = function(workspace) { return newName; }; -/** - * Find all the uses of a named variable. - * @param {string} name Name of variable. - * @param {!Blockly.Workspace} workspace The workspace to find uses in. - * @return {!Array.} Array of block usages. - */ -Blockly.Variables.getUses = function(name, workspace) { - var uses = []; - var blocks = workspace.getAllBlocks(); - // Iterate through every block and check the name. - for (var i = 0; i < blocks.length; i++) { - var blockVariables = blocks[i].getVars(); - if (blockVariables) { - for (var j = 0; j < blockVariables.length; j++) { - var varName = blockVariables[j]; - // Variable name may be null if the block is only half-built. - if (varName && Blockly.Names.equals(varName, name)) { - uses.push(blocks[i]); - } - } - } - } - return uses; -}; - -/** - * When a variable is deleted, find and dispose of all uses of it. - * @param {!Array.} uses An array of blocks using the variable. - * @private - */ -Blockly.Variables.disposeUses_ = function(uses) { - Blockly.Events.setGroup(true); - for (var i = 0; i < uses.length; i++) { - uses[i].dispose(true, false); - } - Blockly.Events.setGroup(false); -}; - -/** - * Delete a variables and all of its uses from the given workspace. - * @param {string} name Name of variable to delete. - * @param {!Blockly.Workspace} workspace The workspace to delete uses from. - */ -Blockly.Variables.delete = function(name, workspace) { - var variableIndex = workspace.variableList.indexOf(name); - if (variableIndex != -1) { - var uses = Blockly.Variables.getUses(name, workspace); - if (uses.length > 1) { - for (var i = 0, block; block = uses[i]; i++) { - if (block.type == 'procedures_defnoreturn' || - block.type == 'procedures_defreturn') { - var procedureName = block.getFieldValue('NAME'); - window.alert( - Blockly.Msg.CANNOT_DELETE_VARIABLE_PROCEDURE.replace('%1', name). - replace('%2', procedureName)); - return; - } - } - window.confirm( - Blockly.Msg.DELETE_VARIABLE_CONFIRMATION.replace('%1', uses.length). - replace('%2', name)); - } - Blockly.Variables.disposeUses_(uses); - workspace.variableList.splice(variableIndex, 1); - } - -}; - /** * Create a new variable on the given workspace. * @param {!Blockly.Workspace} workspace The workspace on which to create the @@ -318,14 +232,22 @@ Blockly.Variables.delete = function(name, workspace) { * variable was chosen. */ Blockly.Variables.createVariable = function(workspace) { - var text = Blockly.Variables.promptName(Blockly.Msg.NEW_VARIABLE_TITLE, ''); - // Since variables are case-insensitive, ensure that if the new variable - // matches with an existing variable, the new case prevails throughout. - if (text) { - workspace.createVariable(text); - return text; + while (true) { + var text = Blockly.Variables.promptName(Blockly.Msg.NEW_VARIABLE_TITLE, ''); + if (text) { + if (workspace.variableIndexOf(text) != -1) { + window.alert(Blockly.Msg.VARIABLE_ALREADY_EXISTS.replace('%1', + text.toLowerCase())); + } else { + workspace.createVariable(text); + break; + } + } else { + text = null; + break; + } } - return null; + return text; }; /** diff --git a/core/workspace.js b/core/workspace.js index b6fca3a29..db7415753 100644 --- a/core/workspace.js +++ b/core/workspace.js @@ -119,6 +119,9 @@ Blockly.Workspace.SCAN_ANGLE = 3; Blockly.Workspace.prototype.addTopBlock = function(block) { this.topBlocks_.push(block); if (this.isFlyout) { + // This is for the (unlikely) case where you have a variable in a block in + // an always-open flyout. It needs to be possible to edit the block in the + // flyout, so the contents of the dropdown need to be correct. var variables = Blockly.Variables.allUsedVariables(block); for (var i = 0; i < variables.length; i++) { if (this.variableList.indexOf(variables[i]) == -1) { @@ -225,13 +228,39 @@ Blockly.Workspace.prototype.updateVariableList = function(clearList) { * @param {string} newName New variable name. */ Blockly.Workspace.prototype.renameVariable = function(oldName, newName) { - // Find the old name in the list and replace it. - var variableIndex = this.variableList.indexOf(oldName); - var newVariableIndex = this.variableList.indexOf(newName); - if (variableIndex != -1 && newVariableIndex == -1) { + // Find the old name in the list. + var variableIndex = this.variableIndexOf(oldName); + var newVariableIndex = this.variableIndexOf(newName); + + // We might be renaming to an existing name but with different case. If so, + // we will also update all of the blocks using the new name to have the + // correct case. + if (newVariableIndex != -1 && + this.variableList[newVariableIndex] != newName) { + var oldCase = this.variableList[newVariableIndex]; + } + + Blockly.Events.setGroup(true); + var blocks = this.getAllBlocks(); + // Iterate through every block. + for (var i = 0; i < blocks.length; i++) { + blocks[i].renameVar(oldName, newName); + if (oldCase) { + blocks[i].renameVar(oldCase, newName); + } + } + Blockly.Events.setGroup(false); + + + if (variableIndex == newVariableIndex || + variableIndex != -1 && newVariableIndex == -1) { + // Only changing case, or renaming to a completely novel name. this.variableList[variableIndex] = newName; } else if (variableIndex != -1 && newVariableIndex != -1) { + // Renaming one existing variable to another existing variable. this.variableList.splice(variableIndex, 1); + // The case might have changed. + this.variableList[newVariableIndex] = newName; } else { this.variableList.push(newName); console.log('Tried to rename an non-existent variable.'); @@ -239,17 +268,90 @@ Blockly.Workspace.prototype.renameVariable = function(oldName, newName) { }; /** - * Create a variables with the given name. + * Create a variable with the given name. * TODO: #468 * @param {string} name The new variable's name. */ Blockly.Workspace.prototype.createVariable = function(name) { - var index = this.variableList.indexOf(name); + var index = this.variableIndexOf(name); if (index == -1) { this.variableList.push(name); } }; +/** + * Find all the uses of a named variable. + * @param {string} name Name of variable. + * @return {!Array.} Array of block usages. + */ +Blockly.Workspace.prototype.getVariableUses = function(name) { + var uses = []; + var blocks = this.getAllBlocks(); + // Iterate through every block and check the name. + for (var i = 0; i < blocks.length; i++) { + var blockVariables = blocks[i].getVars(); + if (blockVariables) { + for (var j = 0; j < blockVariables.length; j++) { + var varName = blockVariables[j]; + // Variable name may be null if the block is only half-built. + if (varName && Blockly.Names.equals(varName, name)) { + uses.push(blocks[i]); + } + } + } + } + return uses; +}; + +/** + * Delete a variables and all of its uses from this workspace. + * @param {string} name Name of variable to delete. + */ +Blockly.Workspace.prototype.deleteVariable = function(name) { + var variableIndex = this.variableIndexOf(name); + if (variableIndex != -1) { + var uses = this.getVariableUses(name); + if (uses.length > 1) { + for (var i = 0, block; block = uses[i]; i++) { + if (block.type == 'procedures_defnoreturn' || + block.type == 'procedures_defreturn') { + var procedureName = block.getFieldValue('NAME'); + window.alert( + Blockly.Msg.CANNOT_DELETE_VARIABLE_PROCEDURE.replace('%1', name). + replace('%2', procedureName)); + return; + } + } + window.confirm( + Blockly.Msg.DELETE_VARIABLE_CONFIRMATION.replace('%1', uses.length). + replace('%2', name)); + } + + Blockly.Events.setGroup(true); + for (var i = 0; i < uses.length; i++) { + uses[i].dispose(true, false); + } + Blockly.Events.setGroup(false); + this.variableList.splice(variableIndex, 1); + } +}; + +/** + * Check whether a variable exists with the given name. The check is + * case-insensitive. + * @param {string} name The name to check for. + * @return {number} The index of the name in the variable list, or -1 if it is + * not present. + */ +Blockly.Workspace.prototype.variableIndexOf = function(name) { + for (var i = 0, varname; varname = this.variableList[i]; i++) { + if (Blockly.Names.equals(varname, name)) { + return i; + } + } + return -1; +}; + /** * Returns the horizontal offset of the workspace. * Intended for LTR/RTL compatibility in XML.