From bd4e4daebb96cd381c471c0dc6745bd238887f41 Mon Sep 17 00:00:00 2001
From: Rachel Fenichel <fenichel@google.com>
Date: Tue, 9 Jan 2018 13:31:13 -0800
Subject: [PATCH] Update comments and use try/catch with setGroup.

---
 core/block.js        |  4 +++-
 core/names.js        |  4 ++--
 core/variable_map.js | 26 ++++++++++++++++----------
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/core/block.js b/core/block.js
index 2bab44e2f..df9c37e43 100644
--- a/core/block.js
+++ b/core/block.js
@@ -275,7 +275,9 @@ Blockly.Block.prototype.dispose = function(healStack) {
  * Call initModel on all fields on the block.
  * May be called more than once.
  * Either initModel or initSvg must be called after creating a block and before
- * first interaction with it.
+ * the first interaction with it.  Interactions include UI actions
+ * (e.g. clicking and dragging) and firing events (e.g. create, delete, and
+ * change).
  * @public
  */
 Blockly.Block.prototype.initModel = function() {
diff --git a/core/names.js b/core/names.js
index 6278c7dd2..a886e41c5 100644
--- a/core/names.js
+++ b/core/names.js
@@ -112,7 +112,7 @@ Blockly.Names.prototype.getNameForUserVariable_ = function(id) {
  * @param {string} name The Blockly entity name (no constraints).
  * @param {string} type The type of entity in Blockly
  *     ('VARIABLE', 'PROCEDURE', 'BUILTIN', etc...).
- * @return {string} An entity name legal for the exported language.
+ * @return {string} An entity name that is legal in the exported language.
  */
 Blockly.Names.prototype.getName = function(name, type) {
   if (type == Blockly.Variables.NAME_TYPE) {
@@ -143,7 +143,7 @@ Blockly.Names.prototype.getName = function(name, type) {
  * @param {string} name The Blockly entity name (no constraints).
  * @param {string} type The type of entity in Blockly
  *     ('VARIABLE', 'PROCEDURE', 'BUILTIN', etc...).
- * @return {string} An entity name legal for the exported language.
+ * @return {string} An entity name that is legal in the exported language.
  */
 Blockly.Names.prototype.getDistinctName = function(name, type) {
   var safeName = this.safeName_(name);
diff --git a/core/variable_map.js b/core/variable_map.js
index 40f150b20..daeced960 100644
--- a/core/variable_map.js
+++ b/core/variable_map.js
@@ -71,13 +71,16 @@ Blockly.VariableMap.prototype.renameVariable = function(variable, newName) {
   var conflictVar = this.getVariable(newName, type);
   var blocks = this.workspace.getAllBlocks();
   Blockly.Events.setGroup(true);
-  // The IDs may match if the rename is a simple case change (name1 -> Name1).
-  if (!conflictVar || conflictVar.getId() == variable.getId()) {
-    this.renameVariableAndUses_(variable, newName, blocks);
-  } else {
-    this.renameVariableWithConflict_(variable, newName, conflictVar, blocks);
+  try {
+    // The IDs may match if the rename is a simple case change (name1 -> Name1).
+    if (!conflictVar || conflictVar.getId() == variable.getId()) {
+      this.renameVariableAndUses_(variable, newName, blocks);
+    } else {
+      this.renameVariableWithConflict_(variable, newName, conflictVar, blocks);
+    }
+  } finally {
+    Blockly.Events.setGroup(false);
   }
-  Blockly.Events.setGroup(false);
 };
 
 /**
@@ -264,11 +267,14 @@ Blockly.VariableMap.prototype.deleteVariableById = function(id) {
 Blockly.VariableMap.prototype.deleteVariableInternal_ = function(variable,
     uses) {
   Blockly.Events.setGroup(true);
-  for (var i = 0; i < uses.length; i++) {
-    uses[i].dispose(true, false);
+  try {
+    for (var i = 0; i < uses.length; i++) {
+      uses[i].dispose(true, false);
+    }
+    this.deleteVariable(variable);
+  } finally {
+    Blockly.Events.setGroup(false);
   }
-  this.deleteVariable(variable);
-  Blockly.Events.setGroup(false);
 };
 
 /* End functions for variable deletion. */