feature request for inifuncs.sh

BuZz

I have some plans to change iniGet to return actual values in the future rather than use a variable, so this may be added then - but it will be after 4.0

meleu

I'm not sure if it would brake any existing code, because it returns 1 if the file to get the value doesn't exist. As we can see here:
https://github.com/RetroPie/RetroPie-Setup/blob/master/scriptmodules/inifuncs.sh#L102

BuZz

Perhaps not (but it would require a thorough review), but I it may clash with my future iniGet changes. I am not making any more changes to iniGet until after 4.0 is out - and none of the current code needs a return value currently.

meleu

@BuZz said in feature request for inifuncs.sh:

I have some plans to change iniGet to return actual values in the future rather than use a variable

How can you do it if the value is a non-integer value?!

BuZz

you echo it (and use command substitution). has a performance hit, so I need to do some tests.

meleu

@BuZz
Oh... It's not an actual return, but OK. I already do this way in some of my scripts.

meleu

Just to clarify/register why I asked for it, and maybe make precocious feature request for the future version of inifuncs.sh.

Currently there is no way to differentiate if 1) the key has a blank value; 2) the key exists and is commented; 3) the key doesn't exist. (OK, maybe there's no reason to differentiate a commented key from a non-existent one).

Example:

[prompt]$ . ~/RetroPie-Setup/scriptmodules/inifuncs.sh 
[prompt]$ iniConfig ' = ' ''
[prompt]$ cat test.cfg 
var1 = value1
var2 =
# var3 = value3

[prompt]$ iniGet var1 test.cfg 
[prompt]$ echo "\"$ini_value\""
"value1"
[prompt]$ iniGet var2 test.cfg 
[prompt]$ echo "\"$ini_value\""
""
[prompt]$ iniGet var3 test.cfg 
[prompt]$ echo "\"$ini_value\""
""
[prompt]$ iniGet non_existent_var4 test.cfg 
[prompt]$ echo "\"$ini_value\""
""

meleu

Another precocious feature request...

I think that would be useful an iniConfigFile to setup only the file to get the values and don't change the delimiter and the quote symbol.

BuZz

Why does it matter if the key is commented out - that is the same as it not existing (It's a user comment). I don't see the need for that.

Not sure why you need the additional functionality - you can switch file by calling iniConfig again (you can always save your delimiter in a variable), or add parameter to iniGet or if you really need adjust the var it uses for the file.

meleu

@BuZz

I have some plans to change iniGet to return actual values in the future rather than use a variable

It made me worried! I'm enjoying to use inifuncs here, but maybe I'll need to adapt what I'm doing to fit your changes...

Why does it matter if the key is commented out (...). I don't see the need for that.

Actually, neither do I.

Not sure why you need the additional functionality - you can switch file by calling iniConfig again (...)

OK, ok... I have to agree that it would be just a cosmetic and would bloat the inifuncs. No need for that.

But the non-zero for an empty ini_value would be useful and won't bloat the inifuncs.

Maybe adding this line at the end of iniGet (the 2 is only to differentiate from the "no config file" error, can be any non-zero):

[[ -z "$ini_value" ]] && return 2

At last: Please, let me know if you care about this kind of discussion/feedback. Sometimes I feel like being annoying...

BuZz

I still can't see a use case for that - at least in the RetroPie code - if we needed to check that it would already be included. I'm not sure this is a good use of my time as none of this seems relevant to RetroPie-Setup. - you are asking for changes with no real world example as to why it is needed. Also when I say I don't want to do something you seem to question my answer everytime (eg with the recent pull request - where in fact your PR was not functional and hadn't been tested).

I'm short on time at the moment, so prefer to spend my free time working on stuff which directly affects RetroPie rather than user requests for changes for other things (I am not even sure why you want these changes).

meleu

OK, sorry. Let's forget it. Those runcommand-onstart stuff already helped me a lot!