-
Notifications
You must be signed in to change notification settings - Fork 94
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
Better API or docs #1
Comments
@hawkrives seemed to have the best idea. @hawkrives care to put your proposed syntax in here? |
@ai If we rewrite this from scratch does this need to be a fork? Looks funny. |
Oh, uh, I'll go look for my proposal. Was it in the old repo somewhere? |
Found it. TL;DR: For other references, check out Browserify: Browserify gets all the arguments outside of the square brackets, and passes the ones inside to the invoked command. The arguments are parsed into something like:
Substack pulled the subarg parsing into its own package, at subarg. An example for PostCSS would probably look something like Note that if you don't want to pass any args to the plugin, you dont need the brackets: |
and/or other proposal:
I took |
@hawkrives Thanks. How would multiple plugin options work? For instance, what if I wanted to pass |
Probably something like (subarg and therefore minimist treat |
Gotcha. I think I like the plugin name on the outside of the brackets a bit better.
I'm also not sure I like the arrow.
Thoughts? Sidenote: I've been using Browserify all night to get more familiar with it's syntax. I can't help but to think there are a lot of brackets and curly braces involved.
|
Anyone have any special feelings about https://github.com/sindresorhus/meow ? Looks pretty kewl to me. 💯 |
@corysimmons meow does not allow to use file config, only field from package.json. We have https://github.com/davidtheclark/cosmiconfig for such cases. |
We can use the two together right? |
Hm.. yep, right. |
💯 |
Adding value of |
That’s standard unix redirection, so it makes sense for it to be there, you can’t really take it out.
Why complicate? Just behave like any other *nix tool. I’m pretty sure what @hawkrives meant was for it to behave akin to |
Yeah, we need to follow these CLI conventions that allow for piping, redirection, etc. |
@corysimmons: @vitorgalvao has it right. I've been thinking that if you give it one input file, and no @corysimmons: About the braces: Babel (and babelify) has developed a lot of configurability with its latest release. And I was thinking that we could just use the subarg library to split up the options, which uses the first word inside the square brackets to denote the name of the … argument group, I guess we'd call it. |
Arrow is cool then. 👍 https://github.com/substack/subarg/blob/master/index.js seems pretty straightforward and hasn't been updated in years. We could make something similar and change the syntax from Even if we use the In fact, I really don't care about the |
@hawkrives Whatcha think? |
BTW, we can move current |
wait, did I not reply last night? whoops.
|
and minimist is actually pretty powerful by itself: you can do automatic booleans and pass nested objects as args. (live example: https://tonicdev.com/hawkrives/56b0cc8b65d8fc0c00633ef6)
{
"_": [],
"p": 1,
"power": false,
"nested": {
"object": {
"key": true,
"value": false
}
}
} |
But maybe we should figure out how we want it to look before we go bikeshedding on argument parsers 😜 |
💯 I still like |
I don't like the above, it's ambiguous. command line arguments should be in the form of a key value pair, or a boolean flag. A command line tool should not have to rely on the arguments order for option parsing as that syntax does. Personally I use minimist which handles nested options well, as @hawkrives wrote. I don't see the point of reinventing the wheel for arguments parsing here, as a good range of solutions for this exist already. |
I'm all about minimist as it makes our job (and my desire to use Meow) that much easier. I'm just personally not a fan of that syntax. I dunno, can someone show me a minimist cmd that might include 2 or 3 postcss plugins with a few options assigned to each one? Like a real world command? |
@davidtheclark 99% of the time, people will be using configs instead of trying to config everything from the CLI. No one is saying we shouldn't heavily promote this approach. I'm just suggesting that we try to make the CLI as flexible as possible so we can support as many use cases as possible. No CLI supports passing functions as args. People still make CLI's that accept args...
This a million times over. I don't want to configure something to play around with something... Anytime I make a semi-serious project (something I'm gonna be working on for more than 30 minutes) I configure it. Otherwise forcing people to configure something is a PITA. |
Finally had the time to read through this all. I think @davidtheclark brings the most sensible valid points to the table. Loading the configuration in the form of JavaScript enables for powerful options that can not be done in JSON on top of that this will work with all plugins. We shouldn't reinvent the CLI but users should be able to configure the plugins by only using the CLI as far as that is possible in my opinion. Also if we're passing multiple plugins with the same configuration flags wouldn't that cause issues? And thus mean we need to get the flags in a 'nested' form to prevent this problem? |
I think everyone except @davidtheclark agrees with this so we should move forward and just make sure to thoroughly document that a config file is the preferred method.
@hawkrives @ben-eb I think we're leaning towards minimist. Is this true for minimist? |
Like, Stock {
"_": ["input.css"],
"p": [
"autoprefixer",
"cssnano"
],
"browsers": [
"> 1%",
"> 1%"
],
"o": "output.css"
} This is kinda what
The brackets give let it know what key to associate the extra options with. {
"_": ["input.css"],
"p": [
{
"_": ["autoprefixer"],
"browsers": "> 1%"
},
{
"_": ["cssnano"],
"browsers": "> 1%"
}
],
"o": "output.css"
} Does that answer your question, @corysimmons? Edit: Live example again: https://tonicdev.com/hawkrives/56b26488414a7c0c0012b301 |
Yeah thanks @hawkrives I agree with @ben-eb that minimist looks best, but minimist just doesn't seem capable of passing plugin options in a robust way. Maybe we should go back to not worrying about what tool we're gonna use and just focus on the API. Once we get that we can make a tool. So can we all agree on this syntax? postcss -p autoprefixer --browsers '> 1%' --no-remove -p cssnano --browsers '> 1%' in.css > out.css If so, I think we can fix minimist to match
|
Minimist looks nicer, I'll agree, but I really like the unambiguousness of the Subarg syntax. |
lol, dammit, now I'm back to thinking (sorry I just really don't like the plugin name being inside the brackets for some reason...) |
We're back to #1 (comment) @ai Read over this thread and pick one so we can move forward. 💃 |
@corysimmons ha-ha :) this question is too tricky. Right now I am thinking too that |
@sindresorhus thinks we should:
sindresorhus/meow#30 (comment) Not sure how I feel about it. On one hand, json-like syntax is pretty easy to parse. On the other hand, where does it stop? Do 2 plugins look like: Anyone like that? |
No:
Alternatively:
Which I think I prefer. |
I’m in agreement with @sindresorhus, for one simple important reason: clarity. I remember fighting with a tool (perhaps it was I’m a heavy CLI user and build a lot of (mostly) Take this autoprefixer example. With @sindresorhus’ suggestion, it’d be trivial to convert. No need to transform their options into From the two examples, I think I prefer the first, but any of them seems like an improvement over the alternatives. |
@ai Objections? Also, is anyone considering stepping up to dev this? |
And as commented in sindresorhus/meow#30 (comment), you can use |
Thinking about submitting a PR with improved instructions on the use of a configuration JSON (instead of CLI arguments). It was kinda tricky for me at first and some people at work also got confused by it. @ai Thoughts? |
It'd be nice if we exposed config files to all 3 standard config types as well as CLI args: https://github.com/davidtheclark/cosmiconfig |
@ai Can you finalize this so someone can move forward? |
@ai ping? |
@RyanZim I don’t use this project, so my thoughts will not be useful :) |
But, right now I am thinking about having one common config for any PostCSS runner: https://github.com/michael-ciniawsky/postcss-load-plugins and https://github.com/michael-ciniawsky/postcss-load-config I think CLI also should use that common config. |
Personally, I agreed on this #1 (comment) and also I'd like to update all the cli option as well at the same time, like a breaking change. |
👋
{
"name": "css",
"main": "postcss.config.js",
"scripts": {
"css:prod": "NODE_ENV=production postcss -o dest/index.css src/index.css",
"css:dev": "NODE_ENV=development postcss -o dest/index.css src/index.css",
},
}
postcss-config-boilerplate
package.json {
"name": "postcss-config-[name]",
"main": "index.js",
"postcss": {
"parser": "sugarss",
"plugins": {
"postcss-import": { option: 'foo', option: 'bar' }
}
},
"dependencies": {
"postcss-import": "^8.1.2"
}
} index.js (postcss.config.js) const options = require('package.json').postcss
const plugins = require('package.json').postcss.plugins
module.exports = (ctx) => {
parser: ctx.parser || options.parser
plugins: {
'postcss-import': ctx.import || plugins['postcss-import']
...
}
} |
Hi guys! Any news on using cosmiconfig? |
@thomasklein Right now, we are working on postcss-cli v3. That is a complete rewrite that will use https://github.com/michael-ciniawsky/postcss-load-config, which uses cosmiconfig under the hood. See https://github.com/postcss/postcss-cli/projects/1 for more details and progress updates. |
v3.0.0 is out, so going to say that fixes this issue. https://github.com/postcss/postcss-cli/releases/tag/v3.0.0 Please open new issues for anything that you think could be improved. Thanks for all the brainstorming here! |
Let’s think how we make CLI better.
The text was updated successfully, but these errors were encountered: