Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support debugging in the browser #109

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Conversation

winniederidder
Copy link
Contributor

@winniederidder winniederidder commented Mar 12, 2022

This PR allows the user to run their code in debugging mode.
The user can set breakpoints at lines of their choice by clicking to the left of the line number in the editor.
Afterwards, they can click the "Debug" button to run their code in debugging mode.
This will run the code and halt at the first encountered breakpoint.
From then onwards, they will get access to common actions such as:

  • Step over (go to next line)
  • Step into (go into line if possible, e.g. jump to called function)
  • Continue (run until next breakpoint
  • Stop (abort run)
    Ideally, they can also see useful local variables. How to exactly display these, is an open question.

This will be implemented for Python by wrapping Python's debugger Pdb. It has all the commands we need, but uses prompt and print to display various items. This will be streamlined with the typical buttons. Maybe we want to allow the user to use Pdb's prompt alongside the buttons?
Current status:

  • Allow setting breakpoints
  • Allow running code in debug mode, passing along the breakpoints
  • Highlight the currently active line in the editor
  • Support the described actions with buttons
  • Display relevant local variables
  • Properly filter Pdb's output to prevent clutter

Other useful debuggers are e.g. Snoop and Birdseye. Snoop should be easy to add support for. Birdseye uses more visualisations so this would be more tricky to integrate directly.

@winniederidder winniederidder added the deploy This PR should be deployed. label Mar 12, 2022
@winniederidder winniederidder temporarily deployed to production March 12, 2022 21:10 Inactive
@github-actions github-actions bot removed the deploy This PR should be deployed. label Mar 12, 2022
@winniederidder winniederidder added the deploy This PR should be deployed. label Mar 20, 2022
@winniederidder winniederidder temporarily deployed to production March 20, 2022 23:00 Inactive
@github-actions github-actions bot removed the deploy This PR should be deployed. label Mar 20, 2022
@winniederidder
Copy link
Contributor Author

@alexmojaki We are thinking about adding support for Birdseye and Snoop. Do you have any caveats/suggestions on how this is best approached? I've taken a look at them and I have some concerns. For Birdseye, it seems to require a seperate rendering server, I'm not sure how easily this can be circumvented. Adding another worker also doesn't sound ideal (as this would be needed for standalone mode). Regarding Snoop, you also add extra functionalities in core/runner/snoop to run the code. What would the difference be with transforming the code in e.g.

@snoop
async def new_main():
    "\n".join(["    " + l for l in code.split("\n")]
await new_main()

and running that in Pyodide. I know it's not a perfectly working implementation but I'm curious how far I would get with that.
Thanks in advance!

@winniederidder winniederidder mentioned this pull request Mar 30, 2022
@alexmojaki
Copy link
Contributor

futurecoder uses a heavily cut down version of birdseye in alexmojaki/birdseye#91

That branch removes the server, database, and a bunch of other stuff that futurecoder doesn't need. It just stores all the data in memory, which futurecoder picks up and returns to the main thread as a big JSON dict. That gets stored locally using localForage. Finally a new tab is opened to display the birdseye UI, which retrieves the local data based on a key in the URL. The UI is just static files, there's no server or second worker. Local storage isn't necessarily the only option, e.g. postMessage to an iframe or something involving service workers might also work.

Integrating snoop should be much easier. The main thing that extra code in futurecoder does is recursively trace all functions in the 'file', while excluding internal stuff. One way to see the difference is to try your suggestion directly in futurecoder by running this code with the normal Run button:

import snoop

@snoop
def main():
    def foo():
        print('hi')
    
    foo()

main()

In particular you'll see a >>> Call to main but no similar call to foo. Now comment out @snoop and click the snoop button and you'll also see >>> Call to main.<locals>.foo. The point is not that foo is defined inside another function, but just that foo is a function at all. Your method would only trace the top-level code.

You should be able to copy all that code almost verbatim, although you should probably leave color disabled to start with. I think that code should actually be moved into python_runner as another mode, with room for some configuration like color. It should continue to only be imported when it's actually needed, especially so that python_runner doesn't depend on snoop. Finally I think it should output to a specialised OutputStream instead of sys.stdout, like what you did with pdb. That was a good idea, and keeping snoop's output separate would be useful for futurecoder.

@winniederidder
Copy link
Contributor Author

You should be able to copy all that code almost verbatim, although you should probably leave color disabled to start with. I think that code should actually be moved into python_runner as another mode, with room for some configuration like color. It should continue to only be imported when it's actually needed, especially so that python_runner doesn't depend on snoop. Finally I think it should output to a specialised OutputStream instead of sys.stdout, like what you did with pdb. That was a good idea, and keeping snoop's output separate would be useful for futurecoder.

How do you envision this then? Moving the execute-method of Futurecoder's EnhancedRunner to a new class in python_runner , moving core.runner.snoop/birdseye into the python_runner package and then calling exec_snoop/birdseye as usual? With then some extra tweaks to capture snoop-output to a separate stream? Or do you have a different idea?

@alexmojaki
Copy link
Contributor

You should be able to copy all that code almost verbatim, although you should probably leave color disabled to start with. I think that code should actually be moved into python_runner as another mode, with room for some configuration like color. It should continue to only be imported when it's actually needed, especially so that python_runner doesn't depend on snoop. Finally I think it should output to a specialised OutputStream instead of sys.stdout, like what you did with pdb. That was a good idea, and keeping snoop's output separate would be useful for futurecoder.

How do you envision this then? Moving the execute-method of Futurecoder's EnhancedRunner to a new class in python_runner , moving core.runner.snoop/birdseye into the python_runner package and then calling exec_snoop/birdseye as usual? With then some extra tweaks to capture snoop-output to a separate stream? Or do you have a different idea?

That's roughly it. Start with just snoop. It doesn't need another class, users will just call run with mode="snoop".

@winniederidder winniederidder changed the base branch from issue40/code-completion to main April 19, 2022 21:03
@winniederidder winniederidder added this to the later milestone May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants