[v3,2/2] MI: Add new command -complete

  There is a CLI command 'complete' intended to use with emacs. Such a command
would also be useful for MI frontends, when separate CLI and MI channels cannot
be used. For example, on Windows (because of lack of PTYs) or when GDB is used
through SSH session.

This commit adds a new '-complete' MI command.

gdb/Changelog:
2019-01-28  Jan Vrany  <jan.vrany@fit.cvut.cz>

	* mi/mi-cmds.h (mi_cmd_complete): New function.
	* mi/mi-main.c (mi_cmd_complete): Likewise.
	* mi/mi-cmds.c: Define new MI command -complete.
	* NEWS: Mention new -complete command.

gdb/doc/ChangeLog:
2019-01-28  Jan Vrany  <jan.vrany@fit.cvut.cz>

	* gdb.texinfo (Miscellaneous GDB/MI Commands): Document new
	MI command -complete.

gdb/testsuite/ChangeLog:
2019-01-28  Jan Vrany  <jan.vrany@fit.cvut.cz>

	* gdb.mi/mi-complete.exp: New file.
	* gdb.mi/mi-complete.cc: Likewise.
---
 gdb/ChangeLog                        |  7 +++
 gdb/NEWS                             |  7 +++
 gdb/doc/ChangeLog                    |  5 ++
 gdb/doc/gdb.texinfo                  | 57 +++++++++++++++++++++
 gdb/mi/mi-cmds.c                     |  2 +
 gdb/mi/mi-cmds.h                     |  1 +
 gdb/mi/mi-main.c                     | 49 ++++++++++++++++++
 gdb/testsuite/ChangeLog              |  5 ++
 gdb/testsuite/gdb.mi/mi-complete.cc  | 21 ++++++++
 gdb/testsuite/gdb.mi/mi-complete.exp | 75 ++++++++++++++++++++++++++++
 10 files changed, 229 insertions(+)
 create mode 100644 gdb/testsuite/gdb.mi/mi-complete.cc
 create mode 100644 gdb/testsuite/gdb.mi/mi-complete.exp
  

> From: Jan Vrany <jan.vrany@fit.cvut.cz>
> Cc: Jan Vrany <jan.vrany@fit.cvut.cz>
> Date: Mon,  4 Mar 2019 14:52:03 +0000
> 
> There is a CLI command 'complete' intended to use with emacs. Such a command
> would also be useful for MI frontends, when separate CLI and MI channels cannot
> be used. For example, on Windows (because of lack of PTYs) or when GDB is used
> through SSH session.
> 
> This commit adds a new '-complete' MI command.

Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index eaef6aa384..3018313a46 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -96,6 +96,13 @@ maint show dwarf unwinders
>  info proc files
>    Display a list of open files for a process.
>  
> +* New MI commands
> +
> +-complete
> +  This lists all the possible completions for the rest of the line, if it
> +  were to be given as a command itself.  This is intended for use by MI frontends
> +  in cases when separate CLI and MI channels cannot be used.

This part is OK.

> +CLI and MI channels - for example: because of lack of PTYs like on Windows or
> +because @sc{gdb} is used remotely via a SSH connection.

We use "@value{GDBN}" instead of a literal "gdb" in the manual.

> +@item completion
> +This field contain a string to be completed on a command line, including
              ^^^^^^^
"contains"

> +@var{command} passed as parameter to @code{-complete} command. If nothing can
                                                                ^^
Two spaces between sentences, please.

> +be completed this field is omitted.
               ^
Comma missing here.

I find this text a bit confusing, so I suggest a slight rewording:

  This field contains the completed @var{command}.  If @var{command}
  has no known completions, this field is omitted.

> +@item matches
> +This field contain (possibly empty) array of matches. It is always present.
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"contains a (possibly empty) list of matches"

> +@end table

What about the max_completions_reached field?  I see it here:

> +^done,completion="break",matches=["break","break-range"],max_completions_reached="0"
  

Hi Jan,

This version generally looks good to me.  There are a few nits
to address, but with those picked, this should be good to go.

On 03/04/2019 02:52 PM, Jan Vrany wrote:

> diff --git a/gdb/NEWS b/gdb/NEWS
> index eaef6aa384..3018313a46 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -96,6 +96,13 @@ maint show dwarf unwinders
>  info proc files
>    Display a list of open files for a process.
>  
> +* New MI commands
> +
> +-complete
> +  This lists all the possible completions for the rest of the line, if it
> +  were to be given as a command itself.  This is intended for use by MI frontends
> +  in cases when separate CLI and MI channels cannot be used.

That line looks too long.  Hit M-q in emacs to reflow it.

> --- a/gdb/mi/mi-cmds.c
> +++ b/gdb/mi/mi-cmds.c
> @@ -23,6 +23,7 @@
>  #include "mi-cmds.h"
>  #include "mi-main.h"
>  
> +

Spurious space here.  Please drop it.

>  struct mi_cmd;
>  static struct mi_cmd **lookup_table (const char *command);
>  static void build_table (struct mi_cmd *commands);
> @@ -75,6 +76,7 @@ static struct mi_cmd mi_cmds[] =

> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -2709,6 +2709,55 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
>    }
>  }
>  
> +/* Implement the "-complete" command.  */
> +
> +void
> +mi_cmd_complete (const char *command, char **argv, int argc)
> +{
> +  if (argc != 1)
> +    {
> +      error (_("Usage: -complete COMMAND"));
> +    }

We don't wrap single-line statements with {}'s.

> +  if (max_completions == 0)
> +    {
> +      error (_("max-completions is zero,"
> +               " completion is disabled.\n"));
> +    }
> +
> +  int quote_char = '\0';
> +  const char *word;
> +
> +  completion_result result = complete (argv[0], &word, &quote_char);
> +
> +  std::string arg_prefix (argv[0], word - argv[0]);
> +
> +  struct ui_out *uiout = current_uiout;
> +
> +  if (result.number_matches > 0)
> +    uiout->field_fmt ("completion", "%s%s", arg_prefix.c_str (), result.match_list[0]);
> +
> +  {
> +    ui_out_emit_list completions_emitter (uiout, "matches");
> +
> +    if (result.number_matches == 1)
> +      {
> +        uiout->field_fmt(NULL, "%s%s", arg_prefix.c_str (), result.match_list[0]);
> +      }

Ditto.  

Missing space before parens in "field_fmt(NULL".

Watch out for too-long lines -- 80 cols is the hard max.

> +    else
> +      {
> +        result.sort_match_list ();
> +        for (size_t i = 0; i < result.number_matches; i++)
> +          {
> +            uiout->field_fmt(NULL, "%s%s", arg_prefix.c_str (),
> +                                           result.match_list[i + 1]);

Missing space before parens in "field_fmt(NULL".

> +          }
> +      }
> +  }
> +  uiout->field_string ("max_completions_reached",
> +                       result.number_matches == max_completions ? "1" : "0");
> +}
> +
> +
>  void
>  _initialize_mi_main (void)
>  {
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 7d8c7908fe..03135837f8 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,8 @@
> +2019-01-28  Jan Vrany  <jan.vrany@fit.cvut.cz>
> +
> +	* gdb.mi/mi-complete.exp: New file.
> +	* gdb.mi/mi-complete.cc: Likewise.
> +
>  2019-01-21  Alan Hayward  <alan.hayward@arm.com>
>  	* gdb.base/infcall-nested-structs.exp: Test C++ in addition to C.
>  
> diff --git a/gdb/testsuite/gdb.mi/mi-complete.cc b/gdb/testsuite/gdb.mi/mi-complete.cc
> new file mode 100644
> index 0000000000..fc85057d69
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-complete.cc
> @@ -0,0 +1,21 @@
> +#include <vector>

Missing copyright header.

> +
> +class A
> +{
> +public:
> +  void push_back(void *value);
> +};
> +
> +void A::push_back(void *value)
> +{
> +  /* nothing */
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    std::vector<int> v;
> +    v.push_back(1);
> +    A a;
> +    a.push_back(&v);
> +    return 0;
> +}

Please adjust the formatting to follow GNU conventions.

> \ No newline at end of file

Please add the missing newline.

> diff --git a/gdb/testsuite/gdb.mi/mi-complete.exp b/gdb/testsuite/gdb.mi/mi-complete.exp
> new file mode 100644
> index 0000000000..e82c2bff40
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-complete.exp
> @@ -0,0 +1,75 @@
> +# Copyright 2018 Free Software Foundation, Inc.

This needs to be 2018-2019 now.

> +
> +# 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/>.
> +
> +# Verify -data-evaluate-expression. There are really minimal tests.

Please replace this with a description of what this testcase is about.

> +
> +# The goal is not to test gdb functionality, which is done by other tests,
> +# but to verify the correct output response to MI operations.
> +#

Drop this.

> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +gdb_exit
> +if [mi_gdb_start] {
> +    continue
> +}
> +
> +standard_testfile .cc
> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {

Spurious double space after "if".

> +     untested "failed to compile"
> +     return -1
> +}
> +
> +mi_run_to_main
> +
> +mi_gdb_test "1-complete br" \
> +            "1\\^done,completion=\"break\",matches=\\\[.*\"break\",.*\"break-range\".*\\\],max_completions_reached=\"0\"" \
> +            "-complete br"
> +
> +# Check empty completion list

Write complete sentences -- add the missing period.

> +mi_gdb_test "5-complete bogus" \
> +            "5\\^done,matches=\\\[\\\],max_completions_reached=\"0\"" \
> +            "-complete bogus"
> +
> +# Check completions for commands with space

Missing period.

> +mi_gdb_test "4-complete \"b mai\"" \
> +            "4\\^done,completion=\"b main\",matches=\\\[.*\"b main\".*\\\],max_completions_reached=\"0\"" \
> +            "-complete \"b mai\""
> +
> +# Check wildmatching

Missing period.

> +mi_gdb_test "5-complete \"b push_ba\"" \
> +            "5\\^done,completion=\"b push_back\\(\",matches=\\\[.*\"b A::push_back\\(void\\*\\)\".*\\\],max_completions_reached=\"0\"" \
> +            "-complete \"b push_ba\" (wildmatching)"

Please see:

 https://sourceware.org/gdb/wiki/GDBTestcaseCookbook?highlight=%28testcase%29#Do_not_use_.22tail_parentheses.22_on_test_messages


> +
> +mi_gdb_test "-info-gdb-mi-command complete" \
> +            "\\^done,command=\{exists=\"true\"\}" \
> +            "-info-gdb-mi-command complete"
> +
> +# Limit max completions and check that max_completions_reached=\"0\" is set
> +# to 1.
> +send_gdb "set max-completions 1\n"
> +
> +mi_gdb_test "2-complete br" \
> +            ".*2\\^done,completion=\"br\[A-Za-z0-9-\]+\",matches=\\\[\"br\[A-Za-z0-9-\]+\"\\\],max_completions_reached=\"1\"" \
> +            "-complete br (max-completions 1)"
> +
> +# Disable completions and check an error is returned
> +send_gdb "set max-completions 0\n"
> +
> +mi_gdb_test "3-complete br" \
> +            ".*3\\^error,msg=\".*" \
> +            "-complete br (max-completions 0)"
> 

Thanks,
Pedro Alves