postcss / postcss-simple-vars

PostCSS plugin for Sass-like variables
MIT License
415 stars 36 forks source link

Option for disable, or write variable in comments differently #51

Closed mihaeltomic closed 8 years ago

mihaeltomic commented 8 years ago

The feature is great for creating documentation, but for example, we have a toolkit/framework with code examples in comments which contain variables. And for this reason, we can't update to 2.0.0. version. i believe this is not isolated case.

There should be an option to disable variables in comments. Variables in a comment and in the code are not on the same level of the hierarchy and they shouldn't mess with each other.

If there can be an option like:

postcss-simple-vars ({
  comments: true|false
})

or:

/** This is comment with <$variable-name> */
/** This is comment with var=$variable-name */

or something similar like cref in C#

Please share you thoughts about this issue.

ai commented 8 years ago

@VinSpee what do you think?

ai commented 8 years ago

I don’t like settings. Best settings is no settings :).

For example, new developer will not know about project settings and will have unexpected behaviour. Also people don’t read a docs and they will miss this feature.

Maybe we should add some special syntax for variables in comments?

mihaeltomic commented 8 years ago

I agree. If there's a way to avoid setting that would be great, but some change should be done. The syntax could be the solution.

VinSpee commented 8 years ago

I can definitely see this issue. I like the idea of allowing interpolation when using a special syntax more, as it allows both use cases simultaneously.

You could add an option like

{ comments: true | false | "optional" } to avoid another major release
ai commented 8 years ago

What syntax we could provide?

andrejmlinarevic commented 8 years ago

This looks good to me. /* This is comment with <$variable-name> /

VinSpee commented 8 years ago

I'm fine with that syntax as well.

mihaeltomic commented 8 years ago

Me too. @ai ?

ai commented 8 years ago

Nice. if we will not have better idea, I will release 3.0 on next week.

danielbayley commented 8 years ago

I think there should definitely be an option to skip variables inside comments without introducing special syntax just for that. I would have thought that skipping anything commented out should be the default behaviour too…

ai commented 8 years ago

@danielbayley option will not work in mixins, because mixin plugin execute vars plugin internally.

danielbayley commented 8 years ago

I don’t like settings. Best settings is no settings :). For example, new developer will not know about project settings

Surely a single setting (and off by default) is preferable to adding special syntax for every variable inside a comment and less cognitive load for said new developers?

and will have unexpected behaviour.

Surely the unexpected behaviour is to expand variables inside comments?

Are we talking about adding special syntax to process them inside comments or leave them alone here?

option will not work in mixins, because mixin plugin execute vars plugin internally.

Shouldn't mixins just skip comments too?

ai commented 8 years ago

Shouldn't mixins just skip comments too?

Original feature request was created for usage in mixins. So we need cover this case. It will be hard to add special option to every plugin, that uses simple-vars inside.

ai commented 8 years ago

New syntax is <<$(name)>>. What do you think? (I don’t release it, so we can change it)

75f7043

VinSpee commented 8 years ago

Looks fine to me!

mihaeltomic commented 8 years ago

Me too!

ai commented 8 years ago

Released in 3.0 and postcss-mixins 5.0