From 4fa8d95b982cf1906fa93037e56383b97549af53 Mon Sep 17 00:00:00 2001
From: Anurag Singh <contact@anurag.sh>
Date: Sat, 27 Apr 2024 22:06:26 +0800
Subject: [PATCH] Move push_timer to measure command substitution timing too

---
 CHANGELOG.rst          |  1 +
 src/exec.rs            |  2 --
 src/parse_execution.rs | 16 ++++++++++------
 src/proc.rs            |  8 --------
 4 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 80d637677..0bc574a40 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -131,6 +131,7 @@ Interactive improvements
 - When exporting interactively defined functions (using ``type``, ``functions`` or ``funcsave``) the function body is now indented, same as in the interactive command line editor (:issue:`8603`).
 - :kbd:`ctrl-x` (``fish_clipboard_copy``) on multiline commands now includes indentation (:issue:`10437`).
 - When using :kbd:`ctrl-x` on Wayland in the VSCode terminal, the clipboard is no longer cleared on :kbd:`ctrl-c`.
+- Measuring a command with `time` now considers the time taken for command substitution (:issue:`9100`).
 
 New or improved bindings
 ^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/src/exec.rs b/src/exec.rs
index a7a45f3aa..0e70fe224 100644
--- a/src/exec.rs
+++ b/src/exec.rs
@@ -42,7 +42,6 @@ use crate::proc::{
 use crate::reader::{reader_run_count, restore_term_mode};
 use crate::redirection::{dup2_list_resolve_chain, Dup2List};
 use crate::threads::{iothread_perform_cant_wait, is_forked_child};
-use crate::timer::push_timer;
 use crate::trace::trace_if_enabled_with_args;
 use crate::wchar::{wstr, WString, L};
 use crate::wchar_ext::ToWString;
@@ -103,7 +102,6 @@ pub fn exec_job(parser: &Parser, job: &Job, block_io: IoChain) -> bool {
         }
         return false;
     }
-    let _timer = push_timer(job.wants_timing() && !no_exec());
 
     // Get the deferred process, if any. We will have to remember its pipes.
     let mut deferred_pipes = PartialPipes::default();
diff --git a/src/parse_execution.rs b/src/parse_execution.rs
index cdb2688ac..9a1f26ca3 100644
--- a/src/parse_execution.rs
+++ b/src/parse_execution.rs
@@ -1585,13 +1585,18 @@ impl<'a> ParseExecutionContext {
             0
         };
 
+        let job_wants_timing = job_node_wants_timing(job_node);
+        let job_is_background = job_node.bg.is_some();
+        let job_is_simple_node = self.job_is_simple_block(job_node);
+
+        // Start the timer here to ensure command substitution is measured
+        let _timer = push_timer(job_wants_timing && (job_is_simple_node || !job_is_background));
+
         // When we encounter a block construct (e.g. while loop) in the general case, we create a "block
         // process" containing its node. This allows us to handle block-level redirections.
         // However, if there are no redirections, then we can just jump into the block directly, which
         // is significantly faster.
-        if self.job_is_simple_block(job_node) {
-            let do_time = job_node.time.is_some();
-            let _timer = push_timer(do_time);
+        if job_is_simple_node {
             let mut block = None;
             let mut result =
                 self.apply_variable_assignments(ctx, None, &job_node.variables, &mut block);
@@ -1639,17 +1644,16 @@ impl<'a> ParseExecutionContext {
         }
 
         let mut props = JobProperties::default();
-        props.initial_background = job_node.bg.is_some();
+        props.initial_background = job_is_background;
         {
             let parser = ctx.parser();
             let ld = &parser.libdata().pods;
             props.skip_notification =
                 ld.is_subshell || parser.is_block() || ld.is_event != 0 || !parser.is_interactive();
             props.from_event_handler = ld.is_event != 0;
-            props.wants_timing = job_node_wants_timing(job_node);
 
             // It's an error to have 'time' in a background job.
-            if props.wants_timing && props.initial_background {
+            if job_wants_timing && job_is_background {
                 return report_error!(
                     self,
                     ctx,
diff --git a/src/proc.rs b/src/proc.rs
index a739d6b20..0bb63eadd 100644
--- a/src/proc.rs
+++ b/src/proc.rs
@@ -740,9 +740,6 @@ pub struct JobProperties {
     /// initial state should be.
     pub initial_background: bool,
 
-    /// Whether the job has the 'time' prefix and so we should print timing for this job.
-    pub wants_timing: bool,
-
     /// Whether this job was created as part of an event handler.
     pub from_event_handler: bool,
 }
@@ -884,11 +881,6 @@ impl Job {
         self.job_flags.borrow_mut()
     }
 
-    // \return whether we should print timing information.
-    pub fn wants_timing(&self) -> bool {
-        self.properties.wants_timing
-    }
-
     /// \return if we want job control.
     pub fn wants_job_control(&self) -> bool {
         self.group().wants_job_control()