Re: [PATCH] -stack-info-frames

Nick Roberts
2005-06-19T22:31:51Z

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