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

rlcompleter: tab on empty prefix => insert spaces #67629

Closed
arigo mannequin opened this issue Feb 11, 2015 · 33 comments
Closed

rlcompleter: tab on empty prefix => insert spaces #67629

arigo mannequin opened this issue Feb 11, 2015 · 33 comments
Labels
deferred-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@arigo
Copy link
Mannequin

arigo mannequin commented Feb 11, 2015

BPO 23441
Nosy @arigo, @pitrou, @larryhastings, @ned-deily, @ezio-melotti, @bitdancer, @berkerpeksag, @vadmium, @1st1
Files
  • rlcompleter_tab2space.diff: Patch
  • rlcompleter-tab.patch: fixes rlcompleter behavior on empty lines
  • issue23441.diff
  • issue23441_rev1.diff
  • 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 2015-07-27.21:13:46.330>
    created_at = <Date 2015-02-11.10:21:44.695>
    labels = ['deferred-blocker', 'type-bug', 'library']
    title = 'rlcompleter: tab on empty prefix => insert spaces'
    updated_at = <Date 2015-11-19.20:12:34.332>
    user = 'https://github.com/arigo'

    bugs.python.org fields:

    activity = <Date 2015-11-19.20:12:34.332>
    actor = 'yselivanov'
    assignee = 'none'
    closed = True
    closed_date = <Date 2015-07-27.21:13:46.330>
    closer = 'berker.peksag'
    components = ['Library (Lib)']
    creation = <Date 2015-02-11.10:21:44.695>
    creator = 'arigo'
    dependencies = []
    files = ['38093', '38487', '40032', '40039']
    hgrepos = []
    issue_num = 23441
    keywords = ['patch']
    message_count = 33.0
    messages = ['235733', '235735', '235750', '235767', '236219', '238102', '238103', '238107', '238238', '238240', '238245', '238377', '238390', '238392', '242276', '246398', '246399', '246400', '246423', '246424', '246444', '246481', '246483', '246670', '247435', '247438', '247477', '247481', '247482', '247484', '247486', '254869', '254932']
    nosy_count = 12.0
    nosy_names = ['arigo', 'pitrou', 'larry', 'ned.deily', 'ezio.melotti', 'r.david.murray', 'dabeaz', 'python-dev', 'berker.peksag', 'martin.panter', 'yselivanov', 'Martin Sekera']
    pr_nums = []
    priority = 'deferred blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue23441'
    versions = ['Python 3.4', 'Python 3.5', 'Python 3.6']

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Feb 11, 2015

    In the interactive prompt:

    >>> if 1:
    ... <press tab here>

    Pressing tab here should just insert 4 spaces. It makes no sense to display all 500 possible "completions" of the empty word, and using tab to indent is a very common feature of any editor with a Python mode.

    Patch attached.

    @arigo arigo mannequin added type-feature A feature request or enhancement stdlib Python modules in the Lib dir labels Feb 11, 2015
    @vadmium
    Copy link
    Member

    vadmium commented Feb 11, 2015

    Main bug report: bpo-22086.

    Can you say if this works when the cursor is not at the start of the entry? E.g. after an existing tab (Ctrl-V Tab on GNU readline) or space, or after an embedded newline (Ctrl-V Enter).

    What happens if you press backspace after pressing tab? Do you have to backspace four times? Another options is to insert a literal tab code instead of spaces.

    @arigo
    Copy link
    Mannequin Author

    arigo mannequin commented Feb 11, 2015

    Ah, thanks, I missed there was already an issue.

    The patch's logic is as follows: when pressing tab anywhere in the line, if the word to complete is empty (which might be for any number of reasons, like the cursor is after another space or a non-word character), then suggest as completion a string of N spaces, where N is a number from 1 to 4 so that the cursor ends up being aligned to a multiple of 4. You have to press backspace several times to remove all these spaces.

    Indeed, it could also suggest a single '\t' character, which would be simpler and revert exactly to the old behavior (including a single backspace to remove the whole tab). I didn't do it only because it would typically indent to 8 characters instead of 4. If people think it is anyway a better idea, I can submit a second (simpler) patch doing that.

    @pitrou
    Copy link
    Member

    pitrou commented Feb 11, 2015

    Thanks a lot for doing this. I think it's a good improvement. I prefer 4 spaces myself (tabs usually insert 8 spaces, which is too wide), but I'm open to other opinions.

    @bitdancer
    Copy link
    Member

    Looks good to me. I prefer 4 space indent as well, and since we have essentially deprecated the use of tabs for spacing in Python, I'd rather not reintroduce it here for the (minor) benefit of making backspacing easier. Just think about the copy and paste consequences....

    I'll miss vi's autoindent more than I will smart backspace, but python never had that to start with so I'm not asking for it :).

    I presume someone could add an enhancement for smart backspace if they were motivated to do so, but I'd say that is a separate issue.

    @MartinSekera
    Copy link
    Mannequin

    MartinSekera mannequin commented Mar 14, 2015

    Is it necessary to force a predefined tab width (4) onto the user here? I prefer 2-character tabs for example, and have all my terminals set up accordingly (setterm --regtabs 2). I presume many people prefer 8-column tabs, hence the default width in most software.

    Additionally, the user shouldn't have to backspace 4 characters to remove one level of indentation. Not only is it annoying, it is also prone to errors if the user miscounts (and ends up with an IndentationError).

    I've been using the attached patch (just 5 lines) to emulate the behavior familiar from unix shells: pressing tab activates the completion function, but indents if the line is empty (i.e. '' or all whitespace). It also indents properly if the cursor is moved to whitespace in front of a line, as one would expect.

    @pitrou
    Copy link
    Member

    pitrou commented Mar 14, 2015

    4 spaces is the recommended indent width in PEP-8. That's what most Python users will expect.

    @ezio-melotti
    Copy link
    Member

    +1 for 4 spaces.

    Smart backspace would be nice too, but I agree it's a separate issue (unless it's trivial to implement). Deeply-nested code in the interactive interpreter is uncommon, and the lack of smart backspace never bothered me.

    @MartinSekera
    Copy link
    Mannequin

    MartinSekera mannequin commented Mar 16, 2015

    Copy that. Would the patch be acceptable if the '\t' was simply changed to 4 spaces?

    To discuss further, it is my opinion that the interpreter should output a \t character when the TAB key is pressed, as this is the behavior of nearly every shell and interpreter (that doesn't autocomplete all the time) there is, including all versions of python. Introducing new behavior for the sake of compliance with PEP-8 (which, as it itself states, applies only to "the Python code comprising the standard library in the main Python distribution"), is in my opinion unnecessary and confusing (since to clear n indent levels the user has to backspace 4n times).

    @bitdancer
    Copy link
    Member

    No program that I work with (and I use cli all the time) outputs a tab when I press the tab key. Now, true, I think I had to configure vi so that that was the case, but nothing else that I remember. Of course, most of those programs use readline and come with some sort of configuration such as what we are (belatedly) supply in python's command line. So, to me, outputting an actual tab character would be *less* consistent with my general (modern) computing experience. Especially since we are, in fact, talking about python now doing "autocomplete all the time" :)

    @MartinSekera
    Copy link
    Mannequin

    MartinSekera mannequin commented Mar 16, 2015

    In the end it doesn't matter what characters end up on the terminal. What matters is the UX of not having to press backspace several times to unindent. That's sloppy design.

    The issue of forcing a custom indent width on a user who might have their tab stops setup differently is another, albeit smaller, issue. I think the interpreter has absolutely no reason to do what's basically UI work (aligning tabbed text with tab-stop columns as in the first patch). That's the terminal's job.

    @bitdancer
    Copy link
    Member

    The CLI is a UI. We're using readline facilities (ie: "the terminal") to improve it. And people cut and paste from the interactive terminal, so I think the presence of tab characters is a negative from that perspective as well.

    @MartinSekera
    Copy link
    Mannequin

    MartinSekera mannequin commented Mar 18, 2015

    But tab characters are rendered by the terminal into spaces. During stdout processing, when the term encounters a \t (0x09), it inserts (into the term buffer that is displayed to the user) as many spaces (0x20) as needed to move the cursor to the nearest tab-stop (setterm --tabs will display them for you). Why do we need to duplicate this inside Python?

    There are no copy&paste issues either, try it yourself: when you copy and paste tab-indented text from the terminal, your text will contain spaces instead of tabs (at whatever width you have your terminal tab stops configured for).

    @vadmium
    Copy link
    Member

    vadmium commented Mar 18, 2015

    I think it would really depend on the particular terminal. Sakura (a Unix terminal using the VTE library, like Gnome Terminal) copied the tab literally:

    $ printf 'tab\ttab|sp   sp\n'
    tab	tab|sp   sp  <== COPIED
    $ hexdump -C
    tab	tab|sp   sp  <== PASTED
    00000000  74 61 62 09 74 61 62 7c  73 70 20 20 20 73 70 0a  |tab.tab|sp   sp.|
    00000010

    @bitdancer
    Copy link
    Member

    Per the discussion in bpo-5845, I'm setting this to release blocker to make sure it gets committed before the next release. I'm also adding 3.4 as I think this is a bug. Any objections to that?

    @bitdancer bitdancer added release-blocker type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Apr 30, 2015
    @dabeaz
    Copy link
    Mannequin

    dabeaz mannequin commented Jul 7, 2015

    This is a problem that will never be fixed. Sure, it was a release blocker in Python 3.4.

    It wasn't fixed.

    It is a release blocker in Python 3.5.

    It won't be fixed.

    They'll just tell you to indent using the spacebar as generations of typists have done for centuries.

    It won't be fixed.

    Why don't you just use ipython or bpython?

    It won't be fixed.

    Doesn't your IDE take care of this?

    It won't be fixed.

    By the way, backspace will never work right either. No, that will never be fixed.

    Did we mention that this will never be fixed?

    You can fix it!

    Yes, you! No, I mean you! Yes, yes, you can.

    Simply edit the file Lib/site.py and comment out the line that does this:

      # enablerlcompleter()
    

    Problem solved. All is well.

    By the way. This problem will never be fixed. That is all.

    @pitrou
    Copy link
    Member

    pitrou commented Jul 7, 2015

    Well, there is a patch, and I think a couple core developers interested in this, so I wouldn't bet on this never getting fixed. Just leave them a bit more time.

    @dabeaz
    Copy link
    Mannequin

    dabeaz mannequin commented Jul 7, 2015

    For what it's worth, I'm kind of tired having to hack site.py every time I upgrade Python in order to avoid being shown 6000 choices when hitting tab on an empty line. It is crazy annoying.

    @larryhastings
    Copy link
    Contributor

    This issue was created 2015/02/11. Python 3.4 was released on 2014/03/16. Is there an earlier duplicate issue for this problem that was marked "release blocker" for 3.4?

    @bitdancer
    Copy link
    Member

    No, that's what I said when I marked this one as a release blocker: the other one *hadn't been*, which is why it didn't get in to 3.4.

    @vadmium
    Copy link
    Member

    vadmium commented Jul 8, 2015

    There are currently two patches proposed, and I presume either patch would satisfy the likes of David Beazley. The first patch is more complex (extending the Completer constructor), while the second is simple. Perhaps it is okay to commit either one, and then continue discussing and refining the behaviour afterwards?

    So far it looks like the only dilemma is that Martin Sekera wants to avoid the multiple backspacing and duplicating terminal behaviour, but David Murray wants to avoid the terminal copying tabs. My weak preference is with Martin’s simpler patch and relying on the Readline and the terminal to handle questions of tab stops and copied text.

    @dabeaz
    Copy link
    Mannequin

    dabeaz mannequin commented Jul 9, 2015

    Frivolity aside, I really wish this issue would get more traction and a fix.

    Indentation is an important part of the Python language (obviously). A pretty standard way to indent is to hit "tab" in whatever environment you're using to edit Python code.

    Yet, at the interactive prompt, tab doesn't actually indent on a blank line. Instead, it autocompletes the builtins. Aside from it being highly annoying (as previously mentioned), it is also an embarrassment.

    Newcomers to Python will very often try things out using the stock interpreter before moving on to more sophisticated environments. The fact that tab is broken from the get-go leaves a pretty sour impression when not even the most basic tutorial examples work at the interactive console (and keep in mind that whitespace sensitivity is probably already an issue on their minds).

    Experienced Python users coming from Python 2 to Python 3 are going to find that tab is busted in Python 3. Well, of course it's busted because everything is busted in Python 3. "Wow, this really sucks as bad as everyone says" they'll say.

    So, with that as context, I'm really hoping I don't have to watch people use a busted tab key for another entire release cycle of Python 3 as I did for Python-3.4.

    I have no particular thoughts about the specifics (tabs vs. spaces) or the amount of indentation. It's the autocomplete on empty line that's the issue.

    @dabeaz
    Copy link
    Mannequin

    dabeaz mannequin commented Jul 9, 2015

    Wanted to add: I see this as being about the same as having a broken window pane on the front of Python 3. Maybe there are awesome things inside, but it makes a bad first impression on anyone who dares to use the interactive console.

    @pitrou
    Copy link
    Member

    pitrou commented Jul 12, 2015

    My weak preference is with Martin’s simpler patch and relying on the
    Readline and the terminal to handle questions of tab stops and copied
    text.

    Fair enough. Still, it would be nice to add a test to test_rlcompleter. Does someone want to do this?

    @dabeaz
    Copy link
    Mannequin

    dabeaz mannequin commented Jul 26, 2015

    It's still broken on Python 3.5b4.

    @berkerpeksag
    Copy link
    Member

    Here is a patch with a simple test case.

    @1st1
    Copy link
    Member

    1st1 commented Jul 27, 2015

    Berker, the patch looks good to me. Please commit.

    Larry, is it possible we can include this in 3.5.0?

    @ned-deily
    Copy link
    Member

    Berker, the part of the patch for test_rlcompleter.py does not apply cleanly. Here's an updated version of it. Both the fix and the test seem to work as advertised on current 3.5 tip. It would be nice to fix this finally.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 27, 2015

    New changeset 82ccdf2df5ac by Berker Peksag in branch '3.4':
    Issue bpo-23441: rcompleter now prints a tab character instead of displaying
    https://hg.python.org/cpython/rev/82ccdf2df5ac

    New changeset d55bdd2dc45e by Berker Peksag in branch '3.5':
    Issue bpo-23441: rcompleter now prints a tab character instead of displaying
    https://hg.python.org/cpython/rev/d55bdd2dc45e

    New changeset 2715734b3378 by Berker Peksag in branch 'default':
    Issue bpo-23441: rcompleter now prints a tab character instead of displaying
    https://hg.python.org/cpython/rev/2715734b3378

    @berkerpeksag
    Copy link
    Member

    Thanks for the patch Martin and thank you all for testing and reviewing patches!

    @pitrou
    Copy link
    Member

    pitrou commented Jul 27, 2015

    Thank you Berker for putting an end to this.

    @vadmium
    Copy link
    Member

    vadmium commented Nov 19, 2015

    The fix here doesn’t work perfectly with the Editline (libedit) version of Readline; see bpo-25660.

    @1st1
    Copy link
    Member

    1st1 commented Nov 19, 2015

    Please see my patch for bpo-25660 -- instead of returning '\t' from the completion function, I use readline API to modify the input buffer directly.

    @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
    deferred-blocker stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    8 participants