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

cmd.py command completion does not work with libedit #102130

Closed
keeely opened this issue Feb 21, 2023 · 17 comments
Closed

cmd.py command completion does not work with libedit #102130

keeely opened this issue Feb 21, 2023 · 17 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@keeely
Copy link

keeely commented Feb 21, 2023

Bug report

As title, the Homebrew community has switched to libedit readline as of Python 3.11. There exists a gnureadline package which allows one to continue using the gnu readline, but unfortunately cmd.py will continue to use the libedit one, and that doesn't always work properly.

Your environment

Python 3.11 MacOs Ventura.

Here's some code that won't work with libedit readline:

import cmd

class MyShell(cmd.Cmd):
    def do_command(self, _arg):
        print(f"running {self}")

MyShell().cmdloop()

Tab-completion appears to fail on Homebrew python 3.11, I suspect due to the libedit readline. The code works fine with Homebrew python versions up to 3.10 which use gnu readline. Also, if you install gnureadline and change all occurrences of import readline in cmd.py to import gnureadline as readline the cmd module works perfectly.

So in summary could we fix cmd.py to work correctly with libedit as well as gnu readline? Or, failing that provide some fairly easy means of overriding the readline used by cmd.py without resorting to nasty monkey patching.

Linked PRs

@keeely keeely added the type-bug An unexpected behavior, bug, or error label Feb 21, 2023
@ned-deily
Copy link
Member

ned-deily commented Feb 21, 2023

Thanks for your report. Alas, the differences between GNU readline and the BSD-based libedit have been a source of confusion for many years for users of many commmand line tools, not just python-based ones. The documentation for Python's readline module discusses some of these differences in a note at the top of the page. In particular, it talks about the two different configuration files that are used, their different syntax, and what is needed to enable tab completion when using libedit. For the record, one of the reasons why many distributions prefer to use libedit by default is its more liberal licensing: GNU readline is GPL-3 licensed.

Suggestions for improving the documentation or specific bugs when following the current suggestion in the docs would be welcomed. Otherwise, I'm not sure there is any action to be taken here.

@keeely
Copy link
Author

keeely commented Feb 21, 2023

Thank you for that very helpful clue. A quick google later, reveals I can change my code to this:

import cmd
import readline

if 'libedit' in readline.__doc__:
    print("Found libedit readline")
    readline.parse_and_bind("bind ^I rl_complete")
else:
    print("Found gnu readline")
    readline.parse_and_bind("tab: complete")


class MyShell(cmd.Cmd):
    def do_command(self, _arg):
        print(f"running {self}")

MyShell().cmdloop()

And after that the tab completion works for both libraries. I have no idea why that works, but it does. The libedit version isn't quite right, because the first press of tab does nothing but sound the bell, then the second (and subsequent) presses work as I'd expect. A whole lot better than no completion, obviously.

It would be nice if Python abstracted this away to make cmd.py work out of the box for both libraries.

@keeely
Copy link
Author

keeely commented Feb 22, 2023

I am going with slightly different code in the end because I don't want the odd behaviour with libedit tab completion to change the user experience. So I've added gnureadline to required dependencies and am hooking the import:

class MetaPathLoader:
    def load_module(self, fullname):
        if fullname in sys.modules:
            return sys.modules[fullname]

        gnu = importlib.import_module("gnureadline")
        sys.modules["readline"] = gnu
        return gnu

class MetaPathFinder:
    def find_module(self, fullname, path=None):
        if fullname is "readline":
            return MetaPathLoader()

sys.meta_path.insert(0, MetaPathFinder())

I hope people agree, this is pretty horrible.

@carljm
Copy link
Member

carljm commented Feb 22, 2023

To me this looks like a legitimate bug report for the cmd module, which promises automatic command completion in its docs, and has code using the readline module to implement that, but that code appears to be GNU readline specific and doesn't work if libedit is used. (Unless we would want to say that the cmd module just doesn't support libedit, but that seems not ideal given its reasonably wide usage, and the fact that the author of a tool using cmd can't necessarily control whether their users have libedit or GNU readline installed.) The recommendations in the doc note for the readline module may provide pointers to how to fix cmd, but aren't in and of themselves an adequate solution for users of cmd: if you write a tool using cmd, it's not reasonable to tell all your users to put things in their ~/.editrc to make your tool work either.

The ideal course of action here would be to figure out the right code needed in cmd.py to support libedit, and update cmd.py so that completion works out of the box with either underlying library.

If we achieve that, there may also be an argument for a follow-up feature where the readline module itself would incorporate some higher-level API that allows enabling completion without the client having to be aware of and check for libedit vs GNU readline (i.e. move the code to handle this from cmd to readline.) But I think that would be a separate issue from fixing the bug in cmd.

@carljm carljm changed the title Brew Python has migrated to libedit readline, and this has exposed an issue in the cmd.py module. cmd.py command completion does not work with libedit Feb 22, 2023
@carljm
Copy link
Member

carljm commented Feb 22, 2023

@hugovk I think maybe we should not tag this bug as OS-mac either; IMO the Homebrew switch to libedit is not really related to the actual bug here (libedit is used on other platforms too); it just means brew users will now be affected by the bug when they weren't before. What do you think?

@hugovk hugovk removed the OS-mac label Feb 22, 2023
@hugovk
Copy link
Member

hugovk commented Feb 22, 2023

Makes sense, I've removed the label 👍

@keeely
Copy link
Author

keeely commented Feb 22, 2023

Whilst it would be great to fix cmd.py to make it perfect, I'd hate to chew through significant Python developer time to resolve all the issues with libedit readline. There are at least two: The first relating to history files, the second to tab completion. Once people start using it in earnest we may find more.

Let's think for a moment who uses cmd.py. It's for command-line interfaces, so for people who are comfortable with that, implying developers or technical people. They won't think twice about installing a GNU dependency on their desktop machine. Let's also think who has a problem with a GNU dependency. That would be someone shipping a closed source product embedding Python with some kind of interactive configuration shell. pfSense or something like it comes to mind, but these kinds of apps/devices usually ship with web interfaces. It's all getting a bit niche.

I don't think it's full libedit support that's needed here, just a straightforward route for application maintainers (such as myself) to get their users working. Adding the dependency on gnureadline is very easy for me. I guess it breaks some fundamental rule in Python if we refer directly to a third-party package in cmd.py, so the next best thing is to pass it in. If it's passed to the Cmd constructor, it could be used in preference to readline.

class Cmd:

    def __init__(self, completekey='tab', stdin=None, stdout=None, readline=None):
        self.readline = readline

# Everywhere readline is imported do something like
    def cmdloop(self, intro=None):
        ...
        if self.use_rawinput and self.completekey:
            try:
                if self.readline is None:
                    import readline
                else:
                    readline = self.readline
                self.old_completer = readline.get_completer()
                readline.set_completer(self.complete)
                readline.parse_and_bind(self.completekey+": complete")
            except ImportError:
                pass

@ned-deily
Copy link
Member

Also, the differences you note here aren't limited to the python cmd module; they potentially affect any use of python's readline module and, for that matter, the Python interpreter's REPL shell. So if someone wants to go further than, say, making doc changes to the cmd module, I think it would be prudent to do some more investigation into how other distributions and users (python and non-python) of libedit vs GNU readline have handled this (as @carljm points out, the issues here are not limited to one platform or distribution) and look at addressing them at the readline module level as much as possible.

@carljm
Copy link
Member

carljm commented Feb 23, 2023

chew through significant Python developer time

Accepting that it's a valid bug is not a promise that core developers (or anyone else) will be able to prioritize fixing it, and it doesn't obligate anyone to use their time against their preferences. It just means that if someone comes along who is interested and motivated to fix it, they have some indication that a fix is desired and a good implementation of a fix is likely to be accepted.

It's all getting a bit niche.

Lots of open issues on CPython are niche :) If they are too niche for anyone to care, they will eventually be closed due to inactivity / lack of interest. But lots of niche issues do find a motivated developer within that niche to fix them.

There are at least two: The first relating to history files, the second to tab completion.

You've provided some details on the tab completion issue; can you say more about the history issue (i.e. what doesn't work, have you discovered any pointers about how to go about fixing it?) We could open a separate issue for it, but I'd be inclined for now to just broaden this issue to generally cover cmd compatibility with libedit, for both completion and history.

the next best thing is to pass it in. If it's passed to the Cmd constructor, it could be used in preference to readline.

I don't think dependency-injecting an alternate readline module is a good long-term solution here. The standard library readline module should at least be good enough to support the standard library's own uses of readline. If someone really wants to inject a different readline module, it's already possible via sys.modules, as you've already found. (I don't think you necessarily have to bother with a metapath finder/loader for this; you can just import gnureadline early in your application's main module, before cmd is imported, and then put it in sys.modules['readline'].)

@keeely
Copy link
Author

keeely commented Feb 23, 2023

That's a fair point. It would be nice to pull in https://pypi.org/project/pyreadline3/ into the standard python distribution as well. my install_requires for dealing with cmd.py requirements is now:

        "pyreadline3; os_name=='nt'",
        "gnureadline; sys_platform=='darwin' and python_version > '3.10.0'",

@dtrodrigues
Copy link
Contributor

Some information about Python's history file format and libedit vs readline is in Homebrew/homebrew-core#113811 (comment)

Another quirk referring to Python history search and libedit is #100610, which also probably should have the "OS-mac" label removed.

@keeely
Copy link
Author

keeely commented Feb 23, 2023

There are at least two: The first relating to history files, the second to tab completion.

You've provided some details on the tab completion issue; can you say more about the history issue (i.e. what doesn't work, have you discovered any pointers about how to go about fixing it?) We could open a separate issue for it, but I'd be inclined for now to just broaden this issue to generally cover cmd compatibility with libedit, for both completion and history.

I have a Mac and I've been through multiple OS updates, multiple Python updates, generally swapped between Homebrew and built-in Python so I don't understand the exact sequence of events, but attempting to read certain types of history files with libedit readline results in the error as described here:
https://stackoverflow.com/questions/29572484/os-x-can-anyone-explain-why-i-was-getting-an-os-error-in-my-pythonrc-py-file

I went back to Python with gnu readline, wrote some history, back to libedit, read the files and it wasn't that simple. I didn't see the error. Maybe newer versions of gnureadline stopped writing something incompatible to the files.

(I don't think you necessarily have to bother with a metapath finder/loader for this; you can just import gnureadline early in your application's main module, before cmd is implemented, and then put it in sys.modules['readline'].)

Yes, I discovered this later on 👍

@Constantin1489

This comment was marked as outdated.

Constantin1489 added a commit to Constantin1489/cpython that referenced this issue Nov 6, 2023
Constantin1489 added a commit to Constantin1489/cpython that referenced this issue Nov 6, 2023
@iritkatriel iritkatriel added the stdlib Python modules in the Lib dir label Nov 26, 2023
@encukou
Copy link
Member

encukou commented Dec 4, 2023

IMO, teaching cmd.py to work with libedit is a new feature, and thus shouldn't be backported.
The Homebrew build of Python 3.12 and below should be able to continue using gnureadline using ./configure --with-readline=readline. If I understand correctly, the Mac default for cases when both are available is editline, but this option will override it.
Please correct me if I'm wrong.

encukou pushed a commit that referenced this issue Dec 5, 2023
---

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
…-107748)

---

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
@hugovk
Copy link
Member

hugovk commented Jun 15, 2024

Triage: can this be closed or is there more to do?

@Constantin1489
Copy link
Contributor

@hugovk #53279 is this relevant?

@encukou
Copy link
Member

encukou commented Jun 17, 2024

Yes, thanks for noticing.
Both can be closed.

@encukou encukou closed this as completed Jun 17, 2024
whitequark added a commit to whitequark/glasgow that referenced this issue Jun 29, 2024
…n load.

Previously this would cause a crash with an opaque EINVAL OSError that
doesn't come from a syscall.

Related to python/cpython#102130.
whitequark added a commit to whitequark/glasgow that referenced this issue Jun 29, 2024
Previously this would cause a crash with an opaque EINVAL OSError that
doesn't come from a syscall.

Related to python/cpython#102130.
whitequark added a commit to whitequark/glasgow that referenced this issue Jun 29, 2024
Previously this would cause a crash with an opaque EINVAL OSError that
doesn't come from a syscall.

Related to python/cpython#102130.
whitequark added a commit to whitequark/glasgow that referenced this issue Jun 29, 2024
Previously this would cause a crash with an opaque EINVAL OSError that
doesn't come from a syscall.

Related to python/cpython#102130.
github-merge-queue bot pushed a commit to GlasgowEmbedded/glasgow that referenced this issue Jun 29, 2024
Previously this would cause a crash with an opaque EINVAL OSError that
doesn't come from a syscall.

Related to python/cpython#102130.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…-107748)

---

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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