diff mbox

[RFA] PR python/13598 - add before_prompt event

Message ID 1463806850-11001-1-git-send-email-tom@tromey.com
State New
Headers show

Commit Message

Tom Tromey May 21, 2016, 5 a.m. UTC
This adds an event that is emitted just before GDB presents a prompt
to the user.  This provides Python code a way to react to whatever
changes might have been made by the previous command.  For example, in
my GUI I use this to track changes to the selected frame and reflect
them in the UI.

Built and regtested on x86-64 Fedora 23.

2016-05-20  Tom Tromey  <tom@tromey.com>

	PR python/13598:
	* python/python.c (gdbpy_before_prompt_hook): Emit before_prompt
	event.
	* python/py-evts.c (gdbpy_initialize_py_events): Add
	before_prompt registry.
	* python/py-events.h (events_object) <before_prompt>: New field.

2016-05-20  Tom Tromey  <tom@tromey.com>

	PR python/13598:
	* python.texi (Events In Python): Document events.before_prompt.

2016-05-20  Tom Tromey  <tom@tromey.com>

	PR python/13598:
	* gdb.python/py-events.exp: Add before_prompt event tests.
---
 gdb/ChangeLog                          |  9 +++++++++
 gdb/doc/ChangeLog                      |  5 +++++
 gdb/doc/python.texi                    |  4 ++++
 gdb/python/py-events.h                 |  1 +
 gdb/python/py-evts.c                   |  3 +++
 gdb/python/python.c                    |  5 +++++
 gdb/testsuite/ChangeLog                |  5 +++++
 gdb/testsuite/gdb.python/py-events.exp | 25 +++++++++++++++++++++++++
 8 files changed, 57 insertions(+)

Comments

Eli Zaretskii May 21, 2016, 6:53 a.m. UTC | #1
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Fri, 20 May 2016 23:00:50 -0600
> 
> 2016-05-20  Tom Tromey  <tom@tromey.com>
> 
> 	PR python/13598:
> 	* python/python.c (gdbpy_before_prompt_hook): Emit before_prompt
> 	event.
> 	* python/py-evts.c (gdbpy_initialize_py_events): Add
> 	before_prompt registry.
> 	* python/py-events.h (events_object) <before_prompt>: New field.
> 
> 2016-05-20  Tom Tromey  <tom@tromey.com>
> 
> 	PR python/13598:
> 	* python.texi (Events In Python): Document events.before_prompt.
> 
> 2016-05-20  Tom Tromey  <tom@tromey.com>
> 
> 	PR python/13598:
> 	* gdb.python/py-events.exp: Add before_prompt event tests.

OK for the documentation part.

Thanks.
Phil Muldoon July 22, 2016, 9:34 a.m. UTC | #2
On 21/05/16 06:00, Tom Tromey wrote:
> This adds an event that is emitted just before GDB presents a prompt
> to the user.  This provides Python code a way to react to whatever
> changes might have been made by the previous command.  For example, in
> my GUI I use this to track changes to the selected frame and reflect
> them in the UI.
> 
> Built and regtested on x86-64 Fedora 23.
> 
> 2016-05-20  Tom Tromey  <tom@tromey.com>
> 
> 	PR python/13598:
> 	* python/python.c (gdbpy_before_prompt_hook): Emit before_prompt
> 	event.
> 	* python/py-evts.c (gdbpy_initialize_py_events): Add
> 	before_prompt registry.
> 	* python/py-events.h (events_object) <before_prompt>: New field.
> 
> 2016-05-20  Tom Tromey  <tom@tromey.com>
> 
> 	PR python/13598:
> 	* python.texi (Events In Python): Document events.before_prompt.
> 
> 2016-05-20  Tom Tromey  <tom@tromey.com>
> 
> 	PR python/13598:
> 	* gdb.python/py-events.exp: Add before_prompt event tests.

Looks good to me. Apologies for the delay in responding.

Note, I'm not a maintainer, so please wait for their approval. Or
approve it yourself (I think you are still a maintainer!)

Cheers

Phil
Yao Qi July 22, 2016, 10:01 a.m. UTC | #3
Hi Tom,

On Sat, May 21, 2016 at 6:00 AM, Tom Tromey <tom@tromey.com> wrote:
> This adds an event that is emitted just before GDB presents a prompt
> to the user.  This provides Python code a way to react to whatever
> changes might have been made by the previous command.  For example, in
> my GUI I use this to track changes to the selected frame and reflect
> them in the UI.

I read this patch multiple times, but don't know the motivation of adding this
event.  In PR python/13598, you mentioned you want to get event when
something is changed.  If so, why don't you add the python event for the changes
you are interested in?  What is your use case of using python code in GDB?
Matt Rice July 22, 2016, 10:22 a.m. UTC | #4
On Fri, Jul 22, 2016 at 3:01 AM, Yao Qi <qiyaoltc@gmail.com> wrote:
> Hi Tom,
>
> On Sat, May 21, 2016 at 6:00 AM, Tom Tromey <tom@tromey.com> wrote:
>> This adds an event that is emitted just before GDB presents a prompt
>> to the user.  This provides Python code a way to react to whatever
>> changes might have been made by the previous command.  For example, in
>> my GUI I use this to track changes to the selected frame and reflect
>> them in the UI.
>
> I read this patch multiple times, but don't know the motivation of adding this
> event.  In PR python/13598, you mentioned you want to get event when
> something is changed.  If so, why don't you add the python event for the changes
> you are interested in?  What is your use case of using python code in GDB?


FWIW, I have long used or more precisely abused the prompt_hook
function for controlling vi by vi --remote to have an editor that
tracks the current file/line which gdb is stopped at, a mechanism for
this which doesn't abuse the actual prompt string formatting functions
by producing some side-effect before returning the prompt string.
such as that proposed by Tom would be nice.

in particular abusing the prompt_hook in this fashion inhibits the
ability to use the python extended-prompt stuff for controlling the
prompt, because the extended-prompt stuff uses the prompt_hook itself!

In particular it's just a nice time to take a snapshot of some
otherwise obtainable property from gdb right before control is handed
back to the user.
Phil Muldoon July 22, 2016, 1:27 p.m. UTC | #5
On 22/07/16 11:01, Yao Qi wrote:
> Hi Tom,
> 
> On Sat, May 21, 2016 at 6:00 AM, Tom Tromey <tom@tromey.com> wrote:
>> This adds an event that is emitted just before GDB presents a prompt
>> to the user.  This provides Python code a way to react to whatever
>> changes might have been made by the previous command.  For example, in
>> my GUI I use this to track changes to the selected frame and reflect
>> them in the UI.
> 
> I read this patch multiple times, but don't know the motivation of adding this
> event.  In PR python/13598, you mentioned you want to get event when
> something is changed.  If so, why don't you add the python event for the changes
> you are interested in?  What is your use case of using python code in GDB?

The use-case seems clear to me. 

Old workflow:

gdb-prompt -> command entered -> gdb does something -> gdb finishes ->
gdb displays prompt

New workflow:

gdb-prompt -> command entered -> gdb does something -> gdb finishes ->
notifies interested observers -> gdb displays prompt

I think it would be useful to know the time when GDB is done
processing the event queue and before it displays the prompt and waits
on user input.

Cheers

Phil
Tom Tromey July 23, 2016, 5 a.m. UTC | #6
>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:

Matt> In particular it's just a nice time to take a snapshot of some
Matt> otherwise obtainable property from gdb right before control is handed
Matt> back to the user.

Yes, this is exactly why I added this.

My original motivation was to get a notification when the selected frame
changed.  However, this happens a lot, sometimes temporarily.  And, I
wasn't interested in temporary frame changes -- just "final" changes,
such as just after a command has completed and just before the prompt is
displayed... which led to this approach.

Right now my GUI uses this event to react appropriately when the user
types "up" or "down".

Tom
Yao Qi Aug. 3, 2016, 8:33 a.m. UTC | #7
On Sat, Jul 23, 2016 at 6:00 AM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:
>
> Matt> In particular it's just a nice time to take a snapshot of some
> Matt> otherwise obtainable property from gdb right before control is handed
> Matt> back to the user.
>
> Yes, this is exactly why I added this.
>
> My original motivation was to get a notification when the selected frame
> changed.  However, this happens a lot, sometimes temporarily.  And, I
> wasn't interested in temporary frame changes -- just "final" changes,
> such as just after a command has completed and just before the prompt is
> displayed... which led to this approach.

If you are interested in changes in selected frames, we can add a python event
selected_frame_changed.  However, as a python api, the event should be
general enough, IOW, your tool isn't interested in temporary frame changes,
but other python users may be interested in _all_ selected frame changes.
Tom Tromey Aug. 3, 2016, 4:07 p.m. UTC | #8
>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:

Yao> If you are interested in changes in selected frames, we can add a
Yao> python event selected_frame_changed.  However, as a python api, the
Yao> event should be general enough, IOW, your tool isn't interested in
Yao> temporary frame changes, but other python users may be interested
Yao> in _all_ selected frame changes.

If a general event is implemented, I'll still need a way to filter out
temporary frame changes, to avoid excessive GUI updates.

Tom
Pedro Alves Aug. 3, 2016, 4:16 p.m. UTC | #9
On 07/23/2016 06:00 AM, Tom Tromey wrote:
>>>>>> "Matt" == Matt Rice <ratmice@gmail.com> writes:
> 
> Matt> In particular it's just a nice time to take a snapshot of some
> Matt> otherwise obtainable property from gdb right before control is handed
> Matt> back to the user.
> 
> Yes, this is exactly why I added this.
> 
> My original motivation was to get a notification when the selected frame
> changed.  However, this happens a lot, sometimes temporarily.  And, I
> wasn't interested in temporary frame changes -- just "final" changes,
> such as just after a command has completed and just before the prompt is
> displayed... which led to this approach.
> 
> Right now my GUI uses this event to react appropriately when the user
> types "up" or "down".

The TUI uses this approach too nowadays.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
commit 0986c744dfecb8177de90020646090e9ed23cfe7
Author:     Patrick Palka <patrick@parcs.ath.cx>
AuthorDate: Tue Jun 30 13:56:49 2015 -0400

    Replace TUI's select_frame hook (PR tui/13378)
    
    The select_frame hook is used by TUI to update TUI's frame and register
    information following changes to the selected frame.  The problem with
    this hook is that it gets called after every single frame change, even
    if the frame change is only temporary or internal.  This is the primary
    cause of flickering and slowdown when running the inferior under TUI
    with conditional breakpoints set.  Internal GDB events are the source of
    many calls to select_frame and these internal events are triggered
    frequently, especially when a few conditional breakpoints are set.
    
    This patch removes the select_frame hook altogether and instead makes
    the frame and register information get updated in two key places (using
    observers): after an inferior stops, and right before displaying a
    prompt.  The latter hook covers the case when frame information must be
    updated following a call to "up", "down" or "frame", and the former
    covers the case when frame and register information must be updated
    after a call to "continue", "step", etc. or after the inferior stops in
    async execution mode.  Together these hooks should cover all the cases
    when frame information ought to be refreshed (and when the relevant
    windows ought to be subsequently updated).
[...]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Thanks,
Pedro Alves
Tom Tromey Aug. 4, 2016, 8:34 p.m. UTC | #10
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> Right now my GUI uses this event to react appropriately when the user
>> types "up" or "down".

Pedro> The TUI uses this approach too nowadays.

Thanks for mentioning that.  I think it's another argument for accepting
this patch.

Tom
Pedro Alves Aug. 5, 2016, 5:30 p.m. UTC | #11
On 08/04/2016 09:34 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
>>> Right now my GUI uses this event to react appropriately when the user
>>> types "up" or "down".
> 
> Pedro> The TUI uses this approach too nowadays.
> 
> Thanks for mentioning that.  I think it's another argument for accepting
> this patch.

I do agree that an event for user-selected frame changes might be nice.

However, I'm fine with the approach in this patch as well, because
I'm sure that even if a frame-change event is added in the future, 
a before_prompt event can still find other uses.

Thanks,
Pedro Alves
Tom Tromey Oct. 9, 2016, 5:23 p.m. UTC | #12
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Pedro> The TUI uses this approach too nowadays.

Tom> Thanks for mentioning that.  I think it's another argument for accepting
Tom> this patch.

Hi!

I'd like this patch to be reconsidered for approval.

Today I tried rewriting this patch to use the new
user_selected_context_changed observer.

However, this runs into problems.

First, this observer is not notified when a breakpoint is hit.
It seems to me like it ought to be.

One idea is to attach a second handler to the "stop" event.  However,
stop events are fired too often -- in particular if you set a breakpoint
and put commands like this:

  silent
  cont

... the "stop" event will be fired for every breakpoint hit -- even
though from a "user" point of view this isn't stopping.

Given that the TUI uses the approach given in the before_prompt patch,
and given that there haven't been any negative comments on the patch
itself, I'd like to suggest approving this patch.  As noted elsewhere,
acceptance of this patch does not prevent inclusion of any future event.

thanks,
Tom
Tom Tromey Feb. 11, 2017, 3:33 p.m. UTC | #13
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> This adds an event that is emitted just before GDB presents a prompt
Tom> to the user.  This provides Python code a way to react to whatever
Tom> changes might have been made by the previous command.  For example, in
Tom> my GUI I use this to track changes to the selected frame and reflect
Tom> them in the UI.

Tom> Built and regtested on x86-64 Fedora 23.

I'd like to ping this patch again.

I tried the suggested alternate approach, which I mentioned last time I
pinged.  However, that approach did not work out correctly.  And, the
approach taken in this patch is what is used by the TUI to do exactly
the same thing.

So, I'd like to ask you to please reconsider this.

Tom
Yao Qi Feb. 12, 2017, 11:02 p.m. UTC | #14
On Sat, Feb 11, 2017 at 3:33 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
>
> Tom> This adds an event that is emitted just before GDB presents a prompt
> Tom> to the user.  This provides Python code a way to react to whatever
> Tom> changes might have been made by the previous command.  For example, in
> Tom> my GUI I use this to track changes to the selected frame and reflect
> Tom> them in the UI.
>
> Tom> Built and regtested on x86-64 Fedora 23.
>
> I'd like to ping this patch again.
>
> I tried the suggested alternate approach, which I mentioned last time I
> pinged.  However, that approach did not work out correctly.  And, the
> approach taken in this patch is what is used by the TUI to do exactly
> the same thing.
>
> So, I'd like to ask you to please reconsider this.
>

Hi Tom,
I am convinced by your arguments.  Your patch is good to me.
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0968b85..c0b6b06 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@ 
+2016-05-20  Tom Tromey  <tom@tromey.com>
+
+	PR python/13598:
+	* python/python.c (gdbpy_before_prompt_hook): Emit before_prompt
+	event.
+	* python/py-evts.c (gdbpy_initialize_py_events): Add
+	before_prompt registry.
+	* python/py-events.h (events_object) <before_prompt>: New field.
+
 2016-05-19  Andreas Schwab  <schwab@suse.de>
 
 	* ia64-libunwind-tdep.c (libunwind_descr): Add cast from void *.
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index fe2e3be..fa14b67 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@ 
+2016-05-20  Tom Tromey  <tom@tromey.com>
+
+	PR python/13598:
+	* python.texi (Events In Python): Document events.before_prompt.
+
 2016-05-17  Tom Tromey  <tom@tromey.com>
 
 	* gdb.texinfo (Supported Languages): Mention Rust.  Update menu.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index ffbf89a..e655db8 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2962,6 +2962,10 @@  A gdb.Frame object representing the frame in which the register was modified.
 Denotes which register was modified.
 @end defvar
 
+@item events.before_prompt
+This event carries no payload.  It is emitted each time @value{GDBN}
+presents a prompt to the user.
+
 @end table
 
 @node Threads In Python
diff --git a/gdb/python/py-events.h b/gdb/python/py-events.h
index 9ecee4c..b969c15 100644
--- a/gdb/python/py-events.h
+++ b/gdb/python/py-events.h
@@ -50,6 +50,7 @@  typedef struct
   eventregistry_object *inferior_call;
   eventregistry_object *memory_changed;
   eventregistry_object *register_changed;
+  eventregistry_object *before_prompt;
 
   PyObject *module;
 
diff --git a/gdb/python/py-evts.c b/gdb/python/py-evts.c
index 95827e4..d388282 100644
--- a/gdb/python/py-evts.c
+++ b/gdb/python/py-evts.c
@@ -89,6 +89,9 @@  gdbpy_initialize_py_events (void)
   if (add_new_registry (&gdb_py_events.clear_objfiles, "clear_objfiles") < 0)
     return -1;
 
+  if (add_new_registry (&gdb_py_events.before_prompt, "before_prompt") < 0)
+    return -1;
+
   if (gdb_pymodule_addobject (gdb_module,
 			      "events",
 			      (PyObject *) gdb_py_events.module) < 0)
diff --git a/gdb/python/python.c b/gdb/python/python.c
index c706644..d22f8dd 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -36,6 +36,7 @@ 
 #include <ctype.h>
 #include "location.h"
 #include "ser-event.h"
+#include "py-event.h"
 
 /* Declared constants and enum for python stack printing.  */
 static const char python_excp_none[] = "none";
@@ -1029,6 +1030,10 @@  gdbpy_before_prompt_hook (const struct extension_language_defn *extlang,
 
   cleanup = ensure_python_env (get_current_arch (), current_language);
 
+  if (!evregpy_no_listeners_p (gdb_py_events.before_prompt)
+      && evpy_emit_event (NULL, gdb_py_events.before_prompt) < 0)
+    goto fail;
+
   if (gdb_python_module
       && PyObject_HasAttrString (gdb_python_module, "prompt_hook"))
     {
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index a773c63..3f585f3 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2016-05-20  Tom Tromey  <tom@tromey.com>
+
+	PR python/13598:
+	* gdb.python/py-events.exp: Add before_prompt event tests.
+
 2016-05-18  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* gdb.mi/mi-threads-interrupt.c: New file.
diff --git a/gdb/testsuite/gdb.python/py-events.exp b/gdb/testsuite/gdb.python/py-events.exp
index de8de07..592367d 100644
--- a/gdb/testsuite/gdb.python/py-events.exp
+++ b/gdb/testsuite/gdb.python/py-events.exp
@@ -210,3 +210,28 @@  gdb_test "continue" ".*event type: continue.*
 .*exit code: 12.*
 .*exit inf: 2.*
 dir ok: True.*" "Inferior 2 terminated."
+
+
+# Test before_prompt event.
+gdb_py_test_multiple "define new user command" \
+    "define xxz" "End with a line saying just .end.." \
+    "set variable \$x = 72" "" \
+    "set variable \$y = 93" "" \
+    "end" ""
+
+gdb_py_test_multiple "add before_prompt listener" \
+    "python" "" \
+    "count = 0" "" \
+    "def listener():" "" \
+    "  global count" "" \
+    "  count = count + 1" "" \
+    "gdb.events.before_prompt.connect(listener)" "" \
+    "end" ""
+
+gdb_test_no_output "set variable \$x = 32" "do something"
+# Result is due to one emitted before "set var" and one emitted before
+# this command.
+gdb_test "python print(count)" 2 "check for before_prompt event"
+
+gdb_test_no_output "xxz" "run a canned sequence"
+gdb_test "python print(count)" 4 "check for before_prompt event"