JS editor update: Ability to convert JS code from editor to blocks (simple flow, pitch, tone, and rhythm blocks) by ebeetles · Pull Request #4591 · sugarlabs/musicblocks

ebeetles · 2025-03-28T03:05:18Z

AST2BlockList function enables user to convert JS code from the JS editor into simple flow, pitch, tone, and rhythm blocks.

Screen-Recording.mp4

github-actions · 2025-03-28T03:05:59Z

✅ All Jest tests passed! This PR is ready to merge.

omsuneri · 2025-03-28T05:44:07Z

@ebeetles I m bit confused with this like how this actually works ?

ebeetles · 2025-03-29T05:02:59Z

@ebeetles I m bit confused with this like how this actually works ?

Since this doesn't work on all blocks yet, this won't actually be visible to the user yet. To test the functionality for yourself locally, just add the following function in jseditor.js and call it in _runcode() shown below. All you need to do now in the webapp is to write code in the jseditor and then press run, which will generate the according blocks.

pikurasa · 2025-03-31T23:03:01Z

Since this doesn't work on all blocks yet, this won't actually be visible to the user yet. To test the functionality for yourself locally, just add the following function in jseditor.js and call it in _runcode() shown below. All you need to do now in the webapp is to write code in the jseditor and then press run, which will generate the according blocks.

I recommend that you make the feature functional within this branch without the need to change anything.

The way our development works, it won't be visible to the user until it gets merged into master.

ebeetles · 2025-04-01T02:56:47Z

Since this doesn't work on all blocks yet, this won't actually be visible to the user yet. To test the functionality for yourself locally, just add the following function in jseditor.js and call it in _runcode() shown below. All you need to do now in the webapp is to write code in the jseditor and then press run, which will generate the according blocks.

I recommend that you make the feature functional within this branch without the need to change anything.

The way our development works, it won't be visible to the user until it gets merged into master.

Thank you for the reply!
I have some questions regarding the development process here. This project is intended to be rather large, in fact I wrote my GSoC proposal as this project. I was wondering if you guys will review small commits (even if they aren't the complete project and won't be visible to the user) and directly merge into master, or would rather have the entire project finished before reviewing.

I'm asking this because if I update jseditor.js as part of this pull request and it gets merged, it won't work for many blocks as of now.

pikurasa · 2025-04-01T14:01:22Z

Thank you for the reply! I have some questions regarding the development process here. This project is intended to be rather large, in fact I wrote my GSoC proposal as this project. I was wondering if you guys will review small commits (even if they aren't the complete project and won't be visible to the user) and directly merge into master, or would rather have the entire project finished before reviewing.

I'm asking this because if I update jseditor.js as part of this pull request and it gets merged, it won't work for many blocks as of now.

The way we typically do a large project like this, especially one that requires considerable refactoring, is in one PR. We can continue testing it throughout.

The other consideration, however, is: do you expect that changes you make will touch upon files other than jseditor.js and related js-editor files (js/js-export/*)? If so, we need to plan this so that we resolve all conflicts before merging.

walterbender · 2025-04-01T21:34:00Z

I think it is pretty interesting to be able to go back and forth between the block editor and the JS editor. But please consider @pikurasa 's suggestion.

ebeetles · 2025-04-02T05:57:16Z

Thank you @walterbender and @pikurasa! I’m happy to follow suggestions on best practice. Also, I’d like to answer @pikurasa’s question.

For the JavaScript-to-MusicBlocks conversion functionality, I expect to add / change the following files:

Add ast2blocklist.js to js/js-exports - main logic to convert an AST generated from JavaScript code to block list format that can be loaded into the block editor
Add ast2blocklist.test.js to js/js-exports - unit tests for the logic in ast2blocklist.js
Add acorn.js to lib - a library that generates AST from JavaScript
Edit js/widgets/jseditor.js - will add a button to the editor, which will convert JS to blocks and load it into the block editor

My first commit is a proof-of-concept supporting around 30 block types, including start, settimbre, newnote, pitch (solfege only), if, for, all number blocks (supporting arbitrary expressions like 1 / 2, Math.abs(-1) / 2, MathUtility.doRandom(1, 2) / 4, etc.), and all boolean blocks (supporting arbitrary boolean expressions like MathUtility.doRandom(0, 1) == 1, false | MathUtility.doRandom(0, 1) > 0, etc.). However, the total number of blocks to be supported is probably more than 10x this current amount.

I was originally going to make small changes and get them reviewed and submitted, since it seems faster and easier to review smaller changes. My plan was to only integrate the functionality with jseditor.js at the end and heavily rely on unit tests in the meantime (we should know exactly what the block list looks like for each snippet of JS code) so the earlier changes would've only contained ast2blocklist.js and ast2blocklist.test.js.

I’m open to completing the entire implementation in one PR, but I'm a little confused about the review process since I would prefer to receive timely feedback throughout the development rather than a single lengthy review at the end. This is my first time contributing to an open-source project, so please forgive me if my question seems ignorant due to lack of experience with this environment.

walterbender · 2025-04-03T11:29:07Z

I think we should take the following approach:
A PR that allows us to explore the basic idea and review the basic approach in the code. Then we can "complete" the process through additional PRs. As long as the initial PR supports the basic note and pitch blocks and the basic arithmetic functions those blocks use, plus, perhaps repeat and action blocks, that should be enough to get a basic feel for how this would work.

ebeetles · 2025-04-04T22:20:21Z

I think we should take the following approach: A PR that allows us to explore the basic idea and review the basic approach in the code. Then we can "complete" the process through additional PRs. As long as the initial PR supports the basic note and pitch blocks and the basic arithmetic functions those blocks use, plus, perhaps repeat and action blocks, that should be enough to get a basic feel for how this would work.

Sounds good! I currently have basic note, pitch, arithmetic functions, start, and repeat blocks working. I would like to add the action block before the full initial PR. Am I correct in assuming that the action block serves as a function in the MusicBlocks language?

Thank you!

walterbender · 2025-04-04T22:34:34Z

yes. FWIW, you should probably use MB to write a few programs so as to get a better understanding.

github-actions · 2025-04-06T02:49:07Z

✅ All Jest tests passed! This PR is ready to merge.

ebeetles · 2025-04-06T03:05:45Z

Hi @walterbender @pikurasa !

The action block conversion is complete (also works for recursion cases). Please see the test cases in js/js-export/test/ast2blocklist.test.js. I have also included all the other file changes accordingly that allows testing the functionality in the JS editor. To test locally, change the enableCodeToBlocks boolean in line 423 of js/widgets/jseditor.js to true.

Please review, and see if the current comments are enough to understand the changes. Also please let me know if you'd prefer a document for explanation.

Thanks!

walterbender · 2025-04-06T22:10:26Z

Can you please remind me how to test this?

ebeetles · 2025-04-07T00:30:07Z

Can you please remind me how to test this?

To test, simply change the enableCodeToBlocks boolean in line 423 of js/widgets/jseditor.js to true. Then, type valid JS code in the JS editor and click the play button in the editor, which will update the canvas with the according blocks. The screenrecording I originally attached is an example of how to test it.

Please let me know if there are further questions!

walterbender · 2025-04-07T13:59:10Z

Works well. I think we'll want to make this a separate action from Play but I think it is a nice direction for the JavaScript editor.

ebeetles · 2025-04-08T00:20:56Z

Works well. I think we'll want to make this a separate action from Play but I think it is a nice direction for the JavaScript editor.

Thanks, the play button is just a temporary placeholder for testing purposes, but I will definitely come up with something different later.

github-actions · 2025-04-20T03:39:12Z

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

ast2blocklist.test.js

github-actions · 2025-04-20T04:23:28Z

✅ All Jest tests passed! This PR is ready to merge.

github-actions · 2025-04-20T04:52:56Z

✅ All Jest tests passed! This PR is ready to merge.

ebeetles · 2025-04-20T04:58:20Z

Hi @walterbender and @pikurasa,
I have added the functionality for dictionary which supports setValue and getValue with different numbers of arguments (function overloading). Since the PR hasn't been reviewed yet, I also cleaned up the commits and the PR now only includes one clean commit with all past changes and the new one.

walterbender · 2025-04-26T20:44:52Z

Maybe you can add a UX element (button) so we can merge this, even incomplete. I think it is really fun.

github-actions · 2025-04-27T02:33:44Z

✅ All Jest tests passed! This PR is ready to merge.

ebeetles · 2025-04-27T02:34:07Z

Hi @walterbender ,
Thanks for the suggestion, I have made the change and it should now be part of the PR.

walterbender · 2025-04-27T10:36:04Z

js/widgets/jseditor.js

+        convertBtn.onclick = this._codeToBlocks.bind(this);
+        menuLeft.appendChild(convertBtn);
+        menubar.appendChild(menuLeft);
+        generateTooltip(convertBtn, "Convert JavaScript to Blocks");


please put the string inside _() so it is available for translation.

Thanks, the commit has been made

github-actions · 2025-04-27T22:08:19Z

✅ All Jest tests passed! This PR is ready to merge.

walterbender · 2025-04-27T22:20:17Z

Not sure why it is looking for toolbar.test.js. It is in the master branch, but you checked out your branch before that. Maybe a rebase will help?

pikurasa · 2025-04-28T00:26:51Z

I tested it. My only nitpick is that it would be nice to have vertical space render (as we do for a default note block).

Probably can just ignore that for now, but I wanted to mention it.

This is what it looks like without the v-spacers:

It's hard to read.

With the v-spacers, it's easier to read:

meganindya

Perhaps unrelated, but I'm getting an error on clicking the button for code to blocks.

Uncaught (in promise) Error: Not enough space to save locally
    at ProjectStorage.set (ProjectStorage.js:186:19)
    at async ProjectStorage.save (ProjectStorage.js:207:9)

We should address this separately, but for things like these, we should consider using the IndexedDB or the Cache API instead of LocalStorage.

Had a few comments in the code. But otherwise fantastic work!

index.html

js/js-export/ast2blocklist.js

ebeetles · 2025-04-28T23:57:13Z

@walterbender @pikurasa @meganindya, thank you so much for the reviews. I have lots of finals this week so I will be very busy the next few days, but I will find time to fix the issues ASAP.

github-actions · 2025-05-03T04:26:11Z

✅ All Jest tests passed! This PR is ready to merge.

ebeetles · 2025-05-03T04:42:04Z

Hi @walterbender, @pikurasa, @meganindya,

I ran git pull upstream master --rebase before making new changes, and now it seems like the commits in this PR includes commits from everyone else. I'm not very familiar with using git, so is there a command to remove everyone else's commits, or do I have to clean up the PR and restart? If I need to restart, how should I do so to not lose the review comments?

My last commit does include everything to address the review comments, (v-spacers, minified acorn library, and removed driver code from ast2blocklist.js).

Thanks!

walterbender · 2025-05-03T12:53:53Z

Hmm. I usually just git rebase master
Not sure the base way to undo this. But if we close this PR, the comments will be preserved. Maybe you can add a link to the closed PR from the new one you open, just to make it easier to find the comments.

github-actions · 2025-05-03T20:16:13Z

✅ All Jest tests passed! This PR is ready to merge.

…t can be loaded into the MusicBlocks editor. This is the first commit for such functionality, which allows users to write JavaScript code in the JS Editor directly and convert the code back to music blocks. Blocks supported in this commit are: 1. settimbre 2. newnote 3. pitch (only supports solfege) 4. if 5. for 6. All number blocks. Supports arbitrary expressions like 1 / 2, Math.abs(-1) / 2, MathUtility.doRandom(1, 2) / 4, etc. 7. All boolean blocks. Supports arbitrary boolean expressions like MathUtility.doRandom(0, 1) == 1, false | MathUtility.doRandom(0, 1) == 1, etc. 8. Action (supports recursion) 9. Dictionary (setValue, getValue, function overloading) New files in the commit: 1. ast2blocklist.js - main logic to convert an AST generated from JavaScript code to block list format that can be loaded into the block editor 2. ast2blocklist.test.js - unit tests that cover all supported blocks conversion 3. acorn.js - library that converts JavaScript code to AST

…musicblocks. Updated structure of ast2blocklist.js for better readability, and makes adding more block support easier. Minor change: Play pitch now supports notes (A-G) in addition to solfege.

…ranslation.

github-actions · 2025-05-03T20:30:07Z

✅ All Jest tests passed! This PR is ready to merge.

…guments, and adds v-spacers based on the information 2. Instead of console.error for errors, it now throws error and shows in console panel of jseditor 3. Uses minified version of acorn library for faster loading 4. Removed driver code from ast2blocklist

github-actions · 2025-05-03T20:45:16Z

✅ All Jest tests passed! This PR is ready to merge.

ebeetles · 2025-05-03T20:53:50Z

Hi @walterbender @pikurasa @meganindya,
I fixed the extra commit problems in the PR with interactive rebasing. Now it only contains my commits with all the updates from your comments and it is ready for review again.

Also @meganindya, regarding your error in this comment #4591 (review), I am unable to reproduce it. If you can still reproduce the error, may I see the whole stack trace so I will know which line of my code caused it?

walterbender · 2025-05-03T21:35:53Z

I've not tried anything too sophisticated, but it is working for me.

walterbender · 2025-05-03T21:37:28Z

There are some linting errors. (I thought we had things arranged so that libraries don't get linted -- we don't want to fix acorn.min.js here.)

…fix lint errors.

github-actions · 2025-05-05T19:29:39Z

✅ All Jest tests passed! This PR is ready to merge.

ebeetles closed this Apr 20, 2025

ebeetles force-pushed the jseditor branch from 9718549 to f08c103 Compare April 20, 2025 04:22

ebeetles reopened this Apr 20, 2025

walterbender reviewed Apr 27, 2025

View reviewed changes

meganindya requested changes Apr 28, 2025

View reviewed changes

index.html Outdated Show resolved Hide resolved

js/js-export/ast2blocklist.js Outdated Show resolved Hide resolved

js/js-export/ast2blocklist.js Outdated Show resolved Hide resolved

js/js-export/ast2blocklist.js Outdated Show resolved Hide resolved

ebeetles force-pushed the jseditor branch from 21c780b to f650ec6 Compare May 3, 2025 20:15

ebeetles added 3 commits May 3, 2025 16:24

Updated UI to include button for converting JS code in the editor to …

0a72f3a

…musicblocks. Updated structure of ast2blocklist.js for better readability, and makes adding more block support easier. Minor change: Play pitch now supports notes (A-G) in addition to solfege.

Updated tooltips in jseditor to use _() so that it is available for t…

456acd5

…ranslation.

ebeetles force-pushed the jseditor branch from f650ec6 to 456acd5 Compare May 3, 2025 20:29

Updated ast2blocklist, ast2blocklist.test, jseditor, and activity to …

2f0eb99

…fix lint errors.

walterbender merged commit 71791d7 into sugarlabs:master May 5, 2025
5 checks passed

Uh oh!

Conversation

ebeetles commented Mar 28, 2025

Uh oh!

github-actions bot commented Mar 28, 2025

Uh oh!

omsuneri commented Mar 28, 2025

Uh oh!

ebeetles commented Mar 29, 2025

Uh oh!

pikurasa commented Mar 31, 2025

Uh oh!

ebeetles commented Apr 1, 2025

Uh oh!

pikurasa commented Apr 1, 2025

Uh oh!

walterbender commented Apr 1, 2025

Uh oh!

ebeetles commented Apr 2, 2025

Uh oh!

walterbender commented Apr 3, 2025

Uh oh!

ebeetles commented Apr 4, 2025

Uh oh!

walterbender commented Apr 4, 2025

Uh oh!

github-actions bot commented Apr 6, 2025

Uh oh!

ebeetles commented Apr 6, 2025

Uh oh!

walterbender commented Apr 6, 2025

Uh oh!

ebeetles commented Apr 7, 2025

Uh oh!

walterbender commented Apr 7, 2025

Uh oh!

ebeetles commented Apr 8, 2025

Uh oh!

github-actions bot commented Apr 20, 2025

Uh oh!

github-actions bot commented Apr 20, 2025

Uh oh!

github-actions bot commented Apr 20, 2025

Uh oh!

ebeetles commented Apr 20, 2025

Uh oh!

walterbender commented Apr 26, 2025

Uh oh!

github-actions bot commented Apr 27, 2025

Uh oh!

ebeetles commented Apr 27, 2025

Uh oh!

walterbender Apr 27, 2025

Choose a reason for hiding this comment

Uh oh!

ebeetles Apr 27, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 27, 2025

Uh oh!

walterbender commented Apr 27, 2025

Uh oh!

pikurasa commented Apr 28, 2025

Uh oh!

meganindya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ebeetles commented Apr 28, 2025

Uh oh!

github-actions bot commented May 3, 2025

Uh oh!

ebeetles commented May 3, 2025

Uh oh!

walterbender commented May 3, 2025

Uh oh!

github-actions bot commented May 3, 2025