classification
Title: tabs don't work correctly in python repl
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.6, Python 3.5
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 2016-10-29 14:14 by cpitclaudel. 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 <TAB> key in repl breaks the repl prompt.

Below is what I see when I hit <TAB> 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
2016-10-29 14:14:48cpitclaudelsetmessages: + msg279674
2016-10-29 04:14:27martin.pantersetmessages: + msg279652
2016-10-27 03:54:10cpitclaudelsetfiles: + completion.py
nosy: + cpitclaudel
messages: + msg279525

2016-03-02 10:33:05berker.peksagsetstatus: open -> closed
2016-02-04 19:09:26yselivanovsetmessages: + msg259591
2016-02-04 19:08:31python-devsetmessages: + msg259590
2016-02-04 17:24:45Yury.Selivanovsetnosy: + Yury.Selivanov
messages: + msg259580
2016-02-04 17:17:36berker.peksagsetmessages: + msg259579
2016-02-04 07:29:30martin.pantersetstatus: closed -> open

messages: + msg259538
2016-02-04 06:56:32berker.peksagsetmessages: + msg259535
2016-02-04 06:26:29yselivanovsetstatus: open -> closed
resolution: fixed
messages: + msg259533

stage: commit review -> resolved
2016-02-04 06:25:23python-devsetnosy: + python-dev
messages: + msg259532
2016-02-04 06:19:48ned.deilysetmessages: + msg259531
2016-02-04 02:51:03berker.peksagsetnosy: + berker.peksag

messages: + msg259526
stage: patch review -> commit review
2015-11-27 03:05:07martin.pantersetmessages: + msg255451
2015-11-24 15:09:35r.david.murraysetmessages: + msg255268
2015-11-24 07:32:04martin.pantersetmessages: + msg255251
2015-11-24 05:42:24martin.pantersetmessages: + msg255248
2015-11-23 17:54:36yselivanovsetnosy: + ncoghlan
2015-11-23 17:53:49yselivanovsetmessages: + msg255205
components: - macOS
2015-11-23 16:25:45r.david.murraysetmessages: + msg255181
2015-11-22 07:07:26larrysetpriority: release blocker -> normal
2015-11-22 07:07:19larrysetnosy: + larry
messages: + msg255085
2015-11-20 23:39:41yselivanovsetmessages: + msg255030
2015-11-20 23:27:50martin.pantersetpriority: high -> release blocker

messages: + msg255029
2015-11-20 23:27:24martin.pantersetpriority: release blocker -> high

messages: + msg255028
stage: patch review
2015-11-20 16:38:50yselivanovsetpriority: high -> release blocker

messages: + msg254998
2015-11-19 20:10:28yselivanovsetfiles: + rlcompleter.patch
keywords: + patch
messages: + msg254931
2015-11-19 01:32:33martin.pantersetmessages: + msg254871
2015-11-19 00:37:53martin.pantersetmessages: + msg254868
2015-11-19 00:20:37martin.pantersetmessages: + msg254866
2015-11-18 22:36:51yselivanovsetmessages: + msg254860
2015-11-18 22:27:23martin.pantersetnosy: + martin.panter
messages: + msg254859
2015-11-18 20:14:30r.david.murraysetnosy: + ronaldoussoren, ned.deily
messages: + msg254856

components: + macOS
type: behavior
2015-11-18 19:58:50yselivanovcreate