Enh #100: Make CLogFilter::getContext() able to handle more precise $logVars #2180

Closed
wants to merge 5 commits into
from

Conversation

6 participants
Contributor

tomtomsen commented Mar 8, 2013

This should fix #100

Needed a recursive method, which I thought would be great in a more global scope. so i put it to CMap.

Added tests and other tests seems to work nicely.

cebe was assigned Mar 10, 2013

@klimov-paul klimov-paul commented on an outdated diff Apr 24, 2013

framework/collections/CMap.php
@@ -118,6 +118,10 @@ public function getKeys()
*/
public function itemAt($key)
{
@klimov-paul

klimov-paul Apr 24, 2013

Member

Why this method is affected?

@klimov-paul klimov-paul commented on an outdated diff Apr 24, 2013

framework/collections/CMap.php
@@ -298,6 +302,35 @@ public static function mergeArray($a,$b)
}
/**
+ * Walks through a given array by a specified path.
+ * Each item of the path array specifies the next nested key
+ * in the array
+ * @param array $array array to be searched
+ * @param array $path array of each level key
+ * @return mixed the value of the array at the end of the path
+ */
@klimov-paul

klimov-paul Apr 24, 2013

Member

Function name sounds inappropriate.
"wander" usually means go to the random direction.
Better be "searchArrayPath" or simething like that.

Contributor

tomtomsen commented Apr 29, 2013

I renamed wanderArray to searchArrayPath (sounds way better, thx)
i thought it would be nice to extend itemAt, so it can accept arrays too. in such case, searchArrayPath is called.

tom tomsen added some commits Apr 29, 2013

Member

klimov-paul commented Apr 29, 2013

I am not sure "CMap::itemAt()" should use "searchArrayPath()".
Perhaps a seprated method should be created for this.
Also if this change applied at least method doc comments should be updated appropriately.

I need someone else opinion on this matter.

Owner

qiangxue commented Apr 29, 2013

I don't think CMap should be touched as the new addition introduces a new usage that is uncommon and doesn't belong to a basic class like CMap.

Just implement everything in CLogFilter. If we find the code may be frequently reused elsewhere, we may refactor it later.

@samdark samdark commented on the diff Jun 24, 2013

framework/collections/CMap.php
@@ -46,7 +46,7 @@ class CMap extends CComponent implements IteratorAggregate,ArrayAccess,Countable
/**
* Constructor.
* Initializes the list with an array or an iterable object.
- * @param array $data the initial data. Default is null, meaning no initialization.
+ * @param array $data the intial data. Default is null, meaning no initialization.
@samdark

samdark Jun 24, 2013

Owner

Why introducing typos? Everything was correct.

@samdark samdark commented on the diff Jun 24, 2013

framework/collections/CMap.php
@@ -225,7 +225,7 @@ public function copyFrom($data)
* If the merge is recursive, the following algorithm is performed:
* <ul>
* <li>the map data is saved as $a, and the source data is saved as $b;</li>
- * <li>if $a and $b both have an array indexed at the same string key, the arrays will be merged using this algorithm;</li>
+ * <li>if $a and $b both have an array indxed at the same string key, the arrays will be merged using this algorithm;</li>
@samdark

samdark Jun 24, 2013

Owner

Why introducing typos? Everything was correct.

@samdark samdark commented on the diff Jun 24, 2013

framework/collections/CMap.php
@@ -272,7 +272,7 @@ public function mergeWith($data,$recursive=true)
* For integer-keyed elements, the elements from the latter array will
* be appended to the former array.
* @param array $a array to be merged to
- * @param array $b array to be merged from. You can specify additional
+ * @param array $b array to be merged from. You can specifiy additional
@samdark

samdark Jun 24, 2013

Owner

Why introducing typos? Everything was correct.

@samdark samdark commented on the diff Jun 24, 2013

CHANGELOG
@@ -52,6 +52,7 @@ Version 1.1.14 work in progress
- Bug #2406: CUrlManager::$urlRuleClass now supports path alias value (as it was described in its PHPDoc before this fix) (resurtm)
- Enh: Better CFileLogRoute performance (Qiang, samdark)
- Enh: Refactored CHttpRequest::getDelete and CHttpRequest::getPut not to use _restParams directly (samdark)
+- Enh #100: CLogFilter::$logVars allows now an array of arrays. (tomtomsen)
@samdark

samdark Jun 24, 2013

Owner

No need for dot at the end.

Owner

samdark commented Jun 24, 2013

Please fix code style.

resurtm closed this in f12658e Jun 28, 2013

Contributor

resurtm commented Jun 28, 2013

@tomtomsen, thanks! Merged manually using better way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment