ping?
I can help with code, but at least need the better macro names :-)
On Sun, Nov 13, 2016 at 11:11 AM, Gustavo Sverzut Barbieri
<barbi...@gmail.com> wrote:
>
>
> Em sexta-feira, 11 de novembro de 2016, Jean Guyomarc'h
> <jean.guyoma...@gmail.com> escreveu:
>>
>> Hi all,
>>
>> I was using Ecore_Getopt with the STORE_DEF_* feature and noticed
>> something I find quite strange, I could qualify as disturbing. The
>> default storage thing seems very odd.
>>
>> (1) Values associated to optional parameters are not handled properly
>>
>> If you run the example, with minimal required arguments (e.g.
>> ./src/examples/ecore/ecore_getopt_example string 33 banana zz), you
>> will get the following (snipped):
>>
>> default-string = (null)
>> default-bool = false
>> default-short = 0
>> default-int = 0
>> default-long = 0
>> default-unsigned-short = 0
>> default-unsigned-int = 0
>> default-unsigned-long = 0
>> default-double = 0.000000
>>
>> The default -* things are values that are supposed to be default
>> values. And they are all wrong. This is even specified in the
>> automatic help of the same program (snipped):
>>
>> --default-string[=DEFAULT-STRING]
>> Store a single string.
>> Type: STR. Default: default-string-value.
>> --default-bool[=DEFAULT-BOOL]
>> Store a single boolean.
>> Type: BOOL. Default: true.
>> --default-short[=DEFAULT-SHORT]
>> Store a single short.
>> Type: SHORT. Default: 123.
>> --default-int[=DEFAULT-INT]
>> Store a single integer.
>> Type: INT. Default: 1234.
>> --default-long[=DEFAULT-LONG]
>> Store a single long integer.
>> Type: LONG. Default: 12345.
>> --default-unsigned-short[=DEFAULT-UNSIGNED-SHORT]
>> Store a single unsigned short integer.
>> Type: USHORT. Default: 123.
>> --default-unsigned-int[=DEFAULT-UNSIGNED-INT]
>> Store a single unsigned integer.
>> Type: UINT. Default: 1234.
>> --default-unsigned-long[=DEFAULT-UNSIGNED-LONG]
>> Store a single unsigned long integer.
>> Type: ULONG. Default: 12345.
>> --default-double[=DEFAULT-DOUBLE]
>> Store a single double.
>> Type: DOUBLE. Default: 12.345000.
>>
>> Every single use of the STORE_DEF stuff (in Ecore_Getopt) I've seen
>> was to manually assign the variable containing the default value to
>> the default value.
>>
>
> My bad, I wrote and was bitten by it some times. But the idea and why I
> didn't change it is because it is the default VALUE if the value is not
> specified, but the option switch must be used.
>
> Like --default-double (instead of --default-long=123) used default specified
> in the macro. But if you didn't specify the switch, nothing is assigned and
> you simply use the variable with preset value.
>
>
>
>>
>> To illustrate this, one typically mentions the default value in the
>> Ecore_Getopt descriptor:
>>
>> ECORE_GETOPT_STORE_DEF_SHORT(0, "default-short", "Store a single short",
>> 123),
>>
>> and pass this identical value to the Ecore_Getopt_Value thing:
>>
>> short def_val = 123; /* << Same than above */
>> Ecore_Getopt_Value values[] = {
>> ECORE_GETOPT_VALUE_SHORT(&def_val),
>> };
>>
>>
>> This behaviour seems very wrong to me, because the implementation of
>> the default value is not left to the getopt but up to the developer.
>> And the developer should not do that. It should provide the default
>> value to the getopt (what it already does) and that's it.
>> If it (mistakenly) affects to the 'def_val' variable in our previous
>> example something different than what it has wirtten in the getopt
>> descriptor, the help and the actual default value will mismatch.
>> That's actually the case in the ecore_getopt example.
>>
>> The code tht the developer has to write should just be:
>>
>> short def_val; /* << default value, automatically assigned, nothing to
>> do here */
>> Ecore_Getopt_Value values[] = {
>> ECORE_GETOPT_VALUE_SHORT(&def_val),
>> };
>>
>> And after parsing, boom, "def_val" will contain the value specified by
>> the programmer, or something else if the user has provided the option
>> that allows to change the default value.
>>
>>
>> (2) You can pass an option with phantom parameters to trigger the default
>> values
>>
>> It gets a bit creepy when looking at the code of Ecore_Getopt. The
>> default value affectation is of course already implemented, but is
>> triggered when the option is provided without value... which I think
>> does not make sense, at all.
>> To illustrate, if I slightly alter my call to the example program:
>>
>> ./src/examples/ecore/ecore_getopt_example string 33 banana zz
>> --default-string
>>
>> I get this (snipped):
>>
>> default-string = default-string-value
>> default-bool = false
>> default-short = 0
>> default-int = 0
>> default-long = 0
>> default-unsigned-short = 0
>> default-unsigned-int = 0
>> default-unsigned-long = 0
>> default-double = 0.000000
>>
>> So now, I have my default value, as specified in the help. But to do
>> so, I actually needed to provide the name of the option without any
>> parameter!? It feels very disturbing. I think such use of the option
>> without any parameter should have errored.
>>
>> Imagine a network application that connects to port 8080 by default.
>> This port can be changed with the --port option that takes exactly one
>> argument (the port number). You could call your program like this:
>>
>> ./myprogram --port 993
>>
>> The value will be set to 993, seems legit. But then, I can do this:
>>
>> ./myprogram --port
>>
>> And that's okay too... The port value would be set to 8080 (the default).
>> What!?
>> To me, that's just not right.
>
>
> You can use required arguments, not the optional. This is the case when you
> do use "_STORE_INT()". See it will render the string differently and you
> must pass the option.
>
> But I see why you wanted to use the "_DEF", because you wanted it to list
> the option's default.
>
> That said there is a clear problem with usability due names. _DEF() should
> better reflect its "a optional argument switch with its default value
> specified". It's not the default value if the switch it's not used. So name
> suggestions are needed to deprecate these macros and use new one instead.
>
> Secondly, I do understand and agree with the usage of default field as both
> documentation and always set if the parameter wasn't found. Which lead to 2
> new items:
>
> - new enum field in Ecore_Getopt_Desc_Arg_Requirement that would be OR'ed,
> it would set the default value even if the switch is not provided. An early
> loop checks for this flag and set initial value for actions that make sense
> (i.e. Store type, choice...)
>
> - if argument is required, use the default value for documentation. Used
> with above extra enum, works "as expected"
>
>
> Your patch is almost good, take the extra enum and handle other actions,
> like store true/false, choice...
>
> And we need better macro names, I'm bad at those as you could see :-D
>
>
>
>
>
>
>>
>>
>>
>> So, here were my grieves. But I'm a man of action, and also proposes
>> solutions :)
>>
>> (1) I have a patch ("HEAD" of branch devs/jayji/ecore-getopt) that I
>> will land on master, unless my proposition is juged rubbish.
>>
>> (2) Changing the behaviour of the phantom parameter needs discussion,
>> as this would break existing scripts that may have relied on this. I
>> feel like they shouldn't have though (and probably have not, as this
>> is very weird). So I don't really know what we should do there (I
>> would nuke that :D).
>>
>> Any remarks/thoughts are welcome.
>>
>> Best regards,
>>
>>
>> ------------------------------------------------------------------------------
>> Developer Access Program for Intel Xeon Phi Processors
>> Access to Intel Xeon Phi processor-based developer platforms.
>> With one year of Intel Parallel Studio XE.
>> Training and support from Colfax.
>> Order your platform today. http://sdm.link/xeonphi
>> _______________________________________________
>> enlightenment-devel mailing list
>> enlightenment-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>
>
>
> --
> Gustavo Sverzut Barbieri
> --------------------------------------
> Mobile: +55 (16) 99354-9890
>
>
--
Gustavo Sverzut Barbieri
--------------------------------------
Mobile: +55 (16) 99354-9890
------------------------------------------------------------------------------
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
