[PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Yao Qi]

Hi,
As reported in gdb/14777, when typing 'fo' in CLI, we will get,

  (gdb) fo
  Ambiguous command "fo": focus, forward-search.

which is inconsistent to the doc about command 'forward-search' that
"abbreviate the command name as fo".

We can fix this problem in two ways, either remove this line above
from the doc or define "fo" as the abbreviated command of
'forward-search'.  IMO, latter is better, so this is what this patch
does.

Regression tested on x86_64-linux.  Is it OK?

gdb/doc:
2012-11-07  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Search): Add kindex for 'fo'.
gdb:
2012-11-07  Yao Qi  <yao@codesourcery.com>

	PR gdb/14777.
	* source.c (_initialize_source): Call add_com_alias to abbreviate
	'forward-search' as 'fo'.
---
 gdb/doc/gdb.texinfo |    1 +
 gdb/source.c        |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c2c7745..c7777e6 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6955,6 +6955,7 @@ regular expression.
 @table @code
 @kindex search
 @kindex forward-search
+@kindex fo @r{(@code{forward-search})}
 @item forward-search @var{regexp}
 @itemx search @var{regexp}
 The command @samp{forward-search @var{regexp}} checks each line,
diff --git a/gdb/source.c b/gdb/source.c
index bd11c63..307fecc 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1953,6 +1953,7 @@ The address is also stored as the value of \"$_\"."));
 Search for regular expression (see regex(3)) from last line listed.\n\
 The matching line number is also stored as the value of \"$_\"."));
   add_com_alias ("search", "forward-search", class_files, 0);
+  add_com_alias ("fo", "forward-search", class_files, 1);
 
   add_com ("reverse-search", class_files, reverse_search_command, _("\
 Search backward for regular expression (see regex(3)) from last line listed.\n\
-- 
1.7.7.6

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Joel Brobecker]

> We can fix this problem in two ways, either remove this line above
> from the doc or define "fo" as the abbreviated command of
> 'forward-search'.  IMO, latter is better, so this is what this patch
> does.

I tend to agree.

> gdb/doc:
> 2012-11-07  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.texinfo (Search): Add kindex for 'fo'.

Needs approval from Eli.

> gdb:
> 2012-11-07  Yao Qi  <yao@codesourcery.com>
> 
> 	PR gdb/14777.
> 	* source.c (_initialize_source): Call add_com_alias to abbreviate
> 	'forward-search' as 'fo'.

OK.

I should also add that we should make sure that we do not approve
updates to the manual that document command abbreviations without
specifically defining the abbreviation in our code. As demonstrated
in this case, implicit abbreviations may become ambiguous over time.

> ---
>  gdb/doc/gdb.texinfo |    1 +
>  gdb/source.c        |    1 +
>  2 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index c2c7745..c7777e6 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -6955,6 +6955,7 @@ regular expression.
>  @table @code
>  @kindex search
>  @kindex forward-search
> +@kindex fo @r{(@code{forward-search})}
>  @item forward-search @var{regexp}
>  @itemx search @var{regexp}
>  The command @samp{forward-search @var{regexp}} checks each line,
> diff --git a/gdb/source.c b/gdb/source.c
> index bd11c63..307fecc 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -1953,6 +1953,7 @@ The address is also stored as the value of \"$_\"."));
>  Search for regular expression (see regex(3)) from last line listed.\n\
>  The matching line number is also stored as the value of \"$_\"."));
>    add_com_alias ("search", "forward-search", class_files, 0);
> +  add_com_alias ("fo", "forward-search", class_files, 1);
>  
>    add_com ("reverse-search", class_files, reverse_search_command, _("\
>  Search backward for regular expression (see regex(3)) from last line listed.\n\
> -- 
> 1.7.7.6

-- 
Joel

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Pedro Alves]

On 11/07/2012 03:13 PM, Joel Brobecker wrote:
>> We can fix this problem in two ways, either remove this line above
>> from the doc or define "fo" as the abbreviated command of
>> 'forward-search'.  IMO, latter is better, so this is what this patch
>> does.
> 
> I tend to agree.
> 
>> gdb/doc:
>> 2012-11-07  Yao Qi  <yao@codesourcery.com>
>>
>> 	* gdb.texinfo (Search): Add kindex for 'fo'.
> 
> Needs approval from Eli.
> 
>> gdb:
>> 2012-11-07  Yao Qi  <yao@codesourcery.com>
>>
>> 	PR gdb/14777.
>> 	* source.c (_initialize_source): Call add_com_alias to abbreviate
>> 	'forward-search' as 'fo'.
> 
> OK.
> 
> I should also add that we should make sure that we do not approve
> updates to the manual that document command abbreviations without
> specifically defining the abbreviation in our code. As demonstrated
> in this case, implicit abbreviations may become ambiguous over time.

Yeah.  It just looks like an oversight of early development.  I see the
forward-search command as early as GDB 2.5.1 (1988), and the "fo"
abbreviation was already documented in gdb-3.3 (1989), the first
release we have that includes the manual.  I don't see an explicit
abbreviation implemented anywhere in between those releases.

-- 
Pedro Alves

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Yao Qi]

On 11/07/2012 11:13 PM, Joel Brobecker wrote:
> I should also add that we should make sure that we do not approve
> updates to the manual that document command abbreviations without
> specifically defining the abbreviation in our code. As demonstrated
> in this case, implicit abbreviations may become ambiguous over time.

Agreed.  I go through the doc today, and spot another three abbreviated
command missing abbreviation definition in code.

  @item show convenience
  Print a list of convenience variables used so far, and their values,
  as well as a list of the convenience functions.
  Abbreviated @code{show conv}.
              ^^^^^^^^^^^^^^^^

  because @value{GDBN} has the command @code{set gnutarget}, abbreviated
@code{set g}:
^^^^^^^^^^^^

  @noindent
  You can abbreviate this command as @code{del tr}.
                                     ^^^^^^^^^^^^^^

patch below is to fix them.
-- 
Yao

gdb:

2012-11-08  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c (_initialize_breakpoint) Call add_alias_cmd to
	abbreviate 'delete tracepoints' to 'delete tr'.
	* corefile.c (_initialize_core): Call add_alias_cmd to
	abbreviate 'set gnutarget' to 'set g'.
	* value.c (_initialize_values): Call add_alias_cmd to abbreviate
	'show convenience' to 'show conv'.
---
 gdb/breakpoint.c |    1 +
 gdb/corefile.c   |    1 +
 gdb/value.c      |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index c7eef24..e16212e 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -16337,6 +16337,7 @@ Delete specified tracepoints.\n\
 Arguments are tracepoint numbers, separated by spaces.\n\
 No argument means delete all tracepoints."),
 	   &deletelist);
+  add_alias_cmd ("tr", "tracepoints", no_class, 1, &deletelist);
 
   c = add_cmd ("tracepoints", class_trace, disable_trace_command, _("\
 Disable specified tracepoints.\n\
diff --git a/gdb/corefile.c b/gdb/corefile.c
index a2ed24f..da6e102 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -456,6 +456,7 @@ Use `set gnutarget auto' to specify automatic detection."),
 				   set_gnutarget_command,
 				   show_gnutarget_string,
 				   &setlist, &showlist);
+  add_alias_cmd ("g", "gnutarget", class_files, 1, &setlist);
 
   if (getenv ("GNUTARGET"))
     set_gnutarget (getenv ("GNUTARGET"));
diff --git a/gdb/value.c b/gdb/value.c
index 3feb1ca..a3cee32 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3380,6 +3380,7 @@ A few convenience variables are given values automatically:\n\
 Convenience functions are defined via the Python API."
 #endif
 	   ), &showlist);
+  add_alias_cmd ("conv", "convenience", no_class, 1, &showlist);
 
   add_cmd ("values", no_set_class, show_values, _("\
 Elements of value history around item number IDX (or last ten)."),
-- 
1.7.7.6

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Pedro Alves]

These are user visible changes, and if we don't tell users about the
change in NEWS, those who have tripped on the docs-not-match-reality
issue before will never try the abbreviations again, thinking they
don't really work, even though they do now work.  IOW, IMO, this
is NEWS worty.

-- 
Pedro Alves

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Yao Qi]

On 11/09/2012 12:10 AM, Pedro Alves wrote:
> These are user visible changes, and if we don't tell users about the
> change in NEWS, those who have tripped on the docs-not-match-reality
> issue before will never try the abbreviations again, thinking they
> don't really work, even though they do now work.  IOW, IMO, this
> is NEWS worty.

Pedro, changes in this patch [1] are not user visible, because even 
without this patch nowadays, the (implicitly defined) abbreviated 
commands still behave correctly.  This patch [1] is to prevent these 
abbreviated commands becoming ambiguous in the future.

Of course, this is a ping to the patch [1].

-- 
Yao (齐尧)

[1] http://sourceware.org/ml/gdb-patches/2012-11/msg00183.html

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Pedro Alves]

On 11/28/2012 10:09 AM, Yao Qi wrote:
> On 11/09/2012 12:10 AM, Pedro Alves wrote:
>> These are user visible changes, and if we don't tell users about the
>> change in NEWS, those who have tripped on the docs-not-match-reality
>> issue before will never try the abbreviations again, thinking they
>> don't really work, even though they do now work.  IOW, IMO, this
>> is NEWS worty.
> 
> Pedro, changes in this patch [1] are not user visible, because even without this patch nowadays, the (implicitly defined) abbreviated commands still behave correctly.  This patch [1] is to prevent these abbreviated commands becoming ambiguous in the future.

I see.

> Of course, this is a ping to the patch [1].

> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -16337,6 +16337,7 @@ Delete specified tracepoints.\n\
>  Arguments are tracepoint numbers, separated by spaces.\n\
>  No argument means delete all tracepoints."),
>  	   &deletelist);
> +  add_alias_cmd ("tr", "tracepoints", no_class, 1, &deletelist);

The command being aliased is class_trace, while the alias is no_class.  Why's that?

-- 
Pedro Alves

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Yao Qi]

On 11/28/2012 06:18 PM, Pedro Alves wrote:
>> +  add_alias_cmd ("tr", "tracepoints", no_class, 1, &deletelist);
> The command being aliased is class_trace, while the alias is no_class.  Why's that?

Because it is a copy-paste error.  How about this one?

-- 
Yao (齐尧)

gdb:

2012-11-28  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c (_initialize_breakpoint) Call add_alias_cmd to
	abbreviate 'delete tracepoints' to 'delete tr'.
	* corefile.c (_initialize_core): Call add_alias_cmd to
	abbreviate 'set gnutarget' to 'set g'.
	* value.c (_initialize_values): Call add_alias_cmd to abbreviate
	'show convenience' to 'show conv'.
---
 gdb/breakpoint.c |    1 +
 gdb/corefile.c   |    1 +
 gdb/value.c      |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 7293d18..3799cbf 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -16339,6 +16339,7 @@ Delete specified tracepoints.\n\
 Arguments are tracepoint numbers, separated by spaces.\n\
 No argument means delete all tracepoints."),
 	   &deletelist);
+  add_alias_cmd ("tr", "tracepoints", class_trace, 1, &deletelist);
 
   c = add_cmd ("tracepoints", class_trace, disable_trace_command, _("\
 Disable specified tracepoints.\n\
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 78dbbc2..eab715d 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -456,6 +456,7 @@ Use `set gnutarget auto' to specify automatic detection."),
 				   set_gnutarget_command,
 				   show_gnutarget_string,
 				   &setlist, &showlist);
+  add_alias_cmd ("g", "gnutarget", class_files, 1, &setlist);
 
   if (getenv ("GNUTARGET"))
     set_gnutarget (getenv ("GNUTARGET"));
diff --git a/gdb/value.c b/gdb/value.c
index 2a1e1f2..07130ed 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3384,6 +3384,7 @@ A few convenience variables are given values automatically:\n\
 Convenience functions are defined via the Python API."
 #endif
 	   ), &showlist);
+  add_alias_cmd ("conv", "convenience", no_class, 1, &showlist);
 
   add_cmd ("values", no_set_class, show_values, _("\
 Elements of value history around item number IDX (or last ten)."),
-- 
1.7.7.6

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Pedro Alves]

On 11/28/2012 10:35 AM, Yao Qi wrote:
> 
> 2012-11-28  Yao Qi  <yao@codesourcery.com>
> 
> 	* breakpoint.c (_initialize_breakpoint) Call add_alias_cmd to

Missing ':'.

> 	abbreviate 'delete tracepoints' to 'delete tr'.
> 	* corefile.c (_initialize_core): Call add_alias_cmd to
> 	abbreviate 'set gnutarget' to 'set g'.
> 	* value.c (_initialize_values): Call add_alias_cmd to abbreviate
> 	'show convenience' to 'show conv'.


OK.

-- 
Pedro Alves

[committed]: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Yao Qi]

On 11/28/2012 07:10 PM, Pedro Alves wrote:
> On 11/28/2012 10:35 AM, Yao Qi wrote:
>>
>> 2012-11-28  Yao Qi  <yao@codesourcery.com>
>>
>> 	* breakpoint.c (_initialize_breakpoint) Call add_alias_cmd to
>
> Missing ':'.
>
>> 	abbreviate 'delete tracepoints' to 'delete tr'.
>> 	* corefile.c (_initialize_core): Call add_alias_cmd to
>> 	abbreviate 'set gnutarget' to 'set g'.
>> 	* value.c (_initialize_values): Call add_alias_cmd to abbreviate
>> 	'show convenience' to 'show conv'.
>
>
> OK.
>

The missing ':' is added into changelog entry, and patch is committed.


-- 
Yao (齐尧)

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Wed, 7 Nov 2012 21:01:07 +0800
> 
>   (gdb) fo
>   Ambiguous command "fo": focus, forward-search.
> 
> which is inconsistent to the doc about command 'forward-search' that
> "abbreviate the command name as fo".
> 
> We can fix this problem in two ways, either remove this line above
> from the doc or define "fo" as the abbreviated command of
> 'forward-search'.  IMO, latter is better, so this is what this patch
> does.

Fine with me, thanks.

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Yao Qi]

On 11/07/2012 09:01 PM, Yao Qi wrote:
> gdb/doc:
> 2012-11-07  Yao Qi<yao@codesourcery.com>
> 
> 	* gdb.texinfo (Search): Add kindex for 'fo'.
> gdb:
> 2012-11-07  Yao Qi<yao@codesourcery.com>
> 
> 	PR gdb/14777.
> 	* source.c (_initialize_source): Call add_com_alias to abbreviate
> 	'forward-search' as 'fo'.

Here is the news entry to this fix, because this fix is visible
to users.  OK?

-- 
Yao

gdb:

2012-11-09  Yao Qi  <yao@codesourcery.com>

	* NEWS: Mention the fix to the ambiguity of 'fo' command.
---
 gdb/NEWS |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 069b84f..5ed029f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -24,6 +24,9 @@
 * The 'cd' command now defaults to using '~' (the home directory) if not
   given an argument.
 
+* The ambiguity of 'fo' command is fixed and it is the abbreviated command
+  of 'forward-search'.
+
 * New configure options
 
 --enable-libmcheck/--disable-libmcheck
-- 
1.7.7.6

Re: [PATCH] partial fix PR 14777: Add abbrev cmd 'fo' for 'forward-search'.

[Eli Zaretskii]

> Date: Fri, 9 Nov 2012 16:34:40 +0800
> From: Yao Qi <yao@codesourcery.com>
> 
> On 11/07/2012 09:01 PM, Yao Qi wrote:
> > gdb/doc:
> > 2012-11-07  Yao Qi<yao@codesourcery.com>
> > 
> > 	* gdb.texinfo (Search): Add kindex for 'fo'.
> > gdb:
> > 2012-11-07  Yao Qi<yao@codesourcery.com>
> > 
> > 	PR gdb/14777.
> > 	* source.c (_initialize_source): Call add_com_alias to abbreviate
> > 	'forward-search' as 'fo'.
> 
> Here is the news entry to this fix, because this fix is visible
> to users.  OK?

Almost.  We don't mention bugfixes in NEWS, so just the second part
will do.  Something like

  The command 'forward-search' can now be abbreviated as 'fo'.

Thanks.