Home Original page

Track changes to options more deeply by shadyvb · Pull Request #573 · xwp/stream

@shadyvb

Traverses array members looking for changes

@shadyvb 

Traverses array members looking for changes

lukecarbis

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?

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

Choose a reason for hiding this comment

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

@shadyvb

frankiejarrett

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?

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.

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.

@frankiejarrett

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

@frankiejarrett

frankiejarrett added a commit that referenced this pull request

Jul 18, 2014 
Track changes to options more deeply