From bb4e36da47243a412512be59a199a63c61b4bee9 Mon Sep 17 00:00:00 2001
From: ridiculousfish <corydoras@ridiculousfish.com>
Date: Mon, 2 Mar 2020 12:34:07 -0800
Subject: [PATCH] Do not remove jobs that need to print a status message

55e3270 introduced a regression where we would remove all completed
jobs. But jobs that want to print a status message get skipped, so
the status message (and associated event handlers) might not get run.

Fix this by making it explicit which jobs are safe to process, and which
should be skipped.

Fixes #6679.
---
 src/proc.cpp | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/proc.cpp b/src/proc.cpp
index ac367c930..2a2a026f2 100644
--- a/src/proc.cpp
+++ b/src/proc.cpp
@@ -610,18 +610,18 @@ static bool process_clean_after_marking(parser_t &parser, bool allow_interactive
     // complete.
     std::vector<event_t> exit_events;
 
+    // A helper to indicate if we should process a job.
+    auto should_process_job = [=](const shared_ptr<job_t> &j) {
+        // Do not attempt to process jobs which are not yet constructed.
+        // Do not attempt to process jobs that need to print a status message,
+        // unless we are interactive, in which case printing is OK.
+        return j->is_constructed() && (interactive || !job_wants_message(j));
+    };
+
     // Print status messages for completed or stopped jobs.
     const bool only_one_job = parser.jobs().size() == 1;
     for (const auto &j : parser.jobs()) {
-        // Skip unconstructed jobs.
-        if (!j->is_constructed()) {
-            continue;
-        }
-
-        // If we are not interactive, skip cleaning jobs that want to print an interactive message.
-        if (!interactive && job_wants_message(j)) {
-            continue;
-        }
+        if (!should_process_job(j)) continue;
 
         // Clean processes within the job.
         // Note this may print the message on behalf of the job, affecting the result of
@@ -655,9 +655,11 @@ static bool process_clean_after_marking(parser_t &parser, bool allow_interactive
     // Remove completed jobs.
     // Do this before calling out to user code in the event handler below, to ensure an event
     // handler doesn't remove jobs on our behalf.
-    auto is_complete = [](const shared_ptr<job_t> &j) { return j->is_completed(); };
+    auto should_remove = [&](const shared_ptr<job_t> &j) {
+        return should_process_job(j) && j->is_completed();
+    };
     auto &jobs = parser.jobs();
-    jobs.erase(std::remove_if(jobs.begin(), jobs.end(), is_complete), jobs.end());
+    jobs.erase(std::remove_if(jobs.begin(), jobs.end(), should_remove), jobs.end());
 
     // Post pending exit events.
     for (const auto &evt : exit_events) {