Skip to content
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

tabs don't work correctly in python repl #69846

Closed
1st1 opened this issue Nov 18, 2015 · 32 comments
Closed

tabs don't work correctly in python repl #69846

1st1 opened this issue Nov 18, 2015 · 32 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@1st1
Copy link
Member

1st1 commented Nov 18, 2015

BPO 25660
Nosy @ronaldoussoren, @ncoghlan, @larryhastings, @ned-deily, @bitdancer, @berkerpeksag, @vadmium, @serhiy-storchaka, @1st1, @cpitclaudel
Files
  • rlcompleter.patch
  • completion.py: Emacs' completion code
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2016-03-02.10:33:05.897>
    created_at = <Date 2015-11-18.19:58:50.887>
    labels = ['interpreter-core', 'type-bug']
    title = "tabs don't work correctly in python repl"
    updated_at = <Date 2016-10-29.14:14:48.215>
    user = 'https://github.com/1st1'

    bugs.python.org fields:

    activity = <Date 2016-10-29.14:14:48.215>
    actor = 'cpitclaudel'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-03-02.10:33:05.897>
    closer = 'berker.peksag'
    components = ['Interpreter Core']
    creation = <Date 2015-11-18.19:58:50.887>
    creator = 'yselivanov'
    dependencies = []
    files = ['41085', '45235']
    hgrepos = []
    issue_num = 25660
    keywords = ['patch']
    message_count = 32.0
    messages = ['254855', '254856', '254859', '254860', '254866', '254868', '254871', '254931', '254998', '255028', '255029', '255030', '255085', '255181', '255205', '255248', '255251', '255268', '255451', '259526', '259531', '259532', '259533', '259535', '259538', '259579', '259580', '259590', '259591', '279525', '279652', '279674']
    nosy_count = 12.0
    nosy_names = ['ronaldoussoren', 'ncoghlan', 'larry', 'ned.deily', 'r.david.murray', 'Yury.Selivanov', 'python-dev', 'berker.peksag', 'martin.panter', 'serhiy.storchaka', 'yselivanov', 'cpitclaudel']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue25660'
    versions = ['Python 3.5', 'Python 3.6']

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 18, 2015

    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.

    @1st1 1st1 added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 18, 2015
    @bitdancer
    Copy link
    Member

    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?

    @bitdancer bitdancer added OS-mac type-bug An unexpected behavior, bug, or error labels Nov 18, 2015
    @vadmium
    Copy link
    Member

    vadmium commented Nov 18, 2015

    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.

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 18, 2015

    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.

    @vadmium
    Copy link
    Member

    vadmium commented Nov 19, 2015

    David Beazley’s description of the bug: “hitting tab inserts spaces and a carriage return at the same time.”

    @vadmium
    Copy link
    Member

    vadmium commented Nov 19, 2015

    Applying my patch at bpo-13501, then building with “autoreconf && ./configure --with-readline=editline”, I can reproduce the funny tab behaviour on Linux.

    @vadmium
    Copy link
    Member

    vadmium commented Nov 19, 2015

    I think this might just be a side effect of the way we abuse the tab completer to insert a literal tab (bpo-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

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 19, 2015

    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.

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 20, 2015

    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.

    @vadmium
    Copy link
    Member

    vadmium commented Nov 20, 2015

    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.

    @vadmium
    Copy link
    Member

    vadmium commented Nov 20, 2015

    Didn’t mean to adjust priority

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 20, 2015

    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.

    @larryhastings
    Copy link
    Contributor

    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.

    @bitdancer
    Copy link
    Member

    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...

    @1st1
    Copy link
    Member Author

    1st1 commented Nov 23, 2015

    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?

    @1st1 1st1 removed the OS-mac label Nov 23, 2015
    @vadmium
    Copy link
    Member

    vadmium commented Nov 24, 2015

    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 bpo-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.

    @vadmium
    Copy link
    Member

    vadmium commented Nov 24, 2015

    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.

    @bitdancer
    Copy link
    Member

    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.

    @vadmium
    Copy link
    Member

    vadmium commented Nov 27, 2015

    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.

    @berkerpeksag
    Copy link
    Member

    With rlcompleter.patch applied, I see the same behavior on my OS X (with libedit) and Ubuntu (with readline) systems.

    Thanks!

    @ned-deily
    Copy link
    Member

    LGTM on OS X with both the system libedit and a MacPorts GNU readline. Let's apply this.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 4, 2016

    New changeset 64417e7a1760 by Yury Selivanov in branch '3.5':
    Issue bpo-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 bpo-25660)
    https://hg.python.org/cpython/rev/87dfadd61e0d

    @1st1
    Copy link
    Member Author

    1st1 commented Feb 4, 2016

    Let's apply this.

    Merged. Martin, Berker, and Ned, thanks for testing this patch out.

    @1st1 1st1 closed this as completed Feb 4, 2016
    @berkerpeksag
    Copy link
    Member

    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

    @vadmium
    Copy link
    Member

    vadmium commented Feb 4, 2016

    See also the Windows buildbots, where the Readline module is not available, but “rlcompleter” still (used to) work.

    @vadmium vadmium reopened this Feb 4, 2016
    @berkerpeksag
    Copy link
    Member

    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.

    @YurySelivanov
    Copy link
    Mannequin

    YurySelivanov mannequin commented Feb 4, 2016

    Berker, I'll fix the windows issue in a few hours. Sorry for breaking things.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 4, 2016

    New changeset 980ea968444c by Yury Selivanov in branch '3.5':
    Issue bpo-25660: Fix a unittest and rlcompleter when readline isn't available
    https://hg.python.org/cpython/rev/980ea968444c

    @1st1
    Copy link
    Member Author

    1st1 commented Feb 4, 2016

    Everything should be OK now (both broken tests and using rlcompleter on Windows). Please confirm.

    @cpitclaudel
    Copy link
    Mannequin

    cpitclaudel mannequin commented Oct 27, 2016

    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).

    @vadmium
    Copy link
    Member

    vadmium commented Oct 29, 2016

    Clément: Yes, that sounds like the intended behaviour, at least when using Readline. Originally discussed in bpo-23441 and bpo-22086. (Though I think the change in behaviour when completion is called outside of Readline is a bug.)

    @cpitclaudel
    Copy link
    Mannequin

    cpitclaudel mannequin commented Oct 29, 2016

    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).

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants