Re: [RFA] Add basic Python API for convenience variables

[Eli Zaretskii <eliz@gnu.org>]

> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Sun, 22 Apr 2018 15:13:09 -0600
> 
> This adds a basic Python API for accessing convenience variables.
> With this, convenience variables can be read and set from Python.
> Although gdb supports convenience variables whose value changes at
> each call, this is not exposed to Python; it could be, but I think
> it's just as good to write a convenience function in this situation.
> 
> This is PR python/23080.

Thanks.

> diff --git a/gdb/NEWS b/gdb/NEWS
> index 63fe30d175..157756597a 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -24,6 +24,12 @@ set|show record btrace cpu
>    Controls the processor to be used for enabling errata workarounds for
>    branch trace decode.
>  
> +* Python API
> +
> +  ** The new functions gdb.convenience_variable and
> +     gdb.set_convenience_variable can be used to get and set the value
> +     of convenience variables.
> +
>  * New targets

This part is OK.

> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -288,6 +288,26 @@ If no exception is raised, the return value is always an instance of
>  @code{gdb.Value} (@pxref{Values From Inferior}).
>  @end defun

This is also OK, but I have a few nit-picking comments:

> +@findex gdb.convenience_variable
> +@defun gdb.convenience_variable (name)
> +Return the value of the convenience variable (@pxref{Convenience
> +Vars}) named @var{name}.  @var{name} must be a string.  The name
> +should not include the @samp{$} that is used to mark a convenience
> +variable in an expression.  If the convenience variable does not
> +exist, then @code{None} is returned.
> +@end defun
> +
> +@findex gdb.set_convenience_variable
> +@defun gdb.set_convenience_variable (name, value)
> +Set the value of the convenience variable (@pxref{Convenience Vars})
> +named @var{name}.  @var{name} must be a string.  The name should not
> +include the @samp{$} that is used to mark a convenience variable in an
> +expression.  If @var{value} is @code{None}, then the convenience
> +variable is removed.  Otherwise, if @var{value} is not a
> +@code{gdb.Value} (@pxref{Values From Inferior}), it is is converted
> +using the @code{gdb.Value} constructor.
> +@end defun

First, can we avoid repeating what NAME can and cannot be or include,
and have that only once?

Second, "named @var{name}" sounds awkward, I would perhaps suggest
"specified by @var{name}" or somesuch.

And lastly, I would generally suggest to avoid starting a sentence
with @var{something} because in the printed manual this produces a
sentence that begins with a lower-case word.

Since these are minor issues, if you don't feel like fixing them, I
won't object.