From 4a2c709fb18f4fe6c175b5d025295b1d8d05698a Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Mon, 27 May 2019 14:52:48 -0700 Subject: [PATCH] Eliminate shell_is_interactive We used to have a global notion of "is the shell interactive" but soon we will want to have multiple independent execution threads, only some of which may be interactive. Start tracking this data per-parser. --- src/builtin.cpp | 2 +- src/builtin_cd.cpp | 4 ++-- src/builtin_complete.cpp | 3 ++- src/builtin_read.cpp | 4 ++-- src/builtin_set.cpp | 10 +++++----- src/common.cpp | 7 +++++-- src/complete.cpp | 12 ++++++------ src/event.cpp | 3 +-- src/fish_key_reader.cpp | 4 +++- src/fish_tests.cpp | 11 ++++++----- src/parse_constants.h | 6 +++--- src/parse_execution.cpp | 8 ++++---- src/parse_tree.cpp | 4 ++-- src/parser.cpp | 13 +++++-------- src/parser.h | 7 +++++++ src/proc.cpp | 36 +++--------------------------------- src/proc.h | 9 --------- src/reader.cpp | 29 ++++++++++++++--------------- src/signal.cpp | 12 ++++++++++-- src/signal.h | 5 ++++- 20 files changed, 85 insertions(+), 104 deletions(-) diff --git a/src/builtin.cpp b/src/builtin.cpp index 9b91f2f12..fd6b51331 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -304,7 +304,7 @@ static int builtin_breakpoint(parser_t &parser, io_streams_t &streams, wchar_t * } // If we're not interactive then we can't enter the debugger. So treat this command as a no-op. - if (!shell_is_interactive()) { + if (!parser.is_interactive()) { return STATUS_CMD_ERROR; } diff --git a/src/builtin_cd.cpp b/src/builtin_cd.cpp index 57adc84b4..ae34ceb12 100644 --- a/src/builtin_cd.cpp +++ b/src/builtin_cd.cpp @@ -61,7 +61,7 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { cmd, dir_in.c_str()); } - if (!shell_is_interactive()) streams.err.append(parser.current_line()); + if (!parser.is_interactive()) streams.err.append(parser.current_line()); return STATUS_CMD_ERROR; } @@ -84,7 +84,7 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { streams.err.append_format(_(L"%ls: '%ls' is not a directory\n"), cmd, dir_in.c_str()); } - if (!shell_is_interactive()) { + if (!parser.is_interactive()) { streams.err.append(parser.current_line()); } diff --git a/src/builtin_complete.cpp b/src/builtin_complete.cpp index add0948cc..8f33632bb 100644 --- a/src/builtin_complete.cpp +++ b/src/builtin_complete.cpp @@ -291,7 +291,8 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) { condition); for (size_t i = 0; i < errors.size(); i++) { streams.err.append_format(L"\n%ls: ", cmd); - streams.err.append(errors.at(i).describe(condition_string)); + streams.err.append( + errors.at(i).describe(condition_string, parser.is_interactive())); } return STATUS_CMD_ERROR; } diff --git a/src/builtin_read.cpp b/src/builtin_read.cpp index 92e525cb1..12258a705 100644 --- a/src/builtin_read.cpp +++ b/src/builtin_read.cpp @@ -212,11 +212,11 @@ static int read_interactive(parser_t &parser, wcstring &buff, int nchars, bool s reader_set_silent_status(silent); reader_set_buffer(commandline, std::wcslen(commandline)); - proc_push_interactive(1); + scoped_push interactive{&parser.libdata().is_interactive, true}; event_fire_generic(parser, L"fish_prompt"); auto mline = reader_readline(nchars); - proc_pop_interactive(); + interactive.restore(); if (mline) { buff = mline.acquire(); if (nchars > 0 && (size_t)nchars < buff.size()) { diff --git a/src/builtin_set.cpp b/src/builtin_set.cpp index 3e6d913cc..303f089d8 100644 --- a/src/builtin_set.cpp +++ b/src/builtin_set.cpp @@ -234,10 +234,10 @@ static int validate_cmd_opts(const wchar_t *cmd, set_cmd_opts_t &opts, //!OCLIN // Check if we are setting a uvar and a global of the same name exists. See // https://github.com/fish-shell/fish-shell/issues/806 static int check_global_scope_exists(const wchar_t *cmd, set_cmd_opts_t &opts, const wchar_t *dest, - io_streams_t &streams, const environment_t &vars) { + io_streams_t &streams, const parser_t &parser) { if (opts.universal) { - auto global_dest = vars.get(dest, ENV_GLOBAL); - if (global_dest && shell_is_interactive()) { + auto global_dest = parser.vars().get(dest, ENV_GLOBAL); + if (global_dest && parser.is_interactive()) { streams.err.append_format(BUILTIN_SET_UVAR_ERR, cmd, dest); } } @@ -673,7 +673,7 @@ static int builtin_set_erase(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, } if (retval != STATUS_CMD_OK) return retval; - return check_global_scope_exists(cmd, opts, dest, streams, parser.vars()); + return check_global_scope_exists(cmd, opts, dest, streams, parser); } /// This handles the common case of setting the entire var to a set of values. @@ -785,7 +785,7 @@ static int builtin_set_set(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, w } if (retval != STATUS_CMD_OK) return retval; - return check_global_scope_exists(cmd, opts, varname, streams, parser.vars()); + return check_global_scope_exists(cmd, opts, varname, streams, parser); } /// The set builtin creates, updates, and erases (removes, deletes) variables. diff --git a/src/common.cpp b/src/common.cpp index 3d418657d..54856d3af 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -56,6 +56,7 @@ #include "future_feature_flags.h" #include "global_safety.h" #include "iothread.h" +#include "parser.h" #include "proc.h" #include "signal.h" #include "wildcard.h" @@ -1791,7 +1792,8 @@ void common_handle_winch(int signal) { static void validate_new_termsize(struct winsize *new_termsize, const environment_t &vars) { if (new_termsize->ws_col == 0 || new_termsize->ws_row == 0) { #ifdef HAVE_WINSIZE - if (shell_is_interactive()) { + // Highly hackish. This seems like it should be moved. + if (is_main_thread() && parser_t::principal_parser().is_interactive()) { debug(1, _(L"Current terminal parameters have rows and/or columns set to zero.")); debug(1, _(L"The stty command can be used to correct this " L"(e.g., stty rows 80 columns 24).")); @@ -1814,7 +1816,8 @@ static void validate_new_termsize(struct winsize *new_termsize, const environmen } if (new_termsize->ws_col < MIN_TERM_COL || new_termsize->ws_row < MIN_TERM_ROW) { - if (shell_is_interactive()) { + // Also highly hackisk. + if (is_main_thread() && parser_t::principal_parser().is_interactive()) { debug(1, _(L"Current terminal parameters set terminal size to unreasonable value.")); debug(1, _(L"Defaulting terminal size to 80x24.")); } diff --git a/src/complete.cpp b/src/complete.cpp index c712b6b2c..e106d5245 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -748,10 +748,10 @@ void completer_t::complete_from_args(const wcstring &str, const wcstring &args, const wcstring &desc, complete_flags_t flags) { bool is_autosuggest = (this->type() == COMPLETE_AUTOSUGGEST); - // If type is COMPLETE_AUTOSUGGEST, it means we're on a background thread, so don't call - // proc_push_interactive. - if (!is_autosuggest) { - proc_push_interactive(0); + bool saved_interactive = false; + if (parser) { + saved_interactive = parser->libdata().is_interactive; + parser->libdata().is_interactive = false; } expand_flags_t eflags{}; @@ -763,8 +763,8 @@ void completer_t::complete_from_args(const wcstring &str, const wcstring &args, std::vector possible_comp = parser_t::expand_argument_list(args, eflags, vars, parser); - if (!is_autosuggest) { - proc_pop_interactive(); + if (parser) { + parser->libdata().is_interactive = saved_interactive; } this->complete_strings(escape_string(str, ESCAPE_ALL), const_desc(desc), possible_comp, flags); diff --git a/src/event.cpp b/src/event.cpp index 19d9778e8..cda927517 100644 --- a/src/event.cpp +++ b/src/event.cpp @@ -276,13 +276,12 @@ static void event_fire_internal(parser_t &parser, const event_t &event) { // Event handlers are not part of the main flow of code, so they are marked as // non-interactive. - proc_push_interactive(0); + scoped_push interactive{&ld.is_interactive, false}; auto prev_statuses = parser.get_last_statuses(); block_t *b = parser.push_block(block_t::event_block(event)); parser.eval(buffer, io_chain_t(), TOP); parser.pop_block(b); - proc_pop_interactive(); parser.set_last_statuses(std::move(prev_statuses)); } } diff --git a/src/fish_key_reader.cpp b/src/fish_key_reader.cpp index d949bc8af..7dd58e5ab 100644 --- a/src/fish_key_reader.cpp +++ b/src/fish_key_reader.cpp @@ -29,6 +29,7 @@ #include "fish_version.h" #include "input.h" #include "input_common.h" +#include "parser.h" #include "print_help.h" #include "proc.h" #include "reader.h" @@ -285,9 +286,10 @@ static void setup_and_process_keys(bool continuous_mode) { set_interactive_session(true); // by definition this program is interactive set_main_thread(); setup_fork_guards(); - proc_push_interactive(1); env_init(); reader_init(); + parser_t &parser = parser_t::principal_parser(); + scoped_push interactive{&parser.libdata().is_interactive, true}; // We need to set the shell-modes for ICRNL, // in fish-proper this is done once a command is run. tcsetattr(STDIN_FILENO, TCSANOW, &shell_modes); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 74dbd22b6..e60b03e8f 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -1013,8 +1013,9 @@ static void test_cancellation() { // Enable fish's signal handling here. We need to make this interactive for fish to install its // signal handlers. - proc_push_interactive(1); - signal_set_handlers(); + parser_t &parser = parser_t::principal_parser(); + scoped_push interactive{&parser.libdata().is_interactive, true}; + signal_set_handlers(true); // This tests that we can correctly ctrl-C out of certain loop constructs, and that nothing gets // printed if we do. @@ -1040,7 +1041,7 @@ static void test_cancellation() { set_interactive_session(iis); // Restore signal handling. - proc_pop_interactive(); + interactive.restore(); signal_reset_handlers(); // Ensure that we don't think we should cancel. @@ -1613,7 +1614,7 @@ static bool expand_test(const wchar_t *in, expand_flags_t flags, ...) { if (errors.empty()) { err(L"Bug: Parse error reported but no error text found."); } else { - err(L"%ls", errors.at(0).describe(wcstring(in)).c_str()); + err(L"%ls", errors.at(0).describe(in, parser->is_interactive()).c_str()); } return false; } @@ -4237,7 +4238,7 @@ static void test_new_parser_errors() { L"code %lu", src.c_str(), expected_code, (unsigned long)errors.at(0).code); for (size_t i = 0; i < errors.size(); i++) { - err(L"\t\t%ls", errors.at(i).describe(src).c_str()); + err(L"\t\t%ls", errors.at(i).describe(src, true).c_str()); } } } diff --git a/src/parse_constants.h b/src/parse_constants.h index 5c234c505..2c916ac5e 100644 --- a/src/parse_constants.h +++ b/src/parse_constants.h @@ -187,9 +187,9 @@ struct parse_error_t { /// Offset and length of the token in the source code that triggered this error. size_t source_start; size_t source_length; - /// Return a string describing the error, suitable for presentation to the user. If skip_caret - /// is false, the offending line with a caret is printed as well. - wcstring describe(const wcstring &src) const; + /// Return a string describing the error, suitable for presentation to the user. If + /// is_interactive is true, the offending line with a caret is printed as well. + wcstring describe(const wcstring &src, bool is_interactive) const; /// Return a string describing the error, suitable for presentation to the user, with the given /// prefix. If skip_caret is false, the offending line with a caret is printed as well. wcstring describe_with_prefix(const wcstring &src, const wcstring &prefix, bool is_interactive, diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index ce7e8bad5..1f82c768b 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -800,7 +800,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process( } // Protect against exec with background processes running - if (process_type == process_type_t::exec && shell_is_interactive()) { + if (process_type == process_type_t::exec && parser->is_interactive()) { bool have_bg = false; for (const auto &bg : parser->jobs()) { // The assumption here is that if it is a foreground job, @@ -1165,7 +1165,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(tnode_t jo // Get terminal modes. struct termios tmodes = {}; - if (shell_is_interactive() && tcgetattr(STDIN_FILENO, &tmodes)) { + if (parser->is_interactive() && tcgetattr(STDIN_FILENO, &tmodes)) { // Need real error handling here. wperror(L"tcgetattr"); return parse_execution_errored; @@ -1236,13 +1236,13 @@ parse_execution_result_t parse_execution_context_t::run_1_job(tnode_t jo auto job_control_mode = get_job_control_mode(); bool wants_job_control = (job_control_mode == job_control_t::all) || - ((job_control_mode == job_control_t::interactive) && shell_is_interactive()); + ((job_control_mode == job_control_t::interactive) && parser->is_interactive()); job_t::properties_t props{}; props.foreground = !job_node_is_background(job_node); props.wants_terminal = wants_job_control && !ld.is_event; props.skip_notification = - ld.is_subshell || ld.is_block || ld.is_event || !shell_is_interactive(); + ld.is_subshell || ld.is_block || ld.is_event || !parser->is_interactive(); props.from_event_handler = ld.is_event; shared_ptr job = std::make_shared(acquire_job_id(), props, block_io, parent_job); diff --git a/src/parse_tree.cpp b/src/parse_tree.cpp index 743a80509..c840f29ed 100644 --- a/src/parse_tree.cpp +++ b/src/parse_tree.cpp @@ -117,8 +117,8 @@ wcstring parse_error_t::describe_with_prefix(const wcstring &src, const wcstring return result; } -wcstring parse_error_t::describe(const wcstring &src) const { - return this->describe_with_prefix(src, wcstring(), shell_is_interactive(), false); +wcstring parse_error_t::describe(const wcstring &src, bool is_interactive) const { + return this->describe_with_prefix(src, wcstring(), is_interactive, false); } void parse_error_offset_source_start(parse_error_list_t *errors, size_t amt) { diff --git a/src/parser.cpp b/src/parser.cpp index 72b58612e..73d7e10b7 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -524,7 +524,7 @@ wcstring parser_t::current_line() { wcstring prefix; // If we are not going to print a stack trace, at least print the line number and filename. - if (!shell_is_interactive() || is_function()) { + if (!is_interactive() || is_function()) { if (file) { append_format(prefix, _(L"%ls (line %d): "), user_presentable_path(file).c_str(), lineno); @@ -535,8 +535,7 @@ wcstring parser_t::current_line() { } } - bool is_interactive = shell_is_interactive(); - bool skip_caret = is_interactive && !is_function(); + bool skip_caret = is_interactive() && !is_function(); // Use an error with empty text. assert(source_offset >= 0); @@ -544,7 +543,7 @@ wcstring parser_t::current_line() { empty_error.source_start = source_offset; wcstring line_info = empty_error.describe_with_prefix(execution_context->get_source(), prefix, - is_interactive, skip_caret); + is_interactive(), skip_caret); if (!line_info.empty()) { line_info.push_back(L'\n'); } @@ -721,8 +720,6 @@ void parser_t::get_backtrace(const wcstring &src, const parse_error_list_t &erro if (!errors.empty()) { const parse_error_t &err = errors.at(0); - const bool is_interactive = shell_is_interactive(); - // Determine if we want to try to print a caret to point at the source error. The // err.source_start <= src.size() check is due to the nasty way that slices work, which is // by rewriting the source. @@ -734,7 +731,7 @@ void parser_t::get_backtrace(const wcstring &src, const parse_error_list_t &erro // Don't include the caret if we're interactive, this is the first line of text, and our // source is at its beginning, because then it's obvious. - skip_caret = (is_interactive && which_line == 1 && err.source_start == 0); + skip_caret = (is_interactive() && which_line == 1 && err.source_start == 0); } wcstring prefix; @@ -751,7 +748,7 @@ void parser_t::get_backtrace(const wcstring &src, const parse_error_list_t &erro } const wcstring description = - err.describe_with_prefix(src, prefix, is_interactive, skip_caret); + err.describe_with_prefix(src, prefix, is_interactive(), skip_caret); if (!description.empty()) { output.append(description); output.push_back(L'\n'); diff --git a/src/parser.h b/src/parser.h index 77d73cc10..c522be72d 100644 --- a/src/parser.h +++ b/src/parser.h @@ -152,6 +152,9 @@ struct library_data_t { /// event nesting level. int is_event{0}; + /// Whether we are currently interactive. + bool is_interactive{false}; + /// Whether we should break or continue the current loop. enum loop_status_t loop_status { loop_status_t::normals }; @@ -352,6 +355,10 @@ class parser_t : public std::enable_shared_from_this { /// than the one curently read. const wchar_t *current_filename() const; + /// Return if we are interactive, which means we are executing a command that the user typed in + /// (and not, say, a prompt). + bool is_interactive() const { return libdata().is_interactive; } + /// Return a string representing the current stack trace. wcstring stack_trace() const; diff --git a/src/proc.cpp b/src/proc.cpp index 87eb0bf72..c3e055864 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -77,21 +77,7 @@ job_control_t get_job_control_mode() { return job_control_mode; } void set_job_control_mode(job_control_t mode) { job_control_mode = mode; } -static int is_interactive = -1; - -bool shell_is_interactive() { - ASSERT_IS_MAIN_THREAD(); - // is_interactive is statically initialized to -1. Ensure it has been dynamically set - // before we're called. - assert(is_interactive != -1); - return is_interactive > 0; -} - -/// A stack containing the values of is_interactive. Used by proc_push_interactive and -/// proc_pop_interactive. -static std::vector interactive_stack; - -void proc_init() { proc_push_interactive(0); } +void proc_init() { signal_set_handlers_once(false); } // Basic thread safe job IDs. The vector consumed_job_ids has a true value wherever the job ID // corresponding to that slot is in use. The job ID corresponding to slot 0 is 1. @@ -860,7 +846,7 @@ void job_t::continue_job(parser_t &parser, bool reclaim_foreground_pgrp, bool se FLOGF(proc_job_run, L"%ls job %d, gid %d (%ls), %ls, %ls", send_sigcont ? L"Continue" : L"Start", job_id, pgid, command_wcstr(), is_completed() ? L"COMPLETED" : L"UNCOMPLETED", - is_interactive ? L"INTERACTIVE" : L"NON-INTERACTIVE"); + parser.libdata().is_interactive ? L"INTERACTIVE" : L"NON-INTERACTIVE"); // Make sure we retake control of the terminal before leaving this function. bool term_transferred = false; @@ -956,26 +942,10 @@ void proc_sanity_check(const parser_t &parser) { } } -void proc_push_interactive(int value) { - ASSERT_IS_MAIN_THREAD(); - int old = is_interactive; - interactive_stack.push_back(is_interactive); - is_interactive = value; - if (old != value) signal_set_handlers(); -} - -void proc_pop_interactive() { - ASSERT_IS_MAIN_THREAD(); - int old = is_interactive; - is_interactive = interactive_stack.back(); - interactive_stack.pop_back(); - if (is_interactive != old) signal_set_handlers(); -} - void proc_wait_any(parser_t &parser) { ASSERT_IS_MAIN_THREAD(); process_mark_finished_children(parser, true /* block_ok */); - process_clean_after_marking(parser, is_interactive); + process_clean_after_marking(parser, parser.libdata().is_interactive); } void hup_background_jobs(const parser_t &parser) { diff --git a/src/proc.h b/src/proc.h index 4b59440e2..414314649 100644 --- a/src/proc.h +++ b/src/proc.h @@ -456,9 +456,6 @@ class job_t { static job_t *from_pid(pid_t pid); }; -/// Whether we are reading from the keyboard right now. -bool shell_is_interactive(); - /// Whether this shell is attached to the keyboard at all. bool is_interactive_session(); void set_interactive_session(bool flag); @@ -509,12 +506,6 @@ event_t proc_create_event(const wchar_t *msg, event_type_t type, pid_t pid, int /// Initializations. void proc_init(); -/// Set new value for is_interactive flag, saving previous value. If needed, update signal handlers. -void proc_push_interactive(int value); - -/// Set is_interactive flag to the previous value. If needed, update signal handlers. -void proc_pop_interactive(); - /// Wait for any process finishing, or receipt of a signal. void proc_wait_any(parser_t &parser); diff --git a/src/reader.cpp b/src/reader.cpp index df2fc0b63..88baa3e60 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -856,6 +856,8 @@ bool reader_thread_job_is_stale() { void reader_write_title(const wcstring &cmd, parser_t &parser, bool reset_cursor_position) { if (!term_supports_setting_title()) return; + scoped_push noninteractive{&parser.libdata().is_interactive, false}; + wcstring fish_title_command = DEFAULT_TITLE; if (function_exists(L"fish_title", parser)) { fish_title_command = L"fish_title"; @@ -867,7 +869,6 @@ void reader_write_title(const wcstring &cmd, parser_t &parser, bool reset_cursor } wcstring_list_t lst; - proc_push_interactive(0); if (exec_subshell(fish_title_command, parser, lst, false /* ignore exit status */) != -1 && !lst.empty()) { std::fputws(L"\x1B]0;", stdout); @@ -877,7 +878,6 @@ void reader_write_title(const wcstring &cmd, parser_t &parser, bool reset_cursor ignore_result(write(STDOUT_FILENO, "\a", 1)); } - proc_pop_interactive(); outputter_t::stdoutput().set_color(rgb_color_t::reset(), rgb_color_t::reset()); if (reset_cursor_position && !lst.empty()) { // Put the cursor back at the beginning of the line (issue #2453). @@ -913,7 +913,7 @@ void reader_data_t::exec_prompt() { // If we have any prompts, they must be run non-interactively. if (left_prompt.size() || right_prompt.size()) { - proc_push_interactive(0); + scoped_push noninteractive{&parser().libdata().is_interactive, false}; exec_mode_prompt(); @@ -936,8 +936,6 @@ void reader_data_t::exec_prompt() { right_prompt_buff += prompt_list.at(i); } } - - proc_pop_interactive(); } // Write the screen title. Do not reset the cursor position: exec_prompt is called when there @@ -1670,7 +1668,7 @@ static bool check_for_orphaned_process(unsigned long loop_count, pid_t shell_pgi } /// Initialize data for interactive use. -static void reader_interactive_init() { +static void reader_interactive_init(parser_t &parser) { // See if we are running interactively. pid_t shell_pgid; @@ -1745,7 +1743,7 @@ static void reader_interactive_init() { } } - signal_set_handlers(); + signal_set_handlers(parser.is_interactive()); } // It shouldn't be necessary to place fish in its own process group and force control @@ -1782,7 +1780,7 @@ static void reader_interactive_init() { invalidate_termsize(); // For compatibility with fish 2.0's $_, now replaced with `status current-command` - parser_t::principal_parser().vars().set_one(L"_", ENV_GLOBAL, L"fish"); + parser.vars().set_one(L"_", ENV_GLOBAL, L"fish"); } /// Destroy data for interactive use. @@ -2097,7 +2095,7 @@ void reader_push(parser_t &parser, const wcstring &name) { reader_data_t *data = current_data(); data->command_line_changed(&data->command_line); if (reader_data_stack.size() == 1) { - reader_interactive_init(); + reader_interactive_init(parser); } data->exec_prompt(); @@ -3544,7 +3542,7 @@ int reader_read(parser_t &parser, int fd, const io_chain_t &io) { // If reader_read is called recursively through the '.' builtin, we need to preserve // is_interactive. This, and signal handler setup is handled by // proc_push_interactive/proc_pop_interactive. - int inter = 0; + bool interactive = false; // This block is a hack to work around https://sourceware.org/bugzilla/show_bug.cgi?id=20632. // See also, commit 396bf12. Without the need for this workaround we would just write: // int inter = ((fd == STDIN_FILENO) && isatty(STDIN_FILENO)); @@ -3552,19 +3550,20 @@ int reader_read(parser_t &parser, int fd, const io_chain_t &io) { struct termios t; int a_tty = isatty(STDIN_FILENO); if (a_tty) { - inter = 1; + interactive = true; } else if (tcgetattr(STDIN_FILENO, &t) == -1 && errno == EIO) { redirect_tty_output(); - inter = 1; + interactive = true; } } - proc_push_interactive(inter); - res = shell_is_interactive() ? read_i(parser) : read_ni(parser, fd, io); + scoped_push interactive_push{&parser.libdata().is_interactive, interactive}; + signal_set_handlers_once(interactive); + + res = parser.is_interactive() ? read_i(parser) : read_ni(parser, fd, io); // If the exit command was called in a script, only exit the script, not the program. reader_set_end_loop(false); - proc_pop_interactive(); return res; } diff --git a/src/signal.cpp b/src/signal.cpp index 9142fc5bd..a9837d863 100644 --- a/src/signal.cpp +++ b/src/signal.cpp @@ -317,7 +317,7 @@ static void set_interactive_handlers() { } /// Sets up appropriate signal handlers. -void signal_set_handlers() { +void signal_set_handlers(bool interactive) { struct sigaction act; act.sa_flags = 0; sigemptyset(&act.sa_mask); @@ -346,11 +346,19 @@ void signal_set_handlers() { FATAL_EXIT(); } - if (shell_is_interactive()) { + if (interactive) { set_interactive_handlers(); } } +void signal_set_handlers_once(bool interactive) { + static std::once_flag s_noninter_once; + std::call_once(s_noninter_once, signal_set_handlers, false); + + static std::once_flag s_inter_once; + if (interactive) std::call_once(s_inter_once, set_interactive_handlers); +} + void signal_handle(int sig) { struct sigaction act; diff --git a/src/signal.h b/src/signal.h index 2b94e4023..af284f36c 100644 --- a/src/signal.h +++ b/src/signal.h @@ -17,7 +17,10 @@ const wchar_t *signal_get_desc(int sig); void signal_reset_handlers(); /// Set signal handlers to fish default handlers. -void signal_set_handlers(); +void signal_set_handlers(bool interactive); + +/// Latch function. This sets signal handlers, but only the first time it is called. +void signal_set_handlers_once(bool interactive); /// Tell fish what to do on the specified signal. ///