From 112fcccb31fcf319c5536fa012ef80d87364ffa2 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 30 Nov 2017 17:12:39 -0800 Subject: [PATCH] Fix renaming --- core/block.js | 18 ++++++++++++++++ core/field_variable.js | 47 ++++++++---------------------------------- core/variables.js | 4 ++-- core/workspace.js | 16 +++++++++++--- 4 files changed, 42 insertions(+), 43 deletions(-) diff --git a/core/block.js b/core/block.js index 570670822..c16129a39 100644 --- a/core/block.js +++ b/core/block.js @@ -724,6 +724,24 @@ Blockly.Block.prototype.renameVar = function(oldName, newName) { } }; +/** + * Notification that a variable is renaming. + * If the name 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. + */ +Blockly.Block.prototype.renameVarById = function(oldId, newId) { + for (var i = 0, input; input = this.inputList[i]; i++) { + for (var j = 0, field; field = input.fieldRow[j]; j++) { + if (field instanceof Blockly.FieldVariable && + oldId == field.getValue()) { + field.setValue(newId); + } + } + } +}; + /** * Returns the language-neutral value from the field of a block. * @param {string} name The name of the field. diff --git a/core/field_variable.js b/core/field_variable.js index e00c6110c..2e2461dc4 100644 --- a/core/field_variable.js +++ b/core/field_variable.js @@ -161,33 +161,6 @@ Blockly.FieldVariable.prototype.getText = function() { return this.variable_ ? this.variable_.name : ''; }; -/** - * Set the variable name. (DEPRECATED) - * @param {string} value New text. - */ -Blockly.FieldVariable.prototype.oldSetValue = function(value) { - var newValue = value; - var newText = value; - - if (this.sourceBlock_) { - var variable = this.sourceBlock_.workspace.getVariableById(value); - if (variable) { - newText = variable.name; - } - // TODO(marisaleung): Remove name lookup after converting all Field Variable - // instances to use ID instead of name. - else if (variable = this.sourceBlock_.workspace.getVariable(value)) { - newValue = variable.getId(); - } - if (Blockly.Events.isEnabled()) { - Blockly.Events.fire(new Blockly.Events.BlockChange( - this.sourceBlock_, 'field', this.name, this.value_, newValue)); - } - } - this.value_ = newValue; - this.setText(newText); -}; - /** * Set the variable ID. * @param {string} id New variable ID, which must reference an existing @@ -319,17 +292,13 @@ Blockly.FieldVariable.prototype.onItemSelected = function(menu, menuItem) { var id = menuItem.getValue(); // TODO(marisaleung): change setValue() to take in an ID as the parameter. // Then remove itemText. - var itemText; if (this.sourceBlock_ && this.sourceBlock_.workspace) { var workspace = this.sourceBlock_.workspace; var variable = workspace.getVariableById(id); // If the item selected is a variable, set itemText to the variable name. - if (variable) { - itemText = variable.name; - } else if (id == Blockly.RENAME_VARIABLE_ID) { + if (id == Blockly.RENAME_VARIABLE_ID) { // Rename variable. - var currentName = this.getText(); - variable = workspace.getVariable(currentName); + variable = this.variable_; Blockly.Variables.renameVariable(workspace, variable); return; } else if (id == Blockly.DELETE_VARIABLE_ID) { @@ -338,10 +307,12 @@ Blockly.FieldVariable.prototype.onItemSelected = function(menu, menuItem) { return; } - // Call any validation function, and allow it to override. - itemText = this.callValidator(itemText); - } - if (itemText !== null) { - this.setValue(itemText); + // TODO: Call any validation function, and allow it to override. + // For now it's unclear whether the validator should act on the id. + //var validatedId = this.callValidator(variable.getId()); } + // if (variable.getId() !== null) { + // this.setValue(validatedId); + // } + this.setValue(id); }; diff --git a/core/variables.js b/core/variables.js index 4788595eb..26dc0b452 100644 --- a/core/variables.js +++ b/core/variables.js @@ -269,7 +269,7 @@ Blockly.Variables.createVariable = function(workspace, opt_callback, opt_type) { * Rename a variable with the given workspace, variableType, and oldName. * @param {!Blockly.Workspace} workspace The workspace on which to rename the * variable. - * @param {?Blockly.VariableModel} variable Variable to rename. + * @param {Blockly.VariableModel} variable Variable to rename. * @param {function(?string=)=} opt_callback A callback. It will * be passed an acceptable new variable name, or null if change is to be * aborted (cancel button), or undefined if an existing variable was chosen. @@ -282,7 +282,7 @@ Blockly.Variables.renameVariable = function(workspace, variable, Blockly.Msg.RENAME_VARIABLE_TITLE.replace('%1', variable.name), defaultName, function(newName) { if (newName) { - workspace.renameVariable(variable.name, newName); + workspace.renameVariableById(variable.getId(), newName); if (opt_callback) { opt_callback(newName); } diff --git a/core/workspace.js b/core/workspace.js index 41ed1f915..a8f7eccfa 100644 --- a/core/workspace.js +++ b/core/workspace.js @@ -248,6 +248,13 @@ Blockly.Workspace.prototype.renameVariableInternal_ = function( variable, newName, opt_type) { var type = variable ? variable.type : (opt_type || ''); var newVariable = this.getVariable(newName, type); + + // If a variable previously existed with the new name, we will coalesce the + // variables and use the ID of the existing variable with the new name. + // Otherwise, we change the current variable's name but not its ID. + var oldId = variable.getId(); + var newId = newVariable ? newVariable.getId() : oldId; + var oldCase; // Find if newVariable case is different. @@ -257,14 +264,17 @@ Blockly.Workspace.prototype.renameVariableInternal_ = function( Blockly.Events.setGroup(true); var blocks = this.getAllBlocks(); + this.variableMap_.renameVariable(variable, newName, type); + + // Iterate through every block and update name. for (var i = 0; i < blocks.length; i++) { - blocks[i].renameVar(variable.name, newName); + blocks[i].renameVarById(oldId, newId); if (oldCase) { - blocks[i].renameVar(oldCase, newName); + blocks[i].renameVarById(newId, newId); + //blocks[i].renameVar(oldCase, newName); } } - this.variableMap_.renameVariable(variable, newName, type); Blockly.Events.setGroup(false); };