Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
JeffersGlass merged 29 commits intopyscript:mainfromJeffersGlass:pyrepl-hooks
Mar 23, 2023

Conversation

JeffersGlass
Copy link
Contributor

@JeffersGlassJeffersGlass commentedJan 11, 2023
edited
Loading

This PR does four major things

  • Add two new plugin lifecycle methods,beforePyReplExec() andafterPyReplExec(), which which are called immediately before and after a py-repl tag is evaluated, respectively.
  • Restore theoutput attribute of <py-repl> tags. If provided, the Element with the ID specified by output is used as:
    • The location to send writes tosys.stdout
    • The location where the result of evaluating the REPL (if any) is display()'d
  • Restore theoutput-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.)
  • Add astderr 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 (usingoutput orstderr attributes) arein addition to writing to the py-temrinal / ioMultiplexer.


This PR is a ready for review. The task list was:

  • AddbeforePyReplExec(),afterPyReplExec() to to plugins
  • Expand StdioDirector plugin to accommodate new <py-repl> attributes
  • Integration Tests:
    • New integration tests for new attributes
    • Update old plugin integration tests with new attributes
  • Documentation
  • Changelog Entries
  • Review for completeness and correctness

Resolves#988,Fixes#1070,Resolves#975,Resolves#1074

@JeffersGlass
Copy link
ContributorAuthor

Apologies that this has stayed in draft for so long - I hope to get this completed and ready to review sometime this week.

marimeireles reacted with heart emoji

@JeffersGlassJeffersGlass added tag: repl tag: pluginsRelated to the infrastructure of how plugins work, or to specific plugins. labelsJan 27, 2023
@JeffersGlassJeffersGlass self-assigned thisJan 27, 2023
@JeffersGlassJeffersGlass marked this pull request as ready for reviewFebruary 6, 2023 15:11
@JeffersGlassJeffersGlass changed the titleAdd REPL plugin hooks; Addoutput,output-mode,stderr attributes [WIP]Add REPL plugin hooks; Addoutput,output-mode,stderr attributesFeb 6, 2023
Copy link
Contributor

@FabioRosadoFabioRosado left a 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 😄

Copy link
Contributor

@marimeirelesmarimeireles left a 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!

@marimeireles
Copy link
Contributor

marimeireles commentedMar 7, 2023
edited
Loading

Oh no, a lot has changed 😕
@JeffersGlass lmk if you want I can take this over and rebase after I finish the event stuff.

@JeffersGlass
Copy link
ContributorAuthor

@marimeireles@FabioRosado Finally got around to addressing the comments and merging frommain! Would you mind doing a re-review?

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.

@marimeireles
Copy link
Contributor

No idea why@JeffersGlass, sorry I missed your comment here.
To me it shows a bunch of unresolved conflicts.
Guess I'll give it a try on these :)

@marimeireles
Copy link
Contributor

marimeireles commentedMar 22, 2023
edited
Loading

Ok, I've rebased 👍
Got quite a few failing tests.

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.

@JeffersGlass
Copy link
ContributorAuthor

Got quite a few failing tests.

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…

@marimeirelesmarimeireles mentioned this pull requestMar 22, 2023
3 tasks
@marimeireles
Copy link
Contributor

Hey@JeffersGlass, that would also work!
I've made a fresh PR here if you think it's easier and if it looks correct for you:#1301
But if the tests were passing locally to you then I might have merged something wrong... I was checking and it seems fine but will wait for you to come back to me so we don't dup. work.

@marimeireles
Copy link
Contributor

Ok great :)
This looks great!
I'll close my dupped PR and we continue from here =)

@JeffersGlass
Copy link
ContributorAuthor

JeffersGlass commentedMar 22, 2023
edited
Loading

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.

@marimeireles
Copy link
Contributor

You can also see the local linting issues by runningpre-commit run --all-files.

JeffersGlass reacted with hooray emoji

Copy link
Contributor

@marimeirelesmarimeireles left a 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! 😊

@marimeirelesmarimeireles self-requested a reviewMarch 22, 2023 17:53
Copy link
Contributor

@marimeirelesmarimeireles left a comment
edited
Loading

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.

Copy link
Contributor

@marimeirelesmarimeireles left a 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!

@JeffersGlass
Copy link
ContributorAuthor

Amazing! I'll get it merged later today!

@JeffersGlassJeffersGlass merged commitef793ae intopyscript:mainMar 23, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@marimeirelesmarimeirelesmarimeireles approved these changes

@FabioRosadoFabioRosadoAwaiting requested review from FabioRosado

Assignees

@JeffersGlassJeffersGlass

Labels
tag: pluginsRelated to the infrastructure of how plugins work, or to specific plugins.tag: repl
Projects
None yet
Milestone
No milestone
3 participants
@JeffersGlass@marimeireles@FabioRosado

[8]ページ先頭

©2009-2025 Movatter.jp