split builtin bg into its own module

Makefile.in

@@ -100,7 +100,7 @@ HAVE_DOXYGEN=@HAVE_DOXYGEN@
 #
 FISH_OBJS := obj/autoload.o obj/builtin.o obj/builtin_bind.o obj/builtin_block.o \
 	obj/builtin_commandline.o obj/builtin_emit.o obj/builtin_functions.o \
-	obj/builtin_fg.o \
+	obj/builtin_bg.o obj/builtin_fg.o \
 	obj/builtin_history.o obj/builtin_status.o obj/builtin_read.o obj/builtin_pwd.o \
 	obj/builtin_source.o obj/builtin_random.o obj/builtin_echo.o \
 	obj/builtin_cd.o obj/builtin_disown.o obj/builtin_function.o \

src/builtin.cpp

@@ -28,6 +28,7 @@
 #include <string>
 
 #include "builtin.h"
+#include "builtin_bg.h"
 #include "builtin_bind.h"
 #include "builtin_block.h"
 #include "builtin_builtin.h"
@@ -102,6 +103,36 @@ void builtin_wperror(const wchar_t *s, io_streams_t &streams) {
     }
 }
 
+static const wchar_t *short_options = L"h";
+static const struct woption long_options[] = {{L"help", no_argument, NULL, 'h'},
+                                              {NULL, 0, NULL, 0}};
+
+int parse_cmd_opts_help_only(struct cmd_opts_help_only *opts, int *optind, int argc, wchar_t **argv,
+                             parser_t &parser, io_streams_t &streams) {
+    wchar_t *cmd = argv[0];
+    int opt;
+    wgetopter_t w;
+    while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, NULL)) != -1) {
+        switch (opt) {  //!OCLINT(too few branches)
+            case 'h': {
+                opts->print_help = true;
+                return STATUS_CMD_OK;
+            }
+            case '?': {
+                builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1]);
+                return STATUS_INVALID_ARGS;
+            }
+            default: {
+                DIE("unexpected retval from wgetopt_long");
+                break;
+            }
+        }
+    }
+
+    *optind = w.woptind;
+    return STATUS_CMD_OK;
+}
+
 /// Count the number of times the specified character occurs in the specified string.
 static int count_char(const wchar_t *str, wchar_t c) {
     int res = 0;
@@ -311,79 +342,6 @@ static int builtin_count(parser_t &parser, io_streams_t &streams, wchar_t **argv
     return argc - 1 == 0 ? STATUS_CMD_ERROR : STATUS_CMD_OK;
 }
 
-/// Helper function for builtin_bg().
-static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j) {
-    assert(j != NULL);
-    if (!j->get_flag(JOB_CONTROL)) {
-        streams.err.append_format(
-            _(L"%ls: Can't put job %d, '%ls' to background because it is not under job control\n"),
-            L"bg", j->job_id, j->command_wcstr());
-        builtin_print_help(parser, streams, L"bg", streams.err);
-        return STATUS_CMD_ERROR;
-    }
-
-    streams.err.append_format(_(L"Send job %d '%ls' to background\n"), j->job_id,
-                              j->command_wcstr());
-    job_promote(j);
-    j->set_flag(JOB_FOREGROUND, false);
-    job_continue(j, job_is_stopped(j));
-    return STATUS_CMD_OK;
-}
-
-/// Builtin for putting a job in the background.
-static int builtin_bg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
-    int res = STATUS_CMD_OK;
-
-    if (!argv[1]) {
-        // No jobs were specified so use the most recent (i.e., last) job.
-        job_t *j;
-        job_iterator_t jobs;
-        while ((j = jobs.next())) {
-            if (job_is_stopped(j) && j->get_flag(JOB_CONTROL) && (!job_is_completed(j))) {
-                break;
-            }
-        }
-
-        if (!j) {
-            streams.err.append_format(_(L"%ls: There are no suitable jobs\n"), argv[0]);
-            res = STATUS_CMD_ERROR;
-        } else {
-            res = send_to_bg(parser, streams, j);
-        }
-
-        return res;
-    }
-
-    // The user specified at least one job to be backgrounded.
-    std::vector<int> pids;
-
-    // If one argument is not a valid pid (i.e. integer >= 0), fail without backgrounding anything,
-    // but still print errors for all of them.
-    for (int i = 1; argv[i]; i++) {
-        int pid = fish_wcstoi(argv[i]);
-        if (errno || pid < 0) {
-            streams.err.append_format(_(L"%ls: '%ls' is not a valid job specifier\n"), L"bg",
-                                      argv[i]);
-            res = STATUS_INVALID_ARGS;
-        }
-        pids.push_back(pid);
-    }
-
-    if (res != STATUS_CMD_OK) return res;
-
-    // Background all existing jobs that match the pids.
-    // Non-existent jobs aren't an error, but information about them is useful.
-    for (auto p : pids) {
-        if (job_t *j = job_get_from_pid(p)) {
-            res |= send_to_bg(parser, streams, j);
-        } else {
-            streams.err.append_format(_(L"%ls: Could not find job '%d'\n"), argv[0], p);
-        }
-    }
-
-    return res;
-}
-
 /// This function handles both the 'continue' and the 'break' builtins that are used for loop
 /// control.
 static int builtin_break_continue(parser_t &parser, io_streams_t &streams, wchar_t **argv) {

src/builtin.h

@@ -116,4 +116,10 @@ void builtin_missing_argument(parser_t &parser, io_streams_t &streams, const wch
                               const wchar_t *opt);
 
 void builtin_wperror(const wchar_t *s, io_streams_t &streams);
+
+struct cmd_opts_help_only {
+    bool print_help = false;
+};
+int parse_cmd_opts_help_only(struct cmd_opts_help_only *opts, int *optind, int argc, wchar_t **argv,
+                             parser_t &parser, io_streams_t &streams);
 #endif

src/builtin_bg.cpp

@@ -0,0 +1,100 @@
+// Implementation of the bg builtin.
+#include "config.h"  // IWYU pragma: keep
+
+#include <errno.h>
+#include <stdlib.h>
+
+#include <memory>
+#include <vector>
+
+#include "builtin.h"
+#include "builtin_bg.h"
+#include "common.h"
+#include "fallback.h"  // IWYU pragma: keep
+#include "io.h"
+#include "proc.h"
+#include "wutil.h"  // IWYU pragma: keep
+
+/// Helper function for builtin_bg().
+static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j) {
+    assert(j != NULL);
+    if (!j->get_flag(JOB_CONTROL)) {
+        streams.err.append_format(
+            _(L"%ls: Can't put job %d, '%ls' to background because it is not under job control\n"),
+            L"bg", j->job_id, j->command_wcstr());
+        builtin_print_help(parser, streams, L"bg", streams.err);
+        return STATUS_CMD_ERROR;
+    }
+
+    streams.err.append_format(_(L"Send job %d '%ls' to background\n"), j->job_id,
+                              j->command_wcstr());
+    job_promote(j);
+    j->set_flag(JOB_FOREGROUND, false);
+    job_continue(j, job_is_stopped(j));
+    return STATUS_CMD_OK;
+}
+
+/// Builtin for putting a job in the background.
+int builtin_bg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
+    const wchar_t *cmd = argv[0];
+    int argc = builtin_count_args(argv);
+    struct cmd_opts_help_only opts;
+
+    int optind;
+    int retval = parse_cmd_opts_help_only(&opts, &optind, argc, argv, parser, streams);
+    if (retval != STATUS_CMD_OK) return retval;
+
+    if (opts.print_help) {
+        builtin_print_help(parser, streams, cmd, streams.out);
+        return STATUS_CMD_OK;
+    }
+
+    if (optind == argc) {
+        // No jobs were specified so use the most recent (i.e., last) job.
+        job_t *j;
+        job_iterator_t jobs;
+        while ((j = jobs.next())) {
+            if (job_is_stopped(j) && j->get_flag(JOB_CONTROL) && (!job_is_completed(j))) {
+                break;
+            }
+        }
+
+        if (!j) {
+            streams.err.append_format(_(L"%ls: There are no suitable jobs\n"), cmd);
+            retval = STATUS_CMD_ERROR;
+        } else {
+            retval = send_to_bg(parser, streams, j);
+        }
+
+        return retval;
+    }
+
+    // The user specified at least one job to be backgrounded.
+    std::vector<int> pids;
+
+    // If one argument is not a valid pid (i.e. integer >= 0), fail without backgrounding anything,
+    // but still print errors for all of them.
+    for (int i = optind; argv[i]; i++) {
+        int pid = fish_wcstoi(argv[i]);
+        if (errno || pid < 0) {
+            streams.err.append_format(_(L"%ls: '%ls' is not a valid job specifier\n"), L"bg",
+                                      argv[i]);
+            retval = STATUS_INVALID_ARGS;
+        }
+        pids.push_back(pid);
+    }
+
+    if (retval != STATUS_CMD_OK) return retval;
+
+    // Background all existing jobs that match the pids.
+    // Non-existent jobs aren't an error, but information about them is useful.
+    for (auto p : pids) {
+        if (job_t *j = job_get_from_pid(p)) {
+            retval |= send_to_bg(parser, streams, j);
+        } else {
+            streams.err.append_format(_(L"%ls: Could not find job '%d'\n"), cmd, p);
+        }
+    }
+
+    return retval;
+}

src/builtin_bg.h

@@ -0,0 +1,9 @@
+// Prototypes for executing builtin_bg function.
+#ifndef FISH_BUILTIN_BG_H
+#define FISH_BUILTIN_BG_H
+
+class parser_t;
+struct io_streams_t;
+
+int builtin_bg(parser_t &parser, io_streams_t &streams, wchar_t **argv);
+#endif

src/builtin_fg.cpp

@@ -15,50 +15,16 @@
 #include "proc.h"
 #include "reader.h"
 #include "tokenizer.h"  // for tok_first
-#include "wgetopt.h"
 #include "wutil.h"  // IWYU pragma: keep
 
-struct cmd_opts {
-    bool print_help = false;
-};
-static const wchar_t *short_options = L"h";
-static const struct woption long_options[] = {{L"help", no_argument, NULL, 'h'},
-                                              {NULL, 0, NULL, 0}};
-
-static int parse_cmd_opts(struct cmd_opts *opts, int *optind, int argc, wchar_t **argv,
-                          parser_t &parser, io_streams_t &streams) {
-    wchar_t *cmd = argv[0];
-    int opt;
-    wgetopter_t w;
-    while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, NULL)) != -1) {
-        switch (opt) {  //!OCLINT(too few branches)
-            case 'h': {
-                opts->print_help = true;
-                return STATUS_CMD_OK;
-            }
-            case '?': {
-                builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1]);
-                return STATUS_INVALID_ARGS;
-            }
-            default: {
-                DIE("unexpected retval from wgetopt_long");
-                break;
-            }
-        }
-    }
-
-    *optind = w.woptind;
-    return STATUS_CMD_OK;
-}
-
 /// Builtin for putting a job in the foreground.
 int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     const wchar_t *cmd = argv[0];
     int argc = builtin_count_args(argv);
-    struct cmd_opts opts;
+    struct cmd_opts_help_only opts;
 
     int optind;
-    int retval = parse_cmd_opts(&opts, &optind, argc, argv, parser, streams);
+    int retval = parse_cmd_opts_help_only(&opts, &optind, argc, argv, parser, streams);
     if (retval != STATUS_CMD_OK) return retval;
 
     if (opts.print_help) {

tests/jobs.err

@@ -1,4 +1,4 @@
-bg: '-23' is not a valid job specifier
+bg: invalid option -23
 fg: No suitable job: 3
 bg: Could not find job '3'
 disown: 'foo' is not a valid job specifier

tests/jobs.in

@@ -1,7 +1,8 @@
 sleep 5 &
 sleep 5 &
 jobs -c
-bg -23 1
+bg -23 1 ^/dev/null
+or echo bg: invalid option -23 >&2
 fg 3
 bg 3
 sleep 1 &