bartbutenaers / node-red-contrib-blockly

A Node Red node for visual programming a function using Blockly
Apache License 2.0
88 stars 22 forks source link

Remove French patch at next Blockly release #98

Closed bartbutenaers closed 10 months ago

bartbutenaers commented 3 years ago

As explained in this pull-request, the patch lines in the french language file should be removed when there is a next Blockly release:

// Overriding 7 strings that have a translation problem in French in the upstream Blockly project
// for versions 5.20210325.0 (Q1 2021), 5.20210325.1 (Q1 2021 Patch 1) and 6.20210701.0 (Q2 2021).
// See https://github.com/google/blockly/issues/5399
// TODO: When there will be a new Blockly release, check that the `msg/js/fr.js` does not
//  contain `<!-- -->`, then bump the minimal Blockly dependency version to that release,
//  and remove the following 7 lines.

Note: the Blockly releases are announced here.

echoix commented 2 years ago

So there seems to be a new release since yesterday. It seems that some quite big changes were made under the hood, so maybe we should wait a couple days, a week or two before testing everything and removing the French patch.

https://github.com/google/blockly/releases/tag/7.20211209.0

echoix commented 2 years ago

I tried today updating the minimal required version, but when using blackly 7 or 8 (instead of 6.202107xx.yy ) some complete tabs of the toolbox, like logic, loops, and others, are either empty or show a text with the tag to replace (like BKY_LOGIC_OPERATOR_AND). I tried debugging a little bit, but quickly got confused to where this toolbox.xml file is generated. When looking at blockly's fit history for each file that could've been changed between the working version (currently used in this package) and the later published ones, I didn't find a way that explains why it is broken afterwards.

Maybe it didn't help that I was using the dev 3.0.0-beta2 Docker image, but either way, I was able to see the changes applied for the original French translation issue (since these blocks aren't broken). It is only some other blocks, at least in the basic blockly blocks that don't work with a newer Blockly release.

See my branch https://github.com/echoix/node-red-contrib-blockly/tree/blockly-upgrade-translation-1 to see the new package.json that bumps dependencies. If you run npm pack, you'll get a .tgz file that you can upload to node-red's manage palette to test the package.

bartbutenaers commented 2 years ago

Hi @echoix,

Sorry for not responding earlier. I have almost no time to do Node-RED stuff during the last months...

when using blockly 7 or 8

Is there any particular feature that would be interesting from those releases for our blockly node? Or do you just want to keep this node a bit in sync with the Blockly releases perhaps. Because I thought that those releases only contain a lot of internal refactoring of the Blockly codebase, but only very few new features.

Anyway I see a series of breaking changes on there release page, so I am not surprised it won't work withouth refactoring from our side.

quickly got confused to where this toolbox.xml file is generated

I don't know the exact details anymore, but it was not a 100% scientific approach. You can see here that we have used the approach from Jeff, which he had reviewed in the Google Blockly forum.

Maybe it didn't help that I was using the dev 3.0.0-beta2 Docker image

Don't know that. I only know that for two other of my nodes, I have (open) issues about problems when using those nodes in the V3 release. Can't keep up with it right now...

bartbutenaers commented 10 months ago

Hi @echoix,

Long time ago... We are now building a new version of this node, to migrate from Blockly 6.x to 10.x If you still use Node-RED and have some time to check whether your fix can be removed, that would be appreciated!

Thanks! Bart

echoix commented 10 months ago

Sorry, I didn't use blockly nor node red since, so it's pretty far back. I remember once trying to see what would it take to upgrade to a newer version, but it was so different that I didn't manage to go any further.

echoix commented 10 months ago

Your guess would be as good as mine

bartbutenaers commented 10 months ago

Ah no problem at all! Then I will guess ;-)

bartbutenaers commented 10 months ago

Removed the fix from Edouard and the comment tags are still not visible. So case closed in our 2.2.0 version.