inifuncs.sh: using basic RE to allow meta-chars in keys , eg. +()
As tabled here: https://retropie.org.uk/forum/topic/30714/inifuncs-sh-use-basic-regular-expression-match-in-iniset
Thanks. I will check if the use of egrep was intentional for some reason or if it was just a leftover from previous code. I'm cautious with this code and have in mind to include some ini tests so we can make sure changes work as intended. Also why https://github.com/RetroPie/RetroPie-Setup/pull/2960 isn't included yet.
Will get back to you on this.
The use of egrep is a leftover from very early code. I wasn't sure about the details when you said this fails Vs using grep. Can you give an example? I think I understand the issue but want to be sure. Btw are there are actual cases where this fails in actual use with these characters in keys?
We quote some characters for sed and perhaps we should do the same for grep anyway.
The change seems sensible but I'd be keen to have some tests in place before incorporating it.
I came across this by chance and by setting deliberatly jzintzv+ecs (see #3352) as emulator key (would be handy to visualize it has an extra switch set, I thought), however I learned that dash is the standard atm. NP with that.
It is not broken as it is now upstream, unless some scriptmodule contributors choose to put + and or () in the scriptmodule as emulator key. :wink:
Revisiting this old ticket as I was doing some ini file tests. I was unable to reproduce the issue (keys with +() in worked with both the egrep and the grep). I can't see an issue with this PR but I would like to reproduce the issue before including the change. Please can you provide an example ini file with a key which iniGet/iniSet are working incorrectly on it without this change.
Thanks for picking this up. Sorry to be too brief on my initial report. First I thought it was okay too, but then I remembered the backstory.
It can be reproduced like this:
- Add deliberately an
+as emulator key in a scriptmodule'sconfig_xyz()function. E.g. change the-to a+for the emulator key in this line - Run the
sudo ./retropie_packages.sh jzintv configurecommand once - Notice the
jzintv+ecsis added correctly in the/opt/retropie/configs/intellivision/emulators.cfg - Run the
sudo ./retropie_packages.sh jzintv configurea second time - Notice that the
jzintv+ecsentry inemulators.cfgis added a second time (=unexpected outcome)
Expected outcome: jzintv+ecs is only added once even after multiple ... configure calls.
I see these options as solution:
- Use
grepinstead ofegrepas initially suggested in the forum post, or - Do programmatic validation and yell at the scriptmodule designer that using
+()|chars is not allowed and stop further processing, or - Least preferred: Add a note in the scriptmodule design tips page that only
-is allowed as separator in emulator keywords. Least preferred because there is no programmatic check about whatever is allowed/forbidden as emulator key(word).
This seems to behave differently on my Desktop PC (With Ubuntu 22.10) vs on Raspbian Buster/Bullseye on the RPI. The issue doesn't occur on my Desktop system which is what I was testing on but I can reproduce it on the Raspberry Pi. Thanks.