Skip to content

Fix: variable names beginning with a keyword and dash #2634

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

Merged
merged 2 commits into from
Sep 22, 2021

Conversation

groenroos
Copy link
Collaborator

What: Closes #2422

Why: Variable and mixin names that begin with a keyword (return, if, else, unless, for, or in), immediately followed by a dash (-), e.g. for-tablet, will trigger an error. They probably shouldn't.

How: Edited the regex of the keyword method in the lexer to ensure the keyword isn't directly followed by a -

Checklist:

  • Documentation
  • Unit Tests
  • Code complete

Sorry, something went wrong.

Verified

This commit was signed with the committer’s verified signature.
groenroos Oskari Groenroos

Verified

This commit was signed with the committer’s verified signature.
groenroos Oskari Groenroos
@groenroos groenroos requested a review from iChenLei September 9, 2021 09:57
@vendethiel
Copy link
Contributor

I don’t think that [ \t]* does anything, does it? Only the 1st match group is skipped.

@groenroos
Copy link
Collaborator Author

I don’t think that [ \t]* does anything, does it? Only the 1st match group is skipped.

Removing it causes errors;

Screenshot 2021-09-09 at 21 21 49

It seems like to me that the whitespace is indeed skipped as well, as it sends the entire captures array to skip;

stylus/lib/lexer.js

Lines 451 to 453 in 91b7701

if (captures = /^(return|if|else|unless|for|in)\b(?!-)[ \t]*/.exec(this.str)) {
var keyword = captures[1];
this.skip(captures);

where skip uses the first value of the array (e.g. for , with trailing whitespace) to calculate the number of chars to skip:

stylus/lib/lexer.js

Lines 130 to 132 in 91b7701

skip: function(len){
var chunk = len[0];
len = chunk ? chunk.length : len;

with the captures/len looking like this:

[
	'for ',
	'for',
	index: 0,
	input: 'for vendor in vendors\n' + ...,
	groups: undefined
]

@vendethiel
Copy link
Contributor

where skip uses the first value of the array (e.g. for , with trailing whitespace)

Ah, if skip uses captures[0] then it makes sense

@iChenLei
Copy link
Member

iChenLei commented Sep 10, 2021

So that's why CI is important, it ensure our change be more controllable.

@groenroos groenroos merged commit 33a5fd9 into stylus:dev Sep 22, 2021
@groenroos groenroos deleted the fix/for-in-var-names branch September 22, 2021 12:58
pirxpilot pushed a commit to pirxpilot/stylus that referenced this pull request Dec 14, 2024
* Fix variables beginning with a keyword and dash
* Add trailing space
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.

Can't use "for-" in variable or mixin name
3 participants