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

Problem with readline module when handing history navigation using up-arrow key #12

Closed
efJerryYang opened this issue Mar 28, 2023 · 3 comments

Comments

@efJerryYang
Copy link
Owner

Initially, I encountered an unknown problem with the readline module, which compelled me to remove its usage in #11. Unfortunately, this brought about a new problem - poor handling of inputs such as cursor movements using left and right arrow keys. Therefore, I had to revert back to the use of the readline module provided by the pyreadline3 package, which is compatible with both Linux and Windows.

I removed the usage in this commit 0f13332, which seemed to be a wrong decision.

I open this issue to track the problem with using the readline module. Although I cannot recall the exact details of the issue, I will monitor my daily usage for a week, and close this issue if no further problems arise.

efJerryYang added a commit that referenced this issue Mar 28, 2023
Poor handling of inputs such as cursor movements using left and right
arrow keys. See more detail at issue #12 below:
#12
efJerryYang added a commit that referenced this issue Mar 28, 2023
Poor handling of inputs such as cursor movements using left and right
arrow keys. See more detail at issue #12 below:
#12
@PraxTube
Copy link
Collaborator

It seems like the multi-line feature is still causing problems. I know that you are used to it by now, but I seriously think that a proper terminal editor is a much cleaner solution. Perhaps we could even allow for a command like !e to make it more accessible.

I think supporting both is also no problem, but if the implementation is causing even more problems I don't know if it's worth it.

@efJerryYang
Copy link
Owner Author

I thought as you have said, but the multi-line handling was actually not directly provided by readline module. You can have a look at the source code (I also forgot some of the logic till I read it again yesterday) that it actually handles the line handling simply by appending submitted lines to a list:

chatgpt-cli/src/utils/io.py

Lines 114 to 124 in 19abe79

# Use readline to handle user input
lines = []
while True:
line = input(prompt)
if line.strip() == "":
break
lines.append(line)
# Update the prompt using readline
prompt = "\r" + " " * len(prompt) + "\r" + " .... "
readline.get_line_buffer()
# Print a message indicating that the input has been submitted

The readline here is mainly for special key handling (e.g., arrow keys), and I tried removing it in commit 0f13332 but caused a problem for processing the arrow keys (e.g., moving the cursor left or right to insert characters). The reason why I open this issue is just as I mentioned earlier in this thread, to track the potential problems caused by readline, but I forgot what the problem is precisely (and it works fine without problems so far).

Currently, if we simply remove it as I did in 0f13332, the prompt provide by the tool will not work properly because there is a feature provided by the Python built-in input() method, it will read actually what the input characters are and not interpret them as special keys, and we should provide other alternatives like you mentioned in #6 which may raise some time cost to refactoring. So I retain its usage at the moment.

Additionally, I think the suggestion of using the abbreviation !e here will be helpful. I will add it later.

@PraxTube
Copy link
Collaborator

I can see your point. Leaving it in as is is probably best.

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

No branches or pull requests

2 participants