Skip to content

Commit

Permalink
feat: Parse newlines in JSON message as row separators. (#6944)
Browse files Browse the repository at this point in the history
* feat: Parse message newlines as endOfRow dummies.

* Fix the multilineinput field test.

* Addressing PR feedback.

* Addressing PR feedback.

* Newline parsing now uses a new custom input.

* npm run format

* Added input_end_row to block factory.

* Addres feedback, fix endrow after external value.
  • Loading branch information
johnnesky authored Aug 11, 2023
1 parent 1a41891 commit f246adb
Show file tree
Hide file tree
Showing 17 changed files with 310 additions and 55 deletions.
67 changes: 50 additions & 17 deletions core/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import {Size} from './utils/size.js';
import type {VariableModel} from './variable_model.js';
import type {Workspace} from './workspace.js';
import {DummyInput} from './inputs/dummy_input.js';
import {EndRowInput} from './inputs/end_row_input.js';
import {ValueInput} from './inputs/value_input.js';
import {StatementInput} from './inputs/statement_input.js';
import {IconType} from './icons/icon_types.js';
Expand Down Expand Up @@ -1339,6 +1340,12 @@ export class Block implements IASTNodeLocation, IDeletable {
return true;
}
}
for (let i = 0; i < this.inputList.length; i++) {
if (this.inputList[i] instanceof EndRowInput) {
// A row-end input is present. Inline value inputs.
return true;
}
}
return false;
}

Expand Down Expand Up @@ -1560,6 +1567,17 @@ export class Block implements IASTNodeLocation, IDeletable {
return this.appendInput(new DummyInput(name, this));
}

/**
* Appends an input that ends the row.
*
* @param name Optional language-neutral identifier which may used to find
* this input again. Should be unique to this block.
* @returns The input object created.
*/
appendEndRowInput(name = ''): Input {
return this.appendInput(new EndRowInput(name, this));
}

/**
* Appends the given input row.
*
Expand Down Expand Up @@ -1628,7 +1646,8 @@ export class Block implements IASTNodeLocation, IDeletable {
this.interpolate_(
json['message' + i],
json['args' + i] || [],
json['lastDummyAlign' + i],
// Backwards compatibility: lastDummyAlign aliases implicitAlign.
json['implicitAlign' + i] || json['lastDummyAlign' + i],
warningPrefix,
);
i++;
Expand Down Expand Up @@ -1765,19 +1784,19 @@ export class Block implements IASTNodeLocation, IDeletable {
* @param message Text contains interpolation tokens (%1, %2, ...) that match
* with fields or inputs defined in the args array.
* @param args Array of arguments to be interpolated.
* @param lastDummyAlign If a dummy input is added at the end, how should it
* be aligned?
* @param implicitAlign If an implicit input is added at the end or in place
* of newline tokens, how should it be aligned?
* @param warningPrefix Warning prefix string identifying block.
*/
private interpolate_(
message: string,
args: AnyDuringMigration[],
lastDummyAlign: string | undefined,
implicitAlign: string | undefined,
warningPrefix: string,
) {
const tokens = parsing.tokenizeInterpolation(message);
this.validateTokens_(tokens, args.length);
const elements = this.interpolateArguments_(tokens, args, lastDummyAlign);
const elements = this.interpolateArguments_(tokens, args, implicitAlign);

// An array of [field, fieldName] tuples.
const fieldStack = [];
Expand Down Expand Up @@ -1855,19 +1874,20 @@ export class Block implements IASTNodeLocation, IDeletable {

/**
* Inserts args in place of numerical tokens. String args are converted to
* JSON that defines a label field. If necessary an extra dummy input is added
* to the end of the elements.
* JSON that defines a label field. Newline characters are converted to
* end-row inputs, and if necessary an extra dummy input is added to the end
* of the elements.
*
* @param tokens The tokens to interpolate
* @param args The arguments to insert.
* @param lastDummyAlign The alignment the added dummy input should have, if
* we are required to add one.
* @param implicitAlign The alignment to use for any implicitly added end-row
* or dummy inputs, if necessary.
* @returns The JSON definitions of field and inputs to add to the block.
*/
private interpolateArguments_(
tokens: Array<string | number>,
args: Array<AnyDuringMigration | string>,
lastDummyAlign: string | undefined,
implicitAlign: string | undefined,
): AnyDuringMigration[] {
const elements = [];
for (let i = 0; i < tokens.length; i++) {
Expand All @@ -1877,11 +1897,20 @@ export class Block implements IASTNodeLocation, IDeletable {
}
// Args can be strings, which is why this isn't elseif.
if (typeof element === 'string') {
// AnyDuringMigration because: Type '{ text: string; type: string; } |
// null' is not assignable to type 'string | number'.
element = this.stringToFieldJson_(element) as AnyDuringMigration;
if (!element) {
continue;
if (element === '\n') {
// Convert newline tokens to end-row inputs.
const newlineInput = {'type': 'input_end_row'};
if (implicitAlign) {
(newlineInput as AnyDuringMigration)['align'] = implicitAlign;
}
element = newlineInput as AnyDuringMigration;
} else {
// AnyDuringMigration because: Type '{ text: string; type: string; }
// | null' is not assignable to type 'string | number'.
element = this.stringToFieldJson_(element) as AnyDuringMigration;
if (!element) {
continue;
}
}
}
elements.push(element);
Expand All @@ -1895,8 +1924,8 @@ export class Block implements IASTNodeLocation, IDeletable {
)
) {
const dummyInput = {'type': 'input_dummy'};
if (lastDummyAlign) {
(dummyInput as AnyDuringMigration)['align'] = lastDummyAlign;
if (implicitAlign) {
(dummyInput as AnyDuringMigration)['align'] = implicitAlign;
}
elements.push(dummyInput);
}
Expand Down Expand Up @@ -1960,6 +1989,9 @@ export class Block implements IASTNodeLocation, IDeletable {
case 'input_dummy':
input = this.appendDummyInput(element['name']);
break;
case 'input_end_row':
input = this.appendEndRowInput(element['name']);
break;
default: {
input = this.appendInputFromRegistry(element['type'], element['name']);
break;
Expand Down Expand Up @@ -1998,6 +2030,7 @@ export class Block implements IASTNodeLocation, IDeletable {
str === 'input_value' ||
str === 'input_statement' ||
str === 'input_dummy' ||
str === 'input_end_row' ||
registry.hasItem(registry.Type.INPUT, str)
);
}
Expand Down
11 changes: 10 additions & 1 deletion core/inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,17 @@
import {Align} from './inputs/align.js';
import {Input} from './inputs/input.js';
import {DummyInput} from './inputs/dummy_input.js';
import {EndRowInput} from './inputs/end_row_input.js';
import {StatementInput} from './inputs/statement_input.js';
import {ValueInput} from './inputs/value_input.js';
import {inputTypes} from './inputs/input_types.js';

export {Align, Input, DummyInput, StatementInput, ValueInput, inputTypes};
export {
Align,
Input,
DummyInput,
EndRowInput,
StatementInput,
ValueInput,
inputTypes,
};
31 changes: 31 additions & 0 deletions core/inputs/end_row_input.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* @license
* Copyright 2023 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import type {Block} from '../block.js';
import {Input} from './input.js';
import {inputTypes} from './input_types.js';

/**
* Represents an input on a block that is always the last input in the row. Any
* following input will be rendered on the next row even if the block has inline
* inputs. Any newline character in a JSON block definition's message will
* automatically be parsed as an end-row input.
*/
export class EndRowInput extends Input {
readonly type = inputTypes.END_ROW;

/**
* @param name Language-neutral identifier which may used to find this input
* again.
* @param block The block containing this input.
*/
constructor(
public name: string,
block: Block,
) {
super(name, block);
}
}
4 changes: 4 additions & 0 deletions core/inputs/input_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,8 @@ export enum inputTypes {
DUMMY = 5,
// An unknown type of input defined by an external developer.
CUSTOM = 6,
// An input with no connections that is always the last input of a row. Any
// subsequent input will be rendered on the next row. Any newline character in
// a JSON block definition's message will be parsed as an end-row input.
END_ROW = 7,
}
21 changes: 16 additions & 5 deletions core/renderers/common/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {RenderedConnection} from '../../rendered_connection.js';
import type {Measurable} from '../measurables/base.js';
import {BottomRow} from '../measurables/bottom_row.js';
import {DummyInput} from '../../inputs/dummy_input.js';
import {EndRowInput} from '../../inputs/end_row_input.js';
import {ExternalValueInput} from '../measurables/external_value_input.js';
import {Field} from '../measurables/field.js';
import {Hat} from '../measurables/hat.js';
Expand Down Expand Up @@ -326,9 +327,9 @@ export class RenderInfo {
} else if (input instanceof ValueInput) {
activeRow.elements.push(new ExternalValueInput(this.constants_, input));
activeRow.hasExternalInput = true;
} else if (input instanceof DummyInput) {
// Dummy inputs have no visual representation, but the information is
// still important.
} else if (input instanceof DummyInput || input instanceof EndRowInput) {
// Dummy and end-row inputs have no visual representation, but the
// information is still important.
activeRow.minHeight = Math.max(
activeRow.minHeight,
input.getSourceBlock() && input.getSourceBlock()!.isShadow()
Expand All @@ -355,15 +356,25 @@ export class RenderInfo {
if (!lastInput) {
return false;
}
// If the previous input was an end-row input, then any following input
// should always be rendered on the next row.
if (lastInput instanceof EndRowInput) {
return true;
}
// A statement input or an input following one always gets a new row.
if (
input instanceof StatementInput ||
lastInput instanceof StatementInput
) {
return true;
}
// Value and dummy inputs get new row if inputs are not inlined.
if (input instanceof ValueInput || input instanceof DummyInput) {
// Value inputs, dummy inputs, and any input following an external value
// input get a new row if inputs are not inlined.
if (
input instanceof ValueInput ||
input instanceof DummyInput ||
lastInput instanceof ValueInput
) {
return !this.isInline;
}
return false;
Expand Down
13 changes: 9 additions & 4 deletions core/renderers/geras/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {RenderInfo as BaseRenderInfo} from '../common/info.js';
import type {Measurable} from '../measurables/base.js';
import type {BottomRow} from '../measurables/bottom_row.js';
import {DummyInput} from '../../inputs/dummy_input.js';
import {EndRowInput} from '../../inputs/end_row_input.js';
import {ExternalValueInput} from '../measurables/external_value_input.js';
import type {Field} from '../measurables/field.js';
import {InRowSpacer} from '../measurables/in_row_spacer.js';
Expand Down Expand Up @@ -90,9 +91,9 @@ export class RenderInfo extends BaseRenderInfo {
} else if (input instanceof ValueInput) {
activeRow.elements.push(new ExternalValueInput(this.constants_, input));
activeRow.hasExternalInput = true;
} else if (input instanceof DummyInput) {
// Dummy inputs have no visual representation, but the information is
// still important.
} else if (input instanceof DummyInput || input instanceof EndRowInput) {
// Dummy and end-row inputs have no visual representation, but the
// information is still important.
activeRow.minHeight = Math.max(
activeRow.minHeight,
this.constants_.DUMMY_INPUT_MIN_HEIGHT,
Expand Down Expand Up @@ -379,8 +380,12 @@ export class RenderInfo extends BaseRenderInfo {
row.width < prevInput.width
) {
rowNextRightEdges.set(row, prevInput.width);
} else {
} else if (row.hasStatement) {
nextRightEdge = row.width;
} else {
// To keep right edges of consecutive non-statement rows aligned, use
// the maximum width.
nextRightEdge = Math.max(nextRightEdge, row.width);
}
prevInput = row;
}
Expand Down
2 changes: 1 addition & 1 deletion core/renderers/measurables/row.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class Row {
hasInlineInput = false;

/**
* Whether the row has any dummy inputs.
* Whether the row has any dummy inputs or end-row inputs.
*/
hasDummyInput = false;

Expand Down
20 changes: 15 additions & 5 deletions core/renderers/zelos/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ goog.declareModuleId('Blockly.zelos.RenderInfo');

import type {BlockSvg} from '../../block_svg.js';
import {DummyInput} from '../../inputs/dummy_input.js';
import {EndRowInput} from '../../inputs/end_row_input.js';
import {FieldImage} from '../../field_image.js';
import {FieldLabel} from '../../field_label.js';
import {FieldTextInput} from '../../field_textinput.js';
Expand Down Expand Up @@ -124,15 +125,24 @@ export class RenderInfo extends BaseRenderInfo {
if (!lastInput) {
return false;
}
// If the previous input was an end-row input, then any following input
// should always be rendered on the next row.
if (lastInput instanceof EndRowInput) {
return true;
}
// A statement input or an input following one always gets a new row.
if (
input instanceof StatementInput ||
lastInput instanceof StatementInput
) {
return true;
}
// Value and dummy inputs get new row if inputs are not inlined.
if (input instanceof ValueInput || input instanceof DummyInput) {
// Value, dummy, and end-row inputs get new row if inputs are not inlined.
if (
input instanceof ValueInput ||
input instanceof DummyInput ||
input instanceof EndRowInput
) {
return !this.isInline || this.isMultiRow;
}
return false;
Expand Down Expand Up @@ -267,9 +277,9 @@ export class RenderInfo extends BaseRenderInfo {
override addInput_(input: Input, activeRow: Row) {
// If we have two dummy inputs on the same row, one aligned left and the
// other right, keep track of the right aligned dummy input so we can add
// padding later.
// padding later. An end-row input after a dummy input also counts.
if (
input instanceof DummyInput &&
(input instanceof DummyInput || input instanceof EndRowInput) &&
activeRow.hasDummyInput &&
activeRow.align === Align.LEFT &&
input.align === Align.RIGHT
Expand Down Expand Up @@ -502,7 +512,7 @@ export class RenderInfo extends BaseRenderInfo {
const connectionWidth = this.outputConnection.width;
const outerShape = this.outputConnection.shape.type;
const constants = this.constants_;
if (this.isMultiRow && this.inputRows.length > 1) {
if (this.inputRows.length > 1) {
switch (outerShape) {
case constants.SHAPES.ROUND: {
// Special case for multi-row round reporter blocks.
Expand Down
Loading

0 comments on commit f246adb

Please sign in to comment.