-
Notifications
You must be signed in to change notification settings - Fork 567
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
feat(format): stylus support #527
feat(format): stylus support #527
Conversation
8c22eb2
to
ac1e93a
Compare
lib/common/formats.js
Outdated
if (prop.comment) { | ||
if (commentStyle === 'short') { | ||
to_ret_prop = to_ret_prop.concat(' // ' + prop.comment); | ||
} else { | ||
to_ret_prop = to_ret_prop.concat(' /* ' + prop.comment + ' */'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved the indentation alignment in a separate commit - if you feel like we should move this out of this PR I'll happily do this.
There was a problem hiding this 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! Cleaning up indentation and alignment is always welcome. We should probably use prettier or something in the future though... That'll be a different PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a good idea! I've removed the indentation cleanup for now.
Thank you for this addition! We are working off the 3.0 branch to target the next major release. We switched the branch for this PR and there are some conflicts. If you could fix these we can merge this into the 3.0 branch. If you need any help with any of the conflicts let me know and I can help out. |
Although leading dollar signs are not mandatory for stylus variables there is a trend to having this, so I also included it in this PR. see https://gist.github.com/zspecza/7220997#why refs amzn#526
ac1e93a
to
2c4ab14
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you again for this addition!
Thanks for merging it! Really appreciate all the work you put into style-dictionary! |
Issue #, if available:
See #526
Description of changes:
This PR adds support for
stylus/variables
.Although leading dollar signs are not mandatory for stylus variables there is a trend to having this, so I also included it in this PR. See https://gist.github.com/zspecza/7220997#why
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.