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

Sass Formatter #1434

Closed
wants to merge 7 commits into from
Closed

Sass Formatter #1434

wants to merge 7 commits into from

Conversation

TheRealSyler
Copy link
Contributor

Related to #1433

not sure if the return values of the unused functions in the SassLanguageMode are an issue.

deleteWhitespace: true,
replaceSpacesOrTabs: true,
setPropertySpace: true
};
Copy link
Member

Choose a reason for hiding this comment

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

Can you extract this into an interface exported from this file? We might have a default config at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i already made the interface here, i can just import the it from sass-formatter, maintaining two instances of the same interface seems unnecessary.

setPropertySpace: true
};
if (this.config.sass && this.config.sass.format) {
sassConfig = this.config.sass.format;
Copy link
Member

Choose a reason for hiding this comment

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

Use Object.assign(this.config.sass.format, sassConfig) to merge them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added in the newest commit.

@octref
Copy link
Member

octref commented Oct 2, 2019

Good work. Thanks a lot!

I have added docs and pushed an integration test. Left some comments as well.

@TheRealSyler
Copy link
Contributor Author

i couldn't test it locally because i installed linux, and i get an error when i try to start the debug server, but this should address your comments, and in theory it should work without braking anything, but every time i think this should work something brakes.

@octref
Copy link
Member

octref commented Nov 11, 2019

Retriggering CI

@octref octref closed this Nov 11, 2019
@octref octref reopened this Nov 11, 2019
@TheRealSyler
Copy link
Contributor Author

moved to #1658

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.

2 participants