diff --git a/blocks/procedures.ts b/blocks/procedures.ts index 21fb883945d..9493a2dc035 100644 --- a/blocks/procedures.ts +++ b/blocks/procedures.ts @@ -308,7 +308,9 @@ const PROCEDURE_DEF_COMMON = { while (paramBlock && !paramBlock.isInsertionMarker()) { const varName = paramBlock.getFieldValue('NAME'); this.arguments_.push(varName); - const variable = this.workspace.getVariable(varName, '')!; + const variable = this.workspace + .getVariableMap() + .getVariable(varName, '')!; this.argumentVarModels_.push(variable); this.paramIds_.push(paramBlock.id); @@ -374,13 +376,13 @@ const PROCEDURE_DEF_COMMON = { oldId: string, newId: string, ) { - const oldVariable = this.workspace.getVariableById(oldId)!; + const oldVariable = this.workspace.getVariableMap().getVariableById(oldId)!; if (oldVariable.getType() !== '') { // Procedure arguments always have the empty type. return; } const oldName = oldVariable.getName(); - const newVar = this.workspace.getVariableById(newId)!; + const newVar = this.workspace.getVariableMap().getVariableById(newId)!; let change = false; for (let i = 0; i < this.argumentVarModels_.length; i++) { diff --git a/core/block.ts b/core/block.ts index af44facda5d..4c8f8084202 100644 --- a/core/block.ts +++ b/core/block.ts @@ -1161,9 +1161,9 @@ export class Block { const vars = []; for (const field of this.getFields()) { if (field.referencesVariables()) { - const model = this.workspace.getVariableById( - field.getValue() as string, - ); + const model = this.workspace + .getVariableMap() + .getVariableById(field.getValue() as string); // Check if the variable actually exists (and isn't just a potential // variable). if (model) { diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 3f2ed32d5ba..3b0bc614803 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -813,7 +813,9 @@ export abstract class Flyout createBlock(originalBlock: BlockSvg): BlockSvg { let newBlock = null; eventUtils.disable(); - const variablesBeforeCreation = this.targetWorkspace.getAllVariables(); + const variablesBeforeCreation = this.targetWorkspace + .getVariableMap() + .getAllVariables(); this.targetWorkspace.setResizesEnabled(false); try { newBlock = this.placeNewBlock(originalBlock); diff --git a/core/serialization/blocks.ts b/core/serialization/blocks.ts index 75ce4ae75cc..af8910b3137 100644 --- a/core/serialization/blocks.ts +++ b/core/serialization/blocks.ts @@ -391,7 +391,7 @@ export function appendInternal( } eventUtils.disable(); - const variablesBeforeCreation = workspace.getAllVariables(); + const variablesBeforeCreation = workspace.getVariableMap().getAllVariables(); let block; try { block = appendPrivate(state, workspace, {parentConnection, isShadow}); diff --git a/core/serialization/variables.ts b/core/serialization/variables.ts index d9c266fb834..a896606fa4e 100644 --- a/core/serialization/variables.ts +++ b/core/serialization/variables.ts @@ -32,7 +32,10 @@ export class VariableSerializer implements ISerializer { * variables. */ save(workspace: Workspace): IVariableState[] | null { - const variableStates = workspace.getAllVariables().map((v) => v.save()); + const variableStates = workspace + .getVariableMap() + .getAllVariables() + .map((v) => v.save()); return variableStates.length ? variableStates : null; } diff --git a/core/variables.ts b/core/variables.ts index f75d673903b..cbbd8843fa1 100644 --- a/core/variables.ts +++ b/core/variables.ts @@ -727,7 +727,7 @@ export function getVariable( // Try to just get the variable, by ID if possible. if (id) { // Look in the real variable map before checking the potential variable map. - variable = workspace.getVariableById(id); + variable = workspace.getVariableMap().getVariableById(id); if (!variable && potentialVariableMap) { variable = potentialVariableMap.getVariableById(id); } @@ -742,7 +742,7 @@ export function getVariable( throw Error('Tried to look up a variable by name without a type'); } // Otherwise look up by name and type. - variable = workspace.getVariable(opt_name, opt_type); + variable = workspace.getVariableMap().getVariable(opt_name, opt_type); if (!variable && potentialVariableMap) { variable = potentialVariableMap.getVariable(opt_name, opt_type); } @@ -809,7 +809,7 @@ export function getAddedVariables( workspace: Workspace, originalVariables: IVariableModel[], ): IVariableModel[] { - const allCurrentVariables = workspace.getAllVariables(); + const allCurrentVariables = workspace.getVariableMap().getAllVariables(); const addedVariables = []; if (originalVariables.length !== allCurrentVariables.length) { for (let i = 0; i < allCurrentVariables.length; i++) { diff --git a/core/xml.ts b/core/xml.ts index ea92522457a..3c06a39cca9 100644 --- a/core/xml.ts +++ b/core/xml.ts @@ -635,7 +635,7 @@ export function domToBlockInternal( ): Block { // Create top-level block. eventUtils.disable(); - const variablesBeforeCreation = workspace.getAllVariables(); + const variablesBeforeCreation = workspace.getVariableMap().getAllVariables(); let topBlock; try { topBlock = domToBlockHeadless(xmlBlock, workspace); diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 9c306a74040..ed41a728c30 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -2450,7 +2450,7 @@ suite('Blocks', function () { const blockA = createRenderedBlock(this.workspace, 'variable_block'); blockA.setCollapsed(true); - const variable = this.workspace.getVariable('x', ''); + const variable = this.workspace.getVariableMap().getVariable('x', ''); this.variableMap.renameVariable(variable, 'y'); this.clock.runAll(); diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index 6921ef1a45c..779684be125 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -26,12 +26,12 @@ suite('Procedures', function () { setup(function () { sharedTestSetup.call(this, {fireEventsNow: false}); this.workspace = Blockly.inject('blocklyDiv', {}); - this.workspace.createVariable('preCreatedVar', '', 'preCreatedVarId'); - this.workspace.createVariable( - 'preCreatedTypedVar', - 'type', - 'preCreatedTypedVarId', - ); + this.workspace + .getVariableMap() + .createVariable('preCreatedVar', '', 'preCreatedVarId'); + this.workspace + .getVariableMap() + .createVariable('preCreatedTypedVar', 'type', 'preCreatedTypedVarId'); defineRowBlock(); this.variableMap = this.workspace.getVariableMap(); }); @@ -433,7 +433,7 @@ suite('Procedures', function () { this.clock.runAll(); assert.isNotNull( - this.workspace.getVariable('param1', ''), + this.workspace.getVariableMap().getVariable('param1', ''), 'Expected the old variable to continue to exist', ); }); @@ -453,7 +453,9 @@ suite('Procedures', function () { this.clock.runAll(); mutatorIcon.setBubbleVisible(false); - const variable = this.workspace.getVariable('param1', ''); + const variable = this.workspace + .getVariableMap() + .getVariable('param1', ''); this.variableMap.renameVariable(variable, 'new name'); assert.isNotNull( @@ -480,7 +482,9 @@ suite('Procedures', function () { .connection.connect(paramBlock.previousConnection); this.clock.runAll(); - const variable = this.workspace.getVariable('param1', ''); + const variable = this.workspace + .getVariableMap() + .getVariable('param1', ''); this.variableMap.renameVariable(variable, 'new name'); assert.equal( @@ -506,7 +510,9 @@ suite('Procedures', function () { this.clock.runAll(); mutatorIcon.setBubbleVisible(false); - const variable = this.workspace.getVariable('param1', ''); + const variable = this.workspace + .getVariableMap() + .getVariable('param1', ''); this.variableMap.renameVariable(variable, 'new name'); assert.isNotNull( @@ -535,7 +541,9 @@ suite('Procedures', function () { this.clock.runAll(); mutatorIcon.setBubbleVisible(false); - const variable = this.workspace.getVariable('param1', ''); + const variable = this.workspace + .getVariableMap() + .getVariable('param1', ''); this.variableMap.renameVariable(variable, 'preCreatedVar'); assert.isNotNull( @@ -562,7 +570,9 @@ suite('Procedures', function () { .connection.connect(paramBlock.previousConnection); this.clock.runAll(); - const variable = this.workspace.getVariable('param1', ''); + const variable = this.workspace + .getVariableMap() + .getVariable('param1', ''); this.variableMap.renameVariable(variable, 'preCreatedVar'); assert.equal( @@ -588,7 +598,9 @@ suite('Procedures', function () { this.clock.runAll(); mutatorIcon.setBubbleVisible(false); - const variable = this.workspace.getVariable('param1', ''); + const variable = this.workspace + .getVariableMap() + .getVariable('param1', ''); this.variableMap.renameVariable(variable, 'preCreatedVar'); assert.isNotNull( diff --git a/tests/mocha/event_test.js b/tests/mocha/event_test.js index 475c76a5f0a..24bbf6bf505 100644 --- a/tests/mocha/event_test.js +++ b/tests/mocha/event_test.js @@ -833,11 +833,9 @@ suite('Events', function () { title: 'Variable events', testCases: variableEventTestCases, setup: (thisObj) => { - thisObj.variable = thisObj.workspace.createVariable( - 'name1', - 'type1', - 'id1', - ); + thisObj.variable = thisObj.workspace + .getVariableMap() + .createVariable('name1', 'type1', 'id1'); }, }, { @@ -1550,7 +1548,9 @@ suite('Events', function () { ); // Expect the workspace to have a variable with ID 'test_var_id'. - assert.isNotNull(this.workspace.getVariableById(TEST_VAR_ID)); + assert.isNotNull( + this.workspace.getVariableMap().getVariableById(TEST_VAR_ID), + ); }); }); suite('Disable orphans', function () { diff --git a/tests/mocha/field_variable_test.js b/tests/mocha/field_variable_test.js index c2b40326f2f..270a662cf0f 100644 --- a/tests/mocha/field_variable_test.js +++ b/tests/mocha/field_variable_test.js @@ -171,7 +171,7 @@ suite('Variable Fields', function () { suite('setValue', function () { setup(function () { - this.workspace.createVariable('name2', null, 'id2'); + this.workspace.getVariableMap().createVariable('name2', null, 'id2'); this.field = new Blockly.FieldVariable(null); initVariableField(this.workspace, this.field); @@ -209,8 +209,8 @@ suite('Variable Fields', function () { assert.include(dropdownOptions[dropdownOptions.length - 1][0], 'Delete'); }; test('Contains variables created before field', function () { - this.workspace.createVariable('name1', '', 'id1'); - this.workspace.createVariable('name2', '', 'id2'); + this.workspace.getVariableMap().createVariable('name1', '', 'id1'); + this.workspace.getVariableMap().createVariable('name2', '', 'id2'); // Expect that the dropdown options will contain the variables that exist const fieldVariable = initVariableField( this.workspace, @@ -228,22 +228,22 @@ suite('Variable Fields', function () { new Blockly.FieldVariable('name1'), ); // Expect that variables created after field creation will show up too. - this.workspace.createVariable('name2', '', 'id2'); + this.workspace.getVariableMap().createVariable('name2', '', 'id2'); assertDropdownContents(fieldVariable, [ ['name1', 'id1'], ['name2', 'id2'], ]); }); test('Contains variables created before and after field', function () { - this.workspace.createVariable('name1', '', 'id1'); - this.workspace.createVariable('name2', '', 'id2'); + this.workspace.getVariableMap().createVariable('name1', '', 'id1'); + this.workspace.getVariableMap().createVariable('name2', '', 'id2'); // Expect that the dropdown options will contain the variables that exist const fieldVariable = initVariableField( this.workspace, new Blockly.FieldVariable('name1'), ); // Expect that variables created after field creation will show up too. - this.workspace.createVariable('name3', '', 'id3'); + this.workspace.getVariableMap().createVariable('name3', '', 'id3'); assertDropdownContents(fieldVariable, [ ['name1', 'id1'], ['name2', 'id2'], @@ -254,9 +254,9 @@ suite('Variable Fields', function () { suite('Validators', function () { setup(function () { - this.workspace.createVariable('name1', null, 'id1'); - this.workspace.createVariable('name2', null, 'id2'); - this.workspace.createVariable('name3', null, 'id3'); + this.workspace.getVariableMap().createVariable('name1', null, 'id1'); + this.workspace.getVariableMap().createVariable('name2', null, 'id2'); + this.workspace.getVariableMap().createVariable('name3', null, 'id3'); this.variableField = initVariableField( this.workspace, new Blockly.FieldVariable('name1'), @@ -281,7 +281,9 @@ suite('Variable Fields', function () { }); test('New Value', function () { // Must create the var so that the field doesn't throw an error. - this.workspace.createVariable('thing2', null, 'other2'); + this.workspace + .getVariableMap() + .createVariable('thing2', null, 'other2'); this.variableField.setValue('other2'); assertFieldValue(this.variableField, 'id2', 'name2'); }); @@ -368,8 +370,8 @@ suite('Variable Fields', function () { }); suite('Get variable types', function () { setup(function () { - this.workspace.createVariable('name1', 'type1'); - this.workspace.createVariable('name2', 'type2'); + this.workspace.getVariableMap().createVariable('name1', 'type1'); + this.workspace.getVariableMap().createVariable('name2', 'type2'); }); test('variableTypes is explicit', function () { // Expect that since variableTypes is defined, it will be the return @@ -467,7 +469,7 @@ suite('Variable Fields', function () { }); suite('Renaming Variables', function () { setup(function () { - this.workspace.createVariable('name1', null, 'id1'); + this.workspace.getVariableMap().createVariable('name1', null, 'id1'); Blockly.defineBlocksWithJsonArray([ { 'type': 'field_variable_test_block', @@ -584,7 +586,7 @@ suite('Variable Fields', function () { }); test('ID', function () { - this.workspace.createVariable('test', '', 'id1'); + this.workspace.getVariableMap().createVariable('test', '', 'id1'); const block = Blockly.serialization.blocks.append( { 'type': 'variables_get', diff --git a/tests/mocha/test_helpers/procedures.js b/tests/mocha/test_helpers/procedures.js index 16ef973350f..2ea54a6c204 100644 --- a/tests/mocha/test_helpers/procedures.js +++ b/tests/mocha/test_helpers/procedures.js @@ -18,7 +18,9 @@ import {assert} from '../../../node_modules/chai/index.js'; function assertBlockVarModels(block, varIds) { const expectedVarModels = []; for (let i = 0; i < varIds.length; i++) { - expectedVarModels.push(block.workspace.getVariableById(varIds[i])); + expectedVarModels.push( + block.workspace.getVariableMap().getVariableById(varIds[i]), + ); } assert.sameDeepOrderedMembers(block.getVarModels(), expectedVarModels); } diff --git a/tests/mocha/test_helpers/variables.js b/tests/mocha/test_helpers/variables.js index dd19b4904c9..3c6ab9332ae 100644 --- a/tests/mocha/test_helpers/variables.js +++ b/tests/mocha/test_helpers/variables.js @@ -15,7 +15,11 @@ import {assert} from '../../../node_modules/chai/index.js'; * @param {!string} id The expected id of the variable. */ export function assertVariableValues(container, name, type, id) { - const variable = container.getVariableById(id); + const variableMap = + container instanceof Blockly.Workspace + ? container.getVariableMap() + : container; + const variable = variableMap.getVariableById(id); assert.isDefined(variable); assert.equal(variable.name, name); assert.equal(variable.type, type); diff --git a/tests/mocha/test_helpers/workspace.js b/tests/mocha/test_helpers/workspace.js index 452933c081a..9a1633da561 100644 --- a/tests/mocha/test_helpers/workspace.js +++ b/tests/mocha/test_helpers/workspace.js @@ -8,7 +8,6 @@ import * as eventUtils from '../../../build/src/core/events/utils.js'; import {assert} from '../../../node_modules/chai/index.js'; import {workspaceTeardown} from './setup_teardown.js'; import {assertVariableValues} from './variables.js'; -import {assertWarnings} from './warnings.js'; export function testAWorkspace() { setup(function () { @@ -102,7 +101,8 @@ export function testAWorkspace() { test('deleteVariableById(id2) one usage', function () { // Deleting variable one usage should not trigger confirm dialog. const stub = sinon.stub(window, 'confirm').returns(true); - this.variableMap.deleteVariableById('id2'); + const id2 = this.variableMap.getVariableById('id2'); + Blockly.Variables.deleteVariable(this.workspace, id2); sinon.assert.notCalled(stub); const variable = this.variableMap.getVariableById('id2'); @@ -116,7 +116,8 @@ export function testAWorkspace() { test('deleteVariableById(id1) multiple usages confirm', function () { // Deleting variable with multiple usages triggers confirm dialog. const stub = sinon.stub(window, 'confirm').returns(true); - this.variableMap.deleteVariableById('id1'); + const id1 = this.variableMap.getVariableById('id1'); + Blockly.Variables.deleteVariable(this.workspace, id1); sinon.assert.calledOnce(stub); const variable = this.variableMap.getVariableById('id1'); @@ -130,7 +131,8 @@ export function testAWorkspace() { test('deleteVariableById(id1) multiple usages cancel', function () { // Deleting variable with multiple usages triggers confirm dialog. const stub = sinon.stub(window, 'confirm').returns(false); - this.variableMap.deleteVariableById('id1'); + const id1 = this.variableMap.getVariableById('id1'); + Blockly.Variables.deleteVariable(this.workspace, id1); sinon.assert.calledOnce(stub); assertVariableValues(this.variableMap, 'name1', 'type1', 'id1'); @@ -143,13 +145,14 @@ export function testAWorkspace() { }); }); - suite('renameVariableById', function () { + suite('renameVariable', function () { setup(function () { this.variableMap.createVariable('name1', 'type1', 'id1'); }); test('No references rename to name2', function () { - this.variableMap.renameVariableById('id1', 'name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'name2'); assertVariableValues(this.variableMap, 'name2', 'type1', 'id1'); // Renaming should not have created a new variable. assert.equal(this.variableMap.getAllVariables().length, 1); @@ -158,7 +161,8 @@ export function testAWorkspace() { test('Reference exists rename to name2', function () { createVarBlocksNoEvents(this.workspace, ['id1']); - this.variableMap.renameVariableById('id1', 'name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'name2'); assertVariableValues(this.variableMap, 'name2', 'type1', 'id1'); // Renaming should not have created a new variable. assert.equal(this.variableMap.getAllVariables().length, 1); @@ -168,7 +172,8 @@ export function testAWorkspace() { test('Reference exists different capitalization rename to Name1', function () { createVarBlocksNoEvents(this.workspace, ['id1']); - this.variableMap.renameVariableById('id1', 'Name1'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'Name1'); assertVariableValues(this.variableMap, 'Name1', 'type1', 'id1'); // Renaming should not have created a new variable. assert.equal(this.variableMap.getAllVariables().length, 1); @@ -180,7 +185,8 @@ export function testAWorkspace() { this.variableMap.createVariable('name2', 'type1', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); - this.variableMap.renameVariableById('id1', 'name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'name2'); // The second variable should remain unchanged. assertVariableValues(this.workspace, 'name2', 'type1', 'id2'); @@ -199,7 +205,8 @@ export function testAWorkspace() { this.variableMap.createVariable('name2', 'type2', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); - this.variableMap.renameVariableById('id1', 'name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'name2'); // Variables with different type are allowed to have the same name. assertVariableValues(this.variableMap, 'name2', 'type1', 'id1'); @@ -214,7 +221,8 @@ export function testAWorkspace() { this.variableMap.createVariable('name2', 'type1', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); - this.variableMap.renameVariableById('id1', 'Name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'Name2'); // The second variable should be updated. assertVariableValues(this.variableMap, 'Name2', 'type1', 'id2'); @@ -233,7 +241,8 @@ export function testAWorkspace() { this.variableMap.createVariable('name2', 'type2', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); - this.variableMap.renameVariableById('id1', 'Name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'Name2'); // Variables with different type are allowed to have the same name. assertVariableValues(this.variableMap, 'Name2', 'type1', 'id1'); @@ -1250,8 +1259,8 @@ export function testAWorkspace() { suite('Variables', function () { function createTwoVarsDifferentTypes(workspace) { - workspace.createVariable('name1', 'type1', 'id1'); - workspace.createVariable('name2', 'type2', 'id2'); + workspace.getVariableMap().createVariable('name1', 'type1', 'id1'); + workspace.getVariableMap().createVariable('name2', 'type2', 'id2'); } suite('createVariable', function () { @@ -1262,11 +1271,11 @@ export function testAWorkspace() { this.workspace.undo(); this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); - assert.isNull(this.workspace.getVariableById('id2')); + assert.isNull(this.variableMap.getVariableById('id2')); this.workspace.undo(); - assert.isNull(this.workspace.getVariableById('id1')); - assert.isNull(this.workspace.getVariableById('id2')); + assert.isNull(this.variableMap.getVariableById('id1')); + assert.isNull(this.variableMap.getVariableById('id2')); }); test('Undo and redo', function () { @@ -1276,7 +1285,7 @@ export function testAWorkspace() { this.workspace.undo(); this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); - assert.isNull(this.workspace.getVariableById('id2')); + assert.isNull(this.variableMap.getVariableById('id2')); this.workspace.undo(true); @@ -1286,13 +1295,13 @@ export function testAWorkspace() { this.workspace.undo(); this.workspace.undo(); - assert.isNull(this.workspace.getVariableById('id1')); - assert.isNull(this.workspace.getVariableById('id2')); + assert.isNull(this.variableMap.getVariableById('id1')); + assert.isNull(this.variableMap.getVariableById('id2')); this.workspace.undo(true); // Expect that variable 'id1' is recreated assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); - assert.isNull(this.workspace.getVariableById('id2')); + assert.isNull(this.variableMap.getVariableById('id2')); }); }); @@ -1300,13 +1309,15 @@ export function testAWorkspace() { test('Undo only no usages', function () { createTwoVarsDifferentTypes(this.workspace); this.clock.runAll(); - this.workspace.deleteVariableById('id1'); - this.workspace.deleteVariableById('id2'); + const id1 = this.variableMap.getVariableById('id1'); + Blockly.Variables.deleteVariable(this.workspace, id1); + const id2 = this.variableMap.getVariableById('id2'); + Blockly.Variables.deleteVariable(this.workspace, id2); this.clock.runAll(); this.workspace.undo(); this.clock.runAll(); - assert.isNull(this.workspace.getVariableById('id1')); + assert.isNull(this.variableMap.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(); @@ -1320,14 +1331,16 @@ export function testAWorkspace() { // Create blocks to refer to both of them. createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); this.clock.runAll(); - this.workspace.deleteVariableById('id1'); - this.workspace.deleteVariableById('id2'); + const id1 = this.variableMap.getVariableById('id1'); + Blockly.Variables.deleteVariable(this.workspace, id1); + const id2 = this.variableMap.getVariableById('id2'); + Blockly.Variables.deleteVariable(this.workspace, id2); this.clock.runAll(); this.workspace.undo(); this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name2'); - assert.isNull(this.workspace.getVariableById('id1')); + assert.isNull(this.variableMap.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(); @@ -1341,20 +1354,22 @@ export function testAWorkspace() { test('Reference exists no usages', function () { createTwoVarsDifferentTypes(this.workspace); this.clock.runAll(); - this.workspace.deleteVariableById('id1'); - this.workspace.deleteVariableById('id2'); + const id1 = this.variableMap.getVariableById('id1'); + Blockly.Variables.deleteVariable(this.workspace, id1); + const id2 = this.variableMap.getVariableById('id2'); + Blockly.Variables.deleteVariable(this.workspace, id2); this.clock.runAll(); this.workspace.undo(); this.clock.runAll(); - assert.isNull(this.workspace.getVariableById('id1')); + assert.isNull(this.variableMap.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(true); this.clock.runAll(); // Expect that both variables are deleted - assert.isNull(this.workspace.getVariableById('id1')); - assert.isNull(this.workspace.getVariableById('id2')); + assert.isNull(this.variableMap.getVariableById('id1')); + assert.isNull(this.variableMap.getVariableById('id2')); this.workspace.undo(); this.clock.runAll(); @@ -1366,7 +1381,7 @@ export function testAWorkspace() { this.workspace.undo(true); this.clock.runAll(); // Expect that variable 'id2' is recreated - assert.isNull(this.workspace.getVariableById('id1')); + assert.isNull(this.variableMap.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); }); @@ -1375,22 +1390,24 @@ export function testAWorkspace() { // Create blocks to refer to both of them. createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); this.clock.runAll(); - this.workspace.deleteVariableById('id1'); - this.workspace.deleteVariableById('id2'); + const id1 = this.variableMap.getVariableById('id1'); + Blockly.Variables.deleteVariable(this.workspace, id1); + const id2 = this.variableMap.getVariableById('id2'); + Blockly.Variables.deleteVariable(this.workspace, id2); this.clock.runAll(); this.workspace.undo(); this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name2'); - assert.isNull(this.workspace.getVariableById('id1')); + assert.isNull(this.variableMap.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(true); this.clock.runAll(); // Expect that both variables are deleted assert.equal(this.workspace.getTopBlocks(false).length, 0); - assert.isNull(this.workspace.getVariableById('id1')); - assert.isNull(this.workspace.getVariableById('id2')); + assert.isNull(this.variableMap.getVariableById('id1')); + assert.isNull(this.variableMap.getVariableById('id2')); this.workspace.undo(); this.clock.runAll(); @@ -1405,84 +1422,19 @@ export function testAWorkspace() { this.clock.runAll(); // Expect that variable 'id2' is recreated assertBlockVarModelName(this.workspace, 0, 'name2'); - assert.isNull(this.workspace.getVariableById('id1')); + assert.isNull(this.variableMap.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); }); - - test('Delete same variable twice no usages', function () { - this.workspace.createVariable('name1', 'type1', 'id1'); - this.workspace.deleteVariableById('id1'); - this.clock.runAll(); - const workspace = this.workspace; - assertWarnings(() => { - workspace.deleteVariableById('id1'); - }, /Can't delete/); - - // Check the undoStack only recorded one delete event. - const undoStack = this.workspace.undoStack_; - assert.equal(undoStack[undoStack.length - 1].type, 'var_delete'); - assert.notEqual(undoStack[undoStack.length - 2].type, 'var_delete'); - - // Undo delete - this.workspace.undo(); - this.clock.runAll(); - assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); - - // Redo delete - this.workspace.undo(true); - this.clock.runAll(); - assert.isNull(this.workspace.getVariableById('id1')); - - // Redo delete, nothing should happen - this.workspace.undo(true); - this.clock.runAll(); - assert.isNull(this.workspace.getVariableById('id1')); - }); - - test('Delete same variable twice with usages', function () { - this.workspace.createVariable('name1', 'type1', 'id1'); - createVarBlocksNoEvents(this.workspace, ['id1']); - this.clock.runAll(); - this.workspace.deleteVariableById('id1'); - this.clock.runAll(); - const workspace = this.workspace; - assertWarnings(() => { - workspace.deleteVariableById('id1'); - }, /Can't delete/); - - // Check the undoStack only recorded one delete event. - const undoStack = this.workspace.undoStack_; - assert.equal(undoStack[undoStack.length - 1].type, 'var_delete'); - assert.equal(undoStack[undoStack.length - 2].type, 'delete'); - assert.notEqual(undoStack[undoStack.length - 3].type, 'var_delete'); - - // Undo delete - this.workspace.undo(); - this.clock.runAll(); - assertBlockVarModelName(this.workspace, 0, 'name1'); - assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); - - // Redo delete - this.workspace.undo(true); - this.clock.runAll(); - assert.equal(this.workspace.getTopBlocks(false).length, 0); - assert.isNull(this.workspace.getVariableById('id1')); - - // Redo delete, nothing should happen - this.workspace.undo(true); - this.clock.runAll(); - assert.equal(this.workspace.getTopBlocks(false).length, 0); - assert.isNull(this.workspace.getVariableById('id1')); - }); }); - suite('renameVariableById', function () { + suite('renameVariable', function () { setup(function () { this.variableMap.createVariable('name1', 'type1', 'id1'); }); test('Reference exists no usages rename to name2', function () { - this.variableMap.renameVariableById('id1', 'name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'name2'); this.clock.runAll(); this.workspace.undo(); @@ -1496,7 +1448,8 @@ export function testAWorkspace() { test('Reference exists with usages rename to name2', function () { createVarBlocksNoEvents(this.workspace, ['id1']); - this.variableMap.renameVariableById('id1', 'name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'name2'); this.clock.runAll(); this.workspace.undo(); @@ -1511,7 +1464,8 @@ export function testAWorkspace() { }); test('Reference exists different capitalization no usages rename to Name1', function () { - this.variableMap.renameVariableById('id1', 'Name1'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'Name1'); this.clock.runAll(); this.workspace.undo(); @@ -1525,7 +1479,8 @@ export function testAWorkspace() { test('Reference exists different capitalization with usages rename to Name1', function () { createVarBlocksNoEvents(this.workspace, ['id1']); - this.variableMap.renameVariableById('id1', 'Name1'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'Name1'); this.clock.runAll(); this.workspace.undo(); @@ -1542,7 +1497,8 @@ export function testAWorkspace() { suite('Two variables rename overlap', function () { test('Same type no usages rename variable with id1 to name2', function () { this.variableMap.createVariable('name2', 'type1', 'id2'); - this.variableMap.renameVariableById('id1', 'name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'name2'); this.clock.runAll(); this.workspace.undo(); @@ -1563,7 +1519,8 @@ export function testAWorkspace() { 'id2', ); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); - this.variableMap.renameVariableById('id1', 'name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'name2'); this.clock.runAll(); this.workspace.undo(); @@ -1581,7 +1538,8 @@ export function testAWorkspace() { test('Same type different capitalization no usages rename variable with id1 to Name2', function () { this.variableMap.createVariable('name2', 'type1', 'id2'); - this.variableMap.renameVariableById('id1', 'Name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'Name2'); this.clock.runAll(); this.workspace.undo(); @@ -1596,9 +1554,10 @@ export function testAWorkspace() { }); test('Same type different capitalization with usages rename variable with id1 to Name2', function () { - this.workspace.createVariable('name2', 'type1', 'id2'); + this.variableMap.createVariable('name2', 'type1', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); - this.variableMap.renameVariableById('id1', 'Name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'Name2'); this.clock.runAll(); this.workspace.undo(); @@ -1618,7 +1577,8 @@ export function testAWorkspace() { test('Different type no usages rename variable with id1 to name2', function () { this.variableMap.createVariable('name2', 'type2', 'id2'); - this.variableMap.renameVariableById('id1', 'name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'name2'); this.clock.runAll(); this.workspace.undo(); @@ -1635,7 +1595,8 @@ export function testAWorkspace() { test('Different type with usages rename variable with id1 to name2', function () { this.variableMap.createVariable('name2', 'type2', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); - this.variableMap.renameVariableById('id1', 'name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'name2'); this.clock.runAll(); this.workspace.undo(); @@ -1655,7 +1616,8 @@ export function testAWorkspace() { test('Different type different capitalization no usages rename variable with id1 to Name2', function () { this.variableMap.createVariable('name2', 'type2', 'id2'); - this.variableMap.renameVariableById('id1', 'Name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'Name2'); this.clock.runAll(); this.workspace.undo(); @@ -1672,7 +1634,8 @@ export function testAWorkspace() { test('Different type different capitalization with usages rename variable with id1 to Name2', function () { this.variableMap.createVariable('name2', 'type2', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); - this.variableMap.renameVariableById('id1', 'Name2'); + const id1 = this.variableMap.getVariableById('id1'); + this.variableMap.renameVariable(id1, 'Name2'); this.clock.runAll(); this.workspace.undo(); diff --git a/tests/mocha/variable_map_test.js b/tests/mocha/variable_map_test.js index b0ceec28e4c..505a221bf80 100644 --- a/tests/mocha/variable_map_test.js +++ b/tests/mocha/variable_map_test.js @@ -388,35 +388,6 @@ suite('Variable Map', function () { ); }); }); - - suite('deleting by ID', function () { - test('delete events are fired when a variable is deleted', function () { - this.variableMap.createVariable('test name', 'test type', 'test id'); - this.variableMap.deleteVariableById('test id'); - - assertEventFired( - this.eventSpy, - Blockly.Events.VarDelete, - { - varType: 'test type', - varName: 'test name', - varId: 'test id', - }, - this.workspace.id, - ); - }); - - test('delete events are not fired when a variable does not exist', function () { - this.variableMap.deleteVariableById('test id'); - - assertEventNotFired( - this.eventSpy, - Blockly.Events.VarDelete, - {}, - this.workspace.id, - ); - }); - }); }); suite('variable rename events', function () { @@ -473,44 +444,6 @@ suite('Variable Map', function () { ); }); }); - - suite('renaming by ID', function () { - test('rename events are fired when a variable is renamed', function () { - this.variableMap.createVariable('test name', 'test type', 'test id'); - this.variableMap.renameVariableById('test id', 'new test name'); - - assertEventFired( - this.eventSpy, - Blockly.Events.VarRename, - { - oldName: 'test name', - newName: 'new test name', - varId: 'test id', - }, - this.workspace.id, - ); - }); - - test('rename events are not fired if the variable name already matches', function () { - this.variableMap.createVariable('test name', 'test type', 'test id'); - this.variableMap.renameVariableById('test id', 'test name'); - - assertEventNotFired( - this.eventSpy, - Blockly.Events.VarRename, - {}, - this.workspace.id, - ); - }); - - test('renaming throws if the variable does not exist', function () { - // Not sure why this throws when the other one doesn't but might - // as well test it. - assert.throws(() => { - this.variableMap.renameVariableById('test id', 'test name'); - }, `Tried to rename a variable that didn't exist`); - }); - }); }); suite('variable type change events', function () { diff --git a/tests/mocha/xml_test.js b/tests/mocha/xml_test.js index 210b8170585..94219f9a067 100644 --- a/tests/mocha/xml_test.js +++ b/tests/mocha/xml_test.js @@ -449,7 +449,7 @@ suite('XML', function () { createGenUidStubWithReturns('1'); this.variableMap.createVariable('name1'); const resultDom = Blockly.Xml.variablesToDom( - this.workspace.getAllVariables(), + this.workspace.getVariableMap().getAllVariables(), ); assert.equal(resultDom.children.length, 1); const resultVariableDom = resultDom.children[0]; @@ -471,7 +471,7 @@ suite('XML', function () { Blockly.Events.enable(); const resultDom = Blockly.Xml.variablesToDom( - this.workspace.getAllVariables(), + this.workspace.getVariableMap().getAllVariables(), ); assert.equal(resultDom.children.length, 2); assertVariableDom(resultDom.children[0], null, 'id1', 'name1'); @@ -479,7 +479,7 @@ suite('XML', function () { }); test('No variables', function () { const resultDom = Blockly.Xml.variablesToDom( - this.workspace.getAllVariables(), + this.workspace.getVariableMap().getAllVariables(), ); assert.equal(resultDom.children.length, 0); });