Feature #53677
closed
GeneralUtility::rmFromList : allow to call GeneralUtility::trimExplode
100%
Description
If a list contains space between comma and value, rmFromList doesn't unset element.
Example :- "1,2, 3,4"
- If we want to delete "3" entry, we have to pass " 3" instead of "3"
Current code is :
static public function rmFromList($element, $list) { $items = explode(',', $list); foreach ($items as $k => $v) { if ($v == $element) { unset($items[$k]); } } return implode(',', $items); }
To avoid breaking changes, it can be introduced a new argument which enable trim.
Improved code :
static public function rmFromList($element, $list, $trimElements = false) { if(!$trimElements){ $items = explode(',', $list); } else{ $items = self::trimExplode(",", $list); } foreach ($items as $k => $v) { if ($v == $element) { unset($items[$k]); } } return implode(',', $items); }
Files
Updated by Gerrit Code Review almost 11 years ago
- Status changed from New to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25723
Updated by Clément MICHELET almost 11 years ago
I'm sorry, I was not able to set a proper testing environnement. I've done a git clone (multiple times) from master by following EGit tutorial. The "t3lib" folder was missing so the installation was broken.
I add a patch file for GeneralUtilityTest to include new test cases. If the new tests cases pass, I will add my vote to the review.
In current test method "rmFromListRemovesElementsFromCommaSeparatedListDataProvider", I've added a spaced value to check if there is no regression (= not removed). I've added a new method and data provider to test when we enable trimElements.
EDIT: Take the last patch file
Updated by Clément MICHELET almost 11 years ago
Updated by Gerrit Code Review almost 11 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25723
Updated by Gerrit Code Review almost 11 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25723
Updated by Gerrit Code Review almost 11 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25723
Updated by Gerrit Code Review almost 11 years ago
Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25723
Updated by Jo Hasenau almost 11 years ago
See my comment on #55104: A CSV list must not contain spaces, so it is not the job of the tool that handles a CSV list to remove them.
Updated by Tomita Militaru almost 11 years ago
I've looked at your comment, but the patch implements another boolean parameter in order to decide when to trim.
Also, I'm not sure if it's mandatory to follow the CSV standard in this case.
Updated by Gerrit Code Review almost 11 years ago
Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25723
Updated by Gerrit Code Review almost 11 years ago
Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25723
Updated by Gerrit Code Review almost 11 years ago
Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25723
Updated by Tomita Militaru almost 11 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset f83873bf1c95f5a371955e969fdb0f0bcc0fcc46.
Updated by