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

Conversation

Projects
None yet
3 participants
@shadyvb
Copy link
Contributor

commented Jun 21, 2014

Traverses array members looking for changes

Track changes to options more deeply
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 );

This comment has been minimized.

Copy link
@lukecarbis

lukecarbis Jun 25, 2014

Contributor

Minor nit: Can you please align equals signs here?

This comment has been minimized.

Copy link
@lukecarbis

lukecarbis Jun 25, 2014

Contributor

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 );

This comment has been minimized.

Copy link
@shadyvb

shadyvb Jun 27, 2014

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

This comment has been minimized.

Copy link
@fjarrett

fjarrett Jun 30, 2014

Contributor

@shadyvb Can you teach me what this means?

This comment has been minimized.

Copy link
@shadyvb

shadyvb Jun 30, 2014

Author Contributor

@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.

This comment has been minimized.

Copy link
@fjarrett

fjarrett Jun 30, 2014

Contributor

@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

This comment has been minimized.

Copy link
Contributor

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

Merge pull request #573 from x-team/deep-options-tracking
Track changes to options more deeply

@fjarrett fjarrett merged commit bed8bd1 into develop Jul 18, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@fjarrett fjarrett deleted the deep-options-tracking branch Jul 18, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.