From a5a3b045d32681d448d4eaca982fb5e3de164f44 Mon Sep 17 00:00:00 2001 From: Sam El-Husseini Date: Thu, 14 May 2020 17:06:35 -0700 Subject: [PATCH] Use the Blockly AST for block toString (#3895) * Use the AST tree to populate block toString, add paranthesis around Number and Boolean connections --- core/block.js | 96 +++++++++++++++++++++++++---- tests/mocha/block_test.js | 112 ++++++++++++++++++++++++++++++++++ tests/mocha/index.html | 1 + tests/mocha/workspace_test.js | 6 +- 4 files changed, 200 insertions(+), 15 deletions(-) diff --git a/core/block.js b/core/block.js index 1bbd06dbe..6c36c989b 100644 --- a/core/block.js +++ b/core/block.js @@ -12,6 +12,7 @@ goog.provide('Blockly.Block'); +goog.require('Blockly.ASTNode'); goog.require('Blockly.Blocks'); goog.require('Blockly.Connection'); goog.require('Blockly.Events'); @@ -1306,23 +1307,94 @@ Blockly.Block.prototype.setCollapsed = function(collapsed) { Blockly.Block.prototype.toString = function(opt_maxLength, opt_emptyToken) { var text = []; var emptyFieldPlaceholder = opt_emptyToken || '?'; - for (var i = 0, input; (input = this.inputList[i]); i++) { - if (input.name == Blockly.Block.COLLAPSED_INPUT_NAME) { - continue; + + // Temporarily set flag to navigate to all fields. + var prevNavigateFields = Blockly.ASTNode.NAVIGATE_ALL_FIELDS; + Blockly.ASTNode.NAVIGATE_ALL_FIELDS = true; + + var node = Blockly.ASTNode.createBlockNode(this); + var rootNode = node; + + /** + * Whether or not to add parentheses around an input. + * @param {!Blockly.Connection} connection The connection. + * @return {boolean} True if we should add parentheses around the input. + */ + function shouldAddParentheses(connection) { + var checks = connection.getCheck(); + if (!checks && connection.targetConnection) { + checks = connection.targetConnection.getCheck(); } - for (var j = 0, field; (field = input.fieldRow[j]); j++) { - text.push(field.getText()); + return !!checks && (checks.indexOf('Boolean') != -1 || + checks.indexOf('Number') != -1); + } + + /** + * Check that we haven't circled back to the original root node. + */ + function checkRoot() { + if (node && node.getType() == rootNode.getType() && + node.getLocation() == rootNode.getLocation()) { + node = null; } - if (input.connection) { - var child = input.connection.targetBlock(); - if (child) { - text.push(child.toString(undefined, opt_emptyToken)); - } else { - text.push(emptyFieldPlaceholder); + } + + // Traverse the AST building up our text string. + while (node) { + switch (node.getType()) { + case Blockly.ASTNode.types.INPUT: + var connection = /** @type {!Blockly.Connection} */ (node.getLocation()); + if (!node.in()) { + text.push(emptyFieldPlaceholder); + } else if (shouldAddParentheses(connection)) { + text.push('('); + } + break; + case Blockly.ASTNode.types.FIELD: + var field = /** @type {Blockly.Field} */ (node.getLocation()); + if (field.name != Blockly.Block.COLLAPSED_FIELD_NAME) { + text.push(field.getText()); + } + break; + } + + var current = node; + node = current.in() || current.next(); + if (!node) { + // Can't go in or next, keep going out until we can go next. + node = current.out(); + checkRoot(); + while (node && !node.next()) { + node = node.out(); + checkRoot(); + // If we hit an input on the way up, possibly close out parentheses. + if (node && node.getType() == Blockly.ASTNode.types.INPUT && + shouldAddParentheses( + /** @type {!Blockly.Connection} */ (node.getLocation()))) { + text.push(')'); + } + } + if (node) { + node = node.next(); } } } - text = text.join(' ').trim() || '???'; + + // Restore state of NAVIGATE_ALL_FIELDS. + Blockly.ASTNode.NAVIGATE_ALL_FIELDS = prevNavigateFields; + + // Run through our text array and simplify expression to remove parentheses + // around single field blocks. + for (var i = 2, l = text.length; i < l; i++) { + if (text[i - 2] == '(' && text[i] == ')') { + text[i - 2] = text[i - 1]; + text.splice(i - 1, 2); + l -= 2; + } + } + + // Join the text array, removing spaces around added paranthesis. + text = text.join(' ').replace(/(\() | (\))/gmi, '$1$2').trim() || '???'; if (opt_maxLength) { // TODO: Improve truncation so that text from this block is given priority. // E.g. "1+2+3+4+5+6+7+8+9=0" should be "...6+7+8+9=0", not "1+2+3+4+5...". diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index 58e20aa69..08f11a84b 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -1680,4 +1680,116 @@ suite('Blocks', function() { }); }); }); + suite('toString', function() { + var toStringTests = [ + { + name: 'statement block', + xml: '' + + '' + + '' + + '10' + + '' + + '' + + '', + toString: 'repeat 10 times do ?', + }, + { + name: 'nested statement blocks', + xml: '' + + '' + + '' + + '10' + + '' + + '' + + '' + + '' + + '' + + '', + toString: 'repeat 10 times do if ? do ?', + }, + { + name: 'nested Boolean output blocks', + xml: '' + + '' + + '' + + 'EQ' + + '' + + '' + + 'AND' + + '' + + '' + + '' + + '' + + '', + toString: 'if ((? and ?) = ?) do ?', + }, + { + name: 'output block', + xml: '' + + 'ROOT' + + '' + + '' + + '9' + + '' + + '' + + '', + toString: 'square root 9', + }, + { + name: 'nested Number output blocks', + xml: '' + + 'ADD' + + '' + + '' + + '1' + + '' + + '' + + 'MULTIPLY' + + '' + + '' + + '10' + + '' + + '' + + '' + + '' + + '5' + + '' + + '' + + '' + + '' + + '' + + '' + + '3' + + '' + + '' + + '', + toString: '(10 × 5) + 3', + }, + { + name: 'nested String output blocks', + xml: '' + + '' + + '' + + '' + + 'Hello' + + '' + + '' + + '' + + '' + + 'World' + + '' + + '' + + '', + toString: 'create text with “ Hello ” “ World ”', + }, + ]; + // Create mocha test cases for each toString test. + toStringTests.forEach(function(t) { + test(t.name, function() { + var block = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(t.xml), + this.workspace); + chai.assert.equal(block.toString(), t.toString); + }); + }); + }); }); diff --git a/tests/mocha/index.html b/tests/mocha/index.html index 078dc71d7..3e238977d 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -12,6 +12,7 @@ + diff --git a/tests/mocha/workspace_test.js b/tests/mocha/workspace_test.js index 9693922ae..db6a2e280 100644 --- a/tests/mocha/workspace_test.js +++ b/tests/mocha/workspace_test.js @@ -669,15 +669,15 @@ function testAWorkspace() { test('After dispose', function() { this.blockA.dispose(); - chai.assert.isNull(this.workspace.getBlockById(this.blockA)); + chai.assert.isNull(this.workspace.getBlockById(this.blockA.id)); chai.assert.equal( this.workspace.getBlockById(this.blockB.id), this.blockB); }); test('After clear', function() { this.workspace.clear(); - chai.assert.isNull(this.workspace.getBlockById(this.blockA)); - chai.assert.isNull(this.workspace.getBlockById(this.blockB)); + chai.assert.isNull(this.workspace.getBlockById(this.blockA.id)); + chai.assert.isNull(this.workspace.getBlockById(this.blockB.id)); }); });