msg254855 - (view) |
Author: Yury Selivanov (yselivanov) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
Date: 2015-11-20 23:27 |
Didn’t mean to adjust priority
|
msg255030 - (view) |
Author: Yury Selivanov (yselivanov) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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)  |
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) *  |
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) *  |
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) *  |
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) *  |
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)  |
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) *  |
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) *  |
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).
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:23 | admin | set | github: 69846 |
2016-10-29 14:14:48 | cpitclaudel | set | messages:
+ msg279674 |
2016-10-29 04:14:27 | martin.panter | set | messages:
+ msg279652 |
2016-10-27 03:54:10 | cpitclaudel | set | files:
+ completion.py nosy:
+ cpitclaudel messages:
+ msg279525
|
2016-03-02 10:33:05 | berker.peksag | set | status: open -> closed |
2016-02-04 19:09:26 | yselivanov | set | messages:
+ msg259591 |
2016-02-04 19:08:31 | python-dev | set | messages:
+ msg259590 |
2016-02-04 17:24:45 | Yury.Selivanov | set | nosy:
+ Yury.Selivanov messages:
+ msg259580
|
2016-02-04 17:17:36 | berker.peksag | set | messages:
+ msg259579 |
2016-02-04 07:29:30 | martin.panter | set | status: closed -> open
messages:
+ msg259538 |
2016-02-04 06:56:32 | berker.peksag | set | messages:
+ msg259535 |
2016-02-04 06:26:29 | yselivanov | set | status: open -> closed resolution: fixed messages:
+ msg259533
stage: commit review -> resolved |
2016-02-04 06:25:23 | python-dev | set | nosy:
+ python-dev messages:
+ msg259532
|
2016-02-04 06:19:48 | ned.deily | set | messages:
+ msg259531 |
2016-02-04 02:51:03 | berker.peksag | set | nosy:
+ berker.peksag
messages:
+ msg259526 stage: patch review -> commit review |
2015-11-27 03:05:07 | martin.panter | set | messages:
+ msg255451 |
2015-11-24 15:09:35 | r.david.murray | set | messages:
+ msg255268 |
2015-11-24 07:32:04 | martin.panter | set | messages:
+ msg255251 |
2015-11-24 05:42:24 | martin.panter | set | messages:
+ msg255248 |
2015-11-23 17:54:36 | yselivanov | set | nosy:
+ ncoghlan
|
2015-11-23 17:53:49 | yselivanov | set | messages:
+ msg255205 components:
- macOS |
2015-11-23 16:25:45 | r.david.murray | set | messages:
+ msg255181 |
2015-11-22 07:07:26 | larry | set | priority: release blocker -> normal |
2015-11-22 07:07:19 | larry | set | nosy:
+ larry messages:
+ msg255085
|
2015-11-20 23:39:41 | yselivanov | set | messages:
+ msg255030 |
2015-11-20 23:27:50 | martin.panter | set | priority: high -> release blocker
messages:
+ msg255029 |
2015-11-20 23:27:24 | martin.panter | set | priority: release blocker -> high
messages:
+ msg255028 stage: patch review |
2015-11-20 16:38:50 | yselivanov | set | priority: high -> release blocker
messages:
+ msg254998 |
2015-11-19 20:10:28 | yselivanov | set | files:
+ rlcompleter.patch keywords:
+ patch messages:
+ msg254931
|
2015-11-19 01:32:33 | martin.panter | set | messages:
+ msg254871 |
2015-11-19 00:37:53 | martin.panter | set | messages:
+ msg254868 |
2015-11-19 00:20:37 | martin.panter | set | messages:
+ msg254866 |
2015-11-18 22:36:51 | yselivanov | set | messages:
+ msg254860 |
2015-11-18 22:27:23 | martin.panter | set | nosy:
+ martin.panter messages:
+ msg254859
|
2015-11-18 20:14:30 | r.david.murray | set | nosy:
+ ronaldoussoren, ned.deily messages:
+ msg254856
components:
+ macOS type: behavior |
2015-11-18 19:58:50 | yselivanov | create | |