Re: [PATCH] -stack-info-frames

[Daniel Jacobowitz <drow@false.org>]

On Sun, Jun 19, 2005 at 03:39:42PM +1200, Nick Roberts wrote:
>  > > Yes, unless Apple's proposed merge will provide the necessary information.
>  > 
>  > Let's not wait on that.  You've demonstrated a use for
>  > -stack-info-frame relative to the current source base.  That's plenty
>  > good enough for me.
>  > 
>  > Of course now we need to re-add the documentation (with example this
>  > time).  A test case would be nice too.  Since we've decided that we do
>  > want this feature, could you put that together?
> 
> 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.

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

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

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

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


-- 
Daniel Jacobowitz
CodeSourcery, LLC