Move functions for updating variables on a workspace into workspace.js, and some lint

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;

core/flyout.js

@@ -89,6 +89,8 @@ Blockly.Flyout = function(workspaceOptions) {
 
   /**
    * List of visible buttons.
+   * @type {!Array.<!Blockly.FlyoutButton>}
+   * @private
    */
   this.buttons_ = [];
 

core/flyout_button.js

@@ -150,6 +150,7 @@ Blockly.FlyoutButton.prototype.dispose = function() {
     this.svgGroup_ = null;
   }
   this.workspace_ = null;
+  this.targetWorkspace_ = null;
 };
 
 /**

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.<!Blockly.Block>} 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.<!Blockly.Block>} 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;
 };
 
 /**

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.<!Blockly.Block>} 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.