From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7106 invoked by alias); 12 Sep 2009 09:18:22 -0000 Received: (qmail 7098 invoked by uid 22791); 12 Sep 2009 09:18:21 -0000 X-SWARE-Spam-Status: No, hits=-0.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_JMF_BL,SPF_SOFTFAIL 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.43rc1) with ESMTP; Sat, 12 Sep 2009 09:18:18 +0000 Received: from conversion-daemon.i_mtaout2.012.net.il by i_mtaout2.012.net.il (HyperSendmail v2004.12) id <0KPU00100PT0EI00@i_mtaout2.012.net.il> for gdb-patches@sourceware.org; Sat, 12 Sep 2009 12:18:15 +0300 (IDT) Received: from HOME-C4E4A596F7 ([84.228.50.163]) by i_mtaout2.012.net.il (HyperSendmail v2004.12) with ESMTPA id <0KPU00LTEPUDBG00@i_mtaout2.012.net.il>; Sat, 12 Sep 2009 12:18:14 +0300 (IDT) Date: Sat, 12 Sep 2009 09:18:00 -0000 From: Eli Zaretskii Subject: Re: Patch: implement new dynamic varobj spec In-reply-to: To: Tom Tromey Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: <83y6okzfl6.fsf@gnu.org> References: 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: 2009-09/txt/msg00366.txt.bz2 > From: Tom Tromey > Date: Thu, 10 Sep 2009 14:58:05 -0600 > > This is the long-awaited dynamic varobj patch. > It implements the spec as described by Vladimir: > > http://sourceware.org/ml/gdb/2009-07/msg00088.html Thanks. > This patch needs a doc review. Sorry for a delay in providing this review. > +Once sent, this command cannot be undone. A minor stylistic nit: I suggest the following variant: Once enabled, this feature cannot be disabled. > +Note that if Python support has not been compiled into @value{GDBN}, > +this command will still succeed. Suggest to add "(and do nothing)" to the end of the sentence Btw, does this silent failure have good reasons? > +This operation returns attributes of the newly-created varobj. These > +include, but are not limited to: Why don't we have an exhaustive list here? > +@item thread-id > +If a fixed variable object is bound to a specific thread, then this is > +the thread's identifier. Is "fixed" used here as opposed to "dynamic"? If so, it would be a good idea to say explicitly what, if anything, this attribute means for a dynamic varobj. (If that's not the intent, please explain the significance of the word "fixed" here; I cannot find "fixed varobj" anywhere else in the manual.) > +reset and all children will be reported. Otherwise, children starting > +at @var{from} (zero-based) and ending just before @var{to} will be > +reported. Since "ending just before" is a bit ambiguous, suggest to reword thusly: Otherwise, children starting at @var{from} (zero-based) and up to and excluding @var{to} will be reported. > +If a child range is requested, it will not affect the range of > +children reported by a future call to @code{-var-update}. Would it be better to say explicitly that only the current calls are affected? Like this: If a child range is requested, it will only affect the current call to @code{-var-update}, but not the future calls. > +For a dynamic varobj, this value cannot reliably be used to form an > +expression. Also, a dynamic varobj will not report the access > +qualifying pseudo-children, regardless of the language. Do we want to give the reader some hints as to how to achieve these with dynamic varobjs? > +A dynamic varobj can supply a display hint to the front end. The > +comes directly from the Python pretty-printer object's > +@code{display_hint} method. @xref{Pretty Printing}. Something is missing in the second sentence. > +children will be reported. Otherwise, children starting at @var{from} > +(zero-based) and ending just before @var{to} will be reported. Same comment as above about "ending just before". Finally, I think we need a @cindex entry for "dynamic varobj" pointing to the section where they are introduced (with a @dfn). Thanks.