-
Notifications
You must be signed in to change notification settings - Fork 908
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
Web playground with WASM #1279
Web playground with WASM #1279
Conversation
37f4d3a
to
6797541
Compare
Nice, this would be cool to have. I published https://ruff.pages.dev/ at some point based on that code, it'd be nice to have that continuously deployed, more prominent, officially part of the codebase with WASM support, etc. Very supportive of this! |
9de5bfd
to
bb3828c
Compare
762b090
to
b6088c3
Compare
this is awesome, testing it in a web app with codemirror and works great! thanks a lot @squiddy and @charliermarsh |
2e1cdc8
to
3a37b67
Compare
Marking as ready. I updated the MR description with some more details. Sharing config & code via URL works now: Example I thought that's nice to have, as it can make sharing bug reports nicer? E.g. for the false-positive RET504 |
Awesome, pumped to review! Will try to get to it today but might be tomorrow. (Permalinks is genius especially for bug reports.) |
.github/workflows/ci.yaml
Outdated
@@ -146,3 +152,42 @@ jobs: | |||
${{ runner.os }}-build- | |||
${{ runner.os }}- | |||
- run: maturin build -b bin | |||
|
|||
# TODO(rgerecke): What part should run on MR, and what only on main? | |||
playground_build: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this should be moved to its own workflow, that runs on main
update, and continuously releases the playground?
The ideal would probably be that we build and deploy a preview URL to Pages on every PR, and that URL gets linked back to the PR via a comment or as the hyperlink on the check completion. But, that doesn't have to happen here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved it to separate workflow.
I agree on the ideal, but I would prefer to get this into a merge-able state first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Sounds good, I know I'm a bit behind on this!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, it's all good. :) I will, from time to time, rebase this to resolve conflicts, but probably not daily
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm gonna go ahead and resolve conflicts, then merge. There are some things I want to tweak, but better to get this merged and run from there! I'll also setup the deployment keys.
padding: 1em; | ||
min-width: 300px; | ||
border-right: 1px solid lightgray; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might spend some time re-styling this, to make the UI more cohesive between the settings pane and the editor. I think the settings should have some kind of show/hide UX too -- perhaps they should even be hidden by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, that sounds reasonable. Styling isn't my strong skill.
Just let me know when you want to tackle that, I'll address your other comments for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's also lots of potential in making the user input nicer, based on the type of the option. E.g. presenting checkboxes for booleans. I didn't go into that to keep the PR smallish.
pub mod updates; | ||
|
||
mod lib_native; | ||
pub use lib_native::check; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what you have here makes sense.
It would also be cool to have permanent, versioned URLs somehow. I don't know exactly what that would look like, but for bug reports, it'd be nice if the URL that was shared was tied to a specific version of Ruff. |
This is necessary to reduce a large (likely generated) function that exceeds the maximum number of locals that the wasm-bindgen tool can handle. https://github.com/bytecodealliance/wasm-tools/blob/b5c3d98e40590512a3b12470ef358d5c7b983b15/crates/wasmparser/src/limits.rs#L29
b0cf8d2
to
0784e45
Compare
I was wondering whether it makes sense to decouple the playground from ruff itself. Have it provide the UI and all of that, but load the WASM files from somewhere depending on the version requested. If we were to include the available options inside the WASM, they would be pretty self-contained. A link such as https://611ef32d.ruff-axw.pages.dev/?version=0.0.119#N4XwJBCWC2AOD2AnALgAngZwHQHcAWAhsgKYBuxiQA would then load the proper WASM file for |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ruff](https://togithub.com/charliermarsh/ruff) | `^0.0.194` -> `^0.0.195` | [![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.195/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.195/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.195/compatibility-slim/0.0.194)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.195/confidence-slim/0.0.194)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>charliermarsh/ruff</summary> ### [`v0.0.195`](https://togithub.com/charliermarsh/ruff/releases/tag/v0.0.195) [Compare Source](https://togithub.com/charliermarsh/ruff/compare/v0.0.194...v0.0.195) #### What's Changed - Add support for `ruff.toml` by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1378 - Update rust python to handle files with BOM by [@​squiddy](https://togithub.com/squiddy) in [astral-sh/ruff#1379 - Only re-associate inline comments during normalization when necessary by [@​squiddy](https://togithub.com/squiddy) in [astral-sh/ruff#1380 - Magic Trailing Commas in isort by [@​colin99d](https://togithub.com/colin99d) in [astral-sh/ruff#1363 - Web playground with WASM by [@​squiddy](https://togithub.com/squiddy) in [astral-sh/ruff#1279 - Enable preview deployments for playground by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1383 - Add ESLint, Prettier, and TypeScript checks by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1384 - Add badge to playground by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1393 - Choose a more interesting example snippet by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1394 - Enable Quick Fix in the playground by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1395 - Only run playground release in main repo by [@​charliermarsh](https://togithub.com/charliermarsh) in [astral-sh/ruff#1396 - Now replace typing.Text with str by [@​colin99d](https://togithub.com/colin99d) in [astral-sh/ruff#1391 **Full Changelog**: astral-sh/ruff@v0.0.194...v0.0.195 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/ixm-one/pytest-cmake-presets). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC43My4zIiwidXBkYXRlZEluVmVyIjoiMzQuNzMuMyJ9--> Signed-off-by: Renovate Bot <bot@renovateapp.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This MR provides a web playground for ruff. https://611ef32d.ruff-axw.pages.dev/
Technical details:
Not all dependencies compile under the wasm target (or make sense to even include). I thought it best to make that explicit by splitting up the lib.rs between native and wasm.
For main.rs I didn't find any other solution, apparantely one can not tell cargo to not build a binary under certain targets. The current main makes use of rayon and the current iterator wrapper breaks code check / clippy. I couldn't think of a good use case for WASM to have a direct entrypoint, so currently under wasm I provide a noop implementation.
Similar to @charliermarsh's original implementation this is based on vite & monaco-editor. The checks currently fail because I included a step to deploy the final result to cloudflare pages. Which obviously doesn't work due to the lack of secrets, and it also needs to be moved to some other main-only workflow.
This powers the configuration in the playground. I'm keeping an eye on the current work on the JSON schema. Potentially that schema can replace the dev command I'm introducing here.
More can be done on the playground itself, but I'd appreciate some reviews, especially on the wasm side of things. Perhaps some of this can be merged and we iterate on the playground in future MRs.