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

[Eli Zaretskii <eliz@gnu.org>]

> From: Doug Evans <dje@google.com>
> Date: Tue, 9 Apr 2013 09:39:41 -0700
> Cc: tromey@redhat.com,    brobecker@adacore.com,    palves@redhat.com,    gdb-patches@sourceware.org
> 
> Hi.  A few comments inline.

Thanks for the review.

>  > -  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.

It's not critical.  But then neither is this:

  GNU gdb (GDB) 7.5.1
  Copyright (C) 2012 Free Software Foundation, Inc.
  License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
  This is free software: you are free to change and redistribute it.
  There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
  and "show warranty" for details.                       ^^^^^^^^^^^^^^^^^^
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

So I thought we should be consistent here, and offer the same for this
line:

  This GDB was configured as "i686-pc-mingw32".

I'm okay with not adding that, but can someone explain why it is
different from advertising "show copying" and "show warranty"?  Should
we remove those two as well?

>  > +/* 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.

Sorry, I'm not following: what 3 lines?  Can you show how would you
like the code be reformatted?

> Also, is there something driving the choice of indenting 13 spaces in?

It lines up to the right of "configure", below the first option:

     This GDB is configured as follows:
	configure --host=i686-pc-mingw32 --target=i686-pc-mingw32
		  --with-auto-load-dir=$debugdir:$datadir/auto-load
		  --with-auto-load-safe-path=$debugdir:$datadir/auto-load
		  --with-expat
		  --with-gdb-datadir=d:/usr/share/gdb (relocatable)
		  --with-jit-reader-dir=d:/usr/lib/gdb (relocatable)
		  --without-libunwind-ia64
		  --with-lzma
		  --with-python=d:/usr/Python26 (relocatable)
		  --with-separate-debug-dir=d:/usr/lib/debug (relocatable)
		  --with-system-gdbinit=d:/usr/etc/gdbinit (relocatable)
		  --with-zlib

> How about 2 or 4?
> For consistency with print_gdb_help I'd go with 2.

print_gdb_help displays a different information, so I see no need for
consistency here.  But I won't argue if others think like you do.

>  >    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.

Except where they aren't:

     --args             Arguments after executable-file are passed to inferior
     -b BAUDRATE        Set serial port baud rate used for remote debugging.
     --batch            Exit after processing options.
     --batch-silent     As for --batch, but suppress all gdb stdout output.
 >>> --return-child-result
			GDB exit code will be the child's exit code.
     --cd=DIR           Change current directory to DIR.
     --command=FILE, -x Execute GDB commands from FILE.
 >>> --eval-command=COMMAND, -ex
			Execute a single GDB command.
			May be used multiple times and in conjunction
			with --command.
 >>> --init-command=FILE, -ix Like -x but execute it before loading inferior.
 >>> --init-eval-command=COMMAND, -iex Like -ex but before loading inferior.
 >>> --core=COREFILE    Analyze the core dump COREFILE.
 >>> --pid=PID          Attach to running process PID.
     --dbx              DBX compatibility mode.
     --directory=DIR    Search for source files in DIR.
     --epoch            Output information used by epoch emacs-GDB interface.
     --exec=EXECFILE    Use EXECFILE as the executable.
     --fullname         Output information used by emacs-GDB interface.
     --help             Print this message.
     --interpreter=INTERP
			Select a specific interpreter / user interface
     -l TIMEOUT         Set timeout in seconds for remote debugging.
     --nw		     Do not use a window interface.
     --nx               Do not read .gdbinit file.
     --quiet            Do not print version number on startup.
     --readnow          Fully read symbol files on first access.
     --se=FILE          Use FILE as symbol file and executable file.
     --symbols=SYMFILE  Read symbols from SYMFILE.
     --tty=TTY          Use TTY for input/output by the program being debugged.
     --version          Print version information and then exit.
     -w                 Use a window interface.
     --write            Set writing into executable and core files.
     --xdb              XDB compatibility mode.

Again, I'm not really wedded to the place where I inserted the new
option, but please tell me near which 'c' option to put it instead ;-).