search

troglobit / editline

A small replacement for GNU readline() for UNIX
https://troglobit.com/projects/editline/
Other
283 stars 58 forks source link

allow Ctrl-D to return eof #2

Closed TobyGoodwin closed 9 years ago

TobyGoodwin commented 9 years ago

Thanks for your work on editline: it's good to see this library still maintained.

I'm puzzled as to what's going on in tty_special.

if (is_ctl_map_key(c))
   return CSdispatch;

if (c == rl_eof && rl_point == 0 && rl_end == 0)
    return CSeof;

return CSdispatch;

The second test here is for the terminal's eof character at the start of a line. But this test comes after the is_ctl_map_key test. Which means that if my eof character is a control character (which it is, being Ctrl-D) I cannot type it. (Instead editline just beeps at me, since there's no character to delete.)

It seems to me that the two tests should be swapped... which then means that the is_ctl_map_key test is redundant, since we will always return CSdispatch if we reach this point. And since this is the only place that is_ctl_map_key is called, that can go too.

Any thoughts?

TobyGoodwin commented 9 years ago

By the way, I couldn't shake the feeling that the code must once have looked like it does in my patch (without is_ctl_map_key etc), and indeed that is the case. Essentially my PR reverts one of the changes in 111fc5e1fbc6bc5f8e0b8cf0092bfccf17c44958, which mainly seems to be about something else. @troglobit can you recall why you did this?

troglobit commented 9 years ago

Hi Toby and thanks for the patch!

The original reason for 111fc5e was to fix a bug in our CLI at work where pressing Ctrl-D crashed it. Unfortunately I blindly imported that fix to the official editline repo without evaluating it first.

Thank you for cleaning up!

troglobit commented 9 years ago

I've merged your pull request manually, as a2bc89d, and added configure script switches to implement the behavior I needed myself.