Merge pull request #2615 from BeksOmega/fixes/Procedures

Fixed Procedure Empty Names & Procedure Parameters
This commit is contained in:
Rachel Fenichel
2019-07-23 11:21:22 -07:00
committed by GitHub
5 changed files with 31 additions and 23 deletions

View File

@@ -349,7 +349,7 @@ Blockly.Blocks['procedures_defnoreturn'] = {
displayRenamedVar_: function(oldName, newName) {
this.updateParams_();
// Update the mutator's variables if the mutator is open.
if (this.mutator.isVisible()) {
if (this.mutator && this.mutator.isVisible()) {
var blocks = this.mutator.workspace_.getAllBlocks(false);
for (var i = 0, block; block = blocks[i]; i++) {
if (block.type == 'procedures_mutatorarg' &&
@@ -588,14 +588,16 @@ Blockly.Blocks['procedures_mutatorarg'] = {
* @this Blockly.FieldTextInput
*/
validator_: function(varName) {
var outerWs = Blockly.Mutator.findParentWs(this.getSourceBlock().workspace);
var sourceBlock = this.getSourceBlock();
var outerWs = Blockly.Mutator.findParentWs(sourceBlock.workspace);
varName = varName.replace(/[\s\xa0]+/g, ' ').replace(/^ | $/g, '');
if (!varName) {
return null;
}
// Prevents duplicate parameter names in functions
var blocks = this.getSourceBlock().workspace.getAllBlocks();
for (var i = 0; i < blocks.length; i += 1) {
var blocks = sourceBlock.workspace.getAllBlocks();
for (var i = 0; i < blocks.length; i++) {
if (blocks[i].id == this.getSourceBlock().id) {
continue;
}
@@ -603,11 +605,11 @@ Blockly.Blocks['procedures_mutatorarg'] = {
return null;
}
}
var model = outerWs.getVariable(varName, '');
if (model && model.name != varName) {
// Rename the variable (case change)
// TODO: This function doesn't exist on the workspace.
outerWs.renameVarById(model.getId(), varName);
outerWs.renameVariableById(model.getId(), varName);
}
if (!model) {
model = outerWs.createVariable(varName, '');

View File

@@ -172,7 +172,7 @@ Blockly.Names.prototype.getDistinctName = function(name, type) {
*/
Blockly.Names.prototype.safeName_ = function(name) {
if (!name) {
name = 'unnamed';
name = Blockly.Msg['UNNAMED_KEY'] || 'unnamed';
} else {
// Unfortunately names in non-latin characters will look like
// _E9_9F_B3_E4_B9_90 which is pretty meaningless.

View File

@@ -92,6 +92,8 @@ Blockly.Procedures.procTupleComparator_ = function(ta, tb) {
/**
* Ensure two identically-named procedures don't exist.
* Take the proposed procedure name, and return a legal name i.e. one that
* is not empty and doesn't collide with other procedures.
* @param {string} name Proposed procedure name.
* @param {!Blockly.Block} block Block to disambiguate.
* @return {string} Non-colliding name.
@@ -101,6 +103,7 @@ Blockly.Procedures.findLegalName = function(name, block) {
// Flyouts can have multiple procedures called 'do something'.
return name;
}
name = name || Blockly.Msg['UNNAMED_KEY'] || 'unnamed';
while (!Blockly.Procedures.isLegalName_(name, block.workspace, block)) {
// Collision with another procedure.
var r = name.match(/^(.*?)(\d+)$/);
@@ -162,7 +165,6 @@ Blockly.Procedures.rename = function(name) {
// Strip leading and trailing whitespace. Beyond this, all names are legal.
name = name.trim();
// Ensure two identically-named procedures don't exist.
var legalName = Blockly.Procedures.findLegalName(name, this.getSourceBlock());
var oldName = this.getValue();
if (oldName != name && oldName != legalName) {

View File

@@ -71,6 +71,9 @@ Blockly.Msg.PROCEDURES_HUE = '290';
/// For more context, see
/// [[Translating:Blockly#infrequent_message_types]].\n{{Identical|Item}}
Blockly.Msg.VARIABLES_DEFAULT_NAME = 'item';
/// default name - A simple, default name for an unnamed function or
// variable. Preferably indicates that the key is unnamed.
Blockly.Msg.UNNAMED_KEY = 'unnamed';
/// button text - Button that sets a calendar to today's date.\n{{Identical|Today}}
Blockly.Msg.TODAY = 'Today';

View File

@@ -141,9 +141,7 @@ suite('Procedures', function() {
chai.assert.equal(this.callBlock.getFieldValue('NAME'), 'start name');
}, 'start name');
});
// TODO: Simple fix, oldText needs to be trimmed too, or we just discard
// the checking (params don't check).
test.skip('Whitespace then Text', function() {
test('Whitespace then Text', function() {
this.callForAllTypes(function() {
var defInput = this.defBlock.getField('NAME');
defInput.htmlInput_ = Object.create(null);
@@ -167,16 +165,15 @@ suite('Procedures', function() {
defInput.htmlInput_.value = '';
defInput.onHtmlInputChange_(null);
chai.assert.equal(this.defBlock.getFieldValue('NAME'), '');
chai.assert.equal(this.callBlock.getFieldValue('NAME'), '');
chai.assert.equal(
this.defBlock.getFieldValue('NAME'),
Blockly.Msg['UNNAMED_KEY']);
chai.assert.equal(
this.callBlock.getFieldValue('NAME'),
Blockly.Msg['UNNAMED_KEY']);
}, 'start name');
});
// TODO: Procedures do not handle having no name correctly. It is a
// problem with newly created procedure blocks having '' as the
// oldName, just like empty procedures. The renameProcedure function
// gets confused when a procedure's name is initially set. This is
// basically #503 again.
test.skip('Set Empty, and Create New', function() {
test('Set Empty, and Create New', function() {
this.callForAllTypes(function() {
var defInput = this.defBlock.getField('NAME');
defInput.htmlInput_ = Object.create(null);
@@ -187,8 +184,12 @@ suite('Procedures', function() {
defInput.onHtmlInputChange_(null);
var newDefBlock = new Blockly.Block(this.workspace, this.defType);
newDefBlock.setFieldValue('new name', 'NAME');
chai.assert.equal(this.defBlock.getFieldValue('NAME'), '');
chai.assert.equal(this.callBlock.getFieldValue('NAME'), '');
chai.assert.equal(
this.defBlock.getFieldValue('NAME'),
Blockly.Msg['UNNAMED_KEY']);
chai.assert.equal(
this.callBlock.getFieldValue('NAME'),
Blockly.Msg['UNNAMED_KEY']);
newDefBlock.dispose();
}, 'start name');
@@ -376,7 +377,7 @@ suite('Procedures', function() {
clearVariables.call(this);
}, 'name');
});
test.skip('lower -> CAPS', function() {
test('lower -> CAPS', function() {
this.callForAllTypes(function() {
createMutator.call(this, ['arg']);
this.argBlock.setFieldValue('ARG', 'NAME');
@@ -385,7 +386,7 @@ suite('Procedures', function() {
clearVariables.call(this);
}, 'name');
});
test.skip('CAPS -> lower', function() {
test('CAPS -> lower', function() {
this.callForAllTypes(function() {
createMutator.call(this, ['ARG']);
this.argBlock.setFieldValue('arg', 'NAME');