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()