From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19228 invoked by alias); 23 Sep 2008 18:24:10 -0000 Received: (qmail 19213 invoked by uid 22791); 23 Sep 2008 18:24:09 -0000 X-Spam-Check-By: sourceware.org Received: from mtaout1.012.net.il (HELO mtaout1.012.net.il) (84.95.2.1) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 23 Sep 2008 18:23:28 +0000 Received: from HOME-C4E4A596F7 ([77.127.116.246]) by i-mtaout1.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0K7N00MOWV346NH0@i-mtaout1.012.net.il> for gdb-patches@sourceware.org; Tue, 23 Sep 2008 21:23:28 +0300 (IDT) Date: Tue, 23 Sep 2008 18:24:00 -0000 From: Eli Zaretskii Subject: Re: [RFA/Ada] Implement Ada tasking support In-reply-to: <20080922233209.GE24389@adacore.com> X-012-Sender: halo1@inter.net.il To: Joel Brobecker Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: References: <20080922233209.GE24389@adacore.com> 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-09/txt/msg00479.txt.bz2 > Date: Mon, 22 Sep 2008 16:32:09 -0700 > From: Joel Brobecker > > + if (warn_if_null) > + printf_filtered (_("Your application does not use any Ada task.\n")); ^^^^ "tasks", I think. > + add_info ("tasks", info_tasks_command, > + _("\ > +Without argument: list all known Ada tasks, with status information.\n\ > +info tasks N: print detailed information of task N.")); IMO, this is not a good documentation string: the first line is supposed to describe the whole command in short, not only one of its options. I suggest to rewrite this (and others) along these lines. Also, I think the first line cannot have commas or periods except at the end of the line, since various HELP commands, like `apropos', display the text up to the first comma or period. > +@table @asis > +@item ID > +Represents gdb's internal task number. ^^^ What happened to @value{GDBN}? > +@item State > +Current state of the task. > + > +@itemize @bullet Why @itemize and not @table? I think the latter would be better. > +@item > +Accepting RV with @var{taskno}: the task is accepting a rendez-vous with the > +task @var{taskno} Period missing at the end of this sentence. > +@item task > +@kindex task "task" is such a generic term that I'd suggest qualifying this index entry: @kindex task@r{ (Ada)} > +@cindex current task ID Same here: qualify with "Ada". > +@kindex task @var{taskno} There's no need to index all the varieties of a command; one variety is enough. Especially since these two entries are on the same page or on consecutive pages. > +@cindex task switching Again, please qualify. > +This command is like the @code{thread @var{threadno}} > +command (@pxref{Threads}). It switches the context of debugging ^^ Two spaces needed here. > +memory writes in order to provide Ada tasking support. When inspecting > +a core file, this means that the core file must be opened with > +read-write privileges, using the command @samp{"set write on"}. Please put here a cross-reference to the description of "set write on". Also, the write markup for this is using the command @kbd{set write on} (@kbd and no quotes). Other than these gotchas, the documentation patch is approved. Thanks.