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 which is implemented using
CLI's 'complete'.

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

	* cli/cli-cmds.h: Export complete_command.
	* cli/cli-cmds.c (complete_command): Update to print completions
	on MI channel if requested.
	* mi/mi-cmds.c: Define new -complete command implemented using
	it CLI command 'complete'.
	* NEWS: Mention new -complete command.

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

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

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

	* gdb.mi/mi-complete.exp: New file.
---
 gdb/ChangeLog                        |  9 +++
 gdb/NEWS                             |  7 ++
 gdb/cli/cli-cmds.c                   | 95 +++++++++++++++++++---------
 gdb/cli/cli-cmds.h                   |  4 ++
 gdb/doc/ChangeLog                    |  5 ++
 gdb/doc/gdb.texinfo                  | 32 ++++++++++
 gdb/mi/mi-cmds.c                     |  3 +
 gdb/testsuite/ChangeLog              |  4 ++
 gdb/testsuite/gdb.mi/mi-complete.exp | 71 +++++++++++++++++++++
 9 files changed, 199 insertions(+), 31 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-complete.exp
  

Polite ping. 

Jan

On Thu, 2019-01-03 at 22:29 +0000, Jan Vrany wrote:
> 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 which is implemented using
> CLI's 'complete'.
> 
> gdb/Changelog:
> 2019-01-03  Jan Vrany  <jan.vrany@fit.cvut.cz>
> 
> 	* cli/cli-cmds.h: Export complete_command.
> 	* cli/cli-cmds.c (complete_command): Update to print completions
> 	on MI channel if requested.
> 	* mi/mi-cmds.c: Define new -complete command implemented using
> 	it CLI command 'complete'.
> 	* NEWS: Mention new -complete command.
> 
> gdb/doc/ChangeLog:
> 2019-01-03  Jan Vrany  <jan.vrany@fit.cvut.cz>
> 
> 	* gdb.texinfo (Miscellaneous GDB/MI Commands): Document new
> 	MI command -complete.
> 
> gdb/testsuite/ChangeLog:
> 2019-01-03  Jan Vrany  <jan.vrany@fit.cvut.cz>
> 
> 	* gdb.mi/mi-complete.exp: New file.
> ---
>  gdb/ChangeLog                        |  9 +++
>  gdb/NEWS                             |  7 ++
>  gdb/cli/cli-cmds.c                   | 95 +++++++++++++++++++---------
>  gdb/cli/cli-cmds.h                   |  4 ++
>  gdb/doc/ChangeLog                    |  5 ++
>  gdb/doc/gdb.texinfo                  | 32 ++++++++++
>  gdb/mi/mi-cmds.c                     |  3 +
>  gdb/testsuite/ChangeLog              |  4 ++
>  gdb/testsuite/gdb.mi/mi-complete.exp | 71 +++++++++++++++++++++
>  9 files changed, 199 insertions(+), 31 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.mi/mi-complete.exp
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 08c7c6c1f4..322b539417 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,12 @@
> +2019-01-03  Jan Vrany  <jan.vrany@fit.cvut.cz>
> +
> +	* cli/cli-cmds.h: Export complete_command.
> +	* cli/cli-cmds.c (complete_command): Update to print completions
> +	on MI channel if requested.
> +	* mi/mi-cmds.c: Define new -complete command implemented using
> +	it CLI command 'complete'.
> +	* NEWS: Mention new -complete command.
> +
>  2018-12-17  Andrew Burgess  <andrew.burgess@embecosm.com>
>  
>  	* dwarf2read.c (struct dwarf2_cu): Convert the fields 'mark',
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 913f3ae3b6..6b561af71f 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.
> +
>  * Changed commands
>  
>  target remote FILENAME
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 135f550b80..468f66dac6 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -223,16 +223,22 @@ help_command (const char *command, int from_tty)
>  /* Note: The "complete" command is used by Emacs to implement completion.
>     [Is that why this function writes output with *_unfiltered?]  */
>  
> -static void
> +void
>  complete_command (const char *arg, int from_tty)
>  {
> +  struct ui_out *uiout = current_uiout;
> +  bool is_mi_like = uiout->is_mi_like_p ();
> +
>    dont_repeat ();
>  
>    if (max_completions == 0)
>      {
> -      /* Only print this for non-mi frontends.  An MI frontend may not
> -	 be able to handle this.  */
> -      if (!current_uiout->is_mi_like_p ())
> +      if (is_mi_like)
> +        {
> +          error (_("max-completions is zero,"
> +		   " completion is disabled.\n"));
> +        }
> +      else
>  	{
>  	  printf_unfiltered (_("max-completions is zero,"
>  			       " completion is disabled.\n"));
> @@ -278,34 +284,61 @@ complete_command (const char *arg, int from_tty)
>    completion_result result
>      = tracker->build_completion_result (word, word - arg, strlen (arg));
>  
> -  if (result.number_matches != 0)
> -    {
> -      if (result.number_matches == 1)
> -	printf_unfiltered ("%s%s\n", arg_prefix.c_str (), result.match_list[0]);
> -      else
> -	{
> -	  result.sort_match_list ();
> -
> -	  for (size_t i = 0; i < result.number_matches; i++)
> -	    {
> -	      printf_unfiltered ("%s%s",
> -				 arg_prefix.c_str (),
> -				 result.match_list[i + 1]);
> -	      if (quote_char)
> -		printf_unfiltered ("%c", quote_char);
> -	      printf_unfiltered ("\n");
> -	    }
> -	}
> -
> -      if (result.number_matches == max_completions)
> -	{
> -	  /* ARG_PREFIX and WORD are included in the output so that emacs
> -	     will include the message in the output.  */
> -	  printf_unfiltered (_("%s%s %s\n"),
> -			     arg_prefix.c_str (), word,
> -			     get_max_completions_reached_message ());
> -	}
> +  {
> +    ui_out_emit_list completions_emitter (uiout, "completions");
> +    if (result.number_matches != 0)
> +      {
> +        if (result.number_matches == 1)
> +          {
> +	    if (is_mi_like)
> +	      uiout->field_fmt(NULL, "%s%s", arg_prefix.c_str (),
> +                                             result.match_list[0]);
> +	    else
> +	      printf_unfiltered ("%s%s\n", arg_prefix.c_str (),
> +                                           result.match_list[0]);
> +          }
> +        else
> +	  {
> +	    result.sort_match_list ();
> +
> +	    for (size_t i = 0; i < result.number_matches; i++)
> +	      {
> +                if (is_mi_like)
> +	          {
> +	            uiout->field_fmt(NULL, "%s%s", arg_prefix.c_str (),
> +                                                   result.match_list[i + 1]);
> +	          }
> +	        else
> +	          {
> +		    printf_unfiltered ("%s%s", arg_prefix.c_str (),
> +                                               result.match_list[i + 1]);
> +		    if (quote_char)
> +		      printf_unfiltered ("%c", quote_char);
> +		    printf_unfiltered ("\n");
> +	          }
> +	      }
> +	  }
> +      }
>      }
> +
> +    if (result.number_matches == max_completions)
> +      {
> +	if (is_mi_like)
> +	  uiout->field_string("max_completions_reached", "1");
> +	else
> +	  {
> +	    /* ARG_PREFIX and WORD are included in the output so that emacs
> +	       will include the message in the output.  */
> +	    printf_unfiltered (_("%s%s %s\n"),
> +				 arg_prefix.c_str (), word,
> +				 get_max_completions_reached_message ());
> +	  }
> +      }
> +    else
> +      {
> +	if (is_mi_like)
> +	  uiout->field_string("max_completions_reached", "0");
> +      }
>  }
>  
>  int
> diff --git a/gdb/cli/cli-cmds.h b/gdb/cli/cli-cmds.h
> index 3fa188dba6..bcd12c0c0b 100644
> --- a/gdb/cli/cli-cmds.h
> +++ b/gdb/cli/cli-cmds.h
> @@ -106,6 +106,10 @@ void init_cmd_lists (void);
>  
>  void init_cli_cmds (void);
>  
> +/* Exported to gdb/mi/mi-cmds.c */
> +
> +void complete_command (const char *, int);
> +
>  int is_complete_command (struct cmd_list_element *cmd);
>  
>  /* Exported to gdb/main.c */
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 68d3068c72..da81f6a6c1 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,8 @@
> +2019-01-03  Jan Vrany  <jan.vrany@fit.cvut.cz>
> +
> +	* gdb.texinfo (Miscellaneous GDB/MI Commands): Document new
> +	MI command -complete.
> +
>  2018-12-13  John Baldwin  <jhb@FreeBSD.org>
>  
>  	* gdb.texinfo (Set Catchpoints): Add an anchor for 'catch syscall'.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index d766e44e63..7f88273145 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -34236,6 +34236,38 @@ fullname="/home/nickrob/myprog.c",line="73",arch="i386:x86_64"@}
>  (gdb)
>  @end smallexample
>  
> +@subheading The @code{-complete} Command
> +@findex -complete
> +
> +@subheading Synopsis
> +
> +@smallexample
> +-complete @var{command}
> +@end smallexample
> +
> +Show a list of completions for partially typed CLI @var{command}. The
> +@var{command} should be given in verbatim without any quoting / escaping.
> +
> +This command is intended for @sc{gdb/mi} frontends that cannot use two separate
> +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.
> +
> +@subheading @value{GDBN} Command
> +
> +The corresponding @value{GDBN} command is @samp{complete}.
> +
> +@subheading Example
> +
> +@smallexample
> +(gdb)
> +-complete br
> +^done,completions=["break","break-range"],max_completions_reached="0"
> +(gdb)
> +-complete b madv
> +^done,completions=["b madvise"],max_completions_reached="0"
> +(gdb)
> +@end smallexample
> +
>  @node Annotations
>  @chapter @value{GDBN} Annotations
>  
> diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
> index 51014eded2..7b510f71d4 100644
> --- a/gdb/mi/mi-cmds.c
> +++ b/gdb/mi/mi-cmds.c
> @@ -20,9 +20,11 @@
>  
>  #include "defs.h"
>  #include "top.h"
> +#include "cli/cli-cmds.h"
>  #include "mi-cmds.h"
>  #include "mi-main.h"
>  
> +
>  struct mi_cmd;
>  static struct mi_cmd **lookup_table (const char *command);
>  static void build_table (struct mi_cmd *commands);
> @@ -75,6 +77,7 @@ static struct mi_cmd mi_cmds[] =
>                     &mi_suppress_notification.breakpoint),
>    DEF_MI_CMD_MI_1 ("catch-unload", mi_cmd_catch_unload,
>                     &mi_suppress_notification.breakpoint),
> +  DEF_MI_CMD_CLI ("complete", "complete", 1),
>    DEF_MI_CMD_MI ("data-disassemble", mi_cmd_disassemble),
>    DEF_MI_CMD_MI ("data-evaluate-expression", mi_cmd_data_evaluate_expression),
>    DEF_MI_CMD_MI ("data-list-changed-registers",
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index ab3a74fffd..40da71b2c5 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,7 @@
> +2019-01-03  Jan Vrany  <jan.vrany@fit.cvut.cz>
> +
> +	* gdb.mi/mi-complete.exp: New file.
> +
>  2018-12-12  Andrew Burgess  <andrew.burgess@embecosm.com>
>  
>  	* gdb.base/annota1.exp: Update a test regexp.
> diff --git a/gdb/testsuite/gdb.mi/mi-complete.exp b/gdb/testsuite/gdb.mi/mi-complete.exp
> new file mode 100644
> index 0000000000..14c763a966
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-complete.exp
> @@ -0,0 +1,71 @@
> +# Copyright 2018 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/>;.
> +
> +# Verify -data-evaluate-expression. There are really minimal tests.
> +
> +# The goal is not to test gdb functionality, which is done by other tests,
> +# but to verify the correct output response to MI operations.
> +#
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +gdb_exit
> +if [mi_gdb_start] {
> +    continue
> +}
> +
> +standard_testfile basics.c
> +
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +     untested "failed to compile"
> +     return -1
> +}
> +
> +mi_run_to_main
> +
> +mi_gdb_test "1-complete br" \
> +            "1\\^done,completions=\\\[.*\"break\",.*\"break-range\".*\\\],max_completions_reached=\"0\"" \
> +            "-complete br"
> +
> +# Check empty completion list
> +mi_gdb_test "5-complete bogus" \
> +            "5\\^done,completions=\\\[\\\],max_completions_reached=\"0\"" \
> +            "-complete bogus"
> +
> +
> +# Check completions for commands with space
> +mi_gdb_test "4-complete b mai" \
> +            "4\\^done,completions=\\\[.*\"b main\".*\\\],max_completions_reached=\"0\"" \
> +            "-complete b mai"
> +
> +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,completions=\\\[\"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 1)"
  

>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:

Jan> This commit adds a new '-complete' MI command which is implemented using
Jan> CLI's 'complete'.

I suspect it would be better to just write a new MI function and then
mildly refactor complete_command so that the two commands can share most
of their implementation.

First, this would make it so we could add options to the MI command as
needed.  (Perhaps the current argument to the command ought to have a
flag to avoid future parsing problems?  I am not sure.)

Second, it would mean that we could make changes to the CLI command
without worrying about breaking MI compatibility.

What do you think of this?

I looked and I see that some other MI commands are implemented directly
by CLI commands.  I am not sure but I suspect this might be a deprecated
approach.

Jan> +  bool is_mi_like = uiout->is_mi_like_p ();

This could be removed by having a more explicit API.

Jan> +    ui_out_emit_list completions_emitter (uiout, "completions");
Jan> +    if (result.number_matches != 0)
Jan> +      {
Jan> +        if (result.number_matches == 1)
Jan> +          {
Jan> +	    if (is_mi_like)
Jan> +	      uiout->field_fmt(NULL, "%s%s", arg_prefix.c_str (),
Jan> +                                             result.match_list[0]);
Jan> +	    else
Jan> +	      printf_unfiltered ("%s%s\n", arg_prefix.c_str (),
Jan> +                                           result.match_list[0]);

Space before "(" in the field_fmt call.  Unifying the cases would be
preferable.  Or another argument for separation.

thanks,
Tom
  

On Thu, 2019-01-17 at 13:29 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
> 
> Jan> This commit adds a new '-complete' MI command which is implemented using
> Jan> CLI's 'complete'.
> 
> I suspect it would be better to just write a new MI function and then
> mildly refactor complete_command so that the two commands can share most
> of their implementation.
> 
> First, this would make it so we could add options to the MI command as
> needed.  (Perhaps the current argument to the command ought to have a
> flag to avoid future parsing problems?  

What do you mean by this? Can you give an example? 

> I am not sure.)
> 
> Second, it would mean that we could make changes to the CLI command
> without worrying about breaking MI compatibility.

As for changes to CLI command, I've got the impression from the code that
the format is rather strict (from use of printf_unfiltered(), for instance),
deliberately so to not break clients (Emacs is mentioned). Indeed, not really
an argument against splitting. 

> 
> What do you think of this?

I thought of that actually. Also, it'd make the MI interface more regular
w.r.t quoting of spaces and other special characters. But in the end I could 
not find a way without duplicating enough code from the original function, 
so hacked it that way. 

Let me try harder :-) 

> 
> I looked and I see that some other MI commands are implemented directly
> by CLI commands.  I am not sure but I suspect this might be a deprecated
> approach.

Yeah, there's quite a lot of them. If it is a deprecated approach, 
I did not know. 

> 
> Jan> +  bool is_mi_like = uiout->is_mi_like_p ();
> 
> This could be removed by having a more explicit API.

I'm sorry, I'm not sure what do you mean here. 

> 
> Jan> +    ui_out_emit_list completions_emitter (uiout, "completions");
> Jan> +    if (result.number_matches != 0)
> Jan> +      {
> Jan> +        if (result.number_matches == 1)
> Jan> +          {
> Jan> +	    if (is_mi_like)
> Jan> +	      uiout->field_fmt(NULL, "%s%s", arg_prefix.c_str (),
> Jan> +                                             result.match_list[0]);
> Jan> +	    else
> Jan> +	      printf_unfiltered ("%s%s\n", arg_prefix.c_str (),
> Jan> +                                           result.match_list[0]);
> 
> Space before "(" in the field_fmt call.  Unifying the cases would be
> preferable.  Or another argument for separation.
> 

Thanks! 
Jan
  

This is a rework of previous patch based on Tom's comments.

Another thing worth considering (not done in this version) is to
have more structured result. Instead of (current version):

-complete "br m"
=^done,completions=["br main", "br madvise"]

respond with something like (proposed change):

-complete "br m"
=^done,text="br m",common="a",matches=["in", "dvise"]

The rarionale is that frontend most likely needs these three
values anyway to implement completion. It can, indeed compute
them from full list as returned now, but GDB has these values
already so it would save the frontend doing the same work again. 

OTOH, current output is more on par with CLI command output. 

What do you think? 

Differences v1 -> v2: 

  * extracted common completion logic to a new helper function
  * implemented MI command using a new mi-specific function rather
    than using CLI implementation.


Jan Vrany (2):
  MI: extract command completion logic from complete_command()
  MI: Add new command -complete

 gdb/ChangeLog                        | 14 ++++++
 gdb/NEWS                             |  7 +++
 gdb/cli/cli-cmds.c                   | 32 +------------
 gdb/completer.c                      | 34 +++++++++++++
 gdb/completer.h                      |  8 ++++
 gdb/doc/ChangeLog                    |  5 ++
 gdb/doc/gdb.texinfo                  | 31 ++++++++++++
 gdb/mi/mi-cmds.c                     |  2 +
 gdb/mi/mi-cmds.h                     |  1 +
 gdb/mi/mi-main.c                     | 44 +++++++++++++++++
 gdb/testsuite/ChangeLog              |  4 ++
 gdb/testsuite/gdb.mi/mi-complete.exp | 71 ++++++++++++++++++++++++++++
 12 files changed, 223 insertions(+), 30 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-complete.exp
  

Polite ping. 

Jan

On Mon, 2019-02-04 at 10:00 +0000, Jan Vrany wrote:
> Polite ping. 
> 
> Jan
> 
> On Mon, 2019-01-28 at 12:40 +0000, Jan Vrany wrote:
> > This is a rework of previous patch based on Tom's comments.
> > 
> > Another thing worth considering (not done in this version) is to
> > have more structured result. Instead of (current version):
> > 
> > -complete "br m"
> > =^done,completions=["br main", "br madvise"]
> > 
> > respond with something like (proposed change):
> > 
> > -complete "br m"
> > =^done,text="br m",common="a",matches=["in", "dvise"]
> > 
> > The rarionale is that frontend most likely needs these three
> > values anyway to implement completion. It can, indeed compute
> > them from full list as returned now, but GDB has these values
> > already so it would save the frontend doing the same work again. 
> > 
> > OTOH, current output is more on par with CLI command output. 
> > 
> > What do you think? 
> > 
> > Differences v1 -> v2: 
> > 
> >   * extracted common completion logic to a new helper function
> >   * implemented MI command using a new mi-specific function rather
> >     than using CLI implementation.
> > 
> > 
> > Jan Vrany (2):
> >   MI: extract command completion logic from complete_command()
> >   MI: Add new command -complete
> > 
> >  gdb/ChangeLog                        | 14 ++++++
> >  gdb/NEWS                             |  7 +++
> >  gdb/cli/cli-cmds.c                   | 32 +------------
> >  gdb/completer.c                      | 34 +++++++++++++
> >  gdb/completer.h                      |  8 ++++
> >  gdb/doc/ChangeLog                    |  5 ++
> >  gdb/doc/gdb.texinfo                  | 31 ++++++++++++
> >  gdb/mi/mi-cmds.c                     |  2 +
> >  gdb/mi/mi-cmds.h                     |  1 +
> >  gdb/mi/mi-main.c                     | 44 +++++++++++++++++
> >  gdb/testsuite/ChangeLog              |  4 ++
> >  gdb/testsuite/gdb.mi/mi-complete.exp | 71 ++++++++++++++++++++++++++++
> >  12 files changed, 223 insertions(+), 30 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.mi/mi-complete.exp
> >
  

Polite ping. 

Jan

On Wed, 2019-02-13 at 09:24 +0000, Jan Vrany wrote:
> Polite ping. 
> 
> Jan
> 
> On Mon, 2019-02-04 at 10:00 +0000, Jan Vrany wrote:
> > Polite ping. 
> > 
> > Jan
> > 
> > On Mon, 2019-01-28 at 12:40 +0000, Jan Vrany wrote:
> > > This is a rework of previous patch based on Tom's comments.
> > > 
> > > Another thing worth considering (not done in this version) is to
> > > have more structured result. Instead of (current version):
> > > 
> > > -complete "br m"
> > > =^done,completions=["br main", "br madvise"]
> > > 
> > > respond with something like (proposed change):
> > > 
> > > -complete "br m"
> > > =^done,text="br m",common="a",matches=["in", "dvise"]
> > > 
> > > The rarionale is that frontend most likely needs these three
> > > values anyway to implement completion. It can, indeed compute
> > > them from full list as returned now, but GDB has these values
> > > already so it would save the frontend doing the same work again. 
> > > 
> > > OTOH, current output is more on par with CLI command output. 
> > > 
> > > What do you think? 
> > > 
> > > Differences v1 -> v2: 
> > > 
> > >   * extracted common completion logic to a new helper function
> > >   * implemented MI command using a new mi-specific function rather
> > >     than using CLI implementation.
> > > 
> > > 
> > > Jan Vrany (2):
> > >   MI: extract command completion logic from complete_command()
> > >   MI: Add new command -complete
> > > 
> > >  gdb/ChangeLog                        | 14 ++++++
> > >  gdb/NEWS                             |  7 +++
> > >  gdb/cli/cli-cmds.c                   | 32 +------------
> > >  gdb/completer.c                      | 34 +++++++++++++
> > >  gdb/completer.h                      |  8 ++++
> > >  gdb/doc/ChangeLog                    |  5 ++
> > >  gdb/doc/gdb.texinfo                  | 31 ++++++++++++
> > >  gdb/mi/mi-cmds.c                     |  2 +
> > >  gdb/mi/mi-cmds.h                     |  1 +
> > >  gdb/mi/mi-main.c                     | 44 +++++++++++++++++
> > >  gdb/testsuite/ChangeLog              |  4 ++
> > >  gdb/testsuite/gdb.mi/mi-complete.exp | 71 ++++++++++++++++++++++++++++
> > >  12 files changed, 223 insertions(+), 30 deletions(-)
> > >  create mode 100644 gdb/testsuite/gdb.mi/mi-complete.exp
> > >
  

>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:

Jan> respond with something like (proposed change):

Jan> -complete "br m"
Jan> =^done,text="br m",common="a",matches=["in", "dvise"]

Jan> The rarionale is that frontend most likely needs these three
Jan> values anyway to implement completion. It can, indeed compute
Jan> them from full list as returned now, but GDB has these values
Jan> already so it would save the frontend doing the same work again. 

Jan> OTOH, current output is more on par with CLI command output. 

Jan> What do you think? 

I think if this form seems more useful to MI users, then it's fine to
implement it.  I would say this is more important than making it similar
to the CLI.

Tom
  

On Wed, 2019-02-20 at 14:20 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
> 
> Jan> respond with something like (proposed change):
> 
> Jan> -complete "br m"
> Jan> =^done,text="br m",common="a",matches=["in", "dvise"]
> 
> Jan> The rarionale is that frontend most likely needs these three
> Jan> values anyway to implement completion. It can, indeed compute
> Jan> them from full list as returned now, but GDB has these values
> Jan> already so it would save the frontend doing the same work again. 
> 
> Jan> OTOH, current output is more on par with CLI command output. 
> 
> Jan> What do you think? 
> 
> I think if this form seems more useful to MI users, then it's fine to
> implement it.  I would say this is more important than making it similar
> to the CLI.

Are there any other GDB/MI users to comment on this? What would you prefer?

Jan
  

>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:

Jan> Are there any other GDB/MI users to comment on this? What would you
Jan> prefer?

Given the lack of response, I think you should just say which you
prefer.  If you think it would be better the "other" way, go for it.
Or if you'd rather the patches you already have, let me know.

thanks,
Tom
  

On Tue, 2019-02-26 at 12:49 -0700, Tom Tromey wrote:
> > > > > > "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
> 
> Or if you'd rather the patches you already have, let me know.
> 

Yes, I would prefer to have it as implemented in v2 patch mini-series
Thanks!

Jan
  

I'm totally not against this new command at all, but I have to say that I'd be
much more thrilled if someone just spent the time to make separate CLI/MI
channels work on Windows too.  The channel doesn't _have_ to be a PTY.

On 02/26/2019 07:49 PM, Tom Tromey wrote:
>>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
> 
> Jan> Are there any other GDB/MI users to comment on this? What would you
> Jan> prefer?
> 
> Given the lack of response, I think you should just say which you
> prefer.  If you think it would be better the "other" way, go for it.
> Or if you'd rather the patches you already have, let me know.

Jan, please consider the wildmatching case.  E.g., when debugging GDB itself:

(gdb) b push_bac<TAB>
Display all 102 possibilities? (y or n)
debug_names::offset_vec_tmpl<unsigned int>::push_back_reorder(unsigned long)
debug_names::offset_vec_tmpl<unsigned long>::push_back_reorder(unsigned long)
std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::push_back(char)
...

The frontend needs to complete "b push_bac" -> "b push_back", and present
the matches.

But the least common denominator is not at the start of the matches
strings.  How will a frontend compute the LCD from the matches list alone?

Please mind the "2018" copyright year in the testcase.

Thanks,
Pedro Alves
  

Hello,

On Wed, 2019-02-27 at 20:41 +0000, Pedro Alves wrote:
> I'm totally not against this new command at all, but I have to say that I'd be
> much more thrilled if someone just spent the time to make separate CLI/MI
> channels work on Windows too.  The channel doesn't _have_ to be a PTY.

I know. That was my initial idea just to use named pipes year or two back
when I started to support Windows. However:

1) Completion CLI is AFAIK implemented using readline which has problems 
   working over pipes. Actually, I never got CLI working satisfactorily
   on Windows even when I just run GDB "normally" from Windows command shell (`cmd.exe`),
   let alone over pipes or alike. 

2) Even if it would work, there are still other usecases which working readline
   and separate CLI/MI channel on Windows would not support. For example:
   
   (a) at the moment we use "my" frontend [1], [2] running on developers laptop 
       connecting over ssh to a GDB that runs on remote RISC-V machine. Essentially,
       instead of lauching gdb locally (on dev's machine) like: 

         gdb path/to/binary

       we do 

         ssh unleashed gdb -i=mi2 path/to/binary

       In this case, opening a secondary MI channel is very problematic. 
       -complete command gives me at least a way to implement completion which
       we found very important. There are still some quirks with that, sure, 
       like when you run `pi` or `shell` commands it completely ruins the MI 
       channel (this is a bug have not yet looked at let alone fixed but it is 
       on my very long list).

   (b) another frontend feature asked was to provide a kind of "workspace" for GDB commands,
       This is essentially a text editor in which you write ad-hoc commands (possibly
       with comments) and execute them by selecting portion of a text and pressing a button
       (or with a shortcut). -complete command would allow me to implement completion
       in such "workspace" 

       (I know, this may sound like a weird UI, but this is essentially what a Smalltalk workspace
       is but for GDB commands. So far users of this frontend are mainly - me including -  
       a small bunch of old smalltalkers working on virtual machines)

[1]: https://bitbucket.org/janvrany/jv-vdb/src/default/
[2]: https://bitbucket.org/janvrany/jv-libgdbs/src/default/
[3]: https://bitbucket.org/janvrany/jv-vdb/src/default/doc/Invoking.md
   
> 
> On 02/26/2019 07:49 PM, Tom Tromey wrote:
> > > > > > > "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
> > 
> > Jan> Are there any other GDB/MI users to comment on this? What would you
> > Jan> prefer?
> > 
> > Given the lack of response, I think you should just say which you
> > prefer.  If you think it would be better the "other" way, go for it.
> > Or if you'd rather the patches you already have, let me know.
> 
> Jan, please consider the wildmatching case.  E.g., when debugging GDB itself:
> 
> (gdb) b push_bac<TAB>
> Display all 102 possibilities? (y or n)
> debug_names::offset_vec_tmpl<unsigned int>::push_back_reorder(unsigned long)
> debug_names::offset_vec_tmpl<unsigned long>::push_back_reorder(unsigned long)
> std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::push_back(char)
> ...
> 
> The frontend needs to complete "b push_bac" -> "b push_back", and present
> the matches.
> 
> But the least common denominator is not at the start of the matches
> strings.  How will a frontend compute the LCD from the matches list alone?

I see. I was not aware of this behavior, thanks for pointing this out! I'll address that
in next iterations. 

Thanks a lot! 

Jan
  

This is a rework of v2 patch based on Pedro's comments.

Differences v2 -> v3:

  * to support wildmatching, the command output had to change to
    provide both, "completion" (what a frontend should complete)
    and "matches" (a list of possible completions)
  * and some formatting issues.

Differences v1 -> v2:

  * extracted common completion logic to a new helper function
  * implemented MI command using a new mi-specific function rather
    than using CLI implementation.

Jan Vrany (2):
  MI: extract command completion logic from complete_command()
  MI: Add new command -complete

 gdb/ChangeLog                        | 14 ++++++
 gdb/NEWS                             |  7 +++
 gdb/cli/cli-cmds.c                   | 30 +----------
 gdb/completer.c                      | 36 +++++++++++++
 gdb/completer.h                      |  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 ++++++++++++++++++++++++++++
 13 files changed, 280 insertions(+), 29 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-complete.cc
 create mode 100644 gdb/testsuite/gdb.mi/mi-complete.exp
  

On 02/28/2019 10:18 AM, Jan Vrany wrote:
> Hello,
> 
> On Wed, 2019-02-27 at 20:41 +0000, Pedro Alves wrote:
>> I'm totally not against this new command at all, but I have to say that I'd be
>> much more thrilled if someone just spent the time to make separate CLI/MI
>> channels work on Windows too.  The channel doesn't _have_ to be a PTY.
> 
> I know. That was my initial idea just to use named pipes year or two back
> when I started to support Windows. However:
> 
> 1) Completion CLI is AFAIK implemented using readline which has problems 
>    working over pipes. Actually, I never got CLI working satisfactorily
>    on Windows even when I just run GDB "normally" from Windows command shell (`cmd.exe`),
>    let alone over pipes or alike. 

Curious.  AFAIK native Windows gdb over cmd.exe should work fine.

Over pipes or the like, indeed I wouldn't be surprised with issues.
"set interactive-mode on" may help.

BTW, (and I just remembered that after sending the previous email), AFAIK,
Eclipse made the GDB console (with new-ui) work in the Windows port by
using WinPTY.

> 
> 2) Even if it would work, there are still other usecases which working readline
>    and separate CLI/MI channel on Windows would not support. For example:
>    
>    (a) at the moment we use "my" frontend [1], [2] running on developers laptop 
>        connecting over ssh to a GDB that runs on remote RISC-V machine. Essentially,
>        instead of lauching gdb locally (on dev's machine) like: 
> 
>          gdb path/to/binary
> 
>        we do 
> 
>          ssh unleashed gdb -i=mi2 path/to/binary
> 
>        In this case, opening a secondary MI channel is very problematic. 

Why's that problematic?  You could forward a tcp port or a unix domain socket
over shh for MI?  Again, there's no real reason that new-ui only works with
ptys, other than noone ever tweaking the code to support other file types.

>        -complete command gives me at least a way to implement completion which
>        we found very important. There are still some quirks with that, sure, 
>        like when you run `pi` or `shell` commands it completely ruins the MI 
>        channel (this is a bug have not yet looked at let alone fixed but it is 
>        on my very long list).
> 

To me, it seems like you'll never manage to mimic gdb's behavior perfectly.
new-ui gets you perfect behavior, because, well, there's no mimicing.

>    (b) another frontend feature asked was to provide a kind of "workspace" for GDB commands,
>        This is essentially a text editor in which you write ad-hoc commands (possibly
>        with comments) and execute them by selecting portion of a text and pressing a button
>        (or with a shortcut). -complete command would allow me to implement completion
>        in such "workspace" 
> 

I see.

>        (I know, this may sound like a weird UI, but this is essentially what a Smalltalk workspace
>        is but for GDB commands. So far users of this frontend are mainly - me including -  
>        a small bunch of old smalltalkers working on virtual machines)
> 
> [1]: https://bitbucket.org/janvrany/jv-vdb/src/default/
> [2]: https://bitbucket.org/janvrany/jv-libgdbs/src/default/
> [3]: https://bitbucket.org/janvrany/jv-vdb/src/default/doc/Invoking.md
>    
>>
>> On 02/26/2019 07:49 PM, Tom Tromey wrote:
>>>>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
>>>
>>> Jan> Are there any other GDB/MI users to comment on this? What would you
>>> Jan> prefer?
>>>
>>> Given the lack of response, I think you should just say which you
>>> prefer.  If you think it would be better the "other" way, go for it.
>>> Or if you'd rather the patches you already have, let me know.
>>
>> Jan, please consider the wildmatching case.  E.g., when debugging GDB itself:
>>
>> (gdb) b push_bac<TAB>
>> Display all 102 possibilities? (y or n)
>> debug_names::offset_vec_tmpl<unsigned int>::push_back_reorder(unsigned long)
>> debug_names::offset_vec_tmpl<unsigned long>::push_back_reorder(unsigned long)
>> std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::push_back(char)
>> ...
>>
>> The frontend needs to complete "b push_bac" -> "b push_back", and present
>> the matches.
>>
>> But the least common denominator is not at the start of the matches
>> strings.  How will a frontend compute the LCD from the matches list alone?
> 
> I see. I was not aware of this behavior, thanks for pointing this out! I'll address that
> in next iterations. 

Another thing that I remembered is that in some cases, GDB's completion actually
replaces the whole complete word, instead of just appending.  For example, try:

  (gdb) handle sigv<TAB>

Results in:

  (gdb) handle SIGVTALRM

ISTR having run into other examples, but not recalling
them right now.

Thanks,
Pedro Alves
  

Hello, 

On Tue, 2019-03-05 at 20:53 +0000, Pedro Alves wrote:
> On 02/28/2019 10:18 AM, Jan Vrany wrote:
> > Hello,
> > 
> > On Wed, 2019-02-27 at 20:41 +0000, Pedro Alves wrote:
> > > I'm totally not against this new command at all, but I have to say that I'd be
> > > much more thrilled if someone just spent the time to make separate CLI/MI
> > > channels work on Windows too.  The channel doesn't _have_ to be a PTY.
> > 
> > I know. That was my initial idea just to use named pipes year or two back
> > when I started to support Windows. However:
> > 
> > 1) Completion CLI is AFAIK implemented using readline which has problems 
> >    working over pipes. Actually, I never got CLI working satisfactorily
> >    on Windows even when I just run GDB "normally" from Windows command shell (`cmd.exe`),
> >    let alone over pipes or alike. 
> 
> Curious.  AFAIK native Windows gdb over cmd.exe should work fine.

Maybe I just don't know how to compile it properly. I have just compiled
fresh c3734e093aab1ce from git (only with this patch 
https://sourceware.org/ml/gdb-patches/2018-10/msg00614.html to make it compile
with Python 3) using using MSYS2 MINGW64 toolchain on Windows 10. 
This is the exact configure command:

bash ../configure --build=x86_64-w64-mingw32 --disable-werror --with-guile=no --with-python=C:\msys64\mingw64\bin\python3 --enable-targets="i686-w64-mingw32,x86_64-w64-mingw32"

Completion by tab seem to work. 

Backspace practially does not, it deletes the character in the line 
buffer (presumably) but not on the screen. Instead, it moves the caret 
one character on the right. Therefore what use see on the screen is not 
what it sent to GDB when she presses enter. 

Moving cursor by left arrow followed by typing has similar issues. 
Same for delete. Same for pressing Ctrl-R for searching the history. 

Generally, internally the state of the buffer might be OK (hard to tell)
but what is shown on the screen is completely out of sync, making it really 
hard to work with, at least for me. 

Again, it might well be just that I don't know how to configure / compile 
things properly. In the past I tried pre-compiled GDBs from cygwin & msys2
various versions of each and each of them has similar issues so I give up
digging deeper. 


> 
> Over pipes or the like, indeed I wouldn't be surprised with issues.
> "set interactive-mode on" may help.
> 
> BTW, (and I just remembered that after sending the previous email), AFAIK,
> Eclipse made the GDB console (with new-ui) work in the Windows port by
> using WinPTY.

I was not aware of this. I'll have a look at eclipse code. Thanks!

> 
> > 2) Even if it would work, there are still other usecases which working readline
> >    and separate CLI/MI channel on Windows would not support. For example:
> >    
> >    (a) at the moment we use "my" frontend [1], [2] running on developers laptop 
> >        connecting over ssh to a GDB that runs on remote RISC-V machine. Essentially,
> >        instead of lauching gdb locally (on dev's machine) like: 
> > 
> >          gdb path/to/binary
> > 
> >        we do 
> > 
> >          ssh unleashed gdb -i=mi2 path/to/binary
> > 
> >        In this case, opening a secondary MI channel is very problematic. 
> 
> Why's that problematic?  You could forward a tcp port or a unix domain socket
> over shh for MI?  Again, there's no real reason that new-ui only works with
> ptys, other than noone ever tweaking the code to support other file types.
> 

This could be a way, yes. I just find it little too fiddly to implement. 
Using ssh command has some problems, using things like libssh has other problems.
Little details, but that's where the devil is. Plus it does not solve the
issues on Windows I described above nor the would it help me with the workspace.
So it seems to me that -complete would be a reasonable way to deal with all that. 

> >        -complete command gives me at least a way to implement completion which
> >        we found very important. There are still some quirks with that, sure, 
> >        like when you run `pi` or `shell` commands it completely ruins the MI 
> >        channel (this is a bug have not yet looked at let alone fixed but it is 
> >        on my very long list).
> > 
> 
> To me, it seems like you'll never manage to mimic gdb's behavior perfectly.
> new-ui gets you perfect behavior, because, well, there's no mimicing.
> 

Well, I'm not seeking to have perfect imitation of gdb's behavior. Just good enough. 

> >    (b) another frontend feature asked was to provide a kind of "workspace" for GDB commands,
> >        This is essentially a text editor in which you write ad-hoc commands (possibly
> >        with comments) and execute them by selecting portion of a text and pressing a button
> >        (or with a shortcut). -complete command would allow me to implement completion
> >        in such "workspace" 
> > 
> 
> I see.
> 
> >        (I know, this may sound like a weird UI, but this is essentially what a Smalltalk workspace
> >        is but for GDB commands. So far users of this frontend are mainly - me including -  
> >        a small bunch of old smalltalkers working on virtual machines)
> > 
> > [1]: https://bitbucket.org/janvrany/jv-vdb/src/default/
> > [2]: https://bitbucket.org/janvrany/jv-libgdbs/src/default/
> > [3]: https://bitbucket.org/janvrany/jv-vdb/src/default/doc/Invoking.md
> >    
> > > On 02/26/2019 07:49 PM, Tom Tromey wrote:
> > > > > > > > > "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:
> > > > 
> > > > Jan> Are there any other GDB/MI users to comment on this? What would you
> > > > Jan> prefer?
> > > > 
> > > > Given the lack of response, I think you should just say which you
> > > > prefer.  If you think it would be better the "other" way, go for it.
> > > > Or if you'd rather the patches you already have, let me know.
> > > 
> > > Jan, please consider the wildmatching case.  E.g., when debugging GDB itself:
> > > 
> > > (gdb) b push_bac<TAB>
> > > Display all 102 possibilities? (y or n)
> > > debug_names::offset_vec_tmpl<unsigned int>::push_back_reorder(unsigned long)
> > > debug_names::offset_vec_tmpl<unsigned long>::push_back_reorder(unsigned long)
> > > std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::push_back(char)
> > > ...
> > > 
> > > The frontend needs to complete "b push_bac" -> "b push_back", and present
> > > the matches.
> > > 
> > > But the least common denominator is not at the start of the matches
> > > strings.  How will a frontend compute the LCD from the matches list alone?
> > 
> > I see. I was not aware of this behavior, thanks for pointing this out! I'll address that
> > in next iterations. 
> 
> Another thing that I remembered is that in some cases, GDB's completion actually
> replaces the whole complete word, instead of just appending.  For example, try:
> 
>   (gdb) handle sigv<TAB>
> 
> Results in:
> 
>   (gdb) handle SIGVTALRM
> 
> ISTR having run into other examples, but not recalling
> them right now.

I think this is handled by v3:

(gdb) 
-complete "handle sigv"
^done,completion="handle SIGVTALRM",matches=["handle SIGVTALRM"],max_completions_reached="0"
(gdb) 


Anyways, another way to look at this commit is that there's already 
CLI command "complete" and this commit "merely" add an MI counterpart of it,
exposing over MI what's already available in CLI. 

If the maintenance cost -complete is considered "too much for little gain", let's 
drop it.  

Thanks! 

Jan

> 
> Thanks,
> Pedro Alves
  

> From: Jan Vrany <jan.vrany@fit.cvut.cz>
> Cc: gdb-patches@sourceware.org, "gdb@sourceware.org" <gdb@sourceware.org>
> Date: Wed, 06 Mar 2019 15:09:33 +0000
> 
> > > 1) Completion CLI is AFAIK implemented using readline which has problems 
> > >    working over pipes. Actually, I never got CLI working satisfactorily
> > >    on Windows even when I just run GDB "normally" from Windows command shell (`cmd.exe`),
> > >    let alone over pipes or alike. 
> > 
> > Curious.  AFAIK native Windows gdb over cmd.exe should work fine.
> 
> Maybe I just don't know how to compile it properly. I have just compiled
> fresh c3734e093aab1ce from git (only with this patch 
> https://sourceware.org/ml/gdb-patches/2018-10/msg00614.html to make it compile
> with Python 3) using using MSYS2 MINGW64 toolchain on Windows 10. 
> This is the exact configure command:
> 
> bash ../configure --build=x86_64-w64-mingw32 --disable-werror --with-guile=no --with-python=C:\msys64\mingw64\bin\python3 --enable-targets="i686-w64-mingw32,x86_64-w64-mingw32"
> 
> Completion by tab seem to work. 
> 
> Backspace practially does not, it deletes the character in the line 
> buffer (presumably) but not on the screen. Instead, it moves the caret 
> one character on the right. Therefore what use see on the screen is not 
> what it sent to GDB when she presses enter. 
> 
> Moving cursor by left arrow followed by typing has similar issues. 
> Same for delete. Same for pressing Ctrl-R for searching the history. 

How did you invoke GDB from cmd.exe, to make these problems appear?
Can you show your exact invocation command line?
  

On Wed, 2019-03-06 at 17:45 +0200, Eli Zaretskii wrote:
> > From: Jan Vrany <jan.vrany@fit.cvut.cz>
> > Cc: gdb-patches@sourceware.org, "gdb@sourceware.org" <gdb@sourceware.org>
> > Date: Wed, 06 Mar 2019 15:09:33 +0000
> > 
> > > > 1) Completion CLI is AFAIK implemented using readline which has problems 
> > > >    working over pipes. Actually, I never got CLI working satisfactorily
> > > >    on Windows even when I just run GDB "normally" from Windows command shell (`cmd.exe`),
> > > >    let alone over pipes or alike. 
> > > 
> > > Curious.  AFAIK native Windows gdb over cmd.exe should work fine.
> > 
> > Maybe I just don't know how to compile it properly. I have just compiled
> > fresh c3734e093aab1ce from git (only with this patch 
> > https://sourceware.org/ml/gdb-patches/2018-10/msg00614.html to make it compile
> > with Python 3) using using MSYS2 MINGW64 toolchain on Windows 10. 
> > This is the exact configure command:
> > 
> > bash ../configure --build=x86_64-w64-mingw32 --disable-werror --with-guile=no --with-python=C:\msys64\mingw64\bin\python3 --enable-targets="i686-w64-mingw32,x86_64-w64-mingw32"
> > 
> > Completion by tab seem to work. 
> > 
> > Backspace practially does not, it deletes the character in the line 
> > buffer (presumably) but not on the screen. Instead, it moves the caret 
> > one character on the right. Therefore what use see on the screen is not 
> > what it sent to GDB when she presses enter. 
> > 
> > Moving cursor by left arrow followed by typing has similar issues. 
> > Same for delete. Same for pressing Ctrl-R for searching the history. 
> 
> How did you invoke GDB from cmd.exe, to make these problems appear?
> Can you show your exact invocation command line?

Just like:

H:\Projects\gdb\master\build-x86_64-msys2\gdb>gdb.exe

HTH, 

Jan
  

> From: Jan Vrany <jan.vrany@fit.cvut.cz>
> Cc: palves@redhat.com, tom@tromey.com, gdb-patches@sourceware.org,
>         gdb@sourceware.org
> Date: Wed, 06 Mar 2019 16:36:39 +0000
> 
> > > Completion by tab seem to work. 
> > > 
> > > Backspace practially does not, it deletes the character in the line 
> > > buffer (presumably) but not on the screen. Instead, it moves the caret 
> > > one character on the right. Therefore what use see on the screen is not 
> > > what it sent to GDB when she presses enter. 
> > > 
> > > Moving cursor by left arrow followed by typing has similar issues. 
> > > Same for delete. Same for pressing Ctrl-R for searching the history. 
> > 
> > How did you invoke GDB from cmd.exe, to make these problems appear?
> > Can you show your exact invocation command line?
> 
> Just like:
> 
> H:\Projects\gdb\master\build-x86_64-msys2\gdb>gdb.exe

But this doesn't create any pipes to communicate with GDB.  Instead,
this communicates via the default stdin/stdout connected to the
console, and GDB should recognize it as such.  So all the problems you
describe should not happen, and indeed don't happen for me when I
invoke GDB from cmd.exe.

Do you perhaps have a ~/.inputrc file, or some other local
customization, which might affect how Readline works?  because what
you describe surely sounds like broken Readline.
  

This is a rework of v3 patch based on further Pedro's comments
and on Eli's comments

Differences v3 -> v4:

  * fixed "few nits" based on Petro's comments
  * fixed/improved documentation based on Eli's comments

Differences v2 -> v3:

  * to support wildmatching, the command output had to change to
    provide both, "completion" (what a frontend should complete)
    and "matches" (a list of possible completions)
  * and some formatting issues.

Differences v1 -> v2:

  * extracted common completion logic to a new helper function
  * implemented MI command using a new mi-specific function rather
    than using CLI implementation.


Jan Vrany (2):
  MI: extract command completion logic from complete_command()
  MI: Add new command -complete

 gdb/ChangeLog                        | 14 ++++++
 gdb/NEWS                             |  7 +++
 gdb/cli/cli-cmds.c                   | 29 +----------
 gdb/completer.c                      | 35 ++++++++++++++
 gdb/completer.h                      |  7 +++
 gdb/doc/ChangeLog                    |  5 ++
 gdb/doc/gdb.texinfo                  | 61 +++++++++++++++++++++++
 gdb/mi/mi-cmds.c                     |  1 +
 gdb/mi/mi-cmds.h                     |  1 +
 gdb/mi/mi-main.c                     | 45 +++++++++++++++++
 gdb/testsuite/ChangeLog              |  5 ++
 gdb/testsuite/gdb.mi/mi-complete.cc  | 40 ++++++++++++++++
 gdb/testsuite/gdb.mi/mi-complete.exp | 72 ++++++++++++++++++++++++++++
 13 files changed, 294 insertions(+), 28 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-complete.cc
 create mode 100644 gdb/testsuite/gdb.mi/mi-complete.exp
  

This is a rework of v4 patch based on further Pedro's comments
and on Eli's comments

Differences v4 -> v5:

  * removed "\n" from error message
  * fixed typo and long lines in documentation

Differences v3 -> v4:

  * fixed "few nits" based on Petro's comments
  * fixed/improved documentation based on Eli's comments

Differences v2 -> v3:

  * to support wildmatching, the command output had to change to
    provide both, "completion" (what a frontend should complete)
    and "matches" (a list of possible completions)
  * and some formatting issues.

Differences v1 -> v2:

  * extracted common completion logic to a new helper function
  * implemented MI command using a new mi-specific function rather
    than using CLI implementation.


Jan Vrany (2):
  MI: extract command completion logic from complete_command()
  MI: Add new command -complete

 gdb/ChangeLog                        | 14 ++++++
 gdb/NEWS                             |  7 +++
 gdb/cli/cli-cmds.c                   | 29 +----------
 gdb/completer.c                      | 35 ++++++++++++++
 gdb/completer.h                      |  7 +++
 gdb/doc/ChangeLog                    |  5 ++
 gdb/doc/gdb.texinfo                  | 65 +++++++++++++++++++++++++
 gdb/mi/mi-cmds.c                     |  1 +
 gdb/mi/mi-cmds.h                     |  1 +
 gdb/mi/mi-main.c                     | 45 +++++++++++++++++
 gdb/testsuite/ChangeLog              |  5 ++
 gdb/testsuite/gdb.mi/mi-complete.cc  | 40 ++++++++++++++++
 gdb/testsuite/gdb.mi/mi-complete.exp | 72 ++++++++++++++++++++++++++++
 13 files changed, 298 insertions(+), 28 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-complete.cc
 create mode 100644 gdb/testsuite/gdb.mi/mi-complete.exp
  

This version mainly addresses comments raised by Simon. Tested with both
Python 2.7 and Python 3(.7).

Changes v2 -> v3

* addressed Simon's comments:
  - https://sourceware.org/ml/gdb-patches/2019-05/msg00406.html
  - https://sourceware.org/ml/gdb-patches/2019-05/msg00407.html
* checked that it compiles with both Python 2.7 and Python 3(.7)
* add tests for MI command returning dictionary with non-string
  keys
* add tests for creating MI commands with invalid names

Changes v1 -> v2

* squashed following commits into one:
  - 8e65585bdab0 Create MI commands using python.
  - 9e2afadd2d4d03 mi/python: C++ify python MI command handling code
  - 0491a0634f7de4 mi/python: Polish MI output of python commands
* moved class mi_command_py to python code
* use gdbpy_ref<> instead of void* to hold on Python object
  in mi_command_py
* release python objects held on by mi_command_py objects
  when finalizing Python
* merged py_mi_invoke() into mi_command_py::invoke()
* added missing copyright headers
* dropped micmdpy_parse_command_name()
* prefixed error messages by MI command name except for
  "Problem parsing arguments:" since old C-implemented MI
  commands do not prefix command name.
* make do_suppress_notification() to return gdb::optional
* split invoke to invoke and do_invoke, making invoke to perform
  common initialization
* add test redefining Python MI command while the very same command
  is running
* documented the fact that in mi_command_py::do_invoke() `this` pointer cannot
  be used after invoking python code since it may got stale.


Didier Nadeau (3):
  Use std::map for MI commands in mi-cmds.c
  Use classes to represent MI Command instead of structures
  Create MI commands using python.

Jan Vrany (2):
  mi/python: Allow redefinition of python MI commands
  mi/python: Add tests for python-defined MI commands

 gdb/ChangeLog                          |  88 +++++
 gdb/Makefile.in                        |   1 +
 gdb/mi/mi-cmd-info.c                   |   4 +-
 gdb/mi/mi-cmds.c                       | 510 +++++++++++++------------
 gdb/mi/mi-cmds.h                       |  87 ++++-
 gdb/mi/mi-main.c                       |  22 +-
 gdb/mi/mi-main.h                       |   1 +
 gdb/mi/mi-parse.c                      |  20 +-
 gdb/mi/mi-parse.h                      |   6 +-
 gdb/python/py-micmd.c                  | 307 +++++++++++++++
 gdb/python/py-micmd.h                  |  60 +++
 gdb/python/python-internal.h           |   2 +
 gdb/python/python.c                    |  13 +-
 gdb/testsuite/ChangeLog                |   5 +
 gdb/testsuite/gdb.python/py-mi-cmd.exp | 132 +++++++
 gdb/testsuite/gdb.python/py-mi-cmd.py  |  54 +++
 16 files changed, 1007 insertions(+), 305 deletions(-)
 create mode 100644 gdb/python/py-micmd.c
 create mode 100644 gdb/python/py-micmd.h
 create mode 100644 gdb/testsuite/gdb.python/py-mi-cmd.exp
 create mode 100644 gdb/testsuite/gdb.python/py-mi-cmd.py
  

Polite ping. 

Thanks! Jan

On Thu, 2019-05-30 at 14:48 +0100, Jan Vrany wrote:
> This version mainly addresses comments raised by Simon. Tested with both
> Python 2.7 and Python 3(.7).
> 
> Changes v2 -> v3
> 
> * addressed Simon's comments:
>   - https://sourceware.org/ml/gdb-patches/2019-05/msg00406.html
>   - https://sourceware.org/ml/gdb-patches/2019-05/msg00407.html
> * checked that it compiles with both Python 2.7 and Python 3(.7)
> * add tests for MI command returning dictionary with non-string
>   keys
> * add tests for creating MI commands with invalid names
> 
> Changes v1 -> v2
> 
> * squashed following commits into one:
>   - 8e65585bdab0 Create MI commands using python.
>   - 9e2afadd2d4d03 mi/python: C++ify python MI command handling code
>   - 0491a0634f7de4 mi/python: Polish MI output of python commands
> * moved class mi_command_py to python code
> * use gdbpy_ref<> instead of void* to hold on Python object
>   in mi_command_py
> * release python objects held on by mi_command_py objects
>   when finalizing Python
> * merged py_mi_invoke() into mi_command_py::invoke()
> * added missing copyright headers
> * dropped micmdpy_parse_command_name()
> * prefixed error messages by MI command name except for
>   "Problem parsing arguments:" since old C-implemented MI
>   commands do not prefix command name.
> * make do_suppress_notification() to return gdb::optional
> * split invoke to invoke and do_invoke, making invoke to perform
>   common initialization
> * add test redefining Python MI command while the very same command
>   is running
> * documented the fact that in mi_command_py::do_invoke() `this` pointer cannot
>   be used after invoking python code since it may got stale.
> 
> 
> Didier Nadeau (3):
>   Use std::map for MI commands in mi-cmds.c
>   Use classes to represent MI Command instead of structures
>   Create MI commands using python.
> 
> Jan Vrany (2):
>   mi/python: Allow redefinition of python MI commands
>   mi/python: Add tests for python-defined MI commands
> 
>  gdb/ChangeLog                          |  88 +++++
>  gdb/Makefile.in                        |   1 +
>  gdb/mi/mi-cmd-info.c                   |   4 +-
>  gdb/mi/mi-cmds.c                       | 510 +++++++++++++------------
>  gdb/mi/mi-cmds.h                       |  87 ++++-
>  gdb/mi/mi-main.c                       |  22 +-
>  gdb/mi/mi-main.h                       |   1 +
>  gdb/mi/mi-parse.c                      |  20 +-
>  gdb/mi/mi-parse.h                      |   6 +-
>  gdb/python/py-micmd.c                  | 307 +++++++++++++++
>  gdb/python/py-micmd.h                  |  60 +++
>  gdb/python/python-internal.h           |   2 +
>  gdb/python/python.c                    |  13 +-
>  gdb/testsuite/ChangeLog                |   5 +
>  gdb/testsuite/gdb.python/py-mi-cmd.exp | 132 +++++++
>  gdb/testsuite/gdb.python/py-mi-cmd.py  |  54 +++
>  16 files changed, 1007 insertions(+), 305 deletions(-)
>  create mode 100644 gdb/python/py-micmd.c
>  create mode 100644 gdb/python/py-micmd.h
>  create mode 100644 gdb/testsuite/gdb.python/py-mi-cmd.exp
>  create mode 100644 gdb/testsuite/gdb.python/py-mi-cmd.py
>