Re: [RFA][patch 1/9] Yet another respin of the patch with initial Python support

[Tom Tromey <tromey@redhat.com>]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> Do I understand correctly that I need not review the doco patches you
Eli> posted prior to this?  If there's something else I should review,
Eli> please point to the corresponding messages.

This is it.

>> +  GDB now has support for scripting using Python.  Whether this is
>> +  available is determined at configure time; you can explicitly
>> +  disable the support using --without-python.

Eli> Is --without-python a configure-time switch or a run-time switch?  If
Eli> the former, I don't think it should be mentioned here.  If the latter,
Eli> we should explicitly say it's a run-time switch.

Configure time.  I changed the text to read:

    * Python scripting

      GDB now has support for scripting using Python.  Whether this is
      available is determined at configure time.

>> +python [code]
>> +  Invoke python code.

Eli> Please say a few word about the form of [code].  Is it a string? if
Eli> so, should it be in quotes?

How about:

    python [CODE]
      Invoke CODE by passing it to the Python interpreter.

I can be as specific as you like here.  I thought, though, that NEWS
was just a teaser and that users should read the manual for details.

>> +  else if (p1 - p == 6 && !strncmp (p, "python", 6))

Eli> Can we avoid literal constants such as 6 here, and instead make it
Eli> dependent on the string "python"?

Here I just followed the existing code in that file.

>> else if (p1 - p == 10 && !strncmp (p, "loop_break", 10))

Eli> Ditto.

This is existing code.  Please read that file, you'll see there are a
dozen cases like this.  I don't mind fixing this, but I think it is
generally preferable to do cleanups in a separate patch.  If you
disagree, let me know and I will go ahead and mash them together.

>> +* Python::                      Scripting @value{GDBN} using Python.

Eli> I think this should not be a separate chapter, but rather a section of
Eli> the "Canned Sequences of Commands" chapter.

This is a drawback of the one-patch-at-a-time submission method.
Later patches, which we have not yet submitted, greatly expand this
chapter.  In particular we add a bunch of subsections describing the
Python API.

>> +@node Python
>> +@chapter Scripting @value{GDBN} using Python
>> +@cindex Python

Eli> First, @cindex entries should begin with lower-case letters, to
Eli> produce deterministic results when index entries are sorted.

Ok.  I fixed this.  You might want to fix the other 121 occurrences ;)

>> +                                This feature is available only if
>> +@value{GDBN} was configured using @code{--with-python}.

Eli> Please use @option for switches, not @code.

Note there's another use of @code in this situation.  I keep copying
the wrong examples :-(

>> +@value{GDBN} exposes a number of features to Python scripts.

Eli> This should end with a colon, and the list of the features should be a
Eli> @table.

I guess those aren't really features, they are more like generic
things that the python-in-gdb programmer ought to know.

So, I don't think a table makes sense, but I suppose the current text
is also misleading.  What do you suggest?

>> +In particular, a @value{GDBN} @code{QUIT} exception is translated to a
>> +Python @code{KeyboardInterrupt} exception, and other @value{GDBN}
>> +exceptions are translated to Python @code{RuntimeError}s.

Eli> I think this is nowhere nearly as clear as it should be.  Even to a C
Eli> hacker, "QUIT exception" is not sufficiently self-explanatory.  Do you
Eli> mean SIGQUIT? then please say so.

Daniel or some other gdb maintainer will have to suggest something.
I don't know what else to call this.  exceptions.h refers to "QUIT
event", but also the enum value is "RETURN_QUIT"... maybe the latter?

Eli> Will Python RuntimeError clear enough to casual GDB users who
Eli> want to use Python scripting, but are not experienced Python
Eli> programmers?  I'm not sure.

People expecting to do this will have to read a Python manual.
There is no way around that.

Eli> And what about stating explicitly
Eli> which exceptions map to which Python RuntimeError.

It does say -- all the rest of them.
AFAICT gdb really only has quit exceptions and "the other kind".

Eli> Finally, "GDB exception" is itself a term that is not defined anywhere
Eli> in the manual.  Strictly speaking, GDB being a C program does not have
Eli> any exception at all.

I guess that is true in some pedantic way, if you assume that only
languages that explicitly have a "throw" statement can have
exceptions.  But, in practice it is manifestly untrue for gdb.  gdb
uses exceptions everywhere, and discusses them in just those terms.

>> +At startup, @value{GDBN} overrides Python's @code{sys.stdout} and
>> +@code{sys.stderr} to print using @value{GDBN}'s output-paging streams.
>> +A Python program which outputs to one of these streams may have its
>> +output interrupted by the user (@pxref{Screen Size}).  In this
>> +situation, a Python @code{KeyboardInterrupt} exception is thrown.

Eli> Is there something here that whoever writes the Python program should
Eli> know to behave correctly?  Does she need, for example, catch
Eli> KeyboardInterrupt and do something in the handler?  If so, we should
Eli> state here what's needed.

Yes, Python programmers might want to know about this in some
circumstances.  It just depends on what they are trying to accomplish.

I don't agree that we need to explain this.  I don't think we should
try to explain Python programming in general.  Instead, I think we
should explain the Python API and refer users to already existing
Python manuals.

>> +@value{GDBN} introduces a new Python module, named @code{gdb}.  All
>> +methods and classes added by @value{GDBN} are placed in this module.

Eli> OK, but what does this mean for whoever writes Python extensions for
Eli> GDB?  While at that, how about explaining why a command FOO is indexed
Eli> as gdb.FOO?

I thought it would be nice to index them both ways.  Do you disagree?

Python programmers know what the module stuff means.  We don't have to
explain that.

>> +Find the value of a @value{GDBN} setting.

Eli> "find" or "display"?  If the former, why the name is "show"?

It mimics the CLI command name.
"find" would be weird, IMO.  I'm open to other names.

>> +                                          @var{variable} is a string
>> +naming the setting to look up; @var{variable} may contain spaces if the
>> +variable has a multi-part name.  For example, @samp{print object} is a
>> +valid variable name.

Eli> So what signals the end of the name?  Also, what exactly is a
Eli> "multi-part name"?

The point of this text is to say that you can invoke:

    gdb.show("print object")

and get a sensible answer.
If you want to rephrase this somehow, that is fine by me.

>> +Print a string to @value{GDBN}'s paginated standard output stream.
>> +Ordinarily your program should not call this function directly; simply
>> +print to @code{sys.stdout} as usual.

Eli> So why is this command available, and why document it?

Completeness.  It is visible in the module.

Tom