Skip to content

Conversation

adqm
Copy link
Contributor

@adqm adqm commented Jul 3, 2025

I'm including a couple of fixes for the WASM demo, which for me made the demo "just work" out of the box for me with a build from following the instructions on this page:

  • Exported HEAPU32 at build time since python.worker.mjs expects it to be there (for determining the version number)
  • Updated the HTML to properly load xterm.js

I also made a few changes to try to improve the usability of the web REPL, also included here:

  • Reset things when starting a new process (without this change, running the same program multiple times would not work, requiring a refresh)
  • Added support for arrow key navigation (and home/end) in the REPL (including history scrollback)
  • Added support for TAB and CTRL+C, trying to mimic the behavior of the regular of those inputs from the Python REPL
  • Replaced the textarea with Ace code editor.
  • Moved the demo to index.html instead of python.html so that the URL shown in the terminal when running server.py takes you to the demo (without needing to append /python.html)

I also put up a demo here that shows the result of a fresh Emscripten build with these changes.

There's definitely still room for improvement here (for example, #124621 suggests updating the demo to use PyREPL), but I'm sharing these changes in case they're useful in the meantime.

Related Issue: #136251

@python-cla-bot
Copy link

python-cla-bot bot commented Jul 3, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Jul 3, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@adqm adqm requested a review from brettcannon as a code owner July 3, 2025 17:52
@bedevere-app
Copy link

bedevere-app bot commented Jul 3, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app
Copy link

bedevere-app bot commented Jul 3, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app
Copy link

bedevere-app bot commented Jul 3, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a full review, just saw some style problems :)

@brettcannon brettcannon removed their request for review July 3, 2025 21:01
@bedevere-app
Copy link

bedevere-app bot commented Jul 3, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@adqm
Copy link
Contributor Author

adqm commented Jul 3, 2025

thanks, @sobolevn! i just ran prettier on that file; it looks like it caught those style issues and quite a few more.

@bedevere-app
Copy link

bedevere-app bot commented Jul 3, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@hoodmane
Copy link
Contributor

I broke this on the main branch again (in one place I check for process incorrectly and cause a crash). In order to avoid breaking it we'll need to add an in-browser smoke test to the emscripten buildbot.

@hoodmane
Copy link
Contributor

Merged main and pushed a commit fixing the problem. I'll run the Emscripten buildbot on this and if it passes it is good to merge.

@hoodmane
Copy link
Contributor

!buildbot emscripten

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @hoodmane for commit 26ddd07 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136252%2Fmerge

The command will test the builders whose names match following regular expression: emscripten

The builders matched are:

  • WASM Emscripten PR

@hoodmane
Copy link
Contributor

Okay it needs the fix from #136909 to unbreak the buildbot...

@hoodmane
Copy link
Contributor

!buildbot emscripten

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @hoodmane for commit 7d33525 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136252%2Fmerge

The command will test the builders whose names match following regular expression: emscripten

The builders matched are:

  • WASM Emscripten PR

@ambv ambv merged commit d1d526a into python:main Jul 21, 2025
41 checks passed
@hoodmane hoodmane added OS-emscripten topic-repl Related to the interactive shell needs backport to 3.14 bugs and security fixes labels Jul 21, 2025
@miss-islington-app
Copy link

Thanks @adqm for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington-app
Copy link

Sorry, @adqm and @ambv, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker d1d526afe7ce62c787b150652a2ba136cb949d74 3.14

@hoodmane
Copy link
Contributor

Looks like first we have to backport #136822.

@ambv ambv added needs backport to 3.14 bugs and security fixes and removed needs backport to 3.14 bugs and security fixes labels Jul 22, 2025
@miss-islington-app
Copy link

Thanks @adqm for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 22, 2025
(cherry picked from commit d1d526a)

Co-authored-by: adam j hartz <[email protected]>
Co-authored-by: Hood Chatham <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jul 22, 2025

GH-136977 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jul 22, 2025
ambv pushed a commit that referenced this pull request Jul 22, 2025
(cherry picked from commit d1d526a)

Co-authored-by: adam j hartz <[email protected]>
Co-authored-by: Hood Chatham <[email protected]>
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
kumaraditya303 pushed a commit to miss-islington/cpython that referenced this pull request Sep 9, 2025
…) (pythonGH-136977)

(cherry picked from commit d1d526a)

Co-authored-by: adam j hartz <[email protected]>
Co-authored-by: Hood Chatham <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OS-emscripten topic-repl Related to the interactive shell

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants