From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6341 invoked by alias); 26 Jul 2008 13:07:00 -0000 Received: (qmail 6333 invoked by uid 22791); 26 Jul 2008 13:06:59 -0000 X-Spam-Check-By: sourceware.org Received: from mtaout2.012.net.il (HELO mtaout2.012.net.il) (84.95.2.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 26 Jul 2008 13:06:35 +0000 Received: from HOME-C4E4A596F7 ([77.127.123.9]) by i_mtaout2.012.net.il (HyperSendmail v2004.12) with ESMTPA id <0K4M00E6O72ZBLN1@i_mtaout2.012.net.il> for gdb-patches@sourceware.org; Sat, 26 Jul 2008 16:06:35 +0300 (IDT) Date: Sat, 26 Jul 2008 13:07:00 -0000 From: Eli Zaretskii Subject: Re: [RFA][patch 1/9] Yet another respin of the patch with initial Python support In-reply-to: <1216653969.31797.6.camel@localhost.localdomain> X-012-Sender: halo1@inter.net.il To: Thiago Jung Bauermann Cc: drow@false.org, tromey@redhat.com, gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: MIME-version: 1.0 Content-type: text/plain; charset=iso-8859-1 Content-transfer-encoding: 8BIT References: <20080429155212.444237503@br.ibm.com> <20080429155304.288626880@br.ibm.com> <20080528205921.GA2969@caradoc.them.org> <20080615181833.uxmo25mg0kko40kw@imap.linux.ibm.com> <1216107418.14956.27.camel@localhost.localdomain> <1216245620.12209.18.camel@localhost.localdomain> <20080718195010.GA14356@caradoc.them.org> <1216653969.31797.6.camel@localhost.localdomain> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-07/txt/msg00475.txt.bz2 > From: Thiago Jung Bauermann > Cc: Tom Tromey , > gdb-patches ml , > Eli Zaretskii > Date: Mon, 21 Jul 2008 12:26:09 -0300 > > This is today's version of the patch. :-) > It addresses the latest review comments, and is good to go AFAIK. > > Eli, > > What do you think of this documentation? Thanks; comments below. Do I understand correctly that I need not review the doco patches you posted prior to this? If there's something else I should review, please point to the corresponding messages. > +* Python scripting > + > + 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. Is --without-python a configure-time switch or a run-time switch? If the former, I don't think it should be mentioned here. If the latter, we should explicitly say it's a run-time switch. > +python [code] > + Invoke python code. Please say a few word about the form of [code]. Is it a string? if so, should it be in quotes? Or maybe it's the name of a file with Python code? Or something else? These all are questions that went through my head when I was reading this NEWS entry. > + else if (p1 - p == 6 && !strncmp (p, "python", 6)) Can we avoid literal constants such as 6 here, and instead make it dependent on the string "python"? > else if (p1 - p == 10 && !strncmp (p, "loop_break", 10)) Ditto. > * Emacs:: Using @value{GDBN} under @sc{gnu} Emacs > * GDB/MI:: @value{GDBN}'s Machine Interface. > * Annotations:: @value{GDBN}'s annotation interface. > +* Python:: Scripting @value{GDBN} using Python. I think this should not be a separate chapter, but rather a section of the "Canned Sequences of Commands" chapter. > +@node Python > +@chapter Scripting @value{GDBN} using Python > +@cindex Python First, @cindex entries should begin with lower-case letters, to produce deterministic results when index entries are sorted. Second, I think "python scripting" will be a better index entry here, especially since the GDB manual joins all indices into one, and you define later an index entry "python" for `python' the command.. Third, I'd add another @cindex entry "scripting with Python". > +You can script @value{GDBN} using the @url{http://www.python.org/, > +Python programming language}. Please use @uref, not @url here. The latter has become a synonym of the former only in the latest versions of Texinfo, so let's not force users upgrade their Texinfo just for that. >+ This feature is available only if > +@value{GDBN} was configured using @code{--with-python}. Please use @option for switches, not @code. > +@node Python Commands > +@section Python Commands This needs index entries; please add them. > +By default, @value{GDBN} will print a stack trace when an error occurs > +in a Python script. This can be controlled using @code{maint set python > +print-stack}. This variable takes a boolean value; if @code{on}, the > +default, then Python stack printing is enabled; if @code{off}, then > +Python stack printing is disabled. I'd rephrase this slightly ("set python print-stack" is not a variable): By default, @value{GDBN} prints a stack trace when an error occurs in a Python script. This can be controlled using @code{maint set python print-stack}: if @code{on}, the default, Python stack printing is enabled; if @code{off}, it is disabled. > +@node Python API > +@section Python API This also needs index entries. > +@value{GDBN} exposes a number of features to Python scripts. This should end with a colon, and the list of the features should be a @table. > +When executing a @code{python} command, uncaught Python exceptions are ^^^ "the @code{python} command" > +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. I think this is nowhere nearly as clear as it should be. Even to a C hacker, "QUIT exception" is not sufficiently self-explanatory. Do you mean SIGQUIT? then please say so. Will Python RuntimeError clear enough to casual GDB users who want to use Python scripting, but are not experienced Python programmers? I'm not sure. And what about stating explicitly which exceptions map to which Python RuntimeError. Finally, "GDB exception" is itself a term that is not defined anywhere in the manual. Strictly speaking, GDB being a C program does not have any exception at all. This all needs to be explained, if we want Python scripting to be useful for writing complex GDB extensions (which is the raison d'être of this feature in the first place). > +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. Is there something here that whoever writes the Python program should know to behave correctly? Does she need, for example, catch KeyboardInterrupt and do something in the handler? If so, we should state here what's needed. > +@value{GDBN} introduces a new Python module, named @code{gdb}. All > +methods and classes added by @value{GDBN} are placed in this module. OK, but what does this mean for whoever writes Python extensions for GDB? While at that, how about explaining why a command FOO is indexed as gdb.FOO? > +@defun execute @var{command} You should not give a @var markup to arguments in @defun; @defun does that itself. > +Evaluate @var{command}, a string, as a @value{GDBN} CLI command. > +Exceptions are translated as noted above. If no error occurs, this > +function will return @code{None}. The last sentence begs a question: what happens if errors _do_ occur? > +@defun show @var{variable} Please remove @var. > +Find the value of a @value{GDBN} setting. "find" or "display"? If the former, why the name is "show"? > + @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. So what signals the end of the name? Also, what exactly is a "multi-part name"? > +If the named variable does not exist, this function throws an exception. Any specific exception? if so, please name it. > +Otherwise, the variable's value is converted to a Python value of the > +appropriate type, and returned. Sounds like it's indeed "find the value" or "return the value", not "show". How about renaming the command to be more mnemonic? > +@defun write @var{string} @var again. > +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. So why is this command available, and why document it? > +@defun flush > +Flush @value{GDBN}'s paginated standard output stream. Ordinarily your > +program should not call this function directly; simply flush > +@code{sys.stdout} as usual. Ditto. Thanks again for working on this.