From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3868 invoked by alias); 26 Jul 2008 17:04:58 -0000 Received: (qmail 3857 invoked by uid 22791); 26 Jul 2008 17:04:57 -0000 X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 26 Jul 2008 17:04:40 +0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id m6QH4RbS027533; Sat, 26 Jul 2008 13:04:27 -0400 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [10.11.255.20]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m6QH4QvK012044; Sat, 26 Jul 2008 13:04:26 -0400 Received: from opsy.redhat.com (vpn-10-41.bos.redhat.com [10.16.10.41]) by pobox.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m6QH4Pjd007180; Sat, 26 Jul 2008 13:04:26 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id E982137824B; Sat, 26 Jul 2008 11:04:24 -0600 (MDT) To: Eli Zaretskii Cc: Thiago Jung Bauermann , drow@false.org, gdb-patches@sourceware.org Subject: Re: [RFA][patch 1/9] Yet another respin of the patch with initial Python support 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> From: Tom Tromey Reply-To: tromey@redhat.com X-Attribution: Tom Date: Sat, 26 Jul 2008 17:04:00 -0000 In-Reply-To: (Eli Zaretskii's message of "Sat\, 26 Jul 2008 16\:06\:30 +0300") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii 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/msg00479.txt.bz2 >>>>> "Eli" == Eli Zaretskii 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