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

Parse --web option correctly #221

Merged
merged 1 commit into from
Dec 30, 2022

Conversation

sideshowcoder
Copy link
Contributor

getParameterOption returns the unparsed / unvalidated result of the option, which might result in incorrect results when reordering options:

php cachetool.phar opcache:status --web --web-path=/var/www

returns --web-path=/var/www as the value for the --web option when using getParameterOption, getting the parsed result is done by using getOption which returns the correctly parsed value, in this case NULL, which should result in the default being set which is FileGetContents.

This avoids the warning that the webclient is unkown when passing --web without specifying the client.

`getParameterOption` returns the unparsed / unvalidated result of the option,
which might result in incorrect results when reordering options:

```
php cachetool.phar opcache:status --web --web-path=/var/www
```

returns --web-path=/var/www as the value for the --web option when using
`getParameterOption`, getting the parsed result is done by using `getOption`
which returns the correctly parsed value, in this case `NULL`, which should
result in the default being set which is FileGetContents.

This avoids the warning that the webclient is unkown when passing --web without
specifying the client.
@sideshowcoder
Copy link
Contributor Author

sideshowcoder commented Dec 30, 2022

This might be a useful change for the other options as well, and I'm happy to go through those but wanted to first fix the problem at hand, which is that I see a warning when passing --web without specifying a client.


phil@phoenix ~/source/cachetool (main) $ sudo php bin/cachetool opcache:status --web --web-path=/var/www/html/ --web-url=http://localhost/
[2022-12-30T14:01:35.416151+00:00] cachetool.WARNING: Web client `--web-path=/var/www/html/` not supported - defaulting to FileGetContents [] []
+----------------------+---------------------------------+
| Name                 | Value                           |
+----------------------+---------------------------------+

@gordalina gordalina merged commit eedac2f into gordalina:main Dec 30, 2022
@gordalina
Copy link
Owner

This is great, thank you! Let me know if you go through the other options, I'll wait for any PRs before I issue the next release.

@sideshowcoder
Copy link
Contributor Author

If you are happy to take the contribution I'll go through them in the new year and get a diff ready thanks!

@gordalina
Copy link
Owner

If you are happy to take the contribution I'll go through them in the new year and get a diff ready thanks!

Yes please! Thank you

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

Successfully merging this pull request may close these issues.

2 participants