Fix renaming

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.

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);
 };

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);
           }

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);
 };