Re: [PATCH v2 1/2] MI: extract command completion logic from complete_command()

[Pedro Alves <palves@redhat.com>]

Hi Jan,

Some formatting nits below.

On 01/28/2019 12:41 PM, Jan Vrany wrote:
> Extract completion logic from CLI complete_command() into a new
> helper function complete().
> 
> gdb/Changelog:
> 
> 	* completer.h (complete): New function.
> 	* completer.c (complete): Likewise.
> 	* cli/cli-cmds.c: (complete_command): Update to use new complete()
> 	function defined in completer.h

Missing period after "in completer.h".

> ---
>  gdb/ChangeLog      |  7 +++++++
>  gdb/cli/cli-cmds.c | 32 ++------------------------------
>  gdb/completer.c    | 34 ++++++++++++++++++++++++++++++++++
>  gdb/completer.h    |  8 ++++++++
>  4 files changed, 51 insertions(+), 30 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 8e03dbf883..17e29a7807 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2019-01-24  Jan Vrany  <jan.vrany@fit.cvut.cz>
> +
> +	* completer.h (complete): New function.
> +	* completer.c (complete): Likewise.
> +	* cli/cli-cmds.c: (complete_command): Update to use new complete() 
> +	function defined in completer.h
> +
>  2019-01-22  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
>  
>  	* event-top.c (handle_line_of_input): use unique_xmalloc_ptr for
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 57cfad441c..f2c2311e68 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -243,41 +243,13 @@ complete_command (const char *arg, int from_tty)
>    if (arg == NULL)
>      arg = "";
>  
> -  completion_tracker tracker_handle_brkchars;
> -  completion_tracker tracker_handle_completions;
> -  completion_tracker *tracker;
> -
>    int quote_char = '\0';
>    const char *word;
> -
> -  TRY
> -    {
> -      word = completion_find_completion_word (tracker_handle_brkchars,
> -					      arg, &quote_char);
> -
> -      /* Completers that provide a custom word point in the
> -	 handle_brkchars phase also compute their completions then.
> -	 Completers that leave the completion word handling to readline
> -	 must be called twice.  */
> -      if (tracker_handle_brkchars.use_custom_word_point ())
> -	tracker = &tracker_handle_brkchars;
> -      else
> -	{
> -	  complete_line (tracker_handle_completions, word, arg, strlen (arg));
> -	  tracker = &tracker_handle_completions;
> -	}
> -    }
> -  CATCH (ex, RETURN_MASK_ALL)
> -    {
> -      return;
> -    }
> -  END_CATCH
> +  
> +  completion_result result = complete (arg, &word, &quote_char);
>  
>    std::string arg_prefix (arg, word - arg);
>  
> -  completion_result result
> -    = tracker->build_completion_result (word, word - arg, strlen (arg));
> -
>    if (result.number_matches != 0)
>      {
>        if (result.number_matches == 1)
> diff --git a/gdb/completer.c b/gdb/completer.c
> index fed815a53c..2f94932184 100644
> --- a/gdb/completer.c
> +++ b/gdb/completer.c
> @@ -1615,6 +1615,40 @@ make_completion_match_str (gdb::unique_xmalloc_ptr<char> &&match_name,
>    return gdb::unique_xmalloc_ptr<char> (newobj);
>  }
>  
> +completion_result
> +complete (const char *line, char const **word, int *quote_char)

Add:

/* See complete.h.  */

Make sure to leave an empty line between comment and function.


> +{
> +  completion_tracker tracker_handle_brkchars;
> +  completion_tracker tracker_handle_completions;
> +  completion_tracker *tracker;
> +  
> +  TRY
> +    {
> +      *word = completion_find_completion_word (tracker_handle_brkchars,
> +					      line, quote_char);
> +
> +      /* Completers that provide a custom word point in the
> +	 handle_brkchars phase also compute their completions then.
> +	 Completers that leave the completion word handling to readline
> +	 must be called twice.  */
> +      if (tracker_handle_brkchars.use_custom_word_point ())
> +	tracker = &tracker_handle_brkchars;
> +      else
> +	{
> +	  complete_line (tracker_handle_completions, *word, line, strlen (line));
> +	  tracker = &tracker_handle_completions;
> +	}
> +    }
> +  CATCH (ex, RETURN_MASK_ALL)
> +    {
> +      return {};
> +    }
> +  END_CATCH
> +
> +  return tracker->build_completion_result (*word, *word - line, strlen (line));  
> +}
> +
> +
>  /* Generate completions all at once.  Does nothing if max_completions
>     is 0.  If max_completions is non-negative, this will collect at
>     most max_completions strings.
> diff --git a/gdb/completer.h b/gdb/completer.h
> index e81243a721..bfbead235b 100644
> --- a/gdb/completer.h
> +++ b/gdb/completer.h
> @@ -510,6 +510,14 @@ extern void complete_line (completion_tracker &tracker,
>  			   const char *line_buffer,
>  			   int point);
>  
> +/* Complete LINE and return completion results. For completion purposes,
> +   cursor position is assumed to be at the end of LINE. WORD is set to 
> +   the end of word to complete. QUOTE_CHAR is set to the opening quote 
> +   character if we found an unclosed quoted substring, '\0' otherwise.
> + */

Double space after periods ending sentences.  '*/' goes at the previous line.

Like this:

/* Complete LINE and return completion results.  For completion
   purposes, cursor position is assumed to be at the end of LINE.
   WORD is set to the end of word to complete.  QUOTE_CHAR is set to
   the opening quote character if we found an unclosed quoted
   substring, '\0' otherwise.  */

> +extern completion_result
> +  complete (const char *line, char const **word, int *quote_char);
> +
>  /* Find the bounds of the word in TEXT for completion purposes, and
>     return a pointer to the end of the word.  Calls the completion
>     machinery for a handle_brkchars phase (using TRACKER) to figure out
> 
Thanks,
Pedro Alves