-
-
Notifications
You must be signed in to change notification settings - Fork 580
Fixed issue #1898: API for querying the active mode(s) #733
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
Conversation
The failing test on PHP-8.1 (nightly) on Ubuntu is not related to this PR and occurs on
This test passes on Windows nightly builds. EDIT: This failed on PHP php/php-src@6690547 but now seems to be okay. |
Although it is possible to obtain the mode value in userland, it is quite cumbersome and possibly brittle to do so. This is fixed with the addition of an `xdebug_mode()` function: xdebug_mode(?string $mode = null) : mixed Returns the enabled xdebug modes as a comma-separated string, or if the `$mode` parameter is supplied, whether the named mode(s) are set. For example, using `xdebug.mode=debug,develop,coverage`. ``` var_dump(xdebug_mode()); // string(22) "develop,coverage,debug" var_dump(xdebug_mode('off')); // bool(false) var_dump(xdebug_mode('debug,coverage')); // bool(true) var_dump(xdebug_mode('trace,debug')); // bool(false) ```
Derek wrote:
I've updated the commit message to match the already-reported issue. Please consider this PR as an example of such a function and do with it as you wish. |
xdebug_str_add_literal(&mode_out, "off"); | ||
} else { | ||
if (XDEBUG_MODE_IS(XDEBUG_MODE_DEVELOP)) { | ||
xdebug_str_add_literal(&mode_out, "develop,"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an array would be better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure Derek is going to do his own thing here, but why do you think that an array would be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, to me, an array of strings seems easier to understand than a single string, where we need to remember that the string actually contains multiple modes in a comma-separated way (for which a check would probably be done by exploding the string anyway)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stof Yeah, I get that. But you don't need to explode the string to do an in_array()
check, because you already have xdebug_mode('modes,you,want,to,check')
functionality which returns true/false.
I have assumed that usage would be either to check a value (as above), or to set a value(s) in a new process - which you can do simply with $mode = xdebug_mode().',modes,you,need'; etc
, rather than having to implode an array.
I also thought that if you returned an array of values, you should really only accept an arrary of values, for consistency, which seemed cumbersome if you only wanted to check a single value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xdebug_mode().',modes,you,need'
is one example why you want array ;-)
Clean code is array_unique([...xdebug_mode(), 'modes', 'you', 'need'])
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mvorisek Why? It doesn't matter (each value is just OR
-ed to the current value) and is consistent with how the ini and env values are parsed/used.
should this be a single function having 2 totally different behavior depending on whether it is called with 1 or 2 parameters, or 2 separate functions ? |
we could have |
What I likely want to do is to extend Right now,
Alternatively I could make multiple arguments to And then perhaps later I can extend this with |
I have for now settled on:
Let me know if that works. With that, I'll close this PR and y'all can follow #737 |
Thanks @derickr Seems like a nice uncomplicated solution. |
Although it is possible to obtain the mode value in userland, it is
quite cumbersome and possibly brittle to do so. This is fixed with the
addition of an
xdebug_mode()
function:xdebug_mode(?string $mode = null) : mixed
Returns the enabled xdebug modes as a comma-separated string, or if the
$mode
parameter is supplied, whether the named mode(s) are set.For example, using
xdebug.mode=debug,develop,coverage
.