Re: [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sun, 4 Dec 2011 18:52:19 +0300
> From: iam ahal <hal9000ed2k@gmail.com>
> Cc: Doug Evans <dje@google.com>, gdb-patches@sourceware.org, eliz@gnu.org, 
> 	pmuldoon@redhat.com, brobecker@adacore.com, pedro@codesourcery.com, 
> 	drow@false.org, jan.kratochvil@redhat.com
> 
> On Wed, Nov 2, 2011 at 11:05 PM, Tom Tromey <tromey@redhat.com> wrote:
> > I don't recall whether this has had a doc review yet.
> 
> You have not reviewed a text for this feature in the documentation.

Below.

> > I think the patch should also include a NEWS entry.
> 
> I don't have an idea how to fill out a NEWS entry. It has descriptions
> only for release versions.

A NEWS entry should announce the new option, see the section "New
options" for a few examples.

> --- gdb-7.3.1-orig/gdb/doc/gdb.texinfo	2011-09-04 21:10:37.000000000 +0400
> +++ gdb-7.3.1/gdb/doc/gdb.texinfo	2011-12-04 18:16:03.182293844 +0400
> @@ -6052,6 +6052,19 @@ unlimited.
>  
>  @item show backtrace limit
>  Display the current limit on backtrace levels.
> +
> +@item set backtrace filename-display
> +@itemx set backtrace filename-display full
> +Display a full filename.  This is the default.
> +
> +@item set backtrace filename-display basename
> +Display only basename of a filename.
> +
> +@item set backtrace filename-display without-compilation-directory
> +Display a filename without the compilation directory part.
> +
> +@item show backtrace filename-display
> +Display the current way to display a filename.
>  @end table

This has several problems.  First of all, it is not a good idea to add
these entries to this particular @table, because the text preceding it
talks about a completely unrelated situation:

  If you need to examine the startup code, or limit the number of levels
  in a backtrace, you can change this behavior:

So I think this needs a separate preamble, 1 or 2 sentences explaining
the situation where this option is useful, and a separate @table.

Second, this:

> +@item set backtrace filename-display
> +@itemx set backtrace filename-display full
> +Display a full filename.  This is the default.

is too general: this option controls only the display of file names in
backtraces, right?  So the description should say "Display ... in
backtraces."

Third, how come "full" is the default? are we changing the present
behavior whereby only the basename is displayed in backtraces?  Or am
I missing something?

Fourth, I couldn't understand clearly what is the effect of
without-compilation-directory value.  I think we should explain it in
a more clear way.

Fifth, "Display the current way to display..." should be reworded to
avoid using "display" twice in a row.

Finally, this option should have a @cindex entry.

I can fix this text, if you want, once I understand the situation with
the default value of this option, and once I understand the semantics
of without-compilation-directory.

> +  add_setshow_enum_cmd ("filename-display", class_obscure,
> +			filename_display_kind_names,
> +			&filename_display_string, _("\
> +Set a way how to display filename."), _("\
> +Show a way how to display filename."), _("\

Again, this should say "in backtraces".

And please remove the "a way" part, it is not needed in both of these
sentences.  Just "Set how to display ..." is good enough.

Thanks.