From 96e2d7d1022169bfa9d5e6c0652f3236ff9da366 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Wed, 24 Jan 2018 17:05:58 -0800 Subject: [PATCH 1/3] Fix #1559 --- core/variables.js | 61 +++++++++++++++++++++++++++++++++++++++++------ msg/messages.js | 2 +- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/core/variables.js b/core/variables.js index 717e9f705..0e12b99d9 100644 --- a/core/variables.js +++ b/core/variables.js @@ -275,19 +275,30 @@ Blockly.Variables.generateUniqueName = function(workspace) { */ Blockly.Variables.createVariableButtonHandler = function( workspace, opt_callback, opt_type) { + var type = opt_type || ''; // This function needs to be named so it can be called recursively. var promptAndCheckWithAlert = function(defaultName) { Blockly.Variables.promptName(Blockly.Msg.NEW_VARIABLE_TITLE, defaultName, function(text) { if (text) { - if (workspace.getVariable(text, opt_type)) { - Blockly.alert(Blockly.Msg.VARIABLE_ALREADY_EXISTS.replace('%1', - text.toLowerCase()), + var existing = Blockly.Variables.nameUsedWithAnyType_(text, + workspace); + if (existing) { + var lowerCase = text.toLowerCase(); + if (existing.type == type) { + var msg = Blockly.Msg.VARIABLE_ALREADY_EXISTS.replace('%1', + lowerCase); + } else { + var msg = Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE; + msg = msg.replace('%1', lowerCase).replace('%2', existing.type); + } + Blockly.alert(msg, function() { promptAndCheckWithAlert(text); // Recurse }); } else { - workspace.createVariable(text, opt_type); + // No conflict + workspace.createVariable(text, type); if (opt_callback) { opt_callback(text); } @@ -340,9 +351,21 @@ Blockly.Variables.renameVariable = function(workspace, variable, Blockly.Variables.promptName(promptText, defaultName, function(newName) { if (newName) { - workspace.renameVariableById(variable.getId(), newName); - if (opt_callback) { - opt_callback(newName); + var existing = Blockly.Variables.nameUsedWithOtherType_(newName, + variable.type, workspace); + if (existing) { + var msg = + Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE.replace( + '%1', newName.toLowerCase()).replace('%2', existing.type); + Blockly.alert(msg, + function() { + promptAndCheckWithAlert(newName); // Recurse + }); + } else { + workspace.renameVariableById(variable.getId(), newName); + if (opt_callback) { + opt_callback(newName); + } } } else { // User canceled prompt without a value. @@ -378,6 +401,30 @@ Blockly.Variables.promptName = function(promptText, defaultText, callback) { }); }; +Blockly.Variables.nameUsedWithOtherType_ = function(name, type, workspace) { + var allVariables = workspace.getVariableMap().getAllVariables(); + + name = name.toLowerCase(); + for (var i = 0, variable; variable = allVariables[i]; i++) { + if (variable.name.toLowerCase() == name && variable.type != type) { + return variable; + } + } + return null; +}; + +Blockly.Variables.nameUsedWithAnyType_ = function(name, workspace) { + var allVariables = workspace.getVariableMap().getAllVariables(); + + name = name.toLowerCase(); + for (var i = 0, variable; variable = allVariables[i]; i++) { + if (variable.name.toLowerCase() == name) { + return variable; + } + } + return null; +}; + /** * Generate XML string for variable field. * @param {!Blockly.VariableModel} variableModel The variable model to generate diff --git a/msg/messages.js b/msg/messages.js index 21f869bb2..ea9d8f91e 100644 --- a/msg/messages.js +++ b/msg/messages.js @@ -136,7 +136,7 @@ Blockly.Msg.NEW_VARIABLE_TITLE = 'New variable name:'; /// alert - Tells the user that the name they entered is already in use. Blockly.Msg.VARIABLE_ALREADY_EXISTS = 'A variable named "%1" already exists.'; /// alert - Tells the user that the name they entered is already in use for another type. -Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE = 'A variable named "%1" already exists for another variable of type "%2".'; +Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE = 'A variable named "%1" already exists for another type: "%2".'; // Variable deletion. /// confirm - Ask the user to confirm their deletion of multiple uses of a variable. From 794520e225a23e993c7024c9a2699f1afe8b1b34 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Wed, 24 Jan 2018 17:17:31 -0800 Subject: [PATCH 2/3] Add documentation --- core/variables.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/core/variables.js b/core/variables.js index 0e12b99d9..d7fa7145d 100644 --- a/core/variables.js +++ b/core/variables.js @@ -401,6 +401,17 @@ Blockly.Variables.promptName = function(promptText, defaultText, callback) { }); }; +/** + * Check whether there exists a variable with the given name but a different + * type. + * @param {string} name The name to search for. + * @param {string} type The type to exclude from the search. + * @param {!Blockly.Workspace} workspace The workspace to search for the + * variable. + * @return {?Blockly.VariableModel} The variable with the given name and a + * different type, or null if none was found. + * @private + */ Blockly.Variables.nameUsedWithOtherType_ = function(name, type, workspace) { var allVariables = workspace.getVariableMap().getAllVariables(); @@ -413,6 +424,15 @@ Blockly.Variables.nameUsedWithOtherType_ = function(name, type, workspace) { return null; }; +/** + * Check whether there exists a variable with the given name of any type. + * @param {string} name The name to search for. + * @param {!Blockly.Workspace} workspace The workspace to search for the + * variable. + * @return {?Blockly.VariableModel} The variable with the given name, or null if + * none was found. + * @private + */ Blockly.Variables.nameUsedWithAnyType_ = function(name, workspace) { var allVariables = workspace.getVariableMap().getAllVariables(); From 68ff863ea93d645b9299e2b79d8e26b31ac2f834 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 25 Jan 2018 13:00:08 -0800 Subject: [PATCH 3/3] Fix indentation --- core/variables.js | 52 +++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/core/variables.js b/core/variables.js index d7fa7145d..e31dbbfe8 100644 --- a/core/variables.js +++ b/core/variables.js @@ -279,37 +279,37 @@ Blockly.Variables.createVariableButtonHandler = function( // This function needs to be named so it can be called recursively. var promptAndCheckWithAlert = function(defaultName) { Blockly.Variables.promptName(Blockly.Msg.NEW_VARIABLE_TITLE, defaultName, - function(text) { - if (text) { - var existing = Blockly.Variables.nameUsedWithAnyType_(text, - workspace); + function(text) { + if (text) { + var existing = + Blockly.Variables.nameUsedWithAnyType_(text, workspace); if (existing) { var lowerCase = text.toLowerCase(); if (existing.type == type) { - var msg = Blockly.Msg.VARIABLE_ALREADY_EXISTS.replace('%1', - lowerCase); + var msg = Blockly.Msg.VARIABLE_ALREADY_EXISTS.replace( + '%1', lowerCase); } else { var msg = Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE; msg = msg.replace('%1', lowerCase).replace('%2', existing.type); } Blockly.alert(msg, - function() { - promptAndCheckWithAlert(text); // Recurse - }); + function() { + promptAndCheckWithAlert(text); // Recurse + }); + } else { + // No conflict + workspace.createVariable(text, type); + if (opt_callback) { + opt_callback(text); + } + } } else { - // No conflict - workspace.createVariable(text, type); + // User canceled prompt. if (opt_callback) { - opt_callback(text); + opt_callback(null); } } - } else { - // User canceled prompt without a value. - if (opt_callback) { - opt_callback(null); - } - } - }); + }); }; promptAndCheckWithAlert(''); }; @@ -354,13 +354,13 @@ Blockly.Variables.renameVariable = function(workspace, variable, var existing = Blockly.Variables.nameUsedWithOtherType_(newName, variable.type, workspace); if (existing) { - var msg = - Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE.replace( - '%1', newName.toLowerCase()).replace('%2', existing.type); + var msg = Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE + .replace('%1', newName.toLowerCase()) + .replace('%2', existing.type); Blockly.alert(msg, - function() { - promptAndCheckWithAlert(newName); // Recurse - }); + function() { + promptAndCheckWithAlert(newName); // Recurse + }); } else { workspace.renameVariableById(variable.getId(), newName); if (opt_callback) { @@ -368,7 +368,7 @@ Blockly.Variables.renameVariable = function(workspace, variable, } } } else { - // User canceled prompt without a value. + // User canceled prompt. if (opt_callback) { opt_callback(null); }