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

Track changes to options more deeply #573

Merged
merged 3 commits into from Jul 18, 2014
Merged

Conversation

shadyvb
Copy link
Contributor

@shadyvb shadyvb commented Jun 21, 2014

Traverses array members looking for changes

Traverses array members looking for changes
if ( is_array( $old_value[ $key ] ) && is_array( $new_value[ $key ] ) ) {
$inner = array();
$parent = $key;
$changed = self::get_changed_keys( $old_value[ $key ], $new_value[ $key ], --$deep );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: Can you please align equals signs here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the pre-decrement used here is overly tricky. My preference is to only use decrements and increments if they're on their own line. It might be more readable this way.

$deep--;
$changed = self::get_changed_keys( $old_value[ $key ], $new_value[ $key ], $deep );

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ( is_array( $old_value[ $key ] ) && is_array( $new_value[ $key ] ) ) {
$inner = array();
$parent = $key;
$deep--;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shadyvb Can you teach me what this means?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fjarrett If you're talking about $deep, i used it so i can control how deep should i check for changes in the array. 0 stands for the first level only ( do not check children at all ), 1 the first level of children, etc .. for options that are stored hierarchically <input name='plugin_name[section][field]' /> .. and since the function is calling itself, i need to decrement the value on each call.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shadyvb Oh I get it now, decrementing the value. So $deep-- is the opposite of $deep++, makes sense. I've just never seen it before so it almost seemed like a typo! Thanks for explaining.

@fjarrett
Copy link
Contributor

fjarrett commented Jul 8, 2014

@shadyvb I've done some basic testing within Stream, and Settings records appear to be working as expected. It would be great to know the specific use-cases I should be testing here that make this PR necessary though.

fjarrett added a commit that referenced this pull request Jul 18, 2014
Track changes to options more deeply
@fjarrett fjarrett merged commit bed8bd1 into develop Jul 18, 2014
@fjarrett fjarrett deleted the deep-options-tracking branch July 18, 2014 18:59
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.

None yet

3 participants