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.
This commit is contained in:
ridiculousfish 2019-05-27 14:52:48 -07:00
parent 3f1d7bbdc5
commit 4a2c709fb1
20 changed files with 85 additions and 104 deletions

View file

@ -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;
}

View file

@ -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());
}

View file

@ -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;
}

View file

@ -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<bool> 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()) {

View file

@ -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.

View file

@ -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."));
}

View file

@ -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<completion_t> 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);

View file

@ -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<bool> 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));
}
}

View file

@ -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<bool> 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);

View file

@ -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<bool> 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());
}
}
}

View file

@ -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,

View file

@ -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<g::job> 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<g::job> 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_t> job = std::make_shared<job_t>(acquire_job_id(), props, block_io, parent_job);

View file

@ -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) {

View file

@ -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');

View file

@ -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<parser_t> {
/// 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;

View file

@ -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<int> 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) {

View file

@ -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);

View file

@ -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<bool> 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<bool> 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<bool> 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;
}

View file

@ -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;

View file

@ -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.
///