Skip to content

Enhance MicroPython Terminal on both Main and Worker #2083

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
May 30, 2024
Merged

Enhance MicroPython Terminal on both Main and Worker #2083

merged 4 commits into from
May 30, 2024

Conversation

WebReflection
Copy link
Contributor

@WebReflection WebReflection commented May 28, 2024 •
edited
Loading

Description

This MR allows MicroPython terminal on the main thread via import code; code.interact() same mechanism used to enable it on workers.

It is based on the following assumptions / constraints:

  • the input is delegated to the JS prompt ... so it's blocking on main ... everything looks fine on worker
  • the cursor is blinking regardless as it is for workers related use cases where eventually code.interact() might be invoked
  • the warning about main being incapable of a terminal is removed on MicroPython only use case
  • nothing changed at all for Pyodide as it doesn't offer a similar functionality (AFAIK, please correct me if I am wrong)
  • previous use cases should be all covered as it's been tested to still work as a regular main terminal
  • to make it work, the whole raised error on main input, which is default in PyScript, is explicitly removed when MicroPython terminal is meant to be used
  • on main, if a loop contains time or utime and there is a sleep, the whole thing is just blocked until the sum of sleeps is completed and the output is shown only after ... on workers, everything works as expected

On both main and worker, these are the enhancements:

  • tab completion
  • history (arrows up and down)
  • paste mode like in the real REPL
  • CTRL+C or CTRL+D work too

/cc @ntoll

Changes

  • bring in discoveries in polyscript around Xterm.js and its ability to offer a full terminal on both main and worker via streams
  • test everything works as expected in both main and workers
  • export the invalidInput string to be able to remove it from hooks when a mpy terminal is found on the page
  • removed ReadLine dependency on MicroPython only use cases
  • split the two similar but not really terminal and lazy loaded each one on demand
  • updated polyscript to its latest with latest Pyodide release too

Checklist

  • All tests pass locally
  • I have updated CHANGELOG.md
  • I have created documentation for this(if applicable)

@WebReflection WebReflection requested a review from ntoll May 28, 2024 14:51
@WebReflection
Copy link
Contributor Author

WebReflection commented May 29, 2024 •
edited
Loading

OK ... after yesterday demo I think I could rewrite this whole thing in a better way and provide just two different terminals, one for Pyodide and one for MicroPython.

The branching logic is a bit overwhelming and this plugin is now too "spaghetti" to me so I might just put this on hold (as draft) and provide a definitive one that makes MicroPython terminal story the best we can have.

@ntoll does this sound reasonable to you? I really don't like the current state of the file and the fact terminal in worker is actually inferior somehow to what we could have instead ...

@WebReflection WebReflection marked this pull request as draft May 29, 2024 11:30
@WebReflection WebReflection changed the title Allow MicroPython Terminal on Main Enhance MicroPython Terminal on both Main and Worker May 29, 2024
@WebReflection WebReflection marked this pull request as ready for review May 29, 2024 15:24
@WebReflection
Copy link
Contributor Author

@ntoll this is final now and tested with both MicroPython (new features) and Pyodide (no regressions).

You can test this live as it's already on npm https://webreflection.github.io/coincident/test/pyterminal/

This also updates polyscript in a way that satisfies the worker.ready requirement without breaking.

I think we should release this 😇

@ntoll
Copy link
Member

ntoll commented May 30, 2024

@WebReflection ok... I've just looked through this. I agree with your "spaghetti" thoughts. This isn't the spaghetti mama used to make... 😋 😉 I notice you've already split out the terminal into a mpy and py version. How much more work would be needed for you to be happy with the state of the code..?

I also agree that having the terminal on main (as well as the worker) for MicroPython is a good thing, with further (later) refinement for Pyodide to come. I think it's important simply because stuff like tab completion, history and all the other good stuff is rather fun to have available for those who simply want a quick REPL for MicroPython. I'm forever going to pyscript.net just to type stuff in to check MicroPython things. ;-)

@WebReflection
Copy link
Contributor Author

How much more work would be needed for you to be happy with the state of the code..?

This is already published on npm, I am OK with the current state of the code. There's some inevitable repetition but the split makes it less bloated anyway and unfortunately the stream dance is never too elegant but super effective.

The onData on the main to avoid blocking on input also is necessary so I think I can't clean up more than this but overall it feels great for what I could test.

ntoll
ntoll approved these changes May 30, 2024
View reviewed changes
Copy link
Member

@ntoll ntoll left a comment

Choose a reason for hiding this comment

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

:shipit:

@ntoll
Copy link
Member

ntoll commented May 30, 2024

OK. Let's get this out to the world, and we can refine our spaghetti recipe moving forwards. 👍 🇮🇹 🍝 👨‍🍳

@WebReflection WebReflection merged commit 9b775ce into pyscript:main May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ntoll ntoll ntoll approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@WebReflection @ntoll