This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH: gdb/mi + doco] -var-update


> 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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]