Issue 25660: tabs don't work correctly in python repl (original) (raw)

Issue25660

process

Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Yury.Selivanov, berker.peksag, cpitclaudel, larry, martin.panter, ncoghlan, ned.deily, python-dev, r.david.murray, ronaldoussoren, serhiy.storchaka, yselivanov
Priority: normal Keywords: patch

Created on 2015-11-18 19:58 by yselivanov, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
rlcompleter.patch yselivanov,2015-11-19 20:10 review
completion.py cpitclaudel,2016-10-27 03:54 Emacs' completion code
Messages (32)
msg254855 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-11-18 19:58
When Python is compiled with readline, repeatedly pressing key in repl breaks the repl prompt. Below is what I see when I hit 4 times. yury@ysmac ~/dev/py/cpython $ ./python.exe Python 3.5.0+ (3.5:4ae62ddf7bc7+, Nov 18 2015, 14:52:57) [GCC 4.2.1 Compatible Apple LLVM 7.0.0 (clang-700.1.76)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> >>> >>> >>> Reproducible when Python is built from source on latest OS X. Works OK when installed from MacPorts. Further, if I add PyOS_ReadlineFunctionPointer = PyOS_StdioReadline; in PyOS_Readline in myreadline.c, everything works as expected, so I think this is a readline bug.
msg254856 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-11-18 20:14
This was reported (on twitter) by David Beazly as well, as applying to the OSX downloaded from python.org. It works fine on linux for me. Is it readline-version dependent, or a bug in the readline shipped with OSX?
msg254859 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-18 22:27
Is this related to the BSD editline library (libedit), or is the actual Gnu Readline library being used? Apparently you can check for "libedit" in readline.__doc__ to tell the difference.
msg254860 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-11-18 22:36
> Is this related to the BSD editline library (libedit), or is the actual Gnu Readline library being used? Apparently you can check for "libedit" in readline.__doc__ to tell the difference. Great suggestion. So on my machine, macports version (no bug) is built against readline; manual build from source -- libedit. So apparently, this is a libedit thing.
msg254866 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-19 00:20
David Beazley’s description of the bug: “hitting tab inserts spaces and a carriage return at the same time.”
msg254868 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-19 00:37
Applying my patch at Issue 13501, then building with “autoreconf && ./configure --with-readline=editline”, I can reproduce the funny tab behaviour on Linux.
msg254871 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-19 01:32
I think this might just be a side effect of the way we abuse the tab completer to insert a literal tab (Issue 23441, revision 82ccdf2df5ac). If I change the code to insert the letter T instead of tabs: if not text.strip('T'): if state == 0: return text + 'T' else: return None I see this behaviour: * Manually type three Ts * Press Tab once, a fourth T is added nicely * Press Tab a second time, it beeps and displays a completion list with a single item, and then completes my line to five Ts * Pressing Tab again repeats the beep, completion list, and appending a T Illustration: >>> TTTT <== Typed T three times, then Tab twice TTTTT <== Completion list from second Tab press >>> TTTTT TTTTTT >>> TTTTTT
msg254931 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-11-19 20:10
Attached is a patch that uses different logic for tabulation. Instead of returning '\t' from the completion callback, it instead calls explicit readline API: "readline.insert_text('\t'); readline.redisplay()" Please review.
msg254998 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-11-20 16:38
Can someone try the patch? I'm fairly confident it should work, and it'd be great to have it fixed in 3.5.1.
msg255028 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-20 23:27
Sorry to throw a potential spanner in the works, but I think this introduces a dependence on the readline module. Consider what happens when the Completer class is used but Readline is not, or even if the “readline” could not be imported (see bottom of rlcompleter.py). Maybe there is a way around this, but it might involve more complicated code.
msg255029 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-20 23:27
Didn’t mean to adjust priority
msg255030 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-11-20 23:39
> Sorry to throw a potential spanner in the works, but I think this introduces a dependence on the readline module. Consider what happens when the Completer class is used but Readline is not, or even if the “readline” could not be imported (see bottom of rlcompleter.py). Hm... This module provides a "Completer" class **specifically** for readline, as its documentation (and name!) indicates.
msg255085 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-11-22 07:07
The "release blocker" priority level is inappropriate for nice-to-have features. Quoting PEP 101, the how-to-make-a-Python-release guide: release blocker - Stops the release dead in its tracks. You may not make any release with any open release blocker bugs. While I'd like to see this fixed too, I'm not holding up the release for it.
msg255181 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-11-23 16:25
The thing is, when it is a regression it should be fixed before the release is issued, even if it is a more "minor" issue. Otherwise, especially in a x.y.1 release, we look pretty bad. However, this one had been broken in the field so long that rule doesn't really apply, I think :) At least it is better in 3.5 and only affects one platform now, instead of all of them. If people didn't have time to get it fixed and committed before your deadline, that's too bad, I'm afraid :(. David Beazly is going to be pissed, though...
msg255205 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-11-23 17:53
> At least it is better in 3.5 and only affects one platform now, instead of all of them. No, it affects any platform where CPython is compiled with libedit instead of readline. And even if it was one platform - OS X is big, bigger than Windows probably in terms of Python users. > If people didn't have time to get it fixed and committed before your deadline, that's too bad, I'm afraid :(. I have a fix (attached to the issue). I think it's OK, but I need someone else to test it and review it. Martin, does the patch fix the problem on Linux with libedit? Do we use libedit on Windows?
msg255248 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-24 05:42
Regarding the purpose of the “Readline completer” module, I agree it is inconvenient. But the documentation <https://docs.python.org/3.5/library/rlcompleter.html> does say “without readline, the Completer class . . . can still be used”. I ran into this problem in Issue 25419 where Serhiy pointed it out. OS X is apparently the main platform where Editline is supported. There are even #ifdef __APPLE__ bits to work around bugs. However, people also seem to use Editline on Free BSD. Testing your current patch on Linux + Editline, it is worse than the 3.5 behaviour. Pressing Tab once appears to work, until you type the next key. It seems to invoke the history search mode (normally done with Ctrl+R), except the “bck:” indicator is not immediately shown. So the next key brings up an old history line, rather than being inserted directly. But if this works fine on OS X’s Editline, then maybe my setup is not right. I also see annoying buffering or flushing problems with my Editline setup. I am using Arch Linux’s libedit-20150325_3.1-1 package. Testing with Linux + Gnu Readline, it seems to work as advertised. I haven’t heard of Editline or Gnu Readline being used on Windows, certainly not in normal setups.
msg255251 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-24 07:32
It seems to be the readline.redisplay() line that triggers the search mode. If I comment out that line, I am back to the 3.5 behaviour that adds extra lines.
msg255268 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-11-24 15:09
We try our best to support libedit, but support for it did come after readline, so there are issues with the support. However, if we are (as we are) enabling completion by default, I think that has to work with libedit, since that's what is used on OSX which, as Yury said, is a major platform for Python. IIRC, on windows one has to install something 3rd party (pyreadline?) to get readline support.
msg255451 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-11-27 03:05
Yury or anyone else: can you confirm if the patch works properly with a proper OS X or BSD libedit (rather than my questionable Linux version?). If so, I think it is okay to ignore my Linux problems, and I could look at fixing the compatibility when the readline module is not available. Another way to look at this may be to fix the Editline (libedit) library to more faithfully emulate the Gnu version.
msg259526 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-02-04 02:51
With rlcompleter.patch applied, I see the same behavior on my OS X (with libedit) and Ubuntu (with readline) systems. Thanks!
msg259531 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2016-02-04 06:19
LGTM on OS X with both the system libedit and a MacPorts GNU readline. Let's apply this.
msg259532 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-02-04 06:25
New changeset 64417e7a1760 by Yury Selivanov in branch '3.5': Issue #25660: Fix TAB key behaviour in REPL. https://hg.python.org/cpython/rev/64417e7a1760 New changeset 87dfadd61e0d by Yury Selivanov in branch 'default': Merge 3.5 (issue #25660) https://hg.python.org/cpython/rev/87dfadd61e0d
msg259533 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-02-04 06:26
> Let's apply this. Merged. Martin, Berker, and Ned, thanks for testing this patch out.
msg259535 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-02-04 06:56
Thanks, Yury! Look like we forgot to update test_rcompleter: ====================================================================== FAIL: test_complete (test.test_rlcompleter.TestRlcompleter) ---------------------------------------------------------------------- Traceback (most recent call last): File "/ssd/buildbot/buildarea/3.5.gps-ubuntu-exynos5-armv7l/build/Lib/test/test_rlcompleter.py", line 82, in test_complete self.assertEqual(completer.complete('', 0), '\t') AssertionError: '' != '\t' + http://buildbot.python.org/all/builders/ARMv7%20Ubuntu%203.5/builds/576/steps/test/logs/stdio
msg259538 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-04 07:29
See also the Windows buildbots, where the Readline module is not available, but “rlcompleter” still (used to) work.
msg259579 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-02-04 17:17
Windows issue is a blocker so I suggest reverting 64417e7a1760. We could apply the same fix in readline.c for 3.5.x but it would probably be a bit hackish.
msg259580 - (view) Author: Yury Selivanov (Yury.Selivanov) * Date: 2016-02-04 17:24
Berker, I'll fix the windows issue in a few hours. Sorry for breaking things.
msg259590 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-02-04 19:08
New changeset 980ea968444c by Yury Selivanov in branch '3.5': Issue #25660: Fix a unittest and rlcompleter when readline isn't available https://hg.python.org/cpython/rev/980ea968444c
msg259591 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-02-04 19:09
Everything should be OK now (both broken tests and using rlcompleter on Windows). Please confirm.
msg279525 - (view) Author: Clément (cpitclaudel) Date: 2016-10-27 03:54
Could this commit be the reason why the attached code behaves differently in 2.7 and 3.5.2? This is the code used by Emacs' default Python mode to do completion; with it (python -i completion.py), pressing "tab" on a plain prompt offers candidates in Python 2.7, but it doesn't in Python 3.5.2 (instead, it inserts a tab).
msg279652 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-10-29 04:14
Clément: Yes, that sounds like the intended behaviour, at least when using Readline. Originally discussed in Issue 23441 and Issue 22086. (Though I think the change in behaviour when completion is called outside of Readline is a bug.)
msg279674 - (view) Author: Clément (cpitclaudel) Date: 2016-10-29 14:14
Thanks Martin for the clarification! I'll leave it to you to decide whether there is something to fix here (I'll admit to not understanding much of that code).
History
Date User Action Args
2022-04-11 14:58:23 admin set github: 69846
2016-10-29 14:14:48 cpitclaudel set messages: +
2016-10-29 04:14:27 martin.panter set messages: +
2016-10-27 03:54:10 cpitclaudel set files: + completion.pynosy: + cpitclaudelmessages: +
2016-03-02 10:33:05 berker.peksag set status: open -> closed
2016-02-04 19:09:26 yselivanov set messages: +
2016-02-04 19:08:31 python-dev set messages: +
2016-02-04 17:24:45 Yury.Selivanov set nosy: + Yury.Selivanovmessages: +
2016-02-04 17:17:36 berker.peksag set messages: +
2016-02-04 07:29:30 martin.panter set status: closed -> openmessages: +
2016-02-04 06:56:32 berker.peksag set messages: +
2016-02-04 06:26:29 yselivanov set status: open -> closedresolution: fixedmessages: + stage: commit review -> resolved
2016-02-04 06:25:23 python-dev set nosy: + python-devmessages: +
2016-02-04 06:19:48 ned.deily set messages: +
2016-02-04 02:51:03 berker.peksag set nosy: + berker.peksagmessages: + stage: patch review -> commit review
2015-11-27 03:05:07 martin.panter set messages: +
2015-11-24 15:09:35 r.david.murray set messages: +
2015-11-24 07:32:04 martin.panter set messages: +
2015-11-24 05:42:24 martin.panter set messages: +
2015-11-23 17:54:36 yselivanov set nosy: + ncoghlan
2015-11-23 17:53:49 yselivanov set messages: + components: - macOS
2015-11-23 16:25:45 r.david.murray set messages: +
2015-11-22 07:07:26 larry set priority: release blocker -> normal
2015-11-22 07:07:19 larry set nosy: + larrymessages: +
2015-11-20 23:39:41 yselivanov set messages: +
2015-11-20 23:27:50 martin.panter set priority: high -> release blockermessages: +
2015-11-20 23:27:24 martin.panter set priority: release blocker -> highmessages: + stage: patch review
2015-11-20 16:38:50 yselivanov set priority: high -> release blockermessages: +
2015-11-19 20:10:28 yselivanov set files: + rlcompleter.patchkeywords: + patchmessages: +
2015-11-19 01:32:33 martin.panter set messages: +
2015-11-19 00:37:53 martin.panter set messages: +
2015-11-19 00:20:37 martin.panter set messages: +
2015-11-18 22:36:51 yselivanov set messages: +
2015-11-18 22:27:23 martin.panter set nosy: + martin.pantermessages: +
2015-11-18 20:14:30 r.david.murray set nosy: + ronaldoussoren, ned.deilymessages: + components: + macOStype: behavior
2015-11-18 19:58:50 yselivanov create