Re: [PATCH] Add option to remove duplicate command history entries

[Pedro Alves <palves@redhat.com>]

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