Skip to content

Commit

Permalink
Expandable blocks redo (#3835)
Browse files Browse the repository at this point in the history 
* New block string parser

* Refactoring compile info for blocks

* Fixing some bugs

* Fixing decompiler

* Add and remove individual parameters

* Decompiler support for optional arguments

* Adding more block stlye options

* Block text with custom CSS classes

* Hack to render inputs correctly

* Fix flickering of expandable blocks in toolbox

* Handle instance methods with optional args

* Adding test library to blockly compiler tests

* Adding compiler tests for expandable blocks

* Adding README to the karma tests

* Linting fixes

* Fixing rendering bugs and adding toggle mode

* Redoing the input breaks for blocks to match the old behavior

* Check both definition name and actual name for parameter options

* Updating tests

* Updating Blockly
  • Loading branch information
@riknoll @pelikhan
riknoll authored and pelikhan committed Feb 9, 2018
1 parent 633855e commit 72f6955
Show file tree
Hide file tree
Showing 26 changed files with 1,792 additions and 604 deletions.
2 changes: 2 additions & 0 deletions karma.conf.js
View file
Expand Up @@ -26,6 +26,7 @@ module.exports = function(config) {
// test assets
{ pattern: 'tests/blocklycompiler-test/cases/*.blocks', watched: false, included: false, served: true, nocache: false },
{ pattern: 'tests/blocklycompiler-test/baselines/*.ts', watched: false, included: false, served: true, nocache: false },
{ pattern: 'tests/blocklycompiler-test/test-library/*', watched: false, included: false, served: true, nocache: false },

// test package files
{ pattern: 'libs/pxt-common/*.ts', watched: false, included: false, served: true, nocache: false },
Expand All @@ -37,6 +38,7 @@ module.exports = function(config) {
proxies: {
"/tests/": "/base/tests/blocklycompiler-test/cases/",
"/baselines/" : "/base/tests/blocklycompiler-test/baselines/",
"/test-library/": "/base/tests/blocklycompiler-test/test-library/",
"/common/": "/base/libs/pxt-common/",
// Needed by webworker
"/blb/": "/base/built/web/"
Expand Down
116 changes: 61 additions & 55 deletions pxtblocks/blocklycompiler.ts
View file
Expand Up @@ -188,14 +188,14 @@ namespace pxt.blocks {
else if (check === "T") {
const func = e.stdCallTable[b.type];
const isArrayGet = b.type === "lists_index_get";
if (isArrayGet || func && func.args.length) {
if (isArrayGet || func && func.comp.thisParameter) {
let parentInput: B.Input;

if (isArrayGet) {
parentInput = b.inputList.filter(i => i.name === "LIST")[0];
}
else {
parentInput = b.inputList.filter(i => i.name === func.args[0].field)[0];
parentInput = b.inputList.filter(i => i.name === func.comp.thisParameter.definitionName)[0];
}

if (parentInput.connection && parentInput.connection.targetBlock()) {
Expand Down Expand Up @@ -389,13 +389,13 @@ namespace pxt.blocks {
default:
if (b.type in e.stdCallTable) {
const call = e.stdCallTable[b.type];
call.args.forEach((p: StdArg, i: number) => {
visibleParams(call, countOptionals(b)).forEach((p, i) => {
const isInstance = call.isExtensionMethod && i === 0;
if (p.field && !b.getFieldValue(p.field)) {
let i = b.inputList.filter((i: B.Input) => i.name == p.field)[0];
if (p.definitionName && !b.getFieldValue(p.definitionName)) {
let i = b.inputList.filter((i: B.Input) => i.name == p.definitionName)[0];
if (i.connection && i.connection.check_) {
if (isInstance && connectionCheck(i) === "Array") {
let gen = handleGenericType(b, p.field);
let gen = handleGenericType(b, p.definitionName);
if (gen) {
return;
}
Expand All @@ -406,7 +406,7 @@ namespace pxt.blocks {
for (let j = 0; j < i.connection.check_.length; j++) {
try {
let t = i.connection.check_[j];
unionParam(e, b, p.field, ground(t));
unionParam(e, b, p.definitionName, ground(t));
break;
}
catch (e) {
Expand Down Expand Up @@ -762,7 +762,7 @@ namespace pxt.blocks {
if (call) {
if (call.imageLiteral)
expr = compileImage(e, b, call.imageLiteral, call.namespace, call.f,
call.args.map(ar => compileArgument(e, b, ar, comments)))
visibleParams(call, countOptionals(b)).map(ar => compileArgument(e, b, ar, comments)))
else
expr = compileStdCall(e, b, call, comments);
}
Expand Down Expand Up @@ -1037,34 +1037,31 @@ namespace pxt.blocks {
return mkStmt(mkInfix(ref, "+=", expr))
}

function eventArgs(call: StdFunc): string[] {
return call.args.map(ar => ar.field).filter(ar => !!ar);
function eventArgs(call: StdFunc, b: B.Block): string[] {
return visibleParams(call, countOptionals(b)).map(ar => ar.definitionName).filter(ar => !!ar);
}

function compileCall(e: Environment, b: B.Block, comments: string[]): JsNode {
const call = e.stdCallTable[b.type];
if (call.imageLiteral)
return mkStmt(compileImage(e, b, call.imageLiteral, call.namespace, call.f, call.args.map(ar => compileArgument(e, b, ar, comments))))
return mkStmt(compileImage(e, b, call.imageLiteral, call.namespace, call.f, visibleParams(call, countOptionals(b)).map(ar => compileArgument(e, b, ar, comments))))
else if (call.hasHandler)
return compileEvent(e, b, call, eventArgs(call), call.namespace, comments)
return compileEvent(e, b, call, eventArgs(call, b), call.namespace, comments)
else
return mkStmt(compileStdCall(e, b, call, comments))
}

function compileArgument(e: Environment, b: B.Block, p: StdArg, comments: string[], beginningOfStatement = false): JsNode {
let lit: any = p.literal;
if (lit)
return lit instanceof String ? H.mkStringLiteral(<string>lit) : H.mkNumberLiteral(<number>lit);
let f = b.getFieldValue(p.field);
function compileArgument(e: Environment, b: B.Block, p: BlockParameter, comments: string[], beginningOfStatement = false): JsNode {
let f = b.getFieldValue(p.definitionName);
if (f != null) {
if (b.getField(p.field) instanceof pxtblockly.FieldTextInput) {
if (b.getField(p.definitionName) instanceof pxtblockly.FieldTextInput) {
return H.mkStringLiteral(f);
}
return mkText(f);
}
else {
attachPlaceholderIf(e, b, p.field);
const target = getInputTargetBlock(b, p.field);
attachPlaceholderIf(e, b, p.definitionName);
const target = getInputTargetBlock(b, p.definitionName);
if (beginningOfStatement && target.type === "lists_create_with") {
// We have to be careful of array literals at the beginning of a statement
// because they can cause errors (i.e. they get parsed as an index). Add a
Expand All @@ -1082,7 +1079,7 @@ namespace pxt.blocks {
args = b.mutation.compileMutation(e, comments).children;
}
else {
args = func.args.map((p: StdArg, i: number) => compileArgument(e, b, p, comments, func.isExtensionMethod && i === 0 && !func.isExpression));
args = visibleParams(func, countOptionals(b)).map((p, i) => compileArgument(e, b, p, comments, func.isExtensionMethod && i === 0 && !func.isExpression));
}

const externalInputs = !b.getInputsInline();
Expand Down Expand Up @@ -1167,10 +1164,10 @@ namespace pxt.blocks {
if (isMutatingBlock(b) && b.mutation.getMutationType() === MutatorTypes.ObjectDestructuringMutator) {
argumentDeclaration = b.mutation.compileMutation(e, comments);
}
else if (stdfun.handlerArgs.length) {
else if (stdfun.comp.handlerArgs.length) {
let handlerArgs: string[] = []; // = stdfun.handlerArgs.map(arg => escapeVarName(b.getFieldValue("HANDLER_" + arg.name), e));
for (let i = 0; i < stdfun.handlerArgs.length; i++) {
const arg = stdfun.handlerArgs[i];
for (let i = 0; i < stdfun.comp.handlerArgs.length; i++) {
const arg = stdfun.comp.handlerArgs[i];
const varName = b.getFieldValue("HANDLER_" + arg.name);
if (varName !== null) {
handlerArgs.push(escapeVarName(varName, e));
Expand Down Expand Up @@ -1207,15 +1204,6 @@ namespace pxt.blocks {
return H.namespaceCall(n, f, [lit].concat(args), false);
}

// A standard function argument may be a field name (see below)
// or a string|number literal.
// The literal is used to hide argument in blocks
// that are available in TD.
export interface StdArg {
field?: string;
literal?: string | number;
}

// A description of each function from the "device library". Types are fetched
// from the Blockly blocks definition.
// - the key is the name of the Blockly.Block that we compile into a device call;
Expand All @@ -1229,9 +1217,8 @@ namespace pxt.blocks {
// is, "basic -> show image" instead of "micro:bit -> show image".
export interface StdFunc {
f: string;
args: StdArg[];
comp: BlockCompileInfo;
attrs: ts.pxtc.CommentAttrs;
handlerArgs?: HandlerArg[];
isExtensionMethod?: boolean;
isExpression?: boolean;
imageLiteral?: number;
Expand Down Expand Up @@ -1407,29 +1394,18 @@ namespace pxt.blocks {
return;
}
e.renames.takenNames[fn.namespace] = true;
let { attrNames, handlerArgs } = pxt.blocks.parameterNames(fn);
let instance = fn.kind == pxtc.SymbolKind.Method || fn.kind == pxtc.SymbolKind.Property;
let args = (fn.parameters || []).map(p => {
if (attrNames[p.name] && attrNames[p.name].name) return { field: attrNames[p.name].name };
else return null;
}).filter(a => !!a);

if (instance && !fn.attributes.defaultInstance) {
args.unshift({
field: attrNames["this"].name
});
}
const comp = pxt.blocks.compileInfo(fn);
const instance = !!comp.thisParameter;

e.stdCallTable[fn.attributes.blockId] = {
namespace: fn.namespace,
f: fn.name,
args: args,
comp,
attrs: fn.attributes,
handlerArgs,
isExtensionMethod: instance,
isExpression: fn.retType && fn.retType !== "void",
imageLiteral: fn.attributes.imageLiteral,
hasHandler: !!handlerArgs.length || fn.parameters && fn.parameters.some(p => (p.type == "() => void" || !!p.properties)),
hasHandler: !!comp.handlerArgs.length || fn.parameters && fn.parameters.some(p => (p.type == "() => void" || !!p.properties)),
property: !fn.parameters,
isIdentity: fn.attributes.shim == "TD_ID"
}
Expand All @@ -1449,9 +1425,9 @@ namespace pxt.blocks {

let stdFunc = e.stdCallTable[b.type];

if (stdFunc && stdFunc.handlerArgs.length) {
if (stdFunc && stdFunc.comp.handlerArgs.length) {
let foundIt = false;
stdFunc.handlerArgs.forEach(arg => {
stdFunc.comp.handlerArgs.forEach(arg => {
if (foundIt) return;
let varName = b.getFieldValue("HANDLER_" + arg.name);
if (varName != null && escapeVarName(varName, e) === name) {
Expand Down Expand Up @@ -1499,8 +1475,8 @@ namespace pxt.blocks {
}

let stdFunc = e.stdCallTable[b.type];
if (stdFunc && stdFunc.handlerArgs.length) {
stdFunc.handlerArgs.forEach(arg => {
if (stdFunc && stdFunc.comp.handlerArgs.length) {
stdFunc.comp.handlerArgs.forEach(arg => {
let varName = b.getFieldValue("HANDLER_" + arg.name)
if (varName != null) {
trackLocalDeclaration(escapeVarName(varName, e), arg.type);
Expand Down Expand Up @@ -1627,7 +1603,7 @@ namespace pxt.blocks {
const call = e.stdCallTable[b.type];
if (call) {
// detect if same event is registered already
const compiledArgs = eventArgs(call).map(arg => compileArg(e, b, arg, []));
const compiledArgs = eventArgs(call, b).map(arg => compileArg(e, b, arg, []));
const key = JSON.stringify({ name: call.f, ns: call.namespace, compiledArgs })
.replace(/"id"\s*:\s*"[^"]+"/g, ''); // remove blockly ids
return key;
Expand Down Expand Up @@ -1784,4 +1760,34 @@ namespace pxt.blocks {
}
return text.substr(text.length - suffix.length) === suffix;
}

function countOptionals(b: Blockly.Block) {
if ((b as MutatingBlock).mutationToDom) {
const el = (b as MutatingBlock).mutationToDom();
if (el.hasAttribute("_expanded")) {
const val = parseInt(el.getAttribute("_expanded"));
return isNaN(val) ? 0 : Math.max(val, 0);
}
}
return 0;
}

function visibleParams({ comp }: StdFunc, optionalCount: number) {
const res: pxt.blocks.BlockParameter[] = [];
if (comp.thisParameter) {
res.push(comp.thisParameter);
}

comp.parameters.forEach(p => {
if (p.isOptional && optionalCount > 0) {
res.push(p);
--optionalCount;
}
else if (!p.isOptional) {
res.push(p);
}
});

return res;
}
}
4 changes: 2 additions & 2 deletions pxtblocks/blocklyimporter.ts
View file
Expand Up @@ -168,11 +168,11 @@ namespace pxt.blocks {
let symbol = blockSymbol(type);
if (!symbol || !b) return;

let params = parameterNames(symbol).attrNames;
let comp = compileInfo(symbol);
symbol.parameters.forEach((p, i) => {
let ptype = info.apis.byQName[p.type];
if (ptype && ptype.kind == pxtc.SymbolKind.Enum) {
let field = getFirstChildWithAttr(block, "field", "name", params[p.name].name);
let field = getFirstChildWithAttr(block, "field", "name", comp.actualNameToParam[p.name].definitionName);
if (field) {
let en = enums[ptype.name + '.' + field.textContent];
if (en) field.textContent = en;
Expand Down

0 comments on commit 72f6955

Please sign in to comment.