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

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

[Nick Roberts]

This patch is very similar to ones that I have previously contributed for
-var-list-children and -stack-list-locals. Testcases to follow
(assuming the patch is favourably received).

Nick


Purpose:  

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.

*** /home/nick/src/gdb/mi/mi-cmd-var.c.~1.21.~	2005-02-13 00:36:20.000000000 +1300
--- /home/nick/src/gdb/mi/mi-cmd-var.c	2005-02-19 22:49:18.000000000 +1300
***************
*** 32,38 ****
  
  extern int varobjdebug;		/* defined in varobj.c */
  
! static int varobj_update_one (struct varobj *var);
  
  /* VAROBJ operations */
  
--- 32,39 ----
  
  extern int varobjdebug;		/* defined in varobj.c */
  
! static int varobj_update_one (struct varobj *var,
! 			      enum print_values print_values);
  
  /* VAROBJ operations */
  
***************
*** 426,436 ****
    struct cleanup *cleanup;
    char *name;
    int nv;
  
!   if (argc != 1)
!     error (_("mi_cmd_var_update: Usage: NAME."));
  
!   name = argv[0];
  
    /* Check if the parameter is a "*" which means that we want
       to update all variables */
--- 427,450 ----
    struct cleanup *cleanup;
    char *name;
    int nv;
+   enum print_values print_values;
  
!   if (argc != 1 && argc != 2)
!     error (_("mi_cmd_var_update: Usage: [PRINT_VALUES] NAME."));
  
!   if (argc == 1)  name = argv[0];
!   else name = (argv[1]);
!  
!   if (argc == 2)
!     if (strcmp (argv[0], "0") == 0
! 	|| strcmp (argv[0], "--no-values") == 0)
!       print_values = PRINT_NO_VALUES;
!     else if (strcmp (argv[0], "1") == 0
! 	     || strcmp (argv[0], "--all-values") == 0)
!       print_values = PRINT_ALL_VALUES;
!     else
!      error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\""));
!   else print_values = PRINT_NO_VALUES;
  
    /* Check if the parameter is a "*" which means that we want
       to update all variables */
***************
*** 450,456 ****
        cr = rootlist;
        while (*cr != NULL)
  	{
! 	  varobj_update_one (*cr);
  	  cr++;
  	}
        xfree (rootlist);
--- 464,470 ----
        cr = rootlist;
        while (*cr != NULL)
  	{
! 	  varobj_update_one (*cr, print_values);
  	  cr++;
  	}
        xfree (rootlist);
***************
*** 467,473 ****
          cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
        else
          cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
!       varobj_update_one (var);
        do_cleanups (cleanup);
      }
      return MI_CMD_DONE;
--- 481,487 ----
          cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
        else
          cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
!       varobj_update_one (var, print_values);
        do_cleanups (cleanup);
      }
      return MI_CMD_DONE;
***************
*** 478,484 ****
     scope), and 1 if it succeeds. */
  
  static int
! varobj_update_one (struct varobj *var)
  {
    struct varobj **changelist;
    struct varobj **cc;
--- 492,498 ----
     scope), and 1 if it succeeds. */
  
  static int
! varobj_update_one (struct varobj *var, enum print_values print_values)
  {
    struct varobj **changelist;
    struct varobj **cc;
***************
*** 524,529 ****
--- 538,545 ----
  	  if (mi_version (uiout) > 1)
  	    cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
  	  ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
+ 	  if (print_values)
+ 	    ui_out_field_string (uiout, "value", varobj_get_value (*cc));
  	  ui_out_field_string (uiout, "in_scope", "true");
  	  ui_out_field_string (uiout, "type_changed", "false");
  	  if (mi_version (uiout) > 1)


*** /home/nick/src/gdb/doc/gdb.texinfo.~1.232.~	2005-02-13 00:36:20.000000000 +1300
--- /home/nick/src/gdb/doc/gdb.texinfo	2005-02-19 23:50:12.000000000 +1300
***************
*** 18921,18933 ****
  @subsubheading Synopsis
  
  @smallexample
!  -var-update @{@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.
  
  
  @node Annotations
  @chapter @value{GDBN} Annotations
--- 18921,18949 ----
  @subsubheading Synopsis
  
  @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.
  
+ @subsubheading Example
+ 
+ @smallexample
+ (@value{GDBP})
+ -var-assign var1 3
+ ^done,value="3"
+ (@value{GDBP})
+ -var-update --all-values *
+ ^done,changelist=[@{name="var1",value="3",in_scope="true",
+ type_changed="false"@}]
+ (@value{GDBP})
+ @end smallexample
  
  @node Annotations
  @chapter @value{GDBN} Annotations

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

[Eli Zaretskii]

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

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

[Nick Roberts]

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

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

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Sun, 20 Feb 2005 12:49:55 +1300
> Cc: gdb-patches@sources.redhat.com
> 
>  > > !      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?

Because they are part of a string that is an argument to the _()
macro.  Such strings are extracted by the gettext package into a
message catalog, which is then used by translators to prepare
translations for those strings.  A translator looking at a string such
as

   Unknown value for PRINT_VALUES: must be: 0 or "--no-values", 1 or "--all-values\

could erroneously decide that "--all-values" and "--no-values" need to
be translated as well, if he/she doesn't know MI too well.

> 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));

Yes.

>  > Also, didn't we decide to leave the messages emitted by MI
>  > untranslatable?
> 
> Are you referring to the underscore with brackets? [ _() ]

Yes.

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

If other messages are marked up with _(), then this is okay.  But I
still think the const char [] strings should be defined once and used
elsewhere in this function.

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

The usage description used @var{print-values}, which is a variable
parameter, but the text does not refer to @var{print-values}.  A
reader will not understand what to put instead of @var{print-values}
if you don't mention it.

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

In this case, users==developers.  Mnemonic names matter even for
developers of GDB front ends.

> 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
> };

I see the reason, but I think it is not important enough to use
"--all-values".  Anyway, the switch text does not need to be similar
to the enum name, even if you use PRINT_ALL_VALUES in the enum.

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

Yes, that'd be good.

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

[Andrew Cagney]

Nick Roberts wrote:
> This patch is very similar to ones that I have previously contributed for
> -var-list-children and -stack-list-locals. Testcases to follow
> (assuming the patch is favourably received).
> 
> Nick
> 
> 
> Purpose:  
> 
> 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.
> 
> *** /home/nick/src/gdb/mi/mi-cmd-var.c.~1.21.~	2005-02-13 00:36:20.000000000 +1300
> --- /home/nick/src/gdb/mi/mi-cmd-var.c	2005-02-19 22:49:18.000000000 +1300
> ***************
> *** 32,38 ****
>   
>   extern int varobjdebug;		/* defined in varobj.c */
>   
> ! static int varobj_update_one (struct varobj *var);
>   
>   /* VAROBJ operations */
>   
> --- 32,39 ----
>   
>   extern int varobjdebug;		/* defined in varobj.c */
>   
> ! static int varobj_update_one (struct varobj *var,
> ! 			      enum print_values print_values);
>   
>   /* VAROBJ operations */
>   
> ***************
> *** 426,436 ****
>     struct cleanup *cleanup;
>     char *name;
>     int nv;
>   
> !   if (argc != 1)
> !     error (_("mi_cmd_var_update: Usage: NAME."));
>   
> !   name = argv[0];
>   
>     /* Check if the parameter is a "*" which means that we want
>        to update all variables */
> --- 427,450 ----
>     struct cleanup *cleanup;
>     char *name;
>     int nv;
> +   enum print_values print_values;
>   
> !   if (argc != 1 && argc != 2)
> !     error (_("mi_cmd_var_update: Usage: [PRINT_VALUES] NAME."));
>   
> !   if (argc == 1)  name = argv[0];
> !   else name = (argv[1]);
> !  
> !   if (argc == 2)
> !     if (strcmp (argv[0], "0") == 0
> ! 	|| strcmp (argv[0], "--no-values") == 0)
> !       print_values = PRINT_NO_VALUES;
> !     else if (strcmp (argv[0], "1") == 0
> ! 	     || strcmp (argv[0], "--all-values") == 0)
> !       print_values = PRINT_ALL_VALUES;
> !     else
> !      error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\""));
> !   else print_values = PRINT_NO_VALUES;

Nick,

it should use mi-getopt (which correctly [?] implements the MI input 
syntax).  If you find doing this alters the commands semantics (quoting 
of parameters might be affected) then we'll need to introduce a new 
``fixed'' command.

Andrew

(I'm fine with the theory)

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

[Nick Roberts]

 > it should use mi-getopt (which correctly [?] implements the MI input 
 > syntax).  If you find doing this alters the commands semantics (quoting 
 > of parameters might be affected) then we'll need to introduce a new 
 > ``fixed'' command.

Hmm. I'm not very happy with adding another level of abstraction, at the
moment. I can see that it might force the command to have the right syntax
but, at least for me, it also makes the code quite unreadable.

This patch uses the same methods as applied to my previous ones for
-var-list-children and -stack-list-locals which, in fact, you guided me
through. mi-getopt was also available at that time.

Are you saying that you won't accept the current patch, with Eli's concerns
addressed, or is it possible to adapt for mi-getopt at a later stage, when
MI is more stable?


Nick

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

[Nick Roberts]

 > > 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.
 > 
 > If other messages are marked up with _(), then this is okay.  But I
 > still think the const char [] strings should be defined once and used
 > elsewhere in this function.

OK but this only makes sense if it is done consistently. So I offer to
do it for all of the MI error messages. I'll submit one example first.

 > >  > 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.
 > 
 > The usage description used @var{print-values}, which is a variable
 > parameter, but the text does not refer to @var{print-values}.  A
 > reader will not understand what to put instead of @var{print-values}
 > if you don't mention it.

I think I follow that now.

 > >  > 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.
 > 
 > In this case, users==developers.  Mnemonic names matter even for
 > developers of GDB front ends.
 > 
 > > 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
 > > };
 > 
 > I see the reason, but I think it is not important enough to use
 > "--all-values".  Anyway, the switch text does not need to be similar
 > to the enum name, even if you use PRINT_ALL_VALUES in the enum.

OK. But again this only makes sense if I change -var-list-children too.
This wouldn't be backward compatible but I doubt that anyone is currently
relying on it. Is that acceptable?


Nick

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

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Mon, 21 Feb 2005 15:34:43 +1300
> Cc: gdb-patches@sources.redhat.com
> 
>  > If other messages are marked up with _(), then this is okay.  But I
>  > still think the const char [] strings should be defined once and used
>  > elsewhere in this function.
> 
> OK but this only makes sense if it is done consistently. So I offer to
> do it for all of the MI error messages.

It should be done for all messages, yes; but I'd like at least to have
the new messages follow that principle.

>  > > 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
>  > > };
>  > 
>  > I see the reason, but I think it is not important enough to use
>  > "--all-values".  Anyway, the switch text does not need to be similar
>  > to the enum name, even if you use PRINT_ALL_VALUES in the enum.
> 
> OK. But again this only makes sense if I change -var-list-children too.

What change did you have in mind?

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

[Nick Roberts]

 > >  > I see the reason, but I think it is not important enough to use
 > >  > "--all-values".  Anyway, the switch text does not need to be similar
 > >  > to the enum name, even if you use PRINT_ALL_VALUES in the enum.
 > > 
 > > OK. But again this only makes sense if I change -var-list-children too.
 > 
 > What change did you have in mind?

info>      -var-list-children [PRINT-VALUES] NAME

info> Returns a list of the children of the specified variable object.
info> With just the variable object name as an argument or with an optional
info> preceding argument of 0 or `--no-values', prints only the names of the
info> variables.  With an optional preceding argument of 1 or `--all-values',
info> also prints their values.                                ^^^^^^^^^^^^

If -var-update uses "--with-values" or "--print-values", then so should
-var-list-children.


Nick

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

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Tue, 22 Feb 2005 16:48:27 +1300
> Cc: gdb-patches@sources.redhat.com
> 
> info>      -var-list-children [PRINT-VALUES] NAME
> 
> info> Returns a list of the children of the specified variable object.
> info> With just the variable object name as an argument or with an optional
> info> preceding argument of 0 or `--no-values', prints only the names of the
> info> variables.  With an optional preceding argument of 1 or `--all-values',
> info> also prints their values.                                ^^^^^^^^^^^^
> 
> If -var-update uses "--with-values" or "--print-values", then so should
> -var-list-children.

Okay.

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

[Nick Roberts]

Here is my latest set of patches.

Eli,

I think it addresses your concerns. I have changed -var-list-children to
use --with-values instead of --all-values. In the unlikely event that any other
frontend is using this, "-var-list-children 1 varno" will still be backwardly
compatible.

Andrew,

Is this acceptable or is mi-getopt a pre-condition?


Nick


*** /home/nick/src/gdb/mi/mi-cmd-var.c.~1.21.~	2005-02-13 00:36:20.000000000 +1300
--- /home/nick/src/gdb/mi/mi-cmd-var.c	2005-02-27 14:14:36.000000000 +1300
***************
*** 30,35 ****
--- 30,40 ----
  #include <ctype.h>
  #include "gdb_string.h"
  
+ const char novalues[] = "\"--no-values\"";
+ const char withvalues[] = "\"--with-values\"";
+ const char simplevalues[] = "\"--simple-values\"";
+ const char allvalues[] = "\"--all-values\"";
+ 
  extern int varobjdebug;		/* defined in varobj.c */
  
  static int varobj_update_one (struct varobj *var);
***************
*** 262,284 ****
      error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
  
    /* Get varobj handle, if a valid var obj name was specified */
!   if (argc == 1) var = varobj_get_handle (argv[0]);
!   else var = varobj_get_handle (argv[1]);
    if (var == NULL)
      error (_("Variable object not found"));
  
    numchild = varobj_list_children (var, &childlist);
    ui_out_field_int (uiout, "numchild", numchild);
    if (argc == 2)
!     if (strcmp (argv[0], "0") == 0
! 	|| strcmp (argv[0], "--no-values") == 0)
!       print_values = PRINT_NO_VALUES;
!     else if (strcmp (argv[0], "1") == 0
! 	     || strcmp (argv[0], "--all-values") == 0)
!       print_values = PRINT_ALL_VALUES;
!     else
!      error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\""));
!   else print_values = PRINT_NO_VALUES;
  
    if (numchild <= 0)
      return MI_CMD_DONE;
--- 267,295 ----
      error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
  
    /* Get varobj handle, if a valid var obj name was specified */
!   if (argc == 1)
!     var = varobj_get_handle (argv[0]);
!   else
!     var = varobj_get_handle (argv[1]);
    if (var == NULL)
      error (_("Variable object not found"));
  
    numchild = varobj_list_children (var, &childlist);
    ui_out_field_int (uiout, "numchild", numchild);
    if (argc == 2)
!     {
!       if (strcmp (argv[0], "0") == 0
! 	  || strcmp (argv[0], "--no-values") == 0)
! 	print_values = PRINT_NO_VALUES;
!       else if (strcmp (argv[0], "1") == 0
! 	       || strcmp (argv[0], "--with-values") == 0)
! 	print_values = PRINT_ALL_VALUES;
!       else
! 	error (_("Unknown value for PRINT_VALUES: must be: 0 or %s, 1 or %s"),
! 	       novalues, withvalues);
!     }
!   else
!     print_values = PRINT_NO_VALUES;
  
    if (numchild <= 0)
      return MI_CMD_DONE;
***************
*** 426,436 ****
    struct cleanup *cleanup;
    char *name;
    int nv;
  
!   if (argc != 1)
!     error (_("mi_cmd_var_update: Usage: NAME."));
  
!   name = argv[0];
  
    /* Check if the parameter is a "*" which means that we want
       to update all variables */
--- 437,466 ----
    struct cleanup *cleanup;
    char *name;
    int nv;
+   enum print_values print_values;
  
!   if (argc != 1 && argc != 2)
!     error (_("mi_cmd_var_update: Usage: [PRINT_VALUES] NAME."));
  
!   if (argc == 1)
!     name = argv[0];
!   else
!     name = (argv[1]);
! 
!   if (argc == 2)
!     {
!       if (strcmp (argv[0], "0") == 0
! 	  || strcmp (argv[0], "--no-values") == 0)
! 	print_values = PRINT_NO_VALUES;
!       else if (strcmp (argv[0], "1") == 0
! 	       || strcmp (argv[0], "--with-values") == 0)
! 	print_values = PRINT_ALL_VALUES;
!       else
! 	error (_("Unknown value for PRINT_VALUES: must be: 0 or %s, 1 or %s"),
! 		 novalues, withvalues);
!     }
!   else
!     print_values = PRINT_NO_VALUES;
  
    /* Check if the parameter is a "*" which means that we want
       to update all variables */
***************
*** 524,529 ****
--- 554,561 ----
  	  if (mi_version (uiout) > 1)
  	    cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
  	  ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
+ 	  if (print_values)
+ 	    ui_out_field_string (uiout, "value", varobj_get_value (*cc));
  	  ui_out_field_string (uiout, "in_scope", "true");
  	  ui_out_field_string (uiout, "type_changed", "false");
  	  if (mi_version (uiout) > 1)


*** /home/nick/src/gdb/mi/mi-cmd-stack.c.~1.25.~	2005-02-13 00:36:20.000000000 +1300
--- /home/nick/src/gdb/mi/mi-cmd-stack.c	2005-02-26 19:50:40.000000000 +1300
***************
*** 151,157 ****
  	    || strcmp (argv[0], "--simple-values") == 0)
       print_values = PRINT_SIMPLE_VALUES;
     else
!      error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\", 2 or \"--simple-values\""));
    list_args_or_locals (1, print_values, frame);
    return MI_CMD_DONE;
  }
--- 151,158 ----
  	    || strcmp (argv[0], "--simple-values") == 0)
       print_values = PRINT_SIMPLE_VALUES;
     else
!      error (_("Unknown value for PRINT_VALUES: must be: 0 or %s, 1 or %s, 2 or %s"),
! 	    novalues, allvalues, simplevalues);
    list_args_or_locals (1, print_values, frame);
    return MI_CMD_DONE;
  }


*** /home/nick/src/gdb/mi/mi-cmds.h.~1.15.~	2005-01-25 22:30:39.000000000 +1300
--- /home/nick/src/gdb/mi/mi-cmds.h	2005-02-26 19:44:02.000000000 +1300
***************
*** 50,55 ****
--- 50,60 ----
     PRINT_SIMPLE_VALUES
  };
  
+ extern const char novalues[];
+ extern const char withvalues[];
+ extern const char simplevalues[];
+ extern const char allvalues[];
+ 
  typedef enum mi_cmd_result (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
  
  /* Older MI commands have this interface. Retained until all old


*** /home/nick/src/gdb/doc/gdb.texinfo.~1.232.~	2005-02-13 00:36:20.000000000 +1300
--- /home/nick/src/gdb/doc/gdb.texinfo	2005-02-26 19:41:39.000000000 +1300
***************
*** 18797,18805 ****
  
  Returns a list of the children of the specified variable object.  With
  just the variable object name as an 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.
  
  @subsubheading Example
  
--- 18797,18805 ----
  
  Returns a list of the children of the specified variable object.  With
  just the variable object name as an argument or with an optional
! preceding argument of 0 or @code{--no-values}, prints only the names
! of the variables.  With an optional value for @var{print-values} of 1
! or @code{--with-values}, also prints their values.
  
  @subsubheading Example
  
***************
*** 18921,18933 ****
  @subsubheading Synopsis
  
  @smallexample
!  -var-update @{@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.
  
  
  @node Annotations
  @chapter @value{GDBN} Annotations
--- 18921,18949 ----
  @subsubheading Synopsis
  
  @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 value for
! @var{print-values} of 0 or @code{--no-values}, prints only the names
! of the variables.  A value for @var{print-values} of 1 or
! @code{--with-values}, also prints their values.
  
+ @subsubheading Example
+ 
+ @smallexample
+ (@value{GDBP})
+ -var-assign var1 3
+ ^done,value="3"
+ (@value{GDBP})
+ -var-update --all-values var1
+ ^done,changelist=[@{name="var1",value="3",in_scope="true",
+ type_changed="false"@}]
+ (@value{GDBP})
+ @end smallexample
  
  @node Annotations
  @chapter @value{GDBN} Annotations

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

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Sun, 27 Feb 2005 14:18:11 +1300
> Cc: gdb-patches@sources.redhat.com
> 
> --- 18797,18805 ----
>   
>   Returns a list of the children of the specified variable object.  With
>   just the variable object name as an argument or with an optional
> ! preceding argument of 0 or @code{--no-values}, prints only the names
> ! of the variables.  With an optional value for @var{print-values} of 1
> ! or @code{--with-values}, also prints their values.

It's better to say

  If @var{print-values} is 1 or @code{--with-values}, also prints
  their values.

And similarly in the other hunk where @var{print-values} is
mentioned.

Otherwise, okay for the documentation part.

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

[Nick Roberts]

 > >   Returns a list of the children of the specified variable object.  With
 > >   just the variable object name as an argument or with an optional
 > > ! preceding argument of 0 or @code{--no-values}, prints only the names
 > > ! of the variables.  With an optional value for @var{print-values} of 1
 > > ! or @code{--with-values}, also prints their values.
 > 
 > It's better to say
 > 
 >   If @var{print-values} is 1 or @code{--with-values}, also prints
 >   their values.
 > 
 > And similarly in the other hunk where @var{print-values} is
 > mentioned.

Clearly this reads much better, but will the reader realise that print-values
is optional? The use of square brackets is explained but this is right at the
beginning of the section on GDB/MI.


Nick

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

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Sun, 27 Feb 2005 21:22:05 +1300
> Cc: gdb-patches@sources.redhat.com
> 
>  >   If @var{print-values} is 1 or @code{--with-values}, also prints
>  >   their values.
>  > 
>  > And similarly in the other hunk where @var{print-values} is
>  > mentioned.
> 
> Clearly this reads much better, but will the reader realise that print-values
> is optional? The use of square brackets is explained but this is right at the
> beginning of the section on GDB/MI.

I think the use of [this] for optional parts is sufficiently
widespread in computer-related documentation, for this to be quite
clear.

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

[Nick Roberts]

I would like to commit this (source code) patch from last month. I accidently
reverted some of changes last time I presented it so I attach it here in its
proper form.

As Andrew doesn't appear to around is there any chance a global maintainer
can look at it? Its pretty lightweight.

Nick

*** /home/nick/src/gdb/mi/mi-cmd-var.c.~1.21.~	2005-02-13 00:36:20.000000000 +1300
--- /home/nick/src/gdb/mi/mi-cmd-var.c	2005-03-01 14:03:45.000000000 +1300
***************
*** 30,38 ****
  #include <ctype.h>
  #include "gdb_string.h"
  
  extern int varobjdebug;		/* defined in varobj.c */
  
! static int varobj_update_one (struct varobj *var);
  
  /* VAROBJ operations */
  
--- 30,44 ----
  #include <ctype.h>
  #include "gdb_string.h"
  
+ const char novalues[] = "\"--no-values\"";
+ const char withvalues[] = "\"--with-values\"";
+ const char simplevalues[] = "\"--simple-values\"";
+ const char allvalues[] = "\"--all-values\"";
+ 
  extern int varobjdebug;		/* defined in varobj.c */
  
! static int varobj_update_one (struct varobj *var,
! 			      enum print_values print_values);
  
  /* VAROBJ operations */
  
***************
*** 262,284 ****
      error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
  
    /* Get varobj handle, if a valid var obj name was specified */
!   if (argc == 1) var = varobj_get_handle (argv[0]);
!   else var = varobj_get_handle (argv[1]);
    if (var == NULL)
      error (_("Variable object not found"));
  
    numchild = varobj_list_children (var, &childlist);
    ui_out_field_int (uiout, "numchild", numchild);
    if (argc == 2)
!     if (strcmp (argv[0], "0") == 0
! 	|| strcmp (argv[0], "--no-values") == 0)
!       print_values = PRINT_NO_VALUES;
!     else if (strcmp (argv[0], "1") == 0
! 	     || strcmp (argv[0], "--all-values") == 0)
!       print_values = PRINT_ALL_VALUES;
!     else
!      error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\""));
!   else print_values = PRINT_NO_VALUES;
  
    if (numchild <= 0)
      return MI_CMD_DONE;
--- 268,296 ----
      error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
  
    /* Get varobj handle, if a valid var obj name was specified */
!   if (argc == 1)
!     var = varobj_get_handle (argv[0]);
!   else
!     var = varobj_get_handle (argv[1]);
    if (var == NULL)
      error (_("Variable object not found"));
  
    numchild = varobj_list_children (var, &childlist);
    ui_out_field_int (uiout, "numchild", numchild);
    if (argc == 2)
!     {
!       if (strcmp (argv[0], "0") == 0
! 	  || strcmp (argv[0], "--no-values") == 0)
! 	print_values = PRINT_NO_VALUES;
!       else if (strcmp (argv[0], "1") == 0
! 	       || strcmp (argv[0], "--with-values") == 0)
! 	print_values = PRINT_ALL_VALUES;
!       else
! 	error (_("Unknown value for PRINT_VALUES: must be: 0 or %s, 1 or %s"),
! 	       novalues, withvalues);
!     }
!   else
!     print_values = PRINT_NO_VALUES;
  
    if (numchild <= 0)
      return MI_CMD_DONE;
***************
*** 426,436 ****
    struct cleanup *cleanup;
    char *name;
    int nv;
  
!   if (argc != 1)
!     error (_("mi_cmd_var_update: Usage: NAME."));
  
!   name = argv[0];
  
    /* Check if the parameter is a "*" which means that we want
       to update all variables */
--- 438,467 ----
    struct cleanup *cleanup;
    char *name;
    int nv;
+   enum print_values print_values;
  
!   if (argc != 1 && argc != 2)
!     error (_("mi_cmd_var_update: Usage: [PRINT_VALUES] NAME."));
  
!   if (argc == 1)
!     name = argv[0];
!   else
!     name = (argv[1]);
! 
!   if (argc == 2)
!     {
!       if (strcmp (argv[0], "0") == 0
! 	  || strcmp (argv[0], "--no-values") == 0)
! 	print_values = PRINT_NO_VALUES;
!       else if (strcmp (argv[0], "1") == 0
! 	       || strcmp (argv[0], "--with-values") == 0)
! 	print_values = PRINT_ALL_VALUES;
!       else
! 	error (_("Unknown value for PRINT_VALUES: must be: 0 or %s, 1 or %s"),
! 		 novalues, withvalues);
!     }
!   else
!     print_values = PRINT_NO_VALUES;
  
    /* Check if the parameter is a "*" which means that we want
       to update all variables */
***************
*** 450,456 ****
        cr = rootlist;
        while (*cr != NULL)
  	{
! 	  varobj_update_one (*cr);
  	  cr++;
  	}
        xfree (rootlist);
--- 481,487 ----
        cr = rootlist;
        while (*cr != NULL)
  	{
! 	  varobj_update_one (*cr, print_values);
  	  cr++;
  	}
        xfree (rootlist);
***************
*** 467,473 ****
          cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
        else
          cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
!       varobj_update_one (var);
        do_cleanups (cleanup);
      }
      return MI_CMD_DONE;
--- 498,504 ----
          cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
        else
          cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
!       varobj_update_one (var, print_values);
        do_cleanups (cleanup);
      }
      return MI_CMD_DONE;
***************
*** 478,484 ****
     scope), and 1 if it succeeds. */
  
  static int
! varobj_update_one (struct varobj *var)
  {
    struct varobj **changelist;
    struct varobj **cc;
--- 509,515 ----
     scope), and 1 if it succeeds. */
  
  static int
! varobj_update_one (struct varobj *var, enum print_values print_values)
  {
    struct varobj **changelist;
    struct varobj **cc;
***************
*** 524,529 ****
--- 555,562 ----
  	  if (mi_version (uiout) > 1)
  	    cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
  	  ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
+ 	  if (print_values)
+ 	    ui_out_field_string (uiout, "value", varobj_get_value (*cc));
  	  ui_out_field_string (uiout, "in_scope", "true");
  	  ui_out_field_string (uiout, "type_changed", "false");
  	  if (mi_version (uiout) > 1)

Diff finished.  Sat Mar 19 16:37:30 2005

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

[Bob Rossi]

On Sat, Mar 19, 2005 at 04:44:57PM +1300, Nick Roberts wrote:
> 
> I would like to commit this (source code) patch from last month. I accidently
> reverted some of changes last time I presented it so I attach it here in its
> proper form.
> 
> As Andrew doesn't appear to around is there any chance a global maintainer
> can look at it? Its pretty lightweight.

Hi Nick,

I have some spare time waiting for my patches to get reviewed, so I
figure'd I'd look at yours. If you care, I have just a few comments.

Bob Rossi

> *** /home/nick/src/gdb/mi/mi-cmd-var.c.~1.21.~	2005-02-13 00:36:20.000000000 +1300
> --- /home/nick/src/gdb/mi/mi-cmd-var.c	2005-03-01 14:03:45.000000000 +1300
> ***************
> *** 30,38 ****
>   #include <ctype.h>
>   #include "gdb_string.h"
>   
>   extern int varobjdebug;		/* defined in varobj.c */
>   
> ! static int varobj_update_one (struct varobj *var);
>   
>   /* VAROBJ operations */
>   
> --- 30,44 ----
>   #include <ctype.h>
>   #include "gdb_string.h"
>   
> + const char novalues[] = "\"--no-values\"";
> + const char withvalues[] = "\"--with-values\"";
> + const char simplevalues[] = "\"--simple-values\"";
> + const char allvalues[] = "\"--all-values\"";

These could be made static.

>   extern int varobjdebug;		/* defined in varobj.c */
>   
> ! static int varobj_update_one (struct varobj *var,
> ! 			      enum print_values print_values);
>   
>   /* VAROBJ operations */
>   
> ***************
> *** 262,284 ****
>       error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
>   
>     /* Get varobj handle, if a valid var obj name was specified */
> !   if (argc == 1) var = varobj_get_handle (argv[0]);
> !   else var = varobj_get_handle (argv[1]);
>     if (var == NULL)
>       error (_("Variable object not found"));
>   
>     numchild = varobj_list_children (var, &childlist);
>     ui_out_field_int (uiout, "numchild", numchild);
>     if (argc == 2)
> !     if (strcmp (argv[0], "0") == 0
> ! 	|| strcmp (argv[0], "--no-values") == 0)
> !       print_values = PRINT_NO_VALUES;
> !     else if (strcmp (argv[0], "1") == 0
> ! 	     || strcmp (argv[0], "--all-values") == 0)
> !       print_values = PRINT_ALL_VALUES;
> !     else
> !      error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\""));
> !   else print_values = PRINT_NO_VALUES;
>   
>     if (numchild <= 0)
>       return MI_CMD_DONE;
> --- 268,296 ----
>       error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
>   
>     /* Get varobj handle, if a valid var obj name was specified */
> !   if (argc == 1)
> !     var = varobj_get_handle (argv[0]);
> !   else
> !     var = varobj_get_handle (argv[1]);
>     if (var == NULL)
>       error (_("Variable object not found"));
>   
>     numchild = varobj_list_children (var, &childlist);
>     ui_out_field_int (uiout, "numchild", numchild);
>     if (argc == 2)
> !     {
> !       if (strcmp (argv[0], "0") == 0
> ! 	  || strcmp (argv[0], "--no-values") == 0)
> ! 	print_values = PRINT_NO_VALUES;
> !       else if (strcmp (argv[0], "1") == 0
> ! 	       || strcmp (argv[0], "--with-values") == 0)

instead of using "--no-values" and "--with-values" you could use the
variable "novalues" and "withvalues".

> ! 	print_values = PRINT_ALL_VALUES;
> !       else
> ! 	error (_("Unknown value for PRINT_VALUES: must be: 0 or %s, 1 or %s"),
> ! 	       novalues, withvalues);
> !     }
> !   else
> !     print_values = PRINT_NO_VALUES;
>   
>     if (numchild <= 0)
>       return MI_CMD_DONE;
> ***************
> *** 426,436 ****
>     struct cleanup *cleanup;
>     char *name;
>     int nv;
>   
> !   if (argc != 1)
> !     error (_("mi_cmd_var_update: Usage: NAME."));
>   
> !   name = argv[0];
>   
>     /* Check if the parameter is a "*" which means that we want
>        to update all variables */
> --- 438,467 ----
>     struct cleanup *cleanup;
>     char *name;
>     int nv;
> +   enum print_values print_values;
>   
> !   if (argc != 1 && argc != 2)
> !     error (_("mi_cmd_var_update: Usage: [PRINT_VALUES] NAME."));
>   
> !   if (argc == 1)
> !     name = argv[0];
> !   else
> !     name = (argv[1]);
> ! 
> !   if (argc == 2)
> !     {
> !       if (strcmp (argv[0], "0") == 0
> ! 	  || strcmp (argv[0], "--no-values") == 0)
> ! 	print_values = PRINT_NO_VALUES;
> !       else if (strcmp (argv[0], "1") == 0
> ! 	       || strcmp (argv[0], "--with-values") == 0)

same as above with the --no-values and --with-values
> ! 	print_values = PRINT_ALL_VALUES;
> !       else
> ! 	error (_("Unknown value for PRINT_VALUES: must be: 0 or %s, 1 or %s"),
> ! 		 novalues, withvalues);
> !     }
> !   else
> !     print_values = PRINT_NO_VALUES;
>   
>     /* Check if the parameter is a "*" which means that we want
>        to update all variables */

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

[Nick Roberts]

 > > I would like to commit this (source code) patch from last month. I
 > > accidently reverted some of changes last time I presented it so I attach
 > > it here in its proper form.
 > > 
 > > As Andrew doesn't appear to around is there any chance a global maintainer
 > > can look at it? Its pretty lightweight.
 > 
 > Hi Nick,
 > 
 > I have some spare time waiting for my patches to get reviewed, so I
 > figure'd I'd look at yours. If you care, I have just a few comments.

With all the MI maintainers off the radar screen its all a bit hypothetical
but please do comment.


Nick

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

[Nick Roberts]

 > I have some spare time waiting for my patches to get reviewed, so I
 > figure'd I'd look at yours. If you care, I have just a few comments.

Sorry, Bob. I read this too quickly the first time and (stupidly) thought
that you had forgot to include the comments.

 > > + const char novalues[] = "\"--no-values\"";
 > > + const char withvalues[] = "\"--with-values\"";
 > > + const char simplevalues[] = "\"--simple-values\"";
 > > + const char allvalues[] = "\"--all-values\"";
 >
 > These could be made static.

Not really. I use them in my patch for mi-cmd-stack.c which I included in my
earlier submission (Sun, 27 Feb 2005 14:18:11 +1300) but left out on (Mar 19)
as it was unchanged.

 ...
 > > !       if (strcmp (argv[0], "0") == 0
 > > ! 	  || strcmp (argv[0], "--no-values") == 0)
 > > ! 	print_values = PRINT_NO_VALUES;
 > > !       else if (strcmp (argv[0], "1") == 0
 > > ! 	       || strcmp (argv[0], "--with-values") == 0)
 > 
 > instead of using "--no-values" and "--with-values" you could use the
 > variable "novalues" and "withvalues".

Yes. That would make sense.

 > > !   if (argc == 2)
 > > !     {
 > > !       if (strcmp (argv[0], "0") == 0
 > > ! 	  || strcmp (argv[0], "--no-values") == 0)
 > > ! 	print_values = PRINT_NO_VALUES;
 > > !       else if (strcmp (argv[0], "1") == 0
 > > ! 	       || strcmp (argv[0], "--with-values") == 0)
 > 
 > same as above with the --no-values and --with-values

Yes.

I will send the revised source (all files apart from doco which Eli has
already approved) to Daniel and cc gdb-patches.

Nick

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

[Daniel Jacobowitz]

On Mon, May 02, 2005 at 02:06:57PM +1200, Nick Roberts wrote:
>  > I have some spare time waiting for my patches to get reviewed, so I
>  > figure'd I'd look at yours. If you care, I have just a few comments.
> 
> Sorry, Bob. I read this too quickly the first time and (stupidly) thought
> that you had forgot to include the comments.
> 
>  > > + const char novalues[] = "\"--no-values\"";
>  > > + const char withvalues[] = "\"--with-values\"";
>  > > + const char simplevalues[] = "\"--simple-values\"";
>  > > + const char allvalues[] = "\"--all-values\"";
>  >
>  > These could be made static.
> 
> Not really. I use them in my patch for mi-cmd-stack.c which I included in my
> earlier submission (Sun, 27 Feb 2005 14:18:11 +1300) but left out on (Mar 19)
> as it was unchanged.

If you want these to be global variables, you need to give them better
names.  Also, the GDB coding style means that multiple words
get_underscores_for_separation.

>  ...
>  > > !       if (strcmp (argv[0], "0") == 0
>  > > ! 	  || strcmp (argv[0], "--no-values") == 0)
>  > > ! 	print_values = PRINT_NO_VALUES;
>  > > !       else if (strcmp (argv[0], "1") == 0
>  > > ! 	       || strcmp (argv[0], "--with-values") == 0)
>  > 
>  > instead of using "--no-values" and "--with-values" you could use the
>  > variable "novalues" and "withvalues".
> 
> Yes. That would make sense.

You'll need to strip the extra quote marks off those variables then -
which seems more natural anyway.

> I will send the revised source (all files apart from doco which Eli has
> already approved) to Daniel and cc gdb-patches.

Please include the documentation with the patch anyway; a complete
patch lets reviewers see the whole picture more easily.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

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

[Nick Roberts]

 > >  > > + const char novalues[] = "\"--no-values\"";
 > >  > > + const char withvalues[] = "\"--with-values\"";
 > >  > > + const char simplevalues[] = "\"--simple-values\"";
 > >  > > + const char allvalues[] = "\"--all-values\"";
 > >  >
 > >  > These could be made static.
 > > 
 > > Not really. I use them in my patch for mi-cmd-stack.c which I included in
 > > my earlier submission (Sun, 27 Feb 2005 14:18:11 +1300) but left out on
 > > (Mar 19) as it was unchanged.
 > 
 > If you want these to be global variables, you need to give them better
 > names.  Also, the GDB coding style means that multiple words
 > get_underscores_for_separation.

I've revised the last patch accordingly. (I sent it before I read this e-mail).

 > Please include the documentation with the patch anyway; a complete
 > patch lets reviewers see the whole picture more easily.

All attached. Please note that the subject title refers to the "other" patch.

Nick

2005-05-02  Nick Roberts  <nickrob@snap.net.nz>

	* mi/mi-cmds.h: Add extern declarations for string constants for
	MI command options.

	* mi/mi-cmd-var.c: Define string constants for MI command options.
	(mi_cmd_var_list_children): : Use string constants instead of
	literals for MI command options.
	(mi_cmd_var_update): New option --with-values for -var-update.
	(varobj_update_one): Add argument for new option.

	* mi/mi-cmd-stack.c (mi_cmd_stack_list_locals): Use string
	constants instead of literals for MI command options.

*** /home/nick/src/gdb/mi/mi-cmds.h.~1.15.~	2005-01-25 22:30:39.000000000 +1300
--- /home/nick/src/gdb/mi/mi-cmds.h	2005-05-02 16:51:37.000000000 +1200
***************
*** 50,55 ****
--- 50,60 ----
     PRINT_SIMPLE_VALUES
  };
  
+ extern const char mi_no_values[];
+ extern const char mi_with_values[];
+ extern const char mi_simple_values[];
+ extern const char mi_all_values[];
+ 
  typedef enum mi_cmd_result (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
  
  /* Older MI commands have this interface. Retained until all old


*** /home/nick/src/gdb/mi/mi-cmd-var.c.~1.21.~	2005-02-13 00:36:20.000000000 +1300
--- /home/nick/src/gdb/mi/mi-cmd-var.c	2005-05-02 16:54:02.000000000 +1200
***************
*** 30,38 ****
  #include <ctype.h>
  #include "gdb_string.h"
  
  extern int varobjdebug;		/* defined in varobj.c */
  
! static int varobj_update_one (struct varobj *var);
  
  /* VAROBJ operations */
  
--- 30,44 ----
  #include <ctype.h>
  #include "gdb_string.h"
  
+ const char mi_no_values[] = "--no-values";
+ const char mi_with_values[] = "--with-values";
+ const char mi_simple_values[] = "--simple-values";
+ const char mi_all_values[] = "--all-values";
+ 
  extern int varobjdebug;		/* defined in varobj.c */
  
! static int varobj_update_one (struct varobj *var,
! 			      enum print_values print_values);
  
  /* VAROBJ operations */
  
***************
*** 262,284 ****
      error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
  
    /* Get varobj handle, if a valid var obj name was specified */
!   if (argc == 1) var = varobj_get_handle (argv[0]);
!   else var = varobj_get_handle (argv[1]);
    if (var == NULL)
      error (_("Variable object not found"));
  
    numchild = varobj_list_children (var, &childlist);
    ui_out_field_int (uiout, "numchild", numchild);
    if (argc == 2)
!     if (strcmp (argv[0], "0") == 0
! 	|| strcmp (argv[0], "--no-values") == 0)
!       print_values = PRINT_NO_VALUES;
!     else if (strcmp (argv[0], "1") == 0
! 	     || strcmp (argv[0], "--all-values") == 0)
!       print_values = PRINT_ALL_VALUES;
!     else
!      error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\""));
!   else print_values = PRINT_NO_VALUES;
  
    if (numchild <= 0)
      return MI_CMD_DONE;
--- 268,296 ----
      error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
  
    /* Get varobj handle, if a valid var obj name was specified */
!   if (argc == 1)
!     var = varobj_get_handle (argv[0]);
!   else
!     var = varobj_get_handle (argv[1]);
    if (var == NULL)
      error (_("Variable object not found"));
  
    numchild = varobj_list_children (var, &childlist);
    ui_out_field_int (uiout, "numchild", numchild);
    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;
  
    if (numchild <= 0)
      return MI_CMD_DONE;
***************
*** 426,436 ****
    struct cleanup *cleanup;
    char *name;
    int nv;
  
!   if (argc != 1)
!     error (_("mi_cmd_var_update: Usage: NAME."));
  
!   name = argv[0];
  
    /* Check if the parameter is a "*" which means that we want
       to update all variables */
--- 438,467 ----
    struct cleanup *cleanup;
    char *name;
    int nv;
+   enum print_values print_values;
  
!   if (argc != 1 && argc != 2)
!     error (_("mi_cmd_var_update: Usage: [PRINT_VALUES] NAME."));
  
!   if (argc == 1)
!     name = argv[0];
!   else
!     name = (argv[1]);
! 
!   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 */
***************
*** 450,456 ****
        cr = rootlist;
        while (*cr != NULL)
  	{
! 	  varobj_update_one (*cr);
  	  cr++;
  	}
        xfree (rootlist);
--- 481,487 ----
        cr = rootlist;
        while (*cr != NULL)
  	{
! 	  varobj_update_one (*cr, print_values);
  	  cr++;
  	}
        xfree (rootlist);
***************
*** 467,473 ****
          cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
        else
          cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
!       varobj_update_one (var);
        do_cleanups (cleanup);
      }
      return MI_CMD_DONE;
--- 498,504 ----
          cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
        else
          cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
!       varobj_update_one (var, print_values);
        do_cleanups (cleanup);
      }
      return MI_CMD_DONE;
***************
*** 478,484 ****
     scope), and 1 if it succeeds. */
  
  static int
! varobj_update_one (struct varobj *var)
  {
    struct varobj **changelist;
    struct varobj **cc;
--- 509,515 ----
     scope), and 1 if it succeeds. */
  
  static int
! varobj_update_one (struct varobj *var, enum print_values print_values)
  {
    struct varobj **changelist;
    struct varobj **cc;
***************
*** 524,529 ****
--- 555,562 ----
  	  if (mi_version (uiout) > 1)
  	    cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
  	  ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
+ 	  if (print_values)
+ 	    ui_out_field_string (uiout, "value", varobj_get_value (*cc));
  	  ui_out_field_string (uiout, "in_scope", "true");
  	  ui_out_field_string (uiout, "type_changed", "false");
  	  if (mi_version (uiout) > 1)


*** /home/nick/src/gdb/mi/mi-cmd-stack.c.~1.25.~	2005-02-13 00:36:20.000000000 +1300
--- /home/nick/src/gdb/mi/mi-cmd-stack.c	2005-05-02 16:55:27.000000000 +1200
***************
*** 142,157 ****
     frame = get_selected_frame (NULL);
  
     if (strcmp (argv[0], "0") == 0
!        || strcmp (argv[0], "--no-values") == 0)
       print_values = PRINT_NO_VALUES;
     else if (strcmp (argv[0], "1") == 0
! 	    || strcmp (argv[0], "--all-values") == 0)
       print_values = PRINT_ALL_VALUES;
     else if (strcmp (argv[0], "2") == 0
! 	    || strcmp (argv[0], "--simple-values") == 0)
       print_values = PRINT_SIMPLE_VALUES;
     else
!      error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\", 2 or \"--simple-values\""));
    list_args_or_locals (1, print_values, frame);
    return MI_CMD_DONE;
  }
--- 142,159 ----
     frame = get_selected_frame (NULL);
  
     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_all_values) == 0)
       print_values = PRINT_ALL_VALUES;
     else if (strcmp (argv[0], "2") == 0
! 	    || strcmp (argv[0], mi_simple_values) == 0)
       print_values = PRINT_SIMPLE_VALUES;
     else
!      error (_("Unknown value for PRINT_VALUES: must be: \
! 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
! 	    mi_no_values, mi_all_values, mi_simple_values);
    list_args_or_locals (1, print_values, frame);
    return MI_CMD_DONE;
  }


*** /home/nick/src/gdb/doc/gdb.texinfo.~1.249.~	2005-05-02 14:28:29.000000000 +1200
--- /home/nick/src/gdb/doc/gdb.texinfo	2005-05-02 19:12:12.000000000 +1200
***************
*** 20331,20336 ****
--- 20331,20342 ----
  variables.  With an optional preceding argument of 1 or @code{--all-values},
  also prints their values.
  
+ Returns a list of the children of the specified variable object.  With
+ just the variable object name as an argument or with an optional
+ preceding argument of 0 or @code{--no-values}, prints only the names
+ of the variables.  With an optional value for @var{print-values} of 1
+ or @code{--with-values}, also prints their values.
+ 
  @subsubheading Example
  
  @smallexample
***************
*** 20451,20463 ****
  @subsubheading Synopsis
  
  @smallexample
!  -var-update @{@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.
  
  
  @node Annotations
  @chapter @value{GDBN} Annotations
--- 20457,20485 ----
  @subsubheading Synopsis
  
  @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 value for
! @var{print-values} of 0 or @code{--no-values}, prints only the names
! of the variables.  A value for @var{print-values} of 1 or
! @code{--with-values}, also prints their values.
! 
! @subsubheading Example
  
+ @smallexample
+ (@value{GDBP})
+ -var-assign var1 3
+ ^done,value="3"
+ (@value{GDBP})
+ -var-update --all-values var1
+ ^done,changelist=[@{name="var1",value="3",in_scope="true",
+ type_changed="false"@}]
+ (@value{GDBP})
+ @end smallexample
  
  @node Annotations
  @chapter @value{GDBN} Annotations

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

[Daniel Jacobowitz]

This is basically OK.  A couple of things:

On Mon, May 02, 2005 at 07:23:50PM +1200, Nick Roberts wrote:
> 2005-05-02  Nick Roberts  <nickrob@snap.net.nz>
> 
> 	* mi/mi-cmds.h: Add extern declarations for string constants for
> 	MI command options.
> 
> 	* mi/mi-cmd-var.c: Define string constants for MI command options.
> 	(mi_cmd_var_list_children): : Use string constants instead of

Stray colon there.

> 	literals for MI command options.
> 	(mi_cmd_var_update): New option --with-values for -var-update.
> 	(varobj_update_one): Add argument for new option.
> 
> 	* mi/mi-cmd-stack.c (mi_cmd_stack_list_locals): Use string
> 	constants instead of literals for MI command options.

> + extern const char mi_no_values[];
> + extern const char mi_with_values[];
> + extern const char mi_simple_values[];
> + extern const char mi_all_values[];

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

> --- 268,296 ----
>       error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
>   
>     /* Get varobj handle, if a valid var obj name was specified */
> !   if (argc == 1)
> !     var = varobj_get_handle (argv[0]);
> !   else
> !     var = varobj_get_handle (argv[1]);
>     if (var == NULL)
>       error (_("Variable object not found"));
>   
>     numchild = varobj_list_children (var, &childlist);
>     ui_out_field_int (uiout, "numchild", numchild);
>     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;
>   
>     if (numchild <= 0)
>       return MI_CMD_DONE;

Andrew's right that it would be nice to use mi_getopt here, but we
can't as-is; it doesn't do long options.  That'd be good to fix
someday.

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.

> !   if (argc == 1)
> !     name = argv[0];
> !   else
> !     name = (argv[1]);

Stray parentheses.

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

> *** /home/nick/src/gdb/doc/gdb.texinfo.~1.249.~	2005-05-02 14:28:29.000000000 +1200
> --- /home/nick/src/gdb/doc/gdb.texinfo	2005-05-02 19:12:12.000000000 +1200
> ***************
> *** 20331,20336 ****
> --- 20331,20342 ----
>   variables.  With an optional preceding argument of 1 or @code{--all-values},
>   also prints their values.
>   
> + Returns a list of the children of the specified variable object.  With
> + just the variable object name as an argument or with an optional
> + preceding argument of 0 or @code{--no-values}, prints only the names
> + of the variables.  With an optional value for @var{print-values} of 1
> + or @code{--with-values}, also prints their values.
> + 
>   @subsubheading Example
>   
>   @smallexample

This paragraph must be in the wrong place.  It's mostly duplicating the
paragraph above it.

> ***************
> *** 20451,20463 ****
>   @subsubheading Synopsis
>   
>   @smallexample
> !  -var-update @{@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.
>   
>   
>   @node Annotations
>   @chapter @value{GDBN} Annotations
> --- 20457,20485 ----
>   @subsubheading Synopsis
>   
>   @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 value for
> ! @var{print-values} of 0 or @code{--no-values}, prints only the names
> ! of the variables.  A value for @var{print-values} of 1 or
> ! @code{--with-values}, also prints their values.

I think the comma probably shouldn't be there.

> ! @subsubheading Example
>   
> + @smallexample
> + (@value{GDBP})
> + -var-assign var1 3
> + ^done,value="3"
> + (@value{GDBP})
> + -var-update --all-values var1
> + ^done,changelist=[@{name="var1",value="3",in_scope="true",
> + type_changed="false"@}]
> + (@value{GDBP})
> + @end smallexample
>   
>   @node Annotations
>   @chapter @value{GDBN} Annotations
> 

-- 
Daniel Jacobowitz
CodeSourcery, LLC

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

[Eli Zaretskii]

> Date: Thu, 16 Jun 2005 23:43:29 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Bob Rossi <bob@brasko.net>, gdb-patches@sources.redhat.com
> 
> I don't remember the entire outcome of your discussion with Eli

My opinion (which Nick eventually accepted, IIRC) is here:

   http://sourceware.org/ml/gdb-patches/2005-02/msg00204.html

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

If you have suggestions for better names for these options, please say
what they are.  Alternatively, if you are saying that the manual patch
doesn't explain them well enough, please point out the unclear text.

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

[Nick Roberts]

 > 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

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

[Daniel Jacobowitz]

On Fri, Jun 17, 2005 at 01:08:58PM +0200, Eli Zaretskii wrote:
> > Date: Thu, 16 Jun 2005 23:43:29 -0400
> > From: Daniel Jacobowitz <drow@false.org>
> > Cc: Bob Rossi <bob@brasko.net>, gdb-patches@sources.redhat.com
> > 
> > I don't remember the entire outcome of your discussion with Eli
> 
> My opinion (which Nick eventually accepted, IIRC) is here:
> 
>    http://sourceware.org/ml/gdb-patches/2005-02/msg00204.html
> 
> > 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.
> 
> If you have suggestions for better names for these options, please say
> what they are.  Alternatively, if you are saying that the manual patch
> doesn't explain them well enough, please point out the unclear text.

The latest manual patch Nick posted was so mangled that I couldn't work
out what the new text was.

Before the patch -stack-list-locals takes --no-values, --all-values, and
--simple-values.  -var-list-children takes --no-values and
--all-values.

After the patch -stack-list-locals takes --no-values, --all-values, and
--simple-values.  -var-list-children takes --no-values and
--with-values.  -var-update takes --no-values and --with-values.

So, we still have --all-values, we've introduced --with-values, and
we've made an incompatible change.  I think that, other than the
incompatible change, I don't have a big problem with this - they are
different commands, they can take different options.  But consistency
would be nice.

I do see that you OK'd the incompatible change:
  http://sourceware.org/ml/gdb-patches/2005-02/msg00232.html

I'm less comfortable with that than you and Nick are; we shipped GDB
6.3 with -var-list-children --all-values, and it's even in the manual.

Your original objection was:
> 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".

Could you elaborate?  I think that --all-values is a reasonable option;
especially since --simple-values would be a reasonable extension here
also.  It causes the values for objects other than
structs/arrays/unions to be printed.

Would you be happier with -var-update --all-values if -var-update
--simple-values also worked?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

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

[Daniel Jacobowitz]

On Fri, Jun 17, 2005 at 11:42:52PM +1200, Nick Roberts wrote:
>  > You've replaced "--all-values" in the source with "--with-values" here. 
>  > Surely that's a bug?

You don't really seem to answer this.  -stack-list-locals today accepts
--all-values and does not accept --with-values.  It has for a year and
a half, so it was in a released version of GDB.  Why're you removing
that?

>  > > !   if (argc == 1)
>  > > !     name = argv[0];
>  > > !   else
>  > > !     name = (argv[1]);
>  > 
>  > Stray parentheses.
> 
> I don't follow.

name = argv[1];

not:

name = (argv[1]);

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

I don't see why the presence of the 0/1 make any difference to the
--with-values/--all-values question.  But if no one is already using
the 0/1 syntax, let's not introduce new uses of it; the existing uses
can stay, but we don't need more.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

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

[Nick Roberts]

Daniel Jacobowitz writes:
 > On Fri, Jun 17, 2005 at 11:42:52PM +1200, Nick Roberts wrote:
 > >  > You've replaced "--all-values" in the source with "--with-values" here. 
 > >  > Surely that's a bug?
 > 
 > You don't really seem to answer this.  -stack-list-locals today accepts
 > --all-values and does not accept --with-values.  It has for a year and
 > a half, so it was in a released version of GDB.  Why're you removing
 > that?

Eli asked me to use --with-values for -var-update.  Since, after the change,
-stack-list-locals would have the same options, I thought they should have
the same names. FWIW my preference is the same as yours: to use --all-values
throughout.

 > name = argv[1];
 > 
 > not:
 > 
 > name = (argv[1]);

OK

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

 > I don't see why the presence of the 0/1 make any difference to the
 > --with-values/--all-values question.  But if no one is already using
 > the 0/1 syntax, let's not introduce new uses of it; the existing uses
 > can stay, but we don't need more.

-var-list-children --all-values VARNUM      works with 6.3.

-var-list-children --with-values VARNUM     would work with 6.4 if these
                                            changes are installed.

-var-list-children 1 VARNUM                 would work with 6.3 and 6.4 if
                                            these changes are installed.


Nick

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

[Eli Zaretskii]

> Date: Fri, 17 Jun 2005 10:04:10 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Nick Roberts <nickrob@snap.net.nz>, Bob Rossi <bob@brasko.net>,
> 	gdb-patches@sources.redhat.com
> 
> I think that, other than the
> incompatible change, I don't have a big problem with this - they are
> different commands, they can take different options.  But consistency
> would be nice.

It is not always possible to be consistent when the sets of options
are different.  But we could keep "--all-values" in the case of
"-var-list-children" for backward compatibility.

> I do see that you OK'd the incompatible change:
>   http://sourceware.org/ml/gdb-patches/2005-02/msg00232.html

That was for consistency's sake ;-)

> I'm less comfortable with that than you and Nick are; we shipped GDB
> 6.3 with -var-list-children --all-values, and it's even in the manual.
> http://sourceware.org/ml/gdb-patches/2005-02/msg00232.html
> Your original objection was:
> > 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".
> 
> Could you elaborate?  I think that --all-values is a reasonable option;

Elaborate? let me try; here are some opposites I can think of:

   with-values       without-values
   values=yes	     values=no
   print-values	     don't-print-values
   all-values	     some-values

"all" simply doesn't sound as an opposite of "no" or "none".

> especially since --simple-values would be a reasonable extension here
> also.

If we extend the -var-* commands like that, I wouldn't object to using
"--all-values" in them.

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

[Daniel Jacobowitz]

Nick, I apologize for the delay, I was trying to keep this one
moving... here goes again.

On Sat, Jun 18, 2005 at 11:53:30AM +0200, Eli Zaretskii wrote:
> "all" simply doesn't sound as an opposite of "no" or "none".
> 
> > especially since --simple-values would be a reasonable extension here
> > also.
> 
> If we extend the -var-* commands like that, I wouldn't object to using
> "--all-values" in them.

Personally, I think --no-values and --all-values are a reasonable pair. 
But in any case, we all seem agreed that --no-values, --simple-values,
and --all-values are a reasonable triplet.

Nick, here's a patch based on yours which adds -var-list-children
--simple-values and -var-update --simple-values/--all-values.  I like
it; I think --simple-values is useful (since for anything other than
simple values, an IDE is likely to want to print each member
individually...).

I didn't revise the documentation because your last posted patch didn't
include the current manual diff.  I also didn't write any testcases. 
Both of these need to be done before the patch goes in.  Tested on
i686-pc-linux-gnu, both the testsuite and by hand for -var-update.

No incompatible changes, option consistency, and behavior consistency. 
I don't think I can do any better than this :-)  Nick, Eli, are you
both OK with this version of the code changes?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-07-03  Nick Roberts  <nickrob@snap.net.nz>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* mi/mi-cmds.h (mi_no_values, mi_simple_values, mi_all_values): New
	declarations.
	* mi/mi-cmd-stack.c (mi_cmd_stack_list_locals): Use string
	constants instead of literals for MI command options.
        * mi/mi-cmd-var.c (mi_no_values, mi_simple_values, mi_all_values):
	New variables.
	(mi_parse_values_option, mi_print_value_p): New functions.
        (mi_cmd_var_list_children): Use mi_parse_values_option and
	mi_print_value_p.
        (mi_cmd_var_update): Support a PRINT_VALUES option.  Update calls
	to varobj_update_one.
        (varobj_update_one): Take a print_values argument.  Call
	mi_print_value_p.

Index: mi/mi-cmd-stack.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-stack.c,v
retrieving revision 1.27
diff -u -p -r1.27 mi-cmd-stack.c
--- mi/mi-cmd-stack.c	19 Jun 2005 03:11:47 -0000	1.27
+++ mi/mi-cmd-stack.c	3 Jul 2005 19:51:35 -0000
@@ -136,16 +136,18 @@ mi_cmd_stack_list_locals (char *command,
    frame = get_selected_frame (NULL);
 
    if (strcmp (argv[0], "0") == 0
-       || strcmp (argv[0], "--no-values") == 0)
+       || strcmp (argv[0], mi_no_values) == 0)
      print_values = PRINT_NO_VALUES;
    else if (strcmp (argv[0], "1") == 0
-	    || strcmp (argv[0], "--all-values") == 0)
+	    || strcmp (argv[0], mi_all_values) == 0)
      print_values = PRINT_ALL_VALUES;
    else if (strcmp (argv[0], "2") == 0
-	    || strcmp (argv[0], "--simple-values") == 0)
+	    || strcmp (argv[0], mi_simple_values) == 0)
      print_values = PRINT_SIMPLE_VALUES;
    else
-     error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\", 2 or \"--simple-values\""));
+     error (_("Unknown value for PRINT_VALUES: must be: \
+0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
+	    mi_no_values, mi_all_values, mi_simple_values);
   list_args_or_locals (1, print_values, frame);
   return MI_CMD_DONE;
 }
Index: mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.21
diff -u -p -r1.21 mi-cmd-var.c
--- mi/mi-cmd-var.c	11 Feb 2005 04:06:11 -0000	1.21
+++ mi/mi-cmd-var.c	3 Jul 2005 19:51:35 -0000
@@ -30,9 +30,14 @@
 #include <ctype.h>
 #include "gdb_string.h"
 
+const char mi_no_values[] = "--no-values";
+const char mi_simple_values[] = "--simple-values";
+const char mi_all_values[] = "--all-values";
+
 extern int varobjdebug;		/* defined in varobj.c */
 
-static int varobj_update_one (struct varobj *var);
+static int varobj_update_one (struct varobj *var,
+			      enum print_values print_values);
 
 /* VAROBJ operations */
 
@@ -247,6 +252,49 @@ mi_cmd_var_info_num_children (char *comm
   return MI_CMD_DONE;
 }
 
+/* Parse a string argument into a print_values value.  */
+
+static enum print_values
+mi_parse_values_option (const char *arg)
+{
+  if (strcmp (arg, "0") == 0
+      || strcmp (arg, mi_no_values) == 0)
+    return PRINT_NO_VALUES;
+  else if (strcmp (arg, "1") == 0
+	   || strcmp (arg, mi_all_values) == 0)
+    return PRINT_ALL_VALUES;
+  else if (strcmp (arg, "2") == 0
+	   || strcmp (arg, mi_simple_values) == 0)
+    return PRINT_SIMPLE_VALUES;
+  else
+    error (_("Unknown value for PRINT_VALUES\n\
+Must be: 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
+	   mi_no_values, mi_simple_values, mi_all_values);
+}
+
+/* Return 1 if given the argument PRINT_VALUES we should display
+   a value of type TYPE.  */
+
+static int
+mi_print_value_p (struct type *type, enum print_values print_values)
+{
+  if (print_values == PRINT_NO_VALUES)
+    return 0;
+
+  if (print_values == PRINT_ALL_VALUES)
+    return 1;
+
+  /* For PRINT_SIMPLE_VALUES, only print the value if it has a type
+     and that type is not a compound type.  */
+
+  if (type == NULL)
+    return 0;
+  type = check_typedef (type);
+  return (TYPE_CODE (type) != TYPE_CODE_ARRAY
+	  && TYPE_CODE (type) != TYPE_CODE_STRUCT
+	  && TYPE_CODE (type) != TYPE_CODE_UNION);
+}
+
 enum mi_cmd_result
 mi_cmd_var_list_children (char *command, char **argv, int argc)
 {
@@ -258,27 +306,23 @@ mi_cmd_var_list_children (char *command,
   char *type;
   enum print_values print_values;
 
-  if (argc != 1 && argc != 2)
+  if (argc > 2)
     error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
 
   /* Get varobj handle, if a valid var obj name was specified */
-  if (argc == 1) var = varobj_get_handle (argv[0]);
-  else var = varobj_get_handle (argv[1]);
+  if (argc == 1)
+    var = varobj_get_handle (argv[0]);
+  else
+    var = varobj_get_handle (argv[1]);
   if (var == NULL)
     error (_("Variable object not found"));
 
   numchild = varobj_list_children (var, &childlist);
   ui_out_field_int (uiout, "numchild", numchild);
   if (argc == 2)
-    if (strcmp (argv[0], "0") == 0
-	|| strcmp (argv[0], "--no-values") == 0)
-      print_values = PRINT_NO_VALUES;
-    else if (strcmp (argv[0], "1") == 0
-	     || strcmp (argv[0], "--all-values") == 0)
-      print_values = PRINT_ALL_VALUES;
-    else
-     error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\""));
-  else print_values = PRINT_NO_VALUES;
+    print_values = mi_parse_values_option (argv[0]);
+  else
+    print_values = PRINT_NO_VALUES;
 
   if (numchild <= 0)
     return MI_CMD_DONE;
@@ -295,9 +339,9 @@ mi_cmd_var_list_children (char *command,
       ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
       ui_out_field_string (uiout, "exp", varobj_get_expression (*cc));
       ui_out_field_int (uiout, "numchild", varobj_get_num_children (*cc));
-      if (print_values)
-	ui_out_field_string (uiout, "value", varobj_get_value (*cc));
       type = varobj_get_type (*cc);
+      if (mi_print_value_p (type, print_values))
+	ui_out_field_string (uiout, "value", varobj_get_value (*cc));
       /* C++ pseudo-variables (public, private, protected) do not have a type */
       if (type)
 	ui_out_field_string (uiout, "type", varobj_get_type (*cc));
@@ -426,11 +470,20 @@ mi_cmd_var_update (char *command, char *
   struct cleanup *cleanup;
   char *name;
   int nv;
+  enum print_values print_values;
 
-  if (argc != 1)
-    error (_("mi_cmd_var_update: Usage: NAME."));
+  if (argc != 1 && argc != 2)
+    error (_("mi_cmd_var_update: Usage: [PRINT_VALUES] NAME."));
 
-  name = argv[0];
+  if (argc == 1)
+    name = argv[0];
+  else
+    name = (argv[1]);
+
+  if (argc == 2)
+    print_values = mi_parse_values_option (argv[0]);
+  else
+    print_values = PRINT_NO_VALUES;
 
   /* Check if the parameter is a "*" which means that we want
      to update all variables */
@@ -450,7 +503,7 @@ mi_cmd_var_update (char *command, char *
       cr = rootlist;
       while (*cr != NULL)
 	{
-	  varobj_update_one (*cr);
+	  varobj_update_one (*cr, print_values);
 	  cr++;
 	}
       xfree (rootlist);
@@ -467,7 +520,7 @@ mi_cmd_var_update (char *command, char *
         cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
       else
         cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
-      varobj_update_one (var);
+      varobj_update_one (var, print_values);
       do_cleanups (cleanup);
     }
     return MI_CMD_DONE;
@@ -478,7 +531,7 @@ mi_cmd_var_update (char *command, char *
    scope), and 1 if it succeeds. */
 
 static int
-varobj_update_one (struct varobj *var)
+varobj_update_one (struct varobj *var, enum print_values print_values)
 {
   struct varobj **changelist;
   struct varobj **cc;
@@ -524,6 +577,8 @@ varobj_update_one (struct varobj *var)
 	  if (mi_version (uiout) > 1)
 	    cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 	  ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
+	  if (mi_print_value_p (varobj_get_type (*cc), print_values))
+	    ui_out_field_string (uiout, "value", varobj_get_value (*cc));
 	  ui_out_field_string (uiout, "in_scope", "true");
 	  ui_out_field_string (uiout, "type_changed", "false");
 	  if (mi_version (uiout) > 1)
Index: mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.16
diff -u -p -r1.16 mi-cmds.h
--- mi/mi-cmds.h	19 Jun 2005 03:12:15 -0000	1.16
+++ mi/mi-cmds.h	3 Jul 2005 19:51:35 -0000
@@ -50,6 +50,10 @@ enum print_values {
    PRINT_SIMPLE_VALUES
 };
 
+extern const char mi_no_values[];
+extern const char mi_simple_values[];
+extern const char mi_all_values[];
+
 typedef enum mi_cmd_result (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
 
 /* Older MI commands have this interface. Retained until all old

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

[Nick Roberts]

Daniel Jacobowitz writes:
 ...
 > Nick, here's a patch based on yours which adds -var-list-children
 > --simple-values and -var-update --simple-values/--all-values.  I like
 > it; I think --simple-values is useful (since for anything other than
 > simple values, an IDE is likely to want to print each member
 > individually...).
 > 
 > I didn't revise the documentation because your last posted patch didn't
 > include the current manual diff.  I also didn't write any testcases. 
 > Both of these need to be done before the patch goes in.  Tested on
 > i686-pc-linux-gnu, both the testsuite and by hand for -var-update.
 > 
 > No incompatible changes, option consistency, and behavior consistency. 
 > I don't think I can do any better than this :-)  Nick, Eli, are you
 > both OK with this version of the code changes?

Yes, this looks good.  I have tested it with my current version of gdb-mi.el
and it seems to work.  I will try to dig out the relevant patch for the
documentation and rework it, if Eli is also agreeable to this revision.

 ...
 > -  if (argc != 1 && argc != 2)
 > +  if (argc > 2)

This gives:

(gdb)
-var-list-children
&"Variable object not found\n"
^error,msg="Variable object not found"
(gdb) 

instead of:

(gdb)
-var-list-children
&"mi_cmd_var_list_children: Usage: NAME.\n"
^error,msg="mi_cmd_var_list_children: Usage: NAME."
(gdb) 

I don't know if that's what you intended.

Nick

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

[Daniel Jacobowitz]

On Mon, Jul 04, 2005 at 03:08:53PM +1200, Nick Roberts wrote:
> Daniel Jacobowitz writes:
>  ...
>  > Nick, here's a patch based on yours which adds -var-list-children
>  > --simple-values and -var-update --simple-values/--all-values.  I like
>  > it; I think --simple-values is useful (since for anything other than
>  > simple values, an IDE is likely to want to print each member
>  > individually...).
>  > 
>  > I didn't revise the documentation because your last posted patch didn't
>  > include the current manual diff.  I also didn't write any testcases. 
>  > Both of these need to be done before the patch goes in.  Tested on
>  > i686-pc-linux-gnu, both the testsuite and by hand for -var-update.
>  > 
>  > No incompatible changes, option consistency, and behavior consistency. 
>  > I don't think I can do any better than this :-)  Nick, Eli, are you
>  > both OK with this version of the code changes?
> 
> Yes, this looks good.  I have tested it with my current version of gdb-mi.el
> and it seems to work.  I will try to dig out the relevant patch for the
> documentation and rework it, if Eli is also agreeable to this revision.

Thanks!  Once we've reached consensus on this, will your current gdb-mi.el
be compatible with FSF GDB again?

>  ...
>  > -  if (argc != 1 && argc != 2)
>  > +  if (argc > 2)
> 
> This gives:
> 
> (gdb)
> -var-list-children
> &"Variable object not found\n"
> ^error,msg="Variable object not found"
> (gdb) 
> 
> instead of:
> 
> (gdb)
> -var-list-children
> &"mi_cmd_var_list_children: Usage: NAME.\n"
> ^error,msg="mi_cmd_var_list_children: Usage: NAME."
> (gdb) 
> 
> I don't know if that's what you intended.

Blech.  That's what I get for trying to be clever.  No, the old version
was right.  I'll correct.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

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

[Eli Zaretskii]

> Date: Sun, 3 Jul 2005 15:56:30 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Nick Roberts <nickrob@snap.net.nz>, Bob Rossi <bob@brasko.net>,
> 	gdb-patches@sources.redhat.com
> 
> Personally, I think --no-values and --all-values are a reasonable pair. 
> But in any case, we all seem agreed that --no-values, --simple-values,
> and --all-values are a reasonable triplet.
> 
> Nick, here's a patch based on yours which adds -var-list-children
> --simple-values and -var-update --simple-values/--all-values.  I like
> it; I think --simple-values is useful (since for anything other than
> simple values, an IDE is likely to want to print each member
> individually...).
> 
> I didn't revise the documentation because your last posted patch didn't
> include the current manual diff.  I also didn't write any testcases. 
> Both of these need to be done before the patch goes in.  Tested on
> i686-pc-linux-gnu, both the testsuite and by hand for -var-update.
> 
> No incompatible changes, option consistency, and behavior consistency. 
> I don't think I can do any better than this :-)  Nick, Eli, are you
> both OK with this version of the code changes?

It's okay with me, but I hope we won't forget the doco changes to go
with these.

Thanks.

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

[Daniel Jacobowitz]

On Mon, Jul 04, 2005 at 07:55:13AM +0200, Eli Zaretskii wrote:
> It's okay with me, but I hope we won't forget the doco changes to go
> with these.

Absolutely!  Nick, here's a copy of the patch with the bug you found
stripped out of it.  Since you've got the current documentation, can
you update that and add some testcases, when you get a chance?  I'd
prefer to have all three go in together.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-07-03  Nick Roberts  <nickrob@snap.net.nz>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* mi/mi-cmds.h (mi_no_values, mi_simple_values, mi_all_values): New
	declarations.
	* mi/mi-cmd-stack.c (mi_cmd_stack_list_locals): Use string
	constants instead of literals for MI command options.
        * mi/mi-cmd-var.c (mi_no_values, mi_simple_values, mi_all_values):
	New variables.
	(mi_parse_values_option, mi_print_value_p): New functions.
        (mi_cmd_var_list_children): Use mi_parse_values_option and
	mi_print_value_p.
        (mi_cmd_var_update): Support a PRINT_VALUES option.  Update calls
	to varobj_update_one.
        (varobj_update_one): Take a print_values argument.  Call
	mi_print_value_p.

Index: mi/mi-cmd-stack.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-stack.c,v
retrieving revision 1.27
diff -u -p -r1.27 mi-cmd-stack.c
--- mi/mi-cmd-stack.c	19 Jun 2005 03:11:47 -0000	1.27
+++ mi/mi-cmd-stack.c	3 Jul 2005 19:51:35 -0000
@@ -136,16 +136,18 @@ mi_cmd_stack_list_locals (char *command,
    frame = get_selected_frame (NULL);
 
    if (strcmp (argv[0], "0") == 0
-       || strcmp (argv[0], "--no-values") == 0)
+       || strcmp (argv[0], mi_no_values) == 0)
      print_values = PRINT_NO_VALUES;
    else if (strcmp (argv[0], "1") == 0
-	    || strcmp (argv[0], "--all-values") == 0)
+	    || strcmp (argv[0], mi_all_values) == 0)
      print_values = PRINT_ALL_VALUES;
    else if (strcmp (argv[0], "2") == 0
-	    || strcmp (argv[0], "--simple-values") == 0)
+	    || strcmp (argv[0], mi_simple_values) == 0)
      print_values = PRINT_SIMPLE_VALUES;
    else
-     error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\", 2 or \"--simple-values\""));
+     error (_("Unknown value for PRINT_VALUES: must be: \
+0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
+	    mi_no_values, mi_all_values, mi_simple_values);
   list_args_or_locals (1, print_values, frame);
   return MI_CMD_DONE;
 }
Index: mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.21
diff -u -p -r1.21 mi-cmd-var.c
--- mi/mi-cmd-var.c	11 Feb 2005 04:06:11 -0000	1.21
+++ mi/mi-cmd-var.c	3 Jul 2005 19:51:35 -0000
@@ -30,9 +30,14 @@
 #include <ctype.h>
 #include "gdb_string.h"
 
+const char mi_no_values[] = "--no-values";
+const char mi_simple_values[] = "--simple-values";
+const char mi_all_values[] = "--all-values";
+
 extern int varobjdebug;		/* defined in varobj.c */
 
-static int varobj_update_one (struct varobj *var);
+static int varobj_update_one (struct varobj *var,
+			      enum print_values print_values);
 
 /* VAROBJ operations */
 
@@ -247,6 +252,49 @@ mi_cmd_var_info_num_children (char *comm
   return MI_CMD_DONE;
 }
 
+/* Parse a string argument into a print_values value.  */
+
+static enum print_values
+mi_parse_values_option (const char *arg)
+{
+  if (strcmp (arg, "0") == 0
+      || strcmp (arg, mi_no_values) == 0)
+    return PRINT_NO_VALUES;
+  else if (strcmp (arg, "1") == 0
+	   || strcmp (arg, mi_all_values) == 0)
+    return PRINT_ALL_VALUES;
+  else if (strcmp (arg, "2") == 0
+	   || strcmp (arg, mi_simple_values) == 0)
+    return PRINT_SIMPLE_VALUES;
+  else
+    error (_("Unknown value for PRINT_VALUES\n\
+Must be: 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
+	   mi_no_values, mi_simple_values, mi_all_values);
+}
+
+/* Return 1 if given the argument PRINT_VALUES we should display
+   a value of type TYPE.  */
+
+static int
+mi_print_value_p (struct type *type, enum print_values print_values)
+{
+  if (print_values == PRINT_NO_VALUES)
+    return 0;
+
+  if (print_values == PRINT_ALL_VALUES)
+    return 1;
+
+  /* For PRINT_SIMPLE_VALUES, only print the value if it has a type
+     and that type is not a compound type.  */
+
+  if (type == NULL)
+    return 0;
+  type = check_typedef (type);
+  return (TYPE_CODE (type) != TYPE_CODE_ARRAY
+	  && TYPE_CODE (type) != TYPE_CODE_STRUCT
+	  && TYPE_CODE (type) != TYPE_CODE_UNION);
+}
+
 enum mi_cmd_result
 mi_cmd_var_list_children (char *command, char **argv, int argc)
 {
@@ -262,23 +310,19 @@ mi_cmd_var_list_children (char *command,
     error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
 
   /* Get varobj handle, if a valid var obj name was specified */
-  if (argc == 1) var = varobj_get_handle (argv[0]);
-  else var = varobj_get_handle (argv[1]);
+  if (argc == 1)
+    var = varobj_get_handle (argv[0]);
+  else
+    var = varobj_get_handle (argv[1]);
   if (var == NULL)
     error (_("Variable object not found"));
 
   numchild = varobj_list_children (var, &childlist);
   ui_out_field_int (uiout, "numchild", numchild);
   if (argc == 2)
-    if (strcmp (argv[0], "0") == 0
-	|| strcmp (argv[0], "--no-values") == 0)
-      print_values = PRINT_NO_VALUES;
-    else if (strcmp (argv[0], "1") == 0
-	     || strcmp (argv[0], "--all-values") == 0)
-      print_values = PRINT_ALL_VALUES;
-    else
-     error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\""));
-  else print_values = PRINT_NO_VALUES;
+    print_values = mi_parse_values_option (argv[0]);
+  else
+    print_values = PRINT_NO_VALUES;
 
   if (numchild <= 0)
     return MI_CMD_DONE;
@@ -295,9 +339,9 @@ mi_cmd_var_list_children (char *command,
       ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
       ui_out_field_string (uiout, "exp", varobj_get_expression (*cc));
       ui_out_field_int (uiout, "numchild", varobj_get_num_children (*cc));
-      if (print_values)
-	ui_out_field_string (uiout, "value", varobj_get_value (*cc));
       type = varobj_get_type (*cc);
+      if (mi_print_value_p (type, print_values))
+	ui_out_field_string (uiout, "value", varobj_get_value (*cc));
       /* C++ pseudo-variables (public, private, protected) do not have a type */
       if (type)
 	ui_out_field_string (uiout, "type", varobj_get_type (*cc));
@@ -426,11 +470,20 @@ mi_cmd_var_update (char *command, char *
   struct cleanup *cleanup;
   char *name;
   int nv;
+  enum print_values print_values;
 
-  if (argc != 1)
-    error (_("mi_cmd_var_update: Usage: NAME."));
+  if (argc != 1 && argc != 2)
+    error (_("mi_cmd_var_update: Usage: [PRINT_VALUES] NAME."));
 
-  name = argv[0];
+  if (argc == 1)
+    name = argv[0];
+  else
+    name = (argv[1]);
+
+  if (argc == 2)
+    print_values = mi_parse_values_option (argv[0]);
+  else
+    print_values = PRINT_NO_VALUES;
 
   /* Check if the parameter is a "*" which means that we want
      to update all variables */
@@ -450,7 +503,7 @@ mi_cmd_var_update (char *command, char *
       cr = rootlist;
       while (*cr != NULL)
 	{
-	  varobj_update_one (*cr);
+	  varobj_update_one (*cr, print_values);
 	  cr++;
 	}
       xfree (rootlist);
@@ -467,7 +520,7 @@ mi_cmd_var_update (char *command, char *
         cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
       else
         cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
-      varobj_update_one (var);
+      varobj_update_one (var, print_values);
       do_cleanups (cleanup);
     }
     return MI_CMD_DONE;
@@ -478,7 +531,7 @@ mi_cmd_var_update (char *command, char *
    scope), and 1 if it succeeds. */
 
 static int
-varobj_update_one (struct varobj *var)
+varobj_update_one (struct varobj *var, enum print_values print_values)
 {
   struct varobj **changelist;
   struct varobj **cc;
@@ -524,6 +577,8 @@ varobj_update_one (struct varobj *var)
 	  if (mi_version (uiout) > 1)
 	    cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 	  ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
+	  if (mi_print_value_p (varobj_get_type (*cc), print_values))
+	    ui_out_field_string (uiout, "value", varobj_get_value (*cc));
 	  ui_out_field_string (uiout, "in_scope", "true");
 	  ui_out_field_string (uiout, "type_changed", "false");
 	  if (mi_version (uiout) > 1)
Index: mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.16
diff -u -p -r1.16 mi-cmds.h
--- mi/mi-cmds.h	19 Jun 2005 03:12:15 -0000	1.16
+++ mi/mi-cmds.h	3 Jul 2005 19:51:35 -0000
@@ -50,6 +50,10 @@ enum print_values {
    PRINT_SIMPLE_VALUES
 };
 
+extern const char mi_no_values[];
+extern const char mi_simple_values[];
+extern const char mi_all_values[];
+
 typedef enum mi_cmd_result (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
 
 /* Older MI commands have this interface. Retained until all old

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

[Nick Roberts]

 >        type = varobj_get_type (*cc);
 > +      if (mi_print_value_p (type, print_values))
 > +	ui_out_field_string (uiout, "value", varobj_get_value (*cc));

type here is char * (perhaps varobj_get_type should be renamed), but you're
treating it as struct type * in mi_print_value_p.  Using "--simple-values" can
give a segmentation fault.

Nick

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

[Nick Roberts]

 > It's okay with me, but I hope we won't forget the doco changes to go
 > with these.


I've changed the English slightly from last time, and because
"-var-list-children" and "-var-update" have the same optional argument, I only
describe it explictly once.  "-stack-list-locals" is slightly different
because print-values is not optional and "--print-simple-values" prints the
type also.

I've also corrected a spelling and made a couple of changes to the
GDB/MI command description format to reflect reality.

Nick


2005-07-05  Nick Roberts  <nickrob@snap.net.nz>

	* gdb.texinfo (GDB/MI Variable Objects): Describe print-values
	option for -var-list-children and -var-update.
	(GDB/MI Stack Manipulation): Simplify description of
	print-values option for -stack-list-locals.
	(GDB/MI Command Description Format): Clarify.
	(Mode Options): Spelling of superseded.


Index: gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.272
diff -u -p -r1.272 gdb.texinfo
--- gdb.texinfo	2 Jul 2005 15:39:47 -0000	1.272
+++ gdb.texinfo	4 Jul 2005 20:59:48 -0000
@@ -1075,7 +1075,7 @@ normal, level 1 is for use when @value{G
 @sc{gnu} Emacs, level 3 is the maximum annotation suitable for programs
 that control @value{GDBN}, and level 2 has been deprecated.
 
-The annotation mechanism has largely been superseeded by @sc{gdb/mi}
+The annotation mechanism has largely been superseded by @sc{gdb/mi}
 (@pxref{GDB/MI}).
 
 @item --args
@@ -17094,19 +17094,14 @@ For each command in the block, the follo
  -command @var{args}@dots{}
 @end smallexample
 
-@subsubheading @value{GDBN} Command
-
-The corresponding @value{GDBN} CLI command.
-
 @subsubheading Result
 
-@subsubheading Out-of-band
+@subsubheading @value{GDBN} Command
 
-@subsubheading Notes
+The corresponding @value{GDBN} CLI command(s), if any.
 
 @subsubheading Example
 
-
 @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 @node GDB/MI Breakpoint Table Commands
 @section @sc{gdb/mi} Breakpoint table commands
@@ -19491,15 +19486,15 @@ Show a single frame:
  -stack-list-locals @var{print-values}
 @end smallexample
 
-Display the local variable names for the current frame.  With an
-argument of 0 or @code{--no-values}, prints only the names of the variables.
-With argument of 1 or @code{--all-values}, prints also their values.  With
-argument of 2 or @code{--simple-values}, prints the name, type and value for
-simple data types and the name and type for arrays, structures and
-unions.  In this last case, the idea is that the user can see the
-value of simple data types immediately and he can create variable
-objects for other data types if he wishes to explore their values in
-more detail.
+Display the local variable names for the current frame.  A value for
+@var{print-values} of 0 or @code{--no-values}, prints only the names
+of the variables; a value of 1 or @code{--all-values}, prints also
+their values; and a value of 2 or @code{--simple-values}, prints the
+name, type and value for simple data types and the name and type for
+arrays, structures and unions.  In this last case, the idea is that
+the user can see the value of simple data types immediately and he can
+create variable objects for other data types if he wishes to explore
+their values in more detail.
 
 @subsubheading @value{GDBN} Command
 
@@ -20464,11 +20459,13 @@ Returns the number of children of a vari
  -var-list-children [@var{print-values}] @var{name}
 @end smallexample
 
-Returns a list of the children of the specified variable object.  With
-just the variable object name as an 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.
+Returns a list of the children of the specified variable object.  A
+single argument or an optional value for @var{print-values} of 0 or
+@code{--no-values}, prints only the names of the variables; a value
+for @var{print-values} of 1 or @code{--all-values}, also prints their
+values; and a value of 2 or @code{--simple-values} prints the name and
+value for simple data types and just the name for arrays, structures
+and unions.
 
 @subsubheading Example
 
@@ -20590,13 +20587,27 @@ subsequent @code{-var-update} list.
 @subsubheading Synopsis
 
 @smallexample
- -var-update @{@var{name} | "*"@}
+ -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.
+A @samp{*} causes all existing variable objects to be updated.  
+The option @var{print-values} determines whether names and values, or
+just names are printed in the manner described for @code{-var-list-children}.
+
+@subsubheading Example
 
+@smallexample
+(@value{GDBP})
+-var-assign var1 3
+^done,value="3"
+(@value{GDBP})
+-var-update --all-values var1
+^done,changelist=[@{name="var1",value="3",in_scope="true",
+type_changed="false"@}]
+(@value{GDBP})
+@end smallexample
 
 @node Annotations
 @chapter @value{GDBN} Annotations

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

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Tue, 5 Jul 2005 09:17:09 +1200
> Cc: Bob Rossi <bob@brasko.net>, gdb-patches@sources.redhat.com
> 
> I've changed the English slightly from last time, and because
> "-var-list-children" and "-var-update" have the same optional argument, I only
> describe it explictly once.  "-stack-list-locals" is slightly different
> because print-values is not optional and "--print-simple-values" prints the
> type also.
> 
> I've also corrected a spelling and made a couple of changes to the
> GDB/MI command description format to reflect reality.

Thanks.

Allow me a few comments:

> +Display the local variable names for the current frame.  A value for
> +@var{print-values} of 0 or @code{--no-values}, prints only the names
> +of the variables; a value of 1 or @code{--all-values}, prints also
> +their values; and a value of 2 or @code{--simple-values}, prints the
> +name, type and value for simple data types and the name and type for
> +arrays, structures and unions.

It would be much simpler to rephrase like this:

   Display the local variable names for the current frame.  If
   @var{print-values} is 0 or @code{--no-values}, print only the names
   of the variables; if it is 1 or @code{--all-values}, print their
   values as well; if it is 2 or @code{--simple-values}, print ...

you get the point.  Note that I also changed "prints" -> "print",
since the sentence before that says "Display", not "Displays".


> +In this last case, the idea is that
> +the user can see the value of simple data types immediately and he can
> +create variable objects for other data types if he wishes to explore
> +their values in more detail.

I think the GNU project's convention is not to use "he" where "she" is
also possible.

> +Returns a list of the children of the specified variable object.  A
> +single argument or an optional value for @var{print-values} of 0 or
> +@code{--no-values}, prints only the names of the variables; a value
> +for @var{print-values} of 1 or @code{--all-values}, also prints their
> +values; and a value of 2 or @code{--simple-values} prints the name and
> +value for simple data types and just the name for arrays, structures
> +and unions.

This could also use similar rephrasing.

> -A @samp{*} causes all existing variable objects to be updated.
> +A @samp{*} causes all existing variable objects to be updated.  

These two lines are identical, except that the second one has 2
trailing blanks.  Please remove those blanks.

> +The option @var{print-values} determines whether names and values, or
> +just names are printed in the manner described for @code{-var-list-children}.

Please add a cross-reference here that points to the description of
"-var-list-children".  (Since the commands don't have each one its own
node, you will have to use @anchor to create a point-able place for
the xref.)

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

[Nick Roberts]

 > > +Display the local variable names for the current frame.  A value for
 > > +@var{print-values} of 0 or @code{--no-values}, prints only the names
 > > +of the variables; a value of 1 or @code{--all-values}, prints also
 > > +their values; and a value of 2 or @code{--simple-values}, prints the
 > > +name, type and value for simple data types and the name and type for
 > > +arrays, structures and unions.
 > 
 > It would be much simpler to rephrase like this:
 > 
 >    Display the local variable names for the current frame.  If
 >    @var{print-values} is 0 or @code{--no-values}, print only the names
 >    of the variables; if it is 1 or @code{--all-values}, print their
 >    values as well; if it is 2 or @code{--simple-values}, print ...
 > 
 > you get the point.  Note that I also changed "prints" -> "print",
 > since the sentence before that says "Display", not "Displays".

OK.  The rest of the documentation uses a mixture of the imperative and
present tense.

Also it should read "for the selected frame".  There seem to be quite a few
places in the manual where "current frame" is used incorrectly
(-stack-select-frame is another example)

 > > +In this last case, the idea is that
 > > +the user can see the value of simple data types immediately and he can
 > > +create variable objects for other data types if he wishes to explore
 > > +their values in more detail.
 > 
 > I think the GNU project's convention is not to use "he" where "she" is
 > also possible.

OK.  And not to use the passive either.  It makes it difficult to express
things clearly in this case.

 > > +Returns a list of the children of the specified variable object.  A
 > > +single argument or an optional value for @var{print-values} of 0 or
 > > +@code{--no-values}, prints only the names of the variables; a value
 > > +for @var{print-values} of 1 or @code{--all-values}, also prints their
 > > +values; and a value of 2 or @code{--simple-values} prints the name and
 > > +value for simple data types and just the name for arrays, structures
 > > +and unions.
 > 
 > This could also use similar rephrasing.

OK.  I've also noted that -var-list-children creates variable objects for
the children if they  don't already exist.

 > > -A @samp{*} causes all existing variable objects to be updated.
 > > +A @samp{*} causes all existing variable objects to be updated.  
 > 
 > These two lines are identical, except that the second one has 2
 > trailing blanks.  Please remove those blanks.

OK.

 > > +The option @var{print-values} determines whether names and values, or
 > > +just names are printed in the manner described for @code{-var-list-children}.
 > 
 > Please add a cross-reference here that points to the description of
 > "-var-list-children".  (Since the commands don't have each one its own
 > node, you will have to use @anchor to create a point-able place for
 > the xref.)

OK.

Nick


2005-07-05  Nick Roberts  <nickrob@snap.net.nz>

	* gdb.texinfo (GDB/MI Variable Objects): Describe print-values
	option for -var-list-children and -var-update.
	(GDB/MI Stack Manipulation): Simplify description of
	print-values option for -stack-list-locals.  Say that
	-stack-select-frame changes selected frame, not current one.
	(GDB/MI Command Description Format): Clarify.
	(Mode Options): Spelling of superseded.


*** gdb.texinfo.~1.272.~	2005-07-04 11:59:22.000000000 +1200
--- gdb.texinfo	2005-07-05 15:23:44.000000000 +1200
***************
*** 1075,1081 ****
  @sc{gnu} Emacs, level 3 is the maximum annotation suitable for programs
  that control @value{GDBN}, and level 2 has been deprecated.
  
! The annotation mechanism has largely been superseeded by @sc{gdb/mi}
  (@pxref{GDB/MI}).
  
  @item --args
--- 1075,1081 ----
  @sc{gnu} Emacs, level 3 is the maximum annotation suitable for programs
  that control @value{GDBN}, and level 2 has been deprecated.
  
! The annotation mechanism has largely been superseded by @sc{gdb/mi}
  (@pxref{GDB/MI}).
  
  @item --args
***************
*** 17094,17112 ****
   -command @var{args}@dots{}
  @end smallexample
  
- @subsubheading @value{GDBN} Command
- 
- The corresponding @value{GDBN} CLI command.
- 
  @subsubheading Result
  
! @subsubheading Out-of-band
  
! @subsubheading Notes
  
  @subsubheading Example
  
- 
  @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
  @node GDB/MI Breakpoint Table Commands
  @section @sc{gdb/mi} Breakpoint table commands
--- 17094,17107 ----
   -command @var{args}@dots{}
  @end smallexample
  
  @subsubheading Result
  
! @subsubheading @value{GDBN} Command
  
! The corresponding @value{GDBN} CLI command(s), if any.
  
  @subsubheading Example
  
  @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
  @node GDB/MI Breakpoint Table Commands
  @section @sc{gdb/mi} Breakpoint table commands
***************
*** 19491,19504 ****
   -stack-list-locals @var{print-values}
  @end smallexample
  
! Display the local variable names for the current frame.  With an
! argument of 0 or @code{--no-values}, prints only the names of the variables.
! With argument of 1 or @code{--all-values}, prints also their values.  With
! argument of 2 or @code{--simple-values}, prints the name, type and value for
! simple data types and the name and type for arrays, structures and
! unions.  In this last case, the idea is that the user can see the
! value of simple data types immediately and he can create variable
! objects for other data types if he wishes to explore their values in
  more detail.
  
  @subsubheading @value{GDBN} Command
--- 19486,19499 ----
   -stack-list-locals @var{print-values}
  @end smallexample
  
! Display the local variable names for the selected frame.  If
! @var{print-values} is 0 or @code{--no-values}, print only the names of
! the variables; if it is 1 or @code{--all-values}, print also their
! values; and if it is 2 or @code{--simple-values}, print the name,
! type and value for simple data types and the name and type for arrays,
! structures and unions.  In this last case, a frontend can immediately
! display the value of simple data types and create variable objects for
! other data types when the the user wishes to explore their values in
  more detail.
  
  @subsubheading @value{GDBN} Command
***************
*** 19531,19537 ****
   -stack-select-frame @var{framenum}
  @end smallexample
  
! Change the current frame.  Select a different frame @var{framenum} on
  the stack.
  
  @subsubheading @value{GDBN} Command
--- 19526,19532 ----
   -stack-select-frame @var{framenum}
  @end smallexample
  
! Change the selected frame.  Select a different frame @var{framenum} on
  the stack.
  
  @subsubheading @value{GDBN} Command
***************
*** 20463,20485 ****
  @smallexample
   -var-list-children [@var{print-values}] @var{name}
  @end smallexample
  
! Returns a list of the children of the specified variable object.  With
! just the variable object name as an 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.
  
  @subsubheading Example
  
  @smallexample
  (@value{GDBP})
   -var-list-children n
!  numchild=@var{n},children=[@{name=@var{name},
   numchild=@var{n},type=@var{type}@},@r{(repeats N times)}]
  (@value{GDBP})
   -var-list-children --all-values n
!  numchild=@var{n},children=[@{name=@var{name},
   numchild=@var{n},value=@var{value},type=@var{type}@},@r{(repeats N times)}]
  @end smallexample
  
--- 20458,20484 ----
  @smallexample
   -var-list-children [@var{print-values}] @var{name}
  @end smallexample
+ @anchor{-var-list-children} 
  
! Return a list of the children of the specified variable object and
! create variable objects for them, if they do not already exist.  With
! a single argument or if @var{print-values} has a value for of 0 or
! @code{--no-values}, print only the names of the variables; if
! @var{print-values} is 1 or @code{--all-values}, also print their
! values; and if it is 2 or @code{--simple-values} print the name and
! value for simple data types and just the name for arrays, structures
! and unions.
  
  @subsubheading Example
  
  @smallexample
  (@value{GDBP})
   -var-list-children n
!  ^done,numchild=@var{n},children=[@{name=@var{name},
   numchild=@var{n},type=@var{type}@},@r{(repeats N times)}]
  (@value{GDBP})
   -var-list-children --all-values n
!  ^done,numchild=@var{n},children=[@{name=@var{name},
   numchild=@var{n},value=@var{value},type=@var{type}@},@r{(repeats N times)}]
  @end smallexample
  
***************
*** 20590,20602 ****
  @subsubheading Synopsis
  
  @smallexample
!  -var-update @{@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.
  
  
  @node Annotations
  @chapter @value{GDBN} Annotations
--- 20589,20616 ----
  @subsubheading Synopsis
  
  @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.  The
! option @var{print-values} determines whether names and values, or just
! names are printed in the manner described for
! @code{@pxref{-var-list-children}}.
! 
! @subsubheading Example
  
+ @smallexample
+ (@value{GDBP})
+ -var-assign var1 3
+ ^done,value="3"
+ (@value{GDBP})
+ -var-update --all-values var1
+ ^done,changelist=[@{name="var1",value="3",in_scope="true",
+ type_changed="false"@}]
+ (@value{GDBP})
+ @end smallexample
  
  @node Annotations
  @chapter @value{GDBN} Annotations

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

[Eli Zaretskii]

> From: Nick Roberts <nickrob@snap.net.nz>
> Date: Tue, 5 Jul 2005 15:25:19 +1200
> Cc: bob@brasko.net, gdb-patches@sources.redhat.com
> 
> 2005-07-05  Nick Roberts  <nickrob@snap.net.nz>
> 
> 	* gdb.texinfo (GDB/MI Variable Objects): Describe print-values
> 	option for -var-list-children and -var-update.
> 	(GDB/MI Stack Manipulation): Simplify description of
> 	print-values option for -stack-list-locals.  Say that
> 	-stack-select-frame changes selected frame, not current one.
> 	(GDB/MI Command Description Format): Clarify.
> 	(Mode Options): Spelling of superseded.

Thanks, this is fine.

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

[Nick Roberts]

 >       ...  Nick, here's a copy of the patch with the bug you found
 > stripped out of it.  Since you've got the current documentation, can
 > you update that and add some testcases, when you get a chance?  I'd
 > prefer to have all three go in together.

Daniel,

Eli has approved the documentation.  I have updated the source patch to work
with --simple-values.  I put the definition of "struct varobj" into varobj.h
to do this and modified mi_print_value_p.  You might prefer another way but
this enabled me to add testcases.  These latest changes are attached.
(existing patches for mi/mi-cmds.h and mi/mi-cmd-stack.c are also required)

Nick


2005-07-06  Nick Roberts  <nickrob@snap.net.nz>
	    Daniel Jacobowitz  <dan@codesourcery.com>

        * mi/mi-cmd-var.c (mi_no_values, mi_simple_values, mi_all_values):
	New variables.
	(mi_parse_values_option, mi_print_value_p): New functions.
        (mi_cmd_var_list_children): Use mi_parse_values_option and
	mi_print_value_p.
        (mi_cmd_var_update): Support a PRINT_VALUES option.  Update calls
	to varobj_update_one.
        (varobj_update_one): Take a print_values argument.  Call
	mi_print_value_p.
	* varobj.c (struct varobj): Move definition to...
	* varobj.h (struct varobj): ...here.

2005-07-06  Nick Roberts  <nickrob@snap.net.nz>

	* mi-var-child.exp: Adapt existing -var-update tests for
	--all-values and --simple-values options.
	Add -var-list-children test for --simple-values option.


Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.54
diff -u -p -r1.54 varobj.c
--- varobj.c	26 Apr 2005 05:03:37 -0000	1.54
+++ varobj.c	6 Jul 2005 09:27:42 -0000
@@ -81,49 +81,6 @@ struct varobj_root
 /* Every variable in the system has a structure of this type defined
    for it. This structure holds all information necessary to manipulate
    a particular object variable. Members which must be freed are noted. */
-struct varobj
-{
-
-  /* Alloc'd name of the variable for this object.. If this variable is a
-     child, then this name will be the child's source name.
-     (bar, not foo.bar) */
-  /* NOTE: This is the "expression" */
-  char *name;
-
-  /* The alloc'd name for this variable's object. This is here for
-     convenience when constructing this object's children. */
-  char *obj_name;
-
-  /* Index of this variable in its parent or -1 */
-  int index;
-
-  /* The type of this variable. This may NEVER be NULL. */
-  struct type *type;
-
-  /* The value of this expression or subexpression.  This may be NULL. */
-  struct value *value;
-
-  /* Did an error occur evaluating the expression or getting its value? */
-  int error;
-
-  /* The number of (immediate) children this variable has */
-  int num_children;
-
-  /* If this object is a child, this points to its immediate parent. */
-  struct varobj *parent;
-
-  /* A list of this object's children */
-  struct varobj_child *children;
-
-  /* Description of the root variable. Points to root variable for children. */
-  struct varobj_root *root;
-
-  /* The format of the output for this object */
-  enum varobj_display_formats format;
-
-  /* Was this variable updated via a varobj_set_value operation */
-  int updated;
-};
 
 /* Every variable keeps a linked list of its children, described
    by the following structure. */


Index: varobj.h
===================================================================
RCS file: /cvs/src/src/gdb/varobj.h,v
retrieving revision 1.4
diff -u -p -r1.4 varobj.h
--- varobj.h	17 Aug 2001 18:56:49 -0000	1.4
+++ varobj.h	6 Jul 2005 09:28:01 -0000
@@ -52,7 +52,49 @@ enum varobj_languages
 extern char *varobj_language_string[];
 
 /* Struct thar describes a variable object instance */
-struct varobj;
+struct varobj
+{
+
+  /* Alloc'd name of the variable for this object.. If this variable is a
+     child, then this name will be the child's source name.
+     (bar, not foo.bar) */
+  /* NOTE: This is the "expression" */
+  char *name;
+
+  /* The alloc'd name for this variable's object. This is here for
+     convenience when constructing this object's children. */
+  char *obj_name;
+
+  /* Index of this variable in its parent or -1 */
+  int index;
+
+  /* The type of this variable. This may NEVER be NULL. */
+  struct type *type;
+
+  /* The value of this expression or subexpression.  This may be NULL. */
+  struct value *value;
+
+  /* Did an error occur evaluating the expression or getting its value? */
+  int error;
+
+  /* The number of (immediate) children this variable has */
+  int num_children;
+
+  /* If this object is a child, this points to its immediate parent. */
+  struct varobj *parent;
+
+  /* A list of this object's children */
+  struct varobj_child *children;
+
+  /* Description of the root variable. Points to root variable for children. */
+  struct varobj_root *root;
+
+  /* The format of the output for this object */
+  enum varobj_display_formats format;
+
+  /* Was this variable updated via a varobj_set_value operation */
+  int updated;
+};
 
 /* API functions */
 

Index: mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.21
diff -u -p -r1.21 mi-cmd-var.c
--- mi/mi-cmd-var.c	11 Feb 2005 04:06:11 -0000	1.21
+++ mi/mi-cmd-var.c	6 Jul 2005 09:28:30 -0000
@@ -30,9 +30,14 @@
 #include <ctype.h>
 #include "gdb_string.h"
 
+const char mi_no_values[] = "--no-values";
+const char mi_simple_values[] = "--simple-values";
+const char mi_all_values[] = "--all-values";
+
 extern int varobjdebug;		/* defined in varobj.c */
 
-static int varobj_update_one (struct varobj *var);
+static int varobj_update_one (struct varobj *var,
+			      enum print_values print_values);
 
 /* VAROBJ operations */
 
@@ -247,6 +252,52 @@ mi_cmd_var_info_num_children (char *comm
   return MI_CMD_DONE;
 }
 
+/* Parse a string argument into a print_values value.  */
+
+static enum print_values
+mi_parse_values_option (const char *arg)
+{
+  if (strcmp (arg, "0") == 0
+      || strcmp (arg, mi_no_values) == 0)
+    return PRINT_NO_VALUES;
+  else if (strcmp (arg, "1") == 0
+	   || strcmp (arg, mi_all_values) == 0)
+    return PRINT_ALL_VALUES;
+  else if (strcmp (arg, "2") == 0
+	   || strcmp (arg, mi_simple_values) == 0)
+    return PRINT_SIMPLE_VALUES;
+  else
+    error (_("Unknown value for PRINT_VALUES\n\
+Must be: 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
+	   mi_no_values, mi_simple_values, mi_all_values);
+}
+
+/* Return 1 if given the argument PRINT_VALUES we should display
+   a value of type TYPE.  */
+
+static int
+mi_print_value_p (struct varobj *var, enum print_values print_values)
+{
+  struct type *type;
+  type = var->type;
+
+  if (type != NULL)
+    type = check_typedef (type);
+
+  if (print_values == PRINT_NO_VALUES)
+    return 0;
+
+  if (print_values == PRINT_ALL_VALUES)
+    return 1;
+
+  /* For PRINT_SIMPLE_VALUES, only print the value if it has a type
+     and that type is not a compound type.  */
+
+  return (TYPE_CODE (type) != TYPE_CODE_ARRAY
+	  && TYPE_CODE (type) != TYPE_CODE_STRUCT
+	  && TYPE_CODE (type) != TYPE_CODE_UNION);
+}
+
 enum mi_cmd_result
 mi_cmd_var_list_children (char *command, char **argv, int argc)
 {
@@ -258,27 +309,23 @@ mi_cmd_var_list_children (char *command,
   char *type;
   enum print_values print_values;
 
-  if (argc != 1 && argc != 2)
+  if (argc > 2)
     error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
 
   /* Get varobj handle, if a valid var obj name was specified */
-  if (argc == 1) var = varobj_get_handle (argv[0]);
-  else var = varobj_get_handle (argv[1]);
+  if (argc == 1)
+    var = varobj_get_handle (argv[0]);
+  else
+    var = varobj_get_handle (argv[1]);
   if (var == NULL)
     error (_("Variable object not found"));
 
   numchild = varobj_list_children (var, &childlist);
   ui_out_field_int (uiout, "numchild", numchild);
   if (argc == 2)
-    if (strcmp (argv[0], "0") == 0
-	|| strcmp (argv[0], "--no-values") == 0)
-      print_values = PRINT_NO_VALUES;
-    else if (strcmp (argv[0], "1") == 0
-	     || strcmp (argv[0], "--all-values") == 0)
-      print_values = PRINT_ALL_VALUES;
-    else
-     error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\""));
-  else print_values = PRINT_NO_VALUES;
+    print_values = mi_parse_values_option (argv[0]);
+  else
+    print_values = PRINT_NO_VALUES;
 
   if (numchild <= 0)
     return MI_CMD_DONE;
@@ -295,12 +342,12 @@ mi_cmd_var_list_children (char *command,
       ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
       ui_out_field_string (uiout, "exp", varobj_get_expression (*cc));
       ui_out_field_int (uiout, "numchild", varobj_get_num_children (*cc));
-      if (print_values)
+      if (mi_print_value_p (*cc, print_values))
 	ui_out_field_string (uiout, "value", varobj_get_value (*cc));
       type = varobj_get_type (*cc);
       /* C++ pseudo-variables (public, private, protected) do not have a type */
       if (type)
-	ui_out_field_string (uiout, "type", varobj_get_type (*cc));
+	ui_out_field_string (uiout, "type", type);
       do_cleanups (cleanup_child);
       cc++;
     }
@@ -426,11 +473,20 @@ mi_cmd_var_update (char *command, char *
   struct cleanup *cleanup;
   char *name;
   int nv;
+  enum print_values print_values;
 
-  if (argc != 1)
-    error (_("mi_cmd_var_update: Usage: NAME."));
+  if (argc != 1 && argc != 2)
+    error (_("mi_cmd_var_update: Usage: [PRINT_VALUES] NAME."));
 
-  name = argv[0];
+  if (argc == 1)
+    name = argv[0];
+  else
+    name = (argv[1]);
+
+  if (argc == 2)
+    print_values = mi_parse_values_option (argv[0]);
+  else
+    print_values = PRINT_NO_VALUES;
 
   /* Check if the parameter is a "*" which means that we want
      to update all variables */
@@ -450,7 +506,7 @@ mi_cmd_var_update (char *command, char *
       cr = rootlist;
       while (*cr != NULL)
 	{
-	  varobj_update_one (*cr);
+	  varobj_update_one (*cr, print_values);
 	  cr++;
 	}
       xfree (rootlist);
@@ -467,7 +523,7 @@ mi_cmd_var_update (char *command, char *
         cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
       else
         cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
-      varobj_update_one (var);
+      varobj_update_one (var, print_values);
       do_cleanups (cleanup);
     }
     return MI_CMD_DONE;
@@ -478,7 +534,7 @@ mi_cmd_var_update (char *command, char *
    scope), and 1 if it succeeds. */
 
 static int
-varobj_update_one (struct varobj *var)
+varobj_update_one (struct varobj *var, enum print_values print_values)
 {
   struct varobj **changelist;
   struct varobj **cc;
@@ -524,6 +580,8 @@ varobj_update_one (struct varobj *var)
 	  if (mi_version (uiout) > 1)
 	    cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 	  ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
+	  if (mi_print_value_p (*cc, print_values))
+	    ui_out_field_string (uiout, "value", varobj_get_value (*cc));
 	  ui_out_field_string (uiout, "in_scope", "true");
 	  ui_out_field_string (uiout, "type_changed", "false");
 	  if (mi_version (uiout) > 1)


Index: testsuite/gdb.mi/mi-var-child.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-child.exp,v
retrieving revision 1.17
diff -u -p -r1.17 mi-var-child.exp
--- testsuite/gdb.mi/mi-var-child.exp	17 Aug 2004 09:38:29 -0000	1.17
+++ testsuite/gdb.mi/mi-var-child.exp	6 Jul 2005 09:56:25 -0000
@@ -9,7 +9,7 @@
 # but WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
-#
+# 
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
@@ -800,8 +800,8 @@ mi_step_to do_children_tests {} {.*var-c
 
 # Test: c_variable-5.6
 # Desc: check that long_array[1] changed
-mi_gdb_test "-var-update *" \
-	"\\^done,changelist=\\\[\{name=\"struct_declarations.long_array.1\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+mi_gdb_test "-var-update --all-values *" \
+	"\\^done,changelist=\\\[\{name=\"struct_declarations.long_array.1\",value=\"2345\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
 	"update all vars struct_declarations.long_array.1 changed"
 
 # Step over "weird->long_array[2] = 3456;"
@@ -810,9 +810,9 @@ mi_step_to do_children_tests {} {.*var-c
 
 # Test: c_variable-5.7
 # Desc: check that long_array[2] changed
-mi_gdb_test "-var-update *" \
-	"\\^done,changelist=\\\[\{name=\"struct_declarations.long_array.2\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
-	"update all vars struct_declarations.long_array.2 changed"
+mi_gdb_test "-var-update --simple-values *" \
+	"\\^done,changelist=\\\[\{name=\"struct_declarations.long_array.2\",value=\"3456\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+	"update all vars struct_declarations.long_array.2 changed, print value"
 
 # Step over:
 #    struct_declarations.long_array[3] = 4567;
@@ -837,6 +837,10 @@ mi_gdb_test "-var-list-children --all-va
         "\\^done,numchild=\"10\",children=\\\[child=\{name=\"struct_declarations.long_array.0\",exp=\"0\",numchild=\"0\",value=\"1234\",type=\"long int\"\},child=\{name=\"struct_declarations.long_array.1\",exp=\"1\",numchild=\"0\",value=\"2345\",type=\"long int\"\},child=\{name=\"struct_declarations.long_array.2\",exp=\"2\",numchild=\"0\",value=\"3456\",type=\"long int\"\},child=\{name=\"struct_declarations.long_array.3\",exp=\"3\",numchild=\"0\",value=\"4567\",type=\"long int\"\},child=\{name=\"struct_declarations.long_array.4\",exp=\"4\",numchild=\"0\",value=\"5678\",type=\"long int\"\},child=\{name=\"struct_declarations.long_array.5\",exp=\"5\",numchild=\"0\",value=\"6789\",type=\"long int\"\},child=\{name=\"struct_declarations.long_array.6\",exp=\"6\",numchild=\"0\",value=\"7890\",type=\"long int\"\},child=\{name=\"struct_declarations.long_array.7\",exp=\"7\",numchild=\"0\",value=\"8901\",type=\"long int\"\},child=\{name=\"struct_declarations.long_array.8\",exp=\"8\",numchild=\"0\",value=\"9012\",type=\"long int\"\},child=\{name=\"struct_declarations.long_array.9\",exp=\"9\",numchild=\"0\",value=\"1234\",type=\"long int\"\}\\\]" \
                 "listing of names and values of children"
 
+mi_gdb_test "-var-list-children --simple-values struct_declarations" \
+        "\\^done,numchild=\"11\",children=\\\[child=\{name=\"var1.integer\",exp=\"integer\",numchild=\"0\",value=\"123\",type=\"int\"\},child=\{name=\"var1.character\",exp=\"character\",numchild=\"0\",value=\"0 '\\0'\",type=\"char\"\},child=\{name=\"var1.char_ptr\",exp=\"char_ptr\",numchild=\"1\",value=\"$hex \\\\\"hello\\\\\"\",type=\"char *\"\},child=\{name=\"var1.long_int\",exp=\"long_int\",numchild=\"0\",value=\"0\",type=\"long int\"\},child=\{name=\"var1.int_ptr_ptr\",exp=\"int_ptr_ptr\",numchild=\"1\",value=\"$hex\",type=\"int **\"\},child=\{name=\"var1.long_array\",exp=\"long_array\",numchild=\"10\",type=\"long int \\\[10\\\]\"\},child=\{name=\"var1.func_ptr\",exp=\"func_ptr\",numchild=\"0\",value=\"0\",type=\"void (*)(void)\"\},child=\{name=\"var1.func_ptr_struct\",exp=\"func_ptr_struct\",numchild=\"0\",value=\"0\",type=\"struct _struct_decl (*)(int, char *, long int)\"\},child=\{name=\"var1.func_ptr_ptr\",exp=\"func_ptr_ptr\",numchild=\"0\",value=\"0\",type=\"struct _struct_decl *(*)(int, char *, long int)\"\},child=\{name=\"var1.u1\",exp=\"u1\",numchild=\"4\",type=\"union \{...\}\"\},child=\{name=\"var1.s2\",exp=\"s2\",numchild=\"4\",type=\"struct \{...\}\"\}\\\]" \
+  "listing of children, simple types: names and values, complex types: values"
+
 # Step over "weird->func_ptr = nothing;"
 set line_dct_a0_0 [gdb_get_line_number "a0 = '0';"]
 mi_step_to do_children_tests {} {.*var-cmd.c} \

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

[Daniel Jacobowitz]

On Wed, Jul 06, 2005 at 10:01:54PM +1200, Nick Roberts wrote:
>  >       ...  Nick, here's a copy of the patch with the bug you found
>  > stripped out of it.  Since you've got the current documentation, can
>  > you update that and add some testcases, when you get a chance?  I'd
>  > prefer to have all three go in together.
> 
> Daniel,
> 
> Eli has approved the documentation.  I have updated the source patch to work
> with --simple-values.  I put the definition of "struct varobj" into varobj.h
> to do this and modified mi_print_value_p.  You might prefer another way but
> this enabled me to add testcases.  These latest changes are attached.
> (existing patches for mi/mi-cmds.h and mi/mi-cmd-stack.c are also required)

For the future, I'd appreciate it if you didn't post partial patches
this way; I went back and dug up the right diffs, which isn't hard, but
is error-prone.  But I can handle it :-)

My version of TCL really did not like your testcase.  Did you attach
the right patch?  This one referenced var1 in the regexp but not in the
command, so it seems that it couldn't have passed.  And it added some
weird trailing whitespace.

This patch includes the missing files, and I redid the varobj change in
a pedantically different way.  Thanks for catching my goof there, by
the way.  And caught a bad argc check on -var-list-children that I
think you picked up from me at some point.

How's it look?  If it looks good to you, I'll check it in, and then you
can commit the documentation and we can work out what happened to your
testcase.

Also, this is for later, but I noticed some inconsistency in reviewing
the docs:

-stack-list-locals
   --no-values doesn't print types.
   --simple-value prints the types for everything, even complex values.
   --all-values doesn't print types.

-var-list-children always prints types, regardless of the PRINT_VALUES
option given.

-var-update never prints types, regardless of the PRINT_VALUES option.

Should some of those be cleaned up?  At a guess, I'd make
-stack-list-locals --all-values print types, and -var-update
--all-values or --simple-values.  Didn't think about it much though.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

2005-07-14  Nick Roberts  <nickrob@snap.net.nz>
	    Daniel Jacobowitz  <dan@codesourcery.com>

	* mi/mi-cmds.h (mi_no_values, mi_simple_values, mi_all_values): New
	declarations.
	* mi/mi-cmd-stack.c (mi_cmd_stack_list_locals): Use string
	constants instead of literals for MI command options.
	* mi/mi-cmd-var.c (mi_no_values, mi_simple_values, mi_all_values):
	New variables.
	(mi_parse_values_option, mi_print_value_p): New functions.
	(mi_cmd_var_list_children): Use mi_parse_values_option and
	mi_print_value_p.
	(mi_cmd_var_update): Support a PRINT_VALUES option.  Update calls
	to varobj_update_one.
	(varobj_update_one): Take a print_values argument.  Call
	mi_print_value_p.
	* varobj.c (varobj_get_gdb_type): New function.
	* varobj.h (varobj_get_gdb_type): New prototype.

Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.54
diff -u -p -r1.54 varobj.c
--- varobj.c	26 Apr 2005 05:03:37 -0000	1.54
+++ varobj.c	15 Jul 2005 01:40:23 -0000
@@ -1,6 +1,7 @@
 /* Implementation of the GDB variable objects API.
 
-   Copyright 1999, 2000, 2001, 2005 Free Software Foundation, Inc.
+   Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2005
+   Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -760,6 +761,14 @@ varobj_get_type (struct varobj *var)
   return thetype;
 }
 
+/* Obtain the type of an object variable.  */
+
+struct type *
+varobj_get_gdb_type (struct varobj *var)
+{
+  return var->type;
+}
+
 enum varobj_languages
 varobj_get_language (struct varobj *var)
 {
Index: varobj.h
===================================================================
RCS file: /cvs/src/src/gdb/varobj.h,v
retrieving revision 1.4
diff -u -p -r1.4 varobj.h
--- varobj.h	17 Aug 2001 18:56:49 -0000	1.4
+++ varobj.h	15 Jul 2005 01:40:23 -0000
@@ -1,5 +1,5 @@
 /* GDB variable objects API.
-   Copyright 1999, 2000 Free Software Foundation, Inc.
+   Copyright 1999, 2000, 2001, 2005 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -85,6 +85,8 @@ extern int varobj_list_children (struct 
 
 extern char *varobj_get_type (struct varobj *var);
 
+extern struct type *varobj_get_gdb_type (struct varobj *var);
+
 extern enum varobj_languages varobj_get_language (struct varobj *var);
 
 extern int varobj_get_attributes (struct varobj *var);
Index: mi/mi-cmd-stack.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-stack.c,v
retrieving revision 1.27
diff -u -p -r1.27 mi-cmd-stack.c
--- mi/mi-cmd-stack.c	19 Jun 2005 03:11:47 -0000	1.27
+++ mi/mi-cmd-stack.c	15 Jul 2005 01:40:23 -0000
@@ -1,5 +1,5 @@
 /* MI Command Set - stack commands.
-   Copyright 2000, 2002, 2003, 2004 Free Software Foundation, Inc.
+   Copyright 2000, 2002, 2003, 2004, 2005 Free Software Foundation, Inc.
    Contributed by Cygnus Solutions (a Red Hat company).
 
    This file is part of GDB.
@@ -136,16 +136,18 @@ mi_cmd_stack_list_locals (char *command,
    frame = get_selected_frame (NULL);
 
    if (strcmp (argv[0], "0") == 0
-       || strcmp (argv[0], "--no-values") == 0)
+       || strcmp (argv[0], mi_no_values) == 0)
      print_values = PRINT_NO_VALUES;
    else if (strcmp (argv[0], "1") == 0
-	    || strcmp (argv[0], "--all-values") == 0)
+	    || strcmp (argv[0], mi_all_values) == 0)
      print_values = PRINT_ALL_VALUES;
    else if (strcmp (argv[0], "2") == 0
-	    || strcmp (argv[0], "--simple-values") == 0)
+	    || strcmp (argv[0], mi_simple_values) == 0)
      print_values = PRINT_SIMPLE_VALUES;
    else
-     error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\", 2 or \"--simple-values\""));
+     error (_("Unknown value for PRINT_VALUES: must be: \
+0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
+	    mi_no_values, mi_all_values, mi_simple_values);
   list_args_or_locals (1, print_values, frame);
   return MI_CMD_DONE;
 }
Index: mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.21
diff -u -p -r1.21 mi-cmd-var.c
--- mi/mi-cmd-var.c	11 Feb 2005 04:06:11 -0000	1.21
+++ mi/mi-cmd-var.c	15 Jul 2005 01:40:23 -0000
@@ -1,6 +1,6 @@
 /* MI Command Set - varobj commands.
 
-   Copyright 2000, 2002, 2004 Free Software Foundation, Inc.
+   Copyright 2000, 2002, 2004, 2005 Free Software Foundation, Inc.
 
    Contributed by Cygnus Solutions (a Red Hat company).
 
@@ -30,9 +30,14 @@
 #include <ctype.h>
 #include "gdb_string.h"
 
+const char mi_no_values[] = "--no-values";
+const char mi_simple_values[] = "--simple-values";
+const char mi_all_values[] = "--all-values";
+
 extern int varobjdebug;		/* defined in varobj.c */
 
-static int varobj_update_one (struct varobj *var);
+static int varobj_update_one (struct varobj *var,
+			      enum print_values print_values);
 
 /* VAROBJ operations */
 
@@ -247,6 +252,49 @@ mi_cmd_var_info_num_children (char *comm
   return MI_CMD_DONE;
 }
 
+/* Parse a string argument into a print_values value.  */
+
+static enum print_values
+mi_parse_values_option (const char *arg)
+{
+  if (strcmp (arg, "0") == 0
+      || strcmp (arg, mi_no_values) == 0)
+    return PRINT_NO_VALUES;
+  else if (strcmp (arg, "1") == 0
+	   || strcmp (arg, mi_all_values) == 0)
+    return PRINT_ALL_VALUES;
+  else if (strcmp (arg, "2") == 0
+	   || strcmp (arg, mi_simple_values) == 0)
+    return PRINT_SIMPLE_VALUES;
+  else
+    error (_("Unknown value for PRINT_VALUES\n\
+Must be: 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
+	   mi_no_values, mi_simple_values, mi_all_values);
+}
+
+/* Return 1 if given the argument PRINT_VALUES we should display
+   a value of type TYPE.  */
+
+static int
+mi_print_value_p (struct type *type, enum print_values print_values)
+{
+  if (type != NULL)
+    type = check_typedef (type);
+
+  if (print_values == PRINT_NO_VALUES)
+    return 0;
+
+  if (print_values == PRINT_ALL_VALUES)
+    return 1;
+
+  /* For PRINT_SIMPLE_VALUES, only print the value if it has a type
+     and that type is not a compound type.  */
+
+  return (TYPE_CODE (type) != TYPE_CODE_ARRAY
+	  && TYPE_CODE (type) != TYPE_CODE_STRUCT
+	  && TYPE_CODE (type) != TYPE_CODE_UNION);
+}
+
 enum mi_cmd_result
 mi_cmd_var_list_children (char *command, char **argv, int argc)
 {
@@ -262,23 +310,19 @@ mi_cmd_var_list_children (char *command,
     error (_("mi_cmd_var_list_children: Usage: [PRINT_VALUES] NAME"));
 
   /* Get varobj handle, if a valid var obj name was specified */
-  if (argc == 1) var = varobj_get_handle (argv[0]);
-  else var = varobj_get_handle (argv[1]);
+  if (argc == 1)
+    var = varobj_get_handle (argv[0]);
+  else
+    var = varobj_get_handle (argv[1]);
   if (var == NULL)
     error (_("Variable object not found"));
 
   numchild = varobj_list_children (var, &childlist);
   ui_out_field_int (uiout, "numchild", numchild);
   if (argc == 2)
-    if (strcmp (argv[0], "0") == 0
-	|| strcmp (argv[0], "--no-values") == 0)
-      print_values = PRINT_NO_VALUES;
-    else if (strcmp (argv[0], "1") == 0
-	     || strcmp (argv[0], "--all-values") == 0)
-      print_values = PRINT_ALL_VALUES;
-    else
-     error (_("Unknown value for PRINT_VALUES: must be: 0 or \"--no-values\", 1 or \"--all-values\""));
-  else print_values = PRINT_NO_VALUES;
+    print_values = mi_parse_values_option (argv[0]);
+  else
+    print_values = PRINT_NO_VALUES;
 
   if (numchild <= 0)
     return MI_CMD_DONE;
@@ -295,12 +339,12 @@ mi_cmd_var_list_children (char *command,
       ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
       ui_out_field_string (uiout, "exp", varobj_get_expression (*cc));
       ui_out_field_int (uiout, "numchild", varobj_get_num_children (*cc));
-      if (print_values)
+      if (mi_print_value_p (varobj_get_gdb_type (*cc), print_values))
 	ui_out_field_string (uiout, "value", varobj_get_value (*cc));
       type = varobj_get_type (*cc);
       /* C++ pseudo-variables (public, private, protected) do not have a type */
       if (type)
-	ui_out_field_string (uiout, "type", varobj_get_type (*cc));
+	ui_out_field_string (uiout, "type", type);
       do_cleanups (cleanup_child);
       cc++;
     }
@@ -426,11 +470,20 @@ mi_cmd_var_update (char *command, char *
   struct cleanup *cleanup;
   char *name;
   int nv;
+  enum print_values print_values;
 
-  if (argc != 1)
-    error (_("mi_cmd_var_update: Usage: NAME."));
+  if (argc != 1 && argc != 2)
+    error (_("mi_cmd_var_update: Usage: [PRINT_VALUES] NAME."));
+
+  if (argc == 1)
+    name = argv[0];
+  else
+    name = (argv[1]);
 
-  name = argv[0];
+  if (argc == 2)
+    print_values = mi_parse_values_option (argv[0]);
+  else
+    print_values = PRINT_NO_VALUES;
 
   /* Check if the parameter is a "*" which means that we want
      to update all variables */
@@ -450,7 +503,7 @@ mi_cmd_var_update (char *command, char *
       cr = rootlist;
       while (*cr != NULL)
 	{
-	  varobj_update_one (*cr);
+	  varobj_update_one (*cr, print_values);
 	  cr++;
 	}
       xfree (rootlist);
@@ -467,7 +520,7 @@ mi_cmd_var_update (char *command, char *
         cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, "changelist");
       else
         cleanup = make_cleanup_ui_out_list_begin_end (uiout, "changelist");
-      varobj_update_one (var);
+      varobj_update_one (var, print_values);
       do_cleanups (cleanup);
     }
     return MI_CMD_DONE;
@@ -478,7 +531,7 @@ mi_cmd_var_update (char *command, char *
    scope), and 1 if it succeeds. */
 
 static int
-varobj_update_one (struct varobj *var)
+varobj_update_one (struct varobj *var, enum print_values print_values)
 {
   struct varobj **changelist;
   struct varobj **cc;
@@ -524,6 +577,8 @@ varobj_update_one (struct varobj *var)
 	  if (mi_version (uiout) > 1)
 	    cleanup = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
 	  ui_out_field_string (uiout, "name", varobj_get_objname (*cc));
+	  if (mi_print_value_p (varobj_get_gdb_type (*cc), print_values))
+	    ui_out_field_string (uiout, "value", varobj_get_value (*cc));
 	  ui_out_field_string (uiout, "in_scope", "true");
 	  ui_out_field_string (uiout, "type_changed", "false");
 	  if (mi_version (uiout) > 1)
Index: mi/mi-cmds.h
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmds.h,v
retrieving revision 1.17
diff -u -p -r1.17 mi-cmds.h
--- mi/mi-cmds.h	6 Jul 2005 14:54:36 -0000	1.17
+++ mi/mi-cmds.h	15 Jul 2005 01:40:23 -0000
@@ -1,6 +1,6 @@
 /* MI Command Set for GDB, the GNU debugger.
 
-   Copyright 2000, 2003, 2004 Free Software Foundation, Inc.
+   Copyright 2000, 2003, 2004, 2005 Free Software Foundation, Inc.
 
    Contributed by Cygnus Solutions (a Red Hat company).
 
@@ -50,6 +50,10 @@ enum print_values {
    PRINT_SIMPLE_VALUES
 };
 
+extern const char mi_no_values[];
+extern const char mi_simple_values[];
+extern const char mi_all_values[];
+
 typedef enum mi_cmd_result (mi_cmd_argv_ftype) (char *command, char **argv, int argc);
 
 /* Older MI commands have this interface. Retained until all old

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

[Nick Roberts]

Daniel Jacobowitz writes:
 > For the future, I'd appreciate it if you didn't post partial patches
 > this way; I went back and dug up the right diffs, which isn't hard, but
 > is error-prone.  But I can handle it :-)

OK.

 > My version of TCL really did not like your testcase.  Did you attach
 > the right patch?  This one referenced var1 in the regexp but not in the
 > command, so it seems that it couldn't have passed.  And it added some
 > weird trailing whitespace.

Thats strange, I don't know how it passed.  I must have got the initial output
using var1 but earlier tests failed until I added extra backslashes etc.  I'll
take another look.

 > This patch includes the missing files, and I redid the varobj change in
 > a pedantically different way.  Thanks for catching my goof there, by
 > the way.  And caught a bad argc check on -var-list-children that I
 > think you picked up from me at some point.

Your change is clearly more object oriented.  I don't know why varobj.c is in
the gdb directory: it's only really used by mi-cmd-var.c.  How about moving it
into the mi directory, or even merging it with mi-cmd-var.c to form one file?
That way all the static functions currently in varobj.c will be automatically
accessible to the functions currently in mi-cmd-var.c.

 > How's it look?  If it looks good to you, I'll check it in, and then you
 > can commit the documentation and we can work out what happened to your
 > testcase.

Yes, it looks good to me.  Perhaps I can test it more fully once you've
committed it.  Now that the hectic release schedule of GDB has slowed down, I
think this is a good way to work.  I'm not sure that you will agree, though.

 > Also, this is for later, but I noticed some inconsistency in reviewing
 > the docs:
 > 
 > -stack-list-locals
 >    --no-values doesn't print types.
 >    --simple-value prints the types for everything, even complex values.
 >    --all-values doesn't print types.
 > 
 > -var-list-children always prints types, regardless of the PRINT_VALUES
 > option given.
 > 
 > -var-update never prints types, regardless of the PRINT_VALUES option.

Previously -stack-list-locals and -var-update didn't print types
while -var-list-children did.
 
For -stack-list-locals, I added the type so that I could display that in
the locals buffer instead of the value(s) for complex types. The user
could then examine the values with variable objects if he wished.

 > Should some of those be cleaned up?  At a guess, I'd make
 > -stack-list-locals --all-values print types, and -var-update
 > --all-values or --simple-values.  Didn't think about it much though.

I think the behaviour should be governed by use not consistency.  I don't
really have an opinion though as I will only use:

-stack-list-locals --simple-values
-var-list-children --all-values
-var-update --all-values

If "-var-update --all-values" prints types, I can easily arrange for Emacs
to ignore it.

Nick

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

[Daniel Jacobowitz]

On Fri, Jul 15, 2005 at 03:58:44PM +1200, Nick Roberts wrote:
>  > This patch includes the missing files, and I redid the varobj change in
>  > a pedantically different way.  Thanks for catching my goof there, by
>  > the way.  And caught a bad argc check on -var-list-children that I
>  > think you picked up from me at some point.
> 
> Your change is clearly more object oriented.  I don't know why varobj.c is in
> the gdb directory: it's only really used by mi-cmd-var.c.  How about moving it
> into the mi directory, or even merging it with mi-cmd-var.c to form one file?
> That way all the static functions currently in varobj.c will be automatically
> accessible to the functions currently in mi-cmd-var.c.

I bet you've got only the GDB module checked out.  Varobj was not
written for MI - it was written for Insight (gdb/gdbtk/).

>  > How's it look?  If it looks good to you, I'll check it in, and then you
>  > can commit the documentation and we can work out what happened to your
>  > testcase.
> 
> Yes, it looks good to me.  Perhaps I can test it more fully once you've
> committed it.  Now that the hectic release schedule of GDB has slowed down, I
> think this is a good way to work.  I'm not sure that you will agree, though.

In general, it's not my favorite approach, but it's growing on me.  I
have checked in the patch now; feel free to check in the docs, since
Eli approved them.  We can sort out testcases next.

BTW, if it's OK with you, I would prefer that you add new testcases
rather than modifying existing ones.  Yes, the ones that are there are
slightly redundant.  But changing what a test case is testing is bad
form for long-term results analysis.

> I think the behaviour should be governed by use not consistency.  I don't
> really have an opinion though as I will only use:

Makes sense.  I can see why displaying the type for -var-update seems
less than useful - although if it displayed dynamic type... hmmm...
well, something to think about much later.

Now that this patch is in, I believe your current gdb-mi.el will work
with CVS GDB.  Is that right?  If so, could you post it?  I promise to
look at it promptly.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

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

[Nick Roberts]

Eli Zaretskii writes:
 > > 2005-07-05  Nick Roberts  <nickrob@snap.net.nz>
 > > 
 > > 	* gdb.texinfo (GDB/MI Variable Objects): Describe print-values
 > > 	option for -var-list-children and -var-update.
 > > 	(GDB/MI Stack Manipulation): Simplify description of
 > > 	print-values option for -stack-list-locals.  Say that
 > > 	-stack-select-frame changes selected frame, not current one.
 > > 	(GDB/MI Command Description Format): Clarify.
 > > 	(Mode Options): Spelling of superseded.
 > 
 > Thanks, this is fine.

Committed.

Nick

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

[Nick Roberts]

 > > That way all the static functions currently in varobj.c will be
 > > automatically accessible to the functions currently in mi-cmd-var.c.
 > 
 > I bet you've got only the GDB module checked out.  Varobj was not
 > written for MI - it was written for Insight (gdb/gdbtk/).

You're right.  That explains a lot, I didn't realise it was shared code.

 > BTW, if it's OK with you, I would prefer that you add new testcases
 > rather than modifying existing ones.  Yes, the ones that are there are
 > slightly redundant.  But changing what a test case is testing is bad
 > form for long-term results analysis.

OK.  I'm not very comfortable with expect, but I'll give it a go.

 > Now that this patch is in, I believe your current gdb-mi.el will work
 > with CVS GDB.  Is that right?  If so, could you post it?  I promise to
 > look at it promptly.

Yes it does work, but not perfectly (if it did it could go straight into
the Emacs CVS repository).  It has changed quite a lot since last time.
Unfortunately it still requires Emacs in CVS, but a release is planned.

Nick


2005-07-16  Nick Roberts  <nickrob@snap.net.nz>

	* mi/gdb-mi.el: Update for changes in Emacs 22.0.50.  Bring more
	features over from gdb-ui.el.  Use "-var-update --all-values" for
	faster re-display of watch expressions.


*** mi/gdb-mi.el.~1.1.~	2004-07-27 09:52:34.000000000 +1200
--- mi/gdb-mi.el	2005-07-15 23:58:53.000000000 +1200
***************
*** 1,13 ****
! ;;; gdb-mi.el (internally gdbmi6.el) - (24th May 2004)
! 
! ;; Run gdb with GDB/MI (-interp=mi) and access CLI using "cli-command"
! ;; (could use "-interpreter-exec console cli-command")
  
  ;; Author: Nick Roberts <nickrob@gnu.org>
  ;; Maintainer: Nick Roberts <nickrob@gnu.org>
  ;; Keywords: unix, tools
  
! ;; Copyright (C) 2004  Free Software Foundation, Inc.
  
  ;; This file is part of GNU GDB.
  
--- 1,10 ----
! ;;; gdb-mi.el
  
  ;; Author: Nick Roberts <nickrob@gnu.org>
  ;; Maintainer: Nick Roberts <nickrob@gnu.org>
  ;; Keywords: unix, tools
  
! ;; Copyright (C) 2004, 2005 Free Software Foundation, Inc.
  
  ;; This file is part of GNU GDB.
  
***************
*** 23,51 ****
  
  ;;; Commentary:
  
! ;; This mode acts as a graphical user interface to GDB and requires GDB 6.1
! ;; onwards. You can interact with GDB through the GUD buffer in the usual way,
! ;; but there are also buffers which control the execution and describe the
! ;; state of your program. It separates the input/output of your program from
! ;; that of GDB and displays expressions and their current values in their own
! ;; buffers. It also uses features of Emacs 21 such as the fringe/display
! ;; margin for breakpoints, and the toolbar (see the GDB Graphical Interface
! ;; section in the Emacs info manual).
  
  ;; Start the debugger with M-x gdbmi.
  
  ;; This file uses GDB/MI as the primary interface to GDB. It is still under
! ;; development and is part of a process to migrate Emacs from annotations
! ;; (as used in gdb-ui.el) to GDB/MI.
! 
  ;; Known Bugs:
  ;;
! 
  ;;; Code:
  
  (require 'gud)
  (require 'gdb-ui)
! \f
  
  ;;;###autoload
  (defun gdbmi (command-line)
--- 20,68 ----
  
  ;;; Commentary:
  
! ;; This mode acts as a graphical user interface to GDB and works with Emacs
! ;; 22.x and the version of GDB with which it is distributed.  You can interact
! ;; with GDB through the GUD buffer in the usual way, but there are also
! ;; buffers which control the execution and describe the state of your program.
! ;; It separates the input/output of your program from that of GDB and displays
! ;; expressions and their current values in their own buffers.  It also uses
! ;; features of Emacs 21 such as the fringe/display margin for breakpoints, and
! ;; the toolbar (see the GDB Graphical Interface section in the Emacs info
! ;; manual).
  
  ;; Start the debugger with M-x gdbmi.
  
  ;; This file uses GDB/MI as the primary interface to GDB. It is still under
! ;; development and is part of a process to migrate Emacs from annotations (as
! ;; used in gdb-ui.el) to GDB/MI.  It runs gdb with GDB/MI (-interp=mi) and
! ;; access CLI using "-interpreter-exec console cli-command".
! ;;
  ;; Known Bugs:
  ;;
! ;; 1) To handle program input, if required, and to avoid extra output in the
! ;; GUD buffer you must not use run, step, next or continue etc but their MI
! ;; counterparts through gud-run, gud-step etc, e.g clicking on the appropriate
! ;; icon in the toolbar.
! ;;
! ;; 2) Some commands send extra prompts to the GUD buffer.
! ;;
! ;; TODO:
! ;; 1) Prefix MI commands with a token instead of queueing commands.
! ;; 2) Use MI command -data-read-memory for memory window.
! ;; 3) Use MI command -data-disassemble for disassembly window.
! ;; 4) Allow separate buffers for Inferior IO and GDB IO.
! ;; 5) Watch windows to work with threads.
! ;;
  ;;; Code:
  
  (require 'gud)
  (require 'gdb-ui)
! 
! (defvar gdb-register-names nil "List of register names.")
! (defvar gdb-changed-registers nil
!   "List of changed register numbers (strings).")
! (defvar gdb-last-command nil)
! (defvar gdb-prompt-name nil)
  
  ;;;###autoload
  (defun gdbmi (command-line)
*************** GUD buffer (I/O of GDB)           | Loca
*** 106,117 ****
    (set (make-local-variable 'gud-minor-mode) 'gdbmi)
    (set (make-local-variable 'gud-marker-filter) 'gud-gdbmi-marker-filter)
    ;;
    (gud-def gud-break (if (not (string-equal mode-name "Machine"))
! 			 (gud-call "-break-insert %f:%l" arg)
  		       (save-excursion
  			 (beginning-of-line)
  			 (forward-char 2)
! 			 (gud-call "-break-insert *%a" arg)))
  	   "\C-b" "Set breakpoint at current line or address.")
    ;;
    (gud-def gud-remove (if (not (string-equal mode-name "Machine"))
--- 123,145 ----
    (set (make-local-variable 'gud-minor-mode) 'gdbmi)
    (set (make-local-variable 'gud-marker-filter) 'gud-gdbmi-marker-filter)
    ;;
+   (gud-def gud-step   "-exec-step %p"              "\C-s" 
+ 	   "Step one source line with display.")
+   (gud-def gud-stepi  "-exec-step-instruction %p"  "\C-i"
+ 	   "Step one instruction with display.")
+   (gud-def gud-next   "-exec-next %p"              "\C-n"
+ 	   "Step one line (skip functions).")
+   (gud-def gud-cont   "-exec-continue"             "\C-r"
+ 	   "Continue with display.")
+   (gud-def gud-finish "-exec-finish"               "\C-f"
+ 	   "Finish executing current function.")
+   (gud-def gud-run    "-exec-run"	     nil    "Run the program.")
    (gud-def gud-break (if (not (string-equal mode-name "Machine"))
! 			 (gud-call "break %f:%l" arg)
  		       (save-excursion
  			 (beginning-of-line)
  			 (forward-char 2)
! 			 (gud-call "break *%a" arg)))
  	   "\C-b" "Set breakpoint at current line or address.")
    ;;
    (gud-def gud-remove (if (not (string-equal mode-name "Machine"))
*************** GUD buffer (I/O of GDB)           | Loca
*** 123,157 ****
  	   "\C-d" "Remove breakpoint at current line or address.")
    ;;
    (gud-def gud-until  (if (not (string-equal mode-name "Machine"))
! 			  (gud-call "until %f:%l" arg)
  			(save-excursion
  			  (beginning-of-line)
  			  (forward-char 2)
! 			  (gud-call "until *%a" arg)))
  	   "\C-u" "Continue to current line or address.")
  
    (define-key gud-minor-mode-map [left-margin mouse-1]
!     'gdb-mouse-toggle-breakpoint)
    (define-key gud-minor-mode-map [left-fringe mouse-1]
      'gdb-mouse-toggle-breakpoint)
  
    (setq comint-input-sender 'gdbmi-send)
    ;;
    ;; (re-)initialise
!   (setq gdb-main-file nil)
!   (setq gdb-current-address "main")
!   (setq gdb-previous-address nil)
!   (setq gdb-previous-frame nil)
!   (setq gdb-current-frame "main")
!   (setq gdb-view-source t)
!   (setq gdb-selected-view 'source)
!   (setq gdb-var-list nil)
!   (setq gdb-var-changed nil)
!   (setq gdb-prompting nil)
!   (setq gdb-current-item nil)
!   (setq gdb-pending-triggers nil)
!   (setq gdb-output-sink 'user)
!   (setq gdb-server-prefix nil)
    ;;
    (setq gdb-buffer-type 'gdbmi)
    ;;
--- 151,193 ----
  	   "\C-d" "Remove breakpoint at current line or address.")
    ;;
    (gud-def gud-until  (if (not (string-equal mode-name "Machine"))
! 			  (gud-call "-exec-until %f:%l" arg)
  			(save-excursion
  			  (beginning-of-line)
  			  (forward-char 2)
! 			  (gud-call "-exec-until *%a" arg)))
  	   "\C-u" "Continue to current line or address.")
  
    (define-key gud-minor-mode-map [left-margin mouse-1]
!     'gdb-mouse-set-clear-breakpoint)
    (define-key gud-minor-mode-map [left-fringe mouse-1]
+     'gdb-mouse-set-clear-breakpoint)
+   (define-key gud-minor-mode-map [left-margin mouse-3]
      'gdb-mouse-toggle-breakpoint)
  
    (setq comint-input-sender 'gdbmi-send)
    ;;
    ;; (re-)initialise
!   (setq gdb-frame-address (if gdb-show-main "main" nil)
!         gdb-previous-frame-address nil
!         gdb-memory-address "main"
!         gdb-previous-frame nil
!         gdb-selected-frame nil
!         gdb-frame-number nil
!         gdb-var-list nil
!         gdb-var-changed nil
!         gdb-prompting nil
!         gdb-input-queue nil
!         gdb-current-item nil
!         gdb-pending-triggers nil
!         gdb-output-sink 'user
!         gdb-server-prefix nil
!         gdb-flush-pending-output nil
!         gdb-location-alist nil
!         gdb-find-file-unhook nil
!         gdb-source-file-list nil
!         gdb-last-command nil
! 	gdb-prompt-name nil)
    ;;
    (setq gdb-buffer-type 'gdbmi)
    ;;
*************** GUD buffer (I/O of GDB)           | Loca
*** 160,187 ****
    ;;
    (if (eq window-system 'w32)
        (gdb-enqueue-input (list "-gdb-set new-console off\n" 'ignore)))
    ;; find source file and compilation directory here
!   (gdb-enqueue-input (list "list main\n"   'ignore))   ; C program
!   (gdb-enqueue-input (list "list MAIN__\n" 'ignore))   ; Fortran program
!   (gdb-enqueue-input (list "info source\n" 'gdbmi-source-info))
    ;;
    (run-hooks 'gdbmi-mode-hook))
  
  ; Force nil till fixed.
  (defconst gdbmi-use-inferior-io-buffer nil)
  
! ; uses --all-values Needs GDB 6.1 onwards.
  (defun gdbmi-var-list-children (varnum)
    (gdb-enqueue-input
-    (list (concat "-var-update " varnum "\n") 'ignore))
-   (gdb-enqueue-input
     (list (concat "-var-list-children --all-values "  
  		 varnum "\n")
  	     `(lambda () (gdbmi-var-list-children-handler ,varnum)))))
  
  (defconst gdbmi-var-list-children-regexp
! "name=\"\\(.*?\\)\",exp=\"\\(.*?\\)\",numchild=\"\\(.*?\\)\",value=\"\\(.*?\\)\""
! )
  
  (defun gdbmi-var-list-children-handler (varnum)
    (with-current-buffer (gdb-get-create-buffer 'gdb-partial-output-buffer)
--- 196,229 ----
    ;;
    (if (eq window-system 'w32)
        (gdb-enqueue-input (list "-gdb-set new-console off\n" 'ignore)))
+   (gdb-enqueue-input (list "-gdb-set height 0\n" 'ignore))
    ;; find source file and compilation directory here
!   (gdb-enqueue-input
!    ; Needs GDB 6.2 onwards.
!    (list "-file-list-exec-source-files\n" 'gdb-get-source-file-list))
!   (gdb-enqueue-input
!    ; Needs GDB 6.0 onwards.
!    (list "-file-list-exec-source-file\n" 'gdb-get-source-file))
!   (gdb-enqueue-input
!    (list "-data-list-register-names\n" 'gdb-get-register-names))
!   (gdb-enqueue-input
!    (list "-gdb-show prompt\n" 'gdb-get-prompt))
    ;;
    (run-hooks 'gdbmi-mode-hook))
  
  ; Force nil till fixed.
  (defconst gdbmi-use-inferior-io-buffer nil)
  
! ; Uses "-var-list-children --all-values".  Needs GDB 6.1 onwards.
  (defun gdbmi-var-list-children (varnum)
    (gdb-enqueue-input
     (list (concat "-var-list-children --all-values "  
  		 varnum "\n")
  	     `(lambda () (gdbmi-var-list-children-handler ,varnum)))))
  
  (defconst gdbmi-var-list-children-regexp
! "name=\"\\(.*?\\)\",exp=\"\\(.*?\\)\",numchild=\"\\(.*?\\)\",\
! value=\"\\(.*?\\)\"")
  
  (defun gdbmi-var-list-children-handler (varnum)
    (with-current-buffer (gdb-get-create-buffer 'gdb-partial-output-buffer)
*************** GUD buffer (I/O of GDB)           | Loca
*** 208,256 ****
  	   (push var var-list)))
         (setq gdb-var-changed t)
         (setq gdb-var-list (nreverse var-list))))))
- \f
- ;(defun gdbmi-send (proc string)
- ;  "A comint send filter for gdb."
- ;  (setq gdb-output-sink 'user)
- ;  (setq gdb-prompting nil)
- ;  (process-send-string proc (concat "-interpreter-exec console \"" string "\"")))
  
  (defun gdbmi-send (proc string)
    "A comint send filter for gdb."
!   (setq gdb-output-sink 'user)
!   (setq gdb-prompting nil)
!   (process-send-string proc (concat string "\n")))
  
! (defcustom gud-gdbmi-command-name "~/gdb/gdb/gdb -interp=mi"
    "Default command to execute an executable under the GDB-UI debugger."
    :type 'string
    :group 'gud)
  
! (defconst gdb-stopped-regexp 
!   "\\((gdb) \n\\*stopped\\|^\\^done\\),reason=.*,file=\"\\(.*\\)\",line=\"\\(.*\\)\".*")
  
! (defconst gdb-console-regexp "~\"\\(.*\\)\\\\n\"")
  
! (defconst gdb-internals-regexp "&\".*\\n\"\n")
  
! (defconst gdb-gdb-regexp "(gdb) \n")
  
! (defconst gdb-running-regexp "^\\^running")
  
! (defun gdbmi-prompt ()
!   "This handler terminates the any collection of output. It also
!   sends the next command (if any) to gdb."
    (unless gdb-pending-triggers
! 	(gdb-get-current-frame)
! 	(gdbmi-invalidate-frames)
! 	(gdbmi-invalidate-breakpoints)
! 	(gdbmi-invalidate-locals)
! 	(dolist (frame (frame-list))
! 	  (when (string-equal (frame-parameter frame 'name) "Speedbar")
! 	    (setq gdb-var-changed t)    ; force update
! 	    (dolist (var gdb-var-list)
! 	      (setcar (nthcdr 5 var) nil))))
! 	(gdb-var-update))
    (let ((sink gdb-output-sink))
      (when (eq sink 'emacs)
        (let ((handler
--- 250,342 ----
  	   (push var var-list)))
         (setq gdb-var-changed t)
         (setq gdb-var-list (nreverse var-list))))))
  
+ ; Uses "-var-update --all-values".  Needs CVS GDB (6.4+).
+ (defun gdbmi-var-update ()
+   (gdb-enqueue-input
+    (list "-var-update --all-values *\n" 'gdbmi-var-update-handler)))
+ 
+ (defconst gdbmi-var-update-regexp "name=\"\\(.*?\\)\",value=\"\\(.*?\\)\"")
+ 
+ (defun gdbmi-var-update-handler ()
+   (with-current-buffer (gdb-get-create-buffer 'gdb-partial-output-buffer)
+     (goto-char (point-min))
+     (while (re-search-forward gdbmi-var-update-regexp nil t)
+ 	(let ((varnum (match-string 1)))
+ 	  (catch 'var-found1
+ 	    (let ((num 0))
+ 	      (dolist (var gdb-var-list)
+ 		(if (string-equal varnum (cadr var))
+ 		    (progn
+ 		      (setcar (nthcdr 5 var) t)
+ 		      (setcar (nthcdr 4 var) (match-string 2))
+ 		      (setcar (nthcdr num gdb-var-list) var)
+ 		      (throw 'var-found1 nil)))
+ 		(setq num (+ num 1))))))
+ 	(setq gdb-var-changed t))))
+ \f
  (defun gdbmi-send (proc string)
    "A comint send filter for gdb."
!   (if gud-running
!       (process-send-string proc (concat string "\n"))
!     (with-current-buffer gud-comint-buffer
!       (remove-text-properties (point-min) (point-max) '(face)))
!     (setq gdb-output-sink 'user)
!     (setq gdb-prompting nil)
!     ;; mimic <RET> key to repeat previous command in GDB
!     (if (string-match "\\S+" string)
! 	(setq gdb-last-command string)
!       (if gdb-last-command (setq string gdb-last-command)))
!     (if gdb-enable-debug-log 
! 	(push (cons 'mi-send (concat string "\n")) gdb-debug-log))
!     (process-send-string
!      proc
!      (if (string-match "^-" string)
! 	 ;; MI command
! 	 (concat string "\n")
!        ;; CLI command
!        (concat "-interpreter-exec console \"" string "\"\n")))))
  
! (defcustom gud-gdbmi-command-name "gdb -interp=mi"
    "Default command to execute an executable under the GDB-UI debugger."
    :type 'string
    :group 'gud)
  
! (defconst gdb-gdb-regexp "(gdb) \n")
  
! (defconst gdb-running-regexp (concat "\\^running\n" gdb-gdb-regexp))
  
! ;; fullname added GDB 6.4+.
! ;; Probably not needed. -stack-info-frame computes filename and line.
! (defconst gdb-stopped-regexp
!   "\\*stopped,reason=.*?,file=\".*?\"\
! ,fullname=\"\\(.*?\\)\",line=\"\\(.*?\\)\"}\n")
  
! (defconst gdb-error-regexp "\\^error,msg=\\(\".+\"\\)\n")
  
! (defconst gdb-done-regexp "\\^done,*\n*")
  
! (defconst gdb-console-regexp "~\\(\".*?[^\\]\"\\)\n")
! 
! (defconst gdb-internals-regexp "&\\(\".*?\\n\"\\)\n")
! 
! (defun gdbmi-prompt1 ()
!   "Queue any GDB commands that the user interface needs."
    (unless gdb-pending-triggers
!     (when (and (boundp 'speedbar-frame) (frame-live-p speedbar-frame))
!       (setq gdb-var-changed t)   ; force update
!       (dolist (var gdb-var-list)
! 	(setcar (nthcdr 5 var) nil))
!       (gdbmi-var-update))
!     (gdbmi-get-selected-frame)
!     (gdbmi-invalidate-frames)
!     (gdbmi-invalidate-breakpoints)
!     (gdb-get-changed-registers)
!     (gdbmi-invalidate-registers)
!     (gdbmi-invalidate-locals)))
! 
! (defun gdbmi-prompt2 ()
!   "Handle any output and send next GDB command."
    (let ((sink gdb-output-sink))
      (when (eq sink 'emacs)
        (let ((handler
*************** GUD buffer (I/O of GDB)           | Loca
*** 267,325 ****
  
  (defun gud-gdbmi-marker-filter (string)
    "Filter GDB/MI output."
!   (if gdb-enable-debug-log (push (cons 'recv string) gdb-debug-log))
!   ;; Recall the left over gud-marker-acc from last time
!   (setq gud-marker-acc (concat gud-marker-acc string))
!   ;; Start accumulating output for the GUD buffer
!   (let ((output ""))
! 
!     (if (string-match gdb-running-regexp gud-marker-acc) 
!        (setq gud-marker-acc (substring gud-marker-acc (match-end 0))
! 	     gud-running t))
! 
!     ;; Remove the trimmings from the console stream.
!     (while (string-match gdb-console-regexp gud-marker-acc) 
!        (setq 
! 	gud-marker-acc (concat (substring gud-marker-acc 0 (match-beginning 0))
! 			       (match-string 1 gud-marker-acc)
! 			       (substring gud-marker-acc (match-end 0)))))
! 
!     ;; Remove log stream containing debugging messages being produced by GDB's
!     ;; internals.
!     (while (string-match gdb-internals-regexp gud-marker-acc) 
!        (setq 
! 	 gud-marker-acc (concat (substring gud-marker-acc 0 (match-beginning 0))
! 				(substring gud-marker-acc (match-end 0)))))
! 
!     (if (string-match gdb-stopped-regexp gud-marker-acc)
!       (setq
! 
!        ;; Extract the frame position from the marker.
!        gud-last-frame (cons (match-string 2 gud-marker-acc)
! 			    (string-to-int (match-string 3 gud-marker-acc)))
! 
!        ;; Append any text before the marker to the output we're going
!        ;; to return - we don't include the marker in this text.
!        output (gdbmi-concat-output output
! 		      (substring gud-marker-acc 0 (match-beginning 0)))
! 
!        ;; Set the accumulator to the remaining text.
!        gud-marker-acc (substring gud-marker-acc (match-end 0))))
!       
!     (while (string-match gdb-gdb-regexp gud-marker-acc) 
!       (setq
! 
!        ;; Append any text up to and including prompt less \n to the output.
!        output (gdbmi-concat-output output
! 		      (substring gud-marker-acc 0 (- (match-end 0) 1)))
! 
!        ;; Set the accumulator to the remaining text.
!        gud-marker-acc (substring gud-marker-acc (match-end 0)))
!       (gdbmi-prompt))
! 
!     (setq output (gdbmi-concat-output output gud-marker-acc))
!     (setq gud-marker-acc "")
!     output))
  
  (defun gdbmi-concat-output (so-far new)
    (let ((sink gdb-output-sink))
--- 353,443 ----
  
  (defun gud-gdbmi-marker-filter (string)
    "Filter GDB/MI output."
!   (if gdb-flush-pending-output
!       nil
!     (if gdb-enable-debug-log (push (cons 'recv (list string gdb-output-sink))
! 					 gdb-debug-log))
!     ;; Recall the left over gud-marker-acc from last time
!     (setq gud-marker-acc (concat gud-marker-acc string))
!     ;; Start accumulating output for the GUD buffer
!     (let ((output ""))
! 
!       (if (string-match gdb-running-regexp gud-marker-acc) 
! 	  (setq
! 	   gud-marker-acc
! 	   (concat (substring gud-marker-acc 0 (match-beginning 0))
! 		   (substring gud-marker-acc (match-end 0)))
! 		gud-running t))
! 
!       (if (string-match gdb-stopped-regexp gud-marker-acc)
! 	  (setq
! 
! 	   ;; Extract the frame position from the marker.
! 	   gud-last-frame (cons (match-string 1 gud-marker-acc)
! 				(string-to-number
! 				 (match-string 2 gud-marker-acc)))
! 
! 	   gud-marker-acc
! 	   (concat (substring gud-marker-acc 0 (match-beginning 0))
! 		   (substring gud-marker-acc (match-end 0)))))
! 
!       ;; Filter error messages going to GUD buffer and
!       ;; display in minibuffer.
!       (if (eq gdb-output-sink 'user)
! 	  (while (string-match gdb-error-regexp gud-marker-acc)
! 	    (message (read (match-string 1 gud-marker-acc)))
! 	    (setq 
! 	     gud-marker-acc
! 	     (concat (substring gud-marker-acc 0 (match-beginning 0))
! 		     (substring gud-marker-acc (match-end 0))))))
! 
!       (if (string-match gdb-done-regexp gud-marker-acc)
! 	  (setq 
! 	   gud-marker-acc
! 	   (concat (substring gud-marker-acc 0 (match-beginning 0))
! 		   (substring gud-marker-acc (match-end 0)))))
! 
!       (when (string-match gdb-gdb-regexp gud-marker-acc)
! 	(setq 
! 	 gud-marker-acc
! 	 (concat (substring gud-marker-acc 0 (match-beginning 0))
! 		   (substring gud-marker-acc (match-end 0))))
! 
! 	;; Remove the trimmings from the console stream.
! 	(while (string-match gdb-console-regexp gud-marker-acc) 
! 	  (setq 
! 	   gud-marker-acc (concat
! 			   (substring gud-marker-acc 0 (match-beginning 0))
! 			   (read (match-string 1 gud-marker-acc))
! 			   (substring gud-marker-acc (match-end 0)))))
! 
! 	;; Remove the trimmings from log stream containing debugging messages
! 	;; being produced by GDB's internals and use warning face.
! 	(while (string-match gdb-internals-regexp gud-marker-acc) 
! 	  (setq 
! 	   gud-marker-acc
! 	   (concat (substring gud-marker-acc 0 (match-beginning 0))
! 		   (let ((error-message
! 			  (read (match-string 1 gud-marker-acc))))
! 		     (put-text-property
! 		      0 (length error-message)
! 		      'face font-lock-warning-face
! 		      error-message)
! 		     error-message)
! 		   (substring gud-marker-acc (match-end 0)))))
! 
! 	(setq output (gdbmi-concat-output output gud-marker-acc))
! 	(setq gud-marker-acc "")
! 	(gdbmi-prompt1)
! 	(unless gdb-input-queue
! 	  (setq output (concat output gdb-prompt-name)))
! 	(gdbmi-prompt2))
! 
!       (when gud-running
! 	(setq output (gdbmi-concat-output output gud-marker-acc))
! 	(setq gud-marker-acc ""))
! 
!        output)))
  
  (defun gdbmi-concat-output (so-far new)
    (let ((sink gdb-output-sink))
*************** GUD buffer (I/O of GDB)           | Loca
*** 354,366 ****
    gdb-break-list-custom)
  
  (defconst gdb-break-list-regexp
! "number=\"\\(.*?\\)\",type=\"\\(.*?\\)\",disp=\"\\(.*?\\)\",enabled=\"\\(.\\)\",addr=\"\\(.*?\\)\",func=\"\\(.*?\\)\",file=\"\\(.*?\\)\",line=\"\\(.*?\\)\"")
  
  (defun gdb-break-list-handler ()
    (setq gdb-pending-triggers (delq 'gdbmi-invalidate-breakpoints
  				  gdb-pending-triggers))
!   (let ((breakpoint nil)
! 	(breakpoints-list nil))
      (with-current-buffer (gdb-get-create-buffer 'gdb-partial-output-buffer)
        (goto-char (point-min))
        (while (re-search-forward gdb-break-list-regexp nil t)
--- 472,484 ----
    gdb-break-list-custom)
  
  (defconst gdb-break-list-regexp
! "number=\"\\(.*?\\)\",type=\"\\(.*?\\)\",disp=\"\\(.*?\\)\",enabled=\"\\(.\\)\",\
! addr=\"\\(.*?\\)\",func=\"\\(.*?\\)\",file=\"\\(.*?\\)\",line=\"\\(.*?\\)\"")
  
  (defun gdb-break-list-handler ()
    (setq gdb-pending-triggers (delq 'gdbmi-invalidate-breakpoints
  				  gdb-pending-triggers))
!   (let ((breakpoint) (breakpoints-list))
      (with-current-buffer (gdb-get-create-buffer 'gdb-partial-output-buffer)
        (goto-char (point-min))
        (while (re-search-forward gdb-break-list-regexp nil t)
*************** GUD buffer (I/O of GDB)           | Loca
*** 393,399 ****
  
  ;;-put breakpoint icons in relevant margins (even those set in the GUD buffer)
  (defun gdb-break-list-custom ()
!   (let ((flag)(address))
      ;;
      ;; remove all breakpoint-icons in source buffers but not assembler buffer
      (dolist (buffer (buffer-list))
--- 511,517 ----
  
  ;;-put breakpoint icons in relevant margins (even those set in the GUD buffer)
  (defun gdb-break-list-custom ()
!   (let ((flag) (bptno))
      ;;
      ;; remove all breakpoint-icons in source buffers but not assembler buffer
      (dolist (buffer (buffer-list))
*************** GUD buffer (I/O of GDB)           | Loca
*** 406,434 ****
  	(goto-char (point-min))
  	(while (< (point) (- (point-max) 1))
  	  (forward-line 1)
! 	  (if (looking-at "[0-9]*\\s-*\\S-*\\s-*\\S-*\\s-*\\(.\\)\\s-*\\S-*\\s-*\\(\\S-*\\):\\([0-9]+\\)")
  	      (progn
! 		(setq flag (char-after (match-beginning 1)))
! 		(let ((line (match-string 3)) (buffer-read-only nil)
! 		      (file (match-string 2)))
  		  (add-text-properties (point-at-bol) (point-at-eol)
! 				       '(mouse-face highlight
! 						    help-echo "mouse-2, RET: visit breakpoint"))
! 		  (with-current-buffer
! 		      (find-file-noselect
! 		       (if (file-exists-p file) file
! 			 (expand-file-name file gdb-cdir)))
! 		    (save-current-buffer
! 		      (set (make-local-variable 'gud-minor-mode) 'gdbmi)
! 		      (set (make-local-variable 'tool-bar-map)
! 			   gud-tool-bar-map))
! 		    ;; only want one breakpoint icon at each location
! 		    (save-excursion
! 		      (goto-line (string-to-number line))
! 		      (gdb-put-breakpoint-icon (eq flag ?y)))))))))
! 	  (end-of-line)))
    (if (gdb-get-buffer 'gdb-assembler-buffer) (gdb-assembler-custom)))
  
  ;; Frames buffer.  This displays a perpetually correct bactrack trace.
  ;;
  (def-gdb-auto-updated-buffer gdb-stack-buffer
--- 524,588 ----
  	(goto-char (point-min))
  	(while (< (point) (- (point-max) 1))
  	  (forward-line 1)
! 	  (if (looking-at
! 	       "\\([0-9]+\\)\\s-+\\S-+\\s-+\\S-+\\s-+\\(.\\)\\s-+\\S-+\\s-+\
! \\(\\S-+\\):\\([0-9]+\\)")
  	      (progn
! 		(setq bptno (match-string 1))
! 		(setq flag (char-after (match-beginning 2)))
! 		(let ((line (match-string 4)) (buffer-read-only nil)
! 		      (file (match-string 3)))
  		  (add-text-properties (point-at-bol) (point-at-eol)
! 		   '(mouse-face highlight
! 		     help-echo "mouse-2, RET: visit breakpoint"))
! 			(unless (file-exists-p file)
! 			   (setq file (cdr (assoc bptno gdb-location-alist))))
! 			(if (and file
! 				 (not (string-equal file "File not found")))
! 			    (with-current-buffer (find-file-noselect file)
! 			      (set (make-local-variable 'gud-minor-mode)
! 				   'gdbmi)
! 			      (set (make-local-variable 'tool-bar-map)
! 				   gud-tool-bar-map)
! 			      ;; only want one breakpoint icon at each location
! 			      (save-excursion
! 				(goto-line (string-to-number line))
! 				(gdb-put-breakpoint-icon (eq flag ?y) bptno)))
! 			  (gdb-enqueue-input
! 			   (list (concat "list "
! 					 (match-string-no-properties 3) ":1\n")
! 				 'ignore))
! 			  (gdb-enqueue-input
! 			   (list "-file-list-exec-source-file\n"
! 				 `(lambda () (gdbmi-get-location
! 					      ,bptno ,line ,flag))))))))))
!       (end-of-line)))
    (if (gdb-get-buffer 'gdb-assembler-buffer) (gdb-assembler-custom)))
  
+ (defun gdbmi-get-location (bptno line flag)
+   "Find the directory containing the relevant source file.
+ Put in buffer and place breakpoint icon."
+   (goto-char (point-min))
+   (catch 'file-not-found
+     (if (re-search-forward gdb-source-file-regexp nil t)
+ 	(delete (cons bptno "File not found") gdb-location-alist)
+ 	(push (cons bptno (match-string 1)) gdb-location-alist)
+       (gdb-resync)
+       (unless (assoc bptno gdb-location-alist)
+ 	(push (cons bptno "File not found") gdb-location-alist)
+ 	(message-box "Cannot find source file for breakpoint location.
+ Add directory to search path for source files using the GDB command, dir."))
+       (throw 'file-not-found nil))
+     (with-current-buffer
+ 	(find-file-noselect (match-string 1))
+       (save-current-buffer
+ 	(set (make-local-variable 'gud-minor-mode) 'gdbmi)
+ 	(set (make-local-variable 'tool-bar-map) gud-tool-bar-map))
+       ;; only want one breakpoint icon at each location
+       (save-excursion
+ 	(goto-line (string-to-number line))
+ 	(gdb-put-breakpoint-icon (eq flag ?y) bptno)))))
+ 
  ;; Frames buffer.  This displays a perpetually correct bactrack trace.
  ;;
  (def-gdb-auto-updated-buffer gdb-stack-buffer
*************** GUD buffer (I/O of GDB)           | Loca
*** 438,444 ****
    gdb-stack-list-frames-custom)
  
  (defconst gdb-stack-list-frames-regexp
! "level=\"\\(.*?\\)\",addr=\"\\(.*?\\)\",func=\"\\(.*?\\)\",file=\"\\(.*?\\)\",line=\"\\(.*?\\)\"")
  
  (defun gdb-stack-list-frames-handler ()
    (setq gdb-pending-triggers (delq 'gdbmi-invalidate-frames
--- 592,599 ----
    gdb-stack-list-frames-custom)
  
  (defconst gdb-stack-list-frames-regexp
! "level=\"\\(.*?\\)\",addr=\"\\(.*?\\)\",func=\"\\(.*?\\)\",\
! file=\".*?\",fullname=\"\\(.*?\\)\",line=\"\\(.*?\\)\"")
  
  (defun gdb-stack-list-frames-handler ()
    (setq gdb-pending-triggers (delq 'gdbmi-invalidate-frames
*************** GUD buffer (I/O of GDB)           | Loca
*** 480,497 ****
  			     '(mouse-face highlight
  			       help-echo "mouse-2, RET: Select frame"))
  	  (beginning-of-line)
! 	  (when (and (or (looking-at "^#[0-9]*\\s-*\\S-* in \\(\\S-*\\)")
! 			 (looking-at "^#[0-9]*\\s-*\\(\\S-*\\)"))
! 		     (equal (match-string 1) gdb-current-frame))
  	    (put-text-property (point-at-bol) (point-at-eol)
  			       'face '(:inverse-video t)))
  	  (forward-line 1))))))
  
  ;; Locals buffer.
! ;; uses "-stack-list-locals 2". Needs GDB 6.1 onwards.
  (def-gdb-auto-updated-buffer gdb-locals-buffer
    gdbmi-invalidate-locals
!   "-stack-list-locals 2\n"
    gdb-stack-list-locals-handler
    gdb-stack-list-locals-custom)
  
--- 635,651 ----
  			     '(mouse-face highlight
  			       help-echo "mouse-2, RET: Select frame"))
  	  (beginning-of-line)
! 	  (when (and (looking-at "^[0-9]+\\s-+\\(\\S-+\\)")
! 		     (equal (match-string 1) gdb-selected-frame))
  	    (put-text-property (point-at-bol) (point-at-eol)
  			       'face '(:inverse-video t)))
  	  (forward-line 1))))))
  
  ;; Locals buffer.
! ;; uses "-stack-list-locals --simple-values". Needs GDB 6.1 onwards.
  (def-gdb-auto-updated-buffer gdb-locals-buffer
    gdbmi-invalidate-locals
!   "-stack-list-locals --simple-values\n"
    gdb-stack-list-locals-handler
    gdb-stack-list-locals-custom)
  
*************** GUD buffer (I/O of GDB)           | Loca
*** 516,522 ****
  	(push local locals-list))))
      (let ((buf (gdb-get-buffer 'gdb-locals-buffer)))
        (and buf (with-current-buffer buf
! 		 (let ((p (point))
  		       (buffer-read-only nil))
  		   (erase-buffer)
  		   (dolist (local locals-list)
--- 670,676 ----
  	(push local locals-list))))
      (let ((buf (gdb-get-buffer 'gdb-locals-buffer)))
        (and buf (with-current-buffer buf
! 		 (let ((p (window-point (get-buffer-window buf 0)))
  		       (buffer-read-only nil))
  		   (erase-buffer)
  		   (dolist (local locals-list)
*************** GUD buffer (I/O of GDB)           | Loca
*** 527,568 ****
  				      "(structure)"
  				    "(array)"))
  			      "\n")))
! 		   (goto-char p)))))))
  
  (defun gdb-stack-list-locals-custom ()
    nil)
  
! (defun gdbmi-source-info ()
!   "Find the source file where the program starts and displays it with related
! buffers."
    (goto-char (point-min))
!   (if (search-forward "source file is " nil t)
!       (if (looking-at "\\S-*")
! 	  (setq gdb-main-file (match-string 0)))
!     (setq gdb-view-source nil))
!   (if (search-forward "directory is " nil t)
!       (if (looking-at "\\S-*:\\(\\S-*\\)")
! 	  (setq gdb-cdir (match-string 1))
! 	(looking-at "\\S-*")
! 	(setq gdb-cdir (match-string 0))))
! 
! ;temporary heuristic
!   (if gdb-main-file
!       (setq gdb-main-file (expand-file-name gdb-main-file gdb-cdir)))
  
!   (if gdb-many-windows
        (gdb-setup-windows)
!     (gdb-get-create-buffer 'gdb-breakpoints-buffer)
!     (when gdb-show-main
!       (switch-to-buffer gud-comint-buffer)
!       (delete-other-windows)
!       (split-window)
!       (other-window 1)
!       (switch-to-buffer
!        (if gdb-view-source
! 	   (gud-find-file gdb-main-file)
! 	 (gdb-get-create-buffer 'gdb-assembler-buffer)))
!       (other-window 1))))
  
  (provide 'gdb-mi)
  ;;; gdbmi.el ends here
--- 681,828 ----
  				      "(structure)"
  				    "(array)"))
  			      "\n")))
! 		   (set-window-point (get-buffer-window buf 0) p)))))))
  
  (defun gdb-stack-list-locals-custom ()
    nil)
  
! \f
! ;; Registers buffer.
! ;;
! (def-gdb-auto-updated-buffer gdb-registers-buffer
!   gdbmi-invalidate-registers
!   "-data-list-register-values x\n"
!   gdb-data-list-register-values-handler
!   gdb-data-list-register-values-custom)
! 
! (defconst gdb-data-list-register-values-regexp
!   "number=\"\\(.*?\\)\",value=\"\\(.*?\\)\"")
! 
! (defun gdb-data-list-register-values-handler ()
!   (setq gdb-pending-triggers (delq 'gdbmi-invalidate-registers
! 				   gdb-pending-triggers))
!   (with-current-buffer (gdb-get-create-buffer 'gdb-partial-output-buffer)
!     (goto-char (point-min))
!     (if (re-search-forward gdb-error-regexp nil t)
! 	(progn
! 	  (let ((match nil))
! 	    (setq match (match-string 1))
! 	    (with-current-buffer (gdb-get-buffer 'gdb-registers-buffer)
! 	      (let ((buffer-read-only nil))
! 		(erase-buffer)
! 		(insert match)
! 		(goto-char (point-min))))))
!       (let ((register-list (reverse gdb-register-names))
! 	    (register nil) (register-string nil) (register-values nil))
! 	(goto-char (point-min))
! 	(while (re-search-forward gdb-data-list-register-values-regexp nil t)
! 	  (setq register (pop register-list))
! 	  (setq register-string (concat register "\t" (match-string 2) "\n"))
! 	  (if (member (match-string 1) gdb-changed-registers)
! 	      (put-text-property 0 (length register-string)
! 				 'face 'font-lock-warning-face
! 				 register-string))
! 	  (setq register-values
! 		(concat register-values register-string)))
! 	(let ((buf (gdb-get-buffer 'gdb-registers-buffer)))
! 	  (with-current-buffer buf
! 	    (let ((p (window-point (get-buffer-window buf 0)))
! 		  (buffer-read-only nil))
! 	      (erase-buffer)
! 	      (insert register-values)
! 	      (set-window-point (get-buffer-window buf 0) p))))))))
! 
! (defun gdb-data-list-register-values-custom ())
! 
! (defun gdb-get-changed-registers ()
!   (if (and (gdb-get-buffer 'gdb-registers-buffer)
! 	   (not (member 'gdb-get-changed-registers gdb-pending-triggers)))
!       (progn
! 	(gdb-enqueue-input
! 	 (list
! 	  "-data-list-changed-registers\n"
! 	  'gdb-get-changed-registers-handler))
! 	(push 'gdb-get-changed-registers gdb-pending-triggers))))
! 
! (defun gdb-get-changed-registers-handler ()
!   (setq gdb-pending-triggers
! 	(delq 'gdb-get-changed-registers gdb-pending-triggers))
!   (setq gdb-changed-registers nil)
!   (with-current-buffer (gdb-get-create-buffer 'gdb-partial-output-buffer)
!     (goto-char (point-min))
!     (while (re-search-forward gdb-data-list-register-names-regexp nil t)
!       (push (match-string 1) gdb-changed-registers))))
! 
! 
! (defconst gdb-data-list-register-names-regexp "\"\\(.*?\\)\"")
! 
! (defun gdb-get-register-names ()
!   "Create a list of register names."
    (goto-char (point-min))
!   (setq gdb-register-names nil)
!   (while (re-search-forward gdb-data-list-register-names-regexp nil t)
!     (push (match-string 1) gdb-register-names)))
! \f
! ;; these functions/variables may go into gdb-ui.el in the near future
! ;; (from gdb-nui.el)
! 
! (defvar gdb-source-file-list nil)
! (defvar gdb-source-file-regexp "fullname=\"\\(.*?\\)\"")
  
! (defun gdb-get-source-file ()
!   "Find the source file where the program starts and display it with related
! buffers, if required."
!   (goto-char (point-min))
!   (if (re-search-forward gdb-source-file-regexp nil t)
!       (setq gdb-main-file (match-string 1)))
!  (if gdb-many-windows
        (gdb-setup-windows)
!    (gdb-get-create-buffer 'gdb-breakpoints-buffer)
!    (if gdb-show-main
!        (let ((pop-up-windows t))
! 	 (display-buffer (gud-find-file gdb-main-file))))))
! 
! (defun gdb-get-source-file-list ()
!   "Create list of source files for current GDB session."
!   (goto-char (point-min))
!   (while (re-search-forward gdb-source-file-regexp nil t)
!     (push (match-string 1) gdb-source-file-list)))
  
+ (defun gdbmi-get-selected-frame ()
+   (if (not (member 'gdbmi-get-selected-frame gdb-pending-triggers))
+       (progn
+ 	(gdb-enqueue-input
+ 	 (list "-stack-info-frame\n" 'gdbmi-frame-handler))
+ 	(push 'gdbmi-get-selected-frame
+ 	       gdb-pending-triggers))))
+ 
+ (defun gdbmi-frame-handler ()
+   (setq gdb-pending-triggers
+ 	(delq 'gdbmi-get-selected-frame gdb-pending-triggers))
+   (with-current-buffer (gdb-get-create-buffer 'gdb-partial-output-buffer)
+     (goto-char (point-min))
+     (when (re-search-forward gdb-stack-list-frames-regexp nil t)
+       (setq gdb-frame-number (match-string 1))
+       (setq gdb-frame-address (match-string 2))
+       (setq gdb-selected-frame (match-string 3))
+       (setq gud-last-frame
+ 	    (cons (match-string 4) (string-to-number (match-string 5))))
+       (gud-display-frame)
+       (if (gdb-get-buffer 'gdb-locals-buffer)
+ 	  (with-current-buffer (gdb-get-buffer 'gdb-locals-buffer)
+ 	    (setq mode-name (concat "Locals:" gdb-selected-frame))))
+       (if (gdb-get-buffer 'gdb-assembler-buffer)
+ 	  (with-current-buffer (gdb-get-buffer 'gdb-assembler-buffer)
+ 	    (setq mode-name (concat "Machine:" gdb-selected-frame)))))))
+ 
+ (defvar gdb-prompt-name-regexp "value=\"\\(.*?\\)\"")
+ 
+ (defun gdb-get-prompt ()
+   "Find prompt for GDB session."
+   (goto-char (point-min))
+   (setq gdb-prompt-name nil)
+   (re-search-forward gdb-prompt-name-regexp nil t)
+   (setq gdb-prompt-name (match-string 1)))
+ 	       
  (provide 'gdb-mi)
  ;;; gdbmi.el ends here

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

[Daniel Jacobowitz]

On Sat, Jul 16, 2005 at 03:12:00AM +1200, Nick Roberts wrote:
>  > BTW, if it's OK with you, I would prefer that you add new testcases
>  > rather than modifying existing ones.  Yes, the ones that are there are
>  > slightly redundant.  But changing what a test case is testing is bad
>  > form for long-term results analysis.
> 
> OK.  I'm not very comfortable with expect, but I'll give it a go.

I have learned a frightening (to me anyway) amount about expect and TCL
lately; bug me with any questions you may have.

>  > Now that this patch is in, I believe your current gdb-mi.el will work
>  > with CVS GDB.  Is that right?  If so, could you post it?  I promise to
>  > look at it promptly.
> 
> Yes it does work, but not perfectly (if it did it could go straight into
> the Emacs CVS repository).  It has changed quite a lot since last time.
> Unfortunately it still requires Emacs in CVS, but a release is planned.

Confused - I thought we'd decided it belonged with GDB, not with
emacs, because of the GDB version dependencies?

I won't try to second-guess you about this code - my LISP sucks.  But
could you explain briefly why you changed from -break-insert to "break"
in some places?  Yet at the same time changed from until to
-exec-until...

Anyway, go ahead and check this in.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

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

[Nick Roberts]

Daniel Jacobowitz writes:
 > On Sat, Jul 16, 2005 at 03:12:00AM +1200, Nick Roberts wrote:
 > >  > BTW, if it's OK with you, I would prefer that you add new testcases
 > >  > rather than modifying existing ones.  Yes, the ones that are there are
 > >  > slightly redundant.  But changing what a test case is testing is bad
 > >  > form for long-term results analysis.
 > > 
 > > OK.  I'm not very comfortable with expect, but I'll give it a go.
 > 
 > I have learned a frightening (to me anyway) amount about expect and TCL
 > lately; bug me with any questions you may have.

Thanks.

 > >  > Now that this patch is in, I believe your current gdb-mi.el will work
 > >  > with CVS GDB.  Is that right?  If so, could you post it?  I promise to
 > >  > look at it promptly.
 > > 
 > > Yes it does work, but not perfectly (if it did it could go straight into
 > > the Emacs CVS repository).  It has changed quite a lot since last time.
 > > Unfortunately it still requires Emacs in CVS, but a release is planned.
 > 
 > Confused - I thought we'd decided it belonged with GDB, not with
 > emacs, because of the GDB version dependencies?

The eventual aim is to move it to Emacs CVS.  While MI and gdb-mi.el are
changing , GDB CVS seems a good place for it.  You're right though, since
it currently requires MI features that are only in GDB CVS, it wouldn't
make sense to move it to Emacs CVS yet.

 > I won't try to second-guess you about this code - my LISP sucks.  But
 > could you explain briefly why you changed from -break-insert to "break"
 > in some places?  Yet at the same time changed from until to
 > -exec-until...

I changed "until" to -exec-until as it's an MI command.  I changed
-break-insert back to "break" because the output from -break-insert spills
into the GUD buffer.  Clearly the proper solution is to handle the output
of -break-insert, but I haven't got that far yet.  I don't know how
important that is because gdb-mi.el will always have to be able to handle
CLI user input from the GUD buffer anyway.

 > Anyway, go ahead and check this in.

Committed.  Thanks.

Nick