[PATCH 2/3] deprecate "skip enable/etc.", doc string cleanup

[PATCH 2/3] deprecate "skip enable/etc.", doc string cleanup

[Doug Evans]

This patch deprecates "skip enable", "skip disable", "skip delete"
in favor of "enable skip", "disable skip", "delete skip".
And it uses the doc string macros of 1/3.

2016-04-17  Doug Evans  <dje@google.com>

	* NEWS: Announce new skip commands, and deprecated ones.
	* skip.c (_initialize_step_skip): Deprecate "skip enable",
	"skip disable", "skip delete" in favor of "enable skip",
	"disable skip", "delete skip". Simplify doc strings.

	doc/
	* gdb.texinfo (Skipping Over Functions and Files): Add docs for
	"enable skip", "disable skip", "delete skip". Mark "skip enable",
	"skip disable", "skip delete" as deprecated.

diff --git a/gdb/NEWS b/gdb/NEWS
index 39d99b7..2042027 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -34,6 +34,17 @@ skip -rfunction regular-expression
    glob-style file names and regular expressions for function names.
    Additionally, a file spec and a function spec may now be combined.

+enable skip [NUMBERS AND/OR RANGES]
+disable skip [NUMBERS AND/OR RANGES]
+delete skip [NUMBERS AND/OR RANGES]
+  These commands replace the now deprecated "skip enable", "skip disable",
+  and "skip delete" commands.
+
+skip enable [NUMBERS AND/OR RANGES]
+skip disable [NUMBERS AND/OR RANGES]
+skip delete [NUMBERS AND/OR RANGES]
+  These commands are now deprecated.
+
  maint info line-table REGEXP
    Display the contents of GDB's internal line table data struture.

diff --git a/gdb/skip.c b/gdb/skip.c
index 15681cc..12725d6 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -713,33 +713,31 @@ If no function name is given, skip the current  
function."),
  	       &skiplist);
    set_cmd_completer (c, location_completer);

-  add_cmd ("enable", class_breakpoint, skip_enable_command, _("\
-Enable skip entries.  You can specify numbers (e.g. \"skip enable 1 3\"), \
-ranges (e.g. \"skip enable 4-8\"), or both (e.g. \"skip enable 1 3  
4-8\").\n\n\
-If you don't specify any numbers or ranges, we'll enable all skip  
entries.\n\n\
-Usage: skip enable [NUMBERS AND/OR RANGES]"),
-	   &skiplist);
-
-  add_cmd ("disable", class_breakpoint, skip_disable_command, _("\
-Disable skip entries.  You can specify numbers (e.g. \"skip disable 1  
3\"), \
-ranges (e.g. \"skip disable 4-8\"), or both (e.g. \"skip disable 1 3  
4-8\").\n\n\
-If you don't specify any numbers or ranges, we'll disable all skip  
entries.\n\n\
-Usage: skip disable [NUMBERS AND/OR RANGES]"),
-	   &skiplist);
-
-  add_cmd ("delete", class_breakpoint, skip_delete_command, _("\
-Delete skip entries.  You can specify numbers (e.g. \"skip delete 1 3\"), \
-ranges (e.g. \"skip delete 4-8\"), or both (e.g. \"skip delete 1 3  
4-8\").\n\n\
-If you don't specify any numbers or ranges, we'll delete all skip  
entries.\n\n\
-Usage: skip delete [NUMBERS AND/OR RANGES]"),
-           &skiplist);
+  add_cmd ("skip", class_breakpoint, skip_enable_command,
+	   _(ENABLE_DOC_STRING ("skip", "enable skip")),
+	   &enablelist);
+  add_cmd ("skip", class_breakpoint, skip_disable_command,
+	   _(DISABLE_DOC_STRING ("skip", "disable skip")),
+	   &disablelist);
+  add_cmd ("skip", class_breakpoint, skip_delete_command,
+	   _(DELETE_DOC_STRING ("skip", "delete skip")),
+           &deletelist);
+
+  /* "skip enable|disable|delete" are deprecated.  */
+  c = add_cmd ("enable", class_breakpoint, skip_enable_command,
+	       _(ENABLE_DOC_STRING ("skip", "skip enable")),
+	       &skiplist);
+  deprecate_cmd (c, "enable skip");
+  c = add_cmd ("disable", class_breakpoint, skip_disable_command,
+	       _(DISABLE_DOC_STRING ("skip", "skip disable")),
+	       &skiplist);
+  deprecate_cmd (c, "disable skip");
+  c = add_cmd ("delete", class_breakpoint, skip_delete_command,
+	       _(DELETE_DOC_STRING ("skip", "skip delete")),
+	       &skiplist);
+  deprecate_cmd (c, "delete skip");

    add_info ("skip", skip_info, _("\
-Display the status of skips.  You can specify numbers (e.g. \"skip info 1  
3\"), \
-ranges (e.g. \"skip info 4-8\"), or both (e.g. \"skip info 1 3 4-8\").\n\n\
-If you don't specify any numbers or ranges, we'll show all skips.\n\n\
-Usage: skip info [NUMBERS AND/OR RANGES]\n\
-The \"Type\" column indicates one of:\n\
-\tfile        - ignored file\n\
-\tfunction    - ignored function"));
+Display the status of skips.\n"
+EDDI_USAGE_DOC_STRING_WITH_ALL ("skip", "info skip", "displayed")));
  }
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7abd55e..a4b14c9 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5679,6 +5679,27 @@ The name or regular expression of the function to  
skip.
  If no function is specified this is @samp{<none>}.
  @end table

+@kindex delete skip
+@item delete skip @r{[}@var{range}@r{]}
+Delete the specified skip(s).  If @var{range} is not specified, delete all
+skips.
+
+@kindex enable skip
+@item enable skip @r{[}@var{range}@r{]}
+Enable the specified skip(s).  If @var{range} is not specified, enable all
+skips.
+
+@kindex disable skip
+@item disable skip @r{[}@var{range}@r{]}
+Disable the specified skip(s).  If @var{range} is not specified, disable  
all
+skips.
+
+@end table
+
+The following commands are deprecated.
+
+@table @code
+
  @kindex skip delete
  @item skip delete @r{[}@var{range}@r{]}
  Delete the specified skip(s).  If @var{range} is not specified, delete all

Re: [PATCH 2/3] deprecate "skip enable/etc.", doc string cleanup

[Eli Zaretskii]

> Date: Mon, 18 Apr 2016 15:47:21 +0000
> From: Doug Evans <dje@google.com>
> 
> This patch deprecates "skip enable", "skip disable", "skip delete"
> in favor of "enable skip", "disable skip", "delete skip".
> And it uses the doc string macros of 1/3.

OK, but I still don't like those macros.  They make it harder to
understand how the doc string will read.

Re: [PATCH 2/3] deprecate "skip enable/etc.", doc string cleanup

[Doug Evans]

On Mon, Apr 18, 2016 at 11:57 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Mon, 18 Apr 2016 15:47:21 +0000
>> From: Doug Evans <dje@google.com>
>>
>> This patch deprecates "skip enable", "skip disable", "skip delete"
>> in favor of "enable skip", "disable skip", "delete skip".
>> And it uses the doc string macros of 1/3.
>
> OK, but I still don't like those macros.  They make it harder to
> understand how the doc string will read.

I'm just going with the flow here.
The explicit locations patch did this (perhaps not
as parameterized as I've done here, but I am
improving the docs here - before it was just
"Give breakpoint numbers." and now it's
something vastly more useful).

The alternative is to describe such things (in this case
how to specify breakpoint/etc. numbers) in one place
and then add a "See mumble." to all the other places.
I actually did that, and then remembered the explicit
locations discussion. And then spent a lot of time
waffling "Crap, what will upstream accept?"
and in the decided to stick with existing practice.

I certainly don't want to replicate all that text again
and again (hence the macros).

Re: [PATCH 2/3] deprecate "skip enable/etc.", doc string cleanup

[Eli Zaretskii]

> From: Doug Evans <dje@google.com>
> Date: Mon, 18 Apr 2016 12:42:04 -0700
> Cc: gdb-patches <gdb-patches@sourceware.org>
> 
> I certainly don't want to replicate all that text again
> and again (hence the macros).

I understand, but put yourself in the position of someone who needs to
review a patch, and sees something like this:

  +Display the status of skips.\n"
  +EDDI_USAGE_DOC_STRING_WITH_ALL ("skip", "info skip", "displayed")));

How does that someone know if the resulting text is good English and
will produce a clear help text, or needs to be fixed in some way?

Re: [PATCH 2/3] deprecate "skip enable/etc.", doc string cleanup

[Doug Evans]

On Mon, Apr 18, 2016 at 12:53 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Doug Evans <dje@google.com>
>> Date: Mon, 18 Apr 2016 12:42:04 -0700
>> Cc: gdb-patches <gdb-patches@sourceware.org>
>>
>> I certainly don't want to replicate all that text again
>> and again (hence the macros).
>
> I understand, but put yourself in the position of someone who needs to
> review a patch, and sees something like this:
>
>   +Display the status of skips.\n"
>   +EDDI_USAGE_DOC_STRING_WITH_ALL ("skip", "info skip", "displayed")));
>
> How does that someone know if the resulting text is good English and
> will produce a clear help text, or needs to be fixed in some way?

I'm open to suggestions.

I really hope the solution is to not expand all those macro
invocations in place.

Re: [PATCH 2/3] deprecate "skip enable/etc.", doc string cleanup

[Pedro Alves]

On 04/18/2016 09:01 PM, Doug Evans wrote:
> On Mon, Apr 18, 2016 at 12:53 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>> From: Doug Evans <dje@google.com>
>>> Date: Mon, 18 Apr 2016 12:42:04 -0700
>>> Cc: gdb-patches <gdb-patches@sourceware.org>
>>>
>>> I certainly don't want to replicate all that text again
>>> and again (hence the macros).
>>
>> I understand, but put yourself in the position of someone who needs to
>> review a patch, and sees something like this:
>>
>>   +Display the status of skips.\n"
>>   +EDDI_USAGE_DOC_STRING_WITH_ALL ("skip", "info skip", "displayed")));
>>
>> How does that someone know if the resulting text is good English and
>> will produce a clear help text, or needs to be fixed in some way?
> 
> I'm open to suggestions.
> 
> I really hope the solution is to not expand all those macro
> invocations in place.
> 

I think the answer is to include the before/after GDB output in
the mail submission / git log.

We actually already ask for that in the contribution checklist [1]:

 "If you're changing the output of some command, include a paste of the
 relevant parts of gdb session, before and after the change."

[1] - https://sourceware.org/gdb/wiki/ContributionChecklist#General_requirements

Thanks,
Pedro Alves

Re: [PATCH 2/3] deprecate "skip enable/etc.", doc string cleanup

[Eli Zaretskii]

> From: Doug Evans <dje@google.com>
> Date: Mon, 18 Apr 2016 13:01:37 -0700
> Cc: gdb-patches <gdb-patches@sourceware.org>
> 
> On Mon, Apr 18, 2016 at 12:53 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Doug Evans <dje@google.com>
> >> Date: Mon, 18 Apr 2016 12:42:04 -0700
> >> Cc: gdb-patches <gdb-patches@sourceware.org>
> >>
> >> I certainly don't want to replicate all that text again
> >> and again (hence the macros).
> >
> > I understand, but put yourself in the position of someone who needs to
> > review a patch, and sees something like this:
> >
> >   +Display the status of skips.\n"
> >   +EDDI_USAGE_DOC_STRING_WITH_ALL ("skip", "info skip", "displayed")));
> >
> > How does that someone know if the resulting text is good English and
> > will produce a clear help text, or needs to be fixed in some way?
> 
> I'm open to suggestions.
> 
> I really hope the solution is to not expand all those macro
> invocations in place.

If I'm the only one who is bothered by such macro-izing of doc
strings, then just ignore me.