-
Notifications
You must be signed in to change notification settings - Fork 31
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
add --with-comments to preserve comments when returning an attribute #103
Conversation
c7d1d83
to
87c1cdb
Compare
Thanks for working on this. Overall, it looks good, but let me point out a few things. (1) The linter check is failing. Some files need to be formatted. (2) Although this cannot be strictly checked in the linter, the Go style convention recommends MixedCaps. That is to say, (3) Regarding being unable to get inline comments, they may belong to an attribute rather than an expression. I'll look into this later, but is this essential for your use case? I am unsure if I can find the time to look into it right away. |
Thanks for looking at this. I was going to go with For inline comments it doesn't matter for my purposes, i just considered it for completeness sake. If this goes out as is should we consider |
87c1cdb
to
a5e5f44
Compare
Thank you for the fix! I'll investigate the inline comments issue later, but I believe it would be technically possible without breaking the CLI interface, so there is no need to rename the flag. Let's merge and move forward! |
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 👍
@wondersd I've shipped it in v0.2.14 🚀 Thank you for your contribution! |
Follow-up to #103. Inline comments belong to attributes, not expressions. When the `--with-comments` flag is specified, we need to return tokens on the right side of the equals sign.
fixes #102
Only returns comments when attribute value is a block. Not sure whats missing to capture inline comments. However for the purposes of the original request this would be close enough