Skip to content
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

pass on the -- flags to scripts #90

Closed
ajaymathur opened this issue Dec 28, 2017 · 15 comments
Closed

pass on the -- flags to scripts #90

ajaymathur opened this issue Dec 28, 2017 · 15 comments

Comments

@ajaymathur
Copy link
Member

ajaymathur commented Dec 28, 2017

this enable to pass options to scripts from cli 🙂

@bradleyayers
Copy link

Not really sure what this issue is about, but I'm hoping it's related. I basically want to run bolt w @heydovetail/dql test --watch, where my test script is just running Jest. Is there a way to do that today without writing another NPM script (e.g. test:watch)?

@lukebatchelor
Copy link
Member

Not currently, no. But yes, I believe that's what this issue was for.

I can't remember exactly where this ended up, but I think the complexity was around how we know which flags are supposed to be passed to the script, and which are passed to bolt itself.

i.e

bolt ws run test --only="@heydovetail/dql" --watch

The only is meant for bolt, the watch is meant for the script.

@d4rkr00t
Copy link
Member

That's why I like npm's approach more then yarn's. Because in npm it's clear which arguments belongs where :)

@lukebatchelor
Copy link
Member

How's that @d4rkr00t ? using an extra --? That's what we do for separating the command when running exec

bolt ws exec --only="@heydovetail/dql" -- yarn test --watch

@d4rkr00t
Copy link
Member

Yeah i like -- as it's clearer to me, that it separates npm flags from command flags...

@lukebatchelor
Copy link
Member

Yea, I agree actually. If @jamiebuilds agrees, I could look at fixing that soon™?

@marcins
Copy link
Contributor

marcins commented Jul 30, 2018

I put some examples of a related issue we ran into in #183. FWIW running a yarn command with -- gives a deprecation warning:

› yarn lint:css -- --bamboo
yarn run v1.9.2
warning From Yarn 1.0 onwards, scripts don't require "--" for options to be forwarded. In a future version, any explicit "--" will be forwarded as-is to the scripts.

@cdoan-atlassian
Copy link

I noticed this command in the readme:
bolt ws exec -- [cmd]

I feel like that might cause confusion if we use the -- there to separate the shell command, vs in other cases where it's separating the params passed to the command

@bradleyayers
Copy link

bradleyayers commented Aug 29, 2018

I'm a 👎🏻to using --. It's more explicit which is great, but for me that's not enough to outweigh the pain of typing it. Explicit is great for programming languages where there are more reads than writes, but for a CLI I think ergonomics win. For example it's why I type -h instead of --help.

/bikeshed

@lukebatchelor
Copy link
Member

The only other way to do it then would be to have the order matter, and I think even with that, it would still cause problems for any tool that took any of the same flags we want to use (i.e meow would either only accept one, or possibly treat it as an array).

Personally, I'd still be for the --. I'll check with Jamie tomorrow.

@bradleyayers
Copy link

The algorithm I had in mind was to treat the first argument not starting with - as the start of the command to execute. So bolt ws exec --only="@heydovetail/dql" yarn test --watch would treat everything after (and including) yarn as the command for exec to run.

It means that you can't run commands that start with a -, but that seems like a reasonable trade-off.

@bradleyayers
Copy link

Is there any movement on this issue?

@lukebatchelor
Copy link
Member

Soon™️

We've decided to go with the -- separator though, fyi.

@drew-walker
Copy link

drew-walker commented Nov 14, 2018

Any more updates on this? We're trying to move to Bolt for Confluence and we have a bunch of commands like this:

  "build:es5": "babel src --out-dir dist/esm --source-maps --ignore **/*-test.js --copy-files",
  "start": "yarn build:es5 --watch"

If we want to switch to Bolt I think as of today it'll mean duplicating the command just for the purpose of adding --watch to the end. Is that right?

@lukebatchelor
Copy link
Member

So, I've just started digging into this and have a much nicer idea that I think should work for the most use cases.

So, if we look at all the ways you have of running commands right now you've got

bolt foo
bolt run foo
bolt p run foo
bolt w @dovetail/bar run foo
bolt ws --only="@dovetail/bar" run foo
bolt ws exec --only="@dovetail/bar" -- yarn foo

and right now only one of those would let you pass flags in easily (the last one). As noted above, right now, yarn will actually pass all flags down to a script, so if you have

"scripts": {
  "test": "jest src/** --bail"
}

You can run yarn test --watch and you'll end up with `jest src/** --bail --watch.

The biggest issues with just passing all flags in that I could see was that we'd need to have a way of knowing which flags are meant for bolt and which were meant for the script being called.

Well, looking deeper into this issue, that actually only really applies to bolt ws run. No other commands take other flags. This means we could easily make bolt pass on all flags for

bolt foo --flag
bolt run foo --flag
bolt p run foo -- flag
bolt w @dovetail/bar run foo --flag

The only tricky one then is bolt ws run. But a simple enough, if not slightly hacky approach, would be to check which flags are known by bolt ws and don't pass those down (i.e --only, --onlyFS, --ignore, --ignoreFS, --parallelNodes, --parallel, --serial).

Obviously the biggest downside here would be that if the flag you wanted to pass down was one of those, you wouldnt be able to.

So the alternative solution would be to make this the only command that uses the -- to pass down extra flags. My only reservations here would be that it's creating an exception. But at the same time, I think it's probably also going to be quite rare.

In code it's certainly less hacky to implement, and so I'm leaning towards that as a solution. I'll have a PR out today to try it out and spar it with a couple of people, but if anyone happens to see this, I'd love some inputs.

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

No branches or pull requests

7 participants