Skip to content

Fix bug introduced by #13595#13658

Merged
devyn merged 1 commit intonushell:mainfrom
ysthakur:fix-range-regression
Aug 21, 2024
Merged

Fix bug introduced by #13595#13658
devyn merged 1 commit intonushell:mainfrom
ysthakur:fix-range-regression

Conversation

@ysthakur
Copy link
Copy Markdown
Member

@ysthakur ysthakur commented Aug 21, 2024

Description

@devyn found that #13595, which made ranges be type-checked at parse time, introduced a bug that caused ../foo to be parsed as a string rather than a command call. This was caused by parse_range returning a Some despite there being parse errors (/foo doesn't match SyntaxShape::Number). To go back to the old behavior, parse_range now returns None anytime there's any parse errors met while parsing the range.

Unfortunately, this means that something like ..$foo will be parsed as a string if $foo isn't defined and as a range if it is defined. That was the behavior before #13595, and it should probably be fixed at some point, but I'm just trying to quickly fix the bug.

User-Facing Changes

Things should go back to the way they were before #13595, except the type-checking stuff from that PR is still here.

Tests + Formatting

Added a test. Reverted another test that tests that 0..<$day is parsed successfully as a string if the variable isn't defined.

After Submitting

@devyn
Copy link
Copy Markdown
Contributor

devyn commented Aug 21, 2024

Thanks for finishing this so quickly! I'll wait for it to pass and then merge, confirm, and do 0.97.1.

@devyn devyn merged commit 34e7bd8 into nushell:main Aug 21, 2024
@ysthakur ysthakur deleted the fix-range-regression branch August 21, 2024 02:35
@hustcer hustcer added this to the v0.97.0 milestone Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants