Fix to not leave any garbage.

[robario]

This realize optional arguments.

{
    "echo": "echo args are",
    "foo": "run-s \"echo -- {1}\" --"
}

The current specification and implementation are strange.
Since users can not be avoided, it is no exaggeration to say that bug.

$ npm run foo
> run-s "echo -- {1}" --
> echo args are "{1}"

args are "{1}"

On the other hand, this p-r's result is

$ npm run foo
> run-s "echo -- {1}" --
> echo args are

args are

Again, the user still have a way to pass-through {1} as is.

[robario]

I'm sorry. I'm in fixing the tests.

[robario]

done.

applyArguments should be applied when there is a placeholder, not an argument.

[mysticatea]

Thank you for this PR.
I'm sorry for late response. I was busy for past several days.

Implement looks good to me.

It had backward compatibility if it keeps placeholders when arguments are nothing. So I could release it as a minor version.
But this is a breaking change, so it needs a major version.

[robario]

I apologize that I had overlooked.
To tell the truth, I noticed a look at the test you have added after it had been released.

[mysticatea]

If it removed placeholders when arguments are nothing, it was a breaking change because the behavior of npm-run-all "foo {0}" will be changed even if it's before the release of current placeholders feature.

I'll merge this PR within a few days.
Indeed, if placeholders remain, it's bad.

[robario]

Yes, I knew it.
Thank you again.

BTW, I have thought hasPlaceholder cheking is not useful and over optimization.
I just follow your code that is hasArguments checking.

[robario]

@mysticatea
Any update on this? In fact, the placeholder is useless until resolve this problem e.g. ls: {@}: No such file or directory.
Thank you for your hard work!

[mysticatea]

I'm sorry, just I have been busy.
My plan is:

1. now I'm working on Improve tests for parallel features. #53.
2. after I finish it, I will merge this and work on Feature: default values for argument placeholders. #54.
3. then I will release 3.0.0.

[robario]

@mysticatea
I know what you mean.
If so, the v2.x's user can NOT use the placeholder feature, does they?
Because >=v2.2.0 has such a bug:

{
  "list": "run-s \"ls -- {@}\" --",
  "ls": "ls"
}
$ npm run list
ls: cannot access '{@}': No such file or directory
$ npm run list ./
# no problem

I think #50 does not mean a breaking change of spec but just a bugfix and which is needed to >=v2.2.0.
Because no one expects the current behavior even if v2.x's user.
#50 is necessary in order to ensure that v2.x's user can use the placeholder feature correctly.

[mysticatea]

This PR is a breaking change.
Because {1} had not been removed before placeholders feature is added.

[robario]

I agree with you as long as you decided so.
But then, almost all of the bug-fixes become breaking change to a greater or lesser.
So still, I think it is the bug of implementation not that of specification.

I'm looking forward to release of v3 to be able to use placeholder feature, since it could not be used in v2.x.
Best Regards.