Skip to content
This repository was archived by the owner on Dec 19, 2024. It is now read-only.

Add extract option to CLI #170

Closed
wants to merge 1 commit into from
Closed

Add extract option to CLI #170

wants to merge 1 commit into from

Conversation

hgl
Copy link

@hgl hgl commented Jun 30, 2015

Such a long journey. :)

@@ -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")
Copy link
Owner

Choose a reason for hiding this comment

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

Not just custom props

Copy link
Author

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.

Copy link
Author

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.

Copy link
Owner

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, ...)" ?

Copy link
Author

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)" ?

Copy link
Author

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

Copy link
Owner

Choose a reason for hiding this comment

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

Hum, interesting :)

Copy link
Author

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. :)

Copy link
Owner

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.

Copy link
Author

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.

@MoOx
Copy link
Owner

MoOx commented Jun 30, 2015

I will merge just before next release. Thanks for this contribution.

@hgl
Copy link
Author

hgl commented Jul 16, 2015

It should probably be called customMediaQueries? That's how the spec named it.

@MoOx
Copy link
Owner

MoOx commented Jul 16, 2015

"Media queries might use some custom defined media." so I don't think this should be named this way at all. It's @custom-media not @custom-media-query or something.

@hgl
Copy link
Author

hgl commented Jul 16, 2015

The whole section is named Custom Media Queries.

When defining @custom-media, the spec says:

A custom media query is defined with the @Custom-Media rule:

The word "custom media" only seems to appear in the syntax, and I think the word "query" is omitted only for abbreviation reason.

@MoOx
Copy link
Owner

MoOx commented Jul 16, 2015

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 */
}

screen looks like a media to me. Not a media query.

@hgl
Copy link
Author

hgl commented Jul 16, 2015

If you look at the syntax definition, screen is <media-type>, which is part of <media-query>.

@MoOx
Copy link
Owner

MoOx commented Jul 16, 2015

Exactly, a media (type) is not a media query. It might be in it, but they are not equal.

media query: screen and (orientation: portrait)
media (type): screen, (orientation: portrait)

@hgl
Copy link
Author

hgl commented Jul 16, 2015

A media type belongs to a media query.

In the case of @media screen, screen is the media query, where the media query only contains a single media type.

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.

@MoOx
Copy link
Owner

MoOx commented Jul 16, 2015

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.

@hgl
Copy link
Author

hgl commented Jul 16, 2015

The object generated by the exact option currently contains a customMedias property, I'm proposing to change that to customMediaQueries, which I think is in the scope of this PR.

If you feel strongly about this. I relent. :)

@MoOx
Copy link
Owner

MoOx commented Jul 16, 2015

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.

@hgl
Copy link
Author

hgl commented Jul 16, 2015

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.

@MoOx
Copy link
Owner

MoOx commented Jul 16, 2015

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 @custom-media syntax. Not a big deal. If some others ask for this we might change it in cssnext v2.x.

Ref #185

@hgl
Copy link
Author

hgl commented Jul 16, 2015

It doesn't have to be in v2 if you accept customMediaQueries.

The only api that uses customMedia is the features option if I'm not wrong. We can create an alias customMediaQueries, and optionally print a deprecating message if users use customMedia, and only drop customMedia in v2. The name of the postcs-custom-media plugin doesn't matter, it can stay as it is until cssnext reaches v2. Users shouldn't care about the names of plugins.

Since the extract option isn't released yet, it won't create a compatibility issue.

@MoOx
Copy link
Owner

MoOx commented Jul 16, 2015

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...

@hgl
Copy link
Author

hgl commented Jul 16, 2015

what does this bring to the end user

The main advantage customMediaQueries has over customMedia is that it doesn't have to deal with the awkwardness of the plural form.

We were arguing about customMedia or customMedias, so I was thinking proposing customMediaQueries might end this awkwardness.

@MoOx MoOx mentioned this pull request Dec 29, 2015
5 tasks
@MoOx MoOx closed this in #238 Jan 4, 2016
@MoOx
Copy link
Owner

MoOx commented Jan 4, 2016

Not relevant anymore. cssnext is now postcss-cssnext. cssnext is deprecated. See postcss-cssnext migration guide http://cssnext.io/postcss/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants