Skip to content

gh-84995: Run sys.__interactivehook__() on asyncio REPL startup #20517

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 8 commits into from
Mar 1, 2024
Merged

gh-84995: Run sys.__interactivehook__() on asyncio REPL startup #20517

merged 8 commits into from
Mar 1, 2024

Conversation

remilapeyre
Copy link
Contributor

@remilapeyre remilapeyre commented May 29, 2020 •
edited by bedevere-bot
Loading

@remilapeyre remilapeyre requested review from 1st1 and asvetlov as code owners May 29, 2020 16:11
@remilapeyre remilapeyre changed the title Run sys.__interactivehook__() on asyncio REPL startup bpo-40818: Run sys.__interactivehook__() on asyncio REPL startup May 29, 2020
ZackerySpytz
ZackerySpytz reviewed May 31, 2020
View reviewed changes
@asvetlov
Copy link
Contributor

@1st1 could you review?

@remilapeyre remilapeyre mannequin mentioned this pull request Apr 10, 2022
Closed
@andriisoldatenko
Copy link

@1st1 is anything missing in this PR?

@AlexWaygood AlexWaygood closed this Jul 5, 2022
@AlexWaygood AlexWaygood reopened this Jul 5, 2022
@ghost
Copy link

ghost commented Jul 5, 2022 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@willingc
Copy link
Contributor

Hi @remilapeyre, Is this PR something you still wish to have open? If you would like to move it forward, we would need a signed CLA and merge conflicts resolved. Please close if you do not wish to continue working on this. Thanks.

willingc
willingc reviewed Apr 26, 2023
View reviewed changes
Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

Please sign CLA and resolve merge conflict. Thanks.

@furkanonder
Copy link
Contributor

@remilapeyre Could you resolve the conflicts?

@gvanrossum gvanrossum changed the title bpo-40818: Run sys.__interactivehook__() on asyncio REPL startup gh-84995: Run sys.__interactivehook__() on asyncio REPL startup Aug 23, 2023
@gvanrossum
Copy link
Member

Looks like OP lost interest (or was never notified). I'll add "Pending" label, if there's no response in 30 days just close the PR.

@gvanrossum gvanrossum added the pending The issue will be closed if no feedback is provided label Aug 23, 2023
@gvanrossum
Copy link
Member

@remilapeyre Please fix merge conflict.

@remilapeyre
Copy link
Contributor Author

Hello, I'm a bit short on time to contribute but will try to put aside some time in the coming weeks to revive the PRs

@gvanrossum
Copy link
Member

@willingc Can i leave this to you?

@itamaro itamaro mentioned this pull request Feb 29, 2024
Closed
@itamaro
Copy link
Contributor

itamaro commented Feb 29, 2024

gh-116095 is a "re-do" of this PR on top of main (sorry, I don't have access to update this PR directly, but I have credited @remilapeyre as the original author)

itamaro
itamaro reviewed Feb 29, 2024
View reviewed changes
gvanrossum
gvanrossum reviewed Feb 29, 2024
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I like the feature and I like the implementation, but I have a few suggestions to reduce the risk of accidentally breaking code that was relying on implementation details of site.py. (Note that I am fine with moving register_readline() to the toplevel scope in site.py.)

Lib/site.py Outdated
Comment on lines 460 to 461
def register_readline():
"""Enable default readline configuration on interactive prompts.
Copy link
Member

Choose a reason for hiding this comment

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

Technically, this function is the "interactive hook". The docstring should probably clarify that.

Also, even though it isn't documented, I think we should maintain a function named enablerlcompleter() which, when calls, does

sys.__interactivehook__ = register_readline

It's likely that someone, somewhere is relying on that function existing, and it's only 2-3 lines to keep them happy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added enablerlcompleter() back and changed the wording in the docstrings. Should I add enablerlcompleter() and register_readline() to the documentation?

remilapeyre and others added 2 commits March 1, 2024 09:50
@remilapeyre @itamaro
Improve wording in News
5f60485 
Co-authored-by: Itamar Oren <itamarost@gmail.com>
@remilapeyre
Address code review comments
caaa7c7
@remilapeyre remilapeyre requested a review from gvanrossum March 1, 2024 09:13
gvanrossum
gvanrossum approved these changes Mar 1, 2024
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Thanks! I'll merge it now. I wouldn't add more docs about this to site.rst, it's intentionally a bit vague.

@gvanrossum gvanrossum merged commit b5949ea into python:main Mar 1, 2024
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
@remilapeyre @willingc
@itamaro @woodruffw
pythongh-84995: Run sys.__interactivehook__() on asyncio REPL startup (…
6331c9c 
…python#20517)

This makes the asyncio REPL (`python -m asyncio`) more usable
and similar to the regular REPL.

This exposes register_readline() as a top-level function in site.py,
but it's intentionally undocumented. 

Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
Co-authored-by: Itamar Oren <itamarost@gmail.com>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@remilapeyre @willingc
@itamaro @adorilson
pythongh-84995: Run sys.__interactivehook__() on asyncio REPL startup (…
10d1c62 
…python#20517)

This makes the asyncio REPL (`python -m asyncio`) more usable
and similar to the regular REPL.

This exposes register_readline() as a top-level function in site.py,
but it's intentionally undocumented. 

Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
Co-authored-by: Itamar Oren <itamarost@gmail.com>
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@remilapeyre @willingc
@itamaro @diegorusso
pythongh-84995: Run sys.__interactivehook__() on asyncio REPL startup (…
605bbe2 
…python#20517)

This makes the asyncio REPL (`python -m asyncio`) more usable
and similar to the regular REPL.

This exposes register_readline() as a top-level function in site.py,
but it's intentionally undocumented. 

Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
Co-authored-by: Itamar Oren <itamarost@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itamaro itamaro itamaro left review comments

@gvanrossum gvanrossum gvanrossum approved these changes

@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner

@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner

@ZackerySpytz ZackerySpytz Awaiting requested review from ZackerySpytz

@willingc willingc Awaiting requested review from willingc willingc is a code owner

Assignees
No one assigned
Labels
topic-asyncio
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.