Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

RFC: cleaner ^C handling in REPL #5973

Merged
merged 1 commit into from

5 participants

@vtjnash
Collaborator

this strips out a the error handling code from the repl, and moves it back into base. this greatly simplifies error handling, since we no longer have two sigint handlers. it also makes cleanup after ^C in the readline repl cleaner.

the biggest change here is that EOF is now a signal for a TTY, instead of a state. I think this better reflects the unique nature of a TTY, where an EOF condition is temporary (signaled by ^D), rather than permanent (signaled by the close of a pipe or socket).

this needs to be tested on windows.

inspired by wanting to fix #1306

@StefanKarpinski

+1 this is much cleaner.

@JeffBezanson

Large net code deletion => I'm sold :)

@timholy
Owner

Ooh, I'm excited. Hoping this cleans up some oddities I've observed.

@vtjnash
Collaborator

Semi-relatedly, the bug where readline doesn't wait for confirmation before doing tab-completion is a side-effect of using the readline-completions interface (there's actually an if statement in the code that says if version == 6 && using-readline-completions, then skip-user-confirmation). If we switched to using the normal interface by replacing rl_getc_function instead, we could (a) simplify the code a lot and (b) not need to wait for readline 7 where this issue is supposed to get fixed.

@timholy I'm not sure what oddities you've observed, but I like how this code runs the julia parse loop now. It makes ^C now much better at clearing bad state, like a frozen tty.

@StefanKarpinski

We're going to kill libreadline with fire as soon as @loladiro has the time to torch it.

@Keno
Owner

Yeah, sorry about that, I'm currently extremely busy. However, if you want to, trying out the new repl is as easy as putting

if isinteractive()
        include(Pkg.dir("REPL/scripts/repl.jl"))
end

into your .juliarc.jl, so there's no need to wait for me ;).

@vtjnash vtjnash merged commit 4e48d5b into master

1 check passed

Details default The Travis CI build passed
@vtjnash vtjnash deleted the jn/eof branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 27, 2014
  1. @vtjnash

    fix #1306

    vtjnash authored
This page is out of date. Refresh to see the latest.
View
39 base/client.jl
@@ -143,7 +143,7 @@ function repl_callback(ast::ANY, show_value)
put!(repl_channel, (ast, show_value))
end
-_eval_done = Condition()
+_repl_start = Condition()
function run_repl()
global const repl_channel = RemoteRef()
@@ -152,19 +152,36 @@ function run_repl()
# install Ctrl-C interrupt handler (InterruptException)
ccall(:jl_install_sigint_handler, Void, ())
- buf = Uint8[0]
- @async begin
- while !eof(STDIN)
- read(STDIN, buf)
- ccall(:jl_read_buffer,Void,(Ptr{Void},Cssize_t),buf,1)
- if _repl_enough_stdin
- wait(_eval_done)
+ buf = Array(Uint8)
+ global _repl_enough_stdin = true
+ input = @async begin
+ try
+ while true
+ if _repl_enough_stdin
+ wait(_repl_start)
+ end
+ try
+ if eof(STDIN) # if TTY, can throw InterruptException, must be in try/catch block
+ return
+ end
+ read(STDIN, buf)
+ ccall(:jl_read_buffer,Void,(Ptr{Void},Cssize_t),buf,length(buf))
+ catch ex
+ if isa(ex,InterruptException)
+ println(STDOUT, "^C")
+ ccall(:jl_reset_input,Void,())
+ repl_callback(nothing, 0)
+ else
+ rethrow(ex)
+ end
+ end
end
+ finally
+ put!(repl_channel,(nothing,-1))
end
- put!(repl_channel,(nothing,-1))
end
- while true
+ while !istaskdone(input)
if have_color
prompt_string = "\01\033[1m\033[32m\02julia> \01\033[0m"*input_color()*"\02"
else
@@ -172,6 +189,7 @@ function run_repl()
end
ccall(:repl_callback_enable, Void, (Ptr{Uint8},), prompt_string)
global _repl_enough_stdin = false
+ notify(_repl_start)
start_reading(STDIN)
(ast, show_value) = take!(repl_channel)
if show_value == -1
@@ -179,7 +197,6 @@ function run_repl()
break
end
eval_user_input(ast, show_value!=0)
- notify(_eval_done)
end
if have_color
View
8 base/stream.jl
@@ -359,8 +359,12 @@ function _uv_hook_readcb(stream::AsyncStream, nread::Int, base::Ptr{Void}, len::
ccall(:jl_forceclose_uv,Void,(Ptr{Void},),stream.handle)
notify_error(stream.readnotify, UVError("readcb",nread))
else
- close(stream)
- notify(stream.readnotify)
+ if isa(stream,TTY)
+ notify_error(stream.readnotify, EOFError())
+ else
+ close(stream)
+ notify(stream.readnotify)
+ end
end
else
notify_filled(stream.buffer, nread, base, len)
View
68 ui/repl-basic.c
@@ -7,20 +7,7 @@ static size_t stdin_buf_maxlen = 128;
static char *given_prompt=NULL, *prompt_to_use=NULL;
static int callback_en=0;
-static void jl_clear_input(void);
-
-#ifdef __WIN32__
-static int repl_sigint_handler_installed = 0;
-static BOOL WINAPI repl_sigint_handler(DWORD wsig) //This needs winapi types to guarantee __stdcall
-{
- if (callback_en) {
- JL_WRITE(jl_uv_stdout, "^C", 2);
- jl_clear_input();
- return 1;
- }
- return 0; // continue to next handler
-}
-#else
+#ifndef __WIN32__
void sigcont_handler(int arg)
{
if (callback_en) {
@@ -28,64 +15,19 @@ void sigcont_handler(int arg)
//jl_write(jl_uv_stdout, stdin_buf, stdin_buf_len); //disabled because the current line was still in the system buffer
}
}
-
-struct sigaction jl_sigint_act = {};
-static void repl_sigint_handler(int sig, siginfo_t *info, void *context)
-{
- if (callback_en) {
- JL_WRITE(jl_uv_stdout, "\n", 1);
- jl_clear_input();
- }
- else {
- if (jl_sigint_act.sa_flags & SA_SIGINFO) {
- jl_sigint_act.sa_sigaction(sig, info, context);
- }
- else {
- void (*f)(int) = jl_sigint_act.sa_handler;
- if (f == SIG_DFL)
- raise(sig);
- else if (f != SIG_IGN)
- f(sig);
- }
- }
-}
#endif
void jl_init_repl(int history)
{
stdin_buf = malloc(stdin_buf_maxlen);
stdin_buf_len = 0;
-#ifdef __WIN32__
- repl_sigint_handler_installed = 0;
-#else
- jl_sigint_act.sa_sigaction = NULL;
+#ifndef __WIN32__
signal(SIGCONT, sigcont_handler);
#endif
}
void jl_prep_terminal(int meta_flag)
{
-#ifdef __WIN32__
- if (!repl_sigint_handler_installed) {
- if (SetConsoleCtrlHandler((PHANDLER_ROUTINE)repl_sigint_handler,1))
- repl_sigint_handler_installed = 1;
- }
-#else
- if (jl_sigint_act.sa_sigaction == NULL) {
- struct sigaction oldact, repl_sigint_act;
- memset(&repl_sigint_act, 0, sizeof(struct sigaction));
- sigemptyset(&repl_sigint_act.sa_mask);
- repl_sigint_act.sa_sigaction = repl_sigint_handler;
- repl_sigint_act.sa_flags = SA_SIGINFO;
- if (sigaction(SIGINT, &repl_sigint_act, &oldact) < 0) {
- JL_PRINTF(JL_STDERR, "sigaction: %s\n", strerror(errno));
- jl_exit(1);
- }
- if (repl_sigint_act.sa_sigaction != oldact.sa_sigaction &&
- jl_sigint_act.sa_sigaction != oldact.sa_sigaction)
- jl_sigint_act = oldact;
- }
-#endif
}
/* Restore the terminal's normal settings and modes. */
@@ -168,8 +110,7 @@ void jl_read_buffer(unsigned char* base, ssize_t nread)
newline = 1;
break;
case '\x03':
- JL_WRITE(jl_uv_stdout, "^C\n", 3);
- jl_clear_input();
+ raise(SIGINT);
break;
case '\x04':
raise(SIGTERM);
@@ -212,10 +153,9 @@ void jl_read_buffer(unsigned char* base, ssize_t nread)
if (newline) basic_stdin_callback();
}
-static void jl_clear_input(void)
+DLLEXPORT void jl_reset_input(void)
{
stdin_buf_len = 0;
stdin_buf[0] = 0;
JL_WRITE(jl_uv_stdout,"\n",1);
- repl_callback_enable(prompt_to_use);
}
View
76 ui/repl-readline.c
@@ -26,8 +26,6 @@
extern int asprintf(char **strp, const char *fmt, ...);
-static void jl_clear_input(void);
-
#define USE_READLINE_STATIC
#include <readline/readline.h>
#include <readline/history.h>
@@ -555,7 +553,7 @@ static int callback_en=0;
void jl_input_line_callback(char *input)
{
int end=0, doprint=1;
- if (!input || ios_eof(ios_stdin)) {
+ if (!input) {
end = 1;
rl_ast = NULL;
}
@@ -736,18 +734,7 @@ static char *do_completions(const char *ch, int c)
return ptr ? strdup(ptr) : NULL;
}
-#ifdef __WIN32__
-static int repl_sigint_handler_installed = 0;
-static BOOL WINAPI repl_sigint_handler(DWORD wsig) //This needs winapi types to guarantee __stdcall
-{
- if (callback_en) {
- JL_WRITE(jl_uv_stdout, "^C", 2);
- jl_clear_input();
- return 1;
- }
- return 0; // continue to next handler
-}
-#else
+#ifndef __WIN32__
void sigtstp_handler(int arg)
{
rl_cleanup_after_signal();
@@ -768,28 +755,6 @@ void sigcont_handler(int arg)
if (callback_en)
rl_forced_update_display();
}
-
-struct sigaction jl_sigint_act = {{0}};
-
-static void repl_sigint_handler(int sig, siginfo_t *info, void *context)
-{
- if (callback_en) {
- JL_WRITE(jl_uv_stdout, "^C", 2);
- jl_clear_input();
- }
- else {
- if (jl_sigint_act.sa_flags & SA_SIGINFO) {
- jl_sigint_act.sa_sigaction(sig, info, context);
- }
- else {
- void (*f)(int) = jl_sigint_act.sa_handler;
- if (f == SIG_DFL)
- raise(sig);
- else if (f != SIG_IGN)
- f(sig);
- }
- }
-}
#endif
static void init_rl(void)
@@ -843,25 +808,6 @@ void jl_prep_terminal(int meta_flag)
rl_instream = rl_in;
#ifdef __WIN32__
if (jl_uv_stdin->type == UV_TTY) uv_tty_set_mode((uv_tty_t*)jl_uv_stdin,1); //raw (and libuv-processed)
- if (!repl_sigint_handler_installed) {
- if (SetConsoleCtrlHandler((PHANDLER_ROUTINE)repl_sigint_handler,1))
- repl_sigint_handler_installed = 1;
- }
-#else
- if (jl_sigint_act.sa_sigaction == NULL) {
- struct sigaction oldact, repl_sigint_act;
- memset(&repl_sigint_act, 0, sizeof(struct sigaction));
- sigemptyset(&repl_sigint_act.sa_mask);
- repl_sigint_act.sa_sigaction = repl_sigint_handler;
- repl_sigint_act.sa_flags = SA_SIGINFO;
- if (sigaction(SIGINT, &repl_sigint_act, &oldact) < 0) {
- JL_PRINTF(JL_STDERR, "sigaction: %s\n", strerror(errno));
- jl_exit(1);
- }
- if (repl_sigint_act.sa_sigaction != oldact.sa_sigaction &&
- jl_sigint_act.sa_sigaction != oldact.sa_sigaction)
- jl_sigint_act = oldact;
- }
#endif
}
/* Restore the terminal's normal settings and modes. */
@@ -882,9 +828,6 @@ void jl_init_repl(int history)
#ifdef __WIN32__
rl_outstream=(void*)jl_uv_stdout;
- repl_sigint_handler_installed = 0;
-#else
- jl_sigint_act.sa_sigaction = NULL;
#endif
rl_catch_signals = 0;
rl_prep_term_function = &jl_prep_terminal;
@@ -920,9 +863,8 @@ void jl_read_buffer(unsigned char *base, ssize_t nread)
rl_callback_read_char();
}
-static void jl_clear_input(void)
+DLLEXPORT void jl_reset_input(void)
{
- //todo: how to do this better / the correct way / ???
//move the cursor to a clean line:
char *p = rl_line_buffer;
int i;
@@ -931,16 +873,10 @@ static void jl_clear_input(void)
jl_putc('\n', jl_uv_stdout);
}
}
- jl_putc('\n', jl_uv_stdout);
- jl_putc('\n', jl_uv_stdout);
+ rl_ast = NULL;
//reset state:
rl_reset_line_state();
reset_indent();
- rl_initialize();
- //and redisplay prompt:
- rl_forced_update_display();
- rl_on_new_line_with_prompt();
-#ifdef __WIN32__
- jl_write(jl_uv_stdout, "\e[4C", 4); //hack: try to fix cursor location
-#endif
+ callback_en = 0;
+ rl_callback_handler_remove();
}
Something went wrong with that request. Please try again.