@javier-de-juan opened this Pull Request on October 24th 2020

This PR tries to add the delete command for config file. For array positions deletion, just admit numeric number because if the code removes one labeled position from the array, all other keys are lost (dumpConfig method related), so I preferred to avoid bad config generation.

@diosmosis commented on October 27th 2020 Member

@tsteur the original issue references having a --value option to delete array elements by value. Is this no longer a requirement?

@tsteur commented on October 27th 2020 Member

@diosmosis good one. That could be useful indeed. @javier-de-juan I think previously one had to set a specific index to delete a value within an index. But the correct index is always hard to know. If it was possible to use --value to delete a value within an array only when it matches that value that might be indeed quite useful. Sorry about that. Would it be possible to add that?

@javier-de-juan commented on October 28th 2020

Ok, there's no problem. But... what's the desired behavior? If 2 settings have the same value the command deletes both? The section is always required, optional? I need a description about what you need to avoid wrong implementation.

@tsteur commented on October 28th 2020 Member

I would say deleting value from an array would only work when specifying the key and the section as well.

Basically --section=General --key=trusted_hosts --value=mydomain.com .

When a value is set, and the configured value is a string (or a number) then we would only delete that value if the configured value matches the value from the value option. AKA if $config->$section[$key] == $configuredValue.

If the configured value is an array, then we only remove that one value from the array that matches the value from the option.

Meaning if [General] trusted_hosts[]="foobar" trusted_hosts[]="barbaz" is configured, then when using --section=General --key=trusted_hosts --value=barbaz then it would remove the last entry from that index. If --value=helloworld was used, nothing would be removed because no value matches that entry.

--section=General --key=trusted_hosts would remove the entire array.

Does this help?

@tsteur commented on November 2nd 2020 Member

@javier-de-juan be great to comment once you made the adjustments then we will do another review. Thanks for all this again

@tsteur commented on December 21st 2020 Member

@javier-de-juan are you maybe still working on this PR?

@sgiehl commented on March 15th 2021 Member

@javier-de-juan do you maybe have some time to apply the review feedback? Would be awesome if we could merge this for the next release.

@tsteur commented on July 27th 2021 Member

@javier-de-juan I will close this PR due to inactivity. If you are still keen on it feel free to let us know and we reopen the PR

This Pull Request was closed on July 27th 2021
Powered by GitHub Issue Mirror