Skip to content

Correctly calculate last parens in ruleset as mixin param#132

Merged
shellscape merged 1 commit intoshellscape:masterfrom
AndreasHogstrom:fix/mixin-param-ruleset-last-parens
Dec 28, 2018
Merged

Correctly calculate last parens in ruleset as mixin param#132
shellscape merged 1 commit intoshellscape:masterfrom
AndreasHogstrom:fix/mixin-param-ruleset-last-parens

Conversation

@AndreasHogstrom
Copy link
Copy Markdown
Contributor

Which issue # if any, does this resolve?
#128

Please check one:

  • New tests created for this change
  • Tests updated for this change

This PR:

  • Adds new API
  • Extends existing API, backwards-compatible
  • Introduces a breaking change
  • Fixes a bug

The following code found in lib/LessParser.js:95 always returns the first closing parenthesis rather than the last, as indicated by the variable name.

const lastParenIndex = tokens.findIndex((t) => t[0] === ')');

This breaks the parsing when there are multiple closing parenthesis in the list of tokens.

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #132 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
- Coverage   96.49%   96.47%   -0.02%     
==========================================
  Files           8        8              
  Lines         228      227       -1     
==========================================
- Hits          220      219       -1     
  Misses          8        8
Impacted Files Coverage Δ
lib/LessParser.js 99.13% <100%> (ø) ⬆️
lib/nodes/inline-comment.js 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 714ce13...aeeb7bb. Read the comment docs.

Copy link
Copy Markdown
Owner

@shellscape shellscape left a comment

Choose a reason for hiding this comment

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

Thank you for contributing a fix!

@jwilsson this looks good to me. please give this a look

Copy link
Copy Markdown
Collaborator

@jwilsson jwilsson left a comment

Choose a reason for hiding this comment

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

Looks good to me too.

Thanks for the fix!

@shellscape shellscape merged commit 402dc5d into shellscape:master Dec 28, 2018
@drserg
Copy link
Copy Markdown

drserg commented Jan 13, 2019

Hi guys! Could you please release this fix? Thanks a lot in advance

@AndreasHogstrom AndreasHogstrom deleted the fix/mixin-param-ruleset-last-parens branch February 13, 2019 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants