[PATCH 9/9] add way to detect if an option was set or not

[Ray Strode]

Hi,

On Mon, Feb 23, 2009 at 3:35 PM,  <william.jon.mccann at gmail.com> wrote:
> From: William Jon McCann <jmccann at redhat.com>
>
May be add a description like:

With integer options there's no obvious value to mean "unset" like there is for
string types (and to a lesser extent boolean types), so this commit
adds a mechanism
to do that.

> --- a/src/client/plymouth.c
> +++ b/src/client/plymouth.c
> @@ -699,20 +699,20 @@ main (int    argc,
>     }
>
>   ply_command_parser_get_options (state.command_parser,
> -                                  "help", &should_help,
> -                                  "debug", &should_be_verbose,
> -                                  "newroot", &chroot_dir,
> -                                  "quit", &should_quit,
> -                                  "ping", &should_ping,
> -                                  "sysinit", &should_sysinit,
> -                                  "show-splash", &should_show_splash,
> -                                  "hide-splash", &should_hide_splash,
> -                                  "message", &message,
> -                                  "ask-for-password", &should_ask_for_password,
> -                                  "ignore-keystroke", &ignore_keystroke,
> -                                  "update", &status,
> -                                  "wait", &should_wait,
> -                                  "details", &report_error,
> +                                  "help", &should_help, NULL,
> +                                  "debug", &should_be_verbose, NULL,
> +                                  "newroot", &chroot_dir, NULL,
> +                                  "quit", &should_quit, NULL,
> +                                  "ping", &should_ping, NULL,
> +                                  "sysinit", &should_sysinit, NULL,
> +                                  "show-splash", &should_show_splash, NULL,
> +                                  "hide-splash", &should_hide_splash, NULL,
> +                                  "message", &message, NULL,
> +                                  "ask-for-password", &should_ask_for_password, NULL,
> +                                  "ignore-keystroke", &ignore_keystroke, NULL,
> +                                  "update", &status, NULL,
> +                                  "wait", &should_wait, NULL,
> +                                  "details", &report_error, NULL,
>                                   NULL);
I'm not a huge fan of this interface.  It makes a long function call
even longer, and the "is it set?" argument will rarely be used.

I'd rather separate:

ply_command_parser_is_option_set and ply_command_parser_is_command_option_set

functions, or alternatively:

ply_command_parser_get_option (parser, "name", &value, &is_set);
and
ply_command_parser_get_command_option (parser, "command", "name",
&value, &is_set);

that only takes one name and has the new argument.  Then you could
just do the atypical cases separately.
[...]
> --- a/src/libply/ply-command-parser.c
> +++ b/src/libply/ply-command-parser.c
> @@ -46,7 +46,7 @@ typedef struct
>   char *name;
>   char *description;
>   ply_command_option_type_t type;
> -
> +  bool was_set;
>   ply_command_option_result_t result;
>  } ply_command_option_t;
no bool in structs please. Instead:

uint32_t was_set : 1;

It packs better that way.

--Ray