Re: [patch] [python] Prompt substitution

[Phil Muldoon <pmuldoon at redhat dot com>]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> +@findex gdb.get_current_prompt
> Phil> +@defun get_current_prompt
> Phil> +Return the current @value{GDBN} prompt as a string.
> Phil> +@end defun
>
> Is this different from gdb.parameter('prompt')?

Nope, it is just a codified API to set the prompt, and one I used in the
testsuite.  We can get rid of it, I have no strong feelings.

> Phil> +@findex gdb.set_current_prompt
> Phil> +@defun set_current_prompt @r{[}new_prompt@r{]}
> Phil> +Sets the @value{GDBN} prompt to @var{new_prompt}.  @var{new_prompt}
> Phil> +must be a string.  This method has no return value.
> Phil> +@end defun
>
> I'm on the fence about this.  It would probably be better to have a
> general facility for setting parameters.

See above.  If it is a problem, lets just remove it.  The current
attached patch has these hunks removed.

> Phil> +@defop Operation {@value{GDBN}} prompt_hook [current_prompt]
> Phil> +If @var{prompt_hook} exists, and is not set to @code{None},
>
> ... really if it is callable.
>
> Phil> +The parameter @code{current_prompt} contains the current @value{GDBN} 
> Phil> +prompt.
>
> If the parameter is not optional, then it should not be surrounded in []
> in the @defop.

Ok to both doc changes.

> Phil> +      observer_notify_before_prompt (pre_gdb_prompt);
> Phil> +      post_gdb_prompt = get_prompt ();
> Phil> +
> Phil> +      /* If the observer changed the prompt, use that prompt overriding
> Phil> +	 any new_prompt that was passed to this function.  */
> Phil> +      if (strcmp (pre_gdb_prompt, post_gdb_prompt) != 0)

> Suppose an observer sets the prompt.
> Won't this free the memory pointed to by pre_gdb_prompt?
> Thus making this strcmp access invalid memory?

Yes you are right.  We should xstrdup pre_gdb_prompt and later free
it.  Sigh. Sorry for bothering you with such an elementary mistake.


> Phil> +	  if (current_gdb_prompt == NULL)
> Phil> +	    {
>
> When can this happen?

Never, I'll remove it.

> Phil> +	      current_prompt =  PyString_FromString (current_gdb_prompt);
>
> Extra space after the '='.
>
> Phil> +	  if ((result != Py_None) && (! PyString_Check (result)))
>
> Too many parens.
>
> Phil> +  const char *prompt = get_prompt ();
> Phil> +  if (prompt)
> Phil> +    return PyString_FromString (prompt);
>
> Likewise about whether this can happen.
>
> Phil> +  { "flush", (PyCFunction)gdbpy_flush, METH_VARARGS | METH_KEYWORDS,
> Phil> +    "Flush gdb's filtered stdout stream." },

This is really weird.  This hunk is not in my patch on disk, or in
the git tree.  Anyway, yes, I will get rid of it.

> This is already in the code and shouldn't be in this patch.
>
> Phil> +      PROMPT (0) =  xstrdup (s);
>
> Extra space.

Ok to this, and other formatting nits.

Current patch attached.

Cheers

Phil

--

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 3a3a9fb..5b26bbb 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21080,6 +21080,22 @@ provided, it is decoded the way that @value{GDBN}'s inbuilt
 @code{break} or @code{edit} commands do (@pxref{Specify Location}).
 @end defun
 
+@defop Operation {@value{GDBN}} prompt_hook current_prompt
+If @var{prompt_hook} is callable, @value{GDBN} will call the method
+assigned to this operation before a prompt is displayed by
+@value{GDBN}.
+
+The parameter @code{current_prompt} contains the current @value{GDBN} 
+prompt.  This method must return a Python string, or @code{None}.  If
+a string is returned, the @value{GDBN} prompt will be set to that
+string.  If @code{None} is returned, @value{GDBN} will continue to use
+the current prompt.
+
+Some prompts cannot be substituted in @value{GDBN}.  Secondary prompts
+such as those used by readline for command input, and annotation
+related prompts are prohibited from being changed.
+@end defop
+
 @node Exception Handling
 @subsubsection Exception Handling
 @cindex python exceptions
diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
index d8c3924..689b303 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -222,6 +222,11 @@ Bytes from @var{data} to @var{data} + @var{len} have been written
 to the current inferior at @var{addr}.
 @end deftypefun
 
+@deftypefun void before_prompt (const char *@var{current_prompt})
+Called before a top-level prompt is displayed.  @var{current_prompt} is
+the current top-level prompt.
+@end deftypefun
+
 @deftypefun void test_notification (int @var{somearg})
 This observer is used for internal testing.  Do not use.  
 See testsuite/gdb.gdb/observer.exp.
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 72cbfdc..21fa425 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -33,6 +33,7 @@
 #include "cli/cli-script.h"     /* for reset_command_nest_depth */
 #include "main.h"
 #include "gdbthread.h"
+#include "observer.h"
 #include "continuations.h"
 #include "gdbcmd.h"		/* for dont_repeat() */
 
@@ -258,7 +259,7 @@ void
 display_gdb_prompt (char *new_prompt)
 {
   int prompt_length = 0;
-  char *gdb_prompt = get_prompt ();
+  char *actual_gdb_prompt = NULL;
 
   /* Reset the nesting depth used when trace-commands is set.  */
   reset_command_nest_depth ();
@@ -268,6 +269,25 @@ display_gdb_prompt (char *new_prompt)
   if (!current_interp_display_prompt_p ())
     return;
 
+  /* Get the prompt before the observers are called as observer hook
+     functions may change the prompt.  Do not call observers on an
+     explicit prompt change as passed to this function, as this forms
+     a temporary prompt, IE, displayed but not set.  */
+  if (! new_prompt)
+    {
+      char *post_gdb_prompt = NULL;
+      char *pre_gdb_prompt = xstrdup (get_prompt ());
+
+      observer_notify_before_prompt (pre_gdb_prompt);
+      post_gdb_prompt = get_prompt ();
+
+      /* If the observer changed the prompt, use that prompt.  */
+      if (strcmp (pre_gdb_prompt, post_gdb_prompt) != 0)
+	actual_gdb_prompt = post_gdb_prompt;
+      
+      xfree (pre_gdb_prompt);
+    }
+
   if (sync_execution && is_running (inferior_ptid))
     {
       /* This is to trick readline into not trying to display the
@@ -289,27 +309,35 @@ display_gdb_prompt (char *new_prompt)
       return;
     }
 
-  if (!new_prompt)
+  /* If the observer changed the prompt, ACTUAL_GDB_PROMPT will not be
+     NULL.  Otherwise, either copy the existing prompt, or set it to
+     NEW_PROMPT.  */
+  if (! actual_gdb_prompt)
     {
-      /* Just use the top of the prompt stack.  */
-      prompt_length = strlen (PREFIX (0)) +
-	strlen (SUFFIX (0)) +
-	strlen (gdb_prompt) + 1;
-
-      new_prompt = (char *) alloca (prompt_length);
-
-      /* Prefix needs to have new line at end.  */
-      strcpy (new_prompt, PREFIX (0));
-      strcat (new_prompt, gdb_prompt);
-      /* Suffix needs to have a new line at end and \032 \032 at
-         beginning.  */
-      strcat (new_prompt, SUFFIX (0));
+      if (! new_prompt)
+	{
+	  /* Just use the top of the prompt stack.  */
+	  prompt_length = strlen (PREFIX (0)) +
+	    strlen (SUFFIX (0)) +
+	    strlen (get_prompt()) + 1;
+
+	  actual_gdb_prompt = (char *) alloca (prompt_length);
+
+	  /* Prefix needs to have new line at end.  */
+	  strcpy (actual_gdb_prompt, PREFIX (0));
+	  strcat (actual_gdb_prompt, get_prompt());
+	  /* Suffix needs to have a new line at end and \032 \032 at
+	     beginning.  */
+	  strcat (actual_gdb_prompt, SUFFIX (0));
+	}
+      else
+	actual_gdb_prompt = new_prompt;;
     }
 
   if (async_command_editing_p)
     {
       rl_callback_handler_remove ();
-      rl_callback_handler_install (new_prompt, input_handler);
+      rl_callback_handler_install (actual_gdb_prompt, input_handler);
     }
   /* new_prompt at this point can be the top of the stack or the one
      passed in.  It can't be NULL.  */
@@ -318,7 +346,7 @@ display_gdb_prompt (char *new_prompt)
       /* Don't use a _filtered function here.  It causes the assumed
          character position to be off, since the newline we read from
          the user is not accounted for.  */
-      fputs_unfiltered (new_prompt, gdb_stdout);
+      fputs_unfiltered (actual_gdb_prompt, gdb_stdout);
       gdb_flush (gdb_stdout);
     }
 }
diff --git a/gdb/python/python.c b/gdb/python/python.c
index f15936d..e3a8d19 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -51,6 +51,7 @@ static int gdbpy_should_print_stack = 0;
 #include "version.h"
 #include "target.h"
 #include "gdbthread.h"
+#include "observer.h"
 
 static PyMethodDef GdbMethods[];
 
@@ -682,6 +683,81 @@ gdbpy_initialize_events (void)
     }
 }
 
+
+
+static void
+before_prompt_hook (const char *current_gdb_prompt)
+{
+  struct cleanup *cleanup;
+  char *prompt = NULL;
+
+  cleanup = ensure_python_env (get_current_arch (), current_language);
+
+  if (PyObject_HasAttrString (gdb_module, "prompt_hook"))
+    {
+      PyObject *hook;
+
+      hook = PyObject_GetAttrString (gdb_module, "prompt_hook");
+      if (hook == NULL)
+	goto fail;
+
+      if (PyCallable_Check (hook))
+	{
+	  PyObject *result;
+	  PyObject *current_prompt;
+
+	  current_prompt = PyString_FromString (current_gdb_prompt);
+	  if (current_prompt == NULL)
+	    goto fail;
+
+	  result = PyObject_CallFunctionObjArgs (hook, current_prompt, NULL);
+
+	  Py_DECREF (current_prompt);
+
+	  if (result == NULL)
+	    goto fail;
+
+	  make_cleanup_py_decref (result);
+
+	  /* Return type should be None, or a String.  If it is None,
+	     fall through, we will not set a prompt.  If it is a
+	     string, set  PROMPT.  Anything else, set an exception.  */
+	  if (result != Py_None && ! PyString_Check (result))
+	    {
+	      PyErr_Format (PyExc_RuntimeError,
+			    _("Return from prompt_hook must " \
+			      "be either a Python string, or None"));
+	      goto fail;
+	    }
+
+	  if (result != Py_None)
+	    {
+	      prompt = python_string_to_host_string (result);
+
+	      if (prompt == NULL)
+		goto fail;
+	      else
+		make_cleanup (xfree, prompt);
+	    }
+	}
+    }
+
+  /* If a prompt has been set, PROMPT will not be NULL.  If it is
+     NULL, do not set the prompt.  */
+  if (prompt != NULL)
+    set_prompt (prompt);
+
+  do_cleanups (cleanup);
+  return;
+
+ fail:
+  gdbpy_print_stack ();
+  do_cleanups (cleanup);
+  return;
+}
+
+
+
 /* Printing.  */
 
 /* A python function to write a single string using gdb's filtered
@@ -1134,6 +1210,8 @@ Enables or disables printing of Python stack traces."),
   gdbpy_initialize_exited_event ();
   gdbpy_initialize_thread_event ();
 
+  observer_attach_before_prompt (before_prompt_hook);
+
   PyRun_SimpleString ("import gdb");
   PyRun_SimpleString ("gdb.pretty_printers = []");
 
@@ -1236,6 +1314,8 @@ def GdbSetPythonDirectory (dir):\n\
 \n\
 # Install the default gdb.PYTHONDIR.\n\
 GdbSetPythonDirectory (gdb.PYTHONDIR)\n\
+# Default prompt hook does nothing.\n\
+prompt_hook = None\n\
 ");
 
   do_cleanups (cleanup);
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index 8906bbc..832afc0 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -193,3 +193,85 @@ gdb_py_test_silent_cmd "set python print-stack on" \
     "Test print-backtrace set setting" 1
 gdb_test "show python print-stack" \
     "Whether Python stack will be printed on error is on.*" \
+
+# Test prompt substituion
+
+gdb_py_test_multiple "prompt substitution" \
+  "python" "" \
+  "someCounter = 0" "" \
+  "def prompt(current):" "" \
+  "   global someCounter" "" \
+  "   if (current == \"testfake \"):" "" \
+  "      return None" "" \
+  "   someCounter = someCounter + 1" "" \
+  "   return \"py prompt \" + str (someCounter) + \" \"" "" \
+  "end" ""
+
+gdb_py_test_multiple "prompt substitution readline" \
+  "python" "" \
+  "pCounter = 0" "" \
+  "def program_prompt(current):" "" \
+  "   global pCounter" "" \
+  "   if (current == \">\"):" "" \
+  "      pCounter = pCounter + 1" "" \
+  "      return \"python line \" + str (pCounter) + \": \"" "" \
+  "   return None" "" \
+  "end" ""
+
+set newprompt "py prompt 1"
+set newprompt2 "py prompt 2"
+set testfake "testfake"
+
+gdb_test_multiple "python gdb.prompt_hook = prompt" "set the hook" {
+    -re "\[\r\n\]$newprompt $" {
+	pass "set hook"
+    }
+}
+
+gdb_test_multiple "set prompt testfake " "set testfake prompt in GDB" {
+    -re "\[\r\n\]$testfake $" {
+	pass "set prompt testfake"
+    }
+}
+
+gdb_test_multiple "show prompt" "show testfake prompt" {
+    -re "Gdb's prompt is \"$testfake \"..* $" {
+	pass "show prompt shows guarded prompt"
+    }
+}
+
+gdb_test_multiple "set prompt blah " "set blah in GDB" {
+    -re "\[\r\n\]$newprompt2 $" {
+	pass "set prompt blah overriden"
+    }
+}
+
+gdb_test_multiple "python gdb.prompt_hook = None" "Delete hook" {
+    -re "\[\r\n\]$newprompt2 $" {
+	pass "Delete old hook"
+    }
+}
+
+gdb_test_multiple "set prompt $gdb_prompt " "set default prompt" {
+    -re "\[\r\n\]$gdb_prompt $" {
+	pass "set default prompt"
+    }
+}
+
+gdb_test_multiple "python gdb.prompt_hook = program_prompt" "set the hook" {
+    -re "\[\r\n\]$gdb_prompt $" {
+	pass "set programming hook"
+    }
+}
+
+gdb_test_multiple "python" "test we ignore substituion for seconday prompts" {
+    -re "\r\n>$" {
+	pass "readline secondary are not substituted"
+    }
+}
+
+gdb_test_multiple "end" "end programming" {
+    -re "\[\r\n\]$gdb_prompt $" {
+	pass "end programming"
+    }
+}
diff --git a/gdb/top.c b/gdb/top.c
index ecb91f2..80d094d 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1133,14 +1133,21 @@ get_prompt (void)
 }
 
 void
-set_prompt (char *s)
+set_prompt (const char *s)
 {
-/* ??rehrauer: I don't know why this fails, since it looks as though
-   assignments to prompt are wrapped in calls to xstrdup...
-   if (prompt != NULL)
-     xfree (prompt);
- */
-  PROMPT (0) = xstrdup (s);
+  /* If S == PROMPT then do not free it or set it.  If we did
+     that, S (which points to PROMPT), would become garbage.  */
+
+  if (s != PROMPT (0))
+    {
+      xfree (PROMPT (0));
+      PROMPT (0) = xstrdup (s);
+
+      /* Also, free and set new_async_prompt so prompt changes sync up
+	 with set/show prompt.  */
+      xfree (new_async_prompt);
+      new_async_prompt = xstrdup (PROMPT (0));
+    }
 }
 
 
diff --git a/gdb/top.h b/gdb/top.h
index 4a6e8fb..24ec2f2 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -55,7 +55,7 @@ extern char *get_prompt (void);
 
 /* This function copies the specified string into the string that
    is used by gdb for its command prompt.  */
-extern void set_prompt (char *);
+extern void set_prompt (const char *);
 
 /* From random places.  */
 extern int readnow_symbol_files;