diff --git a/src/parse_execution.rs b/src/parse_execution.rs index 9a1f26ca3..0964e6d97 100644 --- a/src/parse_execution.rs +++ b/src/parse_execution.rs @@ -1585,18 +1585,27 @@ 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)); + let _timer = { + let wants_timing = job_node_wants_timing(job_node); + // It's an error to have 'time' in a background job. + if wants_timing && job_is_background { + return report_error!( + self, + ctx, + STATUS_INVALID_ARGS.unwrap(), + job_node, + ERROR_TIME_BACKGROUND + ); + } + wants_timing.then(push_timer) + }; // 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 job_is_simple_node { + if self.job_is_simple_block(job_node) { let mut block = None; let mut result = self.apply_variable_assignments(ctx, None, &job_node.variables, &mut block); @@ -1651,17 +1660,6 @@ impl<'a> ParseExecutionContext { props.skip_notification = ld.is_subshell || parser.is_block() || ld.is_event != 0 || !parser.is_interactive(); props.from_event_handler = ld.is_event != 0; - - // It's an error to have 'time' in a background job. - if job_wants_timing && job_is_background { - return report_error!( - self, - ctx, - STATUS_INVALID_ARGS.unwrap(), - job_node, - ERROR_TIME_BACKGROUND - ); - } } let mut job = Job::new(props, self.node_source(job_node)); diff --git a/src/timer.rs b/src/timer.rs index 15ee376b6..ae09a84e7 100644 --- a/src/timer.rs +++ b/src/timer.rs @@ -30,17 +30,12 @@ struct TimerSnapshot { cpu_children: libc::rusage, } -/// If `enabled`, create a `TimerSnapshot` and return a `PrintElapsedOnDrop` object that will print -/// upon being dropped the delta between now and the time that it is dropped at. Otherwise return -/// `None`. -pub fn push_timer(enabled: bool) -> Option { - if !enabled { - return None; - } - - Some(PrintElapsedOnDrop { +/// Create a `TimerSnapshot` and return a `PrintElapsedOnDrop` object that will print upon +/// being dropped the delta between now and the time that it is dropped at. +pub fn push_timer() -> PrintElapsedOnDrop { + PrintElapsedOnDrop { start: TimerSnapshot::take(), - }) + } } /// An enumeration of supported libc rusage types used by [`getrusage()`]. diff --git a/tests/checks/syntax-error-location.fish b/tests/checks/syntax-error-location.fish index 9821d752d..bcc07f882 100644 --- a/tests/checks/syntax-error-location.fish +++ b/tests/checks/syntax-error-location.fish @@ -68,7 +68,7 @@ $fish -c 'echo {$,}' echo "bind -M" | $fish # CHECKERR: bind: -M: option requires an argument -# CHECKERR: Standard input (line 1): +# CHECKERR: Standard input (line 1): # CHECKERR: bind -M # CHECKERR: ^ # CHECKERR: (Type 'help bind' for related documentation) @@ -118,6 +118,11 @@ $fish -c 'echo (time echo foo &)' # CHECKERR: echo (time echo foo &) # CHECKERR: ^~~~~~~~~~~~~~~~^ +$fish -c 'time begin; end &' +# CHECKERR: fish: 'time' is not supported for background jobs. Consider using 'command time'. +# CHECKERR: time begin; end & +# CHECKERR: ^~~~~~~~~~~~~~~~^ + $fish -c 'echo (set -l foo 1 2 3; for $foo in foo; end)' # CHECKERR: fish: Unable to expand variable name '' # CHECKERR: set -l foo 1 2 3; for $foo in foo; end