From f3a4664dea4e03d09bc1d818e489f42b52cfd7f6 Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Thu, 15 Feb 2018 17:18:23 -0800 Subject: [PATCH 1/2] Delete extra variables at the end of a procedure argument edit --- blocks/procedures.js | 70 ++++++++++++++++++++++++++++++-------------- core/mutator.js | 24 +++++++++++++++ 2 files changed, 72 insertions(+), 22 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index 8e890a767..c69b38d3c 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -217,8 +217,7 @@ Blockly.Blocks['procedures_defnoreturn'] = { while (paramBlock) { var varName = paramBlock.getFieldValue('NAME'); this.arguments_.push(varName); - var variable = Blockly.Variables.getOrCreateVariablePackage( - this.workspace, null, varName, ''); + var variable = this.workspace.getVariable(varName, ''); this.argumentVarModels_.push(variable); this.paramIds_.push(paramBlock.id); paramBlock = paramBlock.nextConnection && @@ -469,6 +468,15 @@ Blockly.Blocks['procedures_mutatorarg'] = { */ init: function() { var field = new Blockly.FieldTextInput('x', 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. + var oldShowEditorFn = field.showEditor_.bind(field); + var newShowEditorFn = function() { + this.createdVariables_ = []; + oldShowEditorFn(); + }; + field.showEditor_ = newShowEditorFn.bind(field); + this.appendDummyInput() .appendField(Blockly.Msg.PROCEDURES_MUTATORARG_TITLE) .appendField(field, 'NAME'); @@ -478,43 +486,61 @@ Blockly.Blocks['procedures_mutatorarg'] = { this.setTooltip(Blockly.Msg.PROCEDURES_MUTATORARG_TOOLTIP); this.contextMenu = false; + // Get a handle on the parent workspace. + // TODO: Do I need to delete this during dispose? + this.outerWs_ = Blockly.Mutator.findParentWs(this.workspace); // Create the default variable when we drag the block in from the flyout. // Have to do this after installing the field on the block. - field.onFinishEditing_ = this.createNewVar_; + field.onFinishEditing_ = this.deleteIntermediateVars_; + // Create an empty list so onFinishEditing_ has something to look at, even + // though the editor was never opened. + field.createdVariables_ = []; field.onFinishEditing_('x'); }, /** - * Obtain a valid name for the procedure. + * Obtain a valid name for the procedure argument. Create a variable if + * necessary. * Merge runs of whitespace. Strip leading and trailing whitespace. * Beyond this, all names are legal. - * @param {string} newVar User-supplied name. + * @param {string} varName User-supplied name. * @return {?string} Valid name, or null if a name was not specified. * @private - * @this Blockly.Block + * @this Blockly.FieldTextInput */ - validator_: function(newVar) { - newVar = newVar.replace(/[\s\xa0]+/g, ' ').replace(/^ | $/g, ''); - return newVar || null; + validator_: function(varName) { + varName = varName.replace(/[\s\xa0]+/g, ' ').replace(/^ | $/g, ''); + if (!varName) { + return null; + } + var model = this.sourceBlock_.outerWs_.getVariable(varName, ''); + if (model && model.name != varName) { + // Rename the variable (case change) + this.outerWs_.renameVarById(model.getId(), varName); + } + if (!model) { + model = this.sourceBlock_.outerWs_.createVariable(varName, ''); + if (model && this.createdVariables_) { + this.createdVariables_.push(model); + } + } + return varName; }, /** * Called when focusing away from the text field. - * Creates a new variable with this name. + * Deletes all variables that were created as the user typed their intended + * variable name. * @param {string} newText The new variable name. * @private * @this Blockly.FieldTextInput */ - createNewVar_: function(newText) { - var source = this.sourceBlock_; - if (source && source.workspace && source.workspace.options && - source.workspace.options.parentWorkspace) { - var workspace = source.workspace.options.parentWorkspace; - var variableType = ''; - var variable = workspace.getVariable(newText, variableType); - // If there is a case change, rename the variable. - if (variable && variable.name !== newText) { - workspace.renameVariableById(variable.getId(), newText); - } else { - workspace.createVariable(newText, variableType); + deleteIntermediateVars_: function(newText) { + if (!this.sourceBlock_.outerWs_) { + return; + } + for (var i = 0; i < this.createdVariables_.length; i++) { + var model = this.createdVariables_[i]; + if (model.name != newText) { + this.sourceBlock_.outerWs_.deleteVariableById(model.getId()); } } } diff --git a/core/mutator.js b/core/mutator.js index 1446c48ab..a57727ae2 100644 --- a/core/mutator.js +++ b/core/mutator.js @@ -415,6 +415,30 @@ Blockly.Mutator.reconnect = function(connectionChild, block, inputName) { return false; }; +/** + * Get the parent workspace of a workspace that is inside a mutator, taking into + * account whether it is a flyout. + * @param {?Blockly.Workspace} workspace The workspace that is inside a mutator. + * @return {?Blockly.Workspace} The mutator's parent workspace or null. + * @package + */ +Blockly.Mutator.findParentWs = function(workspace) { + var outerWs = null; + if (workspace && workspace.options) { + var parent = workspace.options.parentWorkspace; + // If we were in a flyout in a mutator, need to go up two levels to find + // the actual parent. + if (workspace.isFlyout) { + if (parent && parent.options) { + outerWs = parent.options.parentWorkspace; + } + } else if (parent) { + outerWs = parent; + } + } + return outerWs; +}; + // Export symbols that would otherwise be renamed by Closure compiler. if (!goog.global['Blockly']) { goog.global['Blockly'] = {}; From af59e27d960fa08783956b8c1f7e26d6395a5d9b Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Fri, 16 Feb 2018 11:02:03 -0800 Subject: [PATCH 2/2] Cleanup --- blocks/procedures.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index c69b38d3c..1874ee1d0 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -470,12 +470,12 @@ Blockly.Blocks['procedures_mutatorarg'] = { var field = new Blockly.FieldTextInput('x', 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. - var oldShowEditorFn = field.showEditor_.bind(field); + field.oldShowEditorFn_ = field.showEditor_; var newShowEditorFn = function() { this.createdVariables_ = []; - oldShowEditorFn(); + this.oldShowEditorFn_(); }; - field.showEditor_ = newShowEditorFn.bind(field); + field.showEditor_ = newShowEditorFn; this.appendDummyInput() .appendField(Blockly.Msg.PROCEDURES_MUTATORARG_TITLE) @@ -486,9 +486,6 @@ Blockly.Blocks['procedures_mutatorarg'] = { this.setTooltip(Blockly.Msg.PROCEDURES_MUTATORARG_TOOLTIP); this.contextMenu = false; - // Get a handle on the parent workspace. - // TODO: Do I need to delete this during dispose? - this.outerWs_ = Blockly.Mutator.findParentWs(this.workspace); // Create the default variable when we drag the block in from the flyout. // Have to do this after installing the field on the block. field.onFinishEditing_ = this.deleteIntermediateVars_; @@ -508,17 +505,18 @@ Blockly.Blocks['procedures_mutatorarg'] = { * @this Blockly.FieldTextInput */ validator_: function(varName) { + var outerWs = Blockly.Mutator.findParentWs(this.sourceBlock_.workspace); varName = varName.replace(/[\s\xa0]+/g, ' ').replace(/^ | $/g, ''); if (!varName) { return null; } - var model = this.sourceBlock_.outerWs_.getVariable(varName, ''); + var model = outerWs.getVariable(varName, ''); if (model && model.name != varName) { // Rename the variable (case change) - this.outerWs_.renameVarById(model.getId(), varName); + outerWs.renameVarById(model.getId(), varName); } if (!model) { - model = this.sourceBlock_.outerWs_.createVariable(varName, ''); + model = outerWs.createVariable(varName, ''); if (model && this.createdVariables_) { this.createdVariables_.push(model); } @@ -534,13 +532,14 @@ Blockly.Blocks['procedures_mutatorarg'] = { * @this Blockly.FieldTextInput */ deleteIntermediateVars_: function(newText) { - if (!this.sourceBlock_.outerWs_) { + var outerWs = Blockly.Mutator.findParentWs(this.sourceBlock_.workspace); + if (!outerWs) { return; } for (var i = 0; i < this.createdVariables_.length; i++) { var model = this.createdVariables_[i]; if (model.name != newText) { - this.sourceBlock_.outerWs_.deleteVariableById(model.getId()); + outerWs.deleteVariableById(model.getId()); } } }