[packagekit] [PATCH 0/2] gnome-packagekit option parsing updates

[Richard Hughes]

On Wed, 2007-10-10 at 23:15 -0400, James Bowes wrote:
> diff --git a/src/pk-application-main.c b/src/pk-application-main.c
> index f2224c7..c55d553 100644
> --- a/src/pk-application-main.c
> +++ b/src/pk-application-main.c
> @@ -66,13 +66,16 @@ main (int argc, char *argv[])
>  {
>         GMainLoop *loop;
>         gboolean verbose = FALSE;
> +       gboolean program_version = FALSE;
>         PkApplication *application = NULL;
>         GOptionContext *context;
>  
>         const GOptionEntry options[] = {
>                 { "verbose", '\0', 0, G_OPTION_ARG_NONE, &verbose,
>                   "Show extra debugging information", NULL },
> -               { NULL}
> +               { "version", '\0', 0, G_OPTION_ARG_NONE,
> &program_version,
> +                 "Show the program version and exit", NULL },
> +       { NULL}

I think you need to use two tabs before {NULL}, not one.

> +       if (program_version) {
> +               g_print (VERSION "\n");
> +               return 0;
> +       }

Can you please do "if (program_version == TRUE) {" - and then we know
that it's boolean, and it's very obvious what it's doing when it's
reviewed. It's very easy to miss a "!". Could you fix up the patch to do
this for all the new code please. Thanks.

> diff --git a/src/pk-install-file.c b/src/pk-install-file.c
> index 6bc0475..9790db0 100644
> --- a/src/pk-install-file.c
> +++ b/src/pk-install-file.c
> @@ -53,6 +53,16 @@ main (int argc, char *argv[])
>  {
>         GOptionContext *context;
>         gboolean ret;
> +       gboolean verbose = FALSE;
> +       gboolean program_version = FALSE;
> +
> +       const GOptionEntry options[] = {
> +               { "verbose", '\0', 0, G_OPTION_ARG_NONE, &verbose,
> +                 "Show extra debugging information", NULL },
> +               { "version", '\0', 0, G_OPTION_ARG_NONE,
> &program_version,
> +                 "Show the program version and exit", NULL },
> +               { NULL}
> +       };

Are you sure this works? pk-install-file takes the first argument
argument as the thing to install (e.g.
(pk-install-file /home/ania/Desktop/tomtom-1.23.rpm") and so I think
this is going to break if we do "pk-install-file
--verbose /home/ania/Desktop/tomtom-1.23.rpm" - you might need to fix my
braindead argc/argv stuff lower down.

> diff --git a/src/pk-install-package.c b/src/pk-install-package.c
> index ab71d28..12f67ee 100644
> --- a/src/pk-install-package.c
> +++ b/src/pk-install-package.c
> @@ -100,6 +100,16 @@ main (int argc, char *argv[])
>  {
>         GOptionContext *context;
>         gboolean ret;
> +       gboolean verbose = FALSE;
> +       gboolean program_version = FALSE;
> +
> +       const GOptionEntry options[] = {
> +               { "verbose", '\0', 0, G_OPTION_ARG_NONE, &verbose,
> +                 "Show extra debugging information", NULL },
> +               { "version", '\0', 0, G_OPTION_ARG_NONE,
> &program_version,
> +                 "Show the program version and exit", NULL },
> +               { NULL}
> +       };

Same problem with pk-install-package...

Other than that, the patches look good. Thanks,

Richard.