Re: PATCH (gdb/mi)

Re: PATCH (gdb/mi)

[Nick Roberts]

> > Eli will probably tell you that you need to update the MI doc's.

> Thanks, that's a good catch.  Yes, I should have told Nick that when
> I saw the patch.

I'll gladly do this but I thought that the patch needs to be approved first.
Secondly, I have signed no copyright assignment for GDB (maybe this change is
small enough not to need it). Finally the CONTRIBUTE file doesn't ask for
documentation to be included when submitting a patch.

Perhaps I've used the wrong subject header and it looks like the patch has
already been committed. My impression now is that:

RFC is for maintainers who ask for comments before committing their own patch.
RFA is for those with write after approval.
commit is for a patch that has been committed.

and, rather oddly

PATCH seems, generally, to be for a commit also.

Most people who post to this list have some kind of write access to the
repository. What subject header should someone without write authority use
when submitting a patch? Some projects have a patch database as well as one
for bugs. Would this be a good idea for GDB?

Nick

Re: PATCH (gdb/mi)

[Eli Zaretskii]

> From: Nick Roberts <nick@nick.uklinux.net>
> Date: Tue, 28 Oct 2003 01:15:08 +0000
> 
> I'll gladly do this but I thought that the patch needs to be approved first.

That's fine with me: you don't need to waste your time working on the
docs if you aren't sure the code will be accepted.  Just don't forget
to add the docs once the code is approved.

> Finally the CONTRIBUTE file doesn't ask for documentation to be
> included when submitting a patch.

It's been suggested to add such a requirement (and even remove
undocumented features already present in the code base), but I
personally never had the resolve to actually do that.  It sounds too
drastic a measure.

> Perhaps I've used the wrong subject header and it looks like the patch has
> already been committed. My impression now is that:
> 
> RFC is for maintainers who ask for comments before committing their own patch.
> RFA is for those with write after approval.
> commit is for a patch that has been committed.

That's not what I know.  RFC is for any contributor who is not sure
the concept she is proposing is valid.  RFA is for when there are no
doubts about the concept, but the actual implementation needs an
approval.

> PATCH seems, generally, to be for a commit also.

That should be COMMIT, I think.

> Most people who post to this list have some kind of write access to the
> repository.

That might be true, but I don't think that's a requirement.  Anyone
could use those categories, AFAIK.

PATCH (gdb/mi)

[Jim Ingham]

Nick,

Yes, this is what our local version does for the -var-list-children 
command ('cept we call the variable print_value not values...), and it 
seems to work well.

Eli will probably tell you that you need to update the MI doc's.

Jim

On Oct 25, 2003, at 9:32 AM, gdb-patches-digest-help@sources.redhat.com 
wrote:

> Purpose:
>
> With variable objects, the value of array elements and structure 
> members 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
> each element/member. This patch adapts the MI command 
> -var-list-children so
> that all the values can be accessed at once allowing the display of an 
> array
> or structure to be expanded rapidly (in the style of Insight or Visual
> Studio).  The existing behaviour of -var-list-children is preserved of 
> backward
> compatibility.
>
> Nick
>
>
> 2003-10-25  Nick Roberts  <nick@nick.uklinux.net>
>
> 	* mi-cmd-var.c (mi_cmd_var_list_children): Print the values of the
> 	children, if required.
>
>
> *** mi-cmd-var.c.~1.16.~	2003-02-02 06:24:04.000000000 +0000
> --- mi-cmd-var.c	2003-10-25 17:03:39.000000000 +0100
> ***************
> *** 255,265 ****
>     struct varobj **childlist;
>     struct varobj **cc;
>     struct cleanup *cleanup_children;
> !   int numchild;
>     char *type;
>
> !   if (argc != 1)
> !     error ("mi_cmd_var_list_children: Usage: NAME.");
>
>     /* Get varobj handle, if a valid var obj name was specified */
>     var = varobj_get_handle (argv[0]);
> --- 255,265 ----
>     struct varobj **childlist;
>     struct varobj **cc;
>     struct cleanup *cleanup_children;
> !   int numchild, values;
>     char *type;
>
> !   if (argc != 1 && argc != 2)
> !     error ("mi_cmd_var_list_children: Usage: NAME [PRINT_VALUES]");
>
>     /* Get varobj handle, if a valid var obj name was specified */
>     var = varobj_get_handle (argv[0]);
> ***************
> *** 268,273 ****
> --- 268,275 ----
>
>     numchild = varobj_list_children (var, &childlist);
>     ui_out_field_int (uiout, "numchild", numchild);
> +   if (argc == 2) values = atoi (argv[1]);
> +   else values = 0;
>
>     if (numchild <= 0)
>       return MI_CMD_DONE;
> ***************
> *** 284,289 ****
> --- 286,293 ----
>         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 (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)
>
>
>
--
Jim Ingham                                   jingham@apple.com
Developer Tools
Apple Computer

Re: PATCH (gdb/mi)

[Eli Zaretskii]

> From: Jim Ingham <jingham@apple.com>
> Date: Mon, 27 Oct 2003 11:03:25 -0800
> 
> Eli will probably tell you that you need to update the MI doc's.

Thanks, that's a good catch.  Yes, I should have told Nick that when
I saw the patch.

Re: PATCH (gdb/mi)

[Andrew Cagney]

> Nick,
> 
> Yes, this is what our local version does for the -var-list-children command ('cept we call the variable print_value not values...), and it seems to work well.

Jim, did Apple add the needed doc and testcase?

Nick, MI changes need:

- assignment
- testcase
- doco
- patch

which is more than the average patch.  This is part of trying to ensure 
that GDB doesn't end up with un-documented un-tested MI mechanisms.

> Eli will probably tell you that you need to update the MI doc's.
> 
> Jim

Re: PATCH (gdb/mi)

[Jim Ingham]

We have added/modified some testcases, but until recently we have not 
been good about this.  Jason went through and added some about 6 months 
ago, but they are not comprehensive.   So while not good, we are not as 
bad about tests as we have been about updating the docs :-)

We have made a goodly number of MI changes, but mostly under the gun, 
and so we tended not to have the time to do the "extras".  It will take 
some work to fill in these bits before we would be able to submit our 
changes.  But they are still a useful source of ideas for other folks 
working on the MI...

Jim

On Oct 30, 2003, at 1:01 PM, Andrew Cagney wrote:

>> Nick,
>> Yes, this is what our local version does for the -var-list-children 
>> command ('cept we call the variable print_value not values...), and 
>> it seems to work well.
>
> Jim, did Apple add the needed doc and testcase?
>
> Nick, MI changes need:
>
> - assignment
> - testcase
> - doco
> - patch
>
> which is more than the average patch.  This is part of trying to 
> ensure that GDB doesn't end up with un-documented un-tested MI 
> mechanisms.
>
>> Eli will probably tell you that you need to update the MI doc's.
>> Jim
>
>
>
--
Jim Ingham                                   jingham@apple.com
Developer Tools
Apple Computer

Re: PATCH (gdb/mi)

[Nick Roberts]

 > Nick, MI changes need:
 > 
 > - assignment

Who initiates this? I have done it for Emacs. Is it the same process for GDB
i.e REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES to fsf-records@gnu.org

 > - testcase

This is harder for me to do than the patch itself, so perhaps you can give me
a few clues.  Will two tests for each of mi2-var-child.exp and mi1-var-child.exp
using "-var-list-children struct_declarations 0" and "-var-list-children
struct_declarations 1" suffice ?

 > - doco

below

 > - patch

Already sent.

If Jim can give you equivalent changes in the near future then that might be
the most simple route. If not, I would like to progress my changes through
the system. 


Nick                                               http://www.nick.uklinux.net



*** gdb.texinfo.~1.183.~	2003-10-24 22:30:49.000000000 +0100
--- gdb.texinfo	2003-11-01 17:14:02.000000000 +0000
***************
*** 18121,18134 ****
  @subsubheading Synopsis
  
  @smallexample
!  -var-list-children @var{name}
  @end smallexample
  
! Returns a list of the children of the specified variable object:
  
  @smallexample
   numchild=@var{n},children=[@{name=@var{name},
   numchild=@var{n},type=@var{type}@},@r{(repeats N times)}]
  @end smallexample
  
  
--- 18121,18145 ----
  @subsubheading Synopsis
  
  @smallexample
!  -var-list-children @var{name} [PRINT-VALUES]
  @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
! second argument of 0, prints only the names of the variables.  With an
! optional second argument of 1, 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 n 1
+  numchild=@var{n},children=[@{name=@var{name},
+  numchild=@var{n},value=@var{VALUE},type=@var{type}@},@r{(repeats N times)}]
  @end smallexample
  
  

Re: PATCH (gdb/mi)

[Andrew Cagney]

>  > Nick, MI changes need:
>  > 
>  > - assignment
> 
> Who initiates this? I have done it for Emacs. Is it the same process for GDB
> i.e REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES to fsf-records@gnu.org

[I've sent nick the form, I thought it had already been sent :-(]

>  > - testcase
> 
> This is harder for me to do than the patch itself, so perhaps you can give me
> a few clues.  Will two tests for each of mi2-var-child.exp and mi1-var-child.exp
> using "-var-list-children struct_declarations 0" and "-var-list-children
> struct_declarations 1" suffice ?

Here I was hopeing that JimI could dig something up.

> If Jim can give you equivalent changes in the near future then that might be
> the most simple route. If not, I would like to progress my changes through
> the system. 

Yes on both counts.  Or I progress something.

> *** gdb.texinfo.~1.183.~	2003-10-24 22:30:49.000000000 +0100
> --- gdb.texinfo	2003-11-01 17:14:02.000000000 +0000
> ***************
> *** 18121,18134 ****
>   @subsubheading Synopsis
>   
>   @smallexample
> !  -var-list-children @var{name}
>   @end smallexample
>   
> ! Returns a list of the children of the specified variable object:
>   
>   @smallexample
>    numchild=@var{n},children=[@{name=@var{name},
>    numchild=@var{n},type=@var{type}@},@r{(repeats N times)}]
>   @end smallexample
>   
>   
> --- 18121,18145 ----
>   @subsubheading Synopsis
>   
>   @smallexample
> !  -var-list-children @var{name} [PRINT-VALUES]
>   @end smallexample

I think the syntax should be:

-var-list-children --print-values @var{name}

I'll see what I can do.

Andrew

Re: PATCH (gdb/mi)

[Eli Zaretskii]

> From: Nick Roberts <nick@nick.uklinux.net>
> Date: Sat, 1 Nov 2003 17:33:01 +0000
> 
>  > - doco
> 
> below

Approved, but please correct this small blunder:

>   @smallexample
> !  -var-list-children @var{name} [PRINT-VALUES]
>   @end smallexample

Since PRINT-VALUES stands for something else, you should use @var,
like this:

   @smallexample
 !  -var-list-children @var{name} [@var{print-values}]
   @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
> ! second argument of 0, prints only the names of the variables.  With an
> ! optional second argument of 1, also prints their values.

If print-values can be only 0 or 1, perhaps it's better to say

  -var-list-children @var{name} [0|1]

PATCH (gdb/mi)

[Nick Roberts]

Purpose:  

With variable objects, the value of array elements and structure members 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
each element/member. This patch adapts the MI command -var-list-children so
that all the values can be accessed at once allowing the display of an array
or structure to be expanded rapidly (in the style of Insight or Visual
Studio).  The existing behaviour of -var-list-children is preserved of backward
compatibility.

Nick


2003-10-25  Nick Roberts  <nick@nick.uklinux.net>

	* mi-cmd-var.c (mi_cmd_var_list_children): Print the values of the
	children, if required.


*** mi-cmd-var.c.~1.16.~	2003-02-02 06:24:04.000000000 +0000
--- mi-cmd-var.c	2003-10-25 17:03:39.000000000 +0100
***************
*** 255,265 ****
    struct varobj **childlist;
    struct varobj **cc;
    struct cleanup *cleanup_children;
!   int numchild;
    char *type;
  
!   if (argc != 1)
!     error ("mi_cmd_var_list_children: Usage: NAME.");
  
    /* Get varobj handle, if a valid var obj name was specified */
    var = varobj_get_handle (argv[0]);
--- 255,265 ----
    struct varobj **childlist;
    struct varobj **cc;
    struct cleanup *cleanup_children;
!   int numchild, values;
    char *type;
  
!   if (argc != 1 && argc != 2)
!     error ("mi_cmd_var_list_children: Usage: NAME [PRINT_VALUES]");
  
    /* Get varobj handle, if a valid var obj name was specified */
    var = varobj_get_handle (argv[0]);
***************
*** 268,273 ****
--- 268,275 ----
  
    numchild = varobj_list_children (var, &childlist);
    ui_out_field_int (uiout, "numchild", numchild);
+   if (argc == 2) values = atoi (argv[1]);
+   else values = 0;
  
    if (numchild <= 0)
      return MI_CMD_DONE;
***************
*** 284,289 ****
--- 286,293 ----
        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 (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)