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

Bug: CLI: Smell on reading parameters #3205

Closed
ivantcholakov opened this issue Jul 3, 2020 · 2 comments · Fixed by #3398
Closed

Bug: CLI: Smell on reading parameters #3205

ivantcholakov opened this issue Jul 3, 2020 · 2 comments · Fixed by #3398
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@ivantcholakov
Copy link

ivantcholakov commented Jul 3, 2020

I am testing a command line like this:

php spark assets:compile test-less-min

The name test-less-min does not appear within the parameters BaseCommand::run(array $params) (at the successor class).

php spark assets:compile test

This works.

If there are dashes inside string parameters, they (the parameters) are wiped out.

I briefly tried to locate what might cause this effect, here is a visual check, I am not sure whether it is enough:

if (mb_strpos($_SERVER['argv'][$i], '-') === false)
- what comment says as an intention and what the following line does are different.

$arg = str_replace('-', '', $_SERVER['argv'][$i]);
- I wonder here why ALL the dashes are to disappear. Maybe using ltrim() would do the job?

@ivantcholakov ivantcholakov added the bug Verified issues on the current code behavior or pull requests that will fix them label Jul 3, 2020
@ivantcholakov
Copy link
Author

ivantcholakov commented Jul 3, 2020

I patched these two lines, but it doesn't seem to be enough. For now I can bypass the problem by using names with underscores:

php spark assets:compile test_less_min

ivantcholakov added a commit to ivantcholakov/starter-public-edition-5 that referenced this issue Jul 3, 2020
@ivantcholakov
Copy link
Author

Yes, the fix works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant