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

Inline comments fix and packages update #61

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ihorbond
Copy link

@ihorbond ihorbond commented Aug 3, 2020

I have updated the node-sass package in order to be able to parse things like sass map-get. I've also added regex to remove both multiline and inline comments

@Levdbas
Copy link
Contributor

Levdbas commented Dec 23, 2020

Nice work @ihorbond , I hope the project owner will merge this soon. Any idea if the sass package instead of the node-sass package will work as well?

@Levdbas
Copy link
Contributor

Levdbas commented Dec 23, 2020

@civan Could you take a look at this PR?

@civan
Copy link
Member

civan commented Dec 30, 2020

Sorry @ihorbond for the late reply, thanks for the nice work, would you take a look at the failing unit tests https://travis-ci.org/github/plentycode/sass-export/jobs/714598510?

@Levdbas
Copy link
Contributor

Levdbas commented Jan 3, 2021

@ihorbond, i've checked your PR and it seems that the group annotation comments are also removed by the REGEX you've added in the getContent() method. By removing these comments this package doesn't function properly anymore and thus the tests fail. A little background: it seems that all variables are grouped inside the export. By using the group annotation comments you can group the variables. Because the annotation marks are removed, all variables are placed inside the default group.

In the tests they also test the existence of other groups besides the default variables one, and thus the test fail.

@Levdbas
Copy link
Contributor

Levdbas commented Feb 22, 2021

Hi @civan , I would like to ask you if you could look into my PR #63 . sass-export is still downloaded 5000 times a week via NPM. So at least updating the dependencies would really help the people out here to use this niche plugin a little longer. Looking forward to your response!

@ihorbond
Copy link
Author

ihorbond commented Mar 6, 2021

sorry guys @Levdbas @civan didn't see this got traction. I could just remove the line with regex for now to make the tests pass

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.

None yet

3 participants