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


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

Re: [PATCH] -stack-info-frames


 > > OK.  I've committed the -stack-info-frame part of the change that posted
 > > (Sat, 18 Jun 2005 10:52:09 +1200).  Perhaps that doesn't follow the
 > > letter of the law but I hope it follows the spirit.  In any case, I find
 > > it easier to make further changes to the repository than juggle patches
 > > (as demonstrated shown with my earlier mangling).
 > 
 > No, Nick.  You don't get to make up rules as you go along, no matter
 > how much the current ones irk you.  That patch was never reviewed and
 > never approved.  Don't do that again; if you won't wait for approval,
 > we'll remove you from write-after-approval.

I wasn't trying to make up the rules, just interpret them.  I posted a very
similar patch earlier which was reviewed.  As no branch/release is imminent,
it seemed a safe thing to do.  I know not to apply judgement again.  Sorry.

 > If you have trouble juggling patches, have a complete checkout for each
 > independent project you are working on.  That's not hard to do.

I'll have to work out a new routine.  Contributing to Emacs works differently.

 > +enum mi_cmd_result
 > +mi_cmd_stack_info_frame (char *command, char **argv, int argc)
 > +{
 > +  if (argc > 0)
 > +    error (_("mi_cmd_stack_info_frame: No arguments required"));
 > +
 > +  print_frame_info (get_selected_frame (NULL), 1, LOC_AND_ADDRESS, 0);
 > +  return MI_CMD_DONE;
 > +}
 > 
 > "No arguments required" doesn't make much sense as an error message; it
 > suggests that no arguments are necessary, but not that any arguments
 > are invalid.  But I see there are two uses of it already, and none of
 > any other format for functions which take no arguements.  So the code
 > parts of the patch are belatedly OK...

Where possible, I just copy what is already there.

 > > This commit is slightly different in two respects:
 > > 
 > > 1) mi_cmd_stack_info_frame uses print_frame_info instead of
 > >    print_stack_frame.  This follows mi_cmd_stack_list_frames and means
 > >    that the argument values aren't printed.
 > > 
 > > 2) The documentation for -stack-info-frame previously said (before I
 > >    removed it) "Get info on the current frame.".  I've corrected this to
 > >    "Get info on the selected frame."  I've also removed the argument
 > >    values from the example as explained in 1).
 > 
 > Despite the fact that you made it up as you went along.  Why did you
 > decide that this change was a better idea?

Which change?

 > The documentation is up to Eli, but I can say with some confidence that
 > it is NOT ok, since you didn't really remove argument values from the
 > examples.  I still see one:
 > 
 > > + @smallexample
 > > + (@value{GDBP})
 > > + -stack-info-frame
 > > + ^done,frame=@{level="1",addr="0x0001076c",func="callee3",
 > > + args=[@{name="strarg",value="0x11940 \"A string argument.\""@}],
 > > + file="../../../devo/gdb/testsuite/gdb.mi/basics.c",
 > > + fullname="/home/foo/bar/devo/gdb/testsuite/gdb.mi/basics.c",line="17"@}
 > > + (@value{GDBP})
 > > + @end smallexample
 > > + 

The patch was OK but the diff wasn't.  I picked up the backup copy by mistake:

*** /home/nick/src/gdb/doc/gdb.texinfo.~1.269~	2005-06-19...
--- /home/nick/src/gdb/doc/gdb.texinfo~	2005-06-19...
***************
                                     ^^^

Nick


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