[patch/rfc] mi interpreter-complete enh req 8058.

[patch/rfc] mi interpreter-complete enh req 8058.

[Matt Rice]

[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]

Attached is a patch i've ported from Apple's gdb...
it implements a tab-completion like function for MI

http://sourceware.org/bugzilla/show_bug.cgi?id=8058

I figured I would post a basic port of the patch for discussion,
before doing to much to it.
there are some things I don't like....

There is no way to know why interp_complete is returning 0:
does the interpreter have a complete_proc function, or did the
complete_proc function fail.

I suppose we could return 0 or -1...

it only implements complete_proc for the console interpreter,
why then 'interpreter-complete' if only the console interpreter implements it?
it has arbitrary limits of 200 completions which has never really been
a part of the gnu credo:

one idea is to introduce a limit argument to the mi command, but that
brings up the idea of bringing in a starting point for resuming
completion, and getting multiple batches

without some form of a continuation it'd have to redo the completion each batch,
it seems difficult/impossible to invalidate the completion e.g. if
loading/completing symbols

so maybe we should forgo the batching but add the limit to the mi command?

I also notice it doesn't implement the all? argument that he mentioned
in the bug report.
also need to add a  -list-features thing...

adding arguments and things obviously would break compatibility with
their gdb, is this an issue?
let me know if you guys have any opinions on these or any other issues....

-interpreter-complete console "b ma"
^done,completions=[c="main",c="malloc",c="malloc@plt"]
(gdb)
-interpreter-complete console "b ma" 1
^done,completions=[c="backtrace",c="break",c="bt"]
(gdb)

[-- Attachment #2: 0001-Add-interpreter-complete-mi-function.patch --]
[-- Type: application/octet-stream, Size: 9325 bytes --]

From 3dff0ffc8f2bebe59d44a496f8e864e234151457 Mon Sep 17 00:00:00 2001
From: Matt Rice <ratmice@gmail.com>
Date: Fri, 2 Oct 2009 08:06:52 -0700
Subject: [PATCH] Add interpreter-complete mi function.

2009-10-02  Matt Rice  <ratmice@gmail.com>
	    Jason Molenda  <jmolenda@apple.com>
	    Jim Ingham  <jingham@apple.com>

	PR gdb/8058
	* cli/cli-cmds.c (cli_interpreter_complete): New function.
	(complete_command): Change to use Call cli_interpreter_complete.
	* cli/cli-cmds.h: Declare cli_interpreter_complete.
	* cli/cli-interp.c (_initialize_cli_interp): Add
	cli_interpreter_complete to interp_procs.
	* interps.c (interp_complete): New function.
	* interps.h: Declare interp_complete, interp_complete_ftype and
	complete_proc.
	* mi/mi-cmds.c (mi_cmds): Add interpreter-complete MI command.
	* mi/mi-cmds.h: Declare mi_cmd_interpreter_complete.
	* mi/mi-interp.c (mi_cmd_interpreter_complete): Add
	interpreter-complete MI command implementation.
---
 gdb/cli/cli-cmds.c   |   37 +++++++++++++++++++++++++++++++++++--
 gdb/cli/cli-cmds.h   |    4 ++++
 gdb/cli/cli-interp.c |    5 ++++-
 gdb/interps.c        |   13 +++++++++++++
 gdb/interps.h        |    8 ++++++++
 gdb/mi/mi-cmds.c     |    1 +
 gdb/mi/mi-cmds.h     |    1 +
 gdb/mi/mi-interp.c   |   44 ++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 110 insertions(+), 3 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index ce7c2a6..4734fc5 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -238,14 +238,21 @@ complete_command (char *arg, int from_tty)
 {
   int i;
   int argpoint;
-  char **completions, *point, *arg_prefix;
 
   dont_repeat ();
 
   if (arg == NULL)
     arg = "";
   argpoint = strlen (arg);
+  cli_interpreter_complete (NULL, arg, arg, argpoint, -1);
+}
 
+int
+cli_interpreter_complete (void *data, char *arg, char *command_buffer,
+                          int argpoint, int limit)
+{
+  char **completions, *point, *arg_prefix;
+  struct cleanup *old_chain;
   /* complete_line assumes that its first argument is somewhere within,
      and except for filenames at the beginning of, the word to be completed.
      The following crude imitation of readline's word-breaking tries to
@@ -254,7 +261,7 @@ complete_command (char *arg, int from_tty)
   while (point > arg)
     {
       if (strchr (rl_completer_word_break_characters, point[-1]) != 0)
-        break;
+	break;
       point--;
     }
 
@@ -263,10 +270,12 @@ complete_command (char *arg, int from_tty)
   arg_prefix[point - arg] = 0;
 
   completions = complete_line (point, arg, argpoint);
+  old_chain = make_cleanup_ui_out_list_begin_end (uiout, "completions");
 
   if (completions)
     {
       int item, size;
+      int items_printed = 0;
 
       for (size = 0; completions[size]; ++size)
 	;
@@ -278,6 +287,11 @@ complete_command (char *arg, int from_tty)
       while (item < size)
 	{
 	  int next_item;
+
+	  ui_out_field_string (uiout, "c", completions[item]);
+	  ui_out_text (uiout, "\n");
+	  items_printed++;
+
 	  printf_unfiltered ("%s%s\n", arg_prefix, completions[item]);
 	  next_item = item + 1;
 	  while (next_item < size
@@ -289,10 +303,29 @@ complete_command (char *arg, int from_tty)
 
 	  xfree (completions[item]);
 	  item = next_item;
+
+	  if (limit != -1 && items_printed >= limit)
+	    {
+	      int i;
+
+	      for (i = item; i < size && completions[i]; i++)
+		{
+		  xfree (completions[i]);
+		  completions[i] = NULL;
+		}
+
+	      size = item;
+	      if (ui_out_is_mi_like_p (uiout))
+		ui_out_field_string (uiout, "limited", "true");
+	      break;
+	    }
 	}
 
       xfree (completions);
     }
+
+  do_cleanups (old_chain);
+  return 1;
 }
 
 int
diff --git a/gdb/cli/cli-cmds.h b/gdb/cli/cli-cmds.h
index 3260fe8..a13b68f 100644
--- a/gdb/cli/cli-cmds.h
+++ b/gdb/cli/cli-cmds.h
@@ -123,6 +123,10 @@ extern void quit_command (char *, int);
 
 extern void source_script (char *, int);
 
+/* Exported to cli-interp.c */
+
+extern int cli_interpreter_complete (void *, char *, char *, int, int);
+
 /* Used everywhere whenever at least one parameter is required and
   none is specified. */
 
diff --git a/gdb/cli/cli-interp.c b/gdb/cli/cli-interp.c
index 91092ed..ce32648 100644
--- a/gdb/cli/cli-interp.c
+++ b/gdb/cli/cli-interp.c
@@ -22,6 +22,7 @@
 #include "wrapper.h"
 #include "event-top.h"
 #include "ui-out.h"
+#include "cli-cmds.h"
 #include "cli-out.h"
 #include "top.h"		/* for "execute_command" */
 #include "gdb_string.h"
@@ -147,7 +148,9 @@ _initialize_cli_interp (void)
     cli_interpreter_resume,	/* resume_proc */
     cli_interpreter_suspend,	/* suspend_proc */
     cli_interpreter_exec,	/* exec_proc */
-    cli_interpreter_display_prompt_p	/* prompt_proc_p */
+    cli_interpreter_display_prompt_p,	/* prompt_proc_p */
+    NULL,
+    cli_interpreter_complete	/* complete_proc */
   };
   struct interp *cli_interp;
 
diff --git a/gdb/interps.c b/gdb/interps.c
index da05ee2..40ba04e 100644
--- a/gdb/interps.c
+++ b/gdb/interps.c
@@ -328,6 +328,19 @@ interp_exec (struct interp *interp, const char *command_str)
   return exception_none;
 }
 
+int
+interp_complete (struct interp *interp,
+                 char *word, char *command_buffer, int cursor, int limit)
+{
+  if (interp->procs->complete_proc != NULL)
+    {
+      return interp->procs->complete_proc (interp->data, word, command_buffer,
+                                           cursor, limit);
+    }
+
+  return 0;
+}
+
 /* A convenience routine that nulls out all the common command hooks.
    Use it when removing your interpreter in its suspend proc.  */
 void
diff --git a/gdb/interps.h b/gdb/interps.h
index de736ce..cab745e 100644
--- a/gdb/interps.h
+++ b/gdb/interps.h
@@ -36,6 +36,10 @@ extern struct gdb_exception interp_exec (struct interp *interp,
 					 const char *command);
 extern int interp_quiet_p (struct interp *interp);
 
+extern int interp_complete (struct interp *interp,
+			    char *word, char *command_buffer,
+			    int cursor, int limit);
+
 typedef void *(interp_init_ftype) (int top_level);
 typedef int (interp_resume_ftype) (void *data);
 typedef int (interp_suspend_ftype) (void *data);
@@ -43,6 +47,9 @@ typedef int (interp_prompt_p_ftype) (void *data);
 typedef struct gdb_exception (interp_exec_ftype) (void *data,
 						  const char *command);
 typedef void (interp_command_loop_ftype) (void *data);
+typedef int (interp_complete_ftype) (void *data, char *word,
+				     char *command_buffer, int cursor,
+				     int limit);
 
 struct interp_procs
 {
@@ -52,6 +59,7 @@ struct interp_procs
   interp_exec_ftype *exec_proc;
   interp_prompt_p_ftype *prompt_proc_p;
   interp_command_loop_ftype *command_loop_proc;
+  interp_complete_ftype *complete_proc;
 };
 
 extern struct interp *interp_new (const char *name, void *data,
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 2c425b7..26f74fd 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -81,6 +81,7 @@ struct mi_cmd mi_cmds[] =
   { "inferior-tty-set", { NULL, 0 }, mi_cmd_inferior_tty_set},
   { "inferior-tty-show", { NULL, 0 }, mi_cmd_inferior_tty_show},
   { "interpreter-exec", { NULL, 0 }, mi_cmd_interpreter_exec},
+  { "interpreter-complete", { NULL, 0 }, mi_cmd_interpreter_complete},
   { "list-features", { NULL, 0 }, mi_cmd_list_features},
   { "list-target-features", { NULL, 0 }, mi_cmd_list_target_features},
   { "list-thread-groups", { NULL, 0 }, mi_cmd_list_thread_groups },  
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 491c1f2..d8cfc6d 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -67,6 +67,7 @@ extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
 extern mi_cmd_argv_ftype mi_cmd_inferior_tty_set;
 extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show;
 extern mi_cmd_argv_ftype mi_cmd_interpreter_exec;
+extern mi_cmd_argv_ftype mi_cmd_interpreter_complete;
 extern mi_cmd_argv_ftype mi_cmd_list_features;
 extern mi_cmd_argv_ftype mi_cmd_list_target_features;
 extern mi_cmd_argv_ftype mi_cmd_list_thread_groups;
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index 5ad4a51..81c7523 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -504,6 +504,50 @@ mi_solib_unloaded (struct so_list *solib)
   gdb_flush (mi->event_channel);
 }
 
+/* This implements the "interpreter complete command" which takes an
+   interpreter, a command string, and optionally a cursor position
+   within the command, and completes the string based on that interpreter's
+   completion function.  */
+
+void
+mi_cmd_interpreter_complete (char *command, char **argv, int argc)
+{
+  struct interp *interp_to_use;
+  int cursor;
+  int limit = 200;
+
+  if (argc < 2 || argc > 3)
+    {
+      error("Wrong # or arguments, should be \"%s interp command <cursor>\".",
+	    command);
+      return;
+    }
+
+  interp_to_use = interp_lookup (argv[0]);
+  if (interp_to_use == NULL)
+    {
+      error("Could not find interpreter \"%s\".", argv[0]);
+      return;
+    }
+
+  if (argc == 3)
+    {
+      cursor = atoi (argv[2]);
+    }
+  else
+    {
+      cursor = strlen (argv[1]);
+    }
+
+  if (interp_complete (interp_to_use, argv[1], argv[1], cursor, limit) == 0)
+    {
+      error("Unable to complete.");
+      return;
+    }
+  else
+    return;
+
+}
 
 extern initialize_file_ftype _initialize_mi_interp; /* -Wmissing-prototypes */
 
-- 
1.6.2.5

Re: [patch/rfc] mi interpreter-complete enh req 8058.

[Tom Tromey]

>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:

Matt> Attached is a patch i've ported from Apple's gdb...
Matt> it implements a tab-completion like function for MI
Matt> http://sourceware.org/bugzilla/show_bug.cgi?id=8058
Matt> I figured I would post a basic port of the patch for discussion,
Matt> before doing to much to it.

Thanks.

Matt> it only implements complete_proc for the console interpreter, why
Matt> then 'interpreter-complete' if only the console interpreter
Matt> implements it?

I have no idea.

Matt> it has arbitrary limits of 200 completions which has never really been
Matt> a part of the gnu credo:
Matt> one idea is to introduce a limit argument to the mi command, but that
Matt> brings up the idea of bringing in a starting point for resuming
Matt> completion, and getting multiple batches

Matt> without some form of a continuation it'd have to redo the
Matt> completion each batch, it seems difficult/impossible to invalidate
Matt> the completion e.g. if loading/completing symbols

Matt> so maybe we should forgo the batching but add the limit to the mi command?

Yeah, some solution here would be nice to have.
This is related to http://sourceware.org/bugzilla/show_bug.cgi?id=9007

I suppose some kind of continuation is not out of the question.

Matt> I also notice it doesn't implement the all? argument that he mentioned
Matt> in the bug report.

If you don't need that, you can ignore it, IMO.

Matt> also need to add a  -list-features thing...

I think it is probably fine not to do that and just let clients catch
the error coming from older GDBs.

Matt> adding arguments and things obviously would break compatibility with
Matt> their gdb, is this an issue?

IMO, it is not an issue.  But, if it is easy to be compatible, we might
as well be.

Matt> -interpreter-complete console "b ma"
Matt> ^done,completions=[c="main",c="malloc",c="malloc@plt"]

It is weird to see all those useless "c=" strings in there :-)

The patch seems reasonable enough.  There are some GNU style issues to
clean up.  And, this would need an update to the manual.

Tom

Re: [patch/rfc] mi interpreter-complete enh req 8058.

[Nick Roberts]

Tom Tromey writes:
 > >>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:
 > 
 > Matt> Attached is a patch i've ported from Apple's gdb...
 > Matt> it implements a tab-completion like function for MI
 > Matt> http://sourceware.org/bugzilla/show_bug.cgi?id=8058
 > Matt> I figured I would post a basic port of the patch for discussion,
 > Matt> before doing to much to it.
 > 
 > Thanks.

PR 8058 goes back to 2003.  I don't see the need for MI to duplicate all of
the CLI functionality.  Emacs just uses the CLI command "complete" which is
clearly just intended for front end use:

complete b ma
&"complete b ma\n"
~"b main\n"
~"b malloc\n"
~"b malloc@plt\n"
^done

The lisp function, gud-gdb-complete-command. is bound to TAB to it works
in the GUD buffer (Emacs' console) just as it does on the command line.

 > ...
 > Matt> -interpreter-complete console "b ma"
 > Matt> ^done,completions=[c="main",c="malloc",c="malloc@plt"]
 > 
 > It is weird to see all those useless "c=" strings in there :-)

Well, if they are going to be present, could we please stick to the syntax
suggested in the manual:

    `TUPLE ==>'
         ` "{}" | "{" RESULT ( "," RESULT )* "}" '

and

   * New GDB/MI commands should only output LISTS containing VALUES.

So either

    ^done,completions={c="main",c="malloc",c="malloc@plt"}

or

    ^done,completions=["main","malloc","malloc@plt"]


-- 
Nick                                           http://users.snap.net.nz/~nickrob

Re: [patch/rfc] mi interpreter-complete enh req 8058.

[Matt Rice]

On Fri, Oct 2, 2009 at 4:10 PM, Nick Roberts <nickrob@snap.net.nz> wrote:

<snip>

> PR 8058 goes back to 2003.  I don't see the need for MI to duplicate all of
> the CLI functionality.  Emacs just uses the CLI command "complete" which is
> clearly just intended for front end use:
>
> complete b ma
> &"complete b ma\n"
> ~"b main\n"
> ~"b malloc\n"
> ~"b malloc@plt\n"
> ^done
>
> The lisp function, gud-gdb-complete-command. is bound to TAB to it works
> in the GUD buffer (Emacs' console) just as it does on the command line.

Thanks, I had missed this CLI command somehow, it will work for now,
ideally i would like it not to be going directly to stdout like that
output infers, I guess i could change the output filehandle, when
executing this command until it finishes,  so that the output history
isn't filled with this temporal information which always bothers me...

additionally its worth noting the patch screws this output up.

Re: [patch/rfc] mi interpreter-complete enh req 8058.

[Nick Roberts]

 > Thanks, I had missed this CLI command somehow, it will work for now,
 > ideally i would like it not to be going directly to stdout like that
 > output infers, I guess i could change the output filehandle, when
 > executing this command until it finishes,  

I'm not sure what you mean.  Everything goes to stdout but you can
eliminate the LOG-STREAM-OUTPUT (prefixed by `&') by using
-interpreter-exec:

(gdb) 
-interpreter-exec console "complete b ma"
~"b main\n"
~"b malloc\n"
~"b malloc@plt\n"
^done
(gdb) 


 >                                           so that the output history
 > isn't filled with this temporal information which always bothers me...

You can use the server prefix to suppress commands being entered into
the command history.

 > additionally its worth noting the patch screws this output up.

Does that not show up in the testsuite?

What front end are you working on?


-- 
Nick                                           http://users.snap.net.nz/~nickrob

Re: [patch/rfc] mi interpreter-complete enh req 8058.

[Matt Rice]

On Sat, Oct 3, 2009 at 5:12 PM, Nick Roberts <nickrob@snap.net.nz> wrote:
>  > Thanks, I had missed this CLI command somehow, it will work for now,
>  > ideally i would like it not to be going directly to stdout like that
>  > output infers, I guess i could change the output filehandle, when
>  > executing this command until it finishes,
>
> I'm not sure what you mean.  Everything goes to stdout but you can
> eliminate the LOG-STREAM-OUTPUT (prefixed by `&') by using
> -interpreter-exec:
>
> (gdb)
> -interpreter-exec console "complete b ma"
> ~"b main\n"
> ~"b malloc\n"
> ~"b malloc@plt\n"
> ^done
> (gdb)
>

I meant the ~"b main\n" etc,

"~" string-output
    The console output stream contains text that should be displayed
in the CLI console window. It contains the textual responses to CLI
commands.

I would like this output not to go to the CLI console window, but to a
new window or a new tab, which switches back to the console window
upon hitting 'enter', but is also a list control,allowing you to
select the tab completion result with the mouse or entering it by
hand, using the complete CLI command means monkeying around at the MI
parser level, parsing the string output based on the command executed.

anyhow, complete CLI command will suffice for now.

>  > so that the output history
>  > isn't filled with this temporal information which always bothers me...
>
> You can use the server prefix to suppress commands being entered into
> the command history.

ahh, sorry i didn't mean that history, I meant the 'CLI console output
stream', but noted.

>  > additionally its worth noting the patch screws this output up.
>
> Does that not show up in the testsuite?

no, it looks like additional unwanted output when executing the
complete command, the output being tested for is still there so the
tests still pass i gather.

> What front end are you working on?

one which doesn't really have any tangible existence  I just saw this
on the bugzilla, and taking a break from other patches...

initially trying to do a faithful reproduction of the CLI, then then
augmenting it with some gui stuff, but mainly keeping the console
roots, no built in editor or fancy stuff.