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

[Nick Roberts <nickrob@snap.net.nz>]

Thanks for the prompt review. I think I understand the comments about
formatting. I can make these changes.

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

Why would they be translated?  Could you please elaborate?

Do you mean something like:

const char novalues[] = "\"--no-values\"";
const char allvalues[] = "\"--all-values\";"

error (_("Unknown value for PRINT_VALUES: must be: 0 or %s, 1 or %s",
       novalues, allvalues));

 > Also, didn't we decide to leave the messages emitted by MI
 > untranslatable?

Are you referring to the underscore with brackets? [ _() ]
I'm not familiar with this device but this line has been cut and pasted from
mi_cmd_var_list_children and all the other error messages in MI have it too.

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

I'm not sure what you mean here. Are you saying that there should be two
examples, one with print-values and one without?  The previous example
explaining -var-assign already demonstrates the use of -var-update without
print-values.

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

If it was a CLI command I would agree but the exact syntax of MI commands only
has to be referred to by developers and not remembered by users.

I've used "--all-values" because, in the case of -var-list-children there is a
third possibility: "--simple-values" and, to me, it seems simpler to have only
three values for print_values (mi-cmds.h):

enum print_values {
   PRINT_NO_VALUES,
   PRINT_ALL_VALUES,
   PRINT_SIMPLE_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.

I've just adapted the previous example, but if you mean replace:

> + -var-update --all-values *

with

> + -var-update --all-values var1

that's no problem.


Nick