This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [PATCH: gdb/mi + doco] -var-update
- From: "Eli Zaretskii" <eliz at gnu dot org>
- To: Nick Roberts <nickrob at snap dot net dot nz>
- Cc: gdb-patches at sources dot redhat dot com
- Date: Sat, 19 Feb 2005 14:35:13 +0200
- Subject: Re: [PATCH: gdb/mi + doco] -var-update
- References: <16919.7660.144228.334687@farnswood.snap.net.nz>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> From: Nick Roberts <nickrob@snap.net.nz>
> 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.