implement status is-breakpoint

This implements `status is-breakpoint` that returns true if the current
shell prompt is displayed in the context of a `breakpoint` command.

This also fixes several bugs. Most notably making `breakpoint` a no-op if
the shell isn't interactive. Also, typing `breakpoint` at an interactive
prompt should be an error rather than creating a new nested debugging
context.

Partial fix for #1310

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Notable fixes and improvements
 - The `COLUMNS` and `LINES` env vars are now correctly set the first time `fish_prompt` is run (#4141).
+- New `status is-breakpoint` command that is true when a prompt is displayed in response to a `breakpoint` command (#1310).
 
 ## Other significant changes
 

doc_src/status.txt

@@ -6,6 +6,7 @@ status
 status is-login
 status is-interactive
 status is-block
+status is-breakpoint
 status is-command-substitution
 status is-no-job-control
 status is-full-job-control
@@ -27,6 +28,8 @@ The following operations (sub-commands) are available:
 
 - `is-block` returns 0 if fish is currently executing a block of code. Also `-b` or `--is-block`.
 
+- `is-breakpoint` returns 0 if fish is currently showing a prompt in the context of a `breakpoint` command. See also the `fish_breakpoint_prompt` function.
+
 - `is-interactive` returns 0 if fish is interactive - that is, connected to a keyboard. Also `-i` or `--is-interactive`.
 
 - `is-login` returns 0 if fish is a login shell - that is, if fish should perform login tasks such as setting up the PATH. Also `-l` or `--is-login`.

share/completions/status.fish

@@ -1,5 +1,5 @@
 # Note that when a completion file is sourced a new block scope is created so `set -l` works.
-set -l __fish_status_all_commands is-login is-interactive is-block is-command-substitution is-no-job-control is-interactive-job-control is-full-job-control current-filename current-line-number print-stack-trace job-control
+set -l __fish_status_all_commands is-login is-interactive is-block is-breakpoint is-command-substitution is-no-job-control is-interactive-job-control is-full-job-control current-filename current-line-number print-stack-trace job-control
 
 # These are the recognized flags.
 complete -c status -s h -l help --description "Display help and exit"
@@ -9,6 +9,7 @@ complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_com
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-interactive -d "Test if this is an interactive shell"
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-command-substitution -d "Test if a command substitution is currently evaluated"
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-block -d "Test if a code block is currently evaluated"
+complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-breakpoing -d "Test if a breakpoint is currently in effect"
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-no-job-control -d "Test if new jobs are never put under job control"
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-interactive-job-control -d "Test if only interactive new jobs are put under job control"
 complete -f -c status -n "not __fish_seen_subcommand_from $__fish_status_all_commands" -a is-full-job-control -d "Test if all new jobs are put under job control"

src/builtin.cpp

@@ -347,17 +347,28 @@ static int builtin_break_continue(parser_t &parser, io_streams_t &streams, wchar
 
 /// Implementation of the builtin breakpoint command, used to launch the interactive debugger.
 static int builtin_breakpoint(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
+    wchar_t *cmd = argv[0];
     if (argv[1] != NULL) {
-        streams.err.append_format(BUILTIN_ERR_ARG_COUNT1, argv[0], 0, builtin_count_args(argv));
+        streams.err.append_format(BUILTIN_ERR_ARG_COUNT1, cmd, 0, builtin_count_args(argv) - 1);
         return STATUS_INVALID_ARGS;
     }
 
+    // If we're not interactive then we can't enter the debugger. So treat this command as a no-op.
+    if (!shell_is_interactive()) {
+        return STATUS_CMD_ERROR;
+    }
+
+    // Ensure we don't allow creating a breakpoint at an interactive prompt. There may be a simpler
+    // or clearer way to do this but this works.
+    const block_t *block1 = parser.block_at_index(1);
+    if (!block1 || block1->type() == BREAKPOINT) {
+        streams.err.append_format(_(L"%ls: Command not valid at an interactive prompt\n"), cmd);
+        return STATUS_ILLEGAL_CMD;
+    }
+
     const breakpoint_block_t *bpb = parser.push_block<breakpoint_block_t>();
-
     reader_read(STDIN_FILENO, streams.io_chain ? *streams.io_chain : io_chain_t());
-
     parser.pop_block(bpb);
-
     return proc_get_last_status();
 }
 

src/builtin.h

@@ -69,6 +69,10 @@ enum { COMMAND_NOT_BUILTIN, BUILTIN_REGULAR, BUILTIN_FUNCTION };
 /// Error message when number expected
 #define BUILTIN_ERR_NOT_NUMBER _(L"%ls: Argument '%ls' is not a number\n")
 
+/// Command that requires a subcommand was invoked without a recognized subcommand.
+#define BUILTIN_ERR_MISSING_SUBCMD _(L"%ls: Expected a subcommand to follow the command\n")
+#define BUILTIN_ERR_INVALID_SUBCMD _(L"%ls: Subcommand '%ls' is not valid\n")
+
 /// The send stuff to foreground message.
 #define FG_MSG _(L"Send job %d, '%ls' to foreground\n")
 

src/builtin_status.cpp

@@ -20,6 +20,7 @@ enum status_cmd_t {
     STATUS_IS_LOGIN = 1,
     STATUS_IS_INTERACTIVE,
     STATUS_IS_BLOCK,
+    STATUS_IS_BREAKPOINT,
     STATUS_IS_COMMAND_SUB,
     STATUS_IS_FULL_JOB_CTRL,
     STATUS_IS_INTERACTIVE_JOB_CTRL,
@@ -38,6 +39,7 @@ const enum_map<status_cmd_t> status_enum_map[] = {
     {STATUS_CURRENT_FUNCTION, L"current-function"},
     {STATUS_CURRENT_LINE_NUMBER, L"current-line-number"},
     {STATUS_IS_BLOCK, L"is-block"},
+    {STATUS_IS_BREAKPOINT, L"is-breakpoint"},
     {STATUS_IS_COMMAND_SUB, L"is-command-substitution"},
     {STATUS_IS_FULL_JOB_CTRL, L"is-full-job-control"},
     {STATUS_IS_INTERACTIVE, L"is-interactive"},
@@ -237,6 +239,9 @@ int builtin_status(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
                 return STATUS_CMD_ERROR;
             }
             optind++;
+        } else {
+            streams.err.append_format(BUILTIN_ERR_INVALID_SUBCMD, cmd, argv[1]);
+            return STATUS_INVALID_ARGS;
         }
     }
 
@@ -315,6 +320,11 @@ int builtin_status(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
             retval = !is_block;
             break;
         }
+        case STATUS_IS_BREAKPOINT: {
+            CHECK_FOR_UNEXPECTED_STATUS_ARGS(opts.status_cmd)
+            retval = !is_breakpoint;
+            break;
+        }
         case STATUS_IS_LOGIN: {
             CHECK_FOR_UNEXPECTED_STATUS_ARGS(opts.status_cmd)
             retval = !is_login;

src/builtin_string.cpp

@@ -1208,9 +1208,10 @@ string_subcommands[] = {
 
 /// The string builtin, for manipulating strings.
 int builtin_string(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
+    wchar_t *cmd = argv[0];
     int argc = builtin_count_args(argv);
     if (argc <= 1) {
-        streams.err.append_format(_(L"string: Expected subcommand\n"));
+        streams.err.append_format(BUILTIN_ERR_MISSING_SUBCMD, cmd);
         builtin_print_help(parser, streams, L"string", streams.err);
         return STATUS_INVALID_ARGS;
     }
@@ -1224,8 +1225,8 @@ int builtin_string(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     while (subcmd->name != 0 && wcscmp(subcmd->name, argv[1]) != 0) {
         subcmd++;
     }
-    if (subcmd->handler == 0) {
+    if (!subcmd->handler) {
-        streams.err.append_format(_(L"string: Unknown subcommand '%ls'\n"), argv[1]);
+        streams.err.append_format(BUILTIN_ERR_INVALID_SUBCMD, cmd, argv[1]);
         builtin_print_help(parser, streams, L"string", streams.err);
         return STATUS_INVALID_ARGS;
     }

src/event.cpp

@@ -444,6 +444,7 @@ void event_fire(const event_t *event) {
             }
         }
         is_event--;
+        assert(is_event >= 0);
     }
 }
 

src/exec.cpp

@@ -1130,7 +1130,7 @@ void exec_job(parser_t &parser, job_t *j) {
 static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst,
                                   bool apply_exit_status) {
     ASSERT_IS_MAIN_THREAD();
-    int prev_subshell = is_subshell;
+    bool prev_subshell = is_subshell;
     const int prev_status = proc_get_last_status();
     bool split_output = false;
 
@@ -1139,7 +1139,7 @@ static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst,
         split_output = true;
     }
 
-    is_subshell = 1;
+    is_subshell = true;
     int subcommand_status = -1;  // assume the worst
 
     // IO buffer creation may fail (e.g. if we have too many open files to make a pipe), so this may

src/fish.cpp

@@ -255,11 +255,11 @@ static int fish_parse_opt(int argc, char **argv, std::vector<std::string> *cmds)
                 break;
             }
             case 'i': {
-                is_interactive_session = 1;
+                is_interactive_session = true;
                 break;
             }
             case 'l': {
-                is_login = 1;
+                is_login = true;
                 break;
             }
             case 'n': {

src/fish_tests.cpp

@@ -805,7 +805,7 @@ static void test_cancellation() {
     // Test for #3780
     // Ugly hack - temporarily set is_interactive_session
     // else we will SIGINT ourselves in response to our child death
-    scoped_push<int> iis(&is_interactive_session, 1);
+    scoped_push<bool> iis(&is_interactive_session, true);
     const wchar_t *child_self_destructor = L"while true ; sh -c 'sleep .25; kill -s INT $$' ; end";
     parser_t::principal_parser().eval(child_self_destructor, io_chain_t(), TOP);
     iis.restore();

src/parser.cpp

@@ -148,9 +148,13 @@ void parser_t::push_block_int(block_t *new_current) {
     // Push it onto our stack. This acquires ownership because of unique_ptr.
     this->block_stack.emplace_back(new_current);
 
-    // Types TOP and SUBST are not considered blocks for the purposes of `status -b`.
+    // Types TOP and SUBST are not considered blocks for the purposes of `status is-block`.
     if (type != TOP && type != SUBST) {
-        is_block = 1;
+        is_block = true;
+    }
+
+    if (type == BREAKPOINT) {
+        is_breakpoint = true;
     }
 
     if (new_current->type() != TOP) {
@@ -174,16 +178,27 @@ void parser_t::pop_block(const block_t *expected) {
 
     if (old->wants_pop_env) env_pop();
 
-    // Figure out if `status -b` should consider us to be in a block now.
+    // Figure out if `status is-block` should consider us to be in a block now.
-    int new_is_block = 0;
+    bool new_is_block = false;
     for (const auto &b : block_stack) {
         const enum block_type_t type = b->type();
         if (type != TOP && type != SUBST) {
-            new_is_block = 1;
+            new_is_block = true;
             break;
         }
     }
     is_block = new_is_block;
+
+    // Are we still in a breakpoint?
+    bool new_is_breakpoint = false;
+    for (const auto &b : block_stack) {
+        const enum block_type_t type = b->type();
+        if (type == BREAKPOINT) {
+            new_is_breakpoint = true;
+            break;
+        }
+    }
+    is_breakpoint = new_is_breakpoint;
 }
 
 const wchar_t *parser_t::get_block_desc(int block) const {

src/proc.cpp

@@ -88,11 +88,12 @@ void print_jobs(void)
 }
 #endif
 
-int is_interactive_session = 0;
+bool is_interactive_session = false;
-int is_subshell = 0;
+bool is_subshell = false;
-int is_block = 0;
+bool is_block = false;
-int is_login = 0;
+bool is_breakpoint = false;
-int is_event = 0;
+bool is_login = false;
+int is_event = false;
 pid_t proc_last_bg_pid = 0;
 int job_control_mode = JOB_CONTROL_INTERACTIVE;
 int no_exec = 0;

src/proc.h

@@ -217,22 +217,26 @@ class job_t {
     io_chain_t all_io_redirections() const;
 };
 
-/// Whether we are running a subshell command.
-extern int is_subshell;
-
-/// Whether we are running a block of commands.
-extern int is_block;
-
 /// Whether we are reading from the keyboard right now.
 bool shell_is_interactive(void);
 
+/// Whether we are running a subshell command.
+extern bool is_subshell;
+
+/// Whether we are running a block of commands.
+extern bool is_block;
+
+/// Whether we are running due to a `breakpoint` command.
+extern bool is_breakpoint;
+
 /// Whether this shell is attached to the keyboard at all.
-extern int is_interactive_session;
+extern bool is_interactive_session;
 
 /// Whether we are a login shell.
-extern int is_login;
+extern bool is_login;
 
-/// Whether we are running an event handler.
+/// Whether we are running an event handler. This is not a bool because we keep count of the event
+/// nesting level.
 extern int is_event;
 
 // List of jobs. We sometimes mutate this while iterating - hence it must be a list, not a vector

tests/string.err

@@ -4,7 +4,7 @@ string match: [
 string match: ^
 
 # string invalidarg
-string: Unknown subcommand 'invalidarg'
+string: Subcommand 'invalidarg' is not valid
 Standard input (line 183): 
 string invalidarg; and echo "unexpected exit 0" >&2
 ^