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

fix: account for multi-line comments in tokens, improve comment style #953

Merged
merged 3 commits into from
Oct 23, 2023

Conversation

jorenbroekema
Copy link
Collaborator

Issue #, if available:

fixes #952

Description of changes:

  • Account for multi-line comments, by splitting them up in the final comment string.
  • Put multi-line comments above the token, making it more readable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jorenbroekema
Copy link
Collaborator Author

I added a second commit with a small change that allows formatting prop to be user-passed, even if the format is css/sass/less/stylus, as well as a small change to allow passing commentStyle "short-above" or "long-above" to ensure the comment is put 1 line above the outputted property, rather then inline next to it.

Let me know if those two things are okay or you prefer it some other way or via a separate PR

Copy link
Member

@dbanksdesign dbanksdesign left a comment

Choose a reason for hiding this comment

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

Overall looks good, just a few minor things! Thank you!

lib/common/formatHelpers/createPropertyFormatter.js Outdated Show resolved Hide resolved
lib/common/formatHelpers/createPropertyFormatter.js Outdated Show resolved Hide resolved
@jorenbroekema jorenbroekema merged commit 18629d8 into amzn:main Oct 23, 2023
3 checks passed
@jorenbroekema jorenbroekema deleted the comment-formatting branch October 23, 2023 19:38
@jorenbroekema jorenbroekema self-assigned this Nov 10, 2023
@chris-dura
Copy link

I added a second commit with a small change that allows formatting prop to be user-passed, even if the format is css/sass/less/stylus, as well as a small change to allow passing commentStyle "short-above" or "long-above" to ensure the comment is put 1 line above the outputted property, rather then inline next to it.

Let me know if those two things are okay or you prefer it some other way or via a separate PR

Should this have documentation updated as well around the additional options for commentStyle?

https://amzn.github.io/style-dictionary/#/formats?id=fileheader


Related, are these types incorrect? The types listed here say the options for commentStyle are "short" | "long" | "none"...

commentStyle?: "short" | "long" | "none";

@jorenbroekema
Copy link
Collaborator Author

I added a second commit with a small change that allows formatting prop to be user-passed, even if the format is css/sass/less/stylus, as well as a small change to allow passing commentStyle "short-above" or "long-above" to ensure the comment is put 1 line above the outputted property, rather then inline next to it.
Let me know if those two things are okay or you prefer it some other way or via a separate PR

Should this have documentation updated as well around the additional options for commentStyle?

https://amzn.github.io/style-dictionary/#/formats?id=fileheader

Related, are these types incorrect? The types listed here say the options for commentStyle are "short" | "long" | "none"...

commentStyle?: "short" | "long" | "none";

Good catch, thanks: #1047

This was referenced Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Comments with newlines are not formatted nicely in createPropertyFormatter
3 participants