-
-
Notifications
You must be signed in to change notification settings - Fork 188
Conversation
@@ -22,6 +22,7 @@ program | |||
.option("-b, --browsers <items>", "browsers list (comma separated)") | |||
.option("-I, --no-import", "do not inline @import") | |||
.option("-U, --no-url", "do not adjust url()") | |||
.option("-e, --extract", "output values of custom properties") |
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.
Not just custom props
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.
Oh, right. Thanks for catching it.
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.
Do you mind I put "output defined variables" there? "custom properties and custom medias" are pretty wordy, and can get longer when new kinds of variables are speced.
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.
"extract some computed values in a file (custom props, media, ...)" ?
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 think width: calc(1px + 1px) => width: 2px
also qualifies as a computed value. How about "extract resolved variables (custom props, media, etc)" ?
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 think "medias" is acceptable. This is from the oxford dictionary:
The word is also increasingly used in the plural form medias, as if it had a conventional singular form media, especially when referring to different forms of new media, and in the sense ‘the material or form used by an artist’: there were great efforts made by the medias of the involved countries
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.
Hum, interesting :)
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.
Please don't hate me, just a final nitpick: if you don't specify a output file path, the extracted json is sent to stdout, so strictly speaking, it's not always extracted into a file.
I won't complain if you want to keep it. :)
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 will see how to handle that. I will probably add this to JS api too.
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.
Great. Thanks for your work.
I will merge just before next release. Thanks for this contribution. |
It should probably be called |
"Media queries might use some custom defined media." so I don't think this should be named this way at all. It's |
The whole section is named Custom Media Queries. When defining
The word "custom media" only seems to appear in the syntax, and I think the word "query" is omitted only for abbreviation reason. |
A query seems the moment you compute some media and ask for some rules: @media screen {
/* "please apply this styles for screen only": this looks like a query */
}
|
If you look at the syntax definition, |
Exactly, a media (type) is not a media query. It might be in it, but they are not equal. media query: |
A media type belongs to a media query. In the case of It's like an integer is a rational number. Even though rational numbers also contains other types of numbers, an integer is still a rational number. |
To my eyes, you cannot really compare "screen and portrait" and numbers. It feels more like an union, not an addition of numbers. Anyway, renaming customMedia to customMediaQueries would be a major breaking change and is not in the scope of this PR. One thing at a time please. Stay focused. You already asked to change "media" to "medias" which does not bring any real value. Please open a new issue if you feel that it's a requirement for the project health. |
The object generated by the exact option currently contains a If you feel strongly about this. I relent. :) |
The problem is, to be coherent with the entire project, this renaming should be done in all the references to "customMedia". So it's not just the scope of this PR which is about extracting some computed values. Naming is another thing imo. It's hard and complicated enough to get its own discussion. |
OK, allow me to try to convince you for the last time. Although you might have attached to "custom media" because that phrase have been used for a long time in postcss, if you search for "custom media" in the spec, the word "query/queries" always follows it. There no such thing as "custom media" in the spec, it only appear in the syntax, which is an abbreviation. If the point of this project is to be spec compliant, why not make the nomenclature also be spec compliant? (even though "custom media" sounds more logical to you.) But if you feel it's too much hassle to change all the names, I understand. |
It's not "too much hassle". It's a breaking change, so this needs a major bump: everyone won't have automatic/easy update (since only crazy people use "*" as a version). As you said, it's always followed by "query/ies" in specs (text). But "customMedia" seems clear enough in the code and make sense with the Ref #185 |
It doesn't have to be in v2 if you accept The only api that uses Since the extract option isn't released yet, it won't create a compatibility issue. |
Sure we can do that. My concern is: what does this bring to the end user? Do you think "custom media" is not clear enough? I like long name when it's relevant (and encourage verbosity), but here I don't see the value, sorry. Even if the specs use "query/ies", we all know that, in CSS, "media" involve the "query" word. I guess it's for that reason that they didn't add it in the syntax... |
The main advantage We were arguing about |
Not relevant anymore. cssnext is now postcss-cssnext. cssnext is deprecated. See postcss-cssnext migration guide http://cssnext.io/postcss/ |
Such a long journey. :)