- Notifications
You must be signed in to change notification settings - Fork1.5k
Add REPL plugin hooks; Addoutput
,output-mode
,stderr
attributes#1106
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Apologies that this has stayed in draft for so long - I hope to get this completed and ready to review sometime this week. |
output
,output-mode
,stderr
attributes [WIP]output
,output-mode
,stderr
attributesThere 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.
Apologies for the LOOOOONG delay in reviewing this PR Jeff! Left some comments/suggestions and approving 😄
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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.
Just some minor comments here
🚀
Great work!
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
marimeireles commentedMar 7, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Oh no, a lot has changed 😕 |
@marimeireles@FabioRosado Finally got around to addressing the comments and merging from For some reason, the Circle CI tests are stuck on a mergability check from a much earlier commit, and won't run the most recent tests at all. Any ideas how to clear those? Everything's merged and passing tests (locally) as of right now, for what it's worth. If they won't clear, I'll open a fresh PR with this branch again I guess. |
No idea why@JeffersGlass, sorry I missed your comment here. |
marimeireles commentedMar 22, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Ok, I've rebased 👍 tests/integration/test_00_support.py ............... [ 8%]tests/integration/test_01_basic.py ................s. [ 18%]tests/integration/test_02_display.py ....................... [ 31%]tests/integration/test_03_element.py ............... [ 39%]tests/integration/test_async.py ....... [ 43%]tests/integration/test_importmap.py xX [ 44%]tests/integration/test_interpreter.py .... [ 47%]tests/integration/test_plugins.py ...F....... [ 53%]tests/integration/test_py_config.py ..FF........ [ 60%]tests/integration/test_py_repl.py ............F..F.FFFFFF [ 73%]tests/integration/test_py_terminal.py ....... [ 76%]tests/integration/test_splashscreen.py ...s.. [ 80%]tests/integration/test_stdio_handling.py ........... [ 86%]tests/integration/test_style.py .. [ 87%]tests/integration/test_warnings_and_banners.py. [ 88%]tests/integration/test_zz_examples.py F.....s............x. [100%] Will make sure I've merged everything correctly cause it was a bunch of files and then force push it here. |
This is very odd to me - I merged from main yesterday, all tests passing locally, and pushed. I think the merge conflicts here are potentially an old circle ci artifact that won’t close? Going to try opening a fresh PR from the branch and see what happens… |
Hey@JeffersGlass, that would also work! |
for more information, seehttps://pre-commit.ci
Ok great :) |
JeffersGlass commentedMar 22, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Sounds good@marimeireles! I suspect I had messed up the inital merge/rebase, but this one is at least free of merge conflicts now as well. Looks like tests are passing here, but linting is not. |
You can also see the local linting issues by running |
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.
Went over it again and it looks good!
Thanks for coming back to this one@JeffersGlass, great work! 😊
marimeireles left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Actually, there's one problem.. Let me check.
I'm looking into:https://github.com/pyscript/pyscript/actions/runs/4492024745/jobs/7901322111?pr=1106
That was introduced by me as I fixed linting.
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.
Okay, this is good now!
Amazing! I'll get it merged later today! |
Uh oh!
There was an error while loading.Please reload this page.
This PR does four major things
beforePyReplExec()
andafterPyReplExec()
, which which are called immediately before and after a py-repl tag is evaluated, respectively.output
attribute of <py-repl> tags. If provided, the Element with the ID specified by output is used as:sys.stdout
output-mode
attribute of <py-repl> tags. If theoutput-mode == 'append'
, the contents of the output element (either specified by theoutput
attribute or the default location as the next sibling of the REPL) arenot cleared before writing new contents. This isnot typical "REPL-like" behavior, but may be wanted if the user is already writing the REPL output somewhere else.(Removed in#881.)stderr
attribute for <py-repl> tags. If provided, the Element with the ID specified by the output is used as the location to send writes tosys.stderr
All writes to the above locations (using
output
orstderr
attributes) arein addition to writing to the py-temrinal / ioMultiplexer.This PR is a ready for review. The task list was:
beforePyReplExec()
,afterPyReplExec()
to to pluginsResolves#988,Fixes#1070,Resolves#975,Resolves#1074