search

bartbutenaers / node-red-contrib-blockly

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

Update bufferBlocksCodeGen.js #43

Closed cymplecy closed 5 years ago

cymplecy commented 5 years ago

fix index NaN issue in buffer-get/set-index

bartbutenaers commented 5 years ago

Hi @cymplecy ,

Is this the same pull request as the other one?? If so, you can close this one ?? Your code fix seems to be very logical.

Thanks for fixing ! Bart

cymplecy commented 5 years ago

No - this is orig request to update the 1.2.0 release - the other one was just me merging your official 1.2.0 with my hotfix into a working version that I can download to my machines

Since you seem happy with my code fix - I'll merge this into the release-1.2.0 and delete my other branches - you'll have to remember to update your local version though

bartbutenaers commented 5 years ago

@cymplecy Quick question (without testing): Why do you generate '[(' + index + ' - 1)]' instead of '[' + index + ' - 1]' ? Doesn't the latter one (with simpler syntax) work well?

cymplecy commented 5 years ago

Defensive programming A lot of JS seems to have () around things plus I'm not a JS programmer

Feel free to change it :)

bartbutenaers commented 5 years ago

@cymplecy I had also noticed that a lot of useless round brackets are generated, which makes the code less readable for novice users that want to learn from the generated code. But I'm not always sure how to get rid of them. Most of the time they are automatically generated by the Blockly.JavaScript.ORDER_ATOMIC, Blockly.JavaScript.ORDER_NONE ... That is still some kind of black box to me ... But if it also works fine in your test without, I propose we skip them.