feat: Improved procedure arg interaction. (#3527)

* feat: Improved procedure arg interaction.

* Added docs.

* Fixed typos and typings.

* Fixed typings?

* Changed visibility to private.
This commit is contained in:
Beka Westberg
2020-01-07 13:55:46 -08:00
committed by alschmiedt
parent 267877115f
commit 6d1bb201f7
8 changed files with 166 additions and 120 deletions

View File

@@ -483,71 +483,6 @@ Blockly.Blocks['procedures_mutatorcontainer'] = {
this.setTooltip(Blockly.Msg['PROCEDURES_MUTATORCONTAINER_TOOLTIP']);
this.contextMenu = false;
},
// TODO: Move this to a validator on the arg blocks, that way it can be
// tested.
/**
* This will create & delete variables and in dialogs workspace to ensure
* that when a new block is dragged out it will have a unique parameter name.
* @param {!Blockly.Events.Abstract} event Change event.
* @this {Blockly.Block}
*/
onchange: function(event) {
if (!this.workspace || this.workspace.isFlyout ||
(event.type != Blockly.Events.BLOCK_DELETE && event.type != Blockly.Events.BLOCK_CREATE)) {
return;
}
var blocks = this.workspace.getAllBlocks(false);
var allVariables = this.workspace.getAllVariables();
if (event.type == Blockly.Events.BLOCK_DELETE) {
var variableNamesToKeep = [];
for (var i = 0; i < blocks.length; i += 1) {
if (blocks[i].getFieldValue('NAME')) {
variableNamesToKeep.push(blocks[i].getFieldValue('NAME'));
}
}
for (var k = 0; k < allVariables.length; k += 1) {
if (variableNamesToKeep.indexOf(allVariables[k].name) == -1) {
this.workspace.deleteVariableById(allVariables[k].getId());
}
}
return;
}
if (event.type != Blockly.Events.BLOCK_CREATE) {
return;
}
var block = this.workspace.getBlockById(event.blockId);
// This is to handle the one none variable block
// Happens when all the blocks are regenerated
if (!block.getField('NAME')) {
return;
}
var varName = block.getFieldValue('NAME');
var variable = this.workspace.getVariable(varName);
if (!variable) {
// This means the parameter name is not in use and we can create the variable.
variable = this.workspace.createVariable(varName);
}
// If the blocks are connected we don't have to check duplicate variables
// This only happens if the dialog box is open
if (block.previousConnection.isConnected() || block.nextConnection.isConnected()) {
return;
}
for (var j = 0; j < blocks.length; j += 1) {
// filter block that was created
if (block.id != blocks[j].id && blocks[j].getFieldValue('NAME') == variable.name) {
// generate new name and set name field
varName = Blockly.Variables.generateUniqueName(this.workspace);
variable = this.workspace.createVariable(varName);
block.setFieldValue(variable.name, 'NAME');
return;
}
}
}
};
Blockly.Blocks['procedures_mutatorarg'] = {
@@ -556,7 +491,8 @@ Blockly.Blocks['procedures_mutatorarg'] = {
* @this {Blockly.Block}
*/
init: function() {
var field = new Blockly.FieldTextInput('x', this.validator_);
var field = new Blockly.FieldTextInput(
Blockly.Procedures.DEFAULT_ARG, this.validator_);
// Hack: override showEditor to do just a little bit more work.
// We don't have a good place to hook into the start of a text edit.
field.oldShowEditorFn_ = field.showEditor_;
@@ -603,7 +539,9 @@ Blockly.Blocks['procedures_mutatorarg'] = {
}
// Prevents duplicate parameter names in functions
var blocks = sourceBlock.workspace.getAllBlocks(false);
var workspace = sourceBlock.workspace.targetWorkspace ||
sourceBlock.workspace;
var blocks = workspace.getAllBlocks(false);
for (var i = 0; i < blocks.length; i++) {
if (blocks[i].id == this.getSourceBlock().id) {
continue;
@@ -613,6 +551,12 @@ Blockly.Blocks['procedures_mutatorarg'] = {
}
}
// Don't create variables for arg blocks that
// only exist in the mutator's flyout.
if (sourceBlock.isInFlyout) {
return varName;
}
var model = outerWs.getVariable(varName, '');
if (model && model.name != varName) {
// Rename the variable (case change)
@@ -626,6 +570,7 @@ Blockly.Blocks['procedures_mutatorarg'] = {
}
return varName;
},
/**
* Called when focusing away from the text field.
* Deletes all variables that were created as the user typed their intended

View File

@@ -72,6 +72,15 @@ Blockly.Mutator.prototype.setBlock = function(block) {
this.block_ = block;
};
/**
* Returns the workspace inside this mutator icon's bubble.
* @return {Blockly.WorkspaceSvg} The workspace inside this mutator icon's
* bubble.
*/
Blockly.Mutator.prototype.getWorkspace = function() {
return this.workspace_;
};
/**
* Draw the mutator icon.
* @param {!Element} group The icon group.

View File

@@ -46,6 +46,12 @@ goog.require('Blockly.Xml');
*/
Blockly.Procedures.NAME_TYPE = Blockly.PROCEDURE_CATEGORY_NAME;
/**
* The default argument for a procedures_mutatorarg block.
* @type {string}
*/
Blockly.Procedures.DEFAULT_ARG = 'x';
/**
* Procedure block type.
* @typedef {{
@@ -271,6 +277,77 @@ Blockly.Procedures.flyoutCategory = function(workspace) {
return xmlList;
};
/**
* Updates the procedure mutator's flyout so that the arg block is not a
* duplicate of another arg.
* @param {!Blockly.Workspace} workspace The procedure mutator's workspace. This
* workspace's flyout is what is being updated.
* @private
*/
Blockly.Procedures.updateMutatorFlyout_ = function(workspace) {
var usedNames = [];
var blocks = workspace.getBlocksByType('procedures_mutatorarg', false);
for (var i = 0, block; (block = blocks[i]); i++) {
usedNames.push(block.getFieldValue('NAME'));
}
var xml = Blockly.utils.xml.createElement('xml');
var argBlock = Blockly.utils.xml.createElement('block');
argBlock.setAttribute('type', 'procedures_mutatorarg');
var nameField = Blockly.utils.xml.createElement('field');
nameField.setAttribute('name', 'NAME');
var argValue = Blockly.Variables.generateUniqueNameFromOptions(
Blockly.Procedures.DEFAULT_ARG, usedNames);
var fieldContent = Blockly.utils.xml.createTextNode(argValue);
nameField.appendChild(fieldContent);
argBlock.appendChild(nameField);
xml.appendChild(argBlock);
workspace.updateToolbox(xml);
};
/**
* Listens for when a procedure mutator is opened. Then it triggers a flyout
* update and adds a mutator change listener to the mutator workspace.
* @param {!Blockly.Events.Abstract} e The event that triggered this listener.
* @package
*/
Blockly.Procedures.mutatorOpenListener = function(e) {
if (e.type != Blockly.Events.UI || e.element != 'mutatorOpen' ||
!e.newValue) {
return;
}
var workspaceId = /** @type {string} */ (e.workspaceId);
var block = Blockly.Workspace.getById(workspaceId)
.getBlockById(e.blockId);
var type = block.type;
if (type != 'procedures_defnoreturn' && type != 'procedures_defreturn') {
return;
}
var workspace = block.mutator.getWorkspace();
Blockly.Procedures.updateMutatorFlyout_(workspace);
workspace.addChangeListener(Blockly.Procedures.mutatorChangeListener_);
};
/**
* Listens for changes in a procedure mutator and triggers flyout updates when
* necessary.
* @param {!Blockly.Events.Abstract} e The event that triggered this listener.
* @private
*/
Blockly.Procedures.mutatorChangeListener_ = function(e) {
if (e.type != Blockly.Events.BLOCK_CREATE &&
e.type != Blockly.Events.BLOCK_DELETE &&
e.type != Blockly.Events.BLOCK_CHANGE) {
return;
}
var workspaceId = /** @type {string} */ (e.workspaceId);
var workspace = /** @type {!Blockly.WorkspaceSvg} */
(Blockly.Workspace.getById(workspaceId));
Blockly.Procedures.updateMutatorFlyout_(workspace);
};
/**
* Find all the callers of a named procedure.
* @param {string} name Name of procedure.

View File

@@ -384,6 +384,21 @@ Blockly.VariableMap.prototype.getAllVariables = function() {
return all_variables;
};
/**
* Returns all of the variable names of all types.
* @return {!Array<string>} All of the variable names of all types.
*/
Blockly.VariableMap.prototype.getAllVariableNames = function() {
var allNames = [];
for (var key in this.variableMap_) {
var variables = this.variableMap_[key];
for (var i = 0, variable; (variable = variables[i]); i++) {
allNames.push(variable.name);
}
}
return allNames;
};
/**
* Find all the uses of a named variable.
* @param {string} id ID of the variable to find.

View File

@@ -206,6 +206,8 @@ Blockly.Variables.flyoutCategoryBlocks = function(workspace) {
return xmlList;
};
Blockly.Variables.VAR_LETTER_OPTIONS = 'ijkmnopqrstuvwxyzabcdefgh'; // No 'l'.
/**
* Return a new variable name that is not yet being used. This will try to
* generate single letter variable names in the range 'i' to 'z' to start with.
@@ -215,44 +217,51 @@ Blockly.Variables.flyoutCategoryBlocks = function(workspace) {
* @return {string} New variable name.
*/
Blockly.Variables.generateUniqueName = function(workspace) {
var variableList = workspace.getAllVariables();
var newName = '';
if (variableList.length) {
var nameSuffix = 1;
var letters = 'ijkmnopqrstuvwxyzabcdefgh'; // No 'l'.
var letterIndex = 0;
var potName = letters.charAt(letterIndex);
while (!newName) {
var inUse = false;
for (var i = 0; i < variableList.length; i++) {
if (variableList[i].name.toLowerCase() == potName) {
// This potential name is already used.
inUse = true;
break;
}
}
if (inUse) {
// Try the next potential name.
letterIndex++;
if (letterIndex == letters.length) {
// Reached the end of the character sequence so back to 'i'.
// a new suffix.
letterIndex = 0;
nameSuffix++;
}
potName = letters.charAt(letterIndex);
if (nameSuffix > 1) {
potName += nameSuffix;
}
} else {
// We can use the current potential name.
newName = potName;
return Blockly.Variables.generateUniqueNameFromOptions(
Blockly.Variables.VAR_LETTER_OPTIONS.charAt(0),
workspace.getAllVariableNames()
);
};
/**
* Returns a unique name that is not present in the usedNames array. This
* will try to generate single letter names in the range a -> z (skip l). It
* will start with the character passed to startChar.
* @param {string} startChar The character to start the search at.
* @param {!Array<string>} usedNames A list of all of the used names.
* @return {string} A unique name that is not present in the usedNames array.
*/
Blockly.Variables.generateUniqueNameFromOptions = function(startChar, usedNames) {
if (!usedNames.length) {
return startChar;
}
var letters = Blockly.Variables.VAR_LETTER_OPTIONS;
var suffix = '';
var letterIndex = letters.indexOf(startChar);
var potName = startChar;
// eslint-disable-next-line no-constant-condition
while (true) {
var inUse = false;
for (var i = 0; i < usedNames.length; i++) {
if (usedNames[i].toLowerCase() == potName) {
inUse = true;
break;
}
}
} else {
newName = 'i';
if (!inUse) {
return potName;
}
letterIndex++;
if (letterIndex == letters.length) {
// Reached the end of the character sequence so back to 'i'.
letterIndex = 0;
suffix = Number(suffix) + 1;
}
potName = letters.charAt(letterIndex) + suffix;
}
return newName;
};
/**

View File

@@ -492,6 +492,14 @@ Blockly.Workspace.prototype.getAllVariables = function() {
return this.variableMap_.getAllVariables();
};
/**
* Returns all variable names of all types.
* @return {!Array<string>} List of all variable names of all types.
*/
Blockly.Workspace.prototype.getAllVariableNames = function() {
return this.variableMap_.getAllVariableNames();
};
/* End functions that are just pass-throughs to the variable map. */
/**

View File

@@ -127,6 +127,7 @@ Blockly.WorkspaceSvg = function(options,
if (Blockly.Procedures && Blockly.Procedures.flyoutCategory) {
this.registerToolboxCategoryCallback(Blockly.PROCEDURE_CATEGORY_NAME,
Blockly.Procedures.flyoutCategory);
this.addChangeListener(Blockly.Procedures.mutatorOpenListener);
}
/**

View File

@@ -398,24 +398,6 @@ suite('Procedures', function() {
clearVariables.call(this);
}, 'name');
});
// TODO: Reenable the following two once arg name validation has been
// moved to the arg blocks.
test.skip('Add Identical Arg', function() {
this.callForAllTypes(function() {
var args = ['x', 'x'];
createMutator.call(this, args);
assertArgs.call(this, ['x', 'i']);
clearVariables.call(this);
});
});
test.skip('Add Identical (except case) Arg', function() {
this.callForAllTypes(function() {
var args = ['x', 'X'];
createMutator.call(this, args);
assertArgs.call(this, ['x', 'i']);
clearVariables.call(this);
});
});
test('Multiple Args', function() {
this.callForAllTypes(function() {
var args = ['arg1', 'arg2', 'arg3'];