Re: [PATCH] Display configuration details in --help

[Doug Evans <dje at google dot com>]

Eli Zaretskii writes:
 > > Date: Thu, 21 Mar 2013 23:16:50 +0200
 > > From: Eli Zaretskii <eliz@gnu.org>
 > > Cc: brobecker@adacore.com, palves@redhat.com, dje@google.com, gdb-patches@sourceware.org
 > > 
 > > > From: Tom Tromey <tromey@redhat.com>
 > > > Cc: Eli Zaretskii <eliz@gnu.org>, palves@redhat.com, dje@google.com,
 > > >         gdb-patches@sourceware.org
 > > > Date: Thu, 21 Mar 2013 14:58:03 -0600
 > > > 
 > > > For the new information, "show configuration" might also be nice to have.
 > > > It would not be much more code and would let people see the info for the
 > > > gdb they are currently using.
 > > 
 > > OK, will do.
 > 
 > Respinning, here's v2 of that patch, which accommodates all the
 > comments till now.  It also includes documentation; please someone
 > review that, as I'm not objective in this case ;-).
 > 
 > OK to commit?

Hi.  A few comments inline.

 > 2013-04-09  Eli Zaretskii  <eliz@gnu.org>
 > 
 > 	* top.c (print_gdb_configuration): New function, displays the
 > 	details about GDB configure-time parameters.
 > 	(print_gdb_version): Mention "show configuration".
 > 
 > 	* cli/cli-cmds.c (show_configuration): New function.
 > 	(_initialize_cli_cmds): Add the "show configuration" command.
 > 
 > 	* main.c (captured_main) <print_configuration>: New static var.
 > 	<long_options>: Use it.
 > 	If --configuration was given, call print_gdb_configuration.
 > 
 > 
 > --- gdb/cli/cli-cmds.c~0	2013-03-12 19:39:44.000000000 +0200
 > +++ gdb/cli/cli-cmds.c	2013-04-09 08:02:46.088591300 +0300
 > @@ -314,6 +314,12 @@ show_version (char *args, int from_tty)
 >    printf_filtered ("\n");
 >  }
 >  
 > +static void
 > +show_configuration (char *args, int from_tty)
 > +{
 > +  print_gdb_configuration (gdb_stdout);
 > +}
 > +
 >  /* Handle the quit command.  */
 >  
 >  void
 > @@ -1756,6 +1762,9 @@ the previous command number shown."),
 >    add_cmd ("version", no_set_class, show_version,
 >  	   _("Show what version of GDB this is."), &showlist);
 >  
 > +  add_cmd ("configuration", no_set_class, show_configuration,
 > +	   _("Show how GDB was configured at build time."), &showlist);
 > +
 >    /* If target is open when baud changes, it doesn't take effect until
 >       the next open (I think, not sure).  */
 >    add_setshow_zinteger_cmd ("remotebaud", no_class, &baud_rate, _("\
 > 
 > 
 > 
 > --- gdb/top.c~0	2013-01-25 16:17:10.000000000 +0200
 > +++ gdb/top.c	2013-04-09 08:02:27.508872200 +0300
 > @@ -1146,15 +1146,101 @@ and \"show warranty\" for details.\n");
 >      {
 >        fprintf_filtered (stream, "%s", host_name);
 >      }
 > -  fprintf_filtered (stream, "\".");
 > +  fprintf_filtered (stream, "\".\n\
 > +Type \"show configuration\" for configuration details.");
 >  
 >    if (REPORT_BUGS_TO[0])
 >      {
 > -      fprintf_filtered (stream, 
 > +      fprintf_filtered (stream,
 >  			_("\nFor bug reporting instructions, please see:\n"));
 >        fprintf_filtered (stream, "%s.", REPORT_BUGS_TO);
 >      }
 >  }
 
I have a bit of a phobia of adding more lines to gdb's initial output.
It's too long already IMO.  [I realize there's -q.]
I'm not objecting per se.  Just wondering how critical this is.

 > +/* Print the details of GDB build-time configuration.  */
 > +void
 > +print_gdb_configuration (struct ui_file *stream)
 > +{
 > +  fprintf_filtered (stream, _("\
 > +This GDB was configured as follows:\n\
 > +   configure --host=%s --target=%s\n\
 > +"), host_name, target_name);
 > +  fprintf_filtered (stream, _("\
 > +             --with-auto-load-dir=%s\n\
 > +             --with-auto-load-safe-path=%s\n\
 > +"), AUTO_LOAD_DIR, AUTO_LOAD_SAFE_PATH);
 > +#if HAVE_LIBEXPAT
 > +  fprintf_filtered (stream, _("\
 > +             --with-expat\n\
 > +"));
 > +#else
 > +  fprintf_filtered (stream, _("\
 > +             --without-expat\n\
 > +"));
 > +#endif

If we've already discussed this, please ignore, but ... :-)
I'd prefer one line per fprintf instead of three.
[I realize there are some cases where one line won't do,
and this style makes them all consistent.  I still prefer
one line for the common case.]
If others don't object to it, then it's not that important.

Also, is there something driving the choice of indenting 13 spaces in?
How about 2 or 4?
For consistency with print_gdb_help I'd go with 2.

 > +  if (GDB_DATADIR[0])
 > +    fprintf_filtered (stream, _("\
 > +             --with-gdb-datadir=%s%s\n\
 > +"), GDB_DATADIR, GDB_DATADIR_RELOCATABLE ? " (relocatable)" : "");
 > +#ifdef ICONV_BIN
 > +  fprintf_filtered (stream, _("\
 > +             --with-iconv-bin=%s%s\n\
 > +"), ICONV_BIN, ICONV_BIN_RELOCATABLE ? " (relocatable)" : "");
 > +#endif
 > +  if (JIT_READER_DIR[0])
 > +    fprintf_filtered (stream, _("\
 > +             --with-jit-reader-dir=%s%s\n\
 > +"), JIT_READER_DIR, JIT_READER_DIR_RELOCATABLE ? " (relocatable)" : "");
 > +#if HAVE_LIBUNWIND_IA64_H
 > +  fprintf_filtered (stream, _("\
 > +             --with-libunwind-ia64\n\
 > +"));
 > +#else
 > +  fprintf_filtered (stream, _("\
 > +             --without-libunwind-ia64\n\
 > +"));
 > +#endif
 > +#if HAVE_LIBLZMA
 > +  fprintf_filtered (stream, _("\
 > +             --with-lzma\n\
 > +"));
 > +#else
 > +  fprintf_filtered (stream, _("\
 > +             --without-lzma\n\
 > +"));
 > +#endif
 > +#ifdef WITH_PYTHON_PATH
 > +  fprintf_filtered (stream, _("\
 > +             --with-python=%s%s\n\
 > +"), WITH_PYTHON_PATH, PYTHON_PATH_RELOCATABLE ? " (relocatable)" : "");
 > +#endif
 > +#ifdef RELOC_SRCDIR
 > +  fprintf_filtered (stream, _("\
 > +             --with-relocated-sources=%s\n\
 > +"), RELOC_SRCDIR);
 > +#endif
 > +  if (DEBUGDIR[0])
 > +    fprintf_filtered (stream, _("\
 > +             --with-separate-debug-dir=%s%s\n\
 > +"), DEBUGDIR, DEBUGDIR_RELOCATABLE ? " (relocatable)" : "");
 > +  if (TARGET_SYSTEM_ROOT[0])
 > +    fprintf_filtered (stream, _("\
 > +             --with-sysroot=%s%s\n\
 > +"), TARGET_SYSTEM_ROOT, TARGET_SYSTEM_ROOT_RELOCATABLE ? " (relocatable)" : "");
 > +  if (SYSTEM_GDBINIT[0])
 > +    fprintf_filtered (stream, _("\
 > +             --with-system-gdbinit=%s%s\n\
 > +"), SYSTEM_GDBINIT, SYSTEM_GDBINIT_RELOCATABLE ? " (relocatable)" : "");
 > +#if HAVE_ZLIB_H
 > +  fprintf_filtered (stream, _("\
 > +             --with-zlib\n\
 > +"));
 > +#else
 > +  fprintf_filtered (stream, _("\
 > +             --without-zlib\n\
 > +"));
 > +#endif
 > +}
 >  
 >  
 >  /* The current top level prompt, settable with "set prompt", and/or
 > 
 > 
 > --- gdb/main.c~4	2013-04-09 08:40:03.404632700 +0300
 > +++ gdb/main.c	2013-04-09 08:08:00.461806500 +0300
 > @@ -322,6 +322,7 @@ captured_main (void *data)
 >       initializer.  */
 >    static int print_help;
 >    static int print_version;
 > +  static int print_configuration;
 >  
 >    /* Pointers to all arguments of --command option.  */
 >    VEC (cmdarg_s) *cmdarg_vec = NULL;
 > @@ -484,6 +485,7 @@ captured_main (void *data)
 >        {"command", required_argument, 0, 'x'},
 >        {"eval-command", required_argument, 0, 'X'},
 >        {"version", no_argument, &print_version, 1},
 > +      {"configuration", no_argument, &print_configuration, 1},
 >        {"x", required_argument, 0, 'x'},
 >        {"ex", required_argument, 0, 'X'},
 >        {"init-command", required_argument, 0, OPT_IX},
 > @@ -727,8 +729,9 @@ captured_main (void *data)
 >  	  }
 >        }
 >  
 > -    /* If --help or --version, disable window interface.  */
 > -    if (print_help || print_version)
 > +    /* If --help or --version or --configuration, disable window
 > +       interface.  */
 > +    if (print_help || print_version || print_configuration)
 >        {
 >  	use_windows = 0;
 >        }
 > @@ -819,6 +822,14 @@ captured_main (void *data)
 >        exit (0);
 >      }
 >  
 > +  if (print_configuration)
 > +    {
 > +      print_gdb_configuration (gdb_stdout);
 > +      wrap_here ("");
 > +      printf_filtered ("\n");
 > +      exit (0);
 > +    }
 > +
 >    /* FIXME: cagney/2003-02-03: The big hack (part 1 of 2) that lets
 >       GDB retain the old MI1 interpreter startup behavior.  Output the
 >       copyright message before the interpreter is installed.  That way
 > @@ -1130,6 +1141,7 @@ Options:\n\n\
 >  #endif
 >    fputs_unfiltered (_("\
 >    --version          Print version information and then exit.\n\
 > +  --configuration    Print details about GDB configuration and then exit.\n\
 >    -w                 Use a window interface.\n\
 >    --write            Set writing into executable and core files.\n\
 >    --xdb              XDB compatibility mode.\n\

The options here are (mostly) sorted alphabetically.