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

Fix #5, Changed behaviour of delete key #14

Merged
merged 3 commits into from
Apr 18, 2023
Merged

Conversation

wader
Copy link
Collaborator

@wader wader commented Mar 19, 2023

  • Changed current delete to CharEOT, added new CharDelete key

wader notes:
Don't expect more char after ex esc rune
Kick reader even if no char is deleted

@wader
Copy link
Collaborator Author

wader commented Mar 19, 2023

Needs cleanup and can't find the repo where i found this fix. Also i've fixed a bug in the original commit where deleting at the end of the line would cause readline to stop writing outpu at all.

@wader
Copy link
Collaborator Author

wader commented Mar 19, 2023

Also related wader/readline@2342177 (Support for windows delete key)

* Changed current delete to CharEOT, added new CharDelete key
@wader wader force-pushed the windows-del-support branch from b506c9d to 23d74df Compare April 1, 2023 09:07
@wader
Copy link
Collaborator Author

wader commented Apr 1, 2023

Rebased on master. Haven't been able to test things as i'm on laptop with no del key, will test later.

Also have to remember what it was that this actually fixed, i think it made the del key work properly in some cases, but it might depend on how the terminal is configured which is confusing. I also had some changes to fix a bug that it introduced (when deleting last on a line when there is no character to delete) related to kick reader. But after rebase on master i see that that stuff has been reworked so maybe it got fixed?

@slingamn
Copy link
Member

slingamn commented Apr 2, 2023

Yep, got rid of public KickRead in #16 :-)

Sorry, what does EOT stand for? Also, what is the intended change in behavior here?

@wader
Copy link
Collaborator Author

wader commented Apr 2, 2023

I honestly don't remember what it actually fixed now but i do remember that it had to do with del-key/ctrl-d behavior and that del-key on an empty readline should not exit the prompt. What makes it confusing is that what terminals sends for del-key/ctrl-d seems to differ and is also configurable.

EOT is probably (ASCII?) end-of-transmission in this context.

Will try to find the original commit, maybe that will shine some light also. I mess up so the original author got lost also.

@wader
Copy link
Collaborator Author

wader commented Apr 2, 2023

The original author is @stdiopt but can't find repo where is originated from :(

@slingamn
Copy link
Member

slingamn commented Apr 3, 2023

del-key on an empty readline should not exit the prompt.

Thanks, I can reproduce this --- I'll treat this as a bug report and then see how to adapt this patch to achieve the desired outcome.

@wader
Copy link
Collaborator Author

wader commented Apr 3, 2023

Great!

@slingamn
Copy link
Member

I finally understand this patch! I made a minor change, I'll probably merge this soon.

@slingamn slingamn force-pushed the windows-del-support branch from a64acbe to 8f36d97 Compare April 17, 2023 07:05
terminal.go Show resolved Hide resolved
operation.go Outdated Show resolved Hide resolved
@wader
Copy link
Collaborator Author

wader commented Apr 17, 2023

Hehe, don't think i remember how it worked anymore :) it usually take some time to get into terminal mode to remember these things.

@slingamn
Copy link
Member

This is a pretty good summary FWIW https://gist.github.com/CMCDragonkai/e45d062aad3be6a3be26da76694fdfbf

@wader
Copy link
Collaborator Author

wader commented Apr 17, 2023

LGTM

@slingamn slingamn merged commit 86cc349 into master Apr 18, 2023
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.

2 participants