Swap variable overrides and time in not statement
Some checks are pending
make test / ubuntu (push) Waiting to run
make test / ubuntu-32bit-static-pcre2 (push) Waiting to run
make test / ubuntu-asan (push) Waiting to run
make test / macos (push) Waiting to run
Rust checks / rustfmt (push) Waiting to run
Rust checks / clippy (push) Waiting to run

This is allowed

	time a=b echo 123

but -- due to an oversight in 3de95038b0 (Make "time" a job prefix,
2019-12-21) -- this is not allowed:

	not time a=b echo 123

Instead, this one one works:

	not a=b time echo 123

which is weird because without the "not" this would run "/bin/time".

It seems wrong that "not" is not like the others. Swap the order
for consistency.

Note that unlike "not", "time" currently needs to come before variable
assignments, so "a=b time true" is disallowed. This matches zsh. POSIX
shells call "/bin/time" here. Since it's ambiguous, erroring out seems
fine. It's weird that we're inconsistent with not here but I guess
"command not" is not expected to have subtly different behavior.

Closes #10890
This commit is contained in:
Johannes Altmanninger 2024-11-21 11:02:34 +01:00
parent 8fd0399ed3
commit 0275c5e803
2 changed files with 18 additions and 2 deletions

View File

@ -1644,16 +1644,16 @@ pub struct NotStatement {
parent: Option<*const dyn Node>, parent: Option<*const dyn Node>,
/// Keyword, either not or exclam. /// Keyword, either not or exclam.
pub kw: KeywordNot, pub kw: KeywordNot,
pub variables: VariableAssignmentList,
pub time: Option<KeywordTime>, pub time: Option<KeywordTime>,
pub variables: VariableAssignmentList,
pub contents: Statement, pub contents: Statement,
} }
implement_node!(NotStatement, branch, not_statement); implement_node!(NotStatement, branch, not_statement);
implement_acceptor_for_branch!( implement_acceptor_for_branch!(
NotStatement, NotStatement,
(kw: (KeywordNot)), (kw: (KeywordNot)),
(variables: (VariableAssignmentList)),
(time: (Option<KeywordTime>)), (time: (Option<KeywordTime>)),
(variables: (VariableAssignmentList)),
(contents: (Statement)), (contents: (Statement)),
); );
impl ConcreteNode for NotStatement { impl ConcreteNode for NotStatement {

View File

@ -38,6 +38,22 @@ not time true
#CHECKERR: {{.*}} #CHECKERR: {{.*}}
#CHECKERR: {{.*}} #CHECKERR: {{.*}}
not time a=b true
#CHECKERR: ___{{.*}}
#CHECKERR: {{.*}}
#CHECKERR: {{.*}}
#CHECKERR: {{.*}}
# Currently illegal syntax. Same in zsh. POSIX shells call the external command "time" here.
a=b time true
#CHECKERR: fish: time: missing man page
#CHECKERR: Documentation may not be installed.
#CHECKERR: `help time` will show an online version
not a=b time true
#CHECKERR: fish: time: missing man page
#CHECKERR: Documentation may not be installed.
#CHECKERR: `help time` will show an online version
$fish -c 'time true&' $fish -c 'time true&'
#CHECKERR: fish: {{.*}} #CHECKERR: fish: {{.*}}
#CHECKERR: time true& #CHECKERR: time true&