Feature #53677: GeneralUtility::rmFromList : allow to call GeneralUtility::trimExplode - TYPO3 Core - TYPO3 Forge

Actions

Copy link

Feature #53677

closed

GeneralUtility::rmFromList : allow to call GeneralUtility::trimExplode

Added by Clément MICHELET over 10 years ago. Updated over 5 years ago.

Status:

Closed

Priority:

Could have

Assignee:

Category:

Target version:

Start date:

Due date:

% Done:

100%

Estimated time:

PHP Version:

Tags:

Complexity:

no-brainer

Sprint Focus:

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

Download all files

53577-GeneralUtilityTest.patch (1.75 KB) 53577-GeneralUtilityTest.patch		Clément MICHELET, 2013-11-29 16:47
53577-GeneralUtilityTest.patch (1.82 KB) 53577-GeneralUtilityTest.patch	Replace wrong patch files which have missing commas	Clément MICHELET, 2013-11-29 18:01

Related issues 2 (0 open — 2 closed)

Actions

Copy link

Updated by Gerrit Code Review over 10 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

Actions

Copy link

Updated by Clément MICHELET over 10 years ago

File 53577-GeneralUtilityTest.patch 53577-GeneralUtilityTest.patch added

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

Actions

Copy link

Updated by Clément MICHELET over 10 years ago

File 53577-GeneralUtilityTest.patch 53577-GeneralUtilityTest.patch added

Actions

Copy link

Updated by Gerrit Code Review over 10 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

Actions

Copy link

Updated by Gerrit Code Review over 10 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

Actions

Copy link

Updated by Gerrit Code Review over 10 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

Actions

Copy link

Updated by Gerrit Code Review over 10 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

Actions

Copy link

Updated by Jo Hasenau over 10 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.

Actions

Copy link

Updated by Tomita Militaru over 10 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.

Actions

Copy link

#10

Updated by Gerrit Code Review about 10 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

Actions

Copy link

#11

Updated by Gerrit Code Review about 10 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

Actions

Copy link

#12

Updated by Gerrit Code Review about 10 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

Actions

Copy link

#13