From b08f44bec449c2f94573ed33ab130def08310ea4 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 20 Oct 2022 18:11:36 +0000 Subject: [PATCH 1/8] fix: expand the IParameterModel interface --- core/interfaces/i_parameter_model.ts | 10 ++++++++++ core/procedures/observable_parameter_model.ts | 14 ++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/core/interfaces/i_parameter_model.ts b/core/interfaces/i_parameter_model.ts index 00d11724034..fe9eda6b0c5 100644 --- a/core/interfaces/i_parameter_model.ts +++ b/core/interfaces/i_parameter_model.ts @@ -25,6 +25,16 @@ export interface IParameterModel { */ setTypes(types: string[]): this; + /** + * Returns the name of this parameter. + */ + getName(): string; + + /** + * Return the types of this parameter. + */ + getTypes(): string[]; + /** * Returns the unique language-neutral ID for the parameter. * diff --git a/core/procedures/observable_parameter_model.ts b/core/procedures/observable_parameter_model.ts index fe0f5bdb1ba..535c6aea164 100644 --- a/core/procedures/observable_parameter_model.ts +++ b/core/procedures/observable_parameter_model.ts @@ -48,6 +48,20 @@ export class ObservableParameterModel implements IParameterModel { 'implement your own custom ParameterModel.'); } + /** + * Returns the name of this parameter. + */ + getName(): string { + return this.variable.name; + } + + /** + * Returns the types of this parameter. + */ + getTypes(): string[] { + return []; + } + /** * Returns the unique language-neutral ID for the parameter. * From 3221e7a39a0b1c5f109940accd97219442c577f2 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 20 Oct 2022 18:27:53 +0000 Subject: [PATCH 2/8] fix: remove support for return types from the concrete procedure model --- core/procedures/observable_procedure_model.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/procedures/observable_procedure_model.ts b/core/procedures/observable_procedure_model.ts index e338d5161d8..95fed9810ac 100644 --- a/core/procedures/observable_procedure_model.ts +++ b/core/procedures/observable_procedure_model.ts @@ -56,8 +56,13 @@ export class ObservableProcedureModel implements IProcedureModel { * Pass null to represent a procedure that does not return. */ setReturnTypes(types: string[]|null): this { - // TODO(#6516): Fire events. + if (types && types.length) { + throw new Error( + 'The built-in ProcedureModel does not support typing. You need to ' + + 'implement your own custom ProcedureModel.'); + } this.returnTypes = types; + // TODO(#65): Fire events. triggerProceduresUpdate(this.workspace); return this; } From 0158363b2437e0dfa3578f2879fb00696523b8c0 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 20 Oct 2022 18:43:55 +0000 Subject: [PATCH 3/8] feat: add an interface for the procedure map, and add getting procedures --- core/interfaces/i_procedure_map.ts | 20 ++++++++++++++++++++ core/procedures/observable_procedure_map.ts | 12 +++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 core/interfaces/i_procedure_map.ts diff --git a/core/interfaces/i_procedure_map.ts b/core/interfaces/i_procedure_map.ts new file mode 100644 index 00000000000..5eb6744b20d --- /dev/null +++ b/core/interfaces/i_procedure_map.ts @@ -0,0 +1,20 @@ +/** + * @license + * Copyright 2022 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + + import {IProcedureModel} from './i_procedure_model.js'; + + +export interface IProcedureMap extends Map { + /** + * Adds the given ProcedureModel to the map of procedure models, so that + * blocks can find it. + */ + add(proc: IProcedureModel): this; + + + /** Returns all of the procedures stored in this map. */ + getProcedures(): IProcedureModel[]; +} diff --git a/core/procedures/observable_procedure_map.ts b/core/procedures/observable_procedure_map.ts index 781cb4682fc..66496b2a8fa 100644 --- a/core/procedures/observable_procedure_map.ts +++ b/core/procedures/observable_procedure_map.ts @@ -7,9 +7,12 @@ import type {IProcedureModel} from '../interfaces/i_procedure_model.js'; import {triggerProceduresUpdate} from './update_procedures.js'; import type {Workspace} from '../workspace.js'; +import {IProcedureMap} from '../interfaces/i_procedure_map.js'; -export class ObservableProcedureMap extends Map { +export class ObservableProcedureMap + extends Map + implements IProcedureMap { constructor(private readonly workspace: Workspace) { super(); } @@ -52,4 +55,11 @@ export class ObservableProcedureMap extends Map { // TODO(#6526): See if this method is actually useful. return this.set(proc.getId(), proc); } + + /** + * Returns all of the procedures stored in this map. + */ + getProcedures(): IProcedureModel[] { + return [...this.values()]; + } } From c2151336d95341204d514271f518a97e7ee24993 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 20 Oct 2022 19:42:24 +0000 Subject: [PATCH 4/8] fix: add procedure map to workspace --- core/workspace.ts | 8 ++++++++ tests/mocha/procedure_map_test.js | 10 ++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/core/workspace.ts b/core/workspace.ts index 101596374cf..f7da47e5193 100644 --- a/core/workspace.ts +++ b/core/workspace.ts @@ -32,6 +32,8 @@ import type * as toolbox from './utils/toolbox.js'; import {VariableMap} from './variable_map.js'; import type {VariableModel} from './variable_model.js'; import type {WorkspaceComment} from './workspace_comment.js'; +import { IProcedureMap } from './interfaces/i_procedure_map.js'; +import { ObservableProcedureMap } from './procedures.js'; /** @@ -110,6 +112,7 @@ export class Workspace implements IASTNodeLocation { private readonly blockDB = new Map(); private readonly typedBlocksDB = new Map(); private variableMap: VariableMap; + private procedureMap: IProcedureMap = new ObservableProcedureMap(this); /** * Blocks in the flyout can refer to variables that don't exist in the main @@ -829,6 +832,11 @@ export class Workspace implements IASTNodeLocation { this.variableMap = variableMap; } + /** Returns the map of all procedures on the workpace. */ + getProcedureMap(): IProcedureMap { + return this.procedureMap; + } + /** * Find the workspace with the specified ID. * diff --git a/tests/mocha/procedure_map_test.js b/tests/mocha/procedure_map_test.js index 34426eff493..0019e812579 100644 --- a/tests/mocha/procedure_map_test.js +++ b/tests/mocha/procedure_map_test.js @@ -8,13 +8,11 @@ import {sharedTestSetup, sharedTestTeardown} from './test_helpers/setup_teardown goog.declareModuleId('Blockly.test.procedureMap'); -suite('Procedure Map', function() { +suite.only('Procedure Map', function() { setup(function() { sharedTestSetup.call(this); this.workspace = new Blockly.Workspace(); - // this.procedureMap = this.workspace.getProcedureMap(); - this.procedureMap = - new Blockly.procedures.ObservableProcedureMap(this.workspace); + this.procedureMap = this.workspace.getProcedureMap(); }); teardown(function() { @@ -84,7 +82,7 @@ suite('Procedure Map', function() { new Blockly.procedures.ObservableProcedureModel(this.workspace); this.procedureMap.add(procedureModel); - procedureModel.setReturnTypes(['return type 1', 'return type 2']); + procedureModel.setReturnTypes([]); chai.assert.isTrue( this.updateSpy.calledOnce, 'Expected an update to be triggered'); @@ -93,7 +91,7 @@ suite('Procedure Map', function() { test('removing the return type triggers an update', function() { const procedureModel = new Blockly.procedures.ObservableProcedureModel(this.workspace) - .setReturnTypes(['return type']); + .setReturnTypes([]); this.procedureMap.add(procedureModel); this.updateSpy.resetHistory(); From f94f8eadad17edb0ce7e41ebaf0c04b191f2abdd Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 20 Oct 2022 19:51:11 +0000 Subject: [PATCH 5/8] chore: format --- core/interfaces/i_procedure_map.ts | 2 +- core/procedures/observable_procedure_map.ts | 5 ++--- core/procedures/observable_procedure_model.ts | 4 ++-- core/workspace.ts | 4 ++-- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/core/interfaces/i_procedure_map.ts b/core/interfaces/i_procedure_map.ts index 5eb6744b20d..0eead4025cc 100644 --- a/core/interfaces/i_procedure_map.ts +++ b/core/interfaces/i_procedure_map.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ - import {IProcedureModel} from './i_procedure_model.js'; +import {IProcedureModel} from './i_procedure_model.js'; export interface IProcedureMap extends Map { diff --git a/core/procedures/observable_procedure_map.ts b/core/procedures/observable_procedure_map.ts index 66496b2a8fa..a9ad169c57a 100644 --- a/core/procedures/observable_procedure_map.ts +++ b/core/procedures/observable_procedure_map.ts @@ -10,9 +10,8 @@ import type {Workspace} from '../workspace.js'; import {IProcedureMap} from '../interfaces/i_procedure_map.js'; -export class ObservableProcedureMap - extends Map - implements IProcedureMap { +export class ObservableProcedureMap extends + Map implements IProcedureMap { constructor(private readonly workspace: Workspace) { super(); } diff --git a/core/procedures/observable_procedure_model.ts b/core/procedures/observable_procedure_model.ts index 95fed9810ac..e214e19560e 100644 --- a/core/procedures/observable_procedure_model.ts +++ b/core/procedures/observable_procedure_model.ts @@ -58,8 +58,8 @@ export class ObservableProcedureModel implements IProcedureModel { setReturnTypes(types: string[]|null): this { if (types && types.length) { throw new Error( - 'The built-in ProcedureModel does not support typing. You need to ' + - 'implement your own custom ProcedureModel.'); + 'The built-in ProcedureModel does not support typing. You need to ' + + 'implement your own custom ProcedureModel.'); } this.returnTypes = types; // TODO(#65): Fire events. diff --git a/core/workspace.ts b/core/workspace.ts index f7da47e5193..0602fa8347e 100644 --- a/core/workspace.ts +++ b/core/workspace.ts @@ -32,8 +32,8 @@ import type * as toolbox from './utils/toolbox.js'; import {VariableMap} from './variable_map.js'; import type {VariableModel} from './variable_model.js'; import type {WorkspaceComment} from './workspace_comment.js'; -import { IProcedureMap } from './interfaces/i_procedure_map.js'; -import { ObservableProcedureMap } from './procedures.js'; +import {IProcedureMap} from './interfaces/i_procedure_map.js'; +import {ObservableProcedureMap} from './procedures.js'; /** From b3d114c5ae6f5c43d559aa7a3be2b9ac76cf9657 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Tue, 25 Oct 2022 16:16:07 +0000 Subject: [PATCH 6/8] fix: add name parameter to procedure model to match parameter model --- core/procedures/observable_procedure_model.ts | 5 ++- tests/mocha/procedure_map_test.js | 38 ++++++++++++------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/core/procedures/observable_procedure_model.ts b/core/procedures/observable_procedure_model.ts index e214e19560e..2972e6ae47b 100644 --- a/core/procedures/observable_procedure_model.ts +++ b/core/procedures/observable_procedure_model.ts @@ -13,13 +13,14 @@ import type {Workspace} from '../workspace.js'; export class ObservableProcedureModel implements IProcedureModel { private id: string; - private name = ''; + private name: string; private parameters: IParameterModel[] = []; private returnTypes: string[]|null = null; private enabled = true; - constructor(private readonly workspace: Workspace, id?: string) { + constructor(private readonly workspace: Workspace, name: string, id?: string) { this.id = id ?? genUid(); + this.name = name; } /** Sets the human-readable name of the procedure. */ diff --git a/tests/mocha/procedure_map_test.js b/tests/mocha/procedure_map_test.js index 0019e812579..848dda8dd18 100644 --- a/tests/mocha/procedure_map_test.js +++ b/tests/mocha/procedure_map_test.js @@ -8,7 +8,7 @@ import {sharedTestSetup, sharedTestTeardown} from './test_helpers/setup_teardown goog.declareModuleId('Blockly.test.procedureMap'); -suite.only('Procedure Map', function() { +suite('Procedure Map', function() { setup(function() { sharedTestSetup.call(this); this.workspace = new Blockly.Workspace(); @@ -38,7 +38,8 @@ suite.only('Procedure Map', function() { suite('procedure map updates', function() { test('inserting a procedure does not trigger an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace); + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name'); this.procedureMap.set(procedureModel.getId(), procedureModel); chai.assert.isFalse( @@ -47,7 +48,8 @@ suite.only('Procedure Map', function() { test('adding a procedure does not trigger an update', function() { this.procedureMap.add( - new Blockly.procedures.ObservableProcedureModel(this.workspace)); + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name')); chai.assert.isFalse( this.updateSpy.called, 'Expected no update to be triggered'); @@ -55,7 +57,8 @@ suite.only('Procedure Map', function() { test('deleting a procedure triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace); + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name'); this.procedureMap.add(procedureModel); this.procedureMap.delete(procedureModel.getId()); @@ -68,7 +71,8 @@ suite.only('Procedure Map', function() { suite('procedure model updates', function() { test('setting the name triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace); + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name'); this.procedureMap.add(procedureModel); procedureModel.setName('new name'); @@ -79,7 +83,8 @@ suite.only('Procedure Map', function() { test('setting the return type triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace); + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name'); this.procedureMap.add(procedureModel); procedureModel.setReturnTypes([]); @@ -90,7 +95,8 @@ suite.only('Procedure Map', function() { test('removing the return type triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace) + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name') .setReturnTypes([]); this.procedureMap.add(procedureModel); this.updateSpy.resetHistory(); @@ -103,7 +109,8 @@ suite.only('Procedure Map', function() { test('disabling the procedure triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace); + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name'); this.procedureMap.add(procedureModel); procedureModel.setEnabled(false); @@ -114,7 +121,8 @@ suite.only('Procedure Map', function() { test('enabling the procedure triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace) + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name') .setEnabled(false); this.procedureMap.add(procedureModel); this.updateSpy.resetHistory(); @@ -127,7 +135,8 @@ suite.only('Procedure Map', function() { test('inserting a parameter triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace); + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name'); this.procedureMap.add(procedureModel); procedureModel.insertParameter( @@ -139,7 +148,8 @@ suite.only('Procedure Map', function() { test('deleting a parameter triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace) + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name') .insertParameter( new Blockly.procedures.ObservableParameterModel( this.workspace)); @@ -159,7 +169,8 @@ suite.only('Procedure Map', function() { new Blockly.procedures.ObservableParameterModel( this.workspace, 'test1'); this.procedureMap.add( - new Blockly.procedures.ObservableProcedureModel(this.workspace) + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name') .insertParameter(parameterModel)); this.updateSpy.resetHistory(); @@ -174,7 +185,8 @@ suite.only('Procedure Map', function() { new Blockly.procedures.ObservableParameterModel( this.workspace, 'test1'); this.procedureMap.add( - new Blockly.procedures.ObservableProcedureModel(this.workspace) + new Blockly.procedures.ObservableProcedureModel( + this.workspace, 'test name') .insertParameter(parameterModel)); this.updateSpy.resetHistory(); From 3f8f22b14b332e0d23cc5847bcafce20ca33e9ff Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Tue, 25 Oct 2022 16:16:58 +0000 Subject: [PATCH 7/8] chore: format --- core/procedures/observable_procedure_model.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/procedures/observable_procedure_model.ts b/core/procedures/observable_procedure_model.ts index 2972e6ae47b..77ffc3b24bc 100644 --- a/core/procedures/observable_procedure_model.ts +++ b/core/procedures/observable_procedure_model.ts @@ -18,7 +18,8 @@ export class ObservableProcedureModel implements IProcedureModel { private returnTypes: string[]|null = null; private enabled = true; - constructor(private readonly workspace: Workspace, name: string, id?: string) { + constructor( + private readonly workspace: Workspace, name: string, id?: string) { this.id = id ?? genUid(); this.name = name; } From 306b17f249b50e0af1eb5a7edbc89a054725a00c Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Tue, 25 Oct 2022 21:09:32 +0000 Subject: [PATCH 8/8] chore: fix comments --- core/procedures/observable_procedure_model.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/core/procedures/observable_procedure_model.ts b/core/procedures/observable_procedure_model.ts index 77ffc3b24bc..be8d05fe599 100644 --- a/core/procedures/observable_procedure_model.ts +++ b/core/procedures/observable_procedure_model.ts @@ -53,9 +53,13 @@ export class ObservableProcedureModel implements IProcedureModel { } /** - * Sets the return type(s) of the procedure. + * Sets whether the procedure has a return value (empty array) or no return + * value (null). * - * Pass null to represent a procedure that does not return. + * The built-in procedure model does not support procedures that have actual + * return types (i.e. non-empty arrays, e.g. ['number']). If you want your + * procedure block to have return types, you need to implement your own + * procedure model. */ setReturnTypes(types: string[]|null): this { if (types && types.length) { @@ -64,7 +68,7 @@ export class ObservableProcedureModel implements IProcedureModel { 'implement your own custom ProcedureModel.'); } this.returnTypes = types; - // TODO(#65): Fire events. + // TODO(#6516): Fire events. triggerProceduresUpdate(this.workspace); return this; }