WIP: print prompt to /dev/tty instead of stderr. #139
Conversation
Currently does not respect posixly_correct and is_interactive options, feedback is appreciated!
There was a problem hiding this comment.
Maybe a nice try as a first step, but it seems there is still a lot of work to be done to support the new behavior. In particular, we will need to review all code fragments that assume the output is stderr. I'm inclined to doubt it's worth the effort.
Thanks anyway, your code helped me see the current situation.
| fwrite(lebuf.buf.contents, 1, lebuf.buf.length, stderr); | ||
| fflush(stderr); | ||
| write(ttyfd, lebuf.buf.contents, lebuf.buf.length); | ||
| fsync(ttyfd); |
There was a problem hiding this comment.
The write function is not buffered. There is no need to flush the data after writing.
There was a problem hiding this comment.
The unbuffered-ness of write also means that write may only write part of the contents. You need to examine the return value of write and keep calling it until the whole contents are written.
If you don't want to do it manually, you should keep using fwrite.
There was a problem hiding this comment.
Also, you should not assume ttyfd is always open.
| @@ -193,7 +193,7 @@ int main(int argc, char **argv) | |||
| input.fd = STDIN_FILENO; | |||
| inputname = NULL; | |||
| if (!options.is_interactive_set && argc == xoptind | |||
| && isatty(STDIN_FILENO) && isatty(STDERR_FILENO)) | |||
There was a problem hiding this comment.
Removing this condition would break POSIX-conformance.
| open_ttyfd(); | ||
| is_interactive_now = is_interactive; | ||
| if (!options.do_job_control_set) | ||
| do_job_control = is_interactive; | ||
| if (do_job_control) { | ||
| open_ttyfd(); | ||
| if (do_job_control) | ||
| ensure_foreground(); |
There was a problem hiding this comment.
The ttyfd should be opened only when the shell starts job control. Especially, when the shell is non-interactive, opening the ttyfd would be useless.
| @@ -1505,7 +1506,7 @@ size_t select_list_item(size_t index, int offset, size_t listsize) | |||
| * terminal and returns true. */ | |||
| bool le_try_print_prompt(const wchar_t *s) | |||
| { | |||
| if (isatty(STDERR_FILENO) && le_setupterm(true)) { | |||
| if (le_setupterm(true)) { | |||
There was a problem hiding this comment.
Removing this condition would break things when stderr is redirected to a non-terminal file.
Currently does not respect posixly_correct and is_interactive options, feedback is appreciated!
See #138 for issue tracker