Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve parsing of numbers and C-style for loops #55

Closed
wants to merge 6 commits into from

Conversation

nathansobo
Copy link
Member

@nathansobo nathansobo commented Jul 30, 2019

Refs atom/language-shellscript#148

I'm doing a couple different changes in a single PR since they would conflict if done independently.

The first is a fix for the linked issue that improves parsing of numeric literals. We previously parsed numbers as words, but this excluded a support for numbers with a custom base, as described here. The tl;dr – You can express a number in the form BASE#NUMBER if you want to express a base other than decimal, up to base 64. The NUMBER portion can include letters, numbers, @ and _ to express various digits.

The second fix adjusts C-style for loops to expect a variable assignment as the first statement inside the loop parens.

The linked issue includes another problem with arithmetic expansion and evaluation $((...)) and ((...)). I will follow up with some fixes for those in this PR before requesting review.

Nathan Sobo added 4 commits July 30, 2019 11:52
Previously, numbers were just parsed as words. This improves the 
accuracy of the grammar, and more importantly, avoids parsing # as the 
start of a comment when a custom base is specified via BASE#NUMBER.
Previously, `i=0` as the first statement in a C-style for loop was 
parsed as a word.
@nathansobo nathansobo removed the request for review from maxbrunsfeld July 30, 2019 22:16
@skovhus
Copy link
Contributor

skovhus commented Mar 3, 2020

@nathansobo @maxbrunsfeld anything I can do to help this over the finish line? :)

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