From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13711 invoked by alias); 17 Jun 2005 11:42:33 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 13680 invoked by uid 22791); 17 Jun 2005 11:42:28 -0000 Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.8) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Fri, 17 Jun 2005 11:42:28 +0000 Received: from farnswood.snap.net.nz (p1-tnt2.snap.net.nz [202.124.108.1]) by viper.snap.net.nz (Postfix) with ESMTP id 9E91353BE81; Fri, 17 Jun 2005 23:41:03 +1200 (NZST) Received: by farnswood.snap.net.nz (Postfix, from userid 501) id 37E5962A99; Fri, 17 Jun 2005 12:42:54 +0100 (BST) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <17074.46909.592235.541072@farnswood.snap.net.nz> Date: Fri, 17 Jun 2005 11:42:00 -0000 To: Daniel Jacobowitz Cc: Bob Rossi , gdb-patches@sources.redhat.com Subject: Re: [PATCH: gdb/mi + doco] -var-update In-Reply-To: <20050617034329.GH17013@nevyn.them.org> References: <16921.18627.457594.938060@farnswood.snap.net.nz> <01c517d0$Blat.v2.4$09a26040@zahav.net.il> <16922.43915.346792.973282@farnswood.snap.net.nz> <01c51898$Blat.v2.4$f6fd05c0@zahav.net.il> <16929.8147.933720.246602@farnswood.snap.net.nz> <16955.41017.161288.832646@farnswood.snap.net.nz> <20050401024942.GA2179@white> <17013.35649.62745.226730@farnswood.snap.net.nz> <20050502040526.GA10023@nevyn.them.org> <17013.54662.20554.239976@farnswood.snap.net.nz> <20050617034329.GH17013@nevyn.them.org> X-SW-Source: 2005-06/txt/msg00253.txt.bz2 > This is basically OK. A couple of things: ... > Stray colon there. ... > Both here and in mi-cmd-var.c, please list these in the changelog: > * mi/mi-cmds.h (mi_no_values, mi_with_values, mi_simple_values) > (mi_all_values): New declarations. > * mi/mi-cmds.c (mi_no_values, mi_with_values, mi_simple_values) > (mi_all_values): New string constants. ... OK. I've got that. > You've replaced "--all-values" in the source with "--with-values" here. > Surely that's a bug? > > I don't remember the entire outcome of your discussion with Eli, but I > find the idea of having --with-values sometimes and --all-values other > times a bit confusing. I went trying to figure out which meant what > and that's when I noticed this problem. I don't mind that much which way its done. These aren't commands that get typed in by the user. If you and Eli decide then I'll implement it, but please see the comment below about "0"/"1" behaviour, > > ! if (argc == 1) > > ! name = argv[0]; > > ! else > > ! name = (argv[1]); > > Stray parentheses. I don't follow. > > ! if (argc == 2) > > ! { > > ! if (strcmp (argv[0], "0") == 0 > > ! || strcmp (argv[0], mi_no_values) == 0) > > ! print_values = PRINT_NO_VALUES; > > ! else if (strcmp (argv[0], "1") == 0 > > ! || strcmp (argv[0], mi_with_values) == 0) > > ! print_values = PRINT_ALL_VALUES; > > ! else > > ! error (_("Unknown value for PRINT_VALUES: \ > > ! must be: 0 or \"%s\", 1 or \"%s\""), mi_no_values, mi_with_values); > > ! } > > ! else > > ! print_values = PRINT_NO_VALUES; > > > > /* Check if the parameter is a "*" which means that we want > > to update all variables */ > > IIRC, you added the "0"/"1" compatibility to -var-list-children to make > life easier for Apple. Is that right? If so, do they need it here > also, or can we get away with just --all-values? I've no real > objection to the 0/1, but they're a bit ugly. I think I originally copied the "0"/"1" arguments for -var-list-children from existing behaviour for -stack-list-locals. I also think that Apple had already done something similar but different (looking through the e-mails their arguments had reverse the order: SHOW-VALUE VAROBJ-HANDLE). If these are removed then I need to keep "-all-values" for -var-list-children for backward compatiblity (GDB 6.1 to 6.3?). > This paragraph must be in the wrong place. It's mostly duplicating the > paragraph above it. ... Whoops. I've diffed the wrong file. I'll re-submit it with the patch for the source when I know what syntax you want. Nick