Fix events for variable renaming

This commit is contained in:
Rachel Fenichel
2017-12-06 14:41:56 -08:00
parent 90e79c6ba9
commit 1506a36b58
4 changed files with 108 additions and 67 deletions

View File

@@ -709,6 +709,7 @@ Blockly.Block.prototype.getVarModels = function() {
/**
* Notification that a variable is renaming.
* TODO (fenichel): consider deleting this.
* If the name matches one of this block's variables, rename it.
* @param {string} oldName Previous name of variable.
* @param {string} newName Renamed variable.
@@ -724,6 +725,23 @@ Blockly.Block.prototype.renameVar = function(oldName, newName) {
}
};
/**
* Notification that a variable is renaming but keeping the same ID. If the
* variable is in use on this block, rerender to show the new name.
* @param {!Blockly.VariableModel} variable The variable being renamed.
* @public
*/
Blockly.Block.prototype.updateVarName = function(variable) {
for (var i = 0, input; input = this.inputList[i]; i++) {
for (var j = 0, field; field = input.fieldRow[j]; j++) {
if (field instanceof Blockly.FieldVariable &&
variable.getId() == field.getValue()) {
field.setText(variable.name);
}
}
}
};
/**
* Notification that a variable is renaming.
* If the name matches one of this block's variables, rename it.

View File

@@ -60,45 +60,80 @@ Blockly.VariableMap.prototype.clear = function() {
/**
* Rename the given variable by updating its name in the variable map.
* @param {Blockly.VariableModel} variable Variable to rename.
* @param {!Blockly.VariableModel} variable Variable to rename.
* @param {string} newName New variable name.
* @package
*/
Blockly.VariableMap.prototype.renameVariable = function(variable, newName) {
var type = variable.type;
var newVariable = this.getVariable(newName, type);
var variableIndex = -1;
var newVariableIndex = -1;
var conflictVar = this.getVariable(newName, type);
// The IDs may match if the rename is a simple case change (name1 -> Name1).
if (!conflictVar || conflictVar.getId() == variable.getId()) {
this.renameVariableNoConflict_(variable, newName);
} else {
this.renameVariableWithConflict_(variable, newName, conflictVar);
}
};
/**
* Update the name of the given variable and refresh all references to it.
* The new name must not conflict with any existing variable names.
* @param {!Blockly.VariableModel} variable Variable to rename.
* @param {string} newName New variable name.
* @private
*/
Blockly.VariableMap.prototype.renameVariableNoConflict_ = function(variable, newName) {
Blockly.Events.fire(new Blockly.Events.VarRename(variable, newName));
variable.name = newName;
var blocks = this.workspace.getAllBlocks();
for (var i = 0; i < blocks.length; i++) {
blocks[i].updateVarName(variable);
}
};
/**
* Update the name of the given variable to the same name as an existing
* variable. The two variables are coalesced into a single variable with the ID
* of the existing variable that was already using newName.
* Refresh all references to the variable.
* @param {!Blockly.VariableModel} variable Variable to rename.
* @param {string} newName New variable name.
* @param {!Blockly.VariableModel} conflictVar The variable that was already
* using newName.
* @private
*/
Blockly.VariableMap.prototype.renameVariableWithConflict_ = function(variable,
newName, conflictVar) {
var type = variable.type;
Blockly.Events.setGroup(true);
Blockly.Events.fire(new Blockly.Events.VarRename(conflictVar, newName));
var oldCase = conflictVar.name;
conflictVar.name = newName;
// These blocks refer to the same variable but the case may have changed.
// No change events should be fired here.
var blocks = this.workspace.getAllBlocks();
if (newName != oldCase) {
for (var i = 0; i < blocks.length; i++) {
blocks[i].updateVarName(conflictVar);
}
}
// These blocks now refer to a different variable.
// These will fire change events.
for (var i = 0; i < blocks.length; i++) {
blocks[i].renameVarById(variable.getId(), conflictVar.getId());
}
// Finally delete the original variable, which is now unreferenced.
Blockly.Events.fire(new Blockly.Events.VarDelete(variable));
// And remove it from the list.
var variableList = this.getVariablesOfType(type);
if (variable) {
variableIndex = variableList.indexOf(variable);
}
if (newVariable) { // see if I can get rid of newVariable dependency
newVariableIndex = variableList.indexOf(newVariable);
}
var variableIndex = variableList.indexOf(variable);
this.variableMap_[type].splice(variableIndex, 1);
if (variableIndex == -1 && newVariableIndex == -1) {
this.createVariable(newName, '');
console.log('Tried to rename an non-existent variable.');
} else if (variableIndex == newVariableIndex ||
variableIndex != -1 && newVariableIndex == -1) {
// Only changing case, or renaming to a completely novel name.
var variableToRename = this.variableMap_[type][variableIndex];
Blockly.Events.fire(new Blockly.Events.VarRename(variableToRename,
newName));
variableToRename.name = newName;
} else if (variableIndex != -1 && newVariableIndex != -1) {
// Renaming one existing variable to another existing variable.
// The case might have changed, so we update the destination ID.
var variableToRename = this.variableMap_[type][newVariableIndex];
Blockly.Events.fire(new Blockly.Events.VarRename(variableToRename,
newName));
var variableToDelete = this.variableMap_[type][variableIndex];
Blockly.Events.fire(new Blockly.Events.VarDelete(variableToDelete));
variableToRename.name = newName;
this.variableMap_[type].splice(variableIndex, 1);
}
Blockly.Events.setGroup(false);
};
/**

View File

@@ -224,35 +224,7 @@ Blockly.Workspace.prototype.renameVariableById = function(id, newName) {
throw new Error('Tried to rename a variable that didn\'t exist. ID: ' + id);
}
var type = variable.type;
var newVariable = this.getVariable(newName, type);
// If a variable previously existed with the new name, we will coalesce the
// variables and use the ID of the existing variable with the new name.
// Otherwise, we change the current variable's name but not its ID.
var oldId = variable.getId();
var newId = newVariable ? newVariable.getId() : oldId;
var oldCase;
// Find if newVariable case is different.
if (newVariable && newVariable.name != newName) {
oldCase = newVariable.name;
}
Blockly.Events.setGroup(true);
var blocks = this.getAllBlocks();
// Iterate through every block and update name.
for (var i = 0; i < blocks.length; i++) {
blocks[i].renameVarById(oldId, newId);
if (oldCase) {
blocks[i].renameVarById(newId, newId);
}
}
this.variableMap_.renameVariable(variable, newName);
Blockly.Events.setGroup(false);
};
/**
@@ -569,6 +541,15 @@ Blockly.Workspace.prototype.getPotentialVariableMap = function() {
return this.potentialVariableMap_;
};
/**
* Return the map of all variables on the workspace.
* @return {?Blockly.VariableMap} The variable map.
* @package
*/
Blockly.Workspace.prototype.getVariableMap = function() {
return this.variableMap_;
};
/**
* Database of all workspaces.
* @private

View File

@@ -106,7 +106,7 @@ Blockly.Xml.fieldToDomVariable_ = function(field, workspace) {
return container;
} else {
// something went wrong?
console.log('no variable in fieldtodom');
console.warn('no variable in fieldtodom');
return null;
}
};
@@ -500,6 +500,7 @@ Blockly.Xml.domToBlock = function(xmlBlock, workspace) {
try {
var topBlock = Blockly.Xml.domToBlockHeadless_(xmlBlock, workspace);
if (workspace.rendered) {
// TODO (fenichel): Otherwise call initModel?
// Hide connections to speed up assembly.
topBlock.setConnectionsHidden(true);
// Generate list of all blocks.
@@ -719,19 +720,25 @@ Blockly.Xml.domToBlockHeadless_ = function(xmlBlock, workspace) {
return block;
};
/**
* Decode an XML variable field tag and set the value of that field.
* @param {!Blockly.Workspace} workspace The workspace that is currently being
* deserialized.
* @param {!Element} xml The field tag to decode.
* @param {string} text The text content of the XML tag.
* @param {!Blockly.FieldVariable} field The field on which the value will be
* set.
* @private
*/
Blockly.Xml.domToFieldVariable_ = function(workspace, xml, text, field) {
// TODO (#1199): When we change setValue and getValue to
// interact with IDs instead of names, update this so that we get
// the variable based on ID instead of textContent.
var type = xml.getAttribute('variabletype') || '';
// TODO (fenichel): Does this need to be explicit or not?
if (type == '\'\'') {
type = '';
}
// TODO: Consider using a different name (var_id?) because this is the
// node's ID.
var id = xml.id;
var variable =
Blockly.Variables.getOrCreateVariable(workspace, id, text, type);
Blockly.Variables.getOrCreateVariable(workspace, xml.id, text, type);
// This should never happen :)
if (type != null && type !== variable.type) {