From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29692 invoked by alias); 19 Feb 2005 12:36:46 -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 29457 invoked from network); 19 Feb 2005 12:36:33 -0000 Received: from unknown (HELO romy.inter.net.il) (192.114.186.66) by sourceware.org with SMTP; 19 Feb 2005 12:36:33 -0000 Received: from zaretski (pns03-208-119.inter.net.il [80.230.208.119]) by romy.inter.net.il (MOS 3.5.6-GR) with ESMTP id AON75922 (AUTH halo1); Sat, 19 Feb 2005 14:36:15 +0200 (IST) Date: Sun, 20 Feb 2005 05:02:00 -0000 From: "Eli Zaretskii" To: Nick Roberts Message-ID: <01c5167f$Blat.v2.4$9a7a6f60@zahav.net.il> Content-Transfer-Encoding: 7BIT Content-Type: text/plain; charset=ISO-8859-1 CC: gdb-patches@sources.redhat.com In-reply-to: <16919.7660.144228.334687@farnswood.snap.net.nz> (message from Nick Roberts on Sun, 20 Feb 2005 00:07:24 +1300) Subject: Re: [PATCH: gdb/mi + doco] -var-update Reply-to: Eli Zaretskii References: <16919.7660.144228.334687@farnswood.snap.net.nz> X-SW-Source: 2005-02/txt/msg00207.txt.bz2 > From: Nick Roberts > Date: Sun, 20 Feb 2005 00:07:24 +1300 > > Currently the MI command "-var-update" gives the names of the variable objects > but not their value which must be accessed individually (using the MI command > -var-evaluate-expression). This means that a front end can take too long > processing a separate MI command for variable object. This patch adapts > "-var-update" so that it gives values, if required. The existing behaviour is > preserved for backward compatibility. Thanks. I think this is a good idea, but I have comments about the implementation and the docs. > ! if (argc == 1) name = argv[0]; > ! else name = (argv[1]); Please change the formatting and indentation to conform to the GNU coding standards. > ! if (argc == 2) > ! if (strcmp (argv[0], "0") == 0 > ! || strcmp (argv[0], "--no-values") == 0) Please add braces for the outer `if' clause. Also, the indentation is not according to the GNU standards. > ! error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\"")); Please remove "--no-values" and "--all-values" from this string. They are literal strings that must not be translated, and in addition they are used several times elsewhere in the code. So I suggest to have them defined only once, as const char [], and the rest of code use those const strings; e.g., in the above case, use %s in the string and pass the strings as additional arguments to the `error' function. Also, didn't we decide to leave the messages emitted by MI untranslatable? > ! else print_values = PRINT_NO_VALUES; Formatting not according to GNU coding standards. > @smallexample > ! -var-update [@var{print-values}] @{@var{name} | "*"@} > @end smallexample > > Update the value of the variable object @var{name} by evaluating its > expression after fetching all the new values from memory or registers. > ! A @samp{*} causes all existing variable objects to be updated. With > ! just a single argument or with an optional preceding argument of 0 or > ! @code{--no-values}, prints only the names of the variables. With an > ! optional preceding argument of 1 or @code{--all-values}, also prints > ! their values. This text should refer to @var{print-values} you used inside @smallexample, otherwise it is not clear what should be used in its stead. Also, I find the choice of "--all-values" unfortunate. The opposite of "--no-values" is something like "--with-values" or "--print-values", not "--all-values". > + @subsubheading Example > + > + @smallexample > + (@value{GDBP}) > + -var-assign var1 3 > + ^done,value="3" > + (@value{GDBP}) > + -var-update --all-values * I'd suggest to have an example that uses a specific name instead of "*". Examples should show typical usage; if you want to show special cases, show them _in_addition_ to typical ones.