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

Use pyreadline3 instead of pyreadline #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

parejkoj
Copy link

@parejkoj parejkoj commented Aug 12, 2022

I updated the pyreadline dependency, import, and readme links. I'm not sure how to test that this will pip-install with the correct dependency, though.

There's also some further discussion in the readme about bugs in the current pyreadline (e.g. issue 48); there aren't specific issue links to convert those to, and I don't know which, if any, of those bugs have been fixed in pyreadline3.

This should fix issues #36 and #37 .

@parejkoj
Copy link
Author

It looks like pyreadline3 PR #9 might fix the color completions bug. If we can get that merged, we can remove that note from this readme.

@parejkoj
Copy link
Author

@brgirgis @athrvvvv: what do you need in order to merge this?

@athrvvvv
Copy link

@parejkoj ohhh i got it. even though we install pyreadline3 via pip install pyreadline3 we have to import it as pyreadline.

@parejkoj
Copy link
Author

@athrvvvv : I'm not sure about that. It certainly looks like pyreadline3 should be imported as exactly that. The thing I don't know how to do is test it.

Copy link

@athrvvvv athrvvvv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import it as pyreadline

@parejkoj
Copy link
Author

parejkoj commented Sep 23, 2022

The import is wrapped internally and it looks like what it is imported as doesn't matter, e.g., no need for import ... as ....

Unless you're saying the import has to be import pyreadline, but it doesn't look like that's correct for pyreadline3. I just tested a pip installl pyreadline3 and it is found via import pyreadline3.

Andrej730 added a commit to Andrej730/fancycompleter that referenced this pull request Feb 7, 2023
@blueyed
Copy link
Collaborator

blueyed commented Mar 13, 2023

To get this merged CI should be green, which involves moving it to GitHub Actions (#46).

Then there might also a compat issue, given that "Version 3.4+ or pyreadline3 runs on Python 3.5+.".

@parejkoj
Copy link
Author

@blueyed : I'll wait on a fix for #46 and rebase to it then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants