From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25872 invoked by alias); 6 Jul 2006 17:23:08 -0000 Received: (qmail 25829 invoked by uid 22791); 6 Jul 2006 17:23:07 -0000 X-Spam-Check-By: sourceware.org Received: from lon-del-04.spheriq.net (HELO lon-del-04.spheriq.net) (195.46.50.101) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 06 Jul 2006 17:23:00 +0000 Received: from lon-out-02.spheriq.net ([195.46.50.130]) by lon-del-04.spheriq.net with ESMTP id k66HMvwq005530 for ; Thu, 6 Jul 2006 17:22:57 GMT Received: from lon-cus-01.spheriq.net (lon-cus-01.spheriq.net [195.46.50.37]) by lon-out-02.spheriq.net with ESMTP id k66HMqDT004340 for ; Thu, 6 Jul 2006 17:22:56 GMT Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by lon-cus-01.spheriq.net with ESMTP id k66HMp3s004030 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=OK); Thu, 6 Jul 2006 17:22:52 GMT Received: from zeta.dmz-eu.st.com (ns2.st.com [164.129.230.9]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 07E39DA42; Thu, 6 Jul 2006 17:22:47 +0000 (GMT) Received: from mail1.bri.st.com (mail1.bri.st.com [164.129.8.218]) by zeta.dmz-eu.st.com (STMicroelectronics) with ESMTP id A811F47332; Thu, 6 Jul 2006 17:22:45 +0000 (GMT) Received: from [164.129.15.13] (bri1043.bri.st.com [164.129.15.13]) by mail1.bri.st.com (MOS 3.5.8-GR) with ESMTP id CHU38356 (AUTH stubbsa); Thu, 6 Jul 2006 18:22:42 +0100 (BST) Message-ID: <44AD46E2.6020207@st.com> Date: Thu, 06 Jul 2006 17:23:00 -0000 From: Andrew STUBBS User-Agent: Thunderbird 1.5.0.4 (Windows/20060516) MIME-Version: 1.0 To: Eli Zaretskii , gdb-patches@sources.redhat.com Subject: Re: [PATCH] command trace / source verbose mode References: <437B6228.8010103@st.com> <437C9C07.4020707@st.com> <20060706131559.GA18827@nevyn.them.org> In-Reply-To: <20060706131559.GA18827@nevyn.them.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-07/txt/msg00040.txt.bz2 Daniel Jacobowitz wrote: > Sorry for not getting back to you on this. Never made it back here > when reviewing patches in reverse. Trying in patch tracker order > today. No problem, but I knew the patch tracker would be a good thing. :) > On Thu, Nov 17, 2005 at 03:04:39PM +0000, Andrew STUBBS wrote: >> +/* Command tracing state. */ >> + >> +int source_verbose = 0; >> +int commandtrace = 0; > > You've got two of these, but you always check them together. One > variable and incrementing/decrementing the trace level around source > would work too, right? Yeah, that might work too. But the downside is that if the 'set debug commandtrace on' is issued multiple times then the user will have to turn it off that many times also. >> + /* Is there a '-v' in the string somewhere? */ >> + if (args && (minusv = strstr(args,"-v"))) > > Is there any benefit to supporting this at the end? We've already got > some other commands that are strictly command [options] [args], I > think, or at least we do in MI; I would recommend following the same > model here. If it starts with -v it's an option. Does it do any harm beyond making the code a little more complex? I was rather expecting you to say something about the fact that it changes the argument to the --command option (I documented the change). >> + add_setshow_boolean_cmd ("commandtrace", no_class, &commandtrace, _("\ > > I don't feel too strongly about this, just personal bias, but how about > something other than run-together words for this? We have a lot of > those in the existing code e.g. remotetimeout, but we've been trying to > either use hyphens or use spaces and sub-menus lately, I think. > Something like "set trace-commands". Or, alternatively "set debug > cli-commands". Well, since we have been using this for a long time now (long before I posted the patch) I will need to keep this version in our own sources for purposes of backwards compatibility. That being so, it really doesn't matter to me what official name is chosen. I think keeping it under 'set debug' is a good idea, but maybe because it is for debugging other than GDB itself it should be promoted to somewhere more prominent. Hmmm, perhaps "set trace-commands" it the best option. I'll have a think about it, work on the other points you raised, and get back to you. Andrew