diff --git a/CHANGELOG.md b/CHANGELOG.md
index acec7edf8..629cf02bb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@
- `type` now no longer requires `which`, which means fish no longer uses it anywhere. Packagers should remove the dependency (#3912).
- Using symbolic permissions with the `umask` command now works (#738).
- Command substitutions now have access to the terminal, allowing tools like `fzf` to work in them (#1362, #3922).
+- `bg`s argument parsing has been reworked. It now fails for invalid arguments but allows non-existent jobs (#3909).
---
diff --git a/doc_src/bg.txt b/doc_src/bg.txt
index 8e9ec2444..7195501ed 100644
--- a/doc_src/bg.txt
+++ b/doc_src/bg.txt
@@ -11,7 +11,17 @@ bg [PID...]
The PID of the desired process is usually found by using process expansion.
+When at least one of the arguments isn't a valid job specifier (i.e. PID),
+`bg` will print an error without backgrounding anything.
+
+When all arguments are valid job specifiers, bg will background all matching jobs that exist.
\subsection bg-example Example
`bg %1` will put the job with job ID 1 in the background.
+
+`bg 123 456 789` will background 123, 456 and 789.
+
+If only 123 and 789 exist, it will still background them and print an error about 456.
+
+`bg 123 banana` or `bg banana 123` will complain that "banana" is not a valid job specifier.
diff --git a/src/builtin.cpp b/src/builtin.cpp
index f1ef3dbcc..e4a256a93 100644
--- a/src/builtin.cpp
+++ b/src/builtin.cpp
@@ -2988,11 +2988,8 @@ static int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
/// Helper function for builtin_bg().
static int send_to_bg(parser_t &parser, io_streams_t &streams, job_t *j, const wchar_t *name) {
- if (j == 0) {
- streams.err.append_format(_(L"%ls: Unknown job '%ls'\n"), L"bg", name);
- builtin_print_help(parser, streams, L"bg", streams.err);
- return STATUS_BUILTIN_ERROR;
- } else if (!j->get_flag(JOB_CONTROL)) {
+ 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());
@@ -3028,16 +3025,30 @@ static int builtin_bg(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
res = send_to_bg(parser, streams, j, _(L"(default)"));
}
} else {
- int i;
- int pid;
+ std::vector pids;
- for (i = 1; argv[i]; i++) {
- pid = fish_wcstoi(argv[i]);
- if (errno || pid < 0 || !job_get_from_pid(pid)) {
- streams.err.append_format(_(L"%ls: '%ls' is not a job\n"), argv[0], argv[i]);
- return STATUS_BUILTIN_ERROR;
+ // 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"), argv[0], argv[i]);
+ res = STATUS_BUILTIN_ERROR;
+ }
+ pids.push_back(pid);
+ }
+ if (res == STATUS_BUILTIN_ERROR) {
+ 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, *argv);
+ } else {
+ streams.err.append_format(_(L"%ls: Could not find job '%d'\n"), argv[0], p);
}
- res |= send_to_bg(parser, streams, job_get_from_pid(pid), *argv);
}
}
diff --git a/tests/jobs.err b/tests/jobs.err
index 13e56fdf5..5a1052d09 100644
--- a/tests/jobs.err
+++ b/tests/jobs.err
@@ -1,2 +1,3 @@
-bg: '3' is not a job
+bg: '-23' is not a valid job specifier
fg: No suitable job: 3
+bg: Could not find job '3'
diff --git a/tests/jobs.in b/tests/jobs.in
index ad0b002e7..e41b64537 100644
--- a/tests/jobs.in
+++ b/tests/jobs.in
@@ -1,6 +1,7 @@
-sleep 1 &
-sleep 1 &
+sleep 5 &
+sleep 5 &
jobs -c
-bg 3
+bg -23 1
fg 3
+bg 3
or exit 0