google / blockly

The web-based visual programming editor.
https://developers.google.com/blockly/
Apache License 2.0
12.37k stars 3.69k forks source link

Provide more user-friendly generator APIs #7326

Open BeksOmega opened 1 year ago

BeksOmega commented 1 year ago

Check for duplicates

Problem

There are a bunch of methods and properties within the generator code that: 1) Are protected, when they need to be public so they can be used by block-code generators. 2) Have confusing names.

The following things are protected but used by generators:

The provideFunction_ method (which is only supposed to be used for developer-defined functions) is public, but has a sad underscore in the name :/

Request

Provide an API that matches what external developers are actually trying to do.

I think it should look something like:

Alternatives considered

Make the protected methods public so they're actually usable. But stick with the less-than-ideal API.

Additional context

Related to https://github.com/google/blockly/issues/6008 (should possibly supercede it)

maribethb commented 1 year ago

It is probably worth adding some new functions (e.g. we already agreed on getVariableName and getProcedureName, and addImport or addProcedure might also make sense, to be discussed when we get to this change) but let's not rename the functions just to remove underscores. It creates a lot of churn for not a lot of benefit. They should be made public though.

cpcallen commented 1 year ago

let's not rename the functions just to remove underscores. It creates a lot of churn for not a lot of benefit.

If we're changing a bunch of other things in the API then I think we should provide (and document) non-underscored names—but there's no reason to cause churn: the existing underscored names can be kept indefinitely as aliases/wrappers.

maribethb commented 1 year ago

Rachel & I discussed further and still think it's not worth the additional overhead and churn of adding aliases:

BeksOmega commented 1 month ago

We had someone running into issues with definitions_ being protected on the forum: https://groups.google.com/g/blockly/c/T0IiSWN4R9M

cpcallen commented 1 month ago

The most recent request for this comes from a user who wants to add Python imports. We also do this in generators/python/math.ts (amongst other places) and also workaround via any:

(generator as AnyDuringMigration).definitions_['import_math'] = 'import math';

(If you are working around this issue in your own code use any instead of our internal alias anyDuringMigration.)