Re: [PATCH v3 06/19] Implement completion limiting for condition_completer.

[Doug Evans <xdje42 at gmail dot com>]

Keith Seitz <keiths@redhat.com> writes:
> There are no changes from the last revision.
>
> --
>
> This patch converts the condition completer to use add_completion.
> A side-effect of this is the similar conversion of complete_internalvar.
>
> Tests have been added to exercise this new behavior.
>
> gdb/ChangeLog
>
> 	* breakpoint.c (condition_completer): Pass completer_data to
> 	complete_internalvar.
> 	Use add_completion.
> 	* value.c: Include completer.h.
> 	(complete_internalvar): Add completer_data argument.
> 	Use add_completion.
> 	* value.h (complete_internalvar): Add completer_data argument.
>
> gdb/testsuite/ChangeLog
>
> 	* gdb.base/condbreak.exp (test_completion): New procedure.
> 	Add more completion tests, with and without limiting.

====
One nit.
Ok with that fixed.

> ---
>  gdb/breakpoint.c                     |    8 +++-
>  gdb/testsuite/gdb.base/condbreak.exp |   70 ++++++++++++++++++++++++++++++++++
>  gdb/value.c                          |    9 ++--
>  gdb/value.h                          |    3 +
>  4 files changed, 83 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 70569df..24243c4 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -1052,7 +1052,7 @@ condition_completer (struct completer_data *cdata,
>  	  /* We don't support completion of history indices.  */
>  	  if (isdigit (text[1]))
>  	    return NULL;
> -	  return complete_internalvar (&text[1]);
> +	  return complete_internalvar (cdata, &text[1]);
>  	}
>  
>        /* We're completing the breakpoint number.  */
> @@ -1065,7 +1065,11 @@ condition_completer (struct completer_data *cdata,
>  	  xsnprintf (number, sizeof (number), "%d", b->number);
>  
>  	  if (strncmp (number, text, len) == 0)
> -	    VEC_safe_push (char_ptr, result, xstrdup (number));
> +	    {
> +	      if (add_completion (cdata, &result, number, NULL, NULL)
> +		  == ADD_COMPLETION_MAX_REACHED)
> +		break;
> +	    }
>  	}
>  
>        return result;
> diff --git a/gdb/testsuite/gdb.base/condbreak.exp b/gdb/testsuite/gdb.base/condbreak.exp
> index fa40a5f..5630ede 100644
> --- a/gdb/testsuite/gdb.base/condbreak.exp
> +++ b/gdb/testsuite/gdb.base/condbreak.exp
> @@ -246,3 +246,73 @@ gdb_test "complete cond 1" "cond 1"
>  gdb_test "set variable \$var = 1"
>  gdb_test "complete cond \$v" "cond \\\$var"
>  gdb_test "complete cond 1 values\[0\].a" "cond 1 values.0..a_field"
> +
> +# Test non-trivial completion and completion-limiting
> +
> +# Delete all breakpoints and create a bunch of new ones.
> +delete_breakpoints
> +for {set i 0} {$i < 20} {incr i} {
> +    with_test_prefix "set breakpoint $i" {
> +	gdb_breakpoint "factorial"
> +    }
> +}
> +
> +# While the completer function does traverse breakpoints in the order
> +# they were created, don't assume that is required for the test.
> +# We only count the number of completions found.  In this case,
> +# this test will create breakpoints 9-19, giving "complete cond 1"
> +# ten total completion possibilities.
> +
> +# A convenience procedure to automate test completion lists.
> +proc test_completion {cmd exp total {limit 0}} {
> +    global gdb_prompt
> +
> +    if {$limit} {
> +	set end "\\\*\\\*\\\* List may be truncated, "
> +	append end "max-completions reached\\\. \\\*\\\*\\\*\r\n"
> +	set testname "limit '$cmd'"
> +    } else {
> +	set end ""
> +	set testname $cmd
> +    }
> +
> +    set seen 0
> +    gdb_test_multiple $cmd $testname {
> +	"$cmd\r\n" { exp_continue }
> +
> +	-re "cond $exp\[0-9\]+\r\n" {
> +	    incr seen
> +	    exp_continue
> +	}
> +
> +	-re ".*$end$gdb_prompt $" {
> +	    if {$seen == $total} {
> +		pass $testname
> +	    } else {
> +		fail "$testname ($seen/$total)"
> +	    }
> +	}
> +    }
> +}

====
Nice test.

> +
> +# Test completion of breakpoint number.
> +with_test_prefix "completion test:" {
> +    test_completion "complete cond 1" "1" 10
> +}
> +
> +# Test completion of breakpoint number using internal variable.
> +for {set i 0} {$i < 10} {incr i} {
> +    gdb_test_no_output "set variable \$var_bp_$i = $i"
> +}
> +
> +test_completion "complete cond \$var_bp" "\\\$var_bp_" 10
> +
> +# Run the above tests with completion limiting.
> +set max_completions 4
> +gdb_test_no_output "set max-completions $max_completions"
> +
> +with_test_prefix "completion test:" {
> +    test_completion "complete cond 1" "1" $max_completions 1
> +}
> +
> +test_completion "complete cond \$var_bp" "\\\$var_bp_" $max_completions 1
> diff --git a/gdb/value.c b/gdb/value.c
> index 63ee94d..22d392a 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -40,6 +40,7 @@
>  #include "tracepoint.h"
>  #include "cp-abi.h"
>  #include "user-regs.h"
> +#include "completer.h"
>  
>  /* Prototypes for exported functions.  */
>  
> @@ -2062,7 +2063,7 @@ lookup_only_internalvar (const char *name)
>     were found.  */
>  
>  VEC (char_ptr) *
> -complete_internalvar (const char *name)
> +complete_internalvar (struct completer_data *cdata, const char *name)
>  {
>    VEC (char_ptr) *result = NULL;
>    struct internalvar *var;
> @@ -2073,9 +2074,9 @@ complete_internalvar (const char *name)
>    for (var = internalvars; var; var = var->next)
>      if (strncmp (var->name, name, len) == 0)
>        {
> -	char *r = xstrdup (var->name);
> -
> -	VEC_safe_push (char_ptr, result, r);
> +	if (add_completion (cdata, &result, var->name, NULL, NULL)
> +	    == ADD_COMPLETION_MAX_REACHED)
> +	  break;
>        }
>  
>    return result;
> diff --git a/gdb/value.h b/gdb/value.h
> index 82deaf2..03df1b7 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -876,7 +876,8 @@ extern struct internalvar *lookup_only_internalvar (const char *name);
>  
>  extern struct internalvar *create_internalvar (const char *name);
>  
> -extern VEC (char_ptr) *complete_internalvar (const char *name);
> +extern VEC (char_ptr) *complete_internalvar (struct completer_data *cdata,
> +					     const char *name);

====
Add a forward decl for struct completer_cdata.

>  
>  /* An internalvar can be dynamically computed by supplying a vector of
>     function pointers to perform various operations.  */