[patch] [python] Prompt substitution

[patch] [python] Prompt substitution

[Phil Muldoon]

I'll start  with a summary of the current patch, then explain why I
reduced the scope of that patch.

This patch allows for simple prompt substitution in GDB.  We install an
observer in display_gdb_prompt, and allow a call to a Python hook
function/class.  The hook function/class can then substitute the prompt
if it wishes to do so.

Originally I wanted this patch to be more expansive.  Some background.
GDB has several ways of handling prompts:

* Normal prompts set with either PROMPT (0), or set_prompt.

* So-called temporary or secondary prompts.  These are prompts that are
  passed to display_gdb_prompt, but never set.  (Command continuation
  markers (IE, ">") are the only users of this).

* prompt_push/prompt_pop which is used by annotations.  Annotations
  also does some other manipulation.

* set/show prompt, which stores the prompt in PROMPT (0), but shows the
  prompt from new_async_prompt.

* MI has its own prompt mechanisms.

I originally wanted two observers.  The first for when a prompt was
going to be set, and the second for when a prompt was going to be
displayed. To do this, I would have to remove the ability to set a
temporary prompt in display_gdb_prompt.

Secondly I would unify prompt setting to set_prompt, and removing
instances of direct prompt setting (IE, PROMPT (0) = xstrdup("blah")
(This incidentally leaks memory and in several instances, the previous
prompt was not freed)).

These operations turned out to be quite complex, if purely mechanical.
The biggest problem was the temporary prompt.  When you pass a value to
display_gdb_prompt it displays that prompt but does not set it.  This
allows internally a simple way to pop the prompt with
display_gdb_prompt (0) (which would just re-display PROMPT (0)).  The
issue here is that observers have to set a prompt for substitution with
set_prompt.  So if we wrote a prompt substitution function that replaced
">" with "command line n", when readline was finished with the command
input, it would pop the prompt with display_gdb_prompt (0). Of course
because our observer has to set the prompt with set_prompt, this would
not work. Popping the temporary prompt would  just re-display "command
line n".  Sigh.  I actually fixed this. Next I started to work on
annotations.

Annotations use pop_prompt/push_prompt.  I started rewriting this logic
to comply with substitution rules.

However, I soon realised I was re-writing most of the prompt code just
to expose an API that really should be unified. While I am in favour of
Python driving API re-design, I think the context should be meaningful
to both the patch and functionality we want to achieve.  So I was left
with this huge patch, and the actual Python API merely being about 5% of
the changes.

So I dropped support for secondary prompts.  I still want to implement
them, but I think the underlying plumbing needs to be improved first.  I
think all prompt changes should be pushed through set_prompt,
display_gdb_prompt should only ever display prompts, and instead of
cheat-popping your gdb prompt with temporary prompts, we should just use
pop/push prompt.  Annotations, I am not sure of.

I have a patch for the set_prompt/display_gdb_prompt which I will submit
later.  This is useful as it squashes some memory leaks.

But for now, I want to concentrate this discussion on the Python
elements of prompt substitution.  We don't have an observer for prompt
changes.  And we won't until the underlying GDB API is fixed/unified.  I
decided because of these changes, exposing anymore to the Python API
now would be unwise. (We will always have a display prompt observer, so I
included that. Hence the long wall of text, and the shortened patch!

Cheers

Phil

--

2011-07-18  Phil Muldoon  <pmuldoon@redhat.com>
            Tom Tromey  <tromey@redhat.com>
	* top.c (set_prompt): Rewrite to free previous prompt, free
	asynch_new_prompt and set both on new prompts.
	* event-top.c (display_gdb_prompt): Add prompt substitution
	logic.
	* python/python.c (before_prompt_hook): New function.
	(gdbpy_get_current_prompt): Ditto.
	(gdbpy_set_current_prompt): Ditto.

2011-07-18  Phil Muldoon  <pmuldoon@redhat.com>

	* gdb.python/python.exp: Add prompt substitution tests.

2011-07-18  Phil Muldoon  <pmuldoon@redhat.com>

	* observer.texi (GDB Observers): Add before_prompt observer.
	* gdb.texinfo (Basic Python): Add documentation for setting,
	getting and prompt substitution.

--

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 3a3a9fb..03f6c49 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21080,6 +21080,33 @@ provided, it is decoded the way that @value{GDBN}'s inbuilt
 @code{break} or @code{edit} commands do (@pxref{Specify Location}).
 @end defun
 
+@findex gdb.get_current_prompt
+@defun get_current_prompt
+Return the current @value{GDBN} prompt as a string.
+@end defun
+
+@findex gdb.set_current_prompt
+@defun set_current_prompt @r{[}new_prompt@r{]}
+Sets the @value{GDBN} prompt to @var{new_prompt}.  @var{new_prompt}
+must be a string.  This method has no return value.
+@end defun
+
+@defop Operation {@value{GDBN}} prompt_hook [current_prompt]
+If @var{prompt_hook} exists, and is not set to @code{None},
+@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..c78b088 100644
--- a/gdb/doc/observer.texi
+++ b/gdb/doc/observer.texi
@@ -222,8 +222,12 @@ 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.
 @end deftypefun
-
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 72cbfdc..a32d1f7 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,24 @@ 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 = get_prompt ();
+
+      observer_notify_before_prompt (pre_gdb_prompt);
+      post_gdb_prompt = get_prompt ();
+
+      /* If the observer changed the prompt, use that prompt overriding
+	 any new_prompt that was passed to this function.  */
+      if (strcmp (pre_gdb_prompt, post_gdb_prompt) != 0)
+	actual_gdb_prompt = post_gdb_prompt;
+    }
+
   if (sync_execution && is_running (inferior_ptid))
     {
       /* This is to trick readline into not trying to display the
@@ -289,27 +308,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 +345,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..de9fb56 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,89 @@ gdbpy_initialize_events (void)
     }
 }
 
+\f
+
+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;
+
+	  if (current_gdb_prompt == NULL)
+	    {
+	      current_prompt = Py_None;
+	      Py_INCREF (Py_None);
+	    }
+	  else
+	    {
+	      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;
+}
+
+\f
+
 /* Printing.  */
 
 /* A python function to write a single string using gdb's filtered
@@ -811,6 +895,33 @@ gdbpy_progspaces (PyObject *unused1, PyObject *unused2)
   return list;
 }
 
+/* Return the current GDB prompt as a string.  */
+static PyObject *
+gdbpy_get_current_prompt (PyObject *self, PyObject *args)
+{
+  const char *prompt = get_prompt ();
+
+  if (prompt)
+    return PyString_FromString (prompt);
+
+  Py_RETURN_NONE;
+}
+
+/* Set the current GDB prompt.  Only accept strings.  */
+static PyObject *
+gdbpy_set_current_prompt (PyObject *self, PyObject *args)
+{
+  char *prompt = NULL;
+
+  if (! PyArg_ParseTuple (args, "s", &prompt))
+    return NULL;
+
+  if (prompt)
+    set_prompt (prompt);
+
+  Py_RETURN_NONE;
+}
+
 \f
 
 /* The "current" objfile.  This is set when gdb detects that a new
@@ -1134,6 +1245,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 +1349,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);
@@ -1338,6 +1453,16 @@ Return the selected thread object." },
   { "inferiors", gdbpy_inferiors, METH_NOARGS,
     "inferiors () -> (gdb.Inferior, ...).\n\
 Return a tuple containing all inferiors." },
+  { "get_current_prompt", gdbpy_get_current_prompt, METH_NOARGS,
+    "get_current_prompt () -> String.\n\
+Return the current GDB prompt" },
+  { "set_current_prompt", (PyCFunction)gdbpy_set_current_prompt,
+    METH_VARARGS,
+    "set_current_prompt (String).\n\
+Set the current GDB prompt." },
+  { "flush", (PyCFunction)gdbpy_flush, METH_VARARGS | METH_KEYWORDS,
+    "Flush gdb's filtered stdout stream." },
+
   {NULL, NULL, 0, NULL}
 };
 
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index 8906bbc..1eda476 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -193,3 +193,104 @@ 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_py_test_silent_cmd "python p = gdb.get_current_prompt\(\)" \
+    "Get the current prompt" 1
+
+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_py_test_silent_cmd "python gdb.set_current_prompt\(p\)" \
+    "Set the default prompt" 1
+
+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"
+    }
+}
+
+# Test get/set prompt
+set foo "foobar "
+gdb_test_multiple "python gdb.set_current_prompt(\"$foo\")" "set prompt" {
+    -re "\[\r\n\]$foo$" {
+	pass "set prompt"
+    }
+}
+gdb_test_multiple "python curr = gdb.get_current_prompt()" "get prompt" {
+    -re "\[\r\n\]$foo$" {
+	pass "get prompt"
+    }
+}
+gdb_test_multiple "python print curr" "print prompt get prompt" {
+    -re "$foo.\[\r\n\]$foo$" {
+	pass "get prompt"
+    }
+}
+
diff --git a/gdb/top.c b/gdb/top.c
index ecb91f2..09cd1ce 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));
+    }
 }
 \f
 
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;

Re: [patch] [python] Prompt substitution

[Tom Tromey]

>>>>> "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')?

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.

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.

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?

Phil> +	  if (current_gdb_prompt == NULL)
Phil> +	    {

When can this happen?

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 already in the code and shouldn't be in this patch.

Phil> +      PROMPT (0) =  xstrdup (s);

Extra space.

Tom

Re: [patch] [python] Prompt substitution

[Phil Muldoon]

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)
     }
 }
 
+\f
+
+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;
+}
+
+\f
+
 /* 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));
+    }
 }
 \f
 
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;

Re: [patch] [python] Prompt substitution

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> Current patch attached.

Code bits are ok.

Remember to send an updated ChangeLog when you update a patch.

Tom

Re: [patch] [python] Prompt substitution

[Eli Zaretskii]

> From: Phil Muldoon <pmuldoon@redhat.com>
> Date: Mon, 18 Jul 2011 16:45:51 +0100
> 
> 2011-07-18  Phil Muldoon  <pmuldoon@redhat.com>
> 
> 	* observer.texi (GDB Observers): Add before_prompt observer.
> 	* gdb.texinfo (Basic Python): Add documentation for setting,
> 	getting and prompt substitution.

Thanks.

> +@defun set_current_prompt @r{[}new_prompt@r{]}
> +Sets the @value{GDBN} prompt to @var{new_prompt}.
   ^^^^
"Set", not "sets", to be consistent with the way we describe other
methods.

You don't say what is the effect of omitting new_prompt (which is
optional, according to the "[...]" syntax).

> +@defop Operation {@value{GDBN}} prompt_hook [current_prompt]
                                               ^^^^^^^^^^^^^^^^
You need @r{} here as well.

Also, what happens if current_prompt is omitted?

> +Called before a top-level prompt is displayed. @var{current_prompt} is
                                                ^^
Two spaces, please.

The documentation parts are okay with these comments.

Re: [patch] [python] Prompt substitution

[Phil Muldoon]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:
>
> Phil> Current patch attached.
>
> Code bits are ok.
>
> Remember to send an updated ChangeLog when you update a patch.

Thanks, and so committed.  One note: as this patch and the upcoming
prompt memory cleanups patch alter set_prompt in similar ways, I changed
this patch to match Pedro's xstrdup request.  I've included the hunk
below.  I also forgot to write a NEWS entry.  I will send a patch for
that soon.

Cheers,

Phil


===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.199
retrieving revision 1.200
diff -u -r1.199 -r1.200
--- src/gdb/top.c	2011/06/27 19:21:50	1.199
+++ src/gdb/top.c	2011/07/21 11:03:45	1.200
@@ -1133,14 +1133,17 @@
 }
 
 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);
+  char *p = xstrdup (s);
+
+  xfree (PROMPT (0));
+  PROMPT (0) = p;
+
+  /* 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));
 }

Re: [patch] [python] Prompt substitution

[Pedro Alves]

On Wednesday 20 July 2011 12:33:24, Phil Muldoon wrote:
> @@ -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;;
>      }

I believe this to be a bit broken in the annotations (prefix and
suffix != NULL) case.  If the observer changes the prompt, we won't print
either the prefix or the suffix, but will print directly what the
observer on the prompt itself only (get_prompt).

Make use of Matt's bug where the python prompt only works
the second time (so to trigger the strcmp):

$ gdb -quiet -ex 'set prompt (foo) ' -ex 'python def prompt(x): return "(bar) "' -ex 'python gdb.prompt_hook = prompt'
(foo) set annotate 2
(bar) 

While without the hook:

$ gdb -quiet
(gdb) set annotate 2

▒▒pre-prompt
(gdb) 
▒▒prompt

I believe it was not intended for the hook to be able
to override the prefix and the suffix as well.  Right?
I'm fixing this along with the get-rid-of-prompt-stack
change.

-- 
Pedro Alves

Re: [patch] [python] Prompt substitution

[Pedro Alves]

On Wednesday 20 July 2011 12:33:24, Phil Muldoon wrote:
> +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"
> +    }
> +}

Hmm:

...
PASS: gdb.python/python.exp: show prompt shows guarded prompt
set prompt blah 
py prompt 2 PASS: gdb.python/python.exp: set prompt blah overriden
python gdb.prompt_hook = None
py prompt 2 PASS: gdb.python/python.exp: Delete old hook

I think this last PASS is actually a bug.  We've disabled the python
hook, so we should get the "set prompt" prompt back ("blah")
instead of still seeing the python prompt.  Agree?

(My WIP no-prompt-stack patch turned that into a FAIL,
but it looks like it's a bug fix, not a regression.)

-- 
Pedro Alves

Re: [patch] [python] Prompt substitution

[Phil Muldoon]

Pedro Alves <pedro@codesourcery.com> writes:


> I believe this to be a bit broken in the annotations (prefix and
> suffix != NULL) case.  If the observer changes the prompt, we won't print
> either the prefix or the suffix, but will print directly what the
> observer on the prompt itself only (get_prompt).

Hmm, interesting.  It passed the annotations tests :(

> I believe it was not intended for the hook to be able
> to override the prefix and the suffix as well.  Right?
> I'm fixing this along with the get-rid-of-prompt-stack
> change.

Nope, not intended to do anything at all with prefix/suffix. AFAIK
prefix/suffix is only used in one particular case of annotations that
matter (basically the control characters that are printed in the
prefix/suffix when annotations > 1).

Anyway, thanks for taking care of this.  And thanks for getting rid of
the crufty prompt-stack too!

Cheers

Phil

Re: [patch] [python] Prompt substitution

[Phil Muldoon]

Pedro Alves <pedro@codesourcery.com> writes:

> On Wednesday 20 July 2011 12:33:24, Phil Muldoon wrote:
>> +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"
>> +    }
>> +}
>
> Hmm:
>
> ...
> PASS: gdb.python/python.exp: show prompt shows guarded prompt
> set prompt blah 
> py prompt 2 PASS: gdb.python/python.exp: set prompt blah overriden
> python gdb.prompt_hook = None
> py prompt 2 PASS: gdb.python/python.exp: Delete old hook
>
> I think this last PASS is actually a bug.  We've disabled the python
> hook, so we should get the "set prompt" prompt back ("blah")
> instead of still seeing the python prompt.  Agree?
>
> (My WIP no-prompt-stack patch turned that into a FAIL,
> but it looks like it's a bug fix, not a regression.)

I think in the case, display_gdb_prompt is just displaying what GDB
knows to be the prompt.  If you delete the hook, whatever was the prompt
when the prompt was set will remain the prompt.  So the hook set the
prompt whenever in time, and when you delete the hook, it won't restore
what the old prompt was.  So if you disable the prompt, then set prompt
foo, then you will get "foo" now until you change it again manually. In
the case of "guarded prompt" (IE >) it won't attempt to alter that
display at all.

Cheers,

Phil

Re: [patch] [python] Prompt substitution

[Pedro Alves]

On Tuesday 30 August 2011 21:33:24, Phil Muldoon wrote:
> Pedro Alves <pedro@codesourcery.com> writes:
> 
> > On Wednesday 20 July 2011 12:33:24, Phil Muldoon wrote:
> >> +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"
> >> +    }
> >> +}
> >
> > Hmm:
> >
> > ...
> > PASS: gdb.python/python.exp: show prompt shows guarded prompt
> > set prompt blah 
> > py prompt 2 PASS: gdb.python/python.exp: set prompt blah overriden
> > python gdb.prompt_hook = None
> > py prompt 2 PASS: gdb.python/python.exp: Delete old hook
> >
> > I think this last PASS is actually a bug.  We've disabled the python
> > hook, so we should get the "set prompt" prompt back ("blah")
> > instead of still seeing the python prompt.  Agree?
> >
> > (My WIP no-prompt-stack patch turned that into a FAIL,
> > but it looks like it's a bug fix, not a regression.)
> 
> I think in the case, display_gdb_prompt is just displaying what GDB
> knows to be the prompt.  If you delete the hook, whatever was the prompt
> when the prompt was set will remain the prompt.  So the hook set the
> prompt whenever in time, and when you delete the hook, it won't restore
> what the old prompt was.  So if you disable the prompt, then set prompt
> foo, then you will get "foo" now until you change it again manually. In
> the case of "guarded prompt" (IE >) it won't attempt to alter that
> display at all.

I see.  I assumed that the python hook worked by overriding
"set prompt", but that the result wouldn't be seen by "show prompt".
That is, set/show prompt always worked at the "this is what
I want the prompt to look like if no scripting overrides it" level.
Okay, I preserved the current behaviour.  Matt's new (uncommitted)
py-prompt.exp tests were great for making sure I did.  :-)

Thanks!

-- 
Pedro Alves

Re: [patch] [python] Prompt substitution

[Phil Muldoon]

Pedro Alves <pedro@codesourcery.com> writes:

> On Tuesday 30 August 2011 21:33:24, Phil Muldoon wrote:

>> I think in the case, display_gdb_prompt is just displaying what GDB
>> knows to be the prompt.  If you delete the hook, whatever was the prompt
>> when the prompt was set will remain the prompt.  So the hook set the
>> prompt whenever in time, and when you delete the hook, it won't restore
>> what the old prompt was.  So if you disable the prompt, then set prompt
>> foo, then you will get "foo" now until you change it again manually. In
>> the case of "guarded prompt" (IE >) it won't attempt to alter that
>> display at all.
>
> I see.  I assumed that the python hook worked by overriding
> "set prompt", but that the result wouldn't be seen by "show prompt".
> That is, set/show prompt always worked at the "this is what
> I want the prompt to look like if no scripting overrides it" level.
> Okay, I preserved the current behaviour.  Matt's new (uncommitted)
> py-prompt.exp tests were great for making sure I did.  :-)

Yeah this was a port from archer, something I did not originally write,
but has been asked for since, well, forever.  I'm not strongly swayed by
the behavior, as it is, or, as in your scenario working around
set_prompt.  Tom might have other thoughts as I think he wrote most of
this stuff.  But FWIW, I don't mind if you change it to your method --
both seem equally valid use-cases.

Cheers,

Phil