Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/blocks/mrc_class_method_def.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,13 @@ export const setup = function() {
Blockly.Blocks[PARAM_CONTAINER_BLOCK_NAME] = METHOD_PARAM_CONTAINER;
};

import { Order, PythonGenerator } from 'blockly/python';
import { Order } from 'blockly/python';
import { ExtendedPythonGenerator } from '../editor/extended_python_generator';


export const pythonFromBlock = function (
block: ClassMethodDefBlock,
generator: PythonGenerator,
generator: ExtendedPythonGenerator,
) {
const blocklyName = block.mrcPythonMethodName ? block.mrcPythonMethodName : block.getFieldValue('NAME');

Expand Down Expand Up @@ -378,6 +379,9 @@ export const pythonFromBlock = function (
// After executing the function body, revisit this block for the return.
xfix2 = xfix1;
}
if(block.mrcPythonMethodName == '__init__'){
branch = generator.INDENT + "self.mechanisms = []\n" + branch;
}
if (returnValue) {
returnValue = generator.INDENT + 'return ' + returnValue + '\n';
} else if (!branch) {
Expand All @@ -397,11 +401,6 @@ export const pythonFromBlock = function (
'(' +
paramString +
'):\n';

if(block.mrcPythonMethodName == '__init__'){
code += generator.INDENT + "self.mechanisms = []\n";

}

code +=
xfix1 +
Expand All @@ -410,6 +409,7 @@ export const pythonFromBlock = function (
xfix2 +
returnValue;
code = generator.scrub_(block, code);
generator.addMethod(funcName, code);

return code;
}
28 changes: 17 additions & 11 deletions src/editor/extended_python_generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import * as commonStorage from '../storage/common_storage';
export class ExtendedPythonGenerator extends PythonGenerator {
private currentModule: commonStorage.Module | null = null;
private mapWorkspaceIdToExportedBlocks: { [key: string]: Block[] } = Object.create(null);
protected methods_: {[key: string]: string} = Object.create(null);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why protected instead of private?
Why the trailing _?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I almost always go for protected instead of private so it can be subclassed later.
The trailing _ was because I was copying the style from definitions_

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm the opposite. I go for the most restrictive visibility first and then make it more visible only if necessary.
If it needs to be subclassed later, it can be changed from private to protected then.
https://google.github.io/styleguide/tsguide.html#visibility

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I agree fully about not making things public by default (as the style guide says), protected seems to be better than private. Imagine the mess we would be in if Blockly had made everything private instead of protected. (I'll admit that this tendency of mine is because I am almost always creating libraries or systems instead of applications.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense for libraries where you (as author of the library) want to allow a class to be subclassed. In this case, it is not a library and within this application, we don't currently want to subclass ExtendedPythonGenerator.

I'm going to approve (and merge) this PR so we can keep making progress.



constructor() {
super('Python');
Expand Down Expand Up @@ -158,6 +160,9 @@ export class ExtendedPythonGenerator extends PythonGenerator {
addImport(importModule: string): void {
this.definitions_['import_' + importModule] = 'import ' + importModule;
}
addMethod(methodName: string, code : string): void {
this.methods_[methodName] = code;
}

classParentFromModuleType(moduleType : string) : string{
if(moduleType == commonStorage.MODULE_TYPE_PROJECT){
Expand Down Expand Up @@ -186,31 +191,32 @@ export class ExtendedPythonGenerator extends PythonGenerator {
// Convert the definitions dictionary into a list.
const imports = [];
const definitions = [];

for (let name in this.definitions_) {
const def = this.definitions_[name];
if (def.match(/^(from\s+\S+\s+)?import\s+\S+/)) {
imports.push(def);
} else {
} else{
definitions.push(def);
}
}
// Call Blockly.CodeGenerator's finish. This is required to be done this way
// because we derive from PythonGenerator which dervies from CodeGenerator
// This section except for the class_def part is all copied from Blockly's
// PythonGenerator. It can't be derived because it needs the class insertion
// in the middle.
code = Blockly.CodeGenerator.prototype.finish(code);
const methods = [];
for (let name in this.methods_){
methods.push(this.methods_[name])
}

this.definitions_ = Object.create(null);
this.functionNames_ = Object.create(null);
this.methods_ = Object.create(null);

this.isInitialized = false;

let class_def = "class " + className + "(" + classParent + "):\n";
if (!code) {
code = "pass";
}

this.nameDB_!.reset();
const allDefs = imports.join('\n') + '\n\n' + definitions.join('\n\n');
return allDefs.replace(/\n\n+/g, '\n\n').replace(/\n*$/, '\n\n\n') + class_def +
this.prefixLines(code, this.INDENT);
this.prefixLines(methods.join('\n\n'), this.INDENT);
}
}

Expand Down