-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
[3.12] gh-109151: Enable readline in the sqlite3 CLI (GH-109152) #110352
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
Conversation
(cherry picked from commit 254e30c) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
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.
@serhiy-storchaka: This change will land in 3.12.1 bugfix, does it server a Changelog entry? I'm fine with no Changelog entry.
@erlend-aasland or @serhiy-storchaka: Feel free to merge the change.
I do not see anything wrong in the Changelog entry. |
Oh sorry, I missed the fact that you already added a Changelog entry :-) |
Merged, thanks. |
Hang on, this is a new feature. Why should it be included in 3.12.1? |
IMO it's minor enough to tolerate it after 3.12.0 release. There is no risk of regression: there is a try/exception, and try/except ImportError to import readline is common. Also, readline is really pleasant to use :-) Well, that's my rationale. |
It is a borderline between a new feature and a bugfix. It is not the first time when a new feature (the sqlite CLI in this case) was polished after release. |
It's really not borderline in my eyes. It's clearly a new feature and one that can be problematic, since importing readline can produce more than just ImportError (especially since it's readline, which wraps a C library of somewhat questionable predictability). Is the lack of readline a problem? How big of a problem? Why wasn't it caught before beta 1? |
It is not critical problem, but it makes the sqlite3 REPL less useful. Try to use Python REPL without readline, it always annoyed me that Python 2.7 did not import it by default. It is not a big problem to me on Linux, because I always use an external readline wrapper, but I think that other users would see the sqlite3 REPL as half-baked without readline support. It wasn't caught before beta 1 because before #109108 I did not know that the CLIs of some of other modules support readline (unlike the raw InteractiveConsole) and that it is so easy to enable it. |
It's mostly a usability issue. Try Python REPL without readline to have an idea. Example when I try to get back with LEFT key to fix a typo:
When I try to navigate in the history with UP key:
Well, the Python REPL remains usable, but honestly, it's way less pleasant to use. |
Yes. IMO it is a usability bug, which is why I think it is fine to backport it. |
I'm sorry, but no. This is a new feature, and shouldn't have been backported to 3.12. I'm fully aware of the consequence of not having readline (I've used interactive Python on more than a few systems without readline support over the years), but importing readline has more consequences than just "cursor keys start working". If the missing readline was a big usability problem, it should have been caught long before rc1, let alone the final release. It can easily break things, and this affects the whole Python release: if any change to 3.12.1 is risky, the entirely 3.12.1 upgrade becomes risky. We make backward compatibility guarantees for that very reason: we need people to be comfortable upgrading Python to new patch versions, so we have to avoid risks like these. |
…ythonGH-109152) (python#110352)" This reverts commit bc1fe35.
I am sorry. I did not think that this change can be controversial. I'll revert it. |
…ythonGH-109152) (python#110352)" This reverts commit bc1fe35.
(cherry picked from commit 254e30c)
Co-authored-by: Serhiy Storchaka storchaka@gmail.com