[LANGUAGE, UI] Integrate Skulpts debugger to Hedy #4465

jpelay · 2023-09-08T02:17:40Z

Description

Skulpt has an integrated debugger that is shipped as a separate entity and is envisioned to work like pdb. The basis that is there, however, can help us a great deal in getting our own debugger to work. For this, first we need to integrate the debugger that comes with Skulpt and edit it so it can work with our code. This will not affect Skulpt, since we are just extending it, so any problems that might come with the debugger will not affect the normal execution of code.

However, by doing this, we are taking the responsibility to maintain this piece of code, but I think is very much worth the cost.

The code in this PR will most likely be a little ugly since this is a proof of concept about getting this to work, and then I'll tackle the task of improving the code and making it adhere to our standards.

fixes #4427

jpelay · 2023-09-08T02:19:08Z

It does execute code!

Felienne · 2023-09-08T15:25:47Z

OMG I LOVE WE ARE EXPLORING THIS!!

I am ccing @juliabolt, she is also exploring more work on the debugger (since yesterday, so she is very new :)) It might make sense in a few weeks to meet (she might also drop into the contributors meeting in a while)

jpelay · 2023-09-08T15:32:10Z

OMG I LOVE WE ARE EXPLORING THIS!!

I am ccing @juliabolt, she is also exploring more work on the debugger (since yesterday, so she is very new :)) It might make sense in a few weeks to meet (she might also drop into the contributors meeting in a while)

Great! Ok! I'm gonna keep working on this and will keep you updated. I also have a functioning version of the debugger from a past version of the Skulpt site, so I can provide that as well, if you'd like

jpelay · 2023-09-09T15:27:12Z

It works now for print!

debugger-Hedy.mp4

jpelay · 2023-09-09T17:54:38Z

Some bad news, the debugger doesn't work with the input function we supply to Skulpt, it only works when the input function is a simple alert from Javascript

debug.3.mp4

Felienne · 2023-09-09T18:16:40Z

Some bad news, the debugger doesn't work with the input function we supply to Skulpt, it only works when the input function is a simple alert from Javascript

debug.3.mp4

Ah what a pity! Could we somehow catch the input and replace it with our box? Something like:

Show our box first, store inout
Run the simple alert, enter the stored input (without the user seeing)
Continue?

But I am not at all sure these things are feasible!

jpelay · 2023-09-09T18:28:39Z

Ah what a pity! Could we somehow catch the input and replace it with our box? Something like:

Show our box first, store inout

Run the simple alert, enter the stored input (without the user seeing)

Continue?

But I am not at all sure these things are feasible!

That's a very good idea, not sure if it's feasible due to the program having to stop in order to do that, but I can try!

juliabolt · 2023-09-12T09:22:00Z

Thanks for tagging me Felienne! And thanks for the updates, I look forward to meeting you Jesus!

jpelay · 2023-09-14T00:26:27Z

So I managed to fixed the problems and it works with ask now!

debugger.ask.woring.mp4

jpelay · 2023-09-14T00:54:30Z

I need to work with the integration between the source mapper and the debugger, but if you hardcode the breakpoints, debugging nested code is possible!

debugger.repeat.block.mp4

Felienne · 2023-09-14T07:08:34Z

I need to work with the integration between the source mapper and the debugger, but if you hardcode the breakpoints, debugging nested code is possible!

debugger.repeat.block.mp4

OOOWWW so great!!!! This is going to help students so much!!

…reakpoint

jpelay · 2023-09-14T20:43:25Z

I'm testing the debugger with each statement to see where I can place the breakpoints in a way that makes sense and works. So far everything works, except for sleeps after repeats. It even works for nested code like this, in the console you can see the current python line being executed:

debugging.nested.blocks.mp4

jpelay · 2023-09-14T21:37:06Z

After testing the debugger with every statement in Hedy it works for almost everything, except the PyGame. Although I think I need to look into it more to decide definitely whether it works with the pygame or not. But it works for everything else!!! In every level!!!

jpelay · 2023-09-15T16:12:40Z

Note to self: Need to change the way the editor handle runtime errors and mimic the way Skulpt handles them in a normal execution path.

jpelay · 2023-09-17T21:18:08Z

Hi @Felienne! I have a question about how can we proceed with respect to marking in which Python lines to place the breakpoints. I have come out with two proposals:

Mark the lines with a comment at compile time. This would consists on a flag being set in the ConvertToPython class, if the flag is set to true we would generate something like this:

__trtl = 100
try:
  __trtl = int(__trtl)
except ValueError:
  raise Exception(f'....')
t.forward(min(600, __trtl) if __trtl > 0 else max(-600, __trtl)) # ___BREAKPOINT___
time.sleep(0.1)
__trtl = 45
try:
  __trtl = int(__trtl)
except ValueError:
  raise Exception(f'...')
t.right(min(600, __trtl) if __trtl > 0 else max(-600, __trtl)) # ___BREAKPOINT___

This has the advantage of not needing any post processing in the Javascript, because we'd only need to iterate over the code, find the lines with # __BREAKPOINT__ in them, and set the breakpoint there.

However, I do understand that by doing this we can be adding a failure point in one of our most critical systems, possibly causing issues with the transpilation process if a bug is introduced.

Process the lines in the front-end, deciding where to put the breakpoints by looking at the source map and then processing the Python code. In this case the source map would look like this:

    "0": {
        "hedy_range": {...},
        "python_range": {...},
        "error": null | exception
        "command": "var" | "assign" | "if" | "while" | "for" | ...
    }
}

Then, in Typescript, we'd iterate in the sourcemap, and depending on the level and which command we're processing, decide where to put the breakpoint. For example, in the case of a repeat on level 7:

if (statement.command === 'repeat) {
    python_code = getCodeFromSourceMap(statement.python_range) // get the Python lines as an array
    for (line in python_code) { 
        if (line.match('/for __i__ in range.+/')) {
            // place breakpoint in this line
        }
    }
}

Some commands, however, like the turtle commands or the if in level 5-7 in the source map appear as the rule command, having we to distinguish between them by processing their respectives strings.

As you can see this method involves more code and we'd be dealing with the Python code without knowing the structure of the underlying code, as opposed as doing it directly at compile time. We'd have to do a ton of string processing for each command of each level, at least when they change. Another downside of this method is that when changing the generated Python code in the transpiler, we'd have to edit this processing as well. However, this would mean that the transpiler code would be unchanged.

What do you think?

Felienne · 2023-09-18T05:30:30Z

Hi @jpelay!

Thanks for the extensive explanation!! This really helped me to quickly understand the two options. I would propose option 1 for two reasons:

The Python transpilation part (while being our most critical system as you say) is the best tested part of the code, I don't fear so much that it will break, and if it break, we will know soon from users. Breaking javascript is trickier to test and to detect for users too.
In general, our line recently has been "more Python, less javascript" (with the HTMX change, for example) More things in one system for writing and testing helps to get a better grip on the code base.

From your text I also read (maybe in between the lines) that you'd prefer option 1? I see a lot of downsides mentioned in option 2. If your main worry for 1 is breaking the transpiler, I think we should take the risk because it is well tested and well understood.

jpelay · 2023-10-29T02:06:06Z

This PR is now ready for review! There are some things that I want to improve and change, but I think we can leave that for a new PR (I need to have this finished so I can present it in my dissertation)

Felienne · 2023-10-29T16:34:33Z

This seems to work GREAT from the front-end @jpelay!! Finally a debugger for if's and loops!!!!

One tiny UX issue I saw is that if you start the debugger, you get the confetti immediately:

Maybe we can suppress it when running from the debugger? Or at least until the final line?

jpelay · 2023-10-29T16:50:11Z

This seems to work GREAT from the front-end @jpelay!! Finally a debugger for if's and loops!!!!

One tiny UX issue I saw is that if you start the debugger, you get the confetti immediately:
Maybe we can suppress it when running from the debugger? Or at least until the final line?

Yes, of course!

Felienne · 2023-10-30T06:49:40Z

Wooohoooooo!!! All seems to be in order! I will deploy now, and then merge, so we can test for a bit in alpha just to be sure.

Felienne · 2023-10-30T08:45:36Z

static/js/app.ts

@@ -18,6 +18,7 @@ import { HedyEditor, EditorType } from './editor';
 import { HedyAceEditorCreator } from './ace-editor';
 import { stopDebug } from "./debugging";

+export let theGlobalDebugger: any;


Would it make sense (not in this PR but in the future!) to also make an interface for the debugger like we do for the editor?

Yes! I'm planning on two things: leave the debugger in JavaScript and write an vendor interface like we do for some other Skulpt code we execute . Or, convert it entilery to TypeScript.

The interface we use for Skulpt is here ((

hedy/static/js/vendor.d.ts

Lines 4 to 40 in 4c4d24e

declare const Sk: {

pre: string;

TurtleGraphics?: {

target?: string;

width?: number;

height?: number;

worldWidth?: number;

worldHeight?: number;

};

execStart: date;

execLimit: number;

globals: Record<string, Variable>;

main_canvas: HTMLCanvasElement;

builtin;

abstr;

builtinFiles?: {

files: Record<string, string>;

};

python3: any;

configure(options: any): void;

misceval: {

asyncToPromise<A>(fn: () => Suspension, handler?: Record<string, () => void>): Promise<A>;

promiseToSuspension;

Suspension: { }

},

importMainWithBody(name: string, dumpJS: boolean, body: string, canSuspend: boolean): Suspension;

setTimeout?: (func: () => void, delay: number) => any;

insertPyGameEvent: (eventName: string) => null;

bindPygameListeners(); void;

unbindPygameListeners(): void;

}

))

Felienne · 2023-10-30T08:57:27Z

static/vendor/skulpt.min.js

Is this a new version of Skulpt (if so, which one) or is this the one we were already supporting (1.3.0 I think? https://github.com/hedyorg/hedy/pull/753/files)? Would be good to document which version we are on, just in case we ever need to update. Because now it is a bit hard to see which it is.

This is a version of Skulpt I built from their source. I also recall we updated that Skulpt version to a more recent one to be able to use Unicode variable names! We did that in this PR: https://github.com/hedyorg/hedy/pull/3119/files

I still need to add the fork to our organization, and include that in the documentation.

This is a version of Skulpt I built from their source. I also recall we updated that Skulpt version to a more recent one to be able to use Unicode variable names! We did that in this PR: https://github.com/hedyorg/hedy/pull/3119/files

Ah yeah, you are right, that is even more recent. We do want to keep that version haha, or we will have to hash all variable names again!

I still need to add the fork to our organization, and include that in the documentation.

I guess we want to do that in a next PR? Shall we make an issue so we don't forget?

Opened #4685

Felienne · 2023-10-30T09:06:50Z

static/js/app.ts

-  outputDiv.append(variableButton);
-  outputDiv.append(variables);
-
-  const storage = window.localStorage;
  let skulptExternalLibraries:{[index: string]:any} = {
      './extensions.js': {
        path: theStaticRoot + "/vendor/skulpt-stdlib-extensions.js",


In light of our new changes, might it be safer to load all Skuplt recourses ourselves? I worry a bit that they might update stuff that breaks our new code (but I can't oversee whether that is a reasonable worry, what do you think @jpelay?)

We added this CDN link in #4490 for performance reasons (so we will make @rix0rrr sad haha)

Sorry, Felienne, but I don't fully understand what you mean by load all Skulpt resources ourselves? I didn't remove that part of the code, the storage = window.localStorage was being used to know if we were in debug mode, no need to do that since now we have two clearly differentiated execution paths!

Ah never mind, my bad! I thought thestaticroot was referring to a generic CDN hosting libraries (something like https://developers.google.com/speed/libraries) but I see now it just refers to our own repo, so we can leave it as is!

Felienne

Let's gooooooooo!!!!

mergify · 2023-10-30T13:28:20Z

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

…ests (#4686) After merging #4465 some Cypress tests were failing. This was because an ad-hoc calcutation I did to figure out the current debugging line. It worked fine locally and even in the tests in the PR, but for some reason failed later on, this PR deabilates that check and I will work on it later this week. Also, after testing in Alpha I found out I wasn't showing the error messages when an error ocurred during execution. How to test? Go to level 3 and write this code: ``` l is 1, 2 print l at 3 ``` Then debug it and when you reach the second line this error should be shown: ![image](https://github.com/hedyorg/hedy/assets/45865185/62836f09-e3e9-403b-bdb0-b99394bd718b)

**Description** Whoops, #4465 broke the embedded editor, this PR adds the required argument to the `runit()` function

**Description** On #4465 we added a debugger to Hedy that needed to use some changes on the Skulpt library. For that I added a Skulpt version that I took from my own fork; now that we have our fork of Skulpt https://github.com/hedyorg/skulpt I'll use this oportunity to delete some `console.log`s that I left there and also explicitly leave a trace to point to a single PR that changed the version of Skulpt we are using.

first attempt at integrating the debugger

751d775

add step mode

85a893e

jpelay added 5 commits September 11, 2023 13:25

removing sleep after repeat bc it doesnt work

9be285a

removing execLimit and timeOut function from debugging configuration

98cac38

debugging logs for skulpt related files

ddc35bc

letting the debugger be shown for all levels

6bcc097

automatic js files

2631202

Felienne assigned jpelay Sep 13, 2023

jpelay added 2 commits September 13, 2023 16:43

Update app.ts

e52ed49

managed to get it working with ask

663d5c8

changing step forward function so it runs as a continue to the next b…

f0bc905

…reakpoint

refactor debugger to be a class

2698feb

jpelay added 2 commits September 15, 2023 19:50

add command name to source map

50eb64d

Merge branch 'main' of github.com:hedyorg/hedy into integrate-debugger

a7c68d0

jpelay added 5 commits October 28, 2023 15:58

Merge branch 'main' of github.com:hedyorg/hedy into integrate-debugger

1d9bdab

add debug parameter

e9b5c05

removin debug console logs

5b978e3

fix is highlighting rule

9ea852a

add cypress tests for debugger

c8bb328

jpelay marked this pull request as ready for review October 29, 2023 02:04

credits to WebTygerJython

a27ebb3

jpelay and others added 4 commits October 29, 2023 12:54

remove success message when debugging

b8636dc

add limit when mapping code to avoid infinite loop

36e60d7

fix source maps in tests

c1e5891

Merge branch 'main' into integrate-debugger

60580e3

Felienne reviewed Oct 30, 2023

View reviewed changes

jpelay mentioned this pull request Oct 30, 2023

Add Skulpt fork to our repos #4685

Closed

Merge branch 'main' into integrate-debugger

5a58abf

Felienne approved these changes Oct 30, 2023

View reviewed changes

mergify bot merged commit 744b1ba into hedyorg:main Oct 30, 2023
9 checks passed

jpelay mentioned this pull request Oct 30, 2023

[FIX] Fix error messages in debugger, and temporary fix for cypress tests #4686

Merged

TiBiBa mentioned this pull request Oct 30, 2023

[BUG FIX] Fix broken embedded editor #4687

Merged

mergify bot pushed a commit that referenced this pull request Oct 31, 2023

[BUG FIX] Fix broken embedded editor (#4687)

0aa621d

**Description** Whoops, #4465 broke the embedded editor, this PR adds the required argument to the `runit()` function

Felienne added the newsletter PRs to talk about for the CM label Nov 3, 2023

jpelay mentioned this pull request Nov 16, 2023

[CHORE] Use Skulpt version from our fork #4760

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LANGUAGE, UI] Integrate Skulpts debugger to Hedy #4465

[LANGUAGE, UI] Integrate Skulpts debugger to Hedy #4465

jpelay commented Sep 8, 2023 •

edited by Felienne

Loading

jpelay commented Sep 8, 2023

Felienne commented Sep 8, 2023

jpelay commented Sep 8, 2023

jpelay commented Sep 9, 2023 •

edited

Loading

jpelay commented Sep 9, 2023

Felienne commented Sep 9, 2023

jpelay commented Sep 9, 2023

juliabolt commented Sep 12, 2023

jpelay commented Sep 14, 2023 •

edited

Loading

jpelay commented Sep 14, 2023

Felienne commented Sep 14, 2023

jpelay commented Sep 14, 2023

jpelay commented Sep 14, 2023

jpelay commented Sep 15, 2023

jpelay commented Sep 17, 2023

Felienne commented Sep 18, 2023 •

edited

Loading

jpelay commented Oct 29, 2023

Felienne commented Oct 29, 2023

jpelay commented Oct 29, 2023

Felienne commented Oct 30, 2023

Felienne Oct 30, 2023

jpelay Oct 30, 2023

Felienne Oct 30, 2023 •

edited

Loading

jpelay Oct 30, 2023

jpelay Oct 30, 2023

Felienne Oct 30, 2023

Felienne Oct 30, 2023

jpelay Oct 30, 2023

Felienne Oct 30, 2023

jpelay Oct 30, 2023

Felienne Oct 30, 2023

Felienne left a comment

mergify bot commented Oct 30, 2023

	declare const Sk: {
	pre: string;
	TurtleGraphics?: {
	target?: string;
	width?: number;
	height?: number;
	worldWidth?: number;
	worldHeight?: number;
	};
	execStart: date;
	execLimit: number;
	globals: Record<string, Variable>;
	main_canvas: HTMLCanvasElement;
	builtin;
	abstr;

	builtinFiles?: {
	files: Record<string, string>;
	};

	python3: any;

	configure(options: any): void;

	misceval: {
	asyncToPromise<A>(fn: () => Suspension, handler?: Record<string, () => void>): Promise<A>;
	promiseToSuspension;
	Suspension: { }
	},

	importMainWithBody(name: string, dumpJS: boolean, body: string, canSuspend: boolean): Suspension;

	setTimeout?: (func: () => void, delay: number) => any;
	insertPyGameEvent: (eventName: string) => null;
	bindPygameListeners(); void;
	unbindPygameListeners(): void;
	}

[LANGUAGE, UI] Integrate Skulpts debugger to Hedy #4465

[LANGUAGE, UI] Integrate Skulpts debugger to Hedy #4465

Conversation

jpelay commented Sep 8, 2023 • edited by Felienne Loading

jpelay commented Sep 8, 2023

Felienne commented Sep 8, 2023

jpelay commented Sep 8, 2023

jpelay commented Sep 9, 2023 • edited Loading

jpelay commented Sep 9, 2023

Felienne commented Sep 9, 2023

jpelay commented Sep 9, 2023

juliabolt commented Sep 12, 2023

jpelay commented Sep 14, 2023 • edited Loading

jpelay commented Sep 14, 2023

Felienne commented Sep 14, 2023

jpelay commented Sep 14, 2023

jpelay commented Sep 14, 2023

jpelay commented Sep 15, 2023

jpelay commented Sep 17, 2023

Felienne commented Sep 18, 2023 • edited Loading

jpelay commented Oct 29, 2023

Felienne commented Oct 29, 2023

jpelay commented Oct 29, 2023

Felienne commented Oct 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Felienne Oct 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Felienne left a comment

Choose a reason for hiding this comment

mergify bot commented Oct 30, 2023

jpelay commented Sep 8, 2023 •

edited by Felienne

Loading

jpelay commented Sep 9, 2023 •

edited

Loading

jpelay commented Sep 14, 2023 •

edited

Loading

Felienne commented Sep 18, 2023 •

edited

Loading

Felienne Oct 30, 2023 •

edited

Loading