[PATCH 2/2] context: rework application options and help output

Aleksander Morgado aleksander at aleksander.es
Mon May 29 11:46:48 UTC 2017


On 18/05/17 12:43, Aleksander Morgado wrote:
> Group together all options that allow configuring the logging output,
> and make them have the same --log-[XXX] prefix.
> 
> Also rework the --help output so that all option groups are printed by
> default (i.e. there is no longer a --help-all option).

I rebased this patch on top of git master (i.e. without patch 1/2) and merged it.

> ---
>  docs/man/ModemManager.8 |  26 +++----
>  src/main.c              |   4 +-
>  src/mm-context.c        | 202 +++++++++++++++++++++++++++++++++++-------------
>  src/mm-context.h        |  14 ++--
>  4 files changed, 168 insertions(+), 78 deletions(-)
> 
> diff --git a/docs/man/ModemManager.8 b/docs/man/ModemManager.8
> index 4f4c4040..d0fd4a51 100644
> --- a/docs/man/ModemManager.8
> +++ b/docs/man/ModemManager.8
> @@ -19,20 +19,6 @@ actual device (Generic AT, vendor-specific AT, QCDM, QMI, MBIM...).
>  ModemManager is a DBus-based system daemon and is not meant to be used directly
>  from the command line.
>  
> -.SH HELP OPTIONS
> -.TP
> -.B \-V, \-\-version
> -Print the ModemManager software version and exit.
> -.TP
> -.B \-h, \-\-help
> -Show application options.
> -.TP
> -.B \-\-help\-all
> -Show application and test options.
> -.TP
> -.B \-\-help\-test
> -Show test options.
> -
>  .SH APPLICATION OPTIONS
>  .TP
>  .B \-\-no\-auto\-scan
> @@ -47,6 +33,14 @@ Runs ModemManager with "DEBUG" log level and without daemonizing. This is useful
>  for debugging, as it directs log output to the controlling terminal in addition to
>  syslog.
>  .TP
> +.B \-V, \-\-version
> +Print the ModemManager software version and exit.
> +.TP
> +.B \-h, \-\-help
> +Show application options.
> +
> +.SH LOGGING OPTIONS
> +.TP
>  .B \-\-log\-level=<level>
>  Sets how much information ModemManager sends to the log destination (usually
>  syslog's "daemon" facility). By default, only informational, warning, and error
> @@ -56,10 +50,10 @@ messages are logged. Given level must be one of "ERR", "WARN", "INFO" or "DEBUG"
>  Specify location of the file where ModemManager will dump its log messages,
>  instead of syslog.
>  .TP
> -.B \-\-timestamps
> +.B \-\-log\-timestamps
>  Include absolute timestamps in the log output.
>  .TP
> -.B \-\-relative-timestamps
> +.B \-\-log\-relative\-timestamps
>  Include timestamps, relative to the start time of the daemon, in the log output.
>  .TP
>  .B \-\-log\-func\-loc
> diff --git a/src/main.c b/src/main.c
> index 06172696..09cb244f 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -141,8 +141,8 @@ main (int argc, char *argv[])
>  
>      if (!mm_log_setup (mm_context_get_log_level (),
>                         mm_context_get_log_file (),
> -                       mm_context_get_timestamps (),
> -                       mm_context_get_relative_timestamps (),
> +                       mm_context_get_log_timestamps (),
> +                       mm_context_get_log_relative_timestamps (),
>                         mm_context_get_log_func_loc (),
>                         &err)) {
>          g_warning ("Failed to set up logging: %s", err->message);
> diff --git a/src/mm-context.c b/src/mm-context.c
> index 0723b0eb..ace91d34 100644
> --- a/src/mm-context.c
> +++ b/src/mm-context.c
> @@ -21,38 +21,48 @@
>  /*****************************************************************************/
>  /* Application context */
>  
> -static gboolean     version_flag;
> -static gboolean     debug;
> -static gboolean     log_func_loc;
> -static const gchar *log_level;
> -static const gchar *log_file;
> -static gboolean     show_ts;
> -static gboolean     rel_ts;
> -
>  #if WITH_UDEV
> -static gboolean no_auto_scan = FALSE;
> +# define NO_AUTO_SCAN_OPTION_FLAG 0
> +# define NO_AUTO_SCAN_DEFAULT     FALSE
>  #else
> -static gboolean no_auto_scan = TRUE;
> +/* Keep the option when udev disabled, just so that the unit test setup can
> + * unconditionally use --no-auto-scan */
> +# define NO_AUTO_SCAN_OPTION_FLAG G_OPTION_FLAG_HIDDEN
> +# define NO_AUTO_SCAN_DEFAULT     TRUE
>  #endif
>  
> +static gboolean     help_flag;
> +static gboolean     version_flag;
> +static gboolean     debug;
> +static gboolean     no_auto_scan = NO_AUTO_SCAN_DEFAULT;
>  static const gchar *initial_kernel_events;
>  
>  static const GOptionEntry entries[] = {
> -    { "version", 'V', 0, G_OPTION_ARG_NONE, &version_flag, "Print version", NULL },
> -    { "debug", 0, 0, G_OPTION_ARG_NONE, &debug, "Run with extended debugging capabilities", NULL },
> -    { "log-func-loc", 0, 0, G_OPTION_ARG_NONE, &log_func_loc, "Enable function location information on log messages", NULL },
> -    { "log-level", 0, 0, G_OPTION_ARG_STRING, &log_level, "Log level: one of ERR, WARN, INFO, DEBUG", "[LEVEL]" },
> -    { "log-file", 0, 0, G_OPTION_ARG_FILENAME, &log_file, "Path to log file", "[PATH]" },
> -    { "timestamps", 0, 0, G_OPTION_ARG_NONE, &show_ts, "Show timestamps in log output", NULL },
> -    { "relative-timestamps", 0, 0, G_OPTION_ARG_NONE, &rel_ts, "Use relative timestamps (from MM start)", NULL },
> -#if WITH_UDEV
> -    { "no-auto-scan", 0, 0, G_OPTION_ARG_NONE, &no_auto_scan, "Don't auto-scan looking for devices", NULL },
> -#else
> -    /* Keep the option when udev disabled, just so that the unit test setup can
> -     * unconditionally use --no-auto-scan */
> -    { "no-auto-scan", 0, G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, &no_auto_scan, NULL, NULL },
> -#endif
> -    { "initial-kernel-events", 0, 0, G_OPTION_ARG_FILENAME, &initial_kernel_events, "Path to initial kernel events file", "[PATH]" },
> +    {
> +        "no-auto-scan", 0, NO_AUTO_SCAN_OPTION_FLAG, G_OPTION_ARG_NONE, &no_auto_scan,
> +        "Don't auto-scan looking for devices",
> +        NULL
> +    },
> +    {
> +        "initial-kernel-events", 0, 0, G_OPTION_ARG_FILENAME, &initial_kernel_events,
> +        "Path to initial kernel events file",
> +        "[PATH]"
> +    },
> +    {
> +        "debug", 0, 0, G_OPTION_ARG_NONE, &debug,
> +        "Run with extended debugging capabilities",
> +        NULL
> +    },
> +    {
> +        "version", 'V', 0, G_OPTION_ARG_NONE, &version_flag,
> +        "Print version",
> +        NULL
> +    },
> +    {
> +        "help", 'h', 0, G_OPTION_ARG_NONE, &help_flag,
> +        "Show help",
> +        NULL
> +    },
>      { NULL }
>  };
>  
> @@ -62,10 +72,68 @@ mm_context_get_debug (void)
>      return debug;
>  }
>  
> +const gchar *
> +mm_context_get_initial_kernel_events (void)
> +{
> +    return initial_kernel_events;
> +}
> +
>  gboolean
> -mm_context_get_log_func_loc (void)
> +mm_context_get_no_auto_scan (void)
>  {
> -    return log_func_loc;
> +    return no_auto_scan;
> +}
> +
> +/*****************************************************************************/
> +/* Log context */
> +
> +static const gchar *log_level;
> +static const gchar *log_file;
> +static gboolean     log_show_ts;
> +static gboolean     log_rel_ts;
> +static gboolean     log_func_loc;
> +
> +static const GOptionEntry log_entries[] = {
> +    {
> +        "log-level", 0, 0, G_OPTION_ARG_STRING, &log_level,
> +        "Log level: one of ERR, WARN, INFO, DEBUG",
> +        "[LEVEL]"
> +    },
> +    {
> +        "log-file", 0, 0, G_OPTION_ARG_FILENAME, &log_file,
> +        "Path to log file",
> +        "[PATH]"
> +    },
> +    {
> +        "log-timestamps", 0, 0, G_OPTION_ARG_NONE, &log_show_ts,
> +        "Show timestamps in log output",
> +        NULL
> +    },
> +    {
> +        "log-relative-timestamps", 0, 0, G_OPTION_ARG_NONE, &log_rel_ts,
> +        "Use relative timestamps (from MM start)",
> +        NULL
> +    },
> +    {
> +        "log-func-loc", 0, 0, G_OPTION_ARG_NONE, &log_func_loc,
> +        "Enable function location information on log messages",
> +        NULL
> +    },
> +    { NULL }
> +};
> +
> +static GOptionGroup *
> +log_get_option_group (void)
> +{
> +    GOptionGroup *group;
> +
> +    group = g_option_group_new ("log",
> +                                "Logging options",
> +                                "Show logging options",
> +                                NULL,
> +                                NULL);
> +    g_option_group_add_entries (group, log_entries);
> +    return group;
>  }
>  
>  const gchar *
> @@ -81,40 +149,46 @@ mm_context_get_log_file (void)
>  }
>  
>  gboolean
> -mm_context_get_timestamps (void)
> +mm_context_get_log_timestamps (void)
>  {
> -    return show_ts;
> +    return log_show_ts;
>  }
>  
>  gboolean
> -mm_context_get_relative_timestamps (void)
> +mm_context_get_log_relative_timestamps (void)
>  {
> -    return rel_ts;
> -}
> -
> -const gchar *
> -mm_context_get_initial_kernel_events (void)
> -{
> -    return initial_kernel_events;
> +    return log_rel_ts;
>  }
>  
>  gboolean
> -mm_context_get_no_auto_scan (void)
> +mm_context_get_log_func_loc (void)
>  {
> -    return no_auto_scan;
> +    return log_func_loc;
>  }
>  
>  /*****************************************************************************/
>  /* Test context */
>  
> -static gboolean test_session;
> -static gboolean test_enable;
> -static gchar *test_plugin_dir;
> +static gboolean  test_session;
> +static gboolean  test_enable;
> +static gchar    *test_plugin_dir;
>  
>  static const GOptionEntry test_entries[] = {
> -    { "test-session", 0, 0, G_OPTION_ARG_NONE, &test_session, "Run in session DBus", NULL },
> -    { "test-enable", 0, 0, G_OPTION_ARG_NONE, &test_enable, "Enable the Test interface in the daemon", NULL },
> -    { "test-plugin-dir", 0, 0, G_OPTION_ARG_FILENAME, &test_plugin_dir, "Path to look for plugins", "[PATH]" },
> +    {
> +        "test-session", 0, 0, G_OPTION_ARG_NONE, &test_session,
> +        "Run in session DBus",
> +        NULL
> +    },
> +    {
> +        "test-enable", 0, 0, G_OPTION_ARG_NONE, &test_enable,
> +        "Enable the Test interface in the daemon",
> +        NULL
> +    },
> +    {
> +        "test-plugin-dir", 0, 0, G_OPTION_ARG_FILENAME, &test_plugin_dir,
> +        "Path to look for plugins",
> +        "[PATH]"
> +    },
>      { NULL }
>  };
>  
> @@ -157,12 +231,22 @@ print_version (void)
>  {
>      g_print ("\n"
>               "ModemManager " MM_DIST_VERSION "\n"
> -             "Copyright (2008 - 2016) The ModemManager authors\n"
> +             "Copyright (C) 2008-2017 The ModemManager authors\n"
>               "License GPLv2+: GNU GPL version 2 or later <http://gnu.org/licenses/gpl-2.0.html>\n"
>               "This is free software: you are free to change and redistribute it.\n"
>               "There is NO WARRANTY, to the extent permitted by law.\n"
>               "\n");
> -    exit (EXIT_SUCCESS);
> +}
> +
> +static void
> +print_help (GOptionContext *context)
> +{
> +    gchar *str;
> +
> +    /* Always print --help-all */
> +    str = g_option_context_get_help (context, FALSE, NULL);
> +    g_print ("%s", str);
> +    g_free (str);
>  }
>  
>  void
> @@ -173,9 +257,11 @@ mm_context_init (gint argc,
>      GOptionContext *ctx;
>  
>      ctx = g_option_context_new (NULL);
> -    g_option_context_set_summary (ctx, "DBus system service to communicate with modems.");
> +    g_option_context_set_summary (ctx, "DBus system service to control mobile broadband modems.");
>      g_option_context_add_main_entries (ctx, entries, NULL);
> +    g_option_context_add_group (ctx, log_get_option_group ());
>      g_option_context_add_group (ctx, test_get_option_group ());
> +    g_option_context_set_help_enabled (ctx, FALSE);
>  
>      if (!g_option_context_parse (ctx, &argc, &argv, &error)) {
>          g_warning ("error: %s", error->message);
> @@ -183,19 +269,27 @@ mm_context_init (gint argc,
>          exit (1);
>      }
>  
> +    if (version_flag) {
> +        print_version ();
> +        g_option_context_free (ctx);
> +        exit (EXIT_SUCCESS);
> +    }
> +
> +    if (help_flag) {
> +        print_help (ctx);
> +        g_option_context_free (ctx);
> +        exit (EXIT_SUCCESS);
> +    }
> +
>      g_option_context_free (ctx);
>  
>      /* Additional setup to be done on debug mode */
>      if (debug) {
>          log_level = "DEBUG";
> -        if (!show_ts && !rel_ts)
> -            show_ts = TRUE;
> +        if (!log_show_ts && !log_rel_ts)
> +            log_show_ts = TRUE;
>      }
>  
> -    /* If just version requested, print and exit */
> -    if (version_flag)
> -        print_version ();
> -
>      /* Initial kernel events processing may only be used if autoscan is disabled */
>  #if WITH_UDEV
>      if (!no_auto_scan && initial_kernel_events) {
> diff --git a/src/mm-context.h b/src/mm-context.h
> index d43bc8c4..e0a0dd07 100644
> --- a/src/mm-context.h
> +++ b/src/mm-context.h
> @@ -23,18 +23,20 @@
>  # define MM_DIST_VERSION VERSION
>  #endif
>  
> -void mm_context_init (gint argc,
> +void mm_context_init (gint    argc,
>                        gchar **argv);
>  
>  gboolean     mm_context_get_debug                 (void);
> -gboolean     mm_context_get_log_func_loc          (void);
> -const gchar *mm_context_get_log_level             (void);
> -const gchar *mm_context_get_log_file              (void);
> -gboolean     mm_context_get_timestamps            (void);
> -gboolean     mm_context_get_relative_timestamps   (void);
>  const gchar *mm_context_get_initial_kernel_events (void);
>  gboolean     mm_context_get_no_auto_scan          (void);
>  
> +/* Logging support */
> +const gchar *mm_context_get_log_level               (void);
> +const gchar *mm_context_get_log_file                (void);
> +gboolean     mm_context_get_log_timestamps          (void);
> +gboolean     mm_context_get_log_relative_timestamps (void);
> +gboolean     mm_context_get_log_func_loc            (void);
> +
>  /* Testing support */
>  gboolean     mm_context_get_test_session    (void);
>  gboolean     mm_context_get_test_enable     (void);
> 


-- 
Aleksander
https://aleksander.es


More information about the ModemManager-devel mailing list