troglobit / editline

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

Optionally return signal type when got a signal #28

Closed abitmore closed 5 years ago

abitmore commented 5 years ago

This feature is useful when need to immediately handle the signal after called readline().

troglobit commented 5 years ago

This is in general a good idea, but if we go the path of signals I'd much rather see we looked at the GNU Readline APIs. Because Editline aims to be a call compatible replacement (with less overhead) and implementing our own APIs for the same thing is best to avoid (principle of least surprise ...)

Here's a stackoverflow thread that sort of addresses what I believe you're aiming for: https://stackoverflow.com/a/27662212

Also, even if I'd accepted the pull request, it does not follow the coding style. Braces and comment style are a couple of things that stand out.

Sorry for nixing your second PR like this, I feel really bad about it, but I also have to look after how Editline evolves, so I hope you understand it's nothing personal?

abitmore commented 5 years ago

No problem, I understand at all.

In this PR I tried to address another issue but not #27, E.G. in case when the user pressed Ctrl+C during input. My code calls readline() in a loop, but the signal handling code is in another thread, so there is a race condition, although readline() signaled the process a SIGINT when processing the Ctrl+C, the signal is probably processed too late, at that time the loop may (or may not) have started a new readline() call and got stuck there. If I go with the callback interface, the race condition is still there. So actually this PR is a dirty hack to get around the race condition, I admit that it looks ugly. Ideally I should fix my own code (but I haven't found an easy way to do so). BTW coding style is easy to fix.

Thanks for the links anyway.

abitmore commented 5 years ago

This PR doesn't fix #27, but I need to fix #27 anyway. Actually, if #27 is fixed, the race condition mentioned above become less of a problem, at least my program would be able to break out during the next realine() call, which is acceptable for me (although not perfect due to the new prompt).

troglobit commented 5 years ago

Hmm, personally I try to avoid threads as much as possible, but for existing applications that's usually out of the question. Aren't you supposed to have a signal handler per thread?

Edit: Just FYI, in case you missed it, there is a --disable-sigint configure option which may be interesting for you. With that Editline will not send SIGINT to itself on Ctrl-C ...