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

Consider adding KeyEvent::Enter #234

Closed
MonsieurMan opened this issue Sep 18, 2019 · 6 comments · Fixed by #236
Closed

Consider adding KeyEvent::Enter #234

MonsieurMan opened this issue Sep 18, 2019 · 6 comments · Fixed by #236

Comments

@MonsieurMan
Copy link
Contributor

Hello !

Just began using the library and I searched for more than 5 minutes if I was having a bug with Enter not being present in the enum.

// Pressing Enter in terminal apps is pretty recurrent, so I really think
// this is easier to use and read
KeyEvent::Enter
// Than this
KeyEvent::Char('\n')

I can send a PR if you're interested

@zrzka
Copy link
Contributor

zrzka commented Sep 18, 2019

I have to say that I like the idea. But this brings my attention:

https://github.com/TimonPost/crossterm/blob/91d07275adbb15e5aab15df52d7f51808faa57dd/crossterm_input/src/input/unix_input.rs#L269

 ~ echo -e "foo\nbar"
foo
bar
 ~ echo -e "foo\nbar" | hexdump 
0000000 66 6f 6f 0a 62 61 72 0a                        
0000008
 ~ echo -e "foo\nbar" | unix2dos
foo
bar
 ~ echo -e "foo\nbar" | unix2dos | hexdump 
0000000 66 6f 6f 0d 0a 62 61 72 0d 0a                  
000000a
 ~ 

Shouldn't we ignore \r on #[cfg(unix)]? \r means nothing on UNIX. IIRC it was used on the old Macs (pre OS X, long time ago). Mentioning it here, because it's on the same line :)

@MonsieurMan
Copy link
Contributor Author

MonsieurMan commented Sep 18, 2019 via email

@zrzka
Copy link
Contributor

zrzka commented Sep 18, 2019

There're two implementations.

#[cfg(unix)]

https://github.com/TimonPost/crossterm/blob/91d07275adbb15e5aab15df52d7f51808faa57dd/crossterm_input/src/input/unix_input.rs#L269

#[cfg(windows)]

https://github.com/TimonPost/crossterm/blob/91d07275adbb15e5aab15df52d7f51808faa57dd/crossterm_input/src/input/windows_input.rs#L294

I'd say that the best solution will be to introduce KeyEvent::Enter, return it on these two lines and trash the | b'\r' in the UNIX implementation.

@MonsieurMan
Copy link
Contributor Author

Just sent a PR, thanks for the guidance !

The hardest part has been generating a new GPG key because I typed the wrong password into my password manager apparently, haha.

@TimonPost
Copy link
Member

We probably want to the same with tab which is /t now

@TimonPost
Copy link
Member

TimonPost commented Sep 19, 2019

The main reason why we used /n is that the user could just print all characters received from the input reader to the terminal. In raw modes all terminal actions are disabled as well are the new lines unless explicit written to the screen. By returning /n or /t the user could still read special events while his input is not affected by the enabled raw mode. Although I do prefer explicitly because I was confused my self with /n /t as well a few times.

december1981 pushed a commit to december1981/crossterm that referenced this issue Oct 26, 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 a pull request may close this issue.

3 participants