public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Add option to remove duplicate command history entries
@ 2015-06-04 16:22 Patrick Palka
  2015-06-04 16:47 ` Eli Zaretskii
  2015-06-09 18:10 ` Pedro Alves
  0 siblings, 2 replies; 14+ messages in thread
From: Patrick Palka @ 2015-06-04 16:22 UTC (permalink / raw)
  To: gdb-patches; +Cc: Patrick Palka

This patch implements the new option "history remove-duplicates", which
controls whether GDB should remove duplicate command-history entries
(off by default).

The motivation for this option is to be able to reduce the prevalence of
basic commands such as "up" and "down" in the history file.  These
common commands crowd out more unique commands in the history file (when
the history file has a fixed size), and they make navigation of the
history file via ^P, ^N and ^R more inconvenient.

[ I decided to go with a basic boolean option instead of a numeric
  lookbehind_threshold option because it is simpler, and the maximum
  lookbehind is already restricted by the number of unique commands
  invoked during the session, a number which will rarely exceed 100.  ]

gdb/ChangeLog:

	* NEWS: Mention the new option "history remove-duplicates".
	* top.c (history_remove_duplicates_p): New static variable.
	(show_history_remove_duplicates_p): New static function.
	(gdb_add_history): Conditionally remove duplicate history
	entries.
	(init_main): Add "history remove-duplicates" option.

gdb/doc/ChangeLog:

	* gdb.texinfo: Document the new option
	"history remove-duplicates".

gdb/testsuite/ChangeLog:

	* gdb.base/history-duplicates.exp: New test.
---
 gdb/NEWS                                      |  4 ++
 gdb/doc/gdb.texinfo                           | 13 ++++++
 gdb/testsuite/gdb.base/history-duplicates.exp | 57 +++++++++++++++++++++++++++
 gdb/top.c                                     | 51 ++++++++++++++++++++++++
 4 files changed, 125 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/history-duplicates.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index bbfb55d..411be32 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -123,6 +123,10 @@ show max-completions
   to avoid generating large completion lists, the computation of
   which can cause the debugger to become temporarily unresponsive.
 
+set history remove-duplicates
+show history remove-duplicates
+  Control the removal of duplicate history entries.
+
 maint set symbol-cache-size
 maint show symbol-cache-size
   Control the size of the symbol cache.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9ea846a..722186c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22607,6 +22607,19 @@ This defaults to the value of the environment variable
 @code{HISTSIZE}, or to 256 if this variable is not set.  If @var{size}
 is @code{unlimited}, the number of commands @value{GDBN} keeps in the
 history list is unlimited.
+
+@cindex remove duplicate history
+@kindex set history remove-duplicates
+@item set history remove-duplicates
+@itemx set history remove-duplicates on
+Remove duplicate history entries added during the current session.  Before a
+history entry is added while this option is on, @value{GDBN} scans the command
+history list in reverse-chronological order and removes the first duplicate
+entry it finds. This option is off by default.
+
+@item set history remove-duplicates off
+Do not remove duplicate history entries.
+
 @end table
 
 History expansion assigns special meaning to the character @kbd{!}.
diff --git a/gdb/testsuite/gdb.base/history-duplicates.exp b/gdb/testsuite/gdb.base/history-duplicates.exp
new file mode 100644
index 0000000..ef6f6a3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/history-duplicates.exp
@@ -0,0 +1,57 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test the operation of the "history remove-duplicates" option.
+
+
+# Check that the previous history entry is ENTRY.
+
+proc check_prev_history_entry { entry } {
+    set test "history entry is $entry"
+
+    # Send ^P followed by ^L.
+    send_gdb "\x10\x0c"
+
+    gdb_expect {
+        -re $entry {
+            pass $test
+        }
+        timeout {
+            fail $test
+        }
+    }
+}
+
+gdb_start
+
+gdb_test "set history remove-duplicates" ""
+
+gdb_test "print 0" ""
+gdb_test "print 1" ""
+gdb_test "print 2" ""
+gdb_test "print 1" ""
+gdb_test "print 1" ""
+gdb_test "print 2" ""
+gdb_test "print 3" ""
+gdb_test "print 3" ""
+gdb_test "print 4" ""
+
+check_prev_history_entry "print 4"
+check_prev_history_entry "print 3"
+check_prev_history_entry "print 2"
+check_prev_history_entry "print 1"
+check_prev_history_entry "print 0"
diff --git a/gdb/top.c b/gdb/top.c
index f8f926b..f5a0819 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -698,6 +698,18 @@ show_history_size (struct ui_file *file, int from_tty,
 		    value);
 }
 
+/* Variable which controls whether duplicate history entries added during this
+   session are scanned for and removed.  */
+static int history_remove_duplicates_p = 0;
+
+static void
+show_history_remove_duplicates_p (struct ui_file *file, int from_tty,
+				  struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file, _("Removal of duplicate history entries is %s.\n"),
+		    value);
+}
+
 static char *history_filename;
 static void
 show_history_filename (struct ui_file *file, int from_tty,
@@ -897,6 +909,34 @@ static int command_count = 0;
 void
 gdb_add_history (const char *command)
 {
+  if (history_remove_duplicates_p)
+    {
+      int lookbehind;
+
+      /* Check to see whether we have an existing command history entry for
+	 this command -- and remove it if so -- but only among history entries
+	 added during this session.  Since we update the history file by
+	 appending to it, history entries that are already stored in the
+	 history file can't be meaningfully deleted.  */
+      using_history ();
+      for (lookbehind = 0; lookbehind < command_count; lookbehind++)
+	{
+	  HIST_ENTRY *temp = previous_history ();
+
+	  if (temp == NULL)
+	    break;
+
+	  if (strcmp (temp->line, command) == 0)
+	    {
+	      HIST_ENTRY *prev = remove_history (where_history ());
+	      command_count--;
+	      free_history_entry (prev);
+	      break;
+	    }
+	}
+      using_history ();
+    }
+
   add_history (command);
   command_count++;
 }
@@ -1862,6 +1902,17 @@ variable \"HISTSIZE\", or to 256 if this variable is not set."),
 			    show_history_size,
 			    &sethistlist, &showhistlist);
 
+  add_setshow_boolean_cmd ("remove-duplicates", no_class,
+			   &history_remove_duplicates_p, _("\
+Set removal of duplicate history entries added during this session."), _("\
+Show removal of duplicate history entries added during this session."), _("\
+Use \"on\" to enable removal of duplicate entries, and \"off\" to disable it.\n\
+Only history entries added during this session are considered for removal.\n\
+Without an argument, removal of duplicate entries is enabled."),
+			   NULL,
+			   show_history_remove_duplicates_p,
+			   &sethistlist, &showhistlist);
+
   add_setshow_filename_cmd ("filename", no_class, &history_filename, _("\
 Set the filename in which to record the command history"), _("\
 Show the filename in which to record the command history"), _("\
-- 
2.4.2.387.gf86f31a.dirty

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-04 16:22 [PATCH] Add option to remove duplicate command history entries Patrick Palka
@ 2015-06-04 16:47 ` Eli Zaretskii
  2015-06-04 18:54   ` Patrick Palka
  2015-06-09 18:10 ` Pedro Alves
  1 sibling, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2015-06-04 16:47 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gdb-patches, patrick

> From: Patrick Palka <patrick@parcs.ath.cx>
> Cc: Patrick Palka <patrick@parcs.ath.cx>
> Date: Thu,  4 Jun 2015 12:21:58 -0400
> 
> This patch implements the new option "history remove-duplicates", which
> controls whether GDB should remove duplicate command-history entries
> (off by default).

Thanks.

> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo: Document the new option
> 	"history remove-duplicates".

This ChangeLog entry should name the node in which you made the
changes, as if it were a function (i.e., in parentheses).

> diff --git a/gdb/NEWS b/gdb/NEWS
> index bbfb55d..411be32 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -123,6 +123,10 @@ show max-completions
>    to avoid generating large completion lists, the computation of
>    which can cause the debugger to become temporarily unresponsive.
>  
> +set history remove-duplicates
> +show history remove-duplicates
> +  Control the removal of duplicate history entries.

This part is OK.

> +@cindex remove duplicate history
> +@kindex set history remove-duplicates
> +@item set history remove-duplicates
> +@itemx set history remove-duplicates on
> +Remove duplicate history entries added during the current session.  Before a

Given the description below, this summary is slightly misleading,
IMO.  Why not simply

  Keep in history of CLI commands only one copy of each command.

> +entry it finds. This option is off by default.
                 ^^
Two spaces between sentences, please.

The documentation parts are okay with these fixed.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-04 16:47 ` Eli Zaretskii
@ 2015-06-04 18:54   ` Patrick Palka
  2015-06-04 19:25     ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2015-06-04 18:54 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On Thu, Jun 4, 2015 at 12:47 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Patrick Palka <patrick@parcs.ath.cx>
>> Cc: Patrick Palka <patrick@parcs.ath.cx>
>> Date: Thu,  4 Jun 2015 12:21:58 -0400
>>
>> This patch implements the new option "history remove-duplicates", which
>> controls whether GDB should remove duplicate command-history entries
>> (off by default).
>
> Thanks.
>
>> gdb/doc/ChangeLog:
>>
>>       * gdb.texinfo: Document the new option
>>       "history remove-duplicates".
>
> This ChangeLog entry should name the node in which you made the
> changes, as if it were a function (i.e., in parentheses).
>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index bbfb55d..411be32 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -123,6 +123,10 @@ show max-completions
>>    to avoid generating large completion lists, the computation of
>>    which can cause the debugger to become temporarily unresponsive.
>>
>> +set history remove-duplicates
>> +show history remove-duplicates
>> +  Control the removal of duplicate history entries.
>
> This part is OK.
>
>> +@cindex remove duplicate history
>> +@kindex set history remove-duplicates
>> +@item set history remove-duplicates
>> +@itemx set history remove-duplicates on
>> +Remove duplicate history entries added during the current session.  Before a
>
> Given the description below, this summary is slightly misleading,
> IMO.  Why not simply
>
>   Keep in history of CLI commands only one copy of each command.

How about I rewrite this section into:

 Keep in the command history list only one copy of each command.  If a new
 command being added to the history list is a duplicate of an older one, the
 older entry is removed from the list.  Only history entries added during the
 current session are considered for removal.  This option is off by default.

>
>> +entry it finds. This option is off by default.
>                  ^^
> Two spaces between sentences, please.
>
> The documentation parts are okay with these fixed.

Everything else fixed.  Thanks for reviewing.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-04 18:54   ` Patrick Palka
@ 2015-06-04 19:25     ` Eli Zaretskii
  0 siblings, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2015-06-04 19:25 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gdb-patches

> From: Patrick Palka <patrick@parcs.ath.cx>
> Date: Thu, 4 Jun 2015 14:53:40 -0400
> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> >> +@cindex remove duplicate history
> >> +@kindex set history remove-duplicates
> >> +@item set history remove-duplicates
> >> +@itemx set history remove-duplicates on
> >> +Remove duplicate history entries added during the current session.  Before a
> >
> > Given the description below, this summary is slightly misleading,
> > IMO.  Why not simply
> >
> >   Keep in history of CLI commands only one copy of each command.
> 
> How about I rewrite this section into:
> 
>  Keep in the command history list only one copy of each command.  If a new
>  command being added to the history list is a duplicate of an older one, the
>  older entry is removed from the list.  Only history entries added during the
>  current session are considered for removal.  This option is off by default.

Perfect, thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-04 16:22 [PATCH] Add option to remove duplicate command history entries Patrick Palka
  2015-06-04 16:47 ` Eli Zaretskii
@ 2015-06-09 18:10 ` Pedro Alves
  2015-06-09 18:41   ` Patrick Palka
  1 sibling, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2015-06-09 18:10 UTC (permalink / raw)
  To: Patrick Palka, gdb-patches

On 06/04/2015 05:21 PM, Patrick Palka wrote:
> This patch implements the new option "history remove-duplicates", which
> controls whether GDB should remove duplicate command-history entries
> (off by default).
> 
> The motivation for this option is to be able to reduce the prevalence of
> basic commands such as "up" and "down" in the history file.  These
> common commands crowd out more unique commands in the history file (when
> the history file has a fixed size), and they make navigation of the
> history file via ^P, ^N and ^R more inconvenient.
> 

Did you consider bash's erasedups and ignoredups?  Specifically,
this seems to implement something like erasedups, and I'm wondering
how you'd fit in ignoredups in this option's UI.  Might be good to
prepare for it with an enum instead, something like:

  "set history duplicates ignore|erase|leave"

WDYT?

(haven't looked at the patch yet)

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-09 18:10 ` Pedro Alves
@ 2015-06-09 18:41   ` Patrick Palka
  2015-06-10 15:12     ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2015-06-09 18:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Tue, Jun 9, 2015 at 2:10 PM, Pedro Alves <palves@redhat.com> wrote:
> On 06/04/2015 05:21 PM, Patrick Palka wrote:
>> This patch implements the new option "history remove-duplicates", which
>> controls whether GDB should remove duplicate command-history entries
>> (off by default).
>>
>> The motivation for this option is to be able to reduce the prevalence of
>> basic commands such as "up" and "down" in the history file.  These
>> common commands crowd out more unique commands in the history file (when
>> the history file has a fixed size), and they make navigation of the
>> history file via ^P, ^N and ^R more inconvenient.
>>
>
> Did you consider bash's erasedups and ignoredups?  Specifically,
> this seems to implement something like erasedups, and I'm wondering
> how you'd fit in ignoredups in this option's UI.  Might be good to
> prepare for it with an enum instead, something like:
>
>   "set history duplicates ignore|erase|leave"
>
> WDYT?

An "ignoredups" option currently seems not useful in GDB since we
already have the empty-command shorthand for running the previous
command again, which does not add to the history.  But if we ever make
the empty-command shorthand toggle-able then an "ignoredups"
equivalent could be useful when the shorthand is turned off.  I am
actually thinking about implementing that too, since I do not like the
shorthand very much and would like to be able to turn it off.  So I
might as well implement "ignoredups" too.

>
> (haven't looked at the patch yet)
>
> Thanks,
> Pedro Alves
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-09 18:41   ` Patrick Palka
@ 2015-06-10 15:12     ` Pedro Alves
  2015-06-19 23:33       ` Patrick Palka
  0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2015-06-10 15:12 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gdb-patches

On 06/09/2015 07:40 PM, Patrick Palka wrote:
> On Tue, Jun 9, 2015 at 2:10 PM, Pedro Alves <palves@redhat.com> wrote:
>> On 06/04/2015 05:21 PM, Patrick Palka wrote:
>>> This patch implements the new option "history remove-duplicates", which
>>> controls whether GDB should remove duplicate command-history entries
>>> (off by default).
>>>
>>> The motivation for this option is to be able to reduce the prevalence of
>>> basic commands such as "up" and "down" in the history file.  These
>>> common commands crowd out more unique commands in the history file (when
>>> the history file has a fixed size), and they make navigation of the
>>> history file via ^P, ^N and ^R more inconvenient.
>>>
>>
>> Did you consider bash's erasedups and ignoredups?  Specifically,
>> this seems to implement something like erasedups, and I'm wondering
>> how you'd fit in ignoredups in this option's UI.  Might be good to
>> prepare for it with an enum instead, something like:
>>
>>   "set history duplicates ignore|erase|leave"
>>
>> WDYT?
> 
> An "ignoredups" option currently seems not useful in GDB since we
> already have the empty-command shorthand for running the previous
> command again, which does not add to the history.  But if we ever make
> the empty-command shorthand toggle-able then an "ignoredups"
> equivalent could be useful when the shorthand is turned off.  I am
> actually thinking about implementing that too, since I do not like the
> shorthand very much and would like to be able to turn it off.  So I
> might as well implement "ignoredups" too.

Alright, sounds good.  I like the shorthand myself, but then
that's why these things have knobs.  :-)

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] Add option to remove duplicate command history entries
  2015-06-10 15:12     ` Pedro Alves
@ 2015-06-19 23:33       ` Patrick Palka
  2015-06-20  6:52         ` Eli Zaretskii
                           ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Patrick Palka @ 2015-06-19 23:33 UTC (permalink / raw)
  To: gdb-patches; +Cc: Patrick Palka

This patch implements the new option "history remove-duplicates", which
controls the removal of duplicate history entries ("off" by default).

The motivation for this option is to be able to reduce the prevalence of
basic commands such as "up" and "down" in the history file.  These
common commands crowd out more unique commands in the history file (when
the history file has a fixed size), and they make navigation of the
history file via ^P, ^N and ^R more inconvenient.

The option takes an integer denoting the number of history entries to
look back at for a history entry that is a duplicate of the latest one.
"history remove-duplicates 1" is equivalent to bash's ignoredups option,
and "history remove-duplicates unlimited" is equivalent to bash's
erasedups option.

[ I decided to go with this integer approach instead of a tri-state enum
  because it's slightly more flexible and seemingly more intuitive than
  leave/erase/ignore.  ]

gdb/ChangeLog:

	* NEWS: Mention the new option "history remove-duplicates".
	* top.c (history_remove_duplicates): New static variable.
	(show_history_remove_duplicates): New static function.
	(gdb_add_history): Conditionally remove duplicate history
	entries.
	(init_main): Add "history remove-duplicates" option.

gdb/doc/ChangeLog:

	* gdb.texinfo (Command History): Document the new option
	"history remove-duplicates".

gdb/testsuite/ChangeLog:

	* gdb.base/history-duplicates.exp: New test.
---
 gdb/NEWS                                      |   4 +
 gdb/doc/gdb.texinfo                           |  15 +++
 gdb/testsuite/gdb.base/history-duplicates.exp | 129 ++++++++++++++++++++++++++
 gdb/top.c                                     |  66 ++++++++++++-
 4 files changed, 213 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/history-duplicates.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 3ec5851..6d29004 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -145,6 +145,10 @@ show max-completions
   to avoid generating large completion lists, the computation of
   which can cause the debugger to become temporarily unresponsive.
 
+set history remove-duplicates
+show history remove-duplicates
+  Control the removal of duplicate history entries.
+
 maint set symbol-cache-size
 maint show symbol-cache-size
   Control the size of the symbol cache.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c9a532a..c5885eb 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22644,6 +22644,21 @@ to 256 if this variable is not set.  Non-numeric values of @env{GDBHISTSIZE}
 are ignored.  If @var{size} is @code{unlimited} or if @env{GDBHISTSIZE} is
 either a negative number or the empty string, then the number of commands
 @value{GDBN} keeps in the history list is unlimited.
+
+@cindex remove duplicate history
+@kindex set history remove-duplicates
+@item set history remove-duplicates @var{size}
+@itemx set history remove-duplicates unlimited
+Control the removal of duplicate history entries in the command history list.
+If @var{size} is non-zero, @value{GDBN} will look back at the last @var{size}
+history entries and remove the first entry that is a duplicate of the current
+entry being added to the command history list.  If @var{size} is
+@code{unlimited} then this lookbehind is unbounded.  If @var{size} is 0, then
+removal of duplicate history entries is disabled.
+
+Only history entries added during the current session are considered for
+removal.  This option is set to 0 by default.
+
 @end table
 
 History expansion assigns special meaning to the character @kbd{!}.
diff --git a/gdb/testsuite/gdb.base/history-duplicates.exp b/gdb/testsuite/gdb.base/history-duplicates.exp
new file mode 100644
index 0000000..f88607f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/history-duplicates.exp
@@ -0,0 +1,129 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test the operation of the "history remove-duplicates" option.
+
+
+# Check that the previous history entry is ENTRY.
+
+proc check_prev_history_entry { entry { test_suffix "" } } {
+    set test "history entry is $entry"
+    if { $test_suffix != "" } {
+        append test " $test_suffix"
+    }
+
+    # Send ^P followed by ^L.
+    send_gdb "\x10\x0c"
+
+    gdb_expect {
+	-re $entry {
+	    pass $test
+	}
+	timeout {
+	    fail $test
+	}
+    }
+}
+
+# By default the option is set to 0.
+gdb_exit
+gdb_start
+gdb_test "show history remove-duplicates" "is 0\\."
+
+# Test the "unlimited" setting.
+with_test_prefix "remove-duplicates=unlimited" {
+    gdb_exit
+    gdb_start
+    gdb_test "set history remove-duplicates unlimited"
+
+    gdb_test "print 0"
+    gdb_test "print 1"
+    gdb_test "print 2"
+    gdb_test "print 1"
+    gdb_test "print 1"
+    gdb_test "print 2"
+    gdb_test "print 3"
+    gdb_test "print 3"
+    gdb_test "print 4"
+    gdb_test "print 1"
+    gdb_test "print 2"
+    gdb_test "print 3"
+    gdb_test "print 4"
+
+    check_prev_history_entry "print 4"
+    check_prev_history_entry "print 3"
+    check_prev_history_entry "print 2"
+    check_prev_history_entry "print 1"
+    check_prev_history_entry "print 0"
+}
+
+
+# Test the "1" setting.
+with_test_prefix "remove-duplicates=1" {
+    gdb_exit
+    gdb_start
+    gdb_test "set history remove-duplicates 1"
+
+    gdb_test "print 0"
+    gdb_test "print 1"
+    gdb_test "print 0"
+    gdb_test "print 2"
+    gdb_test "print 2"
+    gdb_test "print 1"
+
+    check_prev_history_entry "print 1"
+    check_prev_history_entry "print 2"
+    check_prev_history_entry "print 0"
+    check_prev_history_entry "print 1" "(again)"
+    check_prev_history_entry "print 0" "(again)"
+}
+
+
+# Test the "0" setting.
+with_test_prefix "remove-duplicates=0" {
+    gdb_exit
+    gdb_start
+    gdb_test "set history remove-duplicates 0"
+
+    gdb_test "print 0"
+    gdb_test "print 0"
+    gdb_test "print 1"
+    gdb_test "print 1"
+
+    check_prev_history_entry "print 1"
+    check_prev_history_entry "print 1" "(again)"
+    check_prev_history_entry "print 0"
+    check_prev_history_entry "print 0" "(again)"
+}
+
+
+# Test the "2" setting.
+with_test_prefix "remove-duplicates=2" {
+    gdb_exit
+    gdb_start
+    gdb_test "set history remove-duplicates 2"
+
+    gdb_test "print 1"
+    gdb_test "print 2"
+    gdb_test "print 0"
+    gdb_test "print 2"
+    gdb_test "print 0"
+
+    check_prev_history_entry "print 0"
+    check_prev_history_entry "print 2"
+    check_prev_history_entry "print 1"
+}
diff --git a/gdb/top.c b/gdb/top.c
index 6ae84ab..5114c2e 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -698,6 +698,20 @@ show_history_size (struct ui_file *file, int from_tty,
 		    value);
 }
 
+/* Variable associated with the "history remove-duplicates" option.
+   The value -1 means unlimited.  */
+static int history_remove_duplicates = 0;
+
+static void
+show_history_remove_duplicates (struct ui_file *file, int from_tty,
+				struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file,
+		    _("The number of history entries to look back at for "
+		      "duplicates is %s.\n"),
+		    value);
+}
+
 static char *history_filename;
 static void
 show_history_filename (struct ui_file *file, int from_tty,
@@ -897,8 +911,43 @@ static int command_count = 0;
 void
 gdb_add_history (const char *command)
 {
-  add_history (command);
   command_count++;
+
+  if (history_remove_duplicates != 0)
+    {
+      int lookbehind;
+      int lookbehind_threshold;
+
+      /* The lookbehind threshold for finding a duplicate history entry is
+	 bounded by command_count because we can't meaningfully delete
+	 history entries that are already stored in the history file since
+	 the history file is appended to.  */
+      if (history_remove_duplicates == -1
+	  || history_remove_duplicates > command_count)
+	lookbehind_threshold = command_count;
+      else
+	lookbehind_threshold = history_remove_duplicates;
+
+      using_history ();
+      for (lookbehind = 0; lookbehind < lookbehind_threshold; lookbehind++)
+	{
+	  HIST_ENTRY *temp = previous_history ();
+
+	  if (temp == NULL)
+	    break;
+
+	  if (strcmp (temp->line, command) == 0)
+	    {
+	      HIST_ENTRY *prev = remove_history (where_history ());
+	      command_count--;
+	      free_history_entry (prev);
+	      break;
+	    }
+	}
+      using_history ();
+    }
+
+  add_history (command);
 }
 
 /* Safely append new history entries to the history file in a corruption-free
@@ -1872,6 +1921,21 @@ variable \"GDBHISTSIZE\", or to 256 if this variable is not set."),
 			    show_history_size,
 			    &sethistlist, &showhistlist);
 
+  add_setshow_zuinteger_unlimited_cmd ("remove-duplicates", no_class,
+				       &history_remove_duplicates, _("\
+Set how far back in history to look for and remove duplicate entries."), _("\
+Show how far back in history to look for and remove duplicate entries."), _("\
+If set to a non-zero value N, GDB will look back at the last N history entries\n\
+and remove the first history entry that is a duplicate of the most recent\n\
+entry, each time a new history entry is added.\n\
+If set to \"unlimited\", this lookbehind is unbounded.\n\
+Only history entries added during this session are considered for removal.\n\
+If set to 0, removal of duplicate history entries is disabled.\n\
+By default this option is set to 0."),
+			   NULL,
+			   show_history_remove_duplicates,
+			   &sethistlist, &showhistlist);
+
   add_setshow_filename_cmd ("filename", no_class, &history_filename, _("\
 Set the filename in which to record the command history"), _("\
 Show the filename in which to record the command history"), _("\
-- 
2.4.4.410.g43ed522.dirty

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-19 23:33       ` Patrick Palka
@ 2015-06-20  6:52         ` Eli Zaretskii
  2015-06-26 13:35         ` Patrick Palka
                           ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2015-06-20  6:52 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gdb-patches, patrick

> From: Patrick Palka <patrick@parcs.ath.cx>
> Cc: Patrick Palka <patrick@parcs.ath.cx>
> Date: Fri, 19 Jun 2015 19:33:41 -0400
> 
> This patch implements the new option "history remove-duplicates", which
> controls the removal of duplicate history entries ("off" by default).
> 
> The motivation for this option is to be able to reduce the prevalence of
> basic commands such as "up" and "down" in the history file.  These
> common commands crowd out more unique commands in the history file (when
> the history file has a fixed size), and they make navigation of the
> history file via ^P, ^N and ^R more inconvenient.
> 
> The option takes an integer denoting the number of history entries to
> look back at for a history entry that is a duplicate of the latest one.
> "history remove-duplicates 1" is equivalent to bash's ignoredups option,
> and "history remove-duplicates unlimited" is equivalent to bash's
> erasedups option.
> 
> [ I decided to go with this integer approach instead of a tri-state enum
>   because it's slightly more flexible and seemingly more intuitive than
>   leave/erase/ignore.  ]
> 
> gdb/ChangeLog:
> 
> 	* NEWS: Mention the new option "history remove-duplicates".
> 	* top.c (history_remove_duplicates): New static variable.
> 	(show_history_remove_duplicates): New static function.
> 	(gdb_add_history): Conditionally remove duplicate history
> 	entries.
> 	(init_main): Add "history remove-duplicates" option.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Command History): Document the new option
> 	"history remove-duplicates".
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/history-duplicates.exp: New test.

OK for the documentation parts.

Thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-19 23:33       ` Patrick Palka
  2015-06-20  6:52         ` Eli Zaretskii
@ 2015-06-26 13:35         ` Patrick Palka
  2015-06-26 14:18         ` Pedro Alves
  2015-06-26 15:12         ` Patrick Palka
  3 siblings, 0 replies; 14+ messages in thread
From: Patrick Palka @ 2015-06-26 13:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: Patrick Palka

On Fri, Jun 19, 2015 at 7:33 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> This patch implements the new option "history remove-duplicates", which
> controls the removal of duplicate history entries ("off" by default).
>
> The motivation for this option is to be able to reduce the prevalence of
> basic commands such as "up" and "down" in the history file.  These
> common commands crowd out more unique commands in the history file (when
> the history file has a fixed size), and they make navigation of the
> history file via ^P, ^N and ^R more inconvenient.
>
> The option takes an integer denoting the number of history entries to
> look back at for a history entry that is a duplicate of the latest one.
> "history remove-duplicates 1" is equivalent to bash's ignoredups option,
> and "history remove-duplicates unlimited" is equivalent to bash's
> erasedups option.
>
> [ I decided to go with this integer approach instead of a tri-state enum
>   because it's slightly more flexible and seemingly more intuitive than
>   leave/erase/ignore.  ]
>
> gdb/ChangeLog:
>
>         * NEWS: Mention the new option "history remove-duplicates".
>         * top.c (history_remove_duplicates): New static variable.
>         (show_history_remove_duplicates): New static function.
>         (gdb_add_history): Conditionally remove duplicate history
>         entries.
>         (init_main): Add "history remove-duplicates" option.
>
> gdb/doc/ChangeLog:
>
>         * gdb.texinfo (Command History): Document the new option
>         "history remove-duplicates".
>
> gdb/testsuite/ChangeLog:
>
>         * gdb.base/history-duplicates.exp: New test.
> ---
>  gdb/NEWS                                      |   4 +
>  gdb/doc/gdb.texinfo                           |  15 +++
>  gdb/testsuite/gdb.base/history-duplicates.exp | 129 ++++++++++++++++++++++++++
>  gdb/top.c                                     |  66 ++++++++++++-
>  4 files changed, 213 insertions(+), 1 deletion(-)
>  create mode 100644 gdb/testsuite/gdb.base/history-duplicates.exp
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 3ec5851..6d29004 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -145,6 +145,10 @@ show max-completions
>    to avoid generating large completion lists, the computation of
>    which can cause the debugger to become temporarily unresponsive.
>
> +set history remove-duplicates
> +show history remove-duplicates
> +  Control the removal of duplicate history entries.
> +
>  maint set symbol-cache-size
>  maint show symbol-cache-size
>    Control the size of the symbol cache.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index c9a532a..c5885eb 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -22644,6 +22644,21 @@ to 256 if this variable is not set.  Non-numeric values of @env{GDBHISTSIZE}
>  are ignored.  If @var{size} is @code{unlimited} or if @env{GDBHISTSIZE} is
>  either a negative number or the empty string, then the number of commands
>  @value{GDBN} keeps in the history list is unlimited.
> +
> +@cindex remove duplicate history
> +@kindex set history remove-duplicates
> +@item set history remove-duplicates @var{size}
> +@itemx set history remove-duplicates unlimited
> +Control the removal of duplicate history entries in the command history list.
> +If @var{size} is non-zero, @value{GDBN} will look back at the last @var{size}
> +history entries and remove the first entry that is a duplicate of the current
> +entry being added to the command history list.  If @var{size} is
> +@code{unlimited} then this lookbehind is unbounded.  If @var{size} is 0, then
> +removal of duplicate history entries is disabled.
> +
> +Only history entries added during the current session are considered for
> +removal.  This option is set to 0 by default.
> +
>  @end table
>
>  History expansion assigns special meaning to the character @kbd{!}.
> diff --git a/gdb/testsuite/gdb.base/history-duplicates.exp b/gdb/testsuite/gdb.base/history-duplicates.exp
> new file mode 100644
> index 0000000..f88607f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/history-duplicates.exp
> @@ -0,0 +1,129 @@
> +# Copyright 2015 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# This file is part of the gdb testsuite.
> +
> +# Test the operation of the "history remove-duplicates" option.
> +
> +
> +# Check that the previous history entry is ENTRY.
> +
> +proc check_prev_history_entry { entry { test_suffix "" } } {
> +    set test "history entry is $entry"
> +    if { $test_suffix != "" } {
> +        append test " $test_suffix"
> +    }
> +
> +    # Send ^P followed by ^L.
> +    send_gdb "\x10\x0c"
> +
> +    gdb_expect {
> +       -re $entry {
> +           pass $test
> +       }
> +       timeout {
> +           fail $test
> +       }
> +    }
> +}
> +
> +# By default the option is set to 0.
> +gdb_exit
> +gdb_start
> +gdb_test "show history remove-duplicates" "is 0\\."
> +
> +# Test the "unlimited" setting.
> +with_test_prefix "remove-duplicates=unlimited" {
> +    gdb_exit
> +    gdb_start
> +    gdb_test "set history remove-duplicates unlimited"
> +
> +    gdb_test "print 0"
> +    gdb_test "print 1"
> +    gdb_test "print 2"
> +    gdb_test "print 1"
> +    gdb_test "print 1"
> +    gdb_test "print 2"
> +    gdb_test "print 3"
> +    gdb_test "print 3"
> +    gdb_test "print 4"
> +    gdb_test "print 1"
> +    gdb_test "print 2"
> +    gdb_test "print 3"
> +    gdb_test "print 4"
> +
> +    check_prev_history_entry "print 4"
> +    check_prev_history_entry "print 3"
> +    check_prev_history_entry "print 2"
> +    check_prev_history_entry "print 1"
> +    check_prev_history_entry "print 0"
> +}
> +
> +
> +# Test the "1" setting.
> +with_test_prefix "remove-duplicates=1" {
> +    gdb_exit
> +    gdb_start
> +    gdb_test "set history remove-duplicates 1"
> +
> +    gdb_test "print 0"
> +    gdb_test "print 1"
> +    gdb_test "print 0"
> +    gdb_test "print 2"
> +    gdb_test "print 2"
> +    gdb_test "print 1"
> +
> +    check_prev_history_entry "print 1"
> +    check_prev_history_entry "print 2"
> +    check_prev_history_entry "print 0"
> +    check_prev_history_entry "print 1" "(again)"
> +    check_prev_history_entry "print 0" "(again)"
> +}
> +
> +
> +# Test the "0" setting.
> +with_test_prefix "remove-duplicates=0" {
> +    gdb_exit
> +    gdb_start
> +    gdb_test "set history remove-duplicates 0"
> +
> +    gdb_test "print 0"
> +    gdb_test "print 0"
> +    gdb_test "print 1"
> +    gdb_test "print 1"
> +
> +    check_prev_history_entry "print 1"
> +    check_prev_history_entry "print 1" "(again)"
> +    check_prev_history_entry "print 0"
> +    check_prev_history_entry "print 0" "(again)"
> +}
> +
> +
> +# Test the "2" setting.
> +with_test_prefix "remove-duplicates=2" {
> +    gdb_exit
> +    gdb_start
> +    gdb_test "set history remove-duplicates 2"
> +
> +    gdb_test "print 1"
> +    gdb_test "print 2"
> +    gdb_test "print 0"
> +    gdb_test "print 2"
> +    gdb_test "print 0"
> +
> +    check_prev_history_entry "print 0"
> +    check_prev_history_entry "print 2"
> +    check_prev_history_entry "print 1"
> +}
> diff --git a/gdb/top.c b/gdb/top.c
> index 6ae84ab..5114c2e 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -698,6 +698,20 @@ show_history_size (struct ui_file *file, int from_tty,
>                     value);
>  }
>
> +/* Variable associated with the "history remove-duplicates" option.
> +   The value -1 means unlimited.  */
> +static int history_remove_duplicates = 0;
> +
> +static void
> +show_history_remove_duplicates (struct ui_file *file, int from_tty,
> +                               struct cmd_list_element *c, const char *value)
> +{
> +  fprintf_filtered (file,
> +                   _("The number of history entries to look back at for "
> +                     "duplicates is %s.\n"),
> +                   value);
> +}
> +
>  static char *history_filename;
>  static void
>  show_history_filename (struct ui_file *file, int from_tty,
> @@ -897,8 +911,43 @@ static int command_count = 0;
>  void
>  gdb_add_history (const char *command)
>  {
> -  add_history (command);
>    command_count++;
> +
> +  if (history_remove_duplicates != 0)
> +    {
> +      int lookbehind;
> +      int lookbehind_threshold;
> +
> +      /* The lookbehind threshold for finding a duplicate history entry is
> +        bounded by command_count because we can't meaningfully delete
> +        history entries that are already stored in the history file since
> +        the history file is appended to.  */
> +      if (history_remove_duplicates == -1
> +         || history_remove_duplicates > command_count)
> +       lookbehind_threshold = command_count;
> +      else
> +       lookbehind_threshold = history_remove_duplicates;
> +
> +      using_history ();
> +      for (lookbehind = 0; lookbehind < lookbehind_threshold; lookbehind++)
> +       {
> +         HIST_ENTRY *temp = previous_history ();
> +
> +         if (temp == NULL)
> +           break;
> +
> +         if (strcmp (temp->line, command) == 0)
> +           {
> +             HIST_ENTRY *prev = remove_history (where_history ());
> +             command_count--;
> +             free_history_entry (prev);
> +             break;
> +           }
> +       }
> +      using_history ();
> +    }
> +
> +  add_history (command);
>  }
>
>  /* Safely append new history entries to the history file in a corruption-free
> @@ -1872,6 +1921,21 @@ variable \"GDBHISTSIZE\", or to 256 if this variable is not set."),
>                             show_history_size,
>                             &sethistlist, &showhistlist);
>
> +  add_setshow_zuinteger_unlimited_cmd ("remove-duplicates", no_class,
> +                                      &history_remove_duplicates, _("\
> +Set how far back in history to look for and remove duplicate entries."), _("\
> +Show how far back in history to look for and remove duplicate entries."), _("\
> +If set to a non-zero value N, GDB will look back at the last N history entries\n\
> +and remove the first history entry that is a duplicate of the most recent\n\
> +entry, each time a new history entry is added.\n\
> +If set to \"unlimited\", this lookbehind is unbounded.\n\
> +Only history entries added during this session are considered for removal.\n\
> +If set to 0, removal of duplicate history entries is disabled.\n\
> +By default this option is set to 0."),
> +                          NULL,
> +                          show_history_remove_duplicates,
> +                          &sethistlist, &showhistlist);
> +
>    add_setshow_filename_cmd ("filename", no_class, &history_filename, _("\
>  Set the filename in which to record the command history"), _("\
>  Show the filename in which to record the command history"), _("\
> --
> 2.4.4.410.g43ed522.dirty
>

Ping.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-19 23:33       ` Patrick Palka
  2015-06-20  6:52         ` Eli Zaretskii
  2015-06-26 13:35         ` Patrick Palka
@ 2015-06-26 14:18         ` Pedro Alves
  2015-06-26 14:30           ` Patrick Palka
  2015-06-26 15:12         ` Patrick Palka
  3 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2015-06-26 14:18 UTC (permalink / raw)
  To: Patrick Palka, gdb-patches

On 06/20/2015 12:33 AM, Patrick Palka wrote:

> index c9a532a..c5885eb 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -22644,6 +22644,21 @@ to 256 if this variable is not set.  Non-numeric values of @env{GDBHISTSIZE}
>  are ignored.  If @var{size} is @code{unlimited} or if @env{GDBHISTSIZE} is
>  either a negative number or the empty string, then the number of commands
>  @value{GDBN} keeps in the history list is unlimited.
> +
> +@cindex remove duplicate history
> +@kindex set history remove-duplicates
> +@item set history remove-duplicates @var{size}
> +@itemx set history remove-duplicates unlimited
> +Control the removal of duplicate history entries in the command history list.
> +If @var{size} is non-zero, @value{GDBN} will look back at the last @var{size}

Somehow, "size" here sounds a bit confusing to me.  This not about
that size of the duplicates.  :-)  And it's not about when the history
gets to a certain size either.  I'd suggest s/size/count/, or s/size/lookbehind/
or some such.

> +history entries and remove the first entry that is a duplicate of the current
> +entry being added to the command history list.  If @var{size} is
> +@code{unlimited} then this lookbehind is unbounded.  If @var{size} is 0, then
> +removal of duplicate history entries is disabled.
> +
> +Only history entries added during the current session are considered for
> +removal.  This option is set to 0 by default.
> +



> +proc check_prev_history_entry { entry { test_suffix "" } } {
> +    set test "history entry is $entry"
> +    if { $test_suffix != "" } {
> +        append test " $test_suffix"
> +    }
> +
> +    # Send ^P followed by ^L.
> +    send_gdb "\x10\x0c"

I have a feeling this may cause problems, but we'll see.

> +
> +    gdb_expect {
> +	-re $entry {
> +	    pass $test
> +	}
> +	timeout {
> +	    fail $test
> +	}
> +    }
> +}
> +
> +# By default the option is set to 0.
> +gdb_exit
> +gdb_start
> +gdb_test "show history remove-duplicates" "is 0\\."
> +
> +# Test the "unlimited" setting.
> +with_test_prefix "remove-duplicates=unlimited" {
> +    gdb_exit
> +    gdb_start
> +    gdb_test "set history remove-duplicates unlimited"
> +
> +    gdb_test "print 0"
> +    gdb_test "print 1"
> +    gdb_test "print 2"
> +    gdb_test "print 1"
> +    gdb_test "print 1"
> +    gdb_test "print 2"
> +    gdb_test "print 3"
> +    gdb_test "print 3"
> +    gdb_test "print 4"
> +    gdb_test "print 1"
> +    gdb_test "print 2"
> +    gdb_test "print 3"
> +    gdb_test "print 4"

Duplicate test names here.  This should catch all:

 $ make check RUNTESTFLAGS="history-duplicates.exp"
 $ grep "PASS" testsuite/gdb.sum | sort | uniq -c | sort -n

Otherwise this looks good to me.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-26 14:18         ` Pedro Alves
@ 2015-06-26 14:30           ` Patrick Palka
  2015-06-26 14:50             ` Pedro Alves
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Palka @ 2015-06-26 14:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

On Fri, Jun 26, 2015 at 10:18 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/20/2015 12:33 AM, Patrick Palka wrote:
>
>> index c9a532a..c5885eb 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -22644,6 +22644,21 @@ to 256 if this variable is not set.  Non-numeric values of @env{GDBHISTSIZE}
>>  are ignored.  If @var{size} is @code{unlimited} or if @env{GDBHISTSIZE} is
>>  either a negative number or the empty string, then the number of commands
>>  @value{GDBN} keeps in the history list is unlimited.
>> +
>> +@cindex remove duplicate history
>> +@kindex set history remove-duplicates
>> +@item set history remove-duplicates @var{size}
>> +@itemx set history remove-duplicates unlimited
>> +Control the removal of duplicate history entries in the command history list.
>> +If @var{size} is non-zero, @value{GDBN} will look back at the last @var{size}
>
> Somehow, "size" here sounds a bit confusing to me.  This not about
> that size of the duplicates.  :-)  And it's not about when the history
> gets to a certain size either.  I'd suggest s/size/count/, or s/size/lookbehind/
> or some such.

Ah yeah, I had already changed that variable name locally to "count".

>
>> +history entries and remove the first entry that is a duplicate of the current
>> +entry being added to the command history list.  If @var{size} is
>> +@code{unlimited} then this lookbehind is unbounded.  If @var{size} is 0, then
>> +removal of duplicate history entries is disabled.
>> +
>> +Only history entries added during the current session are considered for
>> +removal.  This option is set to 0 by default.
>> +
>
>
>
>> +proc check_prev_history_entry { entry { test_suffix "" } } {
>> +    set test "history entry is $entry"
>> +    if { $test_suffix != "" } {
>> +        append test " $test_suffix"
>> +    }
>> +
>> +    # Send ^P followed by ^L.
>> +    send_gdb "\x10\x0c"
>
> I have a feeling this may cause problems, but we'll see.

Hopefully not because I copied this "technique" of traversing through
history from readline.exp.  I considered using looking at the output
of "show commands" but doing ^P seems easier and more directly tests
the code paths we want to test.

>
>> +
>> +    gdb_expect {
>> +     -re $entry {
>> +         pass $test
>> +     }
>> +     timeout {
>> +         fail $test
>> +     }
>> +    }
>> +}
>> +
>> +# By default the option is set to 0.
>> +gdb_exit
>> +gdb_start
>> +gdb_test "show history remove-duplicates" "is 0\\."
>> +
>> +# Test the "unlimited" setting.
>> +with_test_prefix "remove-duplicates=unlimited" {
>> +    gdb_exit
>> +    gdb_start
>> +    gdb_test "set history remove-duplicates unlimited"
>> +
>> +    gdb_test "print 0"
>> +    gdb_test "print 1"
>> +    gdb_test "print 2"
>> +    gdb_test "print 1"
>> +    gdb_test "print 1"
>> +    gdb_test "print 2"
>> +    gdb_test "print 3"
>> +    gdb_test "print 3"
>> +    gdb_test "print 4"
>> +    gdb_test "print 1"
>> +    gdb_test "print 2"
>> +    gdb_test "print 3"
>> +    gdb_test "print 4"
>
> Duplicate test names here.  This should catch all:

I could use some kind of loop over a list here I think, prefixing each
test name with the index of the list.

>
>  $ make check RUNTESTFLAGS="history-duplicates.exp"
>  $ grep "PASS" testsuite/gdb.sum | sort | uniq -c | sort -n
>
> Otherwise this looks good to me.

Thanks a lot for reviewing!

>
> Thanks,
> Pedro Alves
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Add option to remove duplicate command history entries
  2015-06-26 14:30           ` Patrick Palka
@ 2015-06-26 14:50             ` Pedro Alves
  0 siblings, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2015-06-26 14:50 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gdb-patches

On 06/26/2015 03:30 PM, Patrick Palka wrote:

>>> +    gdb_test "print 0"
>>> +    gdb_test "print 1"
>>> +    gdb_test "print 2"
>>> +    gdb_test "print 1"
>>> +    gdb_test "print 1"
>>> +    gdb_test "print 2"
>>> +    gdb_test "print 3"
>>> +    gdb_test "print 3"
>>> +    gdb_test "print 4"
>>> +    gdb_test "print 1"
>>> +    gdb_test "print 2"
>>> +    gdb_test "print 3"
>>> +    gdb_test "print 4"
>>
>> Duplicate test names here.  This should catch all:
> 
> I could use some kind of loop over a list here I think, prefixing each
> test name with the index of the list.

Yeah, that would be fine.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] Add option to remove duplicate command history entries
  2015-06-19 23:33       ` Patrick Palka
                           ` (2 preceding siblings ...)
  2015-06-26 14:18         ` Pedro Alves
@ 2015-06-26 15:12         ` Patrick Palka
  3 siblings, 0 replies; 14+ messages in thread
From: Patrick Palka @ 2015-06-26 15:12 UTC (permalink / raw)
  To: gdb-patches; +Cc: Patrick Palka

For posterity, this is what I committed.

---
 gdb/ChangeLog                                 |   9 ++
 gdb/NEWS                                      |   4 +
 gdb/doc/ChangeLog                             |   5 ++
 gdb/doc/gdb.texinfo                           |  15 ++++
 gdb/testsuite/ChangeLog                       |   4 +
 gdb/testsuite/gdb.base/history-duplicates.exp | 117 ++++++++++++++++++++++++++
 gdb/top.c                                     |  66 ++++++++++++++-
 7 files changed, 219 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.base/history-duplicates.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 696a593..8cd835d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2015-06-26  Patrick Palka  <patrick@parcs.ath.cx>
 
+	* NEWS: Mention the new option "history remove-duplicates".
+	* top.c (history_remove_duplicates): New static variable.
+	(show_history_remove_duplicates): New static function.
+	(gdb_add_history): Conditionally remove duplicate history
+	entries.
+	(init_main): Add "history remove-duplicates" option.
+
+2015-06-26  Patrick Palka  <patrick@parcs.ath.cx>
+
 	* tui/tui-win.c (focus_completer): New static function.
 	(_initialize_tui_win): Set the completion function of the
 	"focus" command to focus_completer.
diff --git a/gdb/NEWS b/gdb/NEWS
index 3ec5851..6d29004 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -145,6 +145,10 @@ show max-completions
   to avoid generating large completion lists, the computation of
   which can cause the debugger to become temporarily unresponsive.
 
+set history remove-duplicates
+show history remove-duplicates
+  Control the removal of duplicate history entries.
+
 maint set symbol-cache-size
 maint show symbol-cache-size
   Control the size of the symbol cache.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 6d86750..e0beb7f 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2015-06-26  Patrick Palka  <patrick@parcs.ath.cx>
+
+	* gdb.texinfo (Command History): Document the new option
+	"history remove-duplicates".
+
 2015-06-19  Doug Evans  <dje@google.com>
 
 	* stabs.texinfo (ELF Linker Relocation): Mention Sun stabs is no
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c9a532a..20a9563 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22644,6 +22644,21 @@ to 256 if this variable is not set.  Non-numeric values of @env{GDBHISTSIZE}
 are ignored.  If @var{size} is @code{unlimited} or if @env{GDBHISTSIZE} is
 either a negative number or the empty string, then the number of commands
 @value{GDBN} keeps in the history list is unlimited.
+
+@cindex remove duplicate history
+@kindex set history remove-duplicates
+@item set history remove-duplicates @var{count}
+@itemx set history remove-duplicates unlimited
+Control the removal of duplicate history entries in the command history list.
+If @var{count} is non-zero, @value{GDBN} will look back at the last @var{count}
+history entries and remove the first entry that is a duplicate of the current
+entry being added to the command history list.  If @var{count} is
+@code{unlimited} then this lookbehind is unbounded.  If @var{count} is 0, then
+removal of duplicate history entries is disabled.
+
+Only history entries added during the current session are considered for
+removal.  This option is set to 0 by default.
+
 @end table
 
 History expansion assigns special meaning to the character @kbd{!}.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index ce792be..af7f131 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
 2015-06-26  Patrick Palka  <patrick@parcs.ath.cx>
 
+	* gdb.base/history-duplicates.exp: New test.
+
+2015-06-26  Patrick Palka  <patrick@parcs.ath.cx>
+
 	* gdb.base/completion.exp: Test the completion of the "focus"
 	command.
 
diff --git a/gdb/testsuite/gdb.base/history-duplicates.exp b/gdb/testsuite/gdb.base/history-duplicates.exp
new file mode 100644
index 0000000..11bb1ed
--- /dev/null
+++ b/gdb/testsuite/gdb.base/history-duplicates.exp
@@ -0,0 +1,117 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test the operation of the "history remove-duplicates" option.
+
+
+# Check that the previous history entry is ENTRY.
+
+proc check_prev_history_entry { entry { test_suffix "" } } {
+    set test_name "history entry is $entry"
+    if { $test_suffix != "" } {
+	append test_name " $test_suffix"
+    }
+
+    # Send ^P followed by ^L.
+    send_gdb "\x10\x0c"
+
+    gdb_expect {
+	-re $entry {
+	    pass $test_name
+	}
+	timeout {
+	    fail $test_name
+	}
+    }
+}
+
+# Foreach element ELT in THINGS, run the command "print $ELT", making sure that
+# each invocation of "print" has a unique test name.
+
+proc run_print_on_each_thing { things } {
+    set index 0
+
+    foreach thing $things {
+	gdb_test "print $thing" "" "printing $thing (item #$index)"
+	incr index
+    }
+}
+
+# By default the option is set to 0.
+gdb_exit
+gdb_start
+gdb_test "show history remove-duplicates" "is 0\\."
+
+# Test the "unlimited" setting.
+with_test_prefix "remove-duplicates=unlimited" {
+    gdb_exit
+    gdb_start
+    gdb_test "set history remove-duplicates unlimited"
+
+    run_print_on_each_thing { 0 1 2 1 1 2 3 3 4 1 2 3 4 }
+
+    check_prev_history_entry "print 4"
+    check_prev_history_entry "print 3"
+    check_prev_history_entry "print 2"
+    check_prev_history_entry "print 1"
+    check_prev_history_entry "print 0"
+}
+
+
+# Test the "1" setting.
+with_test_prefix "remove-duplicates=1" {
+    gdb_exit
+    gdb_start
+    gdb_test "set history remove-duplicates 1"
+
+    run_print_on_each_thing { 0 1 0 2 2 1 }
+
+    check_prev_history_entry "print 1"
+    check_prev_history_entry "print 2"
+    check_prev_history_entry "print 0"
+    check_prev_history_entry "print 1" "(again)"
+    check_prev_history_entry "print 0" "(again)"
+}
+
+
+# Test the "0" setting.
+with_test_prefix "remove-duplicates=0" {
+    gdb_exit
+    gdb_start
+    gdb_test "set history remove-duplicates 0"
+
+    run_print_on_each_thing { 0 0 1 1 }
+
+    check_prev_history_entry "print 1"
+    check_prev_history_entry "print 1" "(again)"
+    check_prev_history_entry "print 0"
+    check_prev_history_entry "print 0" "(again)"
+}
+
+
+# Test the "2" setting.
+with_test_prefix "remove-duplicates=2" {
+    gdb_exit
+    gdb_start
+    gdb_test "set history remove-duplicates 2"
+
+    run_print_on_each_thing { 1 2 0 2 0 }
+
+    check_prev_history_entry "print 0"
+    check_prev_history_entry "print 2"
+    check_prev_history_entry "print 1"
+}
diff --git a/gdb/top.c b/gdb/top.c
index 77fe096..01fddd2 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -698,6 +698,20 @@ show_history_size (struct ui_file *file, int from_tty,
 		    value);
 }
 
+/* Variable associated with the "history remove-duplicates" option.
+   The value -1 means unlimited.  */
+static int history_remove_duplicates = 0;
+
+static void
+show_history_remove_duplicates (struct ui_file *file, int from_tty,
+				struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (file,
+		    _("The number of history entries to look back at for "
+		      "duplicates is %s.\n"),
+		    value);
+}
+
 static char *history_filename;
 static void
 show_history_filename (struct ui_file *file, int from_tty,
@@ -897,8 +911,43 @@ static int command_count = 0;
 void
 gdb_add_history (const char *command)
 {
-  add_history (command);
   command_count++;
+
+  if (history_remove_duplicates != 0)
+    {
+      int lookbehind;
+      int lookbehind_threshold;
+
+      /* The lookbehind threshold for finding a duplicate history entry is
+	 bounded by command_count because we can't meaningfully delete
+	 history entries that are already stored in the history file since
+	 the history file is appended to.  */
+      if (history_remove_duplicates == -1
+	  || history_remove_duplicates > command_count)
+	lookbehind_threshold = command_count;
+      else
+	lookbehind_threshold = history_remove_duplicates;
+
+      using_history ();
+      for (lookbehind = 0; lookbehind < lookbehind_threshold; lookbehind++)
+	{
+	  HIST_ENTRY *temp = previous_history ();
+
+	  if (temp == NULL)
+	    break;
+
+	  if (strcmp (temp->line, command) == 0)
+	    {
+	      HIST_ENTRY *prev = remove_history (where_history ());
+	      command_count--;
+	      free_history_entry (prev);
+	      break;
+	    }
+	}
+      using_history ();
+    }
+
+  add_history (command);
 }
 
 /* Safely append new history entries to the history file in a corruption-free
@@ -1880,6 +1929,21 @@ variable \"GDBHISTSIZE\", or to 256 if this variable is not set."),
 			    show_history_size,
 			    &sethistlist, &showhistlist);
 
+  add_setshow_zuinteger_unlimited_cmd ("remove-duplicates", no_class,
+				       &history_remove_duplicates, _("\
+Set how far back in history to look for and remove duplicate entries."), _("\
+Show how far back in history to look for and remove duplicate entries."), _("\
+If set to a nonzero value N, GDB will look back at the last N history entries\n\
+and remove the first history entry that is a duplicate of the most recent\n\
+entry, each time a new history entry is added.\n\
+If set to \"unlimited\", this lookbehind is unbounded.\n\
+Only history entries added during this session are considered for removal.\n\
+If set to 0, removal of duplicate history entries is disabled.\n\
+By default this option is set to 0."),
+			   NULL,
+			   show_history_remove_duplicates,
+			   &sethistlist, &showhistlist);
+
   add_setshow_filename_cmd ("filename", no_class, &history_filename, _("\
 Set the filename in which to record the command history"), _("\
 Show the filename in which to record the command history"), _("\
-- 
2.5.0.rc0.5.g91e10c5.dirty

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-06-26 15:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-04 16:22 [PATCH] Add option to remove duplicate command history entries Patrick Palka
2015-06-04 16:47 ` Eli Zaretskii
2015-06-04 18:54   ` Patrick Palka
2015-06-04 19:25     ` Eli Zaretskii
2015-06-09 18:10 ` Pedro Alves
2015-06-09 18:41   ` Patrick Palka
2015-06-10 15:12     ` Pedro Alves
2015-06-19 23:33       ` Patrick Palka
2015-06-20  6:52         ` Eli Zaretskii
2015-06-26 13:35         ` Patrick Palka
2015-06-26 14:18         ` Pedro Alves
2015-06-26 14:30           ` Patrick Palka
2015-06-26 14:50             ` Pedro Alves
2015-06-26 15:12         ` Patrick Palka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).