Skip to content

Add parse error for external commands used in assignment without caret#13585

Merged
sholderbach merged 2 commits intonushell:mainfrom
devyn:parse-error-on-bare-external-command-in-assignment
Aug 12, 2024
Merged

Add parse error for external commands used in assignment without caret#13585
sholderbach merged 2 commits intonushell:mainfrom
devyn:parse-error-on-bare-external-command-in-assignment

Conversation

@devyn
Copy link
Copy Markdown
Contributor

@devyn devyn commented Aug 10, 2024

Description

As per our Wednesday meeting, this adds a parse error when something that would be parsed as an external call is present at the top level, unless the head of the external call begins with a caret (to make it explicit).

I tried to make the error quite descriptive about what should be done.

User-Facing Changes

These now cause a parse error:

$foo = bar
$foo = `bar`

These would have been interpreted as strings before this version, but now they'd be interpreted as external calls. This behavior is consistent with let/mut (which is unaffected by this change).

Here is an example of the error:

Error:   × External command calls must be explicit in assignments
   ╭─[entry #3:1:8]
 1 │ $foo = bar
   ·        ─┬─
   ·         ╰── add a caret (^) before the command name if you intended to run and capture its output
   ╰────
  help: the parsing of assignments was changed in 0.97.0, and this would have previously been treated as a string.
        Alternatively, quote the string with single or double quotes to avoid it being interpreted as a command name. This
        restriction may be removed in a future release.

Tests + Formatting

Tests added to cover the change. Note made about it being temporary.

@ysthakur
Copy link
Copy Markdown
Member

ysthakur commented Aug 11, 2024

Perhaps you could use the nu! macro to run the tests? I think that makes a nu executable available

@devyn
Copy link
Copy Markdown
Contributor Author

devyn commented Aug 11, 2024

Yeah, I guess I might have to. At some point I should really try to unify that infrastructure

Copy link
Copy Markdown
Member

@sholderbach sholderbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome thanks!

@sholderbach sholderbach merged commit 18772b7 into nushell:main Aug 12, 2024
@devyn
Copy link
Copy Markdown
Contributor Author

devyn commented Aug 12, 2024

So fast! Thanks!

@devyn devyn deleted the parse-error-on-bare-external-command-in-assignment branch August 12, 2024 08:43
@hustcer hustcer added this to the v0.97.0 milestone Aug 12, 2024
@fdncred fdncred added the deprecated:pr-errors Deprecated: use A:error-handling, A:error-unhelpful, or A:error-silent-fail instead label Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deprecated:pr-errors Deprecated: use A:error-handling, A:error-unhelpful, or A:error-silent-fail instead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants