diff --git a/core/block.js b/core/block.js index 399752ce..e7686e54 100644 --- a/core/block.js +++ b/core/block.js @@ -1467,7 +1467,8 @@ Blockly.Block.newFieldTextInputFromJson_ = function(options) { */ Blockly.Block.newFieldVariableFromJson_ = function(options) { var varname = Blockly.utils.replaceMessageReferences(options['variable']); - return new Blockly.FieldVariable(varname); + var variableTypes = options['variableTypes']; + return new Blockly.FieldVariable(varname, null, variableTypes); }; /** diff --git a/core/field_variable.js b/core/field_variable.js index a01af2ff..012be7af 100644 --- a/core/field_variable.js +++ b/core/field_variable.js @@ -40,14 +40,17 @@ goog.require('goog.string'); * a unique variable name will be generated. * @param {Function=} opt_validator A function that is executed when a new * option is selected. Its sole argument is the new option value. + * @param {Array.} opt_variableTypes A list of the types of variables to + * include in the dropdown. * @extends {Blockly.FieldDropdown} * @constructor */ -Blockly.FieldVariable = function(varname, opt_validator) { +Blockly.FieldVariable = function(varname, opt_validator, opt_variableTypes) { Blockly.FieldVariable.superClass_.constructor.call(this, Blockly.FieldVariable.dropdownCreate, opt_validator); this.setValue(varname || ''); this.addArgType('variable'); + this.variableTypes = opt_variableTypes; }; goog.inherits(Blockly.FieldVariable, Blockly.FieldDropdown); @@ -128,6 +131,31 @@ Blockly.FieldVariable.prototype.setValue = function(value) { this.setText(newText); }; +/** + * Return a list of variable types to include in the dropdown. + * @return {!Array.} Array of variable types. + * @throws {Error} if variableTypes is an empty array. + * @private + */ +Blockly.FieldVariable.prototype.getVariableTypes_ = function() { + var variableTypes = this.variableTypes; + if (variableTypes === null) { + // If variableTypes is null, return all variable types. + if (this.sourceBlock_) { + var workspace = this.sourceBlock_.workspace; + return workspace.getVariableTypes(); + } + } + variableTypes = variableTypes || ['']; + if (variableTypes.length == 0) { + // Throw an error if variableTypes is an empty list. + var name = this.getText(); + throw new Error('\'variableTypes\' of field variable ' + + name + ' was an empty list'); + } + return variableTypes; +}; + /** * Return a sorted list of variable names for variable dropdown menus. * Include a special option at the end for creating a new variable name. @@ -143,11 +171,16 @@ Blockly.FieldVariable.dropdownCreate = function() { if (this.sourceBlock_) { workspace = this.sourceBlock_.workspace; } - if (workspace) { + var variableTypes = this.getVariableTypes_(); + var variableModelList = []; // Get a copy of the list, so that adding rename and new variable options // doesn't modify the workspace's list. - variableModelList = workspace.getVariablesOfType(''); + for (var i = 0; i < variableTypes.length; i++) { + var variableType = variableTypes[i]; + var variables = workspace.getVariablesOfType(variableType); + variableModelList = variableModelList.concat(variables); + } for (var i = 0; i < variableModelList.length; i++){ if (createSelectedVariable && goog.string.caseInsensitiveEquals(variableModelList[i].name, name)) { @@ -195,7 +228,8 @@ Blockly.FieldVariable.prototype.onItemSelected = function(menu, menuItem) { else if (id == Blockly.RENAME_VARIABLE_ID) { // Rename variable. var currentName = this.getText(); - Blockly.FieldVariable.renameVariablePrompt(workspace, currentName); + variable = workspace.getVariable(currentName); + Blockly.Variables.renameVariable(workspace, variable); return; } else if (id == Blockly.DELETE_VARIABLE_ID) { // Delete variable. @@ -210,20 +244,3 @@ Blockly.FieldVariable.prototype.onItemSelected = function(menu, menuItem) { this.setValue(itemText); } }; - -/** - * Prompt the user to rename a variable. - * @param {Blockly.Workspace} workspace Workspace the variable to rename is in. - * @param {string} currentName Current name of the variable to rename. - */ -Blockly.FieldVariable.renameVariablePrompt = function(workspace, currentName) { - // Rename variable. - Blockly.hideChaff(); - Blockly.Variables.promptName( - Blockly.Msg.RENAME_VARIABLE_TITLE.replace('%1', currentName), currentName, - function(newName) { - if (newName) { - workspace.renameVariable(currentName, newName); - } - }); -}; diff --git a/core/procedures.js b/core/procedures.js index 8cc88c81..af031171 100644 --- a/core/procedures.js +++ b/core/procedures.js @@ -119,6 +119,18 @@ Blockly.Procedures.findLegalName = function(name, block) { * @private */ Blockly.Procedures.isLegalName_ = function(name, workspace, opt_exclude) { + return !Blockly.Procedures.isNameUsed(name, workspace, opt_exclude); +}; + +/** + * Return if the given name is already a procedure name. + * @param {string} name The questionable name. + * @param {!Blockly.Workspace} workspace The workspace to scan for collisions. + * @param {Blockly.Block=} opt_exclude Optional block to exclude from + * comparisons (one doesn't want to collide with oneself). + * @return {boolean} True if the name is used, otherwise return false. + */ +Blockly.Procedures.isNameUsed = function(name, workspace, opt_exclude) { var blocks = workspace.getAllBlocks(); // Iterate through every block and check the name. for (var i = 0; i < blocks.length; i++) { diff --git a/core/variables.js b/core/variables.js index fd167cf0..c5eb908a 100644 --- a/core/variables.js +++ b/core/variables.js @@ -356,6 +356,7 @@ Blockly.Variables.generateUniqueName = function(workspace) { * aborted (cancel button), or undefined if an existing variable was chosen. */ Blockly.Variables.createVariable = function(workspace, opt_callback) { + // 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) { @@ -367,7 +368,7 @@ Blockly.Variables.createVariable = function(workspace, opt_callback) { promptAndCheckWithAlert(text); // Recurse }); } - else if (!Blockly.Procedures.isLegalName_(text, workspace)) { + else if (!Blockly.Procedures.isNameUsed(text, workspace)) { Blockly.alert(Blockly.Msg.PROCEDURE_ALREADY_EXISTS.replace('%1', text.toLowerCase()), function() { @@ -398,6 +399,55 @@ Blockly.Variables.createVariable = function(workspace, opt_callback) { promptAndCheckWithAlert(''); }; +/** + * 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 {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. + */ +Blockly.Variables.renameVariable = function(workspace, variable, + opt_callback) { + // This function needs to be named so it can be called recursively. + var promptAndCheckWithAlert = function(defaultName) { + Blockly.Variables.promptName( + Blockly.Msg.RENAME_VARIABLE_TITLE.replace('%1', variable.name), defaultName, + function(newName) { + if (newName) { + var newVariable = workspace.getVariable(newName); + if (newVariable && newVariable.type != variable.type) { + Blockly.alert(Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE.replace('%1', + newName.toLowerCase()).replace('%2', newVariable.type), + function() { + promptAndCheckWithAlert(newName); // Recurse + }); + } + else if (!Blockly.Procedures.isNameUsed(newName, workspace)) { + Blockly.alert(Blockly.Msg.PROCEDURE_ALREADY_EXISTS.replace('%1', + newName.toLowerCase()), + function() { + promptAndCheckWithAlert(newName); // Recurse + }); + } + else { + workspace.renameVariable(variable.name, newName); + if (opt_callback) { + opt_callback(newName); + } + } + } else { + // User canceled prompt without a value. + if (opt_callback) { + opt_callback(null); + } + } + }); + }; + promptAndCheckWithAlert(''); +}; + /** * Prompt the user for a new variable name. * @param {string} promptText The string of the prompt. diff --git a/core/xml.js b/core/xml.js index 2f431ad5..35a32c14 100644 --- a/core/xml.js +++ b/core/xml.js @@ -610,7 +610,7 @@ Blockly.Xml.domToBlockHeadless_ = function(xmlBlock, workspace) { // TODO (marisaleung): When we change setValue and getValue to // interact with id's instead of names, update this so that we get // the variable based on id instead of textContent. - var type = xmlChild.getAttribute('variabletype') || ''; + var type = xmlChild.getAttribute('variableType') || ''; var variable = workspace.getVariable(text); if (!variable) { variable = workspace.createVariable(text, type, diff --git a/msg/messages.js b/msg/messages.js index 6bed0634..2f1230f7 100644 --- a/msg/messages.js +++ b/msg/messages.js @@ -124,9 +124,11 @@ Blockly.Msg.NEW_VARIABLE = 'Create variable...'; /// prompt - Prompts the user to enter the name for a new variable. See [https://github.com/google/blockly/wiki/Variables#dropdown-menu https://github.com/google/blockly/wiki/Variables#dropdown-menu]. 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.' +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".'; /// alert - Tells the user that the name they entered is already in use for a procedure. -Blockly.Msg.PROCEDURE_ALREADY_EXISTS = 'A procedure named "%1" already exists.' +Blockly.Msg.PROCEDURE_ALREADY_EXISTS = 'A procedure named "%1" already exists.'; // Variable deletion. /// confirm - Ask the user to confirm their deletion of multiple uses of a variable. diff --git a/tests/jsunit/field_variable_test.js b/tests/jsunit/field_variable_test.js index 94c5f7d4..0ef29627 100644 --- a/tests/jsunit/field_variable_test.js +++ b/tests/jsunit/field_variable_test.js @@ -111,7 +111,8 @@ function test_fieldVariable_dropdownCreateVariablesExist() { var result_options = Blockly.FieldVariable.dropdownCreate.call( { 'sourceBlock_': {'workspace': workspace}, - 'getText': function(){return 'name1';} + 'getText': function(){return 'name1';}, + 'getVariableTypes_': function(){return [''];} }); assertEquals(result_options.length, 3); isEqualArrays(result_options[0], ['name1', 'id1']); @@ -128,7 +129,8 @@ function test_fieldVariable_dropdownCreateVariablesExist() { var result_options = Blockly.FieldVariable.dropdownCreate.call( { 'sourceBlock_': {'workspace': workspace}, - 'getText': function(){return 'name1';} + 'getText': function(){return 'name1';}, + 'getVariableTypes_': function(){return [''];} }); assertEquals(result_options.length, 3); isEqualArrays(result_options[0], ['name1', 'id1']); @@ -146,7 +148,8 @@ function test_fieldVariable_dropdownVariableAndTypeDoesNotExist() { var result_options = Blockly.FieldVariable.dropdownCreate.call( { 'sourceBlock_': {'workspace': workspace}, - 'getText': function(){return 'name1';} + 'getText': function(){return 'name1';}, + 'getVariableTypes_': function(){return [''];} }); // Check the options. @@ -169,7 +172,8 @@ function test_fieldVariable_dropdownVariableDoesNotExistTypeDoes() { var result_options = Blockly.FieldVariable.dropdownCreate.call( { 'sourceBlock_': {'workspace': workspace}, - 'getText': function(){return 'name2';} + 'getText': function(){return 'name2';}, + 'getVariableTypes_': function(){return [''];} }); assertEquals(3, result_options.length); @@ -181,3 +185,59 @@ function test_fieldVariable_dropdownVariableDoesNotExistTypeDoes() { fieldVariableTestWithMocks_tearDown(); } + +function test_fieldVariable_getVariableTypes_undefinedVariableTypes() { + // Expect that since variableTypes is undefined, only type empty string + // will be returned. + workspace = new Blockly.Workspace(); + workspace.createVariable('name1', 'type1'); + workspace.createVariable('name2', 'type2'); + var fieldVariable = new Blockly.FieldVariable('name1'); + var resultTypes = fieldVariable.getVariableTypes_(); + isEqualArrays(resultTypes, ['']); + workspace.dispose(); +} + +function test_fieldVariable_getVariableTypes_givenVariableTypes() { + // Expect that since variableTypes is undefined, only type empty string + // will be returned. + workspace = new Blockly.Workspace(); + workspace.createVariable('name1', 'type1'); + workspace.createVariable('name2', 'type2'); + var fieldVariable = new Blockly.FieldVariable('name1', null, ['type1', 'type2']); + var resultTypes = fieldVariable.getVariableTypes_(); + isEqualArrays(resultTypes, ['type1', 'type2']); + workspace.dispose(); +} + +function test_fieldVariable_getVariableTypes_nullVariableTypes() { + // Expect all variable types to be returned. + workspace = new Blockly.Workspace(); + workspace.createVariable('name1', 'type1'); + workspace.createVariable('name2', 'type2'); + var fieldVariable = new Blockly.FieldVariable('name1'); + var mockBlock = fieldVariable_mockBlock(workspace); + fieldVariable.setSourceBlock(mockBlock); + fieldVariable.variableTypes = null; + var resultTypes = fieldVariable.getVariableTypes_(); + isEqualArrays(resultTypes, ['type1', 'type2']); + workspace.dispose(); +} + +function test_fieldVariable_getVariableTypes_emptyListVariableTypes() { + // Expect an error to be thrown. + workspace = new Blockly.Workspace(); + workspace.createVariable('name1', 'type1'); + workspace.createVariable('name2', 'type2'); + var fieldVariable = new Blockly.FieldVariable('name1'); + var mockBlock = fieldVariable_mockBlock(workspace); + fieldVariable.setSourceBlock(mockBlock); + fieldVariable.variableTypes = []; + try { + fieldVariable.getVariableTypes_(); + } catch (e) { + //expected + } finally { + workspace.dispose(); + } +} diff --git a/tests/jsunit/xml_test.js b/tests/jsunit/xml_test.js index 7d0401e9..f0c00469 100644 --- a/tests/jsunit/xml_test.js +++ b/tests/jsunit/xml_test.js @@ -166,7 +166,7 @@ function test_domToWorkspace_VariablesAtTop() { ' name3' + ' ' + ' ' + - ' name3' + + ' name3' + ' ' + ''); Blockly.Xml.domToWorkspace(dom, workspace); @@ -210,7 +210,7 @@ function test_domToWorkspace_VariablesAtTop_MissingType() { ' name1' + ' ' + ' ' + - ' name3' + + ' name3' + ' ' + ''); Blockly.Xml.domToWorkspace(dom, workspace); @@ -233,7 +233,7 @@ function test_domToWorkspace_VariablesAtTop_MismatchBlockType() { ' name1' + ' ' + ' ' + - ' name1' + + ' name1' + ' ' + ''); Blockly.Xml.domToWorkspace(dom, workspace);