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 most annoying part of #8 #17

Merged
merged 2 commits into from
Oct 22, 2017
Merged

Fix most annoying part of #8 #17

merged 2 commits into from
Oct 22, 2017

Conversation

ctrlcctrlv
Copy link
Contributor

Backspace works now on my system. I can't think of a good reason for us
to be defining our own KEY_BS rather than using ncurses' KEY_BACKSPACE -
so let's stop doing that!

Backspace works now on my system. I can't think of a good reason for us
to be defining our own KEY_BS rather than using ncurses' KEY_BACKSPACE -
so let's stop doing that!
@FedeDP
Copy link
Collaborator

FedeDP commented Oct 22, 2017

https://stackoverflow.com/questions/27200597/c-ncurses-key-backspace-not-working
I needed that change because on my system KEY_BACKSPACE is not properly caught.
Can you update PR keeping both? Thanks!

@ctrlcctrlv
Copy link
Contributor Author

How should I do that? Just simple

case KEY_BS:
case KEY_BACKSPACE:

?

What if 127 maps to something else on another system? Then no one will be able to input that key. I think it would be better for you to figure out why your terminal emulator isn't handling this properly, you might be missing a terminfo file, or using the wrong one.

@FedeDP
Copy link
Collaborator

FedeDP commented Oct 22, 2017

I'll have a look, thanks for the hint!

@ctrlcctrlv
Copy link
Contributor Author

Sure :) If I knew your terminal emulator I could give you more help; for example I use termite, which requires its own terminfo file. Sometimes doing export TERM=xterm-256color can solve problems like these, because most terminal emulators are xterm-256color compatible, but it's always better to be using the correct termcap file with the same name as $TERM.

@FedeDP
Copy link
Collaborator

FedeDP commented Oct 22, 2017

I'm using konsole, infocmp shows this: http://dpaste.com/0S9Z810.
Moreover, from here: http://comp.unix.programmer.narkive.com/GTteX1j1/ncurses-backspace#post3:

For many terminals, ^? is the code that indicates the backspace
key. For others, it is ^H.

It sounds like your terminal is sending ^? but your terminal
configuration is telling ncurses that your terminal will send ^H.
It's possible that your TERM variable is set incorrectly, or perhaps
you've unknowingly used stty(1) to change your terminal configuration.

Because both of these codes (^? and ^H) are so commonly associated
with the backspace key in the Unix, a lot of terminal software is
hacked to accept either key as backspace, regardless of the actual
configuration. The result of this fact is that people will have their
backspace key misconfigured and won't know it until they test some
software that they've written themselves.

What do you think? Should be support both or force KEY_BACKSPACE?

@ctrlcctrlv
Copy link
Contributor Author

I think we should force KEY_BACKSPACE because encouraging people to do the wrong thing by making it easier for them to do so is itself wrong, and we could refer to this issue if anyone else asks.

But if you insist on hacking 127 in I can edit my PR to do so.

@FedeDP
Copy link
Collaborator

FedeDP commented Oct 22, 2017

For me it's ok to only support BACKSPACE.
Can you update the pr rebasing on current master too? :)

@ctrlcctrlv
Copy link
Contributor Author

Rebased 😄

@FedeDP FedeDP merged commit 8dcdef0 into ga2arch:master Oct 22, 2017
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