Re: [help-gengetopt] Re: Gengetopt 2.21 enhancement suggestions

[J. David Bryan]

On 18 Oct 2007 at 9:32, Lorenzo Bettini wrote:

> OK so possible solutions are:
> 
> 1. rotate_arg_90
> 2. rotate_enum_90
> 3. ROTATE_90
> 
> any preferences?

Maybe "rotate_value_90"? 

Actually, the best name to use is what makes the most sense when reading 
the resulting user code:

  if (rotate_arg == rotate_arg_90)

or:

  if (rotate_arg == rotate_enum_90)

or:

  if (rotate_arg == rotate_value_90)

"_value_90" might match up best with the 'values="0","90"...' part in the 
.ggo file.

Is any one clearer than the others?


> Actually the proposal of Gyozo, rotate_<value> might fail (though 
> rarely) for the reasons exposed by David

"rotate_90" is nice and clean, and it might have been my first choice, but 
I think that we want to minimize the number of restrictions caused by 
implementation details -- for example, the restriction that the option name 
must be a valid C identifier (so I cannot have an option of "-0" or "-?" or 
"-dy/dx").  This is not so important when writing new programs with 
gengetopt, but it is an issue when using gengetopt with old programs whose 
options are already established.

I think that it would be surprising to the user that a compilation error 
for a duplicate identifier was caused by his specification of 
'values="orig","copy"' in the .ggo file.


> Please, keep in mind that using types (and enums) for values will
> require lots of changes to the generated code (at least this is my
> first impression) since the type of values will not always be string
> and will require to make the generated check functions more generic (to
> avoid code bloat).

I have not studied the gengetopt internals at all, so I may be entirely 
wrong.  But looking at the generated parser, it appears that changing the 
"rotate" option (e.g.) from string to enum would be:

 1. an addition of an enumeration type declaration:

      enum rotate_enum { rotate_value_0, rotate_value_90,
                         rotate_value_180, rotate_value_270 };

 2. a change in the "rotate_arg" type from "char *" to "rotate_enum".

 3. a change to "_parser_internal" to set the arg value to an enum value:

       args_info->rotate_arg = (rotate_enum) found;

...plus other changes related to #2 (initial value, not freeing string 
values, etc.).  In the generated parser, "rotate_arg" only appears a few 
times.  However, I don't know how involved the underlying changes in the 
parser generator are.


> This is only to say that I might not make it to insert this new
> feature in 2.22....

I don't think that this is a crucial feature, as there are workarounds, but 
it would make the user's code simpler, so it would be nice to have at some 
time in the future.

                                      -- Dave