Re: [PATCH 2/2] gdb/python: implement support for sending custom MI async notifications

[Eli Zaretskii <eliz@gnu.org>]

> CC: Jan Vrany <jan.vrany@labware.com>
> Date: Fri,  8 Sep 2023 22:05:04 +0100
> From: Jan Vrany via Gdb-patches <gdb-patches@sourceware.org>
> 
> This commit adds a new Python function, gdb.notify_mi, that can be used
> to emit custom async notification to MI channel.  This can be used, among
> other things, to implement notifications about events MI does not support,
> such as remote connection closed or register change.
> ---
>  gdb/NEWS                                  |  3 ++
>  gdb/doc/python.texi                       | 38 ++++++++++++++++++
>  gdb/python/py-mi.c                        | 49 +++++++++++++++++++++++
>  gdb/python/python-internal.h              |  3 ++
>  gdb/python/python.c                       |  4 ++
>  gdb/testsuite/gdb.python/py-mi-notify.exp | 47 ++++++++++++++++++++++
>  6 files changed, 144 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.python/py-mi-notify.exp

Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 98ff00d5efc..4a1f383a666 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -286,6 +286,9 @@ info main
>       might be array- or string-like, even if they do not have the
>       corresponding type code.
>  
> +  ** New function gdb.notify_mi(NAME, DATA), that emits custom
> +     GDB/MI async notification.
> +

This part is okay.

> +It is possible to emit @sc{gdb/mi} notifications from
> +Python.  This is done with the @code{gdb.notify_mi} function.

We try to avoid using passive voice when it is not required.  Suggest
to rephrase the last sentence:

  Use the @code{gdb.notify_mi} function to do that.

> +Emit a @sc{gdb/mi} asynchronous notification.  @var{name} is the name of the
> +notification, a string.

This begs the question: is any string allowed?  If any string is
allowed as NAME, I think you should tell which field of the async MI
notification will be set from that string.  If NAME must be one of a
fixed list of known notifications, we should say something about where
to find that list.

> +as Python dictionary. The dictionary is converted to converted
                       ^^
Two spaces there.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>